fix(stats): record noop routes in /v1/routing/stats#109
Conversation
WalkthroughThe no-op profile now accepts and propagates an optional ChangesNo-op routing statistics
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_noop_llm_backend.py (1)
110-124: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the noop token counters as well.
This regression only checks request and call counts. It would still pass if token statistics were dropped or recorded under another model. Add assertions for the accumulator’s prompt, completion, and total-token fields using the expected noop values.
🤖 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 `@tests/test_noop_llm_backend.py` around lines 110 - 124, Extend test_noop_call_reports_routing_stats to assert the noop model’s prompt-token, completion-token, and total-token counters in snap["models"]["noop"], using the expected noop values and confirming token statistics are recorded under the noop model.
🤖 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 `@switchyard/lib/profiles/noop.py`:
- Around line 223-225: Move the _stats_accumulator.record_success call in the
noop request flow to after await self.rprocess(...), ensuring request
translation, response construction, and response processing complete
successfully before recording. Keep the existing _NOOP_MODEL arguments and only
record success for fully completed calls.
---
Nitpick comments:
In `@tests/test_noop_llm_backend.py`:
- Around line 110-124: Extend test_noop_call_reports_routing_stats to assert the
noop model’s prompt-token, completion-token, and total-token counters in
snap["models"]["noop"], using the expected noop values and confirming token
statistics are recorded under the noop model.
🪄 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: 255f39dc-7318-4d8f-bf68-fb8d817bff3e
📒 Files selected for processing (2)
switchyard/lib/profiles/noop.pytests/test_noop_llm_backend.py
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com> (cherry picked from commit 1a0c5cf) Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
2eabc4f to
3e68f8a
Compare
Live before/after — the noop route now records routing statsRan the real route-bundle serving path on both Bundle ( defaults: { api_key: unused, base_url: https://openrouter.ai/api/v1, format: openai }
routes:
bench:
type: noopRequest (identical on both): Before ( After (this branch) — the call is counted and recorded under |
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
Before this change, a
type: nooproute never showed up inGET /v1/routing/stats. The no-op profile returned its fixed "pong" response without telling the stats accumulator which model served the request, so the response-side stats processor bucketed the tokens under<unknown>and never counted the call.total_requestsstayed at 0 and there was nonoopentry.Now
run_with_contextsetsctx.selected_model = "noop"and callsrecord_success("noop")before it builds the response. The call is counted once:record_successbumpstotal_requestsand the model'scalls, and the response processor only adds tokens. So the route shows up as thenoopmodel with its request count and token totals.This mirrors how the latency-service backend already reports itself: set
ctx.selected_modelso the RustStatsResponseProcessorattributes usage to the right model, plusrecord_successso the call is counted.This is the route-bundle / route-table serving path (the same path
switchyard launchbuilds). Serving a Python-defined profile throughserve --configuses a separate adapter that does not attach a routing-stats accumulator, so that path is unaffected here.How to try it — serve a route bundle with a
nooproute, send one request, then read the stats:On this branch
/v1/routing/statsshowstotal_requests: 1and anoopmodel withcalls: 1and its token totals; onmainit showstotal_requests: 0and an<unknown>bucket instead.Added
test_noop_call_reports_routing_stats, which builds the profile through the samewith_runtime_components(...)path the route bundle uses and assertstotal_requests == 1, anoopmodel entry withcalls == 1and token totals1/1/2, and no<unknown>bucket.