Conversation
…oads Four dashboard GETs dialed upstream providers on the request path, bounded only by model_discovery_timeout_seconds (10s per unreachable provider) and the models.dev fetch timeout (15s). Each held a browser connection and a pooled database session for the whole dial. Discovery and the models.dev catalog now refresh from background tasks, the same shape the alias, policy, provider and price caches already use, and the reads answer from cache. A provider never dialed still dials, so a cold worker never claims it has no models. ?refresh=true forces a live re-dial on /v1/models/discoverable and /v1/providers/health; /v1/models deliberately has no such flag, since it takes any API key and a provider-wide fanout is an operator action. Measured on a gateway with six unreachable providers: /v1/providers/health 10009ms to 14ms, /v1/models 8577ms to 11ms, /v1/models/discoverable 8577ms to 3ms. Separately, the Usage and Activity pages paged the entire users and api_keys tables on every visit, to name filter options and label rows. /v1/usage now resolves user_alias and api_key_name per row, and the by_user / by_api_key breakdowns carry a label resolved in the same GROUP BY, so both pages build their pickers from a summary they already request. Filter options become the in-window entities ranked by spend rather than every entity that ever existed. Also: no retry on the four discovery-backed dashboard queries, since the global default turned one slow failure into three, and a 30s bound in apiFetch so a hung request returns its connection slot on a known deadline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
had a problem deploying
to
integration-tests
August 7, 2026 12:54 — with 
GitHub Actions
Failure
GitHub Actions
Failure
github-actions
Bot
added
the
missing-template
|
Warning Review limit reached
Next review available in: 40 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (26)
WalkthroughThe change adds background refresh and stale-cache handling for model discovery and models.dev metadata, enriches usage responses with user and API-key labels, updates dashboard filtering and request handling, and regenerates dashboard assets. ChangesModel caching and refresh
Usage labels and analytics
Dashboard runtime and assets
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/UsagePage.tsx (1)
601-607: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse server labels for user groups.
Line 606 uses
row.labelonly forapi_key_id. The grouped-series endpoint also returns labels foruser_id. The User chart legend and User breakdown therefore show opaque IDs instead of aliases.Use
row.labelfor every non-null labeled group. Keep the key as the fallback. UpdateBreakdownTablewith the same fallback. Add tests for the User breakdown andgroup_by=user_id.Proposed fix
- : effectiveGroupBy === "api_key_id" - ? (row.label ?? `${row.key.slice(0, 8)}…`) - : row.key, + : row.label ?? (effectiveGroupBy === "api_key_id" ? `${row.key.slice(0, 8)}…` : row.key),- : row.key} + : row.label ?? row.key}🤖 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 `@web/src/pages/UsagePage.tsx` around lines 601 - 607, Update the group label selection in the UsagePage chart data and BreakdownTable so every non-null group uses row.label when available, regardless of whether effectiveGroupBy is api_key_id or user_id, and falls back to row.key otherwise; preserve the existing Other and unknown handling. Add coverage for the User breakdown and grouped-series requests using group_by=user_id.
🤖 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/configuration.md`:
- Line 84: Revise the model_cache_ttl_seconds documentation to describe cache
and refresh behavior without claiming every read dials when TTL is 0. Exclude
GET /v1/models/{model_id}, clarify that discovery endpoints may dial on
cold-cache requests, and state that provider health can reuse coalesced recent
checks rather than always dialing at request time.
In `@src/gateway/main.py`:
- Around line 184-194: Update the lifespan startup flow around
background_discovery_enabled and background_catalog_enabled so both refreshers
are also started when config.is_hybrid_mode is true, preventing indefinitely
stale cache results. Preserve the documented standalone behavior and add a
hybrid-lifespan regression test verifying the selected refresher-start behavior.
In `@web/src/api/client.ts`:
- Around line 91-101: Extend timeout handling in apiFetch and
extractErrorMessage to cover response-body reads, including response.json(),
while preserving existing ApiError handling and non-timeout JSON errors. Convert
AbortSignal timeout DOMException failures to the same descriptive ApiError used
for fetch timeouts, and add a regression test simulating headers arriving before
a stalled body.
---
Outside diff comments:
In `@web/src/pages/UsagePage.tsx`:
- Around line 601-607: Update the group label selection in the UsagePage chart
data and BreakdownTable so every non-null group uses row.label when available,
regardless of whether effectiveGroupBy is api_key_id or user_id, and falls back
to row.key otherwise; preserve the existing Other and unknown handling. Add
coverage for the User breakdown and grouped-series requests using
group_by=user_id.
🪄 Autofix
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: b9882c0c-f180-419c-9ffb-8bf5bbf272c6
⛔ Files ignored due to path filters (1)
docs/public/openapi.jsonis excluded by!docs/public/openapi.json
📒 Files selected for processing (43)
docs/configuration.mddocs/public/otari.postman_collection.jsonsrc/gateway/api/routes/models.pysrc/gateway/api/routes/providers.pysrc/gateway/api/routes/usage.pysrc/gateway/main.pysrc/gateway/services/model_catalog_service.pysrc/gateway/services/model_discovery_service.pysrc/gateway/services/provider_health_service.pysrc/gateway/static/dashboard/assets/ActivityPage-CfIPwmDz.jssrc/gateway/static/dashboard/assets/ActivityPage-zqcCQMke.jssrc/gateway/static/dashboard/assets/BudgetsPage-CXkIKoXy.jssrc/gateway/static/dashboard/assets/ConfirmDialog-h1eFJM-m.jssrc/gateway/static/dashboard/assets/DocsPage-gokYiawK.jssrc/gateway/static/dashboard/assets/KeysPage-CD3OUZzC.jssrc/gateway/static/dashboard/assets/ModelScopeControl-qtT3hmbU.jssrc/gateway/static/dashboard/assets/ModelsPage-DojQrJDI.jssrc/gateway/static/dashboard/assets/OverviewPage-djfSANxc.jssrc/gateway/static/dashboard/assets/ProvidersPage-B610aRno.jssrc/gateway/static/dashboard/assets/RoutingPage-CjI_jYub.jssrc/gateway/static/dashboard/assets/SettingsPage-3ii2VWM1.jssrc/gateway/static/dashboard/assets/TablePagination-BaVngp9V.jssrc/gateway/static/dashboard/assets/ToolsGuardrailsPage-BiBzJlq4.jssrc/gateway/static/dashboard/assets/UsagePage-9xByG5m8.jssrc/gateway/static/dashboard/assets/UsagePage-oSvga8ZJ.jssrc/gateway/static/dashboard/assets/UsersPage-C2TSN4yb.jssrc/gateway/static/dashboard/assets/index-CLcUiuVX.jssrc/gateway/static/dashboard/assets/index-CWtP4OuS.jssrc/gateway/static/dashboard/index.htmltests/integration/conftest.pytests/integration/test_model_discovery.pytests/integration/test_usage_endpoint.pytests/unit/test_gateway_model_discovery.pytests/unit/test_model_catalog_service.pytests/unit/test_provider_health_service.pyweb/src/api/client.test.tsweb/src/api/client.tsweb/src/api/hooks.tsweb/src/api/types.tsweb/src/pages/ActivityPage.test.tsxweb/src/pages/ActivityPage.tsxweb/src/pages/UsagePage.test.tsxweb/src/pages/UsagePage.tsx
💤 Files with no reviewable changes (3)
- src/gateway/static/dashboard/assets/ActivityPage-CfIPwmDz.js
- src/gateway/static/dashboard/assets/UsagePage-9xByG5m8.js
- src/gateway/static/dashboard/assets/index-CLcUiuVX.js
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces dashboard page-load latency by moving provider/model discovery and models.dev catalog refreshes off the synchronous request path (serving reads from cache and refreshing in the background), and by removing whole-table reads for Usage/Activity labeling by returning user/key labels directly from /v1/usage and its breakdowns.
Changes:
- Add background refreshers for model discovery and models.dev catalog; serve stale cache on dashboard GETs with optional
?refresh=truefor operator-only endpoints. - Extend usage endpoints to outer-join
users/api_keysfor per-row labels and labeled breakdowns, eliminating dashboard-side/v1/usersand/v1/keystable scans. - Add dashboard-side request timeout + disable retries for discovery-backed queries; update unit/integration/dashboard tests and regenerate bundled assets + OpenAPI/Postman artifacts.
Reviewed changes
Copilot reviewed 40 out of 44 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/src/pages/UsagePage.tsx | Builds user/key/model filter options from usage summary breakdown labels instead of loading full users/keys tables. |
| web/src/pages/UsagePage.test.tsx | Updates summary fixtures to include label and asserts picker labeling behavior. |
| web/src/pages/ActivityPage.tsx | Removes useUsers/useKeys usage; labels API key column from usage rows and builds filter options from summary breakdowns. |
| web/src/pages/ActivityPage.test.tsx | Ensures Activity no longer calls /v1/users or /v1/keys; validates labeling fallbacks. |
| web/src/api/types.ts | Adds user_alias / api_key_name on UsageEntry and label on UsageGroupRow. |
| web/src/api/hooks.ts | Disables retries (retry: false) for discovery/models.dev-backed queries to avoid serial slow-failure amplification. |
| web/src/api/client.ts | Adds a 30s request timeout (via AbortSignal) and differentiates timeout vs unreachable-network errors. |
| web/src/api/client.test.ts | Adds tests covering apiFetch timeout behavior and signal passthrough. |
| tests/unit/test_provider_health_service.py | Updates discovery mocks for new serve_stale parameter and adds tests for cached health reads. |
| tests/unit/test_model_catalog_service.py | Adds coverage for serving stale models.dev cache and background-catalog enablement rules. |
| tests/unit/test_gateway_model_discovery.py | Adds extensive coverage for background discovery behavior and refresh interval floor. |
| tests/integration/test_usage_endpoint.py | Asserts /v1/usage rows and summary breakdowns carry joined labels; verifies outer-join behavior for deleted entities. |
| tests/integration/test_model_discovery.py | Adds integration coverage ensuring read endpoints serve expired cache without dialing; validates refresh=true dials. |
| tests/integration/conftest.py | Autouse fixture suppresses background refreshers during integration tests to prevent real outbound calls at app startup. |
| src/gateway/static/dashboard/index.html | Updates hashed JS asset references for rebuilt dashboard bundle. |
| src/gateway/static/dashboard/assets/UsersPage-C2TSN4yb.js | Regenerated bundled dashboard asset. |
| src/gateway/static/dashboard/assets/UsagePage-oSvga8ZJ.js | Regenerated bundled dashboard asset (new hash). |
| src/gateway/static/dashboard/assets/UsagePage-9xByG5m8.js | Removes old bundled asset (hash rollover). |
| src/gateway/static/dashboard/assets/ToolsGuardrailsPage-BiBzJlq4.js | Regenerated bundled dashboard asset. |
| src/gateway/static/dashboard/assets/TablePagination-BaVngp9V.js | Regenerated bundled dashboard asset. |
| src/gateway/static/dashboard/assets/OverviewPage-djfSANxc.js | Regenerated bundled dashboard asset. |
| src/gateway/static/dashboard/assets/ModelScopeControl-qtT3hmbU.js | Regenerated bundled dashboard asset. |
| src/gateway/static/dashboard/assets/ConfirmDialog-h1eFJM-m.js | Regenerated bundled dashboard asset. |
| src/gateway/static/dashboard/assets/BudgetsPage-CXkIKoXy.js | Regenerated bundled dashboard asset. |
| src/gateway/services/provider_health_service.py | Adds serve_stale plumbing so polled health can serve cache while refresher owns dialing. |
| src/gateway/services/model_discovery_service.py | Adds serve-stale reads, force refresh, and a background refresher with an interval floor. |
| src/gateway/services/model_catalog_service.py | Adds serve-stale/force cache reads and a background catalog refresher with an interval floor. |
| src/gateway/main.py | Starts/stops the new discovery + catalog refreshers in the app lifespan and clears caches on shutdown. |
| src/gateway/api/routes/usage.py | Adds labeled usage rows + labeled breakdowns (outer joins) to avoid dashboard whole-table reads. |
| src/gateway/api/routes/providers.py | Serves cached provider health by default when background discovery is enabled; keeps explicit refresh behavior. |
| src/gateway/api/routes/models.py | Serves cached discovery and models.dev metadata; adds refresh param to discoverable models and includes checked_at. |
| docs/public/otari.postman_collection.json | Regenerates Postman collection to reflect new/updated endpoint docs and query params. |
| docs/public/openapi.json | Regenerates OpenAPI schema (checked_at, labels, refresh param). |
| docs/configuration.md | Documents new background refresh behavior and refresh escape hatch semantics. |
Files not reviewed (3)
- src/gateway/static/dashboard/assets/ActivityPage-zqcCQMke.js: Generated file
- src/gateway/static/dashboard/assets/UsagePage-oSvga8ZJ.js: Generated file
- src/gateway/static/dashboard/assets/index-CWtP4OuS.js: Generated file
khaledosman
approved these changes
Aug 7, 2026
khaledosman
left a comment
khaledosman
left a comment
Member
There was a problem hiding this comment.
Approving so this is not held up on review. Eight findings inline. The two serve_stale ones are the same bug in two caches (a cached failure is served as an answer at any age, so both negative TTLs are dead on every read path and one transient error is pinned for the refresh interval); those are the two I would fix before this ships.
🤖 Review generated with Claude Code
…alog fetch Cancelling a task is a request, not a guarantee. The CancelledError is delivered at whatever the task is awaiting, and a nested anyio cancel scope there can consume it: CancelScope.__exit__ calls host_task.uncancel() for each pending uncancellation whenever its own scope was cancelling, then swallows the error it sees. httpx and the provider SDKs implement their per-operation timeouts as exactly those scopes, so a shutdown cancel that races one of their timeouts is absorbed, the refresher loop resumes, and it sleeps out a whole interval (a day, for the models.dev catalog). The unbounded `await task` in the lifespan then never returns, so shutdown hangs. That is what timed out two unit tests on CI at 120s, both inside TestClient.__exit__, and it is a production restart hazard for any gateway with a fetch in flight. _stop_refresher cancels, waits a bounded 5s, and abandons the task with a warning if it will not stop. asyncio.wait rather than `await task`, so a refresher that died on an unexpected error is logged instead of aborting the rest of shutdown (the log writer and pooled search client are closed after it). Applied to all six refreshers, not just the two new ones. Separately, unit tests were starting the real refreshers, so every TestClient(create_app(...)) in tests/unit fetched models.dev for real on CI. The suppression fixture moves from tests/integration/conftest.py to the root conftest so both suites share one definition. Also from review: - A failed models.dev fetch was served at any age under serve_stale, so a transient outage disabled enrichment until the next refresh tick, 24h by default, with no ?refresh flag on /v1/models/metadata to escape it. Only a successful blob is served stale now; a failure falls back to the 60s negative TTL it already had. - The discovery and catalog refreshers re-check their setting per tick instead of being started only when it holds. model_cache_ttl_seconds and models_dev_cache_ttl_seconds are runtime-settable, and raising either from 0 flipped every read onto the serve-from-cache path with no refresher running, which is the "cache nothing refreshes" mode these knobs do not offer. - apiFetch converts a timeout on the response-body read, not just on fetch(): headers can arrive before a stalled body, and a raw DOMException reached callers that only handle ApiError. - The bulk usage delete, the bulk reprice, and the pricing-snapshot refresh get a 5 minute deadline instead of the 30s default. Their duration scales with the data, and the server commits whether or not the browser is still listening, so a 30s abort reported failure for work that succeeded. - allowsCustom on the User and API-key filter pickers. Their options are the in-window top spenders capped at 100, so an entity below that rank was unreachable from the UI; Enter now commits a pasted id, as Model already did. - Documented that a cold provider's first read still dials, that model_discovery_negative_ttl_seconds no longer governs the refresher-owned path, and that models_dev_cache_ttl_seconds now sets a refresh interval. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
temporarily deployed
to
integration-tests
August 7, 2026 13:39 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
github-actions
Bot
removed
the
missing-template
…uggestions Review follow-ups from @khaledosman, Copilot, and CodeRabbit. Reads serve a cached failure at any age, so the refresh interval, not model_discovery_negative_ttl_seconds, decided how fast a recovered provider reappeared: one timed-out dial dropped a provider's models from GET /v1/models and marked it unreachable on /v1/providers/health for the full 300s window, where a read used to re-dial after 30s. A round that reports any failure now comes back on the negative TTL instead of the success cadence, floored the same way, so reads stay off the dial path and recovery keeps its old bound. background_discovery_enabled now also requires model_discovery. An operator who turned that off did so to stop the gateway dialing providers, and a refresher fanning out every 5 minutes for the life of the process is unrequested traffic against a provider that may meter list_models. The operator endpoints still dial when asked. GET /v1/models/{model_id} reads stale-tolerantly like the listing. Nothing on the request path renews cached_at any more and the refresher sleeps after each round, so an entry is expired from the moment the next round starts until its dials finish; a TTL-bounded peek 404'd a model the listing was serving in the same instant, for any provider model with no pricing row and no genai-prices fallback. A negatively cached provider still reports no models. The model typeahead and source picker get their entity filters back. Dropping user_id/api_key_id was only ever needed by the user and key pickers, which now have their own summary; sharing one query meant Activity filtered to one user suggested models only other users had called, and picking one returned an empty table with nothing saying the combination could not match. Same split on the Usage page. Also: a stray fourth quote opened a test docstring, and the model_cache_ttl_seconds and model_discovery rows now describe what the detail endpoint does, what a zero TTL means for the refresher, and that a health re-check coalesces with a recent dial. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
temporarily deployed
to
integration-tests
August 7, 2026 13:57 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Member
Author
|
Note: this comment was drafted by Claude via back-and-forth with @njbrake. The reasoning and decisions are his; the prose is Claude's. All review threads addressed and resolved. Map of comment to fix, since two commits are involved:
Also from CodeRabbit: the The one finding I disagreed with is CodeRabbit's suggestion to start the refreshers in hybrid mode. New tests: |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/api/client.ts (1)
132-141: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCatch
TimeoutErroraroundextractErrorMessagetoo.
apiFetchconverts timeout failures duringfetchand the success-pathresponse.json()read toApiError, but the error-status paths still callawait extractErrorMessage(response)without timeout handling. Ifresponse.json()inextractErrorMessagestalls on a401/403or non-OK response, it escapes as a rawDOMException; wrap it in atry/catchand rethrowApiError(0, timeoutMessage)forisTimeout(error).🤖 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 `@web/src/api/client.ts` around lines 132 - 141, Update the non-success response handling in apiFetch, including the 401/403 branch and the general !response.ok branch, to catch timeout errors from extractErrorMessage. When isTimeout(error) is true, rethrow ApiError with status 0 and the existing timeout message; preserve the current ApiError status and extracted message for non-timeout errors.
🧹 Nitpick comments (1)
web/src/api/client.test.ts (1)
66-78: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGood coverage of the caller-signal message path, one gap remains.
This test correctly exercises the case where a caller-supplied signal exists and the message stays generic. Nice touch pairing it with a comment that explains why quoting "30s" would mislead an operator who set a five-minute budget.
To fully close the loop from the earlier review thread, consider adding a companion test where the mocked
fetchresolves successfully with a 401/403 or non-OK status and a body read that rejects with aTimeoutError. That would confirmextractErrorMessageconverts the failure to anApiErrorthe same way the success-path JSON read does.🤖 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 `@web/src/api/client.test.ts` around lines 66 - 78, Add a companion test near the existing apiFetch timeout coverage that mocks a successful fetch response with a 401, 403, or other non-OK status whose body-reading method rejects with a TimeoutError, then assert apiFetch rejects with an ApiError matching the expected timeout message and status behavior. This should exercise extractErrorMessage through the non-OK response path and mirror the existing success-path JSON-read timeout test.
🤖 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 `@src/gateway/main.py`:
- Around line 249-264: Update the shutdown flow containing alias_refresher,
policy_refresher, provider_refresher, price_refresher, discovery_refresher, and
catalog_refresher to cancel all present refreshers before awaiting any stops,
then await their _stop_refresher operations concurrently so shutdown remains
bounded by one timeout period. Preserve each refresher’s corresponding cache
reset, and add a regression test that uses at least two stuck refreshers to
verify the bounded completion time.
In `@tests/unit/test_gateway_lifespan_shutdown.py`:
- Around line 63-64: Update the test cleanup around task.cancel() to await the
cancelled task inside pytest.raises(asyncio.CancelledError), performing the
second cancellation as requested. Ensure the task is fully awaited before the
test exits, while preserving the existing assertion that it is initially
pending.
In `@web/src/api/client.ts`:
- Around line 84-100: Update the provider-credentials re-encrypt mutation in
hooks.ts to pass signal: longRequestSignal() to its apiFetch call, matching the
existing bulk usage delete, reprice, and pricing-snapshot refresh mutations
while preserving the default timeout for other requests.
In `@web/src/pages/ActivityPage.tsx`:
- Around line 921-925: Update entitySuggestFilters in ActivityPage to spread
filters instead of modelSuggestFilters, while still clearing only user_id and
api_key_id. Keep the active model filter intact when entitySummary uses these
filters, without changing the model picker’s existing modelSuggestFilters
behavior.
- Around line 909-929: Add entitySummary.refetch() to the Activity refresh
handler alongside the existing window-scoped refetch calls, using void as with
the other refetches. Ensure pressing Refresh reloads the user and API-key picker
options.
---
Outside diff comments:
In `@web/src/api/client.ts`:
- Around line 132-141: Update the non-success response handling in apiFetch,
including the 401/403 branch and the general !response.ok branch, to catch
timeout errors from extractErrorMessage. When isTimeout(error) is true, rethrow
ApiError with status 0 and the existing timeout message; preserve the current
ApiError status and extracted message for non-timeout errors.
---
Nitpick comments:
In `@web/src/api/client.test.ts`:
- Around line 66-78: Add a companion test near the existing apiFetch timeout
coverage that mocks a successful fetch response with a 401, 403, or other non-OK
status whose body-reading method rejects with a TimeoutError, then assert
apiFetch rejects with an ApiError matching the expected timeout message and
status behavior. This should exercise extractErrorMessage through the non-OK
response path and mirror the existing success-path JSON-read timeout test.
🪄 Autofix
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: 6fe63d5d-2046-4c10-bb22-743a0a81979b
📒 Files selected for processing (35)
docs/configuration.mdsrc/gateway/api/routes/models.pysrc/gateway/core/config.pysrc/gateway/main.pysrc/gateway/services/model_catalog_service.pysrc/gateway/services/model_discovery_service.pysrc/gateway/static/dashboard/assets/ActivityPage-BiZrg9Wc.jssrc/gateway/static/dashboard/assets/BudgetsPage-CMiX_1TW.jssrc/gateway/static/dashboard/assets/ConfirmDialog-SJU1oKiR.jssrc/gateway/static/dashboard/assets/DocsPage-CGwTgwR_.jssrc/gateway/static/dashboard/assets/KeysPage-DaHoC_xH.jssrc/gateway/static/dashboard/assets/ModelScopeControl-Df0CgnHC.jssrc/gateway/static/dashboard/assets/ModelsPage-CXuELAcH.jssrc/gateway/static/dashboard/assets/OverviewPage-CucRHBgP.jssrc/gateway/static/dashboard/assets/ProvidersPage-BAVrb9dJ.jssrc/gateway/static/dashboard/assets/RoutingPage-Ime0-_3U.jssrc/gateway/static/dashboard/assets/SettingsPage-CLKqwvOz.jssrc/gateway/static/dashboard/assets/TablePagination-D6qbx-Og.jssrc/gateway/static/dashboard/assets/ToolsGuardrailsPage-BBbCaYZB.jssrc/gateway/static/dashboard/assets/UsagePage-CJ2pGI3w.jssrc/gateway/static/dashboard/assets/UsersPage-Ci3xqgbL.jssrc/gateway/static/dashboard/assets/index-BcPLnG91.jssrc/gateway/static/dashboard/index.htmltests/conftest.pytests/integration/test_model_discovery.pytests/unit/test_gateway_lifespan_shutdown.pytests/unit/test_gateway_model_discovery.pytests/unit/test_model_catalog_service.pytests/unit/test_provider_health_service.pyweb/src/api/client.test.tsweb/src/api/client.tsweb/src/api/hooks.tsweb/src/pages/ActivityPage.test.tsxweb/src/pages/ActivityPage.tsxweb/src/pages/UsagePage.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- src/gateway/static/dashboard/index.html
- tests/unit/test_provider_health_service.py
- web/src/pages/ActivityPage.test.tsx
- web/src/pages/UsagePage.tsx
- src/gateway/services/model_catalog_service.py
- src/gateway/api/routes/models.py
- src/gateway/services/model_discovery_service.py
# Conflicts: # src/gateway/static/dashboard/assets/ActivityPage-fH63Z1Im.js # src/gateway/static/dashboard/assets/BudgetsPage-C3eMHXLY.js # src/gateway/static/dashboard/assets/BudgetsPage-DJMj9z5Z.js # src/gateway/static/dashboard/assets/BudgetsPage-Di_q043l.js # src/gateway/static/dashboard/assets/ConfirmDialog-C-RWVwB6.js # src/gateway/static/dashboard/assets/ConfirmDialog-Dvkwda0f.js # src/gateway/static/dashboard/assets/ConfirmDialog-lRO7CIis.js # src/gateway/static/dashboard/assets/DocsPage-Crh3hB4y.js # src/gateway/static/dashboard/assets/DocsPage-D4q8s7aN.js # src/gateway/static/dashboard/assets/DocsPage-omWiBiUs.js # src/gateway/static/dashboard/assets/KeysPage-D2ySTl5G.js # src/gateway/static/dashboard/assets/KeysPage-DBP0rqYO.js # src/gateway/static/dashboard/assets/KeysPage-DvXgAgzE.js # src/gateway/static/dashboard/assets/ModelScopeControl-CYPgEOWk.js # src/gateway/static/dashboard/assets/ModelScopeControl-Cnf_HjRm.js # src/gateway/static/dashboard/assets/ModelScopeControl-DQpF54p9.js # src/gateway/static/dashboard/assets/ModelsPage-BVAOlcUn.js # src/gateway/static/dashboard/assets/OverviewPage-33CntAQu.js # src/gateway/static/dashboard/assets/OverviewPage-DIEI5QfB.js # src/gateway/static/dashboard/assets/OverviewPage-W9tjAThu.js # src/gateway/static/dashboard/assets/ProvidersPage-CkpZWNPU.js # src/gateway/static/dashboard/assets/ProvidersPage-Igqia_Xr.js # src/gateway/static/dashboard/assets/ProvidersPage-ooS_k3AK.js # src/gateway/static/dashboard/assets/RoutingPage-CQcIne5l.js # src/gateway/static/dashboard/assets/SettingsPage-CDU9M0qn.js # src/gateway/static/dashboard/assets/SettingsPage-eRH6Zlbl.js # src/gateway/static/dashboard/assets/SettingsPage-knFypvb1.js # src/gateway/static/dashboard/assets/TablePagination-BpT-8wzM.js # src/gateway/static/dashboard/assets/TablePagination-CMcmgCgb.js # src/gateway/static/dashboard/assets/TablePagination-aSWyG4WX.js # src/gateway/static/dashboard/assets/ToolsGuardrailsPage-CkNbc6ta.js # src/gateway/static/dashboard/assets/ToolsGuardrailsPage-DgYf8d8e.js # src/gateway/static/dashboard/assets/ToolsGuardrailsPage-oZnn27P0.js # src/gateway/static/dashboard/assets/UsagePage-Bt6OQ6El.js # src/gateway/static/dashboard/assets/UsersPage-B5TbXNQ2.js # src/gateway/static/dashboard/assets/UsersPage-CWfTsLqm.js # src/gateway/static/dashboard/assets/UsersPage-D5FDH2kZ.js # src/gateway/static/dashboard/index.html
njbrake
temporarily deployed
to
integration-tests
August 7, 2026 17:06 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
njbrake
temporarily deployed
to
integration-tests
August 7, 2026 17:10 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Four dashboard GETs dialed upstream providers on the request path, bounded only by
model_discovery_timeout_seconds(10s per unreachable provider) and the models.dev fetch timeout (15s). Each held a browser connection slot and a pooled database session for the whole dial.Discovery and the models.dev catalog now refresh in the background and reads answer from cache, using the same refresher shape the alias, policy, provider and price caches already use in the lifespan. A provider that has never been dialed still dials, so a cold worker never claims it has no models.
?refresh=trueforces a live re-dial on/v1/models/discoverableand/v1/providers/health./v1/modelsdeliberately has no such flag: it takes any API key, and a fanout across every provider is an operator action. No new config;model_cache_ttl_seconds = 0still means "dial on read" and now also sets the refresh interval (floored at 30s).Usage and Activity no longer read the users and api_keys tables. They paged both in full on every visit to name filter options and label rows.
/v1/usagenow returnsuser_aliasandapi_key_nameper row (outer-joined, so a row whose owner was deleted survives, unlabelled), andby_user/by_api_keycarry alabelresolved in the same GROUP BY. Both pages build their pickers from a summary they already request.Filter options are now the in-window entities ranked by spend, capped at 100, rather than every entity that ever existed. The User and API-key pickers accept a typed or pasted id on Enter, so an entity below that rank is still reachable.
Also:
retry: falseon the four discovery-backed queries, and a 30s bound inapiFetch(5 minutes for the bulk usage delete and reprice, whose duration scales with the data).Follow-up fixes in this PR
Review found a shutdown hang that CI hit as two 120s unit-test timeouts. Cancelling a task is a request, not a guarantee: anyio's
CancelScope.__exit__callshost_task.uncancel()whenever its own scope was cancelling, and httpx runs its per-operation timeouts as those scopes, so a shutdown cancel racing one of them is absorbed. The refresher loop then resumes and sleeps out a full interval, and the lifespan's unboundedawait tasknever returns._stop_refreshernow waits a bounded 5s and abandons the task with a warning; it applies to all six refreshers, and it also stops a refresher's unexpected error from aborting the rest of shutdown. Unit tests were starting the real refreshers too, so the suppression fixture moved to the root conftest.Plus: a failed models.dev fetch is no longer served at any age (a transient outage disabled enrichment for 24h); both refreshers re-check their runtime-settable knob per tick;
apiFetchconverts a timeout on the body read, not just onfetch().PR Type
Relevant issues
None filed; found while profiling dashboard page loads.
Measured
Same browser repro, gateway with six unreachable providers:
/v1/providers/health/v1/models/v1/models/discoverable/v1/models/metadataActivity and Usage now issue 4 and 3 requests respectively, all under 25ms, and neither touches
/v1/usersor/v1/keys.Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).Regression tests verified in both directions, failing before the fix and passing after: the three
*_serves_an_expired_cache_without_dialingtests,never reads the whole users or api_keys table, the_stop_refreshershutdown tests, the models.dev negative-TTL test, and theapiFetchbody-stall test. 1414 unit, 1020 integration, 488 dashboard.make lint,make openapi-check,make postman-checkclean. Dashboard bundle rebuilt.AI Usage
AI Model/Tool used:
Claude Opus 5 (1M context) via Claude Code.
Any additional AI details you'd like to share:
Implemented by Claude Code, then reviewed by a separate Claude Code agent with no access to the implementing session's reasoning. That review found the shutdown hang, the models.dev negative-TTL regression, and the filter-picker gap, each reproduced in isolation before being fixed.
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
Technical notes
model_cache_ttl_secondsis0.