feat(dashboard): surface failing requests so admins see dropped traffic - #449
feat(dashboard): surface failing requests so admins see dropped traffic#449njbrake wants to merge 3 commits into
Conversation
Requests the gateway itself refused left no trace. The missing-pricing gate refunded the budget reservation and raised its 402 without writing a usage row, so an operator who turned require_pricing on had no way to see that live traffic was being dropped; they found out when a user complained. Every admin view was blind to it: the activity log, the overview error rate, and the recent-activity list all showed nothing. Log the rejection at both missing-pricing gates (the chat, messages, and responses pipeline, and the pass-through routes behind embeddings, images, and rerank) with status=error and cost=null. The reservation refund is unchanged, so nothing is billed and nothing stays reserved; the row exists to make the drop visible and countable. The pricing alarm then reads that count: "N requests failed in the last hour", plus a link into the activity log filtered to those failures, so the reject state reads as an active incident rather than a static config note. A new useFailureCount hook polls it through TanStack Query and resolves the window inside queryFn rather than in the query key, which keeps the key stable and re-anchors on every refetch, so a tab left open keeps reporting the last hour instead of widening to everything since it was opened. Every failure class is counted, not only the pricing rejections: the question in this state is whether traffic is getting through, and over-reporting a failure beats a banner reading "0" while requests are dropping. The activity detail panel's error summary no longer claims the provider returned the error. That was accurate while only provider failures were logged; gateway-side rejections land in that list now, so the copy is source-neutral. Provider diagnostics still stay server-side. Fixes #317
GitHub Actions
Inactive
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe gateway now records missing-pricing rejections as usage errors. The dashboard polls and displays recent gateway failures, supports source filtering in Activity, updates related documentation, and refreshes bundled dashboard assets. ChangesFailure visibility
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/dashboard.md`:
- Around line 148-150: Update the Activity view description in docs/dashboard.md
to remove the claim that filtering by error status shows failure reasons. State
that failed requests are visible in Activity, while diagnostic details remain
available in the gateway logs.
🪄 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: Pro Plus
Run ID: 26dcc8db-da56-4d2b-90ad-832f4d752f00
📒 Files selected for processing (34)
docs/dashboard.mdsrc/gateway/api/routes/_passthrough.pysrc/gateway/api/routes/_pipeline.pysrc/gateway/static/dashboard/assets/ActivityPage-D2jsRt9a.jssrc/gateway/static/dashboard/assets/ActivityPage-DIACtZv5.jssrc/gateway/static/dashboard/assets/AliasesPage-CrabMwlU.jssrc/gateway/static/dashboard/assets/BudgetsPage-DjVks5Lb.jssrc/gateway/static/dashboard/assets/ConfirmDialog-KQrh5qGO.jssrc/gateway/static/dashboard/assets/DocsPage-Bo_X9aMz.jssrc/gateway/static/dashboard/assets/FilterChips-BbDynOdb.jssrc/gateway/static/dashboard/assets/FilterChips-DZjxf-ot.jssrc/gateway/static/dashboard/assets/KeysPage--hq_RPOL.jssrc/gateway/static/dashboard/assets/ModelScopeControl-DYPdGezt.jssrc/gateway/static/dashboard/assets/ModelsPage-Dt9ZAYGy.jssrc/gateway/static/dashboard/assets/OverviewPage-BedTU_aZ.jssrc/gateway/static/dashboard/assets/OverviewPage-DPI1n-nt.jssrc/gateway/static/dashboard/assets/ProvidersPage-BP60GuZv.jssrc/gateway/static/dashboard/assets/ProvidersPage-CC2Glu9P.jssrc/gateway/static/dashboard/assets/SettingsPage-CYDMBdEO.jssrc/gateway/static/dashboard/assets/SettingsPage-DR0J2MHH.jssrc/gateway/static/dashboard/assets/TablePagination-BR55oZ8k.jssrc/gateway/static/dashboard/assets/ToolsGuardrailsPage-B68S03Js.jssrc/gateway/static/dashboard/assets/UsagePage-BYqtE3tC.jssrc/gateway/static/dashboard/assets/UsagePage-Ce_jAzFl.jssrc/gateway/static/dashboard/assets/UsersPage-CZXIBvdD.jssrc/gateway/static/dashboard/assets/index-Br3lxdY5.jssrc/gateway/static/dashboard/assets/index-DV8qvdUj.jssrc/gateway/static/dashboard/index.htmltests/integration/test_require_pricing.pyweb/src/api/hooks.tsweb/src/components/PricingWarning.test.tsxweb/src/components/PricingWarning.tsxweb/src/pages/ActivityPage.test.tsxweb/src/pages/ActivityPage.tsx
💤 Files with no reviewable changes (7)
- src/gateway/static/dashboard/assets/UsagePage-Ce_jAzFl.js
- src/gateway/static/dashboard/assets/SettingsPage-DR0J2MHH.js
- src/gateway/static/dashboard/assets/ActivityPage-DIACtZv5.js
- src/gateway/static/dashboard/assets/FilterChips-DZjxf-ot.js
- src/gateway/static/dashboard/assets/ProvidersPage-BP60GuZv.js
- src/gateway/static/dashboard/assets/index-DV8qvdUj.js
- src/gateway/static/dashboard/assets/OverviewPage-BedTU_aZ.js
| Requests the gateway refused are logged too, so filtering to the `error` | ||
| status shows what is being dropped and why (for example a model with no | ||
| pricing under `require_pricing`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not claim the Activity view shows failure reasons.
Line 148 says the error filter shows why traffic was dropped, but the detail panel deliberately renders only “This request failed” and directs operators to gateway logs. State that failed requests are visible in Activity and that diagnostics remain in gateway logs.
🤖 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 `@docs/dashboard.md` around lines 148 - 150, Update the Activity view
description in docs/dashboard.md to remove the claim that filtering by error
status shows failure reasons. State that failed requests are visible in
Activity, while diagnostic details remain available in the gateway logs.
The count read every status=error row, but imported usage carries that status too: /v1/usage/external-events documents "error" as a first-class value. So a Claude Code session with 5 failed calls made the banner claim the gateway had dropped 5 requests it never saw. Importing 5 failures alongside 2 real 402s reported 7 instead of 2, turning an incident signal into a false alarm. Scope the count to source=gateway, which is sound because RESERVED_SOURCES forbids an import from claiming that source. Carry the same scope on the drill-down link and teach the activity page to honor a source param, so the filtered view reports the number the banner just claimed instead of a larger one. The param has no select of its own (it arrives from the drill-down), so it surfaces as a clearable chip rather than as hidden filter state.
GitHub Actions
Inactive
The Activity entry said filtering to the error status shows what is being dropped "and why", but the request detail panel deliberately renders only "This request failed. Inspect gateway logs for details.": it cannot tell a safe gateway message from a raw provider one, so it shows neither. The doc promised something the UI withholds by design. Say that refused requests are visible there, and that diagnostic detail stays in the gateway logs. Flagged by CodeRabbit on #449.
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR improves operator visibility into dropped traffic by ensuring gateway-side missing-pricing rejections are recorded in UsageLog as failed requests (with cost=null), and by surfacing a live “failed in the last hour” signal in the dashboard’s pricing warning with a drill-down link into Activity filtered to those failures.
Changes:
- Log
require_pricingmissing-pricing rejections asstatus="error"usage rows (budget reservation still refunded; no spend recorded). - Add a polling
useFailureCounthook and extend the PricingWarning banner to show last-hour failure count + link to filtered Activity. - Make Activity’s failure summary source-neutral and add a URL-driven
sourcefilter displayed as a clearable chip; update docs accordingly.
Reviewed changes
Copilot reviewed 26 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/pages/ActivityPage.tsx | Adds source URL filter support (chip-only), threads it through usage queries, and updates failure copy to be source-neutral. |
| web/src/pages/ActivityPage.test.tsx | Tests the source drill-down param and validates source-neutral error summary text. |
| web/src/components/PricingWarning.tsx | Adds live last-hour failure count (polled) and a drill-down link to Activity scoped to gateway failures. |
| web/src/components/PricingWarning.test.tsx | Adds coverage for polling behavior, window re-anchoring, link scope, and quiet-alarm behavior. |
| web/src/api/hooks.ts | Introduces useFailureCount using TanStack Query polling and an anchored rolling window resolved inside queryFn. |
| tests/integration/test_require_pricing.py | Adds integration regression tests asserting missing-pricing 402s are logged as error usage rows with cost=null. |
| src/gateway/api/routes/_pipeline.py | Logs missing-pricing rejections (chat/messages/responses pipeline) before raising 402, without changing refund behavior. |
| src/gateway/api/routes/_passthrough.py | Logs missing-pricing rejections for pass-through routes (e.g. embeddings) before raising 402, without changing refund behavior. |
| docs/dashboard.md | Documents that gateway-refused requests are also visible in Activity (with diagnostics remaining server-side). |
| src/gateway/static/dashboard/index.html | Updates dashboard bundle entrypoint hash. |
| src/gateway/static/dashboard/assets/UsagePage-Ce_jAzFl.js | Removes old built dashboard chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/UsagePage-5E7SwtJ3.js | Adds refreshed built dashboard chunk reflecting new UI behavior. |
| src/gateway/static/dashboard/assets/ToolsGuardrailsPage-B2ogbWmR.js | Bundle refresh referencing new index chunk hash. |
| src/gateway/static/dashboard/assets/TablePagination-BnFQxBEH.js | Bundle refresh referencing new index chunk hash. |
| src/gateway/static/dashboard/assets/SettingsPage-eR11APLm.js | Adds refreshed built Settings page chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/SettingsPage-DR0J2MHH.js | Removes old built Settings page chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/ProvidersPage-FBow933C.js | Adds refreshed built Providers page chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/ProvidersPage-BP60GuZv.js | Removes old built Providers page chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/OverviewPage-KEawx4D6.js | Adds refreshed built Overview page chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/OverviewPage-BedTU_aZ.js | Removes old built Overview page chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/ModelScopeControl-CIHD_icU.js | Bundle refresh referencing new index chunk hash. |
| src/gateway/static/dashboard/assets/FilterChips-DZjxf-ot.js | Removes old built FilterChips chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/FilterChips-BcnyddEO.js | Adds refreshed built FilterChips chunk (bundle refresh). |
| src/gateway/static/dashboard/assets/ConfirmDialog-DCntQ5Rx.js | Bundle refresh referencing new index chunk hash. |
| src/gateway/static/dashboard/assets/AliasesPage-CdcERvTp.js | Bundle refresh referencing new index chunk hash. |
| src/gateway/static/dashboard/assets/ActivityPage-DPTftSxE.js | Adds refreshed built Activity page chunk reflecting new source filter and source-neutral copy. |
| src/gateway/static/dashboard/assets/ActivityPage-DIACtZv5.js | Removes old built Activity page chunk (bundle refresh). |
Files not reviewed (7)
- src/gateway/static/dashboard/assets/ActivityPage-DPTftSxE.js: Generated file
- src/gateway/static/dashboard/assets/FilterChips-BcnyddEO.js: Generated file
- src/gateway/static/dashboard/assets/OverviewPage-KEawx4D6.js: Generated file
- src/gateway/static/dashboard/assets/ProvidersPage-FBow933C.js: Generated file
- src/gateway/static/dashboard/assets/SettingsPage-eR11APLm.js: Generated file
- src/gateway/static/dashboard/assets/UsagePage-5E7SwtJ3.js: Generated file
- src/gateway/static/dashboard/assets/index-C3aWlAKW.js: Generated file
khaledosman
left a comment
khaledosman
left a comment
There was a problem hiding this comment.
The premise correction is the valuable part here: the issue assumed the data already existed and it did not. A banner over status=error without the backend write would have read "0 failed" at precisely the moment traffic was being dropped. Catching that before building the UI, and covering both gates rather than only the one the issue names, is the right call.
I checked the budget-safety property this change could plausibly have broken: both gates are guarded by not budget_exempt, so counts_toward_budget=not budget_exempt is always True on these rows. They cannot be mistaken for imported rows, which means bulk delete and set-price cannot reach them and the source=gateway count does not miss them. That invariant deserves a test (see inline).
Five non-blocking notes inline.
Landing order: this, #450, and #451 all edit docs/dashboard.md and rewrite the same hashed bundle assets, so whoever lands second needs a rebase plus npm --prefix web run build.
🤖 Generated with Claude Code
| # Record the rejection so dropped traffic is visible in the activity | ||
| # log and countable as an error, rather than only reaching the | ||
| # operator as a user complaint. cost stays null: nothing was spent. | ||
| await log_writer.put( |
There was a problem hiding this comment.
This is the third hand-built UsageLog literal in this file (also the success row at ~263 and the provider-error row at ~290), all eleven fields repeated. A local _usage_row(status, **overrides) helper would make the next field addition a one-line change instead of three sites to keep in sync.
| api_key_id=api_key_id, | ||
| user_id=user_id, | ||
| timestamp=datetime.now(UTC), | ||
| model=resolved.model, |
There was a problem hiding this comment.
Logs model=resolved.model (bare, e.g. text-embedding-3-small) where _pipeline.py logs model=model, the full selector (openai:gpt-4o). Each matches its own file's success path, so this PR does not introduce the split, but it does extend it to the new error rows: in the Activity log a rejected embeddings row and a rejected chat row render their Model column differently, and the model filter matches one form or the other.
Not necessarily worth changing here, but the new tests do not pin it either (the chat test asserts the model string, the passthrough one does not), so asserting the expected embeddings model would at least record which form is intended.
|
|
||
| rows = c.get("/v1/usage", params={"status": "error"}, headers=_MASTER_HEADER).json() | ||
| assert len(rows) == 1 | ||
| assert rows[0]["endpoint"] == "/v1/embeddings" |
There was a problem hiding this comment.
Missing the case that guards the invariant this change depends on: a budget-exempt key skips the gate entirely, so no error row is written and no spend moves. The PR description's smoke table covers it (502, not 402), but this is the one place where a future refactor of the budget_exempt condition would silently start writing rows with counts_toward_budget=False, which the dashboard then classifies as imported and offers up for bulk delete. Cheap to pin here.
| staleTime: 0, | ||
| // A failed count is not worth surfacing: it sits beside its own alarm, and | ||
| // the next poll retries anyway. | ||
| retry: false, |
There was a problem hiding this comment.
Resolving the window inside queryFn rather than in the key is the right call and the re-anchoring test pins it well.
One consequence worth confirming is intended: retry: false plus staleTime: 0 means a transient failure leaves failures.data on the last successful value until the next 60s tick, so the banner can display a stale count for up to a minute after the gateway starts erroring. Advisory number, so probably fine, but it is the opposite of failing loud.
| <strong className="font-semibold"> | ||
| {failureCount.toLocaleString()} {failureCount === 1 ? "request" : "requests"} failed in the last hour. | ||
| </strong>{" "} | ||
| <NavLink to="/activity?status=error&range=1h&source=gateway" className="underline underline-offset-2"> |
There was a problem hiding this comment.
NavLink is Link plus active-state styling, and the active state can never apply here (the banner is never rendered on the activity route it points at). Link expresses the intent without the dead machinery.
Description
An admin had no way to see that the gateway was dropping traffic. The
require_pricingbanner told you the config was in a reject state, but not that live requests were actually
failing, or how many.
The premise in the issue was that the data already existed and only needed surfacing. It
turned out that was not quite true for the case the issue leads with. The missing-pricing
gate refunded the budget reservation and raised the 402 without writing a usage row
(
_pipeline.py), so a gateway-side rejection left no trace anywhere. A banner countingstatus=errorwould have read "0 failed" at the exact moment unpriced models were beingrejected. Every admin view was blind to it: the activity log, the overview error rate, and
the recent-activity list all showed nothing.
So this is two small changes:
Record the rejection. Both missing-pricing gates (the chat/messages/responses pipeline
and the pass-through routes used by embeddings, images, and rerank) now log a
status="error"usage row withcost=nullbefore raising the 402. The reservation refundis unchanged, so nothing is billed and nothing stays reserved; the row exists purely to
make the drop visible and countable.
Read that count in the banner. The pricing alarm now shows "N requests failed in the
last hour" with a link into the activity log filtered to those failures
(
/activity?status=error&range=1h), so the reject state reads as an active incidentrather than a static config note. It is served by a new
useFailureCounthook that pollsthrough TanStack Query's
refetchInterval. The window is resolved insidequeryFnratherthan in the query key, which keeps the key stable (a "now"-derived key would mint a new
cache entry on every render) and re-anchors on every refetch, so a tab left open keeps
reporting the last hour instead of quietly widening to everything since it was opened. The
query only runs while the alarm is up.
The count deliberately covers every failure class, not only the pricing rejections. The
operator's question in this state is "is traffic getting through", and over-reporting a
failure is safer than a banner reading "0" while requests are being dropped.
One knock-on fix: the activity detail panel's error summary said "The provider returned an
error." That was accurate when only provider failures were logged. Gateway-side rejections
land in that list now, so the copy is source-neutral ("This request failed."). Provider
diagnostics still stay server-side.
Not included: the issue also floats an "allow unpriced models" toggle in Settings. The
banner's existing "Enable default pricing" button already is that one-click mitigation, so
nothing was added for it.
Verification
Beyond the test suites, this was smoke tested against a running gateway with
require_pricingon and a deliberately fake provider key (the gate rejects beforedispatch, so no real credential is needed):
402 402 402402cost=null/v1/usage/count?status=error&start_date=…{"total": 4}total_spend=0.0,total_reserved=0.0502, not402502, past the gateIn the browser: the banner reported the live count, the link landed on the activity log
with the 1h preset and the Status: Error filter pre-applied, and the count updated itself
from 2 to 7 in 57 seconds with no reload. The Overview page's "Needs attention" strip, the
error-rate tile, and Recent activity all populated as a side effect, having been silent
before.
PR Type
Relevant issues
Fixes #317
Part of #299. Adjacent to #303 (usage and analytics) and #304 (request log viewer): showing
a safe per-row failure reason in the detail panel needs a way to tell a gateway message
from a raw provider message, which belongs with that work rather than here.
Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).Tests added: two integration cases in
tests/integration/test_require_pricing.py(the chatgate and the pass-through gate), both confirmed to fail without the backend change, and
five dashboard cases in
PricingWarning.test.tsxincluding one that pins the re-anchoringbehavior (confirmed to fail against a frozen-window implementation). The OpenAPI spec is
unchanged: no route signature or schema moved.
AI Usage
AI Model/Tool used:
Claude Opus 5 via Claude Code.
Any additional AI details you'd like to share:
Written by Claude working from the issue, then reviewed and smoke tested with @njbrake in
the loop. The decision to add the backend logging (rather than ship a banner that would
have counted zero) and the scope boundary around the detail-panel reason were discussed
rather than assumed.
NOTE:
When responding to reviewer questions, please respond yourself rather than copy/pasting reviewer comments into an AI and pasting back its answer. We want to discuss with you, not your AI :)
Summary
Benefits
Administrators can now identify failed requests in the dashboard and investigate missing-pricing rejections without losing reservation refunds.
Testing
Added coverage for pricing-gate and pass-through logging, failure-count polling, Activity filtering, and dashboard messaging.