fix(admin): use 4h default on model detail - #1854
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (3)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughBackend adds a new admin endpoint returning per-provider history and changes default history window from "24h" to "4h"; frontend reads/validates a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AdminAPI
participant DB as Storage
Client->>AdminAPI: GET /admin/models/{modelId}/providers/history?window=4h
AdminAPI->>DB: Query modelProviderMappingHistory (minute-level) for modelId + window
AdminAPI->>DB: Query projectHourlyModelStats (hour-level) for modelId + window
AdminAPI->>AdminAPI: Aggregate per-provider minute mappings + hourly cost, compute mapHistoryRows
AdminAPI-->>Client: 200 JSON { providerId: [historyPoints], ... }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Pull request overview
Aligns the admin model detail “history window” default across the API and admin UI to reduce unnecessary aggregation work and eliminate an avoidable initial client refetch.
Changes:
- API model detail endpoint now defaults
windowto4hwhen absent. - Admin model detail SSR now parses
?window=(defaulting to4h) and includes it in the server-side API request. - Client model detail now defaults to
4hand attempts to skip the initial refetch when SSR already rendered the active window.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ee/admin/src/components/model-detail-client.tsx | Defaults to 4h and adds “skip first refetch if SSR already has it” behavior. |
| ee/admin/src/app/models/[modelId]/page.tsx | Parses window during SSR, defaults to 4h, passes initialWindow, and includes window in the API query. |
| apps/api/src/routes/admin.ts | Fixes model detail route handler fallback default from 24h to 4h. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| useEffect(() => { | ||
| if (!initialLoadSkippedRef.current && window === initialWindow) { | ||
| initialLoadSkippedRef.current = true; | ||
| return; | ||
| } | ||
| void loadStats(window); | ||
| }, [loadStats, window]); | ||
| }, [initialWindow, loadStats, window]); |
| import type { HistoryWindow } from "@/components/history-chart"; | ||
|
|
||
| const validHistoryWindows = new Set<HistoryWindow>([ | ||
| "1m", | ||
| "2m", | ||
| "5m", | ||
| "15m", | ||
| "1h", | ||
| "2h", | ||
| "4h", | ||
| "12h", | ||
| "24h", | ||
| "2d", | ||
| "7d", | ||
| ]); | ||
|
|
||
| function parseHistoryWindow(value?: string): HistoryWindow { | ||
| if (value && validHistoryWindows.has(value as HistoryWindow)) { | ||
| return value as HistoryWindow; | ||
| } | ||
| return "4h"; | ||
| } |
Summary
4hinstead of24h?window=during SSR and default to4hwhen absentRoot cause
The model detail page had inconsistent defaults across the stack:
/admin/models/{modelId}OpenAPI schema said the default window was4h24hwindow, so it inherited that wider24hquery24h, then immediately re-fetched the same wider window on mountThat meant production was doing a larger-than-expected aggregation scan and duplicate initial requests before the page even settled.
Verification
pnpm --filter api buildpnpm --filter admin buildSummary by CodeRabbit
New Features
Changes