feat: improve dashboard UI and model comparison - #1664
Conversation
- Add custom date range picker with 7d/30d/custom calendar - Fix overview cost chart by splitting into separate renders - Remove CartesianGrid dashed lines from charts - Rewrite cost breakdown as shadcn donut chart with model breakdown - Format large costs as K/M/B in donut center - Fix admin All Time Credits showing $0.00 - Improve model comparison: bigger text, add description, released date, stability, web search, JSON schema, output types; remove aliases row Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughReplaces days-based dashboard filtering with explicit from/to date ranges: adds a DateRangePicker, calendar and chart infra, updates API /activity to accept from/to (with fallback), converts UI pages and charts to use from/to, and batches admin per-organization credit calculations. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant DateRangePicker
participant Router
participant DashboardPage
participant API
participant Chart
User->>DateRangePicker: choose date range (preset/custom)
DateRangePicker->>DateRangePicker: format as yyyy-MM-dd
DateRangePicker->>Router: router.push(URL with from/to)
Router->>DashboardPage: update searchParams (from/to)
DashboardPage->>DashboardPage: extract from/to and build query
DashboardPage->>API: GET /activity?from=...&to=... (&projectId,&apiKeyId)
API->>API: query DB for records within from..to
API-->>DashboardPage: return activity data
DashboardPage->>Chart: pass activity + from/to
Chart->>Chart: generate dateRange & aggregate per day
Chart->>User: render chart (area/pie/etc.)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ui/src/components/usage/model-usage-client.tsx (1)
24-101:⚠️ Potential issue | 🟡 MinorMissing
useEffectredirect for default date range params.Unlike
usage-client.tsxanddashboard-client.tsx, this component doesn't redirect to addfrom/toURL params when they're absent. This means the URL won't reflect the active date range until the user interacts with the picker, creating an inconsistency across pages. The data will still load correctly viagetDateRangeFromParamsdefaults, so this is minor.apps/ui/src/app/dashboard/[orgId]/[projectId]/usage/page.tsx (1)
8-23:⚠️ Potential issue | 🔴 Critical
fromandtoshould come fromsearchParams, notparams.In Next.js App Router,
paramscontains dynamic route segments (e.g.,orgId,projectId), while query string parameters like?from=2026-01-01&to=2026-01-07are provided viasearchParams. Sincefromandtoare query parameters (not URL path segments), they will never be populated fromparams.Compare with
apps/ui/src/app/dashboard/[orgId]/[projectId]/page.tsx(Lines 10–17), which correctly readsfrom/tofromsearchParams.Proposed fix
export default async function UsagePage({ params, + searchParams, }: { params?: Promise<{ projectId?: string; - days?: string; - from?: string; - to?: string; }>; + searchParams?: Promise<{ + from?: string; + to?: string; + }>; }) { const paramsData = await params; const projectId = paramsData?.projectId; + const searchParamsData = searchParams ? await searchParams : {}; const today = new Date(); - const fromParam = paramsData?.from || format(subDays(today, 6), "yyyy-MM-dd"); - const toParam = paramsData?.to || format(today, "yyyy-MM-dd"); + const fromParam = searchParamsData?.from || format(subDays(today, 6), "yyyy-MM-dd"); + const toParam = searchParamsData?.to || format(today, "yyyy-MM-dd");
🤖 Fix all issues with AI agents
In `@apps/api/src/routes/activity.ts`:
- Around line 79-80: The from/to query parameters currently use
z.string().optional() and accept any string; change their Zod schema to validate
date strings (e.g., use z.string().optional().refine(val => !val ||
!isNaN(Date.parse(val)), { message: "invalid date" }) or use z.preprocess to
parse then z.date()) so invalid values like "abc" are rejected; update the
schema that defines from and to in apps/api/src/routes/activity.ts (the from and
to entries shown) and ensure the route handler that consumes them still expects
the validated/parsed format.
- Around line 115-123: The code silently falls back to days-based range when
only one of from/to is provided; update the request validation in the activity
route handler to require both or neither: if (from && to) keep the current
startDate/endDate logic; else if (from || to) immediately return
res.status(400).json({ error: "Both 'from' and 'to' must be provided together"
}); otherwise (neither provided) continue using days (effectiveDays) to compute
startDate/endDate. Ensure you reference the existing variables (from, to, days,
startDate, endDate) and apply the early 400 response in the same scope where
those variables are currently set.
In `@apps/ui/src/lib/components/chart.tsx`:
- Around line 244-248: The tooltip currently uses a falsy check "item.value &&
(...)" which hides valid zero values; change the condition to explicitly allow 0
and only exclude null/undefined/invalid numbers — e.g., replace the check with
Number.isFinite(item.value) (or an equivalent null/NaN guard like item.value !=
null && !Number.isNaN(item.value)) before rendering the span so 0 renders but
NaN/undefined do not; update the condition around the span in chart.tsx where
item.value is used.
🧹 Nitpick comments (12)
apps/ui/src/components/date-range-picker.tsx (2)
24-40:getDateRangeFromParamsdefault range has a subtle inconsistency with theuseEffectredirects in consumer components.In
getDateRangeFromParams(line 35-39), when no params exist,fromis created viasubDays(today, 6)andtoisnew Date(). These Date objects include the current time component. However, when params do exist (lines 30-31), the dates are constructed withT00:00:00— always midnight.This means the first render (before the
useEffectredirect fires) will have time-of-day embedded infrom/to, while subsequent renders will have midnight. This could causeisPresetActive(which compares formattedyyyy-MM-ddstrings) to still work, but it's worth noting for any downstream code that does time-based comparisons.Normalize the default dates to midnight for consistency
const today = new Date(); + today.setHours(0, 0, 0, 0); return { from: subDays(today, 6), to: today, };
42-49:isPresetActiveinstantiatesnew Date()on every call, which may drift from thefrom/tocomputed earlier in the same render.Since
getDateRangeFromParamsandisPresetActiveeach callnew Date()independently, if the clock crosses midnight between calls, the active tab could be incorrect (e.g., showing "custom" instead of "7days"). This is an extremely rare edge case with a safe fallback ("custom" tab), so it's not critical.apps/ui/src/components/usage/usage-client.tsx (1)
74-84: DuplicateduseEffectredirect logic across multiple components.This exact
useEffectpattern (check for missingfrom/to, deletedays, set defaults,router.replace) is repeated inusage-client.tsx(here) anddashboard-client.tsx(lines 76-85). Consider extracting into a shared hook (e.g.,useEnsureDateRangeParams) to reduce duplication.apps/ui/src/components/dashboard/dashboard-client.tsx (1)
317-317:DateRangePickerplacement is outside the header row, unlike other pages.In
usage-client.tsxandmodel-usage-client.tsx, theDateRangePickeris placed inline with the header/controls in a flex row. Here it's placed as a standalone element below the header (line 317), creating a slightly different layout pattern. This may be intentional given the dashboard's more complex header, but worth confirming for visual consistency.apps/ui/src/components/usage/cost-breakdown-chart.tsx (2)
21-21: Deep import from recharts internals may break on version updates.
recharts/types/util/typesis not part of the public API surface. Consider defining a localViewBoxtype or using the type from Recharts' top-level exports if available.Local type alternative
-import type { ViewBox } from "recharts/types/util/types"; + +type ViewBox = { cx?: number; cy?: number; innerRadius?: number; outerRadius?: number };
292-325: Legend item color resolution is brittle — relies on checking"color" in config.
ChartConfigentries use a discriminated union: thecolorproperty is present only whenthemeis absent, and vice versa. The"color" in configcheck on line 306 will betrueeven ifcolorisundefined(since the key exists). This means the fallback"#94a3b8"may not trigger when intended.Since you already set
colorin the config (line 154), this works in practice for this component. However, consider simplifying:Simpler color resolution
style={{ backgroundColor: - (config && "color" in config - ? config.color - : undefined) || "#94a3b8", + config?.color ?? "#94a3b8", }}apps/ui/src/app/dashboard/[orgId]/[projectId]/model-usage/page.tsx (1)
13-18: Remove unuseddaysparameter fromsearchParamstype.The
days?: stringproperty on line 14 is a leftover from the old parameterization and is never read. Keeping it in the type may confuse future readers into thinking it's still supported.Proposed fix
searchParams?: Promise<{ - days?: string; from?: string; to?: string; apiKeyId?: string; }>;apps/ui/src/components/usage/error-rate-chart.tsx (1)
61-63: Date range derivation and dateRange construction are duplicated across chart components.The exact same pattern (lines 61–63 for
from/to/fromStr/toStrand lines 126–132 fortotalDays/dateRangeconstruction) is repeated verbatim inerror-rate-chart.tsx,cache-rate-chart.tsx, andusage-chart.tsx. Consider extracting a shared utility, e.g.,buildDateRange(from: Date, to: Date): string[], to reduce duplication.Example shared utility
// e.g., in a shared date-utils.ts import { addDays, differenceInCalendarDays, format } from "date-fns"; export function buildDateRange(from: Date, to: Date): string[] { const totalDays = differenceInCalendarDays(to, from) + 1; const range: string[] = []; for (let i = 0; i < totalDays; i++) { range.push(format(addDays(from, i), "yyyy-MM-dd")); } return range; }Also applies to: 126-132
apps/ui/src/app/dashboard/[orgId]/[projectId]/page.tsx (1)
13-17:daysis unused in thesearchParamstype.The
daysfield is still declared in the type but never referenced in the function body. Consider removing it to keep the interface clean and avoid confusion.Proposed fix
searchParams?: Promise<{ - days?: string; from?: string; to?: string; }>;apps/ui/src/components/dashboard/activity-chart.tsx (1)
454-468: RedundantgetUniqueModelsrecomputation in render.
getUniqueModels(data.activity)is called three times inside JSX (Lines 454, 455, 463), but the result is already stored inuniqueModelson Line 355. Reuse the existing variable to avoid redundant set construction on every render.Proposed fix
- {getUniqueModels(data.activity).length > 0 ? ( - getUniqueModels(data.activity).map((model, index) => ( + {uniqueModels.length > 0 ? ( + uniqueModels.map((model, index) => ( <Bar key={`${model}-${index}`} dataKey={model} name={model} stackId="models" fill={getModelColor(model, index)} radius={ - index === getUniqueModels(data.activity).length - 1 + index === uniqueModels.length - 1 ? [4, 4, 0, 0] : [0, 0, 0, 0] } />apps/ui/src/components/dashboard/overview.tsx (2)
177-189: Tooltipcontentpasses hardcoded dummy props that are misleading.The
active={true},payload={[{ value: 0 }]}, andlabel="tooltip"are placeholder values that get overridden at runtime by recharts viacloneElement. However, this pattern is confusing and fragile — if someone reads this code, they may think these are the actual values. The same pattern appears on Lines 262–270.The idiomatic recharts approach is to pass the component as a function or only pass custom props:
Proposed fix (costs chart, apply same pattern to requests chart)
<Tooltip - content={ - <CustomTooltip - active={true} - payload={[{ value: 0 }]} - label="tooltip" - metric="costs" - /> - } + content={(props) => <CustomTooltip {...props} metric="costs" />} cursor={{ fill: "color-mix(in srgb, currentColor 15%, transparent)", }} />
97-97:totalDaysis computed but never used.
totalDaysis calculated on Line 97 but not referenced anywhere in the component's output. It appears to be leftover from the migration away from thedaysprop.Proposed fix
- const totalDays = differenceInCalendarDays(to, from) + 1; const dateRange: string[] = []; - for (let i = 0; i < totalDays; i++) { + const totalDays = differenceInCalendarDays(to, from) + 1; + for (let i = 0; i < totalDays; i++) {Or simply inline it:
- const totalDays = differenceInCalendarDays(to, from) + 1; const dateRange: string[] = []; - for (let i = 0; i < totalDays; i++) { + for (let i = 0; i <= differenceInCalendarDays(to, from); i++) {
The days parameter is now optional (defaults to 7) since from/to date range params were added as alternatives. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Chores