feat(ui): add a standalone admin dashboard for models, pricing, and settings#227
Conversation
Otari only served a static get-started splash page, so the first thing an operator wanted after deploying (create virtual keys, watch usage and traffic) meant reaching for the Postman collection. This adds a real admin dashboard. The dashboard is a React 19 + HeroUI v3 single-page app under web/ (Vite, Tailwind v4, TanStack Query, Recharts). It drives the existing standalone management API (/v1/keys, /v1/users, /v1/usage) using the master key, which the operator enters on a sign-in screen and which is held only in the browser tab's session storage. Pages: Usage (totals, requests-over-time chart, per-model breakdown, recent requests), API keys (create, revoke, reactivate, delete), and Users (create, delete, see spend). Serving: in standalone mode the gateway serves index.html at / and the hashed assets under /assets; the get-started tutorial moves to /welcome. Hybrid mode has no local management API, so / keeps serving the tutorial, and the root also falls back to the tutorial when the bundle has not been built. The built bundle is committed under src/gateway/static/dashboard and added to package-data, so the wheel and Docker image ship the dashboard with no Node build stage. A new CI workflow type-checks, tests, and builds the frontend, and fails if the committed bundle is stale. Fixes #225 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds a full React/HeroUI admin dashboard packaged into the gateway's Python distribution, served at ChangesDashboard rollout
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 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 |
Signing in flashed the dashboard and bounced straight back to the login screen. The api client's in-memory master key was synced from React state in an effect, but effects run child-first, so React Query's first request (GET /v1/usage) fired before the key was set. That request went out without the Authorization header, returned 401, and the 401 handler logged the operator back out. A page refresh with a stored key hit the same race. Set the client key synchronously: in login() before the re-render that mounts the dashboard, and in the state initializer so a restored session key is in place during the first render. Also validate the key against a master-key-gated endpoint at sign-in, so a wrong key shows "Invalid master key." instead of silently bouncing, and a correct key only enters the dashboard once confirmed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
tests/unit/test_gateway_root_page.py (1)
49-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a hybrid-mode root test.
The new routing logic has a third branch,
config.is_hybrid_mode, and this file only locks down the standalone-with-bundle and standalone-without-bundle cases. A focused test for hybrid mode returning the tutorial at/would keep that contract from drifting quietly later. Based on learnings, hybrid mode must keep serving the tutorial at/, and test additions should stay close to the changed behavior.🤖 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/unit/test_gateway_root_page.py` around lines 49 - 86, The root page tests currently cover standalone with and without the dashboard bundle, but they do not verify the new config.is_hybrid_mode branch. Add a focused test alongside test_dashboard_is_served_at_root and test_root_falls_back_to_tutorial_without_dashboard that creates an app in hybrid mode and asserts GET "/" returns the tutorial content (the same fallback behavior as the no-dashboard case), using create_app and the existing _config helper to keep the coverage aligned with the routing logic changes.Sources: Coding guidelines, Learnings
🤖 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 @.github/workflows/otari-dashboard.yml:
- Around line 26-32: The workflow’s checkout step is leaving the GitHub token
available to later build steps, so harden the job by disabling credential
persistence in the actions/checkout step and setting the job permissions to
read-only where possible. Update the otari-dashboard workflow around
actions/checkout and the job-level permissions so untrusted build scripts cannot
access the persisted token.
In `@web/src/api/hooks.ts`:
- Around line 27-31: The fixed 1000-item limit in useKeys and useUsers causes
the dashboard to silently miss records past that threshold. Update the query
logic in these hooks to support pagination or another complete-loading strategy
instead of hardcoding limit=1000, and make sure the associated query
keys/consumers still work with the changed fetching behavior.
- Around line 14-17: The query cache keys for the admin data hooks are too
static, so signed-out/signed-in transitions can reuse stale results. Update
useKeys, useUsers, and useUsage to read masterKey from useAuth() and include it
in each queryKey array so the cache is scoped to the current admin session; keep
health unchanged since it is public. Use the existing KEYS, USERS, USAGE, and
HEALTH symbols to locate the hook definitions and apply the key change
consistently.
In `@web/src/components/Field.tsx`:
- Around line 27-33: Use HeroUI’s Description slot in Field’s TextField instead
of rendering the helper text as a raw span, so the component can wire
aria-describedby automatically. Update the Field component to keep label, input,
and description inside TextField, but replace the description span with the
Description component while preserving the same conditional rendering and
styling for the description text.
In `@web/src/lib/usage.ts`:
- Around line 44-60: The usageByDay helper is bucketing by UTC because it builds
the day key with toISOString(), which conflicts with the local-time behavior
described in the function comment. Update usageByDay to derive the bucket key
from the timestamp’s local date parts instead of UTC, keeping the rest of the
aggregation logic in DailyPoint unchanged. Also add a boundary test around local
midnight to verify entries on either side of the day change stay in the correct
bucket.
In `@web/src/pages/KeysPage.tsx`:
- Around line 48-61: The create flow in KeysPage’s submit handler leaves the
previous one-time secret in created until the next successful mutation, so clear
created before calling createKey.mutate and keep the existing onSuccess reset
logic for name and userId. Update the submit function so a new create attempt
removes any previously shown secret immediately, preventing stale data from
remaining visible if the next create fails.
In `@web/src/pages/UsersPage.tsx`:
- Around line 15-29: The user creation flow in UsersPage should be wired to a
real form so keyboard submission works. Move the submit logic currently in
submit into a form onSubmit handler around the userId and alias inputs, and make
the action button submit the form instead of calling the mutation only from
click handlers. Keep the existing createUser.mutate success behavior and
trim/validation logic inside the shared submit path so Enter in either field
triggers the same create flow.
In `@web/src/provider.tsx`:
- Around line 8-30: The Provider component keeps a single QueryClient instance
alive across auth transitions, so cached admin query data can leak between
sessions. Update Provider to detect when the master key changes in AuthProvider
and reset or replace the QueryClient at that point, clearing cached results for
queries like /v1/keys, /v1/users, and /v1/usage before the new session renders.
Use the Provider, QueryClient, and AuthProvider symbols to locate the cache
initialization and add the cache-clearing logic there.
In `@web/src/styles/globals.css`:
- Around line 23-27: Reset the default browser body spacing in the global
stylesheet by updating the body rules in globals.css so the app fills the
viewport without the default 8px frame; keep the existing body styles and add a
margin reset alongside them to prevent stray scrollbars and ensure the dashboard
is flush to the edges.
---
Nitpick comments:
In `@tests/unit/test_gateway_root_page.py`:
- Around line 49-86: The root page tests currently cover standalone with and
without the dashboard bundle, but they do not verify the new
config.is_hybrid_mode branch. Add a focused test alongside
test_dashboard_is_served_at_root and
test_root_falls_back_to_tutorial_without_dashboard that creates an app in hybrid
mode and asserts GET "/" returns the tutorial content (the same fallback
behavior as the no-dashboard case), using create_app and the existing _config
helper to keep the coverage aligned with the routing logic changes.
🪄 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
Run ID: cb14b166-5647-491a-9f30-e88d9d3bc4e2
⛔ Files ignored due to path filters (3)
src/gateway/static/dashboard/favicon.svgis excluded by!**/*.svgweb/package-lock.jsonis excluded by!**/package-lock.jsonweb/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (41)
.github/workflows/otari-dashboard.ymlAGENTS.mdREADME.mdpyproject.tomlsrc/gateway/dashboard.pysrc/gateway/main.pysrc/gateway/static/dashboard/assets/index-BMK0c1iC.csssrc/gateway/static/dashboard/assets/index-Db75MwEA.jssrc/gateway/static/dashboard/index.htmltests/unit/test_gateway_root_page.pyweb/.gitignoreweb/README.mdweb/index.htmlweb/package.jsonweb/src/App.tsxweb/src/api/client.tsweb/src/api/hooks.tsweb/src/api/types.tsweb/src/auth/AuthContext.tsxweb/src/components/AppShell.tsxweb/src/components/Field.tsxweb/src/components/Login.tsxweb/src/components/Table.tsxweb/src/components/ui.tsxweb/src/lib/format.test.tsweb/src/lib/format.tsweb/src/lib/usage.test.tsweb/src/lib/usage.tsweb/src/main.tsxweb/src/pages/KeysPage.test.tsxweb/src/pages/KeysPage.tsxweb/src/pages/UsagePage.tsxweb/src/pages/UsersPage.tsxweb/src/provider.tsxweb/src/styles/globals.cssweb/src/test/setup.tsweb/src/vite-env.d.tsweb/tsconfig.app.jsonweb/tsconfig.jsonweb/tsconfig.node.jsonweb/vite.config.ts
Add a small link on the sign-in screen to the auth-free /welcome tutorial page, so a first-time visitor who lands on the dashboard can reach the get-started guide without a master key. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
Reshape the dashboard around at-a-glance data instead of charts. Adds an Overview landing page with high-level stat cards (users, providers, models, API keys) and recent traffic totals (requests, tokens, cost, errors) plus a top-models table. Splits the old chart-heavy Usage page into two table-only views: Model usage (per-model rollup with prompt/completion/total tokens and cost) and Usage (per-request log with token counts). Keys and Users are unchanged. Drops the recharts dependency, which roughly halves the JS bundle (709 KB to 350 KB, 216 KB to 109 KB gzipped). Adds a /v1/models query to count providers and models, with providers also unioned from usage logs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
Add a Pricing sidebar page backed by the /v1/pricing API. It lists the current price per model (newest past-effective row, deduped from the price history), shows the provider and effective date, and supports inline editing of the input and output per-million prices. Saving an edit posts a new price effective now; there is also a "Set pricing" form for adding a model and a delete action per row. Pricing edits invalidate the models query so model-derived views stay fresh. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
Add a read-only GET /v1/settings endpoint (master/api-key gated) exposing non-secret runtime flags: mode, version, default_pricing, and require_pricing. The dashboard's Pricing page reads it and shows a banner explaining whether the genai-prices default fallback is active: when on, unpriced models below are metered from community rates and an explicit price overrides them; when off, a warning notes unpriced models are rejected (402) or served untracked depending on require_pricing. Pre-set OTARI_DEFAULT_PRICING=true in the Railway template (alongside the existing OTARI_REQUIRE_PRICING=false) so an env-only deploy meters common models out of the box, and document it in the Railway README. The image default stays false; this is deployment config, not a product default change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
web/src/api/hooks.ts (1)
125-129: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftThe pricing table has the same silent 1000-row cap issue.
This repeats the earlier truncation pattern: once pricing records grow past 1000, the dashboard will quietly hide the rest. Please paginate or otherwise load the complete result set instead of hardcoding the cap.
🤖 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/hooks.ts` around lines 125 - 129, The usePricing hook is hardcoding a 1000-item limit in its apiFetch call, which can silently truncate the pricing list. Update usePricing to load the full result set by paginating through /v1/pricing or otherwise fetching all pages until complete, instead of relying on a fixed limit in the queryFn.
🧹 Nitpick comments (2)
web/src/pages/PricingPage.test.tsx (1)
75-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNice end-to-end check on the edit→POST payload. One gap worth filling given the row-error finding in
PricingPage.tsx: there's no test covering a failed edit or delete. A test that hasmockApireturn a non-200 for the POST and asserts the user sees an error message would lock in the fix for the silent-failure issue (and guard against regressions).🤖 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/PricingPage.test.tsx` around lines 75 - 97, Add a test in PricingPage.test.tsx that covers a failed edit flow: use mockApi to make the POST from PricingPage return a non-200 response, then exercise the edit path through the PricingPage UI and assert an error message is shown to the user. Place the check around the existing edit/save interaction in the PricingPage component test so it specifically verifies the failed POST handling and prevents the silent-failure regression.tests/unit/test_settings_endpoint.py (1)
19-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the non-master-key rejection case.
These tests cover anonymous and master-key paths, but not the admin-only contract. A small regression test for “valid API key gets 401/403” would lock this down nicely. As per coding guidelines, “Keep test additions close to the changed behavior, using unit tests for pure logic and integration tests for route or database behavior.”
🤖 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/unit/test_settings_endpoint.py` around lines 19 - 45, Add a regression test in test_settings_endpoint for the admin-only auth contract by checking that a valid non-master API key is rejected when calling the settings route. Reuse the existing _client helper and /v1/settings request pattern from test_settings_requires_auth and test_settings_reports_pricing_flags, but send a Bearer token that is not sk-test-master and assert the response is 401 or 403 as appropriate. Keep the test alongside the current settings endpoint tests and name it clearly so the rejection case is easy to find.Source: Coding guidelines
🤖 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/api/routes/settings.py`:
- Around line 29-39: The get_settings admin route is using
verify_api_key_or_master_key, which allows ordinary API keys to access
non-secret gateway settings; switch the dependency on the router.get handler in
get_settings to master-key-only authentication, and add a regression test that
confirms a non-master key is rejected while the master key still succeeds.
In `@web/src/api/client.ts`:
- Around line 56-60: The admin key validation in the API client only treats 401
as an invalid key, so 403 from the gateway still falls through as a generic
error. Update the response handling in the client validation flow in the
function that checks the sign-in/admin key response so that both 401 and 403
return false before the ApiError path, keeping the “wrong key” behavior
consistent for forbidden master-key-only endpoints.
In `@web/src/lib/usage.ts`:
- Around line 49-73: The usage aggregation in usageByModel is currently keyed
only by entry.model, which merges rows from different providers that share the
same model name; update the bucketing logic to key by both provider and model so
each provider/model pair becomes its own ModelUsage row. Keep the existing
fallback handling for unknown provider/model, and make sure any callers that use
row.model as a React key switch to a composite key based on both provider and
model. Add a regression test covering the “same model, different providers” case
to verify requests, tokens, and cost stay separated.
In `@web/src/pages/OverviewPage.tsx`:
- Around line 21-29: The Providers card is using a partial count because
providerCount in OverviewPage is derived from both entries and models.data, but
the loading state only waits for one source. Update the loading condition for
the Providers card to stay in a loading placeholder until both usage.isLoading
and models.isLoading are false, so the count is not rendered before
models.data[].owned_by has been merged into the set. Use the providerCount
useMemo and the related card rendering logic to locate the fix.
In `@web/src/pages/PricingPage.tsx`:
- Around line 211-235: Row-level edit/delete failures are not surfaced because
the `useSetPricing()` and `useDeletePricing()` instances created in
`PricingPage` are not the same ones used by `PricingRow`. Move the error UI
closer to the live mutations by rendering an `ErrorBanner` (or inline error
message) inside `PricingRow` using its own `setPricing.error ??
deletePricing.error`, or alternatively lift the mutations up so the page-level
banner is connected to the actual mutation instances. Also remove the unused
top-level mutation reads from `PricingPage` if you keep the row-local approach.
---
Duplicate comments:
In `@web/src/api/hooks.ts`:
- Around line 125-129: The usePricing hook is hardcoding a 1000-item limit in
its apiFetch call, which can silently truncate the pricing list. Update
usePricing to load the full result set by paginating through /v1/pricing or
otherwise fetching all pages until complete, instead of relying on a fixed limit
in the queryFn.
---
Nitpick comments:
In `@tests/unit/test_settings_endpoint.py`:
- Around line 19-45: Add a regression test in test_settings_endpoint for the
admin-only auth contract by checking that a valid non-master API key is rejected
when calling the settings route. Reuse the existing _client helper and
/v1/settings request pattern from test_settings_requires_auth and
test_settings_reports_pricing_flags, but send a Bearer token that is not
sk-test-master and assert the response is 401 or 403 as appropriate. Keep the
test alongside the current settings endpoint tests and name it clearly so the
rejection case is easy to find.
In `@web/src/pages/PricingPage.test.tsx`:
- Around line 75-97: Add a test in PricingPage.test.tsx that covers a failed
edit flow: use mockApi to make the POST from PricingPage return a non-200
response, then exercise the edit path through the PricingPage UI and assert an
error message is shown to the user. Place the check around the existing
edit/save interaction in the PricingPage component test so it specifically
verifies the failed POST handling and prevents the silent-failure regression.
🪄 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
Run ID: 37dfdfe5-1402-497f-b2de-e7635aa28076
⛔ Files ignored due to path filters (2)
docs/public/openapi.jsonis excluded by!docs/public/openapi.jsonweb/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
deploy/railway/README.mddeploy/railway/template.jsonsrc/gateway/api/main.pysrc/gateway/api/routes/settings.pysrc/gateway/static/dashboard/assets/index-BGbAEBXR.csssrc/gateway/static/dashboard/assets/index-CjS5umBt.jssrc/gateway/static/dashboard/index.htmltests/unit/test_settings_endpoint.pyweb/package.jsonweb/src/App.tsxweb/src/api/client.tsweb/src/api/hooks.tsweb/src/api/types.tsweb/src/auth/AuthContext.tsxweb/src/components/AppShell.tsxweb/src/components/Login.test.tsxweb/src/components/Login.tsxweb/src/components/ui.tsxweb/src/lib/pricing.test.tsweb/src/lib/pricing.tsweb/src/lib/usage.test.tsweb/src/lib/usage.tsweb/src/pages/ModelsPage.tsxweb/src/pages/OverviewPage.tsxweb/src/pages/PricingPage.test.tsxweb/src/pages/PricingPage.tsxweb/src/pages/UsagePage.tsx
✅ Files skipped from review due to trivial changes (2)
- web/src/lib/pricing.test.ts
- deploy/railway/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/gateway/static/dashboard/index.html
- web/src/components/ui.tsx
- web/src/auth/AuthContext.tsx
… models Two dashboard improvements around models and pricing: Models page now lists the full /v1/models catalog (every configured and discovered model) joined with usage stats and price columns, instead of only models that had traffic. This reconciles the page with the Overview "Models" tile, which counts the catalog. Pricing page can backfill cost for a model that ran while unpriced. The form offers a picker of used-but-unpriced models; after setting a price, a panel runs the new master-key-gated POST /v1/usage/backfill, which recomputes cost on that model's empty usage rows from their recorded token counts and the current price, then folds each user's backfilled total into their spend via the same path live requests use. Already-costed rows are untouched, so it is safe to re-run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
Resolve the CodeRabbit review findings on the dashboard PR: - settings: require the master key on GET /v1/settings (was any valid API key), with a regression test for a non-master token. - usage summary: bucket by (provider, model) instead of model alone, so the same model name from two providers is not merged and misattributed. This also fixes the Models page join, which keyed usage by bare model but looked it up by the catalog's provider:model id, so catalog rows showed zero usage. - pricing: surface inline edit/delete errors in the row (they were swallowed because the page-level mutation instances were never the ones mutated). - auth: clear the query cache on login and logout so cached admin data cannot render to a later session in the same tab; treat 403 like 401 at sign-in. - overview: keep the Providers card loading until both sources are ready. - ci: harden the dashboard workflow checkout (no persisted credentials, read-only contents permission). - styles: reset the body margin explicitly. Also regenerate docs/public/otari.postman_collection.json, which was stale after adding /v1/settings and /v1/usage/backfill and was failing the openapi-spec CI check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
…and Pricing When default_pricing is on, the gateway meters unpriced models from the bundled genai-prices dataset, but that rate was never visible: /v1/models and the Pricing page only ever showed database prices. Enrich /v1/models so each model's pricing is the effective rate (DB first, else the genai-prices fallback when enabled) plus a pricing_source of "configured" / "default" / "none". The Models page now shows a price for every model and tags fallback ones "default". The Pricing page lists used-but-unpriced models that the fallback is metering as read-only "default" rows with their rate and an Override action that pre-fills the set-pricing form. Database pricing always takes precedence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub Actions
Inactive
The dashboard is served from the gateway's origin, so the app fetches /v1 and /health as same-origin paths. The Vite dev server serves only the SPA, so those calls 404 and the app cannot sign in, which made hot-reload development unusable. Proxy /v1 and /health to a gateway (localhost:8000 by default, OTARI_DEV_API to point elsewhere), and document it. Dev-server config only; the production bundle is unchanged.
Which gateway the dev server targets decides which master key signs you in, and the sign-in path reports 401 and 403 alike as an invalid master key, so pointing at the wrong gateway looks like a bad key. Print the resolved API target and whether it came from OTARI_DEV_API or the default.
The dashboard had grown six pages across three unrelated jobs. Narrow it to one: configure models and their prices, and watch what they cost. - Merge Models and Pricing into one page. They already rendered the same model+price columns from different endpoints; now one row per model carries its provider, price source, price, and usage, with the price editable inline and clearable back to the default. Pricing a model that already has traffic offers the usage backfill, instead of offering it after every save. - Cut Overview to the four tiles that describe traffic (requests, tokens, cost, errors). The user/provider/model counts were configuration facts, not activity, and Top models duplicated the Models page. - Drop the API keys and Users pages, and the hooks and types they used. Key and user management are still available over the API and in git history; this is about what the dashboard chooses to show.
# Conflicts: # README.md # src/gateway/api/routes/models.py # src/gateway/main.py
An alias is a display name declared in config.yml, not a model: pricing, budgets, and usage all key on the model it resolves to. The gateway accepts a POST /v1/pricing for an alias key and returns 200, but nothing ever reads that row, so offering a price edit on an alias row would report success and change nothing. Mark alias rows (owned_by == the gateway's ALIAS_OWNED_BY) with an alias chip, show the target's inherited rate, and replace the price actions with a note that it is priced by its target. Classify them by owned_by rather than pricing_source, which the gateway leaves at "none" for aliases even when a price is shown, and ignore any pricing row stored under an alias's own name so a dead rate cannot surface as this row's price.
|
@khaledosman thanks for the review! I slimmed down the PR a whole bunch to just get an initial implementation off the ground: future PRs can then build on top and add more pages for monitoring things like the API keys, usage etc. I think it's now ready for you to take another look and then should be good to merge. |
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/gateway/services/pricing_service.py (1)
84-86: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not silently adopt another provider's pricing.
When a configured or custom provider ID misses, the provider-agnostic retry can match the same model name under an unrelated provider. That can undercharge or overcharge requests instead of failing closed. Resolve configured instances to a known provider type first, or only use the agnostic lookup when no provider was supplied.
As per coding guidelines, billable code must fail closed for models without authoritative pricing.
🤖 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 `@src/gateway/services/pricing_service.py` around lines 84 - 86, Update the pricing lookup flow around the attempts construction so a supplied provider never falls back to the provider-agnostic (None, model) lookup. Resolve configured or custom provider IDs to a known provider type before pricing lookup, and only append/use the agnostic attempt when no provider was supplied; otherwise fail closed when authoritative pricing is unavailable.Source: Coding guidelines
src/gateway/api/routes/models.py (1)
338-363: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winHide discovered alias targets as well as pricing-only targets.
Phase 2 suppresses alias targets, but phase 1 has already published any discovered target. With model discovery enabled, callers therefore receive both the alias and the provider/model name the alias is intended to hide. Compute
alias_targetsbefore phase 1 and skip matching discovered keys there too.Proposed direction
aliases = effective_aliases(config) + alias_targets = _alias_target_keys(config, aliases) merged: dict[str, ModelObject] = {} if config.model_discovery: ... for provider_name, model in discovered: model_key = f"{provider_name}:{model.id}" + if normalize_pricing_key(config, model_key) in alias_targets: + continue pricing = pricing_map.pop(model_key, None) ... - alias_targets = _alias_target_keys(config, aliases) for model_key, pricing in pricing_map.items():🤖 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 `@src/gateway/api/routes/models.py` around lines 338 - 363, Update the model merge flow around phase 1 and _alias_target_keys so alias_targets is computed before iterating discovered models. Skip discovered entries whose model_key matches an alias target, while preserving normal publication for other discovered models and the existing phase 2 suppression for pricing-only entries.
🟡 Minor comments (6)
web/src/components/UpdatePrompt.test.tsx-57-63 (1)
57-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore
window.locationafter replacing it.
vi.restoreAllMocks()does not undo this direct property replacement, so the mocked location and reload function leak into later tests. Save the original descriptor and restore it inafterEachor afinallyblock.🤖 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/components/UpdatePrompt.test.tsx` around lines 57 - 63, Update the test containing “offers a reload once the gateway serves a different build” to capture window.location’s original property descriptor before Object.defineProperty replaces it, then restore that descriptor in afterEach or a finally block so the mocked location and reload function cannot leak into subsequent tests.web/src/api/client.ts-85-88 (1)
85-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear the invalid session on 403 as well.
The validation path treats both 401 and 403 as invalid admin credentials, but subsequent API calls only log out on 401. A restored or downgraded non-master key can therefore leave the dashboard stuck in an authenticated state with every request failing.
Proposed fix
- if (response.status === 401) { + if (response.status === 401 || response.status === 403) { unauthorizedHandler?.(); - throw new ApiError(401, await extractErrorMessage(response)); + throw new ApiError(response.status, await extractErrorMessage(response)); }🤖 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 85 - 88, Update the response-status handling in the API client so the existing unauthorizedHandler is invoked for both 401 and 403 responses, while preserving the current ApiError behavior and extracted error message.web/src/lib/format.ts-22-36 (1)
22-36: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid rendering sub-million values as
1000K.Rounding makes values such as
999999display as1000Kinstead of1M. Promote the rounded boundary to the next unit.Proposed fix
if (value >= 1000) { - return `${Math.round(value / 1000)}K`; + const thousands = Math.round(value / 1000); + return thousands >= 1000 ? "1M" : `${thousands}K`; }🤖 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/lib/format.ts` around lines 22 - 36, Update formatContext so the thousands branch promotes values whose rounded result reaches 1000K into the millions representation, preventing sub-million inputs such as 999999 from rendering as 1000K. Preserve the existing formatting for nullish, ordinary thousands, and exact million-or-higher values.web/src/pages/SettingsPage.test.tsx-62-91 (1)
62-91: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWait for settings to load before clicking the switches.
The switches render disabled before data arrives, so
findByRolecan resolve too early and the click can be ignored. Wait for the loaded version marker or for the switch to become enabled in both tests.Proposed fix
renderWithClient(<SettingsPage />); - await screen.findByRole("switch", { name: "Model discovery" }); + await screen.findByText(/Version 1.2.3/); await user.click(screen.getByRole("switch", { name: "Model discovery" }));renderWithClient(<SettingsPage />); - await screen.findByRole("switch", { name: "Default pricing" }); + await screen.findByText(/Version 1.2.3/); await user.click(screen.getByRole("switch", { name: "Default pricing" }));🤖 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/SettingsPage.test.tsx` around lines 62 - 91, Update both tests, “patches a setting when its switch is toggled” and “enables default pricing from off,” to wait for the settings-loaded state before clicking. Use the loaded version marker or wait until the corresponding switch is enabled, then perform the existing click and assertions.tests/integration/test_alias_api.py-49-61 (1)
49-61: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDispose the synchronous engine during teardown.
The fixture drops its tables but never calls
engine.dispose(), so repeated tests can retain pooled database connections.Proposed fix
finally: dispose_override() Base.metadata.drop_all(bind=engine) with engine.connect() as conn: conn.execute(text("DROP TABLE IF EXISTS alembic_version CASCADE")) conn.commit() + engine.dispose()🤖 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/integration/test_alias_api.py` around lines 49 - 61, Update the fixture teardown around the synchronous engine created by create_engine to call engine.dispose() after dropping the tables and alembic_version metadata, ensuring pooled connections are released while preserving the existing cleanup order.tests/integration/test_model_aliases.py-68-82 (1)
68-82: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDispose the synchronous engine during fixture cleanup.
Each client fixture creates a new pooled engine, but cleanup never calls
engine.dispose(). Across this large module, idle PostgreSQL connections can accumulate and destabilize CI.Proposed cleanup
finally: dispose_override() - Base.metadata.drop_all(bind=engine) - with engine.connect() as conn: - conn.execute(text("DROP TABLE IF EXISTS alembic_version CASCADE")) - conn.commit() + try: + Base.metadata.drop_all(bind=engine) + with engine.connect() as conn: + conn.execute(text("DROP TABLE IF EXISTS alembic_version CASCADE")) + conn.commit() + finally: + engine.dispose()🤖 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/integration/test_model_aliases.py` around lines 68 - 82, Update the _build_client fixture cleanup to dispose the locally created synchronous engine after dropping the tables and alembic_version metadata. Call engine.dispose() in the finally path after all database operations complete.
🧹 Nitpick comments (1)
tests/integration/test_model_aliases.py (1)
104-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the decorative repeated-hyphen separators from the alias tests.
tests/integration/test_model_aliases.py#L104-L106: retain only the descriptive section heading.tests/integration/test_model_aliases.py#L138-L140: retain only the descriptive section heading.tests/integration/test_model_aliases.py#L373-L375: retain only the descriptive section heading.tests/integration/test_model_aliases.py#L484-L486: retain only the descriptive section heading.tests/unit/test_model_aliases.py#L13-L15: retain only the descriptive section heading.tests/unit/test_model_aliases.py#L31-L33: retain only the descriptive section heading.tests/unit/test_model_aliases.py#L108-L110: retain only the descriptive section heading.tests/unit/test_model_aliases.py#L151-L153: retain only the descriptive section heading.As per coding guidelines, “Avoid em dashes and double hyphens as prose separators in documentation and comments.”
🤖 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/integration/test_model_aliases.py` around lines 104 - 106, Remove the repeated-hyphen separator lines around the descriptive section headings at tests/integration/test_model_aliases.py lines 104-106, 138-140, 373-375, and 484-486, and tests/unit/test_model_aliases.py lines 13-15, 31-33, 108-110, and 151-153; retain each heading text unchanged.Source: Coding guidelines
🤖 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 `@alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py`:
- Line 27: Add database-side timestamp defaults to the non-nullable columns:
update updated_at in alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py lines
27-27, and both created_at and updated_at in
alembic/versions/f1a3b5c7d9e2_add_model_aliases.py lines 27-28, using
server_default=sa.text("CURRENT_TIMESTAMP").
In `@src/gateway/api/routes/aliases.py`:
- Around line 112-124: Update the post-commit cache handling in the alias
mutation flow around refresh_alias_cache so cache-refresh failures cannot turn a
successful create, update, or delete into an HTTP 500. Prefer updating the local
cache from the known mutation; otherwise catch and isolate refresh_alias_cache
failures while allowing the committed response to return normally. Apply the
same change to the related mutation block noted at lines 141-150.
- Around line 102-110: Update the mutation flow around refresh_alias_cache and
_validate to serialize alias writes across workers using the project’s existing
transaction/locking mechanism. Within the serialized transaction, reload the
current alias state, run _validate against that freshly loaded state, then
perform the existing update-or-create logic for ModelAlias so concurrent
requests cannot commit an invalid alias chain.
In `@src/gateway/api/routes/models.py`:
- Around line 404-410: Update the provider mapping in the model discovery route
around discover_models_with_status and DiscoverableProvider so discovery.error
is replaced with the stable client-facing message “Provider discovery failed”
when present, while preserving the success case. Log only sanitized diagnostic
metadata server-side and never emit the raw upstream error in responses or logs.
In `@src/gateway/api/routes/pricing.py`:
- Around line 127-144: Update the pricing validation around
resolve_effective_alias so stored aliases are checked through the current
database session’s ModelAlias query, while config.aliases is checked directly
for configured aliases. Reject any matching alias before creating or updating
pricing, rather than relying solely on the potentially stale
resolve_effective_alias cache.
In `@src/gateway/api/routes/settings.py`:
- Around line 83-90: Update the updates transaction handling around
stage_override and db.commit to roll back on SQLAlchemyError, then raise a
generic HTTPException with an appropriate public error status and message
instead of re-raising the database exception or exposing its details.
In `@src/gateway/core/config.py`:
- Around line 732-749: Update the YAML-to-environment bridging flow around
_bridge_yaml_fields_to_env so values injected from YAML do not appear as
explicit environment overrides during later load_config calls. Preserve
caller-provided environment values separately and restore or exclude bridged
values before subsequent loads, or pass the loaded GatewayConfig directly to
remaining read sites; ensure a later YAML configuration can replace values from
an earlier load.
- Around line 407-412: Change the default value of the mcp_allow_loopback
configuration field to False so loopback MCP requests are denied unless
explicitly enabled by operators. Preserve the existing field name, environment
alias, and description context.
In `@src/gateway/services/model_discovery_service.py`:
- Around line 142-152: Update the provider-error handling around _short_error
and the affected lines 183–187 so raw exception text is neither returned nor
logged. Return a fixed safe message through the API, and log only the provider
instance/context and exception type, excluding exc and _short_error output from
logs or responses.
In `@src/gateway/services/pricing_service.py`:
- Around line 223-232: Update flat_request_cost so it only returns early when
pricing is None, not when pricing.input_price_per_million is zero; always divide
the present rate by 1_000_000 so legitimate zero-valued pricing remains handled
through the normal calculation.
In `@src/gateway/services/runtime_settings_service.py`:
- Around line 8-11: Update the runtime billing override flow around
default_pricing so changes propagate to every worker rather than only the
handling worker. Add shared invalidation or polling, following the existing
alias refresher pattern, and ensure each worker refreshes the stored billing
setting and applies it to its in-memory configuration and process-wide pricing
flag.
In `@tests/integration/test_alias_api.py`:
- Around line 231-236: Update the /v1/chat/completions handler to catch the
selector-resolution ValueError and raise an HTTPException with status 400 and
the existing “Invalid model format” detail. In the test around client.post,
replace pytest.raises(ValueError) with assertions on the returned response
status code and detail.
In `@web/src/api/hooks.ts`:
- Around line 151-155: Update usePricing to retrieve all pricing pages using the
API’s skip and limit parameters, combining results until no additional prices
remain. Preserve the existing query behavior for the dashboard while ensuring
pricing records beyond the first 1000 are accessible.
In `@web/src/components/AppShell.tsx`:
- Around line 21-30: Make the sidebar preference storage access in
readStoredSidebarWidth and readStoredCollapsed defensive by wrapping
localStorage reads in try/catch and returning DEFAULT_SIDEBAR or false on
failure. Apply the same protection to the corresponding sidebar preference
writes around lines 81-90, swallowing storage errors so disabled or restricted
localStorage cannot crash the dashboard.
In `@web/src/pages/ModelsPage.tsx`:
- Around line 925-936: Make each model row rendered by the rows.map selection
flow keyboard-accessible by adding a focusable control, preferably within the
model cell, that invokes onSelect(row.key) on Enter and Space while preserving
click selection and the existing row display. Ensure the control has appropriate
button semantics and does not trigger selection twice through event bubbling.
---
Outside diff comments:
In `@src/gateway/api/routes/models.py`:
- Around line 338-363: Update the model merge flow around phase 1 and
_alias_target_keys so alias_targets is computed before iterating discovered
models. Skip discovered entries whose model_key matches an alias target, while
preserving normal publication for other discovered models and the existing phase
2 suppression for pricing-only entries.
In `@src/gateway/services/pricing_service.py`:
- Around line 84-86: Update the pricing lookup flow around the attempts
construction so a supplied provider never falls back to the provider-agnostic
(None, model) lookup. Resolve configured or custom provider IDs to a known
provider type before pricing lookup, and only append/use the agnostic attempt
when no provider was supplied; otherwise fail closed when authoritative pricing
is unavailable.
---
Minor comments:
In `@tests/integration/test_alias_api.py`:
- Around line 49-61: Update the fixture teardown around the synchronous engine
created by create_engine to call engine.dispose() after dropping the tables and
alembic_version metadata, ensuring pooled connections are released while
preserving the existing cleanup order.
In `@tests/integration/test_model_aliases.py`:
- Around line 68-82: Update the _build_client fixture cleanup to dispose the
locally created synchronous engine after dropping the tables and alembic_version
metadata. Call engine.dispose() in the finally path after all database
operations complete.
In `@web/src/api/client.ts`:
- Around line 85-88: Update the response-status handling in the API client so
the existing unauthorizedHandler is invoked for both 401 and 403 responses,
while preserving the current ApiError behavior and extracted error message.
In `@web/src/components/UpdatePrompt.test.tsx`:
- Around line 57-63: Update the test containing “offers a reload once the
gateway serves a different build” to capture window.location’s original property
descriptor before Object.defineProperty replaces it, then restore that
descriptor in afterEach or a finally block so the mocked location and reload
function cannot leak into subsequent tests.
In `@web/src/lib/format.ts`:
- Around line 22-36: Update formatContext so the thousands branch promotes
values whose rounded result reaches 1000K into the millions representation,
preventing sub-million inputs such as 999999 from rendering as 1000K. Preserve
the existing formatting for nullish, ordinary thousands, and exact
million-or-higher values.
In `@web/src/pages/SettingsPage.test.tsx`:
- Around line 62-91: Update both tests, “patches a setting when its switch is
toggled” and “enables default pricing from off,” to wait for the settings-loaded
state before clicking. Use the loaded version marker or wait until the
corresponding switch is enabled, then perform the existing click and assertions.
---
Nitpick comments:
In `@tests/integration/test_model_aliases.py`:
- Around line 104-106: Remove the repeated-hyphen separator lines around the
descriptive section headings at tests/integration/test_model_aliases.py lines
104-106, 138-140, 373-375, and 484-486, and tests/unit/test_model_aliases.py
lines 13-15, 31-33, 108-110, and 151-153; retain each heading text unchanged.
🪄 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
Run ID: 167d2fb3-56bc-400e-a95f-41399702a49e
⛔ Files ignored due to path filters (1)
docs/public/openapi.jsonis excluded by!docs/public/openapi.json
📒 Files selected for processing (68)
.github/workflows/otari-dashboard.yml.gitignoreAGENTS.mdREADME.mdalembic/env.pyalembic/versions/a2c4e6f8b0d1_add_runtime_settings.pyalembic/versions/f1a3b5c7d9e2_add_model_aliases.pydeploy/railway/README.mddocs/public/otari.postman_collection.jsonpyproject.tomlsrc/gateway/api/main.pysrc/gateway/api/routes/aliases.pysrc/gateway/api/routes/models.pysrc/gateway/api/routes/pricing.pysrc/gateway/api/routes/providers.pysrc/gateway/api/routes/settings.pysrc/gateway/core/config.pysrc/gateway/core/database.pysrc/gateway/dashboard.pysrc/gateway/main.pysrc/gateway/models/entities.pysrc/gateway/services/alias_service.pysrc/gateway/services/model_catalog_service.pysrc/gateway/services/model_discovery_service.pysrc/gateway/services/pricing_service.pysrc/gateway/services/provider_kwargs.pysrc/gateway/services/provider_metadata_service.pysrc/gateway/services/runtime_settings_service.pysrc/gateway/static/dashboard/assets/index-CBdXIuFr.jssrc/gateway/static/dashboard/assets/index-CeA0apWS.csssrc/gateway/static/dashboard/index.htmltests/integration/test_alias_api.pytests/integration/test_cache_control_headers.pytests/integration/test_model_aliases.pytests/integration/test_model_discovery.pytests/integration/test_model_metadata_endpoint.pytests/integration/test_providers_endpoint.pytests/integration/test_settings_toggle.pytests/unit/test_alias_service.pytests/unit/test_dashboard_build.pytests/unit/test_database_async_url_migrations.pytests/unit/test_model_aliases.pytests/unit/test_model_catalog_service.pytests/unit/test_models_default_pricing.pytests/unit/test_provider_metadata.pytests/unit/test_settings_endpoint.pyweb/README.mdweb/index.htmlweb/src/App.tsxweb/src/api/client.tsweb/src/api/hooks.tsweb/src/api/types.tsweb/src/auth/AuthContext.tsxweb/src/components/AppShell.tsxweb/src/components/Field.tsxweb/src/components/Login.tsxweb/src/components/ModelComboBox.tsxweb/src/components/Table.tsxweb/src/components/UpdatePrompt.test.tsxweb/src/components/UpdatePrompt.tsxweb/src/lib/format.test.tsweb/src/lib/format.tsweb/src/pages/ModelsPage.test.tsxweb/src/pages/ModelsPage.tsxweb/src/pages/SettingsPage.test.tsxweb/src/pages/SettingsPage.tsxweb/src/styles/globals.cssweb/vite.config.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- web/index.html
- deploy/railway/README.md
- web/src/lib/format.test.ts
- web/README.md
- web/src/components/Field.tsx
- web/vite.config.ts
- src/gateway/static/dashboard/index.html
- web/src/components/Table.tsx
- AGENTS.md
- web/src/auth/AuthContext.tsx
- pyproject.toml
- web/src/components/Login.tsx
- src/gateway/main.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/gateway/services/pricing_service.py (1)
84-86: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not silently adopt another provider's pricing.
When a configured or custom provider ID misses, the provider-agnostic retry can match the same model name under an unrelated provider. That can undercharge or overcharge requests instead of failing closed. Resolve configured instances to a known provider type first, or only use the agnostic lookup when no provider was supplied.
As per coding guidelines, billable code must fail closed for models without authoritative pricing.
🤖 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 `@src/gateway/services/pricing_service.py` around lines 84 - 86, Update the pricing lookup flow around the attempts construction so a supplied provider never falls back to the provider-agnostic (None, model) lookup. Resolve configured or custom provider IDs to a known provider type before pricing lookup, and only append/use the agnostic attempt when no provider was supplied; otherwise fail closed when authoritative pricing is unavailable.Source: Coding guidelines
src/gateway/api/routes/models.py (1)
338-363: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winHide discovered alias targets as well as pricing-only targets.
Phase 2 suppresses alias targets, but phase 1 has already published any discovered target. With model discovery enabled, callers therefore receive both the alias and the provider/model name the alias is intended to hide. Compute
alias_targetsbefore phase 1 and skip matching discovered keys there too.Proposed direction
aliases = effective_aliases(config) + alias_targets = _alias_target_keys(config, aliases) merged: dict[str, ModelObject] = {} if config.model_discovery: ... for provider_name, model in discovered: model_key = f"{provider_name}:{model.id}" + if normalize_pricing_key(config, model_key) in alias_targets: + continue pricing = pricing_map.pop(model_key, None) ... - alias_targets = _alias_target_keys(config, aliases) for model_key, pricing in pricing_map.items():🤖 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 `@src/gateway/api/routes/models.py` around lines 338 - 363, Update the model merge flow around phase 1 and _alias_target_keys so alias_targets is computed before iterating discovered models. Skip discovered entries whose model_key matches an alias target, while preserving normal publication for other discovered models and the existing phase 2 suppression for pricing-only entries.
🟡 Minor comments (6)
web/src/components/UpdatePrompt.test.tsx-57-63 (1)
57-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore
window.locationafter replacing it.
vi.restoreAllMocks()does not undo this direct property replacement, so the mocked location and reload function leak into later tests. Save the original descriptor and restore it inafterEachor afinallyblock.🤖 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/components/UpdatePrompt.test.tsx` around lines 57 - 63, Update the test containing “offers a reload once the gateway serves a different build” to capture window.location’s original property descriptor before Object.defineProperty replaces it, then restore that descriptor in afterEach or a finally block so the mocked location and reload function cannot leak into subsequent tests.web/src/api/client.ts-85-88 (1)
85-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear the invalid session on 403 as well.
The validation path treats both 401 and 403 as invalid admin credentials, but subsequent API calls only log out on 401. A restored or downgraded non-master key can therefore leave the dashboard stuck in an authenticated state with every request failing.
Proposed fix
- if (response.status === 401) { + if (response.status === 401 || response.status === 403) { unauthorizedHandler?.(); - throw new ApiError(401, await extractErrorMessage(response)); + throw new ApiError(response.status, await extractErrorMessage(response)); }🤖 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 85 - 88, Update the response-status handling in the API client so the existing unauthorizedHandler is invoked for both 401 and 403 responses, while preserving the current ApiError behavior and extracted error message.web/src/lib/format.ts-22-36 (1)
22-36: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid rendering sub-million values as
1000K.Rounding makes values such as
999999display as1000Kinstead of1M. Promote the rounded boundary to the next unit.Proposed fix
if (value >= 1000) { - return `${Math.round(value / 1000)}K`; + const thousands = Math.round(value / 1000); + return thousands >= 1000 ? "1M" : `${thousands}K`; }🤖 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/lib/format.ts` around lines 22 - 36, Update formatContext so the thousands branch promotes values whose rounded result reaches 1000K into the millions representation, preventing sub-million inputs such as 999999 from rendering as 1000K. Preserve the existing formatting for nullish, ordinary thousands, and exact million-or-higher values.web/src/pages/SettingsPage.test.tsx-62-91 (1)
62-91: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWait for settings to load before clicking the switches.
The switches render disabled before data arrives, so
findByRolecan resolve too early and the click can be ignored. Wait for the loaded version marker or for the switch to become enabled in both tests.Proposed fix
renderWithClient(<SettingsPage />); - await screen.findByRole("switch", { name: "Model discovery" }); + await screen.findByText(/Version 1.2.3/); await user.click(screen.getByRole("switch", { name: "Model discovery" }));renderWithClient(<SettingsPage />); - await screen.findByRole("switch", { name: "Default pricing" }); + await screen.findByText(/Version 1.2.3/); await user.click(screen.getByRole("switch", { name: "Default pricing" }));🤖 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/SettingsPage.test.tsx` around lines 62 - 91, Update both tests, “patches a setting when its switch is toggled” and “enables default pricing from off,” to wait for the settings-loaded state before clicking. Use the loaded version marker or wait until the corresponding switch is enabled, then perform the existing click and assertions.tests/integration/test_alias_api.py-49-61 (1)
49-61: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDispose the synchronous engine during teardown.
The fixture drops its tables but never calls
engine.dispose(), so repeated tests can retain pooled database connections.Proposed fix
finally: dispose_override() Base.metadata.drop_all(bind=engine) with engine.connect() as conn: conn.execute(text("DROP TABLE IF EXISTS alembic_version CASCADE")) conn.commit() + engine.dispose()🤖 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/integration/test_alias_api.py` around lines 49 - 61, Update the fixture teardown around the synchronous engine created by create_engine to call engine.dispose() after dropping the tables and alembic_version metadata, ensuring pooled connections are released while preserving the existing cleanup order.tests/integration/test_model_aliases.py-68-82 (1)
68-82: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDispose the synchronous engine during fixture cleanup.
Each client fixture creates a new pooled engine, but cleanup never calls
engine.dispose(). Across this large module, idle PostgreSQL connections can accumulate and destabilize CI.Proposed cleanup
finally: dispose_override() - Base.metadata.drop_all(bind=engine) - with engine.connect() as conn: - conn.execute(text("DROP TABLE IF EXISTS alembic_version CASCADE")) - conn.commit() + try: + Base.metadata.drop_all(bind=engine) + with engine.connect() as conn: + conn.execute(text("DROP TABLE IF EXISTS alembic_version CASCADE")) + conn.commit() + finally: + engine.dispose()🤖 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/integration/test_model_aliases.py` around lines 68 - 82, Update the _build_client fixture cleanup to dispose the locally created synchronous engine after dropping the tables and alembic_version metadata. Call engine.dispose() in the finally path after all database operations complete.
🧹 Nitpick comments (1)
tests/integration/test_model_aliases.py (1)
104-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the decorative repeated-hyphen separators from the alias tests.
tests/integration/test_model_aliases.py#L104-L106: retain only the descriptive section heading.tests/integration/test_model_aliases.py#L138-L140: retain only the descriptive section heading.tests/integration/test_model_aliases.py#L373-L375: retain only the descriptive section heading.tests/integration/test_model_aliases.py#L484-L486: retain only the descriptive section heading.tests/unit/test_model_aliases.py#L13-L15: retain only the descriptive section heading.tests/unit/test_model_aliases.py#L31-L33: retain only the descriptive section heading.tests/unit/test_model_aliases.py#L108-L110: retain only the descriptive section heading.tests/unit/test_model_aliases.py#L151-L153: retain only the descriptive section heading.As per coding guidelines, “Avoid em dashes and double hyphens as prose separators in documentation and comments.”
🤖 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/integration/test_model_aliases.py` around lines 104 - 106, Remove the repeated-hyphen separator lines around the descriptive section headings at tests/integration/test_model_aliases.py lines 104-106, 138-140, 373-375, and 484-486, and tests/unit/test_model_aliases.py lines 13-15, 31-33, 108-110, and 151-153; retain each heading text unchanged.Source: Coding guidelines
🤖 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 `@alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py`:
- Line 27: Add database-side timestamp defaults to the non-nullable columns:
update updated_at in alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py lines
27-27, and both created_at and updated_at in
alembic/versions/f1a3b5c7d9e2_add_model_aliases.py lines 27-28, using
server_default=sa.text("CURRENT_TIMESTAMP").
In `@src/gateway/api/routes/aliases.py`:
- Around line 112-124: Update the post-commit cache handling in the alias
mutation flow around refresh_alias_cache so cache-refresh failures cannot turn a
successful create, update, or delete into an HTTP 500. Prefer updating the local
cache from the known mutation; otherwise catch and isolate refresh_alias_cache
failures while allowing the committed response to return normally. Apply the
same change to the related mutation block noted at lines 141-150.
- Around line 102-110: Update the mutation flow around refresh_alias_cache and
_validate to serialize alias writes across workers using the project’s existing
transaction/locking mechanism. Within the serialized transaction, reload the
current alias state, run _validate against that freshly loaded state, then
perform the existing update-or-create logic for ModelAlias so concurrent
requests cannot commit an invalid alias chain.
In `@src/gateway/api/routes/models.py`:
- Around line 404-410: Update the provider mapping in the model discovery route
around discover_models_with_status and DiscoverableProvider so discovery.error
is replaced with the stable client-facing message “Provider discovery failed”
when present, while preserving the success case. Log only sanitized diagnostic
metadata server-side and never emit the raw upstream error in responses or logs.
In `@src/gateway/api/routes/pricing.py`:
- Around line 127-144: Update the pricing validation around
resolve_effective_alias so stored aliases are checked through the current
database session’s ModelAlias query, while config.aliases is checked directly
for configured aliases. Reject any matching alias before creating or updating
pricing, rather than relying solely on the potentially stale
resolve_effective_alias cache.
In `@src/gateway/api/routes/settings.py`:
- Around line 83-90: Update the updates transaction handling around
stage_override and db.commit to roll back on SQLAlchemyError, then raise a
generic HTTPException with an appropriate public error status and message
instead of re-raising the database exception or exposing its details.
In `@src/gateway/core/config.py`:
- Around line 732-749: Update the YAML-to-environment bridging flow around
_bridge_yaml_fields_to_env so values injected from YAML do not appear as
explicit environment overrides during later load_config calls. Preserve
caller-provided environment values separately and restore or exclude bridged
values before subsequent loads, or pass the loaded GatewayConfig directly to
remaining read sites; ensure a later YAML configuration can replace values from
an earlier load.
- Around line 407-412: Change the default value of the mcp_allow_loopback
configuration field to False so loopback MCP requests are denied unless
explicitly enabled by operators. Preserve the existing field name, environment
alias, and description context.
In `@src/gateway/services/model_discovery_service.py`:
- Around line 142-152: Update the provider-error handling around _short_error
and the affected lines 183–187 so raw exception text is neither returned nor
logged. Return a fixed safe message through the API, and log only the provider
instance/context and exception type, excluding exc and _short_error output from
logs or responses.
In `@src/gateway/services/pricing_service.py`:
- Around line 223-232: Update flat_request_cost so it only returns early when
pricing is None, not when pricing.input_price_per_million is zero; always divide
the present rate by 1_000_000 so legitimate zero-valued pricing remains handled
through the normal calculation.
In `@src/gateway/services/runtime_settings_service.py`:
- Around line 8-11: Update the runtime billing override flow around
default_pricing so changes propagate to every worker rather than only the
handling worker. Add shared invalidation or polling, following the existing
alias refresher pattern, and ensure each worker refreshes the stored billing
setting and applies it to its in-memory configuration and process-wide pricing
flag.
In `@tests/integration/test_alias_api.py`:
- Around line 231-236: Update the /v1/chat/completions handler to catch the
selector-resolution ValueError and raise an HTTPException with status 400 and
the existing “Invalid model format” detail. In the test around client.post,
replace pytest.raises(ValueError) with assertions on the returned response
status code and detail.
In `@web/src/api/hooks.ts`:
- Around line 151-155: Update usePricing to retrieve all pricing pages using the
API’s skip and limit parameters, combining results until no additional prices
remain. Preserve the existing query behavior for the dashboard while ensuring
pricing records beyond the first 1000 are accessible.
In `@web/src/components/AppShell.tsx`:
- Around line 21-30: Make the sidebar preference storage access in
readStoredSidebarWidth and readStoredCollapsed defensive by wrapping
localStorage reads in try/catch and returning DEFAULT_SIDEBAR or false on
failure. Apply the same protection to the corresponding sidebar preference
writes around lines 81-90, swallowing storage errors so disabled or restricted
localStorage cannot crash the dashboard.
In `@web/src/pages/ModelsPage.tsx`:
- Around line 925-936: Make each model row rendered by the rows.map selection
flow keyboard-accessible by adding a focusable control, preferably within the
model cell, that invokes onSelect(row.key) on Enter and Space while preserving
click selection and the existing row display. Ensure the control has appropriate
button semantics and does not trigger selection twice through event bubbling.
---
Outside diff comments:
In `@src/gateway/api/routes/models.py`:
- Around line 338-363: Update the model merge flow around phase 1 and
_alias_target_keys so alias_targets is computed before iterating discovered
models. Skip discovered entries whose model_key matches an alias target, while
preserving normal publication for other discovered models and the existing phase
2 suppression for pricing-only entries.
In `@src/gateway/services/pricing_service.py`:
- Around line 84-86: Update the pricing lookup flow around the attempts
construction so a supplied provider never falls back to the provider-agnostic
(None, model) lookup. Resolve configured or custom provider IDs to a known
provider type before pricing lookup, and only append/use the agnostic attempt
when no provider was supplied; otherwise fail closed when authoritative pricing
is unavailable.
---
Minor comments:
In `@tests/integration/test_alias_api.py`:
- Around line 49-61: Update the fixture teardown around the synchronous engine
created by create_engine to call engine.dispose() after dropping the tables and
alembic_version metadata, ensuring pooled connections are released while
preserving the existing cleanup order.
In `@tests/integration/test_model_aliases.py`:
- Around line 68-82: Update the _build_client fixture cleanup to dispose the
locally created synchronous engine after dropping the tables and alembic_version
metadata. Call engine.dispose() in the finally path after all database
operations complete.
In `@web/src/api/client.ts`:
- Around line 85-88: Update the response-status handling in the API client so
the existing unauthorizedHandler is invoked for both 401 and 403 responses,
while preserving the current ApiError behavior and extracted error message.
In `@web/src/components/UpdatePrompt.test.tsx`:
- Around line 57-63: Update the test containing “offers a reload once the
gateway serves a different build” to capture window.location’s original property
descriptor before Object.defineProperty replaces it, then restore that
descriptor in afterEach or a finally block so the mocked location and reload
function cannot leak into subsequent tests.
In `@web/src/lib/format.ts`:
- Around line 22-36: Update formatContext so the thousands branch promotes
values whose rounded result reaches 1000K into the millions representation,
preventing sub-million inputs such as 999999 from rendering as 1000K. Preserve
the existing formatting for nullish, ordinary thousands, and exact
million-or-higher values.
In `@web/src/pages/SettingsPage.test.tsx`:
- Around line 62-91: Update both tests, “patches a setting when its switch is
toggled” and “enables default pricing from off,” to wait for the settings-loaded
state before clicking. Use the loaded version marker or wait until the
corresponding switch is enabled, then perform the existing click and assertions.
---
Nitpick comments:
In `@tests/integration/test_model_aliases.py`:
- Around line 104-106: Remove the repeated-hyphen separator lines around the
descriptive section headings at tests/integration/test_model_aliases.py lines
104-106, 138-140, 373-375, and 484-486, and tests/unit/test_model_aliases.py
lines 13-15, 31-33, 108-110, and 151-153; retain each heading text unchanged.
🪄 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
Run ID: 167d2fb3-56bc-400e-a95f-41399702a49e
⛔ Files ignored due to path filters (1)
docs/public/openapi.jsonis excluded by!docs/public/openapi.json
📒 Files selected for processing (68)
.github/workflows/otari-dashboard.yml.gitignoreAGENTS.mdREADME.mdalembic/env.pyalembic/versions/a2c4e6f8b0d1_add_runtime_settings.pyalembic/versions/f1a3b5c7d9e2_add_model_aliases.pydeploy/railway/README.mddocs/public/otari.postman_collection.jsonpyproject.tomlsrc/gateway/api/main.pysrc/gateway/api/routes/aliases.pysrc/gateway/api/routes/models.pysrc/gateway/api/routes/pricing.pysrc/gateway/api/routes/providers.pysrc/gateway/api/routes/settings.pysrc/gateway/core/config.pysrc/gateway/core/database.pysrc/gateway/dashboard.pysrc/gateway/main.pysrc/gateway/models/entities.pysrc/gateway/services/alias_service.pysrc/gateway/services/model_catalog_service.pysrc/gateway/services/model_discovery_service.pysrc/gateway/services/pricing_service.pysrc/gateway/services/provider_kwargs.pysrc/gateway/services/provider_metadata_service.pysrc/gateway/services/runtime_settings_service.pysrc/gateway/static/dashboard/assets/index-CBdXIuFr.jssrc/gateway/static/dashboard/assets/index-CeA0apWS.csssrc/gateway/static/dashboard/index.htmltests/integration/test_alias_api.pytests/integration/test_cache_control_headers.pytests/integration/test_model_aliases.pytests/integration/test_model_discovery.pytests/integration/test_model_metadata_endpoint.pytests/integration/test_providers_endpoint.pytests/integration/test_settings_toggle.pytests/unit/test_alias_service.pytests/unit/test_dashboard_build.pytests/unit/test_database_async_url_migrations.pytests/unit/test_model_aliases.pytests/unit/test_model_catalog_service.pytests/unit/test_models_default_pricing.pytests/unit/test_provider_metadata.pytests/unit/test_settings_endpoint.pyweb/README.mdweb/index.htmlweb/src/App.tsxweb/src/api/client.tsweb/src/api/hooks.tsweb/src/api/types.tsweb/src/auth/AuthContext.tsxweb/src/components/AppShell.tsxweb/src/components/Field.tsxweb/src/components/Login.tsxweb/src/components/ModelComboBox.tsxweb/src/components/Table.tsxweb/src/components/UpdatePrompt.test.tsxweb/src/components/UpdatePrompt.tsxweb/src/lib/format.test.tsweb/src/lib/format.tsweb/src/pages/ModelsPage.test.tsxweb/src/pages/ModelsPage.tsxweb/src/pages/SettingsPage.test.tsxweb/src/pages/SettingsPage.tsxweb/src/styles/globals.cssweb/vite.config.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- web/index.html
- deploy/railway/README.md
- web/src/lib/format.test.ts
- web/README.md
- web/src/components/Field.tsx
- web/vite.config.ts
- src/gateway/static/dashboard/index.html
- web/src/components/Table.tsx
- AGENTS.md
- web/src/auth/AuthContext.tsx
- pyproject.toml
- web/src/components/Login.tsx
- src/gateway/main.py
🛑 Comments failed to post (15)
alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py (1)
27-27: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add database-side defaults to the new non-null timestamp columns. ORM defaults do not apply to direct SQL or migration operations.
alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py#L27-L27: addserver_default=sa.text("CURRENT_TIMESTAMP")toupdated_at.alembic/versions/f1a3b5c7d9e2_add_model_aliases.py#L27-L28: add the same server default tocreated_atandupdated_at.As per coding guidelines, new non-nullable columns must define a
server_default.📍 Affects 2 files
alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py#L27-L27(this comment)alembic/versions/f1a3b5c7d9e2_add_model_aliases.py#L27-L28🤖 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 `@alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py` at line 27, Add database-side timestamp defaults to the non-nullable columns: update updated_at in alembic/versions/a2c4e6f8b0d1_add_runtime_settings.py lines 27-27, and both created_at and updated_at in alembic/versions/f1a3b5c7d9e2_add_model_aliases.py lines 27-28, using server_default=sa.text("CURRENT_TIMESTAMP").Source: Coding guidelines
src/gateway/api/routes/aliases.py (2)
102-110: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize alias validation with alias writes.
Two concurrent requests can validate against snapshots taken before either commit. For example, creating
aandb -> a:modelconcurrently can leave a forbidden alias chain in storage. Serialize mutations across workers, then reload and validate inside that transaction.🤖 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 `@src/gateway/api/routes/aliases.py` around lines 102 - 110, Update the mutation flow around refresh_alias_cache and _validate to serialize alias writes across workers using the project’s existing transaction/locking mechanism. Within the serialized transaction, reload the current alias state, run _validate against that freshly loaded state, then perform the existing update-or-create logic for ModelAlias so concurrent requests cannot commit an invalid alias chain.
112-124: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not fail the request after the mutation has committed.
refresh_alias_cache()can raise after the commit. The client then receives a 500 although the create, update, or delete succeeded; retrying a delete returns 404. Update the local cache from the known mutation, or catch refresh failures and let the background refresher converge.Also applies to: 141-150
🤖 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 `@src/gateway/api/routes/aliases.py` around lines 112 - 124, Update the post-commit cache handling in the alias mutation flow around refresh_alias_cache so cache-refresh failures cannot turn a successful create, update, or delete into an HTTP 500. Prefer updating the local cache from the known mutation; otherwise catch and isolate refresh_alias_cache failures while allowing the committed response to return normally. Apply the same change to the related mutation block noted at lines 141-150.src/gateway/api/routes/models.py (1)
404-410: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not return raw provider discovery errors.
discovery.errororiginates from provider calls and is copied directly into the response. Return a stable message such as"Provider discovery failed"and keep sanitized diagnostic metadata in server logs.As per coding guidelines, raw upstream provider errors must not be exposed in responses or 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 `@src/gateway/api/routes/models.py` around lines 404 - 410, Update the provider mapping in the model discovery route around discover_models_with_status and DiscoverableProvider so discovery.error is replaced with the stable client-facing message “Provider discovery failed” when present, while preserving the success case. Log only sanitized diagnostic metadata server-side and never emit the raw upstream error in responses or logs.Source: Coding guidelines
src/gateway/api/routes/pricing.py (1)
127-144: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Check stored aliases against the database, not the eventual cache.
resolve_effective_alias()can be stale for 30 seconds on another worker. A newly created alias may pass this guard and receive a pricing row keyed by its display name, which becomes unreachable after cache refresh. QueryModelAliasusing this session while checkingconfig.aliasesdirectly.🤖 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 `@src/gateway/api/routes/pricing.py` around lines 127 - 144, Update the pricing validation around resolve_effective_alias so stored aliases are checked through the current database session’s ModelAlias query, while config.aliases is checked directly for configured aliases. Reject any matching alias before creating or updating pricing, rather than relying solely on the potentially stale resolve_effective_alias cache.src/gateway/api/routes/settings.py (1)
83-90: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Map database failures to an explicit public API error.
The bare re-raise delegates SQLAlchemy exceptions to framework and debug handling. Roll back, then return a generic
HTTPExceptionwithout exposing database details.Proposed fix
-from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, HTTPException, status ... except SQLAlchemyError: await db.rollback() - raise + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Database error", + ) from NoneAs per coding guidelines, API routes must raise explicit
HTTPExceptioninstances without exposing implementation details.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if updates: try: for key, value in updates.items(): await stage_override(db, key, value) await db.commit() except SQLAlchemyError: await db.rollback() raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Database error", ) from None🤖 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 `@src/gateway/api/routes/settings.py` around lines 83 - 90, Update the updates transaction handling around stage_override and db.commit to roll back on SQLAlchemyError, then raise a generic HTTPException with an appropriate public error status and message instead of re-raising the database exception or exposing its details.Source: Coding guidelines
src/gateway/core/config.py (2)
407-412: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Default the loopback SSRF gate to closed.
mcp_allow_loopback=Truepermits requests to localhost services in a default deployment. Default this toFalse; operators using trusted sidecars can opt in.Proposed fix
mcp_allow_loopback: bool = Field( - default=True, + default=False,As per coding guidelines, security-affecting configuration flags must fail closed by default.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.mcp_allow_loopback: bool = Field( default=False, description=( "SSRF gate: allow MCP server URLs that resolve to loopback (useful for same-host " "sidecars). On by default. Legacy env alias: GATEWAY_MCP_ALLOW_LOOPBACK." ),🤖 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 `@src/gateway/core/config.py` around lines 407 - 412, Change the default value of the mcp_allow_loopback configuration field to False so loopback MCP requests are denied unless explicitly enabled by operators. Preserve the existing field name, environment alias, and description context.Source: Coding guidelines
732-749: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not let bridged YAML values contaminate later configuration loads.
setdefault()leaves each YAML value inos.environ. A subsequentload_config()then treats that stale value as an explicit environment override, so another YAML file cannot replace it. Track caller-provided environment values separately, or passGatewayConfigdirectly to the remaining read sites.Also applies to: 785-802
🤖 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 `@src/gateway/core/config.py` around lines 732 - 749, Update the YAML-to-environment bridging flow around _bridge_yaml_fields_to_env so values injected from YAML do not appear as explicit environment overrides during later load_config calls. Preserve caller-provided environment values separately and restore or exclude bridged values before subsequent loads, or pass the loaded GatewayConfig directly to remaining read sites; ensure a later YAML configuration can replace values from an earlier load.src/gateway/services/model_discovery_service.py (1)
142-152: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not return or log raw provider exceptions.
Line 186 logs
exc, and Line 187 returns it through the API. Provider errors can contain credentials, endpoint details, or upstream payloads. Return a fixed message and log only the instance and exception type.Proposed fix
except Exception as exc: - logger.info("Model discovery failed for instance '%s': %s", instance, exc) - return ProviderDiscovery(provider=instance, models=[], error=_short_error(exc)) + logger.info( + "Model discovery failed for instance '%s' (%s)", + instance, + type(exc).__name__, + ) + return ProviderDiscovery( + provider=instance, + models=[], + error="Model discovery failed. Check the provider configuration.", + )As per coding guidelines, raw upstream provider errors must not appear in responses or logs.
Also applies to: 183-187
🤖 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 `@src/gateway/services/model_discovery_service.py` around lines 142 - 152, Update the provider-error handling around _short_error and the affected lines 183–187 so raw exception text is neither returned nor logged. Return a fixed safe message through the API, and log only the provider instance/context and exception type, excluding exc and _short_error output from logs or responses.Source: Coding guidelines
src/gateway/services/pricing_service.py (1)
223-232: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the falsy-zero billing check.
A zero rate is legitimate and already evaluates to zero after division. Only the absent pricing object needs special handling.
Proposed fix
def flat_request_cost(pricing: ModelPricing | None) -> float: - if pricing is None or not pricing.input_price_per_million: + if pricing is None: return 0.0 return pricing.input_price_per_million / 1_000_000As per coding guidelines, billing checks must distinguish missing values from legitimate zero values.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.def flat_request_cost(pricing: ModelPricing | None) -> float: """Flat USD cost of one request for a model priced per request. Moderations convention: ``input_price_per_million`` stores the per-request rate scaled by 1e6 (USD per million requests), so one request costs the stored rate divided by 1e6. Unpriced models are treated as free. """ if pricing is None: return 0.0 return pricing.input_price_per_million / 1_000_000🤖 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 `@src/gateway/services/pricing_service.py` around lines 223 - 232, Update flat_request_cost so it only returns early when pricing is None, not when pricing.input_price_per_million is zero; always divide the present rate by 1_000_000 so legitimate zero-valued pricing remains handled through the normal calculation.Source: Coding guidelines
src/gateway/services/runtime_settings_service.py (1)
8-11: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Propagate runtime billing settings to every worker.
Only the handling worker updates
default_pricing; other workers retain the previous billing policy until restart. Identical requests can therefore be billed differently indefinitely. Add shared invalidation or polling similar to the alias refresher.Also applies to: 44-50
🤖 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 `@src/gateway/services/runtime_settings_service.py` around lines 8 - 11, Update the runtime billing override flow around default_pricing so changes propagate to every worker rather than only the handling worker. Add shared invalidation or polling, following the existing alias refresher pattern, and ensure each worker refreshes the stored billing setting and applies it to its in-memory configuration and process-wide pricing flag.tests/integration/test_alias_api.py (1)
231-236: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== File inventory ==\n' git ls-files tests/integration/test_alias_api.py git ls-files | rg '(^|/)(api|alias|model).*(py)$|(^|/)main\.py$|(^|/)app\.py$' || true printf '\n== Outline of target test file ==\n' ast-grep outline tests/integration/test_alias_api.py --view expanded || true printf '\n== Relevant snippets from target test file ==\n' sed -n '180,280p' tests/integration/test_alias_api.py printf '\n== Search for Invalid model format ==\n' rg -n "Invalid model format|model format|HTTPException|ValueError" tests -g '*.py' || true printf '\n== Search for chat/completions handler and model resolution ==\n' rg -n "/v1/chat/completions|resolve.*model|model.*resolve|fast" . -g '*.py' || trueRepository: mozilla-ai/otari
Length of output: 50372
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Relevant implementation files (if any) ==\n' rg -n "Invalid model format|HTTPException|ValueError|chat/completions|resolve.*model|model.*resolve" . -g '*.py' -g '*.ts' -g '*.js' || trueRepository: mozilla-ai/otari
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -euo pipefail # Focus on the endpoint and the alias/model resolution path once the file list is known. for f in $(git ls-files | rg '(^|/)(api|alias|model).*(py)$|(^|/)main\.py$|(^|/)app\.py$' || true); do echo "### $f" ast-grep outline "$f" --view expanded || true doneRepository: mozilla-ai/otari
Length of output: 48270
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Target test snippet ==" sed -n '220,250p' tests/integration/test_alias_api.py echo echo "== Find the API handler and model-resolution helpers ==" rg -n "chat/completions|Invalid model format|HTTPException|ValueError|resolve.*model|model.*format|alias" . -g '*.py' echo echo "== Narrow file list around likely implementation files ==" git ls-files | rg '(^|/)(api|alias|model|chat).*\.py$|(^|/)main\.py$|(^|/)app\.py$'Repository: mozilla-ai/otari
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== test_alias_api around the failing test ==" sed -n '216,240p' tests/integration/test_alias_api.py echo echo "== chat route around model resolution ==" sed -n '330,410p' src/gateway/api/routes/chat.py echo echo "== pipeline resolver helper ==" sed -n '398,430p' src/gateway/api/routes/_pipeline.py echo echo "== provider selector resolver ==" sed -n '119,190p' src/gateway/services/provider_kwargs.py echo echo "== TestClient setup in integration conftest ==" sed -n '1,120p' tests/integration/conftest.pyRepository: mozilla-ai/otari
Length of output: 12433
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== lines 216-240 of tests/integration/test_alias_api.py ==" nl -ba tests/integration/test_alias_api.py | sed -n '216,240p' echo echo "== lines 330-410 of src/gateway/api/routes/chat.py ==" nl -ba src/gateway/api/routes/chat.py | sed -n '330,410p' echo echo "== lines 398-430 of src/gateway/api/routes/_pipeline.py ==" nl -ba src/gateway/api/routes/_pipeline.py | sed -n '398,430p' echo echo "== lines 119-190 of src/gateway/services/provider_kwargs.py ==" nl -ba src/gateway/services/provider_kwargs.py | sed -n '119,190p' echo echo "== lines 1-120 of tests/integration/conftest.py ==" nl -ba tests/integration/conftest.py | sed -n '1,120p'Repository: mozilla-ai/otari
Length of output: 253
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path files = [ ("tests/integration/test_alias_api.py", 216, 240), ("src/gateway/api/routes/chat.py", 330, 410), ("src/gateway/api/routes/_pipeline.py", 398, 430), ("src/gateway/services/provider_kwargs.py", 119, 190), ("tests/integration/conftest.py", 1, 120), ] for path, start, end in files: print(f"== {path} {start}-{end} ==") lines = Path(path).read_text().splitlines() for i in range(start - 1, min(end, len(lines))): print(f"{i+1:>4}: {lines[i]}") print() PYRepository: mozilla-ai/otari
Length of output: 14308
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path targets = [ ("src/gateway/api/routes/chat.py", 340, 390), ("tests/integration/conftest.py", 180, 320), ("src/gateway/api/routes/_passthrough.py", 150, 250), ] for path, start, end in targets: print(f"== {path} {start}-{end} ==") lines = Path(path).read_text().splitlines() for i in range(start - 1, min(end, len(lines))): print(f"{i+1:>4}: {lines[i]}") print() PYRepository: mozilla-ai/otari
Length of output: 13190
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path for path, needle in [ ("src/gateway/api/routes/chat.py", "except ValueError"), ("src/gateway/api/routes/chat.py", "resolve_dispatch_provider("), ("tests/integration/conftest.py", "TestClient("), ("tests/integration/conftest.py", "raise_server_exceptions"), ]: print(f"== {path}: {needle} ==") lines = Path(path).read_text().splitlines() for i, line in enumerate(lines, 1): if needle in line: start = max(1, i - 8) end = min(len(lines), i + 12) for j in range(start, end + 1): print(f"{j:>4}: {lines[j-1]}") print() PYRepository: mozilla-ai/otari
Length of output: 3135
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path path = Path("src/gateway/api/routes/chat.py") lines = path.read_text().splitlines() needles = ["except ValueError", "resolve_dispatch_provider(", "chat_completions(", "HTTPException("] for needle in needles: print(f"== {needle} ==") for i, line in enumerate(lines, 1): if needle in line: start = max(1, i - 10) end = min(len(lines), i + 16) for j in range(start, end + 1): print(f"{j:>4}: {lines[j-1]}") print() PYRepository: mozilla-ai/otari
Length of output: 11596
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== chat.py around resolve_dispatch_provider call ==" rg -n -A 20 -B 20 "resolve_dispatch_provider\(ctx, config, request\.model\)" src/gateway/api/routes/chat.py echo echo "== chat.py around chat_completions function ==" rg -n -A 40 -B 20 "async def chat_completions" src/gateway/api/routes/chat.py echo echo "== conftest TestClient fixture ==" rg -n -A 20 -B 20 "TestClient\(" tests/integration/conftest.pyRepository: mozilla-ai/otari
Length of output: 7635
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== provider_kwargs invalid format context ==" rg -n -A 20 -B 20 "Invalid model format|provider:model|resolve_effective_alias|split_model_provider" src/gateway/services/provider_kwargs.py echo echo "== chat.py invalid model / ValueError handling context ==" rg -n -A 20 -B 20 "Invalid model format|except ValueError|ValueError as e|resolve_provider_selector" src/gateway/api/routes/chat.pyRepository: mozilla-ai/otari
Length of output: 6286
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path path = Path("tests/integration/test_batches_endpoint.py") lines = path.read_text().splitlines() for start, end in [(150, 175), (220, 245)]: print(f"== {path} {start}-{end} ==") for i in range(start - 1, min(end, len(lines))): print(f"{i+1:>4}: {lines[i]}") print() PYRepository: mozilla-ai/otari
Length of output: 2086
Return a 400 for this invalid model selector
Catch the selector-resolutionValueErrorin/v1/chat/completionsand raise a 400HTTPExceptioninstead; then assert the response status anddetailhere rather than expecting the client call to raise.🤖 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/integration/test_alias_api.py` around lines 231 - 236, Update the /v1/chat/completions handler to catch the selector-resolution ValueError and raise an HTTPException with status 400 and the existing “Invalid model format” detail. In the test around client.post, replace pytest.raises(ValueError) with assertions on the returned response status code and detail.Source: Coding guidelines
web/src/api/hooks.ts (1)
151-155: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect the pricing endpoint's pagination contract and its frontend consumers. ast-grep outline src web/src --items all --match 'pricing|Pricing' rg -n -C3 '/v1/pricing|limit|offset|PricingResponse' src web/src testsRepository: mozilla-ai/otari
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the pricing hook and the backend pricing API implementation. git ls-files 'web/src/api/hooks.ts' 'src/**' 'web/src/**' | sed -n '1,200p' echo '--- web/src/api/hooks.ts ---' sed -n '130,180p' web/src/api/hooks.ts echo '--- backend pricing references ---' rg -n -C 3 '/v1/pricing|PricingResponse|limit=1000|offset=|page=|pagination' src web/src testsRepository: mozilla-ai/otari
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find the exact backend handler and any pricing-related schema/types. rg -n -C 4 'v1/pricing|PricingResponse|pricing' src web/src tests --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/static/**' | head -n 300Repository: mozilla-ai/otari
Length of output: 20882
🏁 Script executed:
#!/bin/bash set -euo pipefail # Narrow search to the hook's consumer path and the server pricing route. rg -n -C 4 'usePricing|queryKey: \[PRICING\]|apiFetch<PricingResponse\[]>|/v1/pricing' web/src srcRepository: mozilla-ai/otari
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the relevant files and show the exact hook implementation. git ls-files 'web/src/api/hooks.ts' 'web/src/**/*pricing*' 'src/**/*pricing*' | sort echo '---' sed -n '145,165p' web/src/api/hooks.tsRepository: mozilla-ai/otari
Length of output: 1056
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the pricing route and response schema in small slices. echo '--- src/gateway/api/routes/pricing.py ---' sed -n '1,260p' src/gateway/api/routes/pricing.py echo '--- web/src/api/types.ts (pricing-related slice) ---' rg -n -C 4 'PricingResponse|SetPricingRequest|pricing' web/src/api/types.ts web/src/api/*.ts web/src/api/*.tsxRepository: mozilla-ai/otari
Length of output: 19004
Pricing list can be truncated at 1000 rows.
usePricing()only requests the first page (/v1/pricing?limit=1000), while the API also supportsskip, so any additional prices never show up in the dashboard. Add paging or a clear truncated/total indicator so operators can reach the full set.🤖 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/hooks.ts` around lines 151 - 155, Update usePricing to retrieve all pricing pages using the API’s skip and limit parameters, combining results until no additional prices remain. Preserve the existing query behavior for the dashboard while ensuring pricing records beyond the first 1000 are accessible.web/src/components/AppShell.tsx (1)
21-30: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Treat
localStorageas fallible.These reads and writes can throw when storage is disabled or restricted, crashing the dashboard over a cosmetic preference. Catch storage failures and fall back to defaults, matching the defensive behavior already used by authentication storage.
Also applies to: 81-90
🤖 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/components/AppShell.tsx` around lines 21 - 30, Make the sidebar preference storage access in readStoredSidebarWidth and readStoredCollapsed defensive by wrapping localStorage reads in try/catch and returning DEFAULT_SIDEBAR or false on failure. Apply the same protection to the corresponding sidebar preference writes around lines 81-90, swallowing storage errors so disabled or restricted localStorage cannot crash the dashboard.web/src/pages/ModelsPage.tsx (1)
925-936: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash ast-grep outline web/src/components/Table.tsx --items all --match Tr --view expanded rg -n -C3 'function Tr|export.*Tr|tabIndex|onKeyDown' \ web/src/components/Table.tsx \ web/src/pages/ModelsPage.test.tsxRepository: mozilla-ai/otari
Length of output: 579
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,140p' web/src/components/Table.tsx printf '\n--- ModelsPage selection area ---\n' sed -n '880,980p' web/src/pages/ModelsPage.tsx printf '\n--- ModelsPage tests mentioning selection or keyboard ---\n' rg -n -C3 'selectedKey|onSelect|keyboard|Enter|Space|tabIndex|onKeyDown|Tr\(' web/src/pages/ModelsPage.test.tsxRepository: mozilla-ai/otari
Length of output: 6478
🏁 Script executed:
#!/bin/bash set -euo pipefail nl -ba web/src/components/Table.tsx | sed -n '1,120p'Repository: mozilla-ai/otari
Length of output: 194
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -C4 'selectedKey|onSelect|detail|alias|price|model row|row selection|selected row|selected model' web/src/pages/ModelsPage.tsxRepository: mozilla-ai/otari
Length of output: 21211
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,260p' web/src/pages/ModelsPage.tsxRepository: mozilla-ai/otari
Length of output: 8980
Make model-row selection keyboard-accessible.
This row click is the only way to select a model and open the edit/alias actions, so keyboard users get stuck here. Add a focusable control with Enter/Space handling (or a button in the model cell); a regression test would be a good guardrail.
🤖 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/ModelsPage.tsx` around lines 925 - 936, Make each model row rendered by the rows.map selection flow keyboard-accessible by adding a focusable control, preferably within the model cell, that invokes onSelect(row.key) on Enter and Space while preserving click selection and the existing row display. Ensure the control has appropriate button semantics and does not trigger selection twice through event bubbling.
Follow-up review cleanups for the admin dashboard PR: - pricing: page through GET /v1/pricing (skip+limit) in the dashboard instead of a single limit=1000 request, so a gateway with a long price history no longer silently drops older rows from the models table. - pricing_service: memoize the genai-prices resolution per (provider, model, day). A single GET /v1/models resolved each model twice (context window in one phase, default price in another); the data is date-granular, so one lookup per model per day is equivalent and avoids re-walking the dataset. Bounded cache with a reset hook, cleared between tests so a cached price cannot mask a fail-safe test. - model_catalog_service: stream the models.dev catalog with a byte cap so a misbehaving upstream cannot force an unbounded response into memory; overflow degrades to "no enrichment" like any other fetch failure. - hybrid_mode: stub the new management endpoints (settings, aliases, providers, pricing) so hitting them in hybrid mode returns the same "manage via the platform UI" 404 as the other standalone-only resources, instead of a bare Not Found. Covered by a new test. - comment: fix a stale relative-path example in the api client. Local checks: gateway unit suite + hybrid-surface test pass, ruff and mypy --strict clean; dashboard typecheck, vitest (46), and a byte-identical production rebuild of the committed bundle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GitHub Actions
Inactive
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 `@src/gateway/api/routes/hybrid_mode.py`:
- Around line 40-43: Update settings_disabled, aliases_disabled,
providers_disabled, and pricing_disabled in
src/gateway/api/routes/hybrid_mode.py at lines 40-43, 46-49, 52-55, and 58-61 to
accept path: str = "" in their signatures, satisfying the corresponding
{path:path} routes while preserving the existing disabled-handler behavior.
🪄 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
Run ID: 5cf02cee-571f-4294-996d-a46a22e1f9aa
📒 Files selected for processing (9)
src/gateway/api/routes/hybrid_mode.pysrc/gateway/services/model_catalog_service.pysrc/gateway/services/pricing_service.pysrc/gateway/static/dashboard/assets/index-D_mEm_7h.jssrc/gateway/static/dashboard/index.htmltests/conftest.pytests/integration/test_hybrid_mode_surface.pyweb/src/api/client.tsweb/src/api/hooks.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/gateway/services/model_catalog_service.py
- web/src/api/hooks.ts
| @router.api_route("/v1/settings/{path:path}", methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]) | ||
| @router.api_route("/v1/settings", methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]) | ||
| async def settings_disabled() -> None: | ||
| _raise_disabled() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Declare the path parameter in the handler signatures.
Hey there! FastAPI is pretty strict about routing: it requires all path parameters defined in a route template (like {path:path}) to be explicitly declared in the endpoint function's signature. Without it, FastAPI will raise a FastAPIError at startup and fail to mount the routes.
Adding path: str = "" to these functions cleanly resolves the issue. It satisfies FastAPI's validation and handles both the exact match (using the default) and the subpath match perfectly.
src/gateway/api/routes/hybrid_mode.py#L40-L43: Addpath: str = ""to thesettings_disabledsignature.src/gateway/api/routes/hybrid_mode.py#L46-L49: Addpath: str = ""to thealiases_disabledsignature.src/gateway/api/routes/hybrid_mode.py#L52-L55: Addpath: str = ""to theproviders_disabledsignature.src/gateway/api/routes/hybrid_mode.py#L58-L61: Addpath: str = ""to thepricing_disabledsignature.
🛠️ Proposed fix pattern
-async def settings_disabled() -> None:
+async def settings_disabled(path: str = "") -> None:
_raise_disabled()(Apply the same signature update to aliases, providers, and pricing)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @router.api_route("/v1/settings/{path:path}", methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]) | |
| @router.api_route("/v1/settings", methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]) | |
| async def settings_disabled() -> None: | |
| _raise_disabled() | |
| `@router.api_route`("/v1/settings/{path:path}", methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]) | |
| `@router.api_route`("/v1/settings", methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]) | |
| async def settings_disabled(path: str = "") -> None: | |
| _raise_disabled() |
📍 Affects 1 file
src/gateway/api/routes/hybrid_mode.py#L40-L43(this comment)src/gateway/api/routes/hybrid_mode.py#L46-L49src/gateway/api/routes/hybrid_mode.py#L52-L55src/gateway/api/routes/hybrid_mode.py#L58-L61
🤖 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 `@src/gateway/api/routes/hybrid_mode.py` around lines 40 - 43, Update
settings_disabled, aliases_disabled, providers_disabled, and pricing_disabled in
src/gateway/api/routes/hybrid_mode.py at lines 40-43, 46-49, 52-55, and 58-61 to
accept path: str = "" in their signatures, satisfying the corresponding
{path:path} routes while preserving the existing disabled-handler behavior.
khaledosman
left a comment
khaledosman
left a comment
There was a problem hiding this comment.
Review by Claude Code (requested by @khaledosman).
Approving: CI is fully green, there are no security or correctness blockers, and everything from the previous round was verified fixed (openapi spec regenerated; git status --porcelain now catches untracked bundle files; the pricing hook pages past the 1000-row cap; /assets gets a real Cache-Control: immutable; the docs/README/AGENTS/Login copy matches the slimmed Models + Settings scope). Master-key handling in the browser is solid: sessionStorage only, key only ever in the Authorization header, a 401 anywhere drops it and clears the query cache, and there is no XSS surface. The alias work is the standout on both sides: the validation matrix (shadowing, chaining, provider collisions), routing tested through the real dispatch path, and the pricing-an-alias rejection with proof nothing was written.
The inline comments below are non-blocking, ranked roughly by severity. The ones I would prioritize in a follow-up: the pricing memoization caching transient failures for a day (billing-adjacent), the three integration files that dropped the drop_all teardown, the missing user-key authorization tests on the new management endpoints, and the two frontend bugs (stale-prop Add form, unguarded localStorage).
Not inline, for the record:
- models.dev coverage: every test patches
_fetch, so the real fetch body (timeout,raise_for_status, the 64 MB streaming cap, non-dict JSON guard) and the whole cache policy in_read_cache(success TTL,ttl=0, the 60s negative TTL, the single-flight lock) never execute in the suite. The cache half is trivially unit-testable in the same style astest_alias_service.py. - Hybrid mode: nothing pins that
/v1/models/metadataand/v1/models/discoverableare unreachable in hybrid, only GET (not PATCH) is exercised on the new stubs, and no test asserts hybrid/serves the tutorial rather than the dashboard (the existing fallback test covers the missing-bundle condition, which is a different branch). - Smaller test notes:
test_deleted_alias_stops_routingpins a transport-levelValueErrorwhere production callers get an unhandled 500; three files use loosein (401, 403)where sister tests assert exact 401; the alias routes'SQLAlchemyErrorbranches andrun_alias_refresherhave no coverage;PATCH /v1/settingshas no invalid-value (422) or empty-body test. - Smaller frontend notes:
metadataByKeymints a new{}every render until metadata resolves (invalidates themodelRows/filteredModelsmemos), andreleaseCutoffusesDate.now()inside the memo deps;ModelRow.aliasSourceis computed but never read;InfoTooltiphard-codesid="pricing-info-tip", a duplicate-id bug the moment it is instantiated twice;providerFromModelKeyreturns a literal "—" that becomes a provider option in the dropdown once someone prices a colon-free key.
| result = _resolve_genai_price_uncached(provider, model, as_of) | ||
| if len(_price_cache) >= _PRICE_CACHE_MAX: | ||
| _price_cache.clear() | ||
| _price_cache[key] = result |
There was a problem hiding this comment.
This memoizes the generic-exception path too: _resolve_genai_price_uncached returns None from its except Exception branch, and that None is now cached under (provider, model, day). On main, a genai-prices hiccup degraded a single request to "unpriced" and the next request retried; now it sticks until the date changes or the cap clears the cache. On the billing path with default_pricing=true and require_pricing=false, that is up to a day of unmetered $0 usage for the affected model (security instructions §0.4); with require_pricing=true it is a day of 402s. Low probability since the dataset is bundled and local, but the fix is cheap: cache successes and genuine LookupError misses, and skip caching when the uncached call hit the exception branch (e.g. have it return a distinct sentinel).
| stored = await refresh_alias_cache(db) | ||
| _validate(config, request.name, request.target, stored) | ||
|
|
||
| alias = (await db.execute(select(ModelAlias).where(ModelAlias.name == request.name))).scalar_one_or_none() |
There was a problem hiding this comment.
Nit: two concurrent POSTs creating the same new name both pass this scalar_one_or_none check; the loser hits the primary-key constraint and surfaces as a generic 500 "Database error" instead of a clean retry or 409. stage_override in the settings route has the same shape for a new key. Single-operator surface, so fine to leave, just noting it is a known 500.
| try: | ||
| with TestClient(app) as client: | ||
| yield client | ||
| finally: |
There was a problem hiding this comment.
This _make_client (and its copies in test_providers_endpoint.py and test_model_metadata_endpoint.py) kept the engine-dispose half of the pattern from test_model_discovery.py but dropped the Base.metadata.drop_all(bind=engine) teardown. Every app startup commits a bootstrap User + APIKey row (bootstrap_api_key defaults to true), and test_models_carry_context_window commits a ModelPricing row for openai:gpt-4o that nothing deletes, so state leaks into the shared session-scoped Postgres and becomes an ordering hazard under -n auto. Restoring the drop_all in the three finally blocks matches the file this was copied from. Extracting the now four-fold duplicated helper into tests/integration/conftest.py would also prevent the next copy from drifting.
|
|
||
|
|
||
| def test_alias_routes_require_master_key(client: TestClient) -> None: | ||
| assert client.get(ALIASES).status_code == 401 |
There was a problem hiding this comment.
These auth tests only cover the no-key case. None of the new management endpoints except /v1/models/discoverable is tested against a valid non-master user key, so swapping verify_master_key for verify_api_key_or_master_key on POST /v1/aliases, PATCH /v1/settings, GET /v1/providers, or GET /v1/models/metadata (a one-token diff that would hand any tenant key control of routing and billing toggles) would not fail a single test. test_discoverable_requires_master_key already shows the pattern: mint a key via /v1/keys, assert 401 here while it still works on /v1/models. One parametrized test across the five new handlers would close this; DELETE /v1/aliases/{name} currently has no auth test at all.
| reset_db() | ||
|
|
||
|
|
||
| def test_hybrid_mode_disables_dashboard_management_endpoints(monkeypatch: pytest.MonkeyPatch) -> None: |
There was a problem hiding this comment.
Good stub coverage for the four prefixes, but only via GET. The stubs declare all methods, and the higher-risk case is a state-changing PATCH /v1/settings or POST /v1/aliases in hybrid; one loop over methods would pin that. Also nothing asserts /v1/models/metadata and /v1/models/discoverable are unreachable in hybrid (re-registering models.router there would go unnoticed), and nothing asserts hybrid / serves the tutorial rather than the dashboard; the existing fallback test covers the missing-bundle branch, which is a different condition.
| }) { | ||
| return ( | ||
| <tr | ||
| onClick={onClick} |
There was a problem hiding this comment.
The clickable <tr> has no tabIndex, no key handler, and no focus styling, and row selection is the only way to reach the detail panel, which is the only place to edit or clear a price and the only "Alias to this model" entry point. A keyboard-only or screen-reader operator cannot select a row at all (WCAG 2.1.1). aria-selected on a plain table row is also not announced, and the sortable headers never set aria-sort (sort state is conveyed only by the glyph). Minimum viable fix: tabIndex={0}, Enter/Space handler, a visible focus ring, and aria-sort on the sorted <th>.
|
|
||
| function readStoredSidebarWidth(): number { | ||
| if (typeof window === "undefined") return DEFAULT_SIDEBAR; | ||
| const raw = window.localStorage.getItem(SIDEBAR_WIDTH_KEY); |
There was a problem hiding this comment.
Unguarded localStorage access (here, lines 30/83/89, and the sort-persist effect in ModelsPage.tsx): in a browser where storage access throws (Chrome with "block all cookies", some private modes), this useState initializer throws and the whole shell white-screens right after sign-in. AuthContext.tsx deliberately wraps every storage call in try/catch; the same treatment (or a tiny shared safeStorage helper) is needed at these five sites.
| } | ||
|
|
||
| export function formatCost(value: number | null | undefined): string { | ||
| if (value == null) { |
There was a problem hiding this comment.
formatCost(null) still returns "$0.00", and format.test.ts locks that contract in. Every current call site guards null and renders a dash, so nothing user-visible is wrong today, but the helper's null branch contradicts the dash convention and the next unguarded caller reprises exactly the bug flagged last round (unpriced indistinguishable from free). Returning "—" from the helper (and from formatNumber(null), same pattern) would let the call-site ternaries disappear.
| const loadedBuild = useRef<string | null>(null); | ||
|
|
||
| if (data && loadedBuild.current === null) { | ||
| loadedBuild.current = data.build; |
There was a problem hiding this comment.
The baseline is "first successful poll", not the build the tab actually loaded: if the first /dashboard-build.json poll fails (retry: false) and a deploy lands before the next tick, the stale tab records the new build id as its own and never prompts. Baking the build id into the bundle at build time (e.g. import.meta.env define) would make the comparison exact. Separately, writing loadedBuild.current during render is the ref-write-in-render anti-pattern; a useEffect on data avoids it.
|
|
||
| async function fetchAllPricing(): Promise<PricingResponse[]> { | ||
| const all: PricingResponse[] = []; | ||
| for (let skip = 0; ; skip += PRICING_PAGE_SIZE) { |
There was a problem hiding this comment.
The pagination fix from last round looks right, one robustness note: this loop terminates only when a page comes back short, so a backend or proxy that ignores/rewrites skip turns it into an unbounded request loop. A max-page cap or a seen-key check would bound it.
| <tr | ||
| onClick={onClick} | ||
| aria-selected={selected} | ||
| className={`border-b border-[var(--otari-line)] last:border-b-0 ${onClick ? "cursor-pointer" : ""} ${ | ||
| selected ? "bg-[var(--otari-brand-tint)]" : "hover:bg-[var(--otari-bg)]" | ||
| } ${className}`} | ||
| > |
| it("offers a reload once the gateway serves a different build", async () => { | ||
| const reload = vi.fn(); | ||
| // jsdom's location.reload cannot be spied on directly. | ||
| Object.defineProperty(window, "location", { | ||
| configurable: true, | ||
| value: { ...window.location, reload }, | ||
| }); | ||
| mockGateway(); | ||
|
|
||
| const client = renderWithClient(<UpdatePrompt />); | ||
| await waitForLoadedBuild(client, "build-a"); | ||
|
|
||
| servedBuild = "build-b"; | ||
| await act(() => client.refetchQueries({ queryKey: ["build"] })); | ||
|
|
||
| expect(await screen.findByText(/An update is available/)).toBeInTheDocument(); | ||
| await userEvent.click(screen.getByRole("button", { name: "Update now" })); | ||
| expect(reload).toHaveBeenCalled(); | ||
| }); |
Backend: - models: withhold discovered alias targets from GET /v1/models, not just pricing-only ones, so discovery cannot leak the provider:model an alias hides. - pricing_service: do not memoize a transient genai-prices failure; a raised lookup is retried on the next request instead of pinning the model to unpriced until the date rolls over. - settings: map SQLAlchemyError to a generic 500 rather than re-raising the DB exception, matching the convention in the other management routes. - aliases: isolate the post-commit alias cache refresh so a refresh failure cannot turn a committed create/delete into a 500. Frontend: - client: drop the session and bounce to sign-in on 403 as well as 401. - format: promote a rounded 1000K context window to 1M. - AppShell, ModelsPage: guard localStorage access so blocked storage cannot white-screen the shell. - Table: make clickable rows keyboard operable (tab focus, Enter/Space, focus ring) and set aria-sort on sorted headers. - ModelsPage: remount the Add form on mode/target change so "Alias to this model" reseeds an already-open form; surface discoverable/providers/metadata query errors in the banner. - hooks: cap the pricing pagination walk so a backend that ignores skip cannot spin an unbounded loop. Tests: - Add non-master-key rejection coverage for the management endpoints, hybrid state-changing and model-management surface assertions, engine.dispose and drop_all teardown in the affected fixtures, and regressions for the discovery alias-target hiding and transient-failure retry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GitHub Actions
Inactive
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/integration/test_hybrid_mode_surface.py (2)
88-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the JSON payload for the DELETE request.
Hey there! Great to see this thorough coverage of the HTTP verbs. Just a small detail for consistency: it would be good to also assert the JSON payload for the
DELETErequest, just like we do forPOSTandPATCH. This makes absolutely sure the response is coming from our specific hybrid mode gate rather than a generic "route not found" 404.💡 Proposed fix
- assert client.delete("/v1/aliases/x").status_code == 404 + delete_alias = client.delete("/v1/aliases/x") + assert delete_alias.status_code == 404 + assert delete_alias.json() == expected🤖 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/integration/test_hybrid_mode_surface.py` at line 88, Update the DELETE assertion in the hybrid mode surface test to validate both the 404 status and the response JSON payload, matching the expected hybrid mode gate payload used by the POST and PATCH assertions. Keep the existing endpoint and status-code check unchanged.
90-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnsure reliable test teardown.
One quick thought on our cleanup strategy: manual cleanup at the end of a test function can be risky because if any assertion fails, the test aborts and the cleanup is entirely skipped. This can leave the global configuration or database in a dirty state, leading to confusing cascading failures in other tests. Consider moving
reset_config()andreset_db()into a shared Pytest fixture with ayieldstatement (or wrapping the test bodies in atry...finallyblock) so the cleanup runs no matter what.
tests/integration/test_hybrid_mode_surface.py#L90-L91: Ensure teardown runs even if the assertions above fail.tests/integration/test_hybrid_mode_surface.py#L110-L111: Ensure teardown runs even if the assertions above fail.tests/integration/test_hybrid_mode_surface.py#L132-L133: Ensure teardown runs even if the assertions above fail.🤖 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/integration/test_hybrid_mode_surface.py` around lines 90 - 91, Move the reset_config() and reset_db() calls out of the individual test bodies in tests/integration/test_hybrid_mode_surface.py and into a shared Pytest yield fixture so teardown executes even when assertions fail. Apply this to the cleanup sites at lines 90-91, 110-111, and 132-133; each site requires no direct change once the fixture handles cleanup.
🤖 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.
Nitpick comments:
In `@tests/integration/test_hybrid_mode_surface.py`:
- Line 88: Update the DELETE assertion in the hybrid mode surface test to
validate both the 404 status and the response JSON payload, matching the
expected hybrid mode gate payload used by the POST and PATCH assertions. Keep
the existing endpoint and status-code check unchanged.
- Around line 90-91: Move the reset_config() and reset_db() calls out of the
individual test bodies in tests/integration/test_hybrid_mode_surface.py and into
a shared Pytest yield fixture so teardown executes even when assertions fail.
Apply this to the cleanup sites at lines 90-91, 110-111, and 132-133; each site
requires no direct change once the fixture handles cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa4d14f8-fdfd-4c29-beca-d13fd9d08cfa
📒 Files selected for processing (21)
src/gateway/api/routes/aliases.pysrc/gateway/api/routes/models.pysrc/gateway/api/routes/settings.pysrc/gateway/services/pricing_service.pysrc/gateway/static/dashboard/assets/index-BmfhJJsH.csssrc/gateway/static/dashboard/assets/index-C8ycIJF3.jssrc/gateway/static/dashboard/index.htmltests/integration/test_alias_api.pytests/integration/test_hybrid_mode_surface.pytests/integration/test_model_aliases.pytests/integration/test_model_metadata_endpoint.pytests/integration/test_providers_endpoint.pytests/unit/test_default_model_pricing.pyweb/src/api/client.tsweb/src/api/hooks.tsweb/src/components/AppShell.tsxweb/src/components/Table.tsxweb/src/components/UpdatePrompt.test.tsxweb/src/lib/format.tsweb/src/pages/ModelsPage.tsxweb/src/pages/SettingsPage.test.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- src/gateway/static/dashboard/index.html
- web/src/pages/SettingsPage.test.tsx
- web/src/components/Table.tsx
- tests/integration/test_model_metadata_endpoint.py
- web/src/components/AppShell.tsx
- web/src/api/client.ts
- web/src/api/hooks.ts
- web/src/components/UpdatePrompt.test.tsx
- src/gateway/api/routes/aliases.py
- src/gateway/api/routes/settings.py
- web/src/lib/format.ts
- tests/integration/test_model_aliases.py
- tests/integration/test_providers_endpoint.py
- tests/integration/test_alias_api.py
- src/gateway/api/routes/models.py
- web/src/pages/ModelsPage.tsx
- src/gateway/services/pricing_service.py
Description
Note: this PR description was drafted by Claude via back-and-forth with @njbrake. The reasoning and decisions are his; the prose is Claude's.
Adds a standalone admin web dashboard for Otari, plus the gateway endpoints and services that back it. Until now the only web page was a static get-started splash, so an operator who wanted to see which models the gateway exposes, price them, or manage aliases had to reach for the API directly.
The dashboard (frontend)
A React 19 + HeroUI v3 single-page app under
web/(Vite, Tailwind v4, TanStack Query). It drives the standalone management API using the master key, which the operator enters on a sign-in screen and which is held only in the browser tab's session storage (a 401 anywhere drops it and returns to sign-in).This first pass is deliberately small: two pages behind a collapsible, resizable sidebar.
Gateway changes (backend)
This PR is not only the frontend. To back the dashboard, it adds a set of management endpoints and services to the gateway. They are standalone-only and master-key gated, and none of them change the request/completion hot path.
api/routes/aliases.py,services/alias_service.py): create, list, and delete named aliases (for examplefast-modeltoopenai:gpt-4o-mini) that resolve to a realprovider:modelat dispatch time. NewModelAliasentity and themodel_aliasestable (migrationf1a3b5c7d9e2).GET /v1/models/discoverable,services/model_discovery_service.py): reports the models each configured provider's credentials can actually reach, so the UI can offer a real picker instead of free text.GET /v1/models/metadata,services/model_catalog_service.py): fetches and caches the public models.dev catalog to annotate each model with context window, release date, input/output modalities, and capability flags.GET /v1/modelsnow also carriescontext_window. Two new config flags gate and tune it:models_dev_metadata(on/off) andmodels_dev_cache_ttl_seconds.GET /v1/providers,services/provider_metadata_service.py): per-provider name, docs URL, env key, and capability flags, combined from any-llm's provider metadata and the genai-prices catalog.GET/PATCH /v1/settings,services/runtime_settings_service.py): read and toggle model discovery and default pricing without a redeploy. Overrides are persisted in the DB and re-applied on startup. NewRuntimeSettingentity and theruntime_settingstable (migrationa2c4e6f8b0d1).services/pricing_service.py,api/routes/pricing.py): a shared genai-prices resolver, acontext_windowhelper, per-model default-pricing status on the model listing, and a price-history endpoint.Supporting plumbing:
core/config.pygains the models.dev flags and related wiring;core/database.pygains session helpers used by the new services; andmodels/entities.pygains the two entities above with their Alembic migrations.Serving and packaging
Serving (
src/gateway/main.py,src/gateway/dashboard.py): in standalone mode the gateway servesindex.htmlat/and the hashed assets under/assets; the get-started tutorial moves to/welcome. Hybrid mode has no local management API, so/keeps serving the tutorial, and the root also falls back to the tutorial when the bundle has not been built. Navigation is client-side, so no server catch-all route is needed.The built bundle is committed under
src/gateway/static/dashboardand added topackage-data, so the wheel and Docker image ship the dashboard with no Node build stage. The Dockerfile is unchanged. A CI workflow (.github/workflows/otari-dashboard.yml) type-checks, tests, and builds the frontend onweb/**changes, and fails if the committed bundle is stale.PR Type
Relevant issues
Fixes #225
Checklist
tests/unit,tests/integration, and the frontend Vitest suite).make lint,make typecheck,make test, plus the dashboard typecheck, tests, and build).make openapi-check,make postman-check).AI Usage
AI Model/Tool used: Claude Code (Opus 4.8)
Any additional AI details you'd like to share:
Implemented by Claude Code via back-and-forth with @njbrake, who made the scoping and design decisions (packaging approach, page scope, mount point, and the models-page and settings design). Frontend checks pass locally: the TypeScript typecheck, the Vitest suite, and the production build with the committed bundle rebuilt to match. The branch is merged up to date with
main, with the OpenAPI spec and Postman collection regenerated; the new endpoints' integration tests pass locally, and CI runs the full Python unit and integration suites.Summary
models.dev, context-window data, provider metadata, and resilient pricing resolution./; hybrid mode retains the tutorial at/welcomeand disables dashboard management APIs.