feat(ui): add React + HeroUI admin dashboard for keys, users, and usage#227
feat(ui): add React + HeroUI admin dashboard for keys, users, and usage#227njbrake wants to merge 9 commits into
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>
njbrake
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds gateway support for serving a packaged dashboard bundle, a React/Vite frontend with auth, API, shell, shared UI, and pages for usage, keys, users, models, overview, and pricing, plus CI and docs that keep the committed dashboard output synchronized. ChangesWeb Dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 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>
njbrake
temporarily deployed
to
integration-tests
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
| - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | ||
|
|
||
| - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 | ||
| with: | ||
| node-version: 22 | ||
| cache: npm | ||
| cache-dependency-path: web/package-lock.json |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
Consider hardening the checkout step to protect the workflow token.
Great work on getting this CI pipeline set up! I noticed a small but important security detail in the checkout step. By default, actions/checkout persists the GITHUB_TOKEN into the local Git configuration. Since the following steps run your build scripts (which can process PR-controlled code), that token is unnecessarily accessible to untrusted input.
Let's tighten that up with a best practice: disable credential persistence and explicitly set read-only permissions for the job. It's a quick tweak that makes the workflow significantly more secure.
Suggested hardening
jobs:
dashboard:
runs-on: ubuntu-latest
+ permissions:
+ contents: read
defaults:
run:
working-directory: web
steps:
- - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
+ - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
+ with:
+ persist-credentials: false📝 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.
| - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | |
| - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 | |
| with: | |
| node-version: 22 | |
| cache: npm | |
| cache-dependency-path: web/package-lock.json | |
| jobs: | |
| dashboard: | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: read | |
| defaults: | |
| run: | |
| working-directory: web | |
| steps: | |
| - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | |
| with: | |
| persist-credentials: false | |
| - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 | |
| with: | |
| node-version: 22 | |
| cache: npm | |
| cache-dependency-path: web/package-lock.json |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 26-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/otari-dashboard.yml around lines 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.
| const KEYS = "keys"; | ||
| const USERS = "users"; | ||
| const USAGE = "usage"; | ||
| const HEALTH = "health"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
fgrep -r "sessionStorage" --include "*.ts" --include "*.tsx" src/web/src | head -20Repository: mozilla-ai/otari
Length of output: 199
🏁 Script executed:
rg "useAuth|AuthContext|AuthProvider" --type ts --type tsx web/srcRepository: mozilla-ai/otari
Length of output: 182
🏁 Script executed:
rg "queryClient.invalidateQueries|queryClient.removeQueries" --type ts --type tsx web/srcRepository: mozilla-ai/otari
Length of output: 182
🏁 Script executed:
fgrep -r "sessionStorage" --include "*.ts" --include "*.tsx" web/srcRepository: mozilla-ai/otari
Length of output: 477
🏁 Script executed:
rg -g '*.{ts,tsx}' "(useAuth|AuthContext|AuthProvider)" web/srcRepository: mozilla-ai/otari
Length of output: 1618
🏁 Script executed:
rg -g '*.{ts,tsx}' "queryClient.(invalidateQueries|removeQueries|clear)" web/srcRepository: mozilla-ai/otari
Length of output: 797
🏁 Script executed:
cat -n web/src/api/hooks.tsRepository: mozilla-ai/otari
Length of output: 3674
🏁 Script executed:
cat -n web/src/auth/AuthContext.tsxRepository: mozilla-ai/otari
Length of output: 3171
🏁 Script executed:
cat -n web/src/api/client.tsRepository: mozilla-ai/otari
Length of output: 3754
Scope the query cache to the signed-in admin session.
The current useKeys and useUsers hooks use static query keys (["keys"], ["users"]). Because TanStack Query caches results by key, signing out and back in with a different master key can cause the dashboard to briefly render the previous session's data before the new fetch completes.
Include the current masterKey (from useAuth()) in the query key arrays for useKeys, useUsers, and useUsage. This ensures the cache is automatically invalidated whenever the admin session changes. The health check can remain static as it is public.
Example pattern:
const { masterKey } = useAuth();
return useQuery({
queryKey: [KEYS, masterKey ?? "anon"], // or just [KEYS, masterKey] if query requires auth
// ...
});This is a low-effort fix that prevents a potentially confusing (and privacy-sensitive) data leak during session transitions.
🤖 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 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.
| export function useKeys() { | ||
| return useQuery({ | ||
| queryKey: [KEYS], | ||
| queryFn: () => apiFetch<KeyInfo[]>("/v1/keys?limit=1000"), | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The hard 1000-row cap will silently hide records.
useKeys() and useUsers() stop at the first 1000 items, and this layer does not expose pagination or a truncated-state warning. Once a deployment grows past that, operators lose access to older users and keys from the dashboard.
Also applies to: 67-71
🤖 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 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.
| <TextField value={value} onChange={onChange} isRequired={isRequired} className="flex flex-col gap-1"> | ||
| <Label className="text-sm font-medium text-[var(--otari-ink)]"> | ||
| {label} | ||
| {isRequired ? <span className="text-[var(--otari-brand-dark)]"> *</span> : null} | ||
| </Label> | ||
| <Input type={type} placeholder={placeholder} autoFocus={autoFocus} /> | ||
| {description ? <span className="text-xs text-[var(--otari-muted)]">{description}</span> : null} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
cat -n web/src/components/Field.tsxRepository: mozilla-ai/otari
Length of output: 1468
🏁 Script executed:
node - <<'JS'
// Quick check of Heroui TextField documentation patterns via known API structure
// HeroUI TextField typically accepts Description as a child slot
const expectedPattern = `
<TextField>
<Label>...</Label>
<Input />
<Description>...</Description>
</TextField>
`;
console.log("HeroUI TextField typically slots: Label, Input, Description");
console.log("Current code uses raw span for description at line 33");
JSRepository: mozilla-ai/otari
Length of output: 268
🌐 Web query:
HeroUI TextField Description slot accessibility helper text
💡 Result:
HeroUI (formerly NextUI) provides a dedicated Description component designed specifically for providing accessible helper text within form fields [1][2][3]. When used within a TextField component, it is automatically associated with the input for screen reader announcements [1][4]. Accessibility Features: - Semantic Association: When using the Description component inside a TextField, HeroUI handles the necessary aria-describedby associations automatically, ensuring that screen readers announce the helper text when the input receives focus [1][4]. - Automatic State Management: HeroUI components are built on React Aria primitives, which provide robust WCAG-compliant behavior [3][5]. The TextField component automatically manages accessibility attributes based on the field's state [6][4]. - Conditional Visibility: For web implementations, the TextField component includes built-in logic that can automatically hide the description slot when the field is in an invalid state (often replacing it with an error message) [6]. Implementation Example: You can use the Description component as a child of the TextField [1]: import {TextField, Label, Input, Description} from '@heroui/react'; Email We'll never share your email If you need to manually handle accessibility for custom inputs, you can use the aria-describedby attribute to link an element's ID to your description element, though this is managed automatically when using the standard TextField compound component [6].
Citations:
- 1: https://www.heroui.com/docs/react/components/description
- 2: https://www.heroui.com/
- 3: https://github.com/heroui-inc/heroui
- 4: https://canary.heroui.com/docs/guide/forms
- 5: https://registry.npmjs.org/@heroui/react
- 6: https://www.heroui.com/docs/react/components/text-field
Use HeroUI's Description slot for accessibility.
HeroUI's TextField is designed to automatically handle aria-describedby associations when the Description component is used as a child. Relying on a raw <span> leaves the helper text visually present but effectively invisible to screen readers.
Suggested fix
-import { Input, Label, TextField } from "`@heroui/react`";
+import { Description, Input, Label, TextField } from "`@heroui/react`";
@@
- {description ? <span className="text-xs text-[var(--otari-muted)]">{description}</span> : null}
+ {description ? <Description className="text-xs text-[var(--otari-muted)]">{description}</Description> : null}📝 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.
| <TextField value={value} onChange={onChange} isRequired={isRequired} className="flex flex-col gap-1"> | |
| <Label className="text-sm font-medium text-[var(--otari-ink)]"> | |
| {label} | |
| {isRequired ? <span className="text-[var(--otari-brand-dark)]"> *</span> : null} | |
| </Label> | |
| <Input type={type} placeholder={placeholder} autoFocus={autoFocus} /> | |
| {description ? <span className="text-xs text-[var(--otari-muted)]">{description}</span> : null} | |
| <TextField value={value} onChange={onChange} isRequired={isRequired} className="flex flex-col gap-1"> | |
| <Label className="text-sm font-medium text-[var(--otari-ink)]"> | |
| {label} | |
| {isRequired ? <span className="text-[var(--otari-brand-dark)]"> *</span> : null} | |
| </Label> | |
| <Input type={type} placeholder={placeholder} autoFocus={autoFocus} /> | |
| {description ? <Description className="text-xs text-[var(--otari-muted)]">{description}</Description> : null} |
🤖 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/Field.tsx` around lines 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.
| // Buckets entries by calendar day (local time), returning chronological points. | ||
| export function usageByDay(entries: UsageEntry[]): DailyPoint[] { | ||
| const buckets = new Map<string, DailyPoint>(); | ||
|
|
||
| for (const entry of entries) { | ||
| const date = new Date(entry.timestamp); | ||
| if (Number.isNaN(date.getTime())) { | ||
| continue; | ||
| } | ||
| const key = date.toISOString().slice(0, 10); | ||
| const point = buckets.get(key) ?? { date: key, requests: 0, cost: 0 }; | ||
| point.requests += 1; | ||
| point.cost += entry.cost ?? 0; | ||
| buckets.set(key, point); | ||
| } | ||
|
|
||
| return [...buckets.values()].sort((a, b) => a.date.localeCompare(b.date)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Bucket by the actual local day, not UTC.
Line 53 uses toISOString(), which normalizes the timestamp to UTC before slicing the date. That shifts requests across day boundaries for operators outside UTC, so the “Requests over time” chart can show traffic on the wrong day even though this helper says it buckets by local time. Please build the key from the local date parts instead, and add a boundary test for timestamps around midnight so this does not quietly drift later.
Suggested fix
export function usageByDay(entries: UsageEntry[]): DailyPoint[] {
const buckets = new Map<string, DailyPoint>();
for (const entry of entries) {
const date = new Date(entry.timestamp);
if (Number.isNaN(date.getTime())) {
continue;
}
- const key = date.toISOString().slice(0, 10);
+ const key = [
+ date.getFullYear(),
+ String(date.getMonth() + 1).padStart(2, "0"),
+ String(date.getDate()).padStart(2, "0"),
+ ].join("-");
const point = buckets.get(key) ?? { date: key, requests: 0, cost: 0 };
point.requests += 1;
point.cost += entry.cost ?? 0;
buckets.set(key, point);
}📝 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.
| // Buckets entries by calendar day (local time), returning chronological points. | |
| export function usageByDay(entries: UsageEntry[]): DailyPoint[] { | |
| const buckets = new Map<string, DailyPoint>(); | |
| for (const entry of entries) { | |
| const date = new Date(entry.timestamp); | |
| if (Number.isNaN(date.getTime())) { | |
| continue; | |
| } | |
| const key = date.toISOString().slice(0, 10); | |
| const point = buckets.get(key) ?? { date: key, requests: 0, cost: 0 }; | |
| point.requests += 1; | |
| point.cost += entry.cost ?? 0; | |
| buckets.set(key, point); | |
| } | |
| return [...buckets.values()].sort((a, b) => a.date.localeCompare(b.date)); | |
| // Buckets entries by calendar day (local time), returning chronological points. | |
| export function usageByDay(entries: UsageEntry[]): DailyPoint[] { | |
| const buckets = new Map<string, DailyPoint>(); | |
| for (const entry of entries) { | |
| const date = new Date(entry.timestamp); | |
| if (Number.isNaN(date.getTime())) { | |
| continue; | |
| } | |
| const key = [ | |
| date.getFullYear(), | |
| String(date.getMonth() + 1).padStart(2, "0"), | |
| String(date.getDate()).padStart(2, "0"), | |
| ].join("-"); | |
| const point = buckets.get(key) ?? { date: key, requests: 0, cost: 0 }; | |
| point.requests += 1; | |
| point.cost += entry.cost ?? 0; | |
| buckets.set(key, point); | |
| } | |
| return [...buckets.values()].sort((a, b) => a.date.localeCompare(b.date)); |
🤖 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/usage.ts` around lines 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.
| const submit = () => { | ||
| createKey.mutate( | ||
| { | ||
| key_name: name.trim() || null, | ||
| user_id: userId.trim() || null, | ||
| }, | ||
| { | ||
| onSuccess: (data) => { | ||
| setCreated(data); | ||
| setName(""); | ||
| setUserId(""); | ||
| }, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear the previous one-time secret before starting another create.
created stays on screen until the next success. If a follow-up create fails, the form still shows the old secret, which makes it a little too easy to copy the wrong key.
Suggested fix
const submit = () => {
+ setCreated(null);
createKey.mutate(
{
key_name: name.trim() || null,
user_id: userId.trim() || null,
},📝 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.
| const submit = () => { | |
| createKey.mutate( | |
| { | |
| key_name: name.trim() || null, | |
| user_id: userId.trim() || null, | |
| }, | |
| { | |
| onSuccess: (data) => { | |
| setCreated(data); | |
| setName(""); | |
| setUserId(""); | |
| }, | |
| }, | |
| ); | |
| const submit = () => { | |
| setCreated(null); | |
| createKey.mutate( | |
| { | |
| key_name: name.trim() || null, | |
| user_id: userId.trim() || null, | |
| }, | |
| { | |
| onSuccess: (data) => { | |
| setCreated(data); | |
| setName(""); | |
| setUserId(""); | |
| }, | |
| }, | |
| ); |
🤖 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/KeysPage.tsx` around lines 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.
| const submit = () => { | ||
| if (!userId.trim()) { | ||
| return; | ||
| } | ||
| createUser.mutate( | ||
| { user_id: userId.trim(), alias: alias.trim() || null }, | ||
| { | ||
| onSuccess: () => { | ||
| setUserId(""); | ||
| setAlias(""); | ||
| onClose(); | ||
| }, | ||
| }, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Wrap the create flow in a real form.
Right now pressing Enter in either field does nothing because this UI never emits a submit event. Using <form onSubmit> keeps keyboard submission and native form semantics working.
Suggested fix
- const submit = () => {
+ const submit = (event?: React.FormEvent) => {
+ event?.preventDefault();
if (!userId.trim()) {
return;
}
@@
- <Card>
- <Card.Content className="flex flex-col gap-4 p-5">
+ <Card>
+ <Card.Content as="form" onSubmit={submit} className="flex flex-col gap-4 p-5">
@@
- <Button variant="primary" isDisabled={createUser.isPending || !userId.trim()} onPress={submit}>
+ <Button type="submit" variant="primary" isDisabled={createUser.isPending || !userId.trim()}>
{createUser.isPending ? "Creating…" : "Create user"}
</Button>Also applies to: 31-49
🤖 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/UsersPage.tsx` around lines 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.
| export function Provider({ children }: { children: ReactNode }) { | ||
| const [queryClient] = useState( | ||
| () => | ||
| new QueryClient({ | ||
| defaultOptions: { | ||
| queries: { | ||
| refetchOnWindowFocus: false, | ||
| retry: (failureCount, error) => { | ||
| // Never retry auth failures; they won't fix themselves. | ||
| if (error instanceof ApiError && (error.status === 401 || error.status === 403)) { | ||
| return false; | ||
| } | ||
| return failureCount < 2; | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| return ( | ||
| <QueryClientProvider client={queryClient}> | ||
| <AuthProvider>{children}</AuthProvider> | ||
| </QueryClientProvider> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reset the query cache when the master key changes.
This QueryClient survives logout and relogin, so cached /v1/keys, /v1/users, and /v1/usage data can render from the previous session before the next key is validated. In the same tab, that briefly exposes admin data to someone who does not actually have a valid key.
Suggested fix
import { QueryClient, QueryClientProvider } from "`@tanstack/react-query`";
-import { useState } from "react";
+import { useEffect, useState } from "react";
import type { ReactNode } from "react";
import { ApiError } from "`@/api/client`";
-import { AuthProvider } from "`@/auth/AuthContext`";
+import { AuthProvider, useAuth } from "`@/auth/AuthContext`";
+
+function ResetQueryCacheOnAuthChange({
+ children,
+ queryClient,
+}: {
+ children: ReactNode;
+ queryClient: QueryClient;
+}) {
+ const { masterKey } = useAuth();
+
+ useEffect(() => {
+ queryClient.clear();
+ }, [masterKey, queryClient]);
+
+ return <>{children}</>;
+}
export function Provider({ children }: { children: ReactNode }) {
const [queryClient] = useState(
@@
return (
- <QueryClientProvider client={queryClient}>
- <AuthProvider>{children}</AuthProvider>
- </QueryClientProvider>
+ <AuthProvider>
+ <QueryClientProvider client={queryClient}>
+ <ResetQueryCacheOnAuthChange queryClient={queryClient}>{children}</ResetQueryCacheOnAuthChange>
+ </QueryClientProvider>
+ </AuthProvider>
);
}🤖 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/provider.tsx` around lines 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.
| body { | ||
| background-color: var(--otari-bg); | ||
| color: var(--otari-ink); | ||
| font-family: ui-sans-serif, system-ui, -apple-system, "Segoe UI", Roboto, sans-serif; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset the default body margin.
The browser's default body margin will leave an 8px frame around the full-height app and can introduce stray scrollbars. Adding a reset here keeps the dashboard flush to the viewport.
Suggested fix
body {
+ margin: 0;
background-color: var(--otari-bg);
color: var(--otari-ink);
font-family: ui-sans-serif, system-ui, -apple-system, "Segoe UI", Roboto, sans-serif;
}📝 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.
| body { | |
| background-color: var(--otari-bg); | |
| color: var(--otari-ink); | |
| font-family: ui-sans-serif, system-ui, -apple-system, "Segoe UI", Roboto, sans-serif; | |
| } | |
| body { | |
| margin: 0; | |
| background-color: var(--otari-bg); | |
| color: var(--otari-ink); | |
| font-family: ui-sans-serif, system-ui, -apple-system, "Segoe UI", Roboto, sans-serif; | |
| } |
🤖 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/styles/globals.css` around lines 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.
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>
njbrake
temporarily deployed
to
integration-tests
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>
njbrake
temporarily deployed
to
integration-tests
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>
njbrake
temporarily deployed
to
integration-tests
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>
njbrake
temporarily deployed
to
integration-tests
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
| @router.get("", dependencies=[Depends(verify_api_key_or_master_key)]) | ||
| async def get_settings( | ||
| config: Annotated[GatewayConfig, Depends(get_config)], | ||
| ) -> GatewaySettings: | ||
| """Return non-secret runtime settings for the admin dashboard.""" | ||
| return GatewaySettings( | ||
| mode=config.effective_mode, | ||
| version=__version__, | ||
| default_pricing=config.default_pricing, | ||
| require_pricing=config.require_pricing, | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require the master key here, not any valid API key.
verify_api_key_or_master_key also admits ordinary user keys, so this admin-facing route currently exposes gateway mode/version/pricing-policy details outside the management boundary. Please switch this dependency to master-key-only auth and add a regression test for a non-master key.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gateway/api/routes/settings.py` around lines 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.
| if (response.status === 401) { | ||
| return false; | ||
| } | ||
| if (!response.ok) { | ||
| throw new ApiError(response.status, await extractErrorMessage(response)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Treat 403 as an invalid admin key too.
This validation flow only returns false for 401. If the gateway rejects non-master API keys with 403, the sign-in screen will show a generic error instead of a clean “wrong key” result. Handle both statuses the same way here. As per coding guidelines, “Admin and management endpoints under /v1/users, /v1/budgets, /v1/keys, /v1/pricing, and /v1/usage must require the master key; user keys must receive 401/403.”
🤖 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 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.
Source: Coding guidelines
| export function usageByModel(entries: UsageEntry[]): ModelUsage[] { | ||
| const buckets = new Map<string, ModelUsage>(); | ||
|
|
||
| for (const entry of entries) { | ||
| const model = entry.model || "unknown"; | ||
| const row = | ||
| buckets.get(model) ?? | ||
| ({ | ||
| model, | ||
| provider: entry.provider || "—", | ||
| requests: 0, | ||
| promptTokens: 0, | ||
| completionTokens: 0, | ||
| totalTokens: 0, | ||
| cost: 0, | ||
| } satisfies ModelUsage); | ||
| row.requests += 1; | ||
| row.promptTokens += entry.prompt_tokens ?? 0; | ||
| row.completionTokens += entry.completion_tokens ?? 0; | ||
| row.totalTokens += entry.total_tokens ?? 0; | ||
| row.cost += entry.cost ?? 0; | ||
| if (row.provider === "—" && entry.provider) { | ||
| row.provider = entry.provider; | ||
| } | ||
| buckets.set(model, row); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Bucket model usage by (provider, model), not just model.
Right now rows are keyed only by entry.model, so the same model name coming from two providers gets merged into one bucket and whichever provider wins first is the one shown. That makes the Overview and Model Usage tables misattribute requests, tokens, and cost whenever providers share model names.
Please split the bucket key by both fields and add a regression test for “same model, different providers.” Also note the callers currently use row.model as the React key, so they’ll need a composite key once this is fixed.
Suggested fix
export function usageByModel(entries: UsageEntry[]): ModelUsage[] {
const buckets = new Map<string, ModelUsage>();
for (const entry of entries) {
const model = entry.model || "unknown";
+ const provider = entry.provider || "—";
+ const key = `${provider}::${model}`;
const row =
- buckets.get(model) ??
+ buckets.get(key) ??
({
model,
- provider: entry.provider || "—",
+ provider,
requests: 0,
promptTokens: 0,
completionTokens: 0,
totalTokens: 0,
cost: 0,
} satisfies ModelUsage);
row.requests += 1;
row.promptTokens += entry.prompt_tokens ?? 0;
row.completionTokens += entry.completion_tokens ?? 0;
row.totalTokens += entry.total_tokens ?? 0;
row.cost += entry.cost ?? 0;
- if (row.provider === "—" && entry.provider) {
- row.provider = entry.provider;
- }
- buckets.set(model, row);
+ buckets.set(key, row);
}📝 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.
| export function usageByModel(entries: UsageEntry[]): ModelUsage[] { | |
| const buckets = new Map<string, ModelUsage>(); | |
| for (const entry of entries) { | |
| const model = entry.model || "unknown"; | |
| const row = | |
| buckets.get(model) ?? | |
| ({ | |
| model, | |
| provider: entry.provider || "—", | |
| requests: 0, | |
| promptTokens: 0, | |
| completionTokens: 0, | |
| totalTokens: 0, | |
| cost: 0, | |
| } satisfies ModelUsage); | |
| row.requests += 1; | |
| row.promptTokens += entry.prompt_tokens ?? 0; | |
| row.completionTokens += entry.completion_tokens ?? 0; | |
| row.totalTokens += entry.total_tokens ?? 0; | |
| row.cost += entry.cost ?? 0; | |
| if (row.provider === "—" && entry.provider) { | |
| row.provider = entry.provider; | |
| } | |
| buckets.set(model, row); | |
| export function usageByModel(entries: UsageEntry[]): ModelUsage[] { | |
| const buckets = new Map<string, ModelUsage>(); | |
| for (const entry of entries) { | |
| const model = entry.model || "unknown"; | |
| const provider = entry.provider || "—"; | |
| const key = `${provider}::${model}`; | |
| const row = | |
| buckets.get(key) ?? | |
| ({ | |
| model, | |
| provider, | |
| requests: 0, | |
| promptTokens: 0, | |
| completionTokens: 0, | |
| totalTokens: 0, | |
| cost: 0, | |
| } satisfies ModelUsage); | |
| row.requests += 1; | |
| row.promptTokens += entry.prompt_tokens ?? 0; | |
| row.completionTokens += entry.completion_tokens ?? 0; | |
| row.totalTokens += entry.total_tokens ?? 0; | |
| row.cost += entry.cost ?? 0; | |
| buckets.set(key, row); |
🤖 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/usage.ts` around lines 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.
| const providerCount = useMemo(() => { | ||
| const names = new Set(providersFromUsage(entries)); | ||
| for (const model of models.data?.data ?? []) { | ||
| if (model.owned_by) { | ||
| names.add(model.owned_by); | ||
| } | ||
| } | ||
| return names.size; | ||
| }, [entries, models.data]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the Providers card loading until both data sources are ready.
providerCount is built from both usage entries and model metadata, but the card only shows a loading placeholder while both queries are loading. If usage finishes first, this briefly renders a partial count that omits models.data[].owned_by.
Using models.isLoading || usage.isLoading here avoids the undercount flicker. Small fix, fewer “wait, why did the number change?” moments.
Suggested fix
- <StatCard label="Providers" value={dash(providerCount, models.isLoading && usage.isLoading)} />
+ <StatCard label="Providers" value={dash(providerCount, models.isLoading || usage.isLoading)} />Also applies to: 43-43
🤖 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/OverviewPage.tsx` around lines 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.
| export function PricingPage() { | ||
| const pricing = usePricing(); | ||
| const setPricing = useSetPricing(); | ||
| const deletePricing = useDeletePricing(); | ||
| const [showForm, setShowForm] = useState(false); | ||
|
|
||
| const rows = useMemo(() => currentPricing(pricing.data ?? []), [pricing.data]); | ||
|
|
||
| return ( | ||
| <div className="flex flex-col gap-6"> | ||
| <PageHeader | ||
| title="Pricing" | ||
| description="View and edit per-model token prices. Saving an edit records a new price effective now." | ||
| action={ | ||
| <Button variant="primary" onPress={() => setShowForm((value) => !value)}> | ||
| {showForm ? "Hide form" : "Set pricing"} | ||
| </Button> | ||
| } | ||
| /> | ||
|
|
||
| <DefaultPricingBanner /> | ||
|
|
||
| {showForm ? <AddPricingForm onClose={() => setShowForm(false)} /> : null} | ||
|
|
||
| <ErrorBanner error={pricing.error ?? setPricing.error ?? deletePricing.error} /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Row-level edit/delete errors are silently swallowed.
Each call to useSetPricing() / useDeletePricing() returns its own independent mutation instance with its own error state. The setPricing and deletePricing declared here in PricingPage (Lines 213-214) are never the ones that actually .mutate() — those live inside PricingRow (Lines 109-110) and AddPricingForm (Line 41). So the ErrorBanner on Line 235 will always read null for setPricing.error / deletePricing.error.
AddPricingForm is fine because it renders its own banner (Line 78), but PricingRow doesn't render any error UI. The practical effect: if an inline price edit or a delete fails (e.g. a 4xx/5xx from the gateway), the user sees nothing — the row just quietly stays put.
A couple of ways to close the gap:
- Render an
ErrorBannerinsidePricingRowfor its ownsetPricing.error/deletePricing.error, or - Lift the mutations into
PricingPageand pass handlers/state down so the page-level banner is actually wired to the live instances.
The first option keeps the diff smaller and puts the message right next to the failing row.
🛠️ Sketch: surface the error within the row
return (
<Tr>
<Td className="font-medium break-all">{row.model_key}</Td>
...
</Tr>
);Add a dedicated error row/cell (or a small inline banner under the actions) bound to setPricing.error ?? deletePricing.error inside PricingRow. If you prefer the lifted approach instead, drop the unused setPricing/deletePricing instances from PricingPage so they don't read as wired-up when they aren't.
You can confirm the duplicate-instance behavior in the hook definitions:
#!/bin/bash
# Confirm each pricing hook returns an independent useMutation result
ast-grep run --pattern 'export function useSetPricing() { $$$ }' --lang tsx web/src/api/hooks.ts
ast-grep run --pattern 'export function useDeletePricing() { $$$ }' --lang tsx web/src/api/hooks.ts
# Confirm PricingRow renders no ErrorBanner
rg -nP 'ErrorBanner|useSetPricing|useDeletePricing' web/src/pages/PricingPage.tsx🤖 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.tsx` around lines 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.
… 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>
njbrake
temporarily deployed
to
integration-tests
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>
njbrake
temporarily deployed
to
integration-tests
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>
njbrake
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
1 participant
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 real admin web dashboard for Otari. Until now the only web page was a static get-started splash, so the first thing an operator wanted after deploying (create virtual keys, watch usage and traffic) meant reaching for the Postman collection.
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 (a 401 anywhere drops it and returns to sign-in).Pages:
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 section switching, so no server catch-all route is needed.Packaging: 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 (verified the files land in both the sdist and the wheel). The Dockerfile is unchanged.CI: a new
.github/workflows/otari-dashboard.ymltype-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).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).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). All Definition of Done checks pass locally: ruff, mypy (188 files), unit suite (541 passed / 1 skipped), OpenAPI freshness, plus the frontend typecheck, Vitest, and build.
Summary
Added a real admin web dashboard for Otari with a master-key sign-in flow and operator-focused pages: Overview, Usage (traffic/spend), Models, API Keys (create with one-time secret reveal, activate/revoke, re-activate/delete), Users (create/delete), and Pricing (view/edit/delete per-model token pricing, including fallback/default pricing status).
Updated routing so standalone mode serves the dashboard at
/and moved the welcome/tutorial to/welcome(hybrid mode still shows the tutorial at/). The prebuilt dashboard frontend is bundled into the Python package so it ships with the wheel/Docker image without requiring a Node build stage.Added documentation and a dedicated CI workflow that type-checks, tests, and builds the dashboard on
web/**changes, and fails if the committed bundled frontend output becomes stale.Technical notes
/welcomeroute.