docs: Update Benchmark README with updated configs - #1905
Conversation
Documentation preview |
Greptile SummaryThis PR updates the benchmark quickstart guide to replace the old standalone
|
| Filename | Overview |
|---|---|
| benchmark/README.md | Restructured quickstart guide: replaces the old venv/requirements.txt workflow with a poetry-based server setup and a new AIPerf client section; also fixes a documentation bug in the latency clamping description (less → greater than LATENCY_MAX_SECONDS) |
| benchmark/Procfile | Appended /v1 to MAIN_MODEL_BASE_URL to align with the upstream fix for duplicated /v1 URL paths from #1862 |
| benchmark/requirements.txt | Deleted; server-side dependencies now installed via poetry, and the aiperf client tool is installed separately in its own venv |
| examples/configs/content_safety_local/config.yml | Both model base_urls updated to include /v1 (localhost:8000/v1 and localhost:8001/v1), consistent with the Procfile change and the #1862 URL-path fix |
Sequence Diagram
sequenceDiagram
participant AIPerf as AIPerf Client (aiperf_env)
participant GR as Guardrails Server :9000
participant AppLLM as Mock App LLM :8000
participant CSLLM as Mock CS LLM :8001
Note over GR,CSLLM: Terminal 1 - honcho start (benchmark dir)
Note over AIPerf: Terminal 2 - repo root (aiperf_env)
AIPerf->>GR: POST /v1/chat/completions
GR->>CSLLM: POST /v1/chat/completions (content safety)
CSLLM-->>GR: safe/unsafe response
GR->>AppLLM: POST /v1/chat/completions (main inference)
AppLLM-->>GR: mock response
GR-->>AIPerf: final response
Note over AIPerf: results saved to aiperf_results/
Reviews (4): Last reviewed commit: "Fix copy-paste error with LATENCY_MAX_SE..." | Re-trigger Greptile
WalkthroughThis PR updates benchmark infrastructure and model configuration. The Procfile now explicitly passes ChangesBenchmark Setup and Model Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
benchmark/README.md (1)
179-179:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCorrect the latency max-bound condition description.
The max clamp condition is inverted in prose. For a max bound, the sampled value should be clamped when it is greater than
LATENCY_MAX_SECONDS.Suggested doc fix
-* If the sampled value is less than `LATENCY_MAX_SECONDS`, it is set to `LATENCY_MAX_SECONDS`. +* If the sampled value is greater than `LATENCY_MAX_SECONDS`, it is set to `LATENCY_MAX_SECONDS`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmark/README.md` at line 179, The documentation's description of the latency max clamp is inverted: update the sentence that references LATENCY_MAX_SECONDS so it reads that if a sampled latency is greater than LATENCY_MAX_SECONDS it is clamped to LATENCY_MAX_SECONDS (i.e., replace "less than `LATENCY_MAX_SECONDS`" with "greater than `LATENCY_MAX_SECONDS`" in the line describing the max clamp).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmark/README.md`:
- Around line 125-126: The sentence fragment "This makes requests against" in
benchmark/README.md is incomplete; update that sentence to state the target of
the requests and any relevant context (e.g., "This makes requests against the
local benchmark server at http://localhost:8080" or "This makes requests against
the deployed API endpoint to measure latency and throughput"), ensuring the
sentence follows surrounding instructions and clarifies what is being requested
and why.
- Around line 29-30: Replace the incorrect occurrences of the string "set ulimit
-n65536" in the README with the proper shell command "ulimit -n 65536" (remove
the "set" prefix and add a space between -n and the value); search for the exact
token "set ulimit -n65536" to find both occurrences and update them so the
quickstart steps correctly increase the file-descriptor limit.
---
Outside diff comments:
In `@benchmark/README.md`:
- Line 179: The documentation's description of the latency max clamp is
inverted: update the sentence that references LATENCY_MAX_SECONDS so it reads
that if a sampled latency is greater than LATENCY_MAX_SECONDS it is clamped to
LATENCY_MAX_SECONDS (i.e., replace "less than `LATENCY_MAX_SECONDS`" with
"greater than `LATENCY_MAX_SECONDS`" in the line describing the max clamp).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 56daaf89-9f3a-41db-a13f-2be16c376b32
📒 Files selected for processing (4)
benchmark/Procfilebenchmark/README.mdbenchmark/requirements.txtexamples/configs/content_safety_local/config.yml
💤 Files with no reviewable changes (1)
- benchmark/requirements.txt
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Happy to see the README is updated |
Description
This PR updates the top-level README with instructions on using the Mock LLMs, self-contained Procfile for server code, and AIPerf installation and use on the client-side. It more clearly describes where server-side and client-side dependencies are installed and run.
It also updates configs after merging #1862
Related Issue(s)
#1862 Fixes duplication of /v1 URLs, the configs in this PR are updated to reflect that.
Checklist
Summary by CodeRabbit
Documentation
Refactor