feat(leaderboard): add user model drill-down and user insights page (#861)#886
feat(leaderboard): add user model drill-down and user insights page (#861)#886
Conversation
…861) - Add user-level modelStats data layer with null model preservation - Cache key isolation for user scope includeModelStats - Admin-only includeUserModelStats API gate (403 for non-admin) - Extract shared ModelBreakdownColumn/Row UI primitive - Admin-only expandable user rows with per-model breakdown - User name links to /dashboard/leaderboard/user/[userId] - Admin-only user insights page with overview cards, key trend chart, and model breakdown - Server actions for getUserInsightsOverview, KeyTrend, ModelBreakdown - Full i18n support (zh-CN, zh-TW, en, ja, ru) - 54 new tests across 6 test files
📝 WalkthroughWalkthrough新增按模型分解的数据路径与展示：扩展排行榜仓储与缓存以支持 includeModelStats/includeUserModelStats，新增管理员用户洞察后端动作与仓储，添加用户洞察页面及多个前端组件，并引入通用 ModelBreakdownColumn 组件与大量翻译与单元测试覆盖。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant new functionality for administrators, enabling a deeper understanding of individual user activity and resource consumption. It provides a comprehensive user insights dashboard accessible directly from the leaderboard, allowing admins to drill down into per-model usage statistics and historical trends. This enhances administrative oversight and facilitates more granular analysis of platform usage. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-actions
bot
added
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a user model drill-down in the leaderboard and a new user insights page, which are valuable additions for admin users. However, a critical vulnerability exists in the caching logic of the leaderboard API route, where sensitive data can be publicly cached even when global usage view is disabled, potentially leading to unauthorized data exposure via CDNs or shared caches. Furthermore, the review identified areas for improvement, including typing enhancements for better maintainability in new server actions and a bug in the user overview cards displaying an incorrect metric. Specific suggestions for these points are provided in the detailed comments.
src/app/api/leaderboard/route.ts
Outdated
| "Cache-Control": includeUserModelStats | ||
| ? "private, no-store" | ||
| : "public, s-maxage=60, stale-while-revalidate=120", |
There was a problem hiding this comment.
The API route sets a public cache header for leaderboard data even when the data is restricted to admins (i.e., when allowGlobalUsageView is disabled). This allows a CDN or shared cache to store the admin-only response and serve it to unauthorized users who request the same URL.
To fix this, the Cache-Control header should be set to private, no-store whenever the data is sensitive, which includes cases where allowGlobalUsageView is false.
| "Cache-Control": includeUserModelStats | |
| ? "private, no-store" | |
| : "public, s-maxage=60, stale-while-revalidate=120", | |
| "Cache-Control": (includeUserModelStats || !systemSettings.allowGlobalUsageView) | |
| ? "private, no-store" | |
| : "public, s-maxage=60, stale-while-revalidate=120", |
| key: "cacheHitRate", | ||
| label: t("cacheHitRate"), | ||
| value: `${overview.todayErrorRate.toFixed(1)}%`, | ||
| icon: Activity, | ||
| }, |
There was a problem hiding this comment.
There appears to be a bug in this metric card. The label is for 'Cache Hit Rate', but the value is being populated from overview.todayErrorRate. This will display the error rate under the cache hit rate label, which is misleading for users. Please use the correct property for cache hit rate from the overview object.
{
key: "cacheHitRate",
label: t("cacheHitRate"),
value: `${overview.cacheHitRate.toFixed(1)}%`,
icon: Activity,
},
src/actions/admin-user-insights.ts
Outdated
| export async function getUserInsightsKeyTrend( | ||
| targetUserId: number, | ||
| timeRange: string | ||
| ): Promise<ActionResult<unknown>> { |
There was a problem hiding this comment.
For better type safety and code clarity, the return type of getUserInsightsKeyTrend should be more specific. It currently returns Promise<ActionResult<unknown>>, but since the scope is hardcoded to 'keys', the data will always be DatabaseKeyStatRow[].
| export async function getUserInsightsKeyTrend( | |
| targetUserId: number, | |
| timeRange: string | |
| ): Promise<ActionResult<unknown>> { | |
| export async function getUserInsightsKeyTrend( | |
| targetUserId: number, | |
| timeRange: string | |
| ): Promise<ActionResult<import("@/types/statistics").DatabaseKeyStatRow[]>> { |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export async function getUserInsightsModelBreakdown( | ||
| targetUserId: number, | ||
| startDate?: string, | ||
| endDate?: string | ||
| ): Promise< | ||
| ActionResult<{ | ||
| breakdown: AdminUserModelBreakdownItem[]; | ||
| currencyCode: string; | ||
| }> | ||
| > { | ||
| const session = await getSession(); | ||
| if (!session || session.user.role !== "admin") { | ||
| return { ok: false, error: "Unauthorized" }; | ||
| } | ||
|
|
||
| const [breakdown, settings] = await Promise.all([ | ||
| getUserModelBreakdown(targetUserId, startDate, endDate), | ||
| getSystemSettings(), | ||
| ]); | ||
|
|
||
| return { | ||
| ok: true, | ||
| data: { | ||
| breakdown, | ||
| currencyCode: settings.currencyDisplay, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Missing date format validation for startDate / endDate
getUserInsightsModelBreakdown accepts startDate and endDate as arbitrary strings and passes them directly to the repository, which embeds them in SQL via sql\${startDate}::date``. No YYYY-MM-DD format check is performed.
If a caller passes a value like "not-a-date" or "01/25/2025", PostgreSQL will reject the cast and surface a raw DB error to the client rather than a clean validation error.
For comparison, the API route (src/app/api/leaderboard/route.ts) uses a DATE_REGEX = /^\d{4}-\d{2}-\d{2}$/ guard for the same kind of date parameters. The same pattern should be applied here:
// Add near the top of the file, alongside VALID_TIME_RANGES:
const DATE_REGEX = /^\d{4}-\d{2}-\d{2}$/;
// In getUserInsightsModelBreakdown, before calling getUserModelBreakdown:
if (startDate && !DATE_REGEX.test(startDate)) {
return { ok: false, error: "Invalid startDate format: use YYYY-MM-DD" };
}
if (endDate && !DATE_REGEX.test(endDate)) {
return { ok: false, error: "Invalid endDate format: use YYYY-MM-DD" };
}
if (startDate && endDate && new Date(startDate) > new Date(endDate)) {
return { ok: false, error: "startDate must not be after endDate" };
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/admin-user-insights.ts
Line: 85-112
Comment:
**Missing date format validation for `startDate` / `endDate`**
`getUserInsightsModelBreakdown` accepts `startDate` and `endDate` as arbitrary strings and passes them directly to the repository, which embeds them in SQL via `sql\`${startDate}::date\``. No YYYY-MM-DD format check is performed.
If a caller passes a value like `"not-a-date"` or `"01/25/2025"`, PostgreSQL will reject the cast and surface a raw DB error to the client rather than a clean validation error.
For comparison, the API route (`src/app/api/leaderboard/route.ts`) uses a `DATE_REGEX = /^\d{4}-\d{2}-\d{2}$/` guard for the same kind of date parameters. The same pattern should be applied here:
```typescript
// Add near the top of the file, alongside VALID_TIME_RANGES:
const DATE_REGEX = /^\d{4}-\d{2}-\d{2}$/;
// In getUserInsightsModelBreakdown, before calling getUserModelBreakdown:
if (startDate && !DATE_REGEX.test(startDate)) {
return { ok: false, error: "Invalid startDate format: use YYYY-MM-DD" };
}
if (endDate && !DATE_REGEX.test(endDate)) {
return { ok: false, error: "Invalid endDate format: use YYYY-MM-DD" };
}
if (startDate && endDate && new Date(startDate) > new Date(endDate)) {
return { ok: false, error: "startDate must not be after endDate" };
}
```
How can I resolve this? If you propose a fix, please make it concise.| key: "cacheHitRate", | ||
| label: t("cacheHitRate"), | ||
| value: `${overview.todayErrorRate.toFixed(1)}%`, | ||
| icon: Activity, | ||
| }, |
There was a problem hiding this comment.
Wrong metric value shown in "Cache Hit Rate" card
The fourth card resolves its label to "Cache Hit Rate" via t("cacheHitRate"), but the value uses overview.todayErrorRate — the error rate percentage — rather than a cache hit rate.
The OverviewMetrics interface (defined in src/repository/overview.ts) only contains: todayRequests, todayCost, avgResponseTime, and todayErrorRate. There is no cacheHitRate property in the overview data.
This causes admins viewing the user insights page to see an error rate percentage labeled as "Cache Hit Rate", which is actively misleading.
The fix requires either:
- Relabeling the card as "Error Rate" and updating the i18n key in all 5 language files; or
- Computing a real cache hit rate in the overview repository layer, adding it to
OverviewMetrics, and referencing that field here.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx
Line: 72-76
Comment:
**Wrong metric value shown in "Cache Hit Rate" card**
The fourth card resolves its label to "Cache Hit Rate" via `t("cacheHitRate")`, but the value uses `overview.todayErrorRate` — the error rate percentage — rather than a cache hit rate.
The `OverviewMetrics` interface (defined in `src/repository/overview.ts`) only contains: `todayRequests`, `todayCost`, `avgResponseTime`, and `todayErrorRate`. There is no `cacheHitRate` property in the overview data.
This causes admins viewing the user insights page to see an error rate percentage labeled as "Cache Hit Rate", which is actively misleading.
The fix requires either:
1. Relabeling the card as "Error Rate" and updating the i18n key in all 5 language files; or
2. Computing a real cache hit rate in the overview repository layer, adding it to `OverviewMetrics`, and referencing that field here.
How can I resolve this? If you propose a fix, please make it concise.| const dk = `key-${row.key_id}`; | ||
| entry[`${dk}_calls`] = row.api_calls || 0; | ||
| const cost = row.total_cost; | ||
| entry[`${dk}_cost`] = | ||
| typeof cost === "number" ? cost : cost != null ? Number.parseFloat(cost) || 0 : 0; | ||
| } |
There was a problem hiding this comment.
Computed cost data per key is never rendered in the chart
Each chart data entry computes both calls and cost values per key (lines 80–83), storing them as ${dk}_calls and ${dk}_cost. However, the Area elements in the chart (lines 178–191) only consume the calls data key. The cost entries are computed and stored but silently discarded when the chart renders.
The "Key Usage Trend" chart currently visualizes only call counts — admins cannot see per-key cost trends on this page despite the data being fetched and computed. Consider either removing the unused cost calculation to avoid confusion, or adding a metric selector to the card header so users can switch between calls and cost views.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx
Line: 79-84
Comment:
**Computed cost data per key is never rendered in the chart**
Each chart data entry computes both calls and cost values per key (lines 80–83), storing them as `${dk}_calls` and `${dk}_cost`. However, the `Area` elements in the chart (lines 178–191) only consume the calls data key. The cost entries are computed and stored but silently discarded when the chart renders.
The "Key Usage Trend" chart currently visualizes only call counts — admins cannot see per-key cost trends on this page despite the data being fetched and computed. Consider either removing the unused cost calculation to avoid confusion, or adding a metric selector to the card header so users can switch between calls and cost views.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (5)
src/components/analytics/model-breakdown-column.tsx (1)
100-135: 共享组件别默认绑死myUsage.stats命名空间。这个组件已经被抽成通用 primitive，但 fallback 仍然读取
myUsage.stats.*。只要新调用方漏传labels，就会静默拿到错误文案，而且类型层面也发现不了。更稳妥的是让labels变为必填，或者把翻译函数 / namespace 显式透传进来。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/analytics/model-breakdown-column.tsx` around lines 100 - 135, The useLabels helper currently defaults to the hardcoded translations namespace "myUsage.stats" when labels are omitted, causing shared consumers to silently get wrong text; update useLabels (and the ModelBreakdownLabels usage) so labels is required or accept an explicit translation function/namespace parameter instead of calling useTranslations("myUsage.stats") inside useLabels — e.g., change the signature of useLabels to require labels: ModelBreakdownLabels or to accept (labels?: ModelBreakdownLabels, t?: Translator) and remove the internal useTranslations("myUsage.stats") call, then update callers of useLabels/ModelBreakdownColumn to pass the proper labels or translation function.src/repository/leaderboard.ts (1)
16-30: 把新公开类型里的空值语义写清楚。
modelStats现在在运行时已经区分了“未请求”和“请求后为空”，UserModelStat.model也允许null表示 unknown bucket；但接口层没有把这些约定写出来。建议像ProviderLeaderboardEntry.modelStats一样补齐注释，避免调用方把undefined、[]和null混用。Based on learnings: "In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined."建议修改
export interface UserModelStat { + /** null 表示 unknown / empty-model bucket */ model: string | null; totalRequests: number; totalCost: number; totalTokens: number; } export interface LeaderboardEntry { userId: number; userName: string; totalRequests: number; totalCost: number; totalTokens: number; + /** + * 可选：按模型拆分 + * - undefined: 未请求 includeModelStats + * - []: 已请求 includeModelStats，但该 user 下无可用模型统计 + */ modelStats?: UserModelStat[]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/leaderboard.ts` around lines 16 - 30, The public interfaces lack explicit semantics for null vs undefined vs empty arrays: update UserModelStat and LeaderboardEntry to document and enforce meanings (e.g., add JSDoc to UserModelStat.model stating null = "unknown bucket", and to LeaderboardEntry.modelStats stating undefined = "not populated by this API", null = "queried but no model-level stats exist", and [] = "populated and zero entries"); if desired, adjust the type of modelStats to modelStats?: UserModelStat[] | null to reflect those distinctions and keep UserModelStat.model: string | null with the documented meaning. Ensure references to the symbols UserModelStat, UserModelStat.model, LeaderboardEntry, and modelStats are updated accordingly so callers won’t conflate undefined, null, and empty arrays.tests/unit/api/leaderboard-route.test.ts (1)
325-349: 403 用例最好顺手卡住“不会触发数据读取”。这两个分支现在只校验了状态码。若后续有人把鉴权挪到
getLeaderboardWithCache之后，测试仍会是绿色。这里补一条expect(mocks.getLeaderboardWithCache).not.toHaveBeenCalled()，能更准确地守住 admin gate。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/api/leaderboard-route.test.ts` around lines 325 - 349, The two tests that assert 403 should also assert that the data-loading helper was not invoked: add an assertion in both "non-admin + includeUserModelStats=1 returns 403" and "non-admin with allowGlobalUsageView + includeUserModelStats=1 returns 403" to verify mocks.getLeaderboardWithCache was not called (expect(mocks.getLeaderboardWithCache).not.toHaveBeenCalled()), so the tests fail if auth is moved after getLeaderboardWithCache; place this check after the existing expect(response.status).toBe(403) in each test.tests/unit/components/model-breakdown-column.test.tsx (1)
22-35: 这个Dialogmock 会让交互相关断言失真。
DialogContent在 mock 里始终渲染，所以后面那些“弹窗里出现了某些文案”的断言，即使真实的 trigger/open 逻辑坏掉也会继续通过。这里至少要把 open 行为模拟出来，或者改成 DOM render + 用户交互去验证。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/components/model-breakdown-column.test.tsx` around lines 22 - 35, The Dialog mock in tests/unit/components/model-breakdown-column.test.tsx always renders DialogContent so interaction assertions are false positives; update the mock for Dialog/DialogContent (and related DialogHeader/DialogTitle) to honor an "open" or "onOpenChange" prop (or remove the mock and render the real Dialog) so content only appears when open is true, then update tests to trigger the real open action via user events (e.g., click the trigger) and assert that DialogContent appears only after the interaction.tests/unit/dashboard/user-insights-page.test.tsx (1)
365-411: 建议：补充错误状态测试当前测试覆盖了正常数据返回的场景，建议增加 API 返回错误时的测试用例（如
ok: false），以及UserModelBreakdown组件的日期范围 Apply/Clear 交互测试。📝 示例：错误状态测试
it("handles API error gracefully", async () => { mockGetUserInsightsModelBreakdown.mockResolvedValue({ ok: false, error: "Failed to fetch data", }); const { UserModelBreakdown } = await import( "@/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown" ); const { container, unmount } = renderWithProviders(<UserModelBreakdown userId={10} />); await flushMicrotasks(); // 验证错误状态处理 // ... unmount(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/dashboard/user-insights-page.test.tsx` around lines 365 - 411, Add a test that covers the API error path by having mockGetUserInsightsModelBreakdown.mockResolvedValue return { ok: false, error: "..." }, then import UserModelBreakdown and render it with renderWithProviders(userId={10}), await flushMicrotasks(), and assert the component shows the expected error/fallback UI (e.g. an error message or empty state selector like [data-testid='user-insights-model-breakdown-list'] not present). Also add a separate test that imports UserModelBreakdown, renders it, simulates the date-range interactions (open date inputs, set range, click Apply, assert list updates; click Clear and assert list resets) using the same renderWithProviders/flushMicrotasks/unmount pattern and targeting the date controls and data-testid attributes already present in the component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/actions/admin-user-insights.ts`:
- Around line 85-103: The action getUserInsightsModelBreakdown currently
forwards startDate/endDate to the repo and can throw DB errors on invalid dates
or reversed ranges; add input validation at the top of
getUserInsightsModelBreakdown (similar to getUserInsightsKeyTrend): ensure
startDate/endDate, if provided, match an expected date format (e.g. ISO
YYYY-MM-DD) and parse successfully, and verify startDate <= endDate; if
validation fails return an ActionResult with ok: false and a clear error message
instead of calling getUserModelBreakdown/getSystemSettings. Reference symbols:
getUserInsightsModelBreakdown, startDate, endDate, getUserModelBreakdown,
getSystemSettings.
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-insights-view.tsx:
- Around line 23-30: The back Button currently calls
router.push("/dashboard/leaderboard?scope=user") which loses context like
period/dateRange/tag/group; change the onClick handler in the UserInsightsView
component to first check for a returnTo query parameter (e.g., read
router.query.returnTo or a prop) and, if present, router.push(returnTo),
otherwise fall back to the existing URL ("/dashboard/leaderboard?scope=user");
ensure you preserve any existing query params when constructing the fallback or
when parsing returnTo so filters and date ranges are restored.
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx:
- Around line 41-48: The queryFn currently throws on non-ok responses which ends
up treated as empty state; change the useQuery usage for the user key trend (the
call using queryKey ["user-insights-key-trend", userId, timeRange] and queryFn
that calls getUserInsightsKeyTrend) to expose and handle query errors
separately: don't swallow errors into the "noData" path—use the useQuery return
values (e.g., isError and error) alongside isLoading and rawData to render an
explicit error UI or message when the request fails, and keep the empty/no-data
rendering only when rawData is present but empty; apply the same fix to the
other identical block at the later location (lines around the second useQuery
block noted in the comment).
- Around line 80-83: The chart only ever renders `${key.dataKey}_calls` and has
no UI to switch to cost, so although `total_cost` is included in `chartData`
(the `entry[`${dk}_cost`]` assignment) the cost series is never reachable;
update the component in user-key-trend-chart.tsx to support both metrics by
adding a view-mode state (e.g., 'calls' | 'cost') and a small toggle control in
the header to switch modes, then change the chart-series dataKey logic (the
place currently hardcoding `${key.dataKey}_calls`) to use the selected mode and
build the series dataKey dynamically (`${key.dataKey}_calls` or
`${key.dataKey}_cost`), ensure axis/tooltip/label units switch accordingly, and
apply the same fix in the other two analogous blocks (the logic around lines
118-129 and 178-189) so cost is actually selectable and rendered.
- Around line 87-89: The chart config is using a hardcoded user-facing label
("Calls") which bypasses i18n; update the ChartConfig construction (the const
config used to set calls: { label: "Calls" }) to use the translation key instead
(e.g., t('dashboard.leaderboard.userInsights.calls') or the existing
request/class copy) via the app's i18n helper so the tooltip/legend pulls
localized text; ensure you import/use the same translator instance used in this
component and add the new translation key entries for all supported locales if
they don't exist.
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx:
- Around line 113-115: Replace the hard-coded "OK" label in the Button (the JSX
using onClick={handleApplyRange}) with a translated string pulled from the i18n
helper (use the key leaderboard.userInsights.apply), and add that key to the
five locale translation files (zh-CN, zh-TW, en, ja, ru) with appropriate
translations; ensure the component imports/uses the same translation function
(e.g., t or useTranslations) already used in this file so the Button label reads
t('leaderboard.userInsights.apply') instead of the literal "OK".
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx:
- Around line 52-77: 当前 metrics 数组只提供单一当前值，未实现“今日 vs 昨日”对比且 cacheHitRate 键错误地使用
overview.todayErrorRate；请改为为每个项（keys: todayRequests, todayCost, avgResponseTime,
cacheHitRate）携带 today、yesterday 和 delta 字段并复用项目中已有的比较展示组件（如 ComparisonCard
或相同数据结构），把值计算逻辑改为分别使用 overview.todayRequests /
overview.yesterdayRequests、formatCurrency(overview.todayCost) /
formatCurrency(overview.yesterdayCost)、formatResponseTime(overview.avgResponseTime)
/ formatResponseTime(overview.yesterdayAvgResponseTime) 和 cacheHitRate 使用
overview.todayCacheHitRate / overview.yesterdayCacheHitRate，同时修正原先对
todayErrorRate 的误用并确保 delta 用百分比或绝对值格式化与现有 comparison 组件兼容。
- Around line 72-75: The card currently labeled as cacheHitRate (key:
"cacheHitRate", label: t("cacheHitRate")) is showing overview.todayErrorRate;
fix by either (A) showing the real cache hit rate: replace value
`${overview.todayErrorRate.toFixed(1)}%` with the correct field (e.g.
`${overview.todayCacheHitRate.toFixed(1)}%` or the actual cache hit property on
overview) or (B) if the intent is to show error rate, rename key and label to
"errorRate" (key: "errorRate", label: t("errorRate")) and keep the value as
overview.todayErrorRate; ensure the icon/context matches the chosen metric.
In `@src/app/`[locale]/dashboard/leaderboard/user/[userId]/page.tsx:
- Around line 20-23: Tighten userId validation so only positive integers are
accepted: check the raw path param userIdStr matches a positive-integer pattern
(e.g. no decimals, no exponent, not Infinity) or parse and verify
Number.isInteger(userId) && userId > 0 before calling findUserById; if the check
fails, call redirect({ href: "/dashboard/leaderboard", locale }) as before.
Ensure you reference and validate userIdStr/userId and gate the findUserById
call accordingly.
In `@src/app/api/leaderboard/route.ts`:
- Around line 145-149: The branch that returns a hardcoded English message when
includeUserModelStats is true and the user is not admin should instead return a
stable error code; change the NextResponse.json payload in the
includeUserModelStats && !isAdmin branch to return a machine-readable error
field (e.g. { errorCode: "INCLUDE_USER_MODEL_STATS_ADMIN_REQUIRED" } or { code:
"FORBIDDEN_INCLUDE_USER_MODEL_STATS" }) with status 403 so the frontend can
translate it, leaving NextResponse.json, includeUserModelStats and isAdmin
checks intact.
In `@src/repository/admin-user-insights.ts`:
- Around line 38-44: 当前对 startDate/endDate 的比较直接使用 `::date` 截断（在条件里对
`usageLedger.createdAt` 用 sql`${startDate}::date` / sql`(${endDate}::date +
INTERVAL '1 day')`），未考虑系统时区，会导致与 `leaderboard.ts` 的自定义日期窗口不一致；请复用
`leaderboard.ts` 中对自定义日期窗口的时区折算逻辑，或在构造 SQL 条件时对 `usageLedger.createdAt` 与传入的
`startDate`/`endDate` 明确使用 `AT TIME ZONE` 做时区转换，然后再进行 `>=` / `<` 比较，确保在
`conditions.push(...)` 中比较的边界以系统时区为准（参照 `startDate`, `endDate`,
`usageLedger.createdAt`, `sql` 使用位置）。
- Around line 31-34: The COALESCE used for modelField preserves empty or
whitespace-only model names, causing inconsistent grouping vs leaderboard;
update the expression to wrap both ${usageLedger.model} and
${usageLedger.originalModel} with NULLIF(TRIM(...), '') so empty/whitespace
strings become NULL before COALESCE (i.e., use
NULLIF(TRIM(${usageLedger.originalModel}), '') and
NULLIF(TRIM(${usageLedger.model}), '') in the two COALESCE branches, keeping the
billingModelSource conditional and sql<string> usage intact).
In `@tests/unit/repository/leaderboard-user-model-stats.test.ts`:
- Around line 10-16: The orderBy mock in createChainMock returns pre-sorted data
so the test is a false positive; update the tests to verify the repository
actually calls orderBy with the expected sort expression (e.g., "totalCost
desc") on the second query chain instead of relying on returned data. Modify
createChainMock/orderBy to record its input arguments (or use vi.fn()
assertions) and in the failing test assert that createChainMock().orderBy was
called with the correct ordering expression for the second chain (or expose the
passed args from the mock and assert they equal "totalCost desc"); apply the
same change to the other tests mentioned (lines 287-324) that rely on this mock.
---
Nitpick comments:
In `@src/components/analytics/model-breakdown-column.tsx`:
- Around line 100-135: The useLabels helper currently defaults to the hardcoded
translations namespace "myUsage.stats" when labels are omitted, causing shared
consumers to silently get wrong text; update useLabels (and the
ModelBreakdownLabels usage) so labels is required or accept an explicit
translation function/namespace parameter instead of calling
useTranslations("myUsage.stats") inside useLabels — e.g., change the signature
of useLabels to require labels: ModelBreakdownLabels or to accept (labels?:
ModelBreakdownLabels, t?: Translator) and remove the internal
useTranslations("myUsage.stats") call, then update callers of
useLabels/ModelBreakdownColumn to pass the proper labels or translation
function.
In `@src/repository/leaderboard.ts`:
- Around line 16-30: The public interfaces lack explicit semantics for null vs
undefined vs empty arrays: update UserModelStat and LeaderboardEntry to document
and enforce meanings (e.g., add JSDoc to UserModelStat.model stating null =
"unknown bucket", and to LeaderboardEntry.modelStats stating undefined = "not
populated by this API", null = "queried but no model-level stats exist", and []
= "populated and zero entries"); if desired, adjust the type of modelStats to
modelStats?: UserModelStat[] | null to reflect those distinctions and keep
UserModelStat.model: string | null with the documented meaning. Ensure
references to the symbols UserModelStat, UserModelStat.model, LeaderboardEntry,
and modelStats are updated accordingly so callers won’t conflate undefined,
null, and empty arrays.
In `@tests/unit/api/leaderboard-route.test.ts`:
- Around line 325-349: The two tests that assert 403 should also assert that the
data-loading helper was not invoked: add an assertion in both "non-admin +
includeUserModelStats=1 returns 403" and "non-admin with allowGlobalUsageView +
includeUserModelStats=1 returns 403" to verify mocks.getLeaderboardWithCache was
not called (expect(mocks.getLeaderboardWithCache).not.toHaveBeenCalled()), so
the tests fail if auth is moved after getLeaderboardWithCache; place this check
after the existing expect(response.status).toBe(403) in each test.
In `@tests/unit/components/model-breakdown-column.test.tsx`:
- Around line 22-35: The Dialog mock in
tests/unit/components/model-breakdown-column.test.tsx always renders
DialogContent so interaction assertions are false positives; update the mock for
Dialog/DialogContent (and related DialogHeader/DialogTitle) to honor an "open"
or "onOpenChange" prop (or remove the mock and render the real Dialog) so
content only appears when open is true, then update tests to trigger the real
open action via user events (e.g., click the trigger) and assert that
DialogContent appears only after the interaction.
In `@tests/unit/dashboard/user-insights-page.test.tsx`:
- Around line 365-411: Add a test that covers the API error path by having
mockGetUserInsightsModelBreakdown.mockResolvedValue return { ok: false, error:
"..." }, then import UserModelBreakdown and render it with
renderWithProviders(userId={10}), await flushMicrotasks(), and assert the
component shows the expected error/fallback UI (e.g. an error message or empty
state selector like [data-testid='user-insights-model-breakdown-list'] not
present). Also add a separate test that imports UserModelBreakdown, renders it,
simulates the date-range interactions (open date inputs, set range, click Apply,
assert list updates; click Clear and assert list resets) using the same
renderWithProviders/flushMicrotasks/unmount pattern and targeting the date
controls and data-testid attributes already present in the component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da8b4785-6ba5-4bf3-be64-d469148337b2
📒 Files selected for processing (23)
messages/en/dashboard.jsonmessages/ja/dashboard.jsonmessages/ru/dashboard.jsonmessages/zh-CN/dashboard.jsonmessages/zh-TW/dashboard.jsonsrc/actions/admin-user-insights.tssrc/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsxsrc/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-insights-view.tsxsrc/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsxsrc/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsxsrc/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsxsrc/app/[locale]/dashboard/leaderboard/user/[userId]/page.tsxsrc/app/[locale]/my-usage/_components/statistics-summary-card.tsxsrc/app/api/leaderboard/route.tssrc/components/analytics/model-breakdown-column.tsxsrc/lib/redis/leaderboard-cache.tssrc/repository/admin-user-insights.tssrc/repository/leaderboard.tstests/unit/actions/admin-user-insights.test.tstests/unit/api/leaderboard-route.test.tstests/unit/components/model-breakdown-column.test.tsxtests/unit/dashboard/user-insights-page.test.tsxtests/unit/repository/leaderboard-user-model-stats.test.ts
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={() => router.push("/dashboard/leaderboard?scope=user")} | ||
| > | ||
| <ArrowLeft className="h-4 w-4 mr-2" /> | ||
| {t("backToLeaderboard")} | ||
| </Button> |
There was a problem hiding this comment.
返回按钮会把用户带回“默认榜单”，不是来源榜单。
这里直接 push("/dashboard/leaderboard?scope=user")，从筛选后的榜单或自定义日期榜单进入详情后，period/dateRange/tag/group 等上下文都会丢掉。建议优先读取并恢复一个 returnTo 参数，没有时再 fallback 到默认地址。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-insights-view.tsx
around lines 23 - 30, The back Button currently calls
router.push("/dashboard/leaderboard?scope=user") which loses context like
period/dateRange/tag/group; change the onClick handler in the UserInsightsView
component to first check for a returnTo query parameter (e.g., read
router.query.returnTo or a prop) and, if present, router.push(returnTo),
otherwise fall back to the existing URL ("/dashboard/leaderboard?scope=user");
ensure you preserve any existing query params when constructing the fallback or
when parsing returnTo so filters and date ranges are restored.
| const { data: rawData, isLoading } = useQuery({ | ||
| queryKey: ["user-insights-key-trend", userId, timeRange], | ||
| queryFn: async () => { | ||
| const result = await getUserInsightsKeyTrend(userId, timeRange); | ||
| if (!result.ok) throw new Error(result.error); | ||
| return result.data as DatabaseKeyStatRow[]; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
不要把查询失败伪装成“无数据”。
queryFn 抛错后，这里会直接落到空态分支，最终给管理员展示 noData。这样会把鉴权失败、服务端异常和真实空数据混在一起，排障时很难判断是“没有使用记录”还是“接口坏了”。建议显式区分 isError 和空数据态。
Also applies to: 134-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx
around lines 41 - 48, The queryFn currently throws on non-ok responses which
ends up treated as empty state; change the useQuery usage for the user key trend
(the call using queryKey ["user-insights-key-trend", userId, timeRange] and
queryFn that calls getUserInsightsKeyTrend) to expose and handle query errors
separately: don't swallow errors into the "noData" path—use the useQuery return
values (e.g., isError and error) alongside isLoading and rawData to render an
explicit error UI or message when the request fails, and keep the empty/no-data
rendering only when rawData is present but empty; apply the same fix to the
other identical block at the later location (lines around the second useQuery
block noted in the comment).
| entry[`${dk}_calls`] = row.api_calls || 0; | ||
| const cost = row.total_cost; | ||
| entry[`${dk}_cost`] = | ||
| typeof cost === "number" ? cost : cost != null ? Number.parseFloat(cost) || 0 : 0; |
There was a problem hiding this comment.
把成本趋势补出来。
这里已经把 total_cost 写进了 chartData，但图表始终只渲染 ${key.dataKey}_calls，界面上也没有切换 requests/cost 的入口。按这次需求，管理员需要看得到按 Key 拆分的请求量和成本趋势；当前成本数据虽然取回来了，但整个路径都不可达。
Also applies to: 118-129, 178-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx
around lines 80 - 83, The chart only ever renders `${key.dataKey}_calls` and has
no UI to switch to cost, so although `total_cost` is included in `chartData`
(the `entry[`${dk}_cost`]` assignment) the cost series is never reachable;
update the component in user-key-trend-chart.tsx to support both metrics by
adding a view-mode state (e.g., 'calls' | 'cost') and a small toggle control in
the header to switch modes, then change the chart-series dataKey logic (the
place currently hardcoding `${key.dataKey}_calls`) to use the selected mode and
build the series dataKey dynamically (`${key.dataKey}_calls` or
`${key.dataKey}_cost`), ensure axis/tooltip/label units switch accordingly, and
apply the same fix in the other two analogous blocks (the logic around lines
118-129 and 178-189) so cost is actually selectable and rendered.
| const config: ChartConfig = { | ||
| calls: { label: "Calls" }, | ||
| }; |
There was a problem hiding this comment.
把 Calls 改成翻译键。
这是用户可见文本，当前硬编码会绕过多语言体系；一旦 tooltip/legend 使用到这个 label，非英文 locale 就会直接漏翻。可复用已有的请求类文案，或补一个 dashboard.leaderboard.userInsights 下的新键。
As per coding guidelines, "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx
around lines 87 - 89, The chart config is using a hardcoded user-facing label
("Calls") which bypasses i18n; update the ChartConfig construction (the const
config used to set calls: { label: "Calls" }) to use the translation key instead
(e.g., t('dashboard.leaderboard.userInsights.calls') or the existing
request/class copy) via the app's i18n helper so the tooltip/legend pulls
localized text; ensure you import/use the same translator instance used in this
component and add the new translation key entries for all supported locales if
they don't exist.
| const modelField = | ||
| billingModelSource === "original" | ||
| ? sql<string>`COALESCE(${usageLedger.originalModel}, ${usageLedger.model})` | ||
| : sql<string>`COALESCE(${usageLedger.model}, ${usageLedger.originalModel})`; |
There was a problem hiding this comment.
把空模型归一化逻辑和排行榜保持一致。
这里直接 COALESCE(...) 会把 "" / 仅空白字符串保留成独立分组，而 src/repository/leaderboard.ts 已经用 NULLIF(TRIM(...), '') 把它们并到 unknown bucket。现在同一个用户在排行榜展开行和 insights 页面里可能会看到不一致的模型拆分，甚至出现空白模型名。
建议修改
- const modelField =
- billingModelSource === "original"
- ? sql<string>`COALESCE(${usageLedger.originalModel}, ${usageLedger.model})`
- : sql<string>`COALESCE(${usageLedger.model}, ${usageLedger.originalModel})`;
+ const rawModelField =
+ billingModelSource === "original"
+ ? sql<string>`COALESCE(${usageLedger.originalModel}, ${usageLedger.model})`
+ : sql<string>`COALESCE(${usageLedger.model}, ${usageLedger.originalModel})`;
+ const modelField = sql<string | null>`NULLIF(TRIM(${rawModelField}), '')`;📝 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 modelField = | |
| billingModelSource === "original" | |
| ? sql<string>`COALESCE(${usageLedger.originalModel}, ${usageLedger.model})` | |
| : sql<string>`COALESCE(${usageLedger.model}, ${usageLedger.originalModel})`; | |
| const rawModelField = | |
| billingModelSource === "original" | |
| ? sql<string>`COALESCE(${usageLedger.originalModel}, ${usageLedger.model})` | |
| : sql<string>`COALESCE(${usageLedger.model}, ${usageLedger.originalModel})`; | |
| const modelField = sql<string | null>`NULLIF(TRIM(${rawModelField}), '')`; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repository/admin-user-insights.ts` around lines 31 - 34, The COALESCE
used for modelField preserves empty or whitespace-only model names, causing
inconsistent grouping vs leaderboard; update the expression to wrap both
${usageLedger.model} and ${usageLedger.originalModel} with NULLIF(TRIM(...), '')
so empty/whitespace strings become NULL before COALESCE (i.e., use
NULLIF(TRIM(${usageLedger.originalModel}), '') and
NULLIF(TRIM(${usageLedger.model}), '') in the two COALESCE branches, keeping the
billingModelSource conditional and sql<string> usage intact).
| if (startDate) { | ||
| conditions.push(gte(usageLedger.createdAt, sql`${startDate}::date`)); | ||
| } | ||
|
|
||
| if (endDate) { | ||
| conditions.push(lt(usageLedger.createdAt, sql`(${endDate}::date + INTERVAL '1 day')`)); | ||
| } |
There was a problem hiding this comment.
自定义日期范围这里没有按系统时区换算。
leaderboard.ts 的日期窗口会先按系统时区折算后再比较 createdAt。这里直接用 ::date 比较会按数据库会话时区截断，在非 UTC 系统时区下会把边界前后各错几个小时，导致 insights 页面和排行榜/日志对同一日期看到的结果不一致。建议复用 leaderboard.ts 的 custom date-window 处理，或者至少在这里用 AT TIME ZONE 明确换算。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repository/admin-user-insights.ts` around lines 38 - 44, 当前对
startDate/endDate 的比较直接使用 `::date` 截断（在条件里对 `usageLedger.createdAt` 用
sql`${startDate}::date` / sql`(${endDate}::date + INTERVAL '1
day')`），未考虑系统时区，会导致与 `leaderboard.ts` 的自定义日期窗口不一致；请复用 `leaderboard.ts`
中对自定义日期窗口的时区折算逻辑，或在构造 SQL 条件时对 `usageLedger.createdAt` 与传入的
`startDate`/`endDate` 明确使用 `AT TIME ZONE` 做时区转换，然后再进行 `>=` / `<` 比较，确保在
`conditions.push(...)` 中比较的边界以系统时区为准（参照 `startDate`, `endDate`,
`usageLedger.createdAt`, `sql` 使用位置）。
| const createChainMock = (resolvedData: unknown[]) => ({ | ||
| from: vi.fn().mockReturnThis(), | ||
| innerJoin: vi.fn().mockReturnThis(), | ||
| where: vi.fn().mockReturnThis(), | ||
| groupBy: vi.fn().mockReturnThis(), | ||
| orderBy: vi.fn().mockResolvedValue(resolvedData), | ||
| }); |
There was a problem hiding this comment.
这个排序用例目前是伪阳性。
这里的 orderBy mock 只是把预置好的有序数据直接返回，所以即使仓储查询里不再按 totalCost desc 排序，这个测试也一样会通过。更稳妥的做法是断言第二个 chain 的 orderBy 确实被调用，或者让 mock 暴露出传入的排序表达式来校验。
Also applies to: 287-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/repository/leaderboard-user-model-stats.test.ts` around lines 10 -
16, The orderBy mock in createChainMock returns pre-sorted data so the test is a
false positive; update the tests to verify the repository actually calls orderBy
with the expected sort expression (e.g., "totalCost desc") on the second query
chain instead of relying on returned data. Modify createChainMock/orderBy to
record its input arguments (or use vi.fn() assertions) and in the failing test
assert that createChainMock().orderBy was called with the correct ordering
expression for the second chain (or expose the passed args from the mock and
assert they equal "totalCost desc"); apply the same change to the other tests
mentioned (lines 287-324) that rely on this mock.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR implements user leaderboard drill-down functionality and a dedicated user insights page with overview cards, trend charts, and model breakdown. While the overall architecture is sound with proper auth layering and cache isolation, there are 3 issues requiring attention before merge.
PR Size: L
- Lines changed: 3,051 (2,750 additions + 301 deletions)
- Files changed: 23
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 1 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 0 | 0 |
| Performance | 0 | 0 | 1 | 0 |
| Total | 1 | 1 | 1 | 0 |
Critical Issues (Must Fix)
[LOGIC-BUG] Cache Hit Rate card displays error rate instead of cache hit rate
Location: src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx line 74
Problem: The "Cache Hit Rate" metric card displays overview.todayErrorRate instead of an actual cache hit rate. This is a critical UI bug that actively misleads admins - they believe they are viewing cache performance when they are actually seeing error rates.
// Current (WRONG):
{
key: "cacheHitRate",
label: t("cacheHitRate"),
value: `${overview.todayErrorRate.toFixed(1)}%`, // This is error rate!
icon: Activity,
}Fix: Either add a cacheHitRate field to OverviewMetrics type and populate it, or temporarily remove/replace this card until the data is available.
High Priority Issues (Should Fix)
[ERROR-NO-USER-FEEDBACK] Date inputs lack format validation in server action
Location: src/actions/admin-user-insights.ts lines 85-112 (getUserInsightsModelBreakdown)
Problem: The action passes startDate and endDate directly to the repository without validating the YYYY-MM-DD format. Unlike the API route (/api/leaderboard/route.ts lines 112-114) which validates with DATE_REGEX, invalid inputs here will reach the database and surface raw SQL errors to clients.
Fix: Add the same DATE_REGEX validation in the server action before passing dates to the repository.
Medium Priority Issues (Consider Fixing)
[PERFORMANCE-ISSUE] Computed cost data is discarded in chart
Location: src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx lines 80-84, 183
Problem: The code computes per-key cost values and stores them in chart data, but the AreaChart only renders call counts. The cost data is silently discarded - this is wasted computation.
Fix: Either remove the unused cost computation, or add a metric toggle to let users switch between viewing calls vs cost.
Review Coverage
- Logic and correctness - 1 critical bug found
- Security (OWASP Top 10) - Clean, proper auth checks throughout
- Error handling - 1 gap identified (date validation)
- Type safety - Clean, proper TypeScript usage
- Documentation accuracy - Clean
- Test coverage - Good, 54 new tests across 5 test files
- Code clarity - Clean, well-structured components
Summary
Recommendation: Request Changes - The critical bug where "Cache Hit Rate" displays error rate must be fixed before merge. The date validation gap should also be addressed to prevent SQL errors reaching users.
The overall architecture is sound with proper:
- Auth gating at all layers (page, server actions, API route)
- Redis cache key isolation with
:includeModelStatssuffix - Consistent billingModelSource-aware model resolution pattern
- Good test coverage (54 new tests)
Automated review by Claude AI
| key: "cacheHitRate", | ||
| label: t("cacheHitRate"), | ||
| value: `${overview.todayErrorRate.toFixed(1)}%`, | ||
| icon: Activity, |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] User Insights "Cache Hit Rate" card is rendering error rate
Evidence: src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx:72-75
{
key: "cacheHitRate",
label: t("cacheHitRate"),
value: `${overview.todayErrorRate.toFixed(1)}%`,
}todayErrorRate is an error-rate metric, but the label (and i18n strings) say "Cache Hit Rate", so the page will mislead admins.
Suggested fix (align label + key with the actual value):
// src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx
{
key: "errorRate",
label: t("errorRate"),
value: `${overview.todayErrorRate.toFixed(1)}%`,
icon: Activity,
}And update i18n keys under leaderboard.userInsights in all 5 locales (e.g. messages/en/dashboard.json) from cacheHitRate -> errorRate with the correct translation.
| } | ||
|
|
||
| const statistics = await getStatisticsWithCache(timeRange, "keys", targetUserId); | ||
|
|
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Key trend action returns raw stats with non-string date on cache miss (can crash the chart)
Evidence:
src/actions/admin-user-insights.ts:77-80
const statistics = await getStatisticsWithCache(timeRange, "keys", targetUserId);
return { ok: true, data: statistics };getStatisticsWithCache()can return DB rows wheredateis aDateobject on a cold cache (e.g.src/repository/statistics.ts:228pushesdate: new Date(bucketTime)).- The client assumes
dateis a string and doesrow.date.split("T")[0]insrc/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx:73, which will throw ifrow.dateis aDate.
Suggested fix (normalize + type the action response so the client contract is stable):
// src/actions/admin-user-insights.ts
import type { DatabaseKeyStatRow } from "@/types/statistics";
export async function getUserInsightsKeyTrend(
targetUserId: number,
timeRange: string
): Promise<ActionResult<DatabaseKeyStatRow[]>> {
// ...auth + validation...
const statistics = (await getStatisticsWithCache(
timeRange,
"keys",
targetUserId
)) as DatabaseKeyStatRow[];
const normalized = statistics.map((row) => ({
...row,
date: new Date(row.date).toISOString(),
}));
return { ok: true, data: normalized };
}
| /> | ||
| <Button size="sm" variant="outline" className="h-7 text-xs" onClick={handleApplyRange}> | ||
| OK | ||
| </Button> |
There was a problem hiding this comment.
[MEDIUM] [STANDARD-VIOLATION] Hardcoded button label bypasses i18n
Evidence:
src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx:113-115
<Button ...>
OK
</Button>- Project rule:
CLAUDE.md:14— "i18n Required - All user-facing strings must use i18n (5 languages supported). Never hardcode display text"
Suggested fix (reuse existing common translation):
// src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx
const tCommon = useTranslations("common");
// ...
<Button size="sm" variant="outline" className="h-7 text-xs" onClick={handleApplyRange}>
{tCommon("ok")}
</Button>
|
|
||
| const cards = container.querySelectorAll("[data-testid^='user-insights-metric-']"); | ||
| expect(cards.length).toBe(4); | ||
|
|
There was a problem hiding this comment.
[MEDIUM] [TEST-INCOMPLETE] Overview cards test doesn\x27t assert the rate metric wiring
Evidence: tests/unit/dashboard/user-insights-page.test.tsx:238-279
This test only asserts todayRequests and avgResponseTime, so a mislabeled/miswired rate metric (see the cacheHitRate/todayErrorRate mismatch) won\x27t be caught.
Suggested fix (assert the 4th card label + value explicitly):
// tests/unit/dashboard/user-insights-page.test.tsx
const errorRate = container.querySelector(
"[data-testid=user-insights-metric-errorRate]"
);
expect(errorRate).not.toBeNull();
expect(errorRate!.textContent).toContain("Error Rate");
expect(errorRate!.textContent).toContain("2.5%");
There was a problem hiding this comment.
Code Review Summary
This PR is XL and combines data-layer changes, API behavior, UI, i18n, and tests. I found user-insights correctness issues (miswired metric + unstable key-trend data contract) and an i18n rule violation.
PR Size: XL
- Lines changed: 3051
- Files changed: 23
- Split suggestion (XL): split into (1) repository/API/cache changes, (2) user-insights UI + shared components + i18n, (3) tests-only.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 2 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 1 | 0 | 0 |
| Comments/Docs | 0 | 0 | 1 | 0 |
| Tests | 0 | 0 | 1 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- (Confidence 100)
src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx:72-75— "Cache Hit Rate" card is wired tooverview.todayErrorRate(error rate), so admins see a mislabeled metric.
High Priority Issues (Should Fix)
- (Confidence 90)
src/actions/admin-user-insights.ts:77-80+src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx:73— key-trend action can return rows wheredateis aDate(cold cache), but the client callsrow.date.split(...), which can throw. - (Confidence 85)
src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx:113-115— hardcodedOKlabel violates i18n; use a translatedcommon.ok. - (Confidence 85)
tests/unit/dashboard/user-insights-page.test.tsx:238-279— overview cards test doesn’t assert the rate metric label/value, so wiring bugs slip through.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Applied size/XL label and submitted a PR review on #886 with 4 inline comments:
src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx:72-75— “Cache Hit Rate” card is wired totodayErrorRate(mislabeled metric).src/actions/admin-user-insights.ts:77-80— key-trend action returns raw stats wheredatecan be aDateon cold cache; client code can throw (suggested normalization + stronger typing).src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx:113-115— hardcodedOKbutton label bypasses i18n.tests/unit/dashboard/user-insights-page.test.tsx:238-279— overview cards test doesn’t assert the rate metric label/value, so wiring bugs slip through.
) - Fix critical bug: overview card labeled "Cache Hit Rate" was showing todayErrorRate; renamed to "Error Rate" with correct i18n (5 locales) - Fix security: Cache-Control now private when allowGlobalUsageView=false - Add date validation (YYYY-MM-DD regex + range check) in getUserInsightsModelBreakdown server action - Normalize key trend date field to ISO string to prevent client crash when cache returns Date objects - Replace hardcoded "OK" button with tCommon("ok") i18n key - Replace hardcoded "Calls" chart label with tStats("requests") - Tighten userId validation to positive integers only - Add NULLIF(TRIM()) to model field for consistency with leaderboard.ts - Use machine-readable error code for 403 response - Strengthen return type from unknown to DatabaseKeyStatRow[] - Update tests: assert errorRate metric, date normalization, date validation, and error code assertions
🧪 测试结果
总体结果: ✅ 所有测试通过 |
| modalCacheHitRate: labels.modal.cacheHitRate, | ||
| modalCacheTokens: labels.modal.cacheTokens, | ||
| modalPerformanceHigh: labels.modal.performanceHigh, | ||
| modalPerformanceMedium: labels.modal.performanceMedium, | ||
| modalPerformanceLow: labels.modal.performanceLow, | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| unknownModel: t("unknownModel"), | ||
| modalRequests: t("modal.requests"), | ||
| modalCost: t("modal.cost"), | ||
| modalInputTokens: t("modal.inputTokens"), | ||
| modalOutputTokens: t("modal.outputTokens"), | ||
| modalCacheCreationTokens: t("modal.cacheWrite"), | ||
| modalCacheReadTokens: t("modal.cacheRead"), | ||
| modalTotalTokens: t("modal.totalTokens"), | ||
| modalCacheHitRate: t("modal.cacheHitRate"), | ||
| modalCacheTokens: t("modal.cacheTokens"), | ||
| modalPerformanceHigh: t("modal.performanceHigh"), | ||
| modalPerformanceMedium: t("modal.performanceMedium"), |
There was a problem hiding this comment.
myUsage.stats namespace loaded unconditionally, breaking the "i18n-agnostic" contract
The PR description and ModelBreakdownLabels interface both aim to make this component i18n-agnostic. However, useLabels always calls useTranslations("myUsage.stats") regardless of whether external labels are provided:
function useLabels(labels?: ModelBreakdownLabels) {
const t = useTranslations("myUsage.stats"); // ← always called
if (labels) {
return { ... }; // t is never used here
}
return { ... /* uses t */ };
}This means every page that renders ModelBreakdownRow (including the new admin user insights page) must have myUsage.stats available in its next-intl message tree. If the project uses per-page namespace scoping, calling this component from dashboard/leaderboard/user/[userId] without providing myUsage messages will throw a MissingTranslationError at runtime.
Since labels are always externally provided in the user insights context (user-model-breakdown.tsx passes a full labels object), the fallback translation path is only needed in statistics-summary-card.tsx. Consider making labels required when the component is used outside myUsage, or extract the default labels directly into statistics-summary-card.tsx and always require labels in the shared primitive:
function useLabels(labels: ModelBreakdownLabels) {
// No useTranslations call needed — caller is responsible for labels
return {
unknownModel: labels.unknownModel,
modalRequests: labels.modal.requests,
// ...
};
}Then statistics-summary-card.tsx can construct the labels object using its own useTranslations("myUsage.stats") call before passing them in.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/analytics/model-breakdown-column.tsx
Line: 113-133
Comment:
**`myUsage.stats` namespace loaded unconditionally, breaking the "i18n-agnostic" contract**
The PR description and `ModelBreakdownLabels` interface both aim to make this component i18n-agnostic. However, `useLabels` always calls `useTranslations("myUsage.stats")` regardless of whether external `labels` are provided:
```typescript
function useLabels(labels?: ModelBreakdownLabels) {
const t = useTranslations("myUsage.stats"); // ← always called
if (labels) {
return { ... }; // t is never used here
}
return { ... /* uses t */ };
}
```
This means every page that renders `ModelBreakdownRow` (including the new admin user insights page) must have `myUsage.stats` available in its next-intl message tree. If the project uses per-page namespace scoping, calling this component from `dashboard/leaderboard/user/[userId]` without providing `myUsage` messages will throw a `MissingTranslationError` at runtime.
Since `labels` are always externally provided in the user insights context (`user-model-breakdown.tsx` passes a full `labels` object), the fallback translation path is only needed in `statistics-summary-card.tsx`. Consider making `labels` required when the component is used outside `myUsage`, or extract the default labels directly into `statistics-summary-card.tsx` and always require `labels` in the shared primitive:
```typescript
function useLabels(labels: ModelBreakdownLabels) {
// No useTranslations call needed — caller is responsible for labels
return {
unknownModel: labels.unknownModel,
modalRequests: labels.modal.requests,
// ...
};
}
```
Then `statistics-summary-card.tsx` can construct the labels object using its own `useTranslations("myUsage.stats")` call before passing them in.
How can I resolve this? If you propose a fix, please make it concise.| ); | ||
| if (!result.ok) throw new Error(result.error); | ||
| return result.data; | ||
| }, | ||
| }); | ||
|
|
||
| const handleApplyRange = () => { | ||
| setAppliedRange({ start: startDate || undefined, end: endDate || undefined }); | ||
| }; | ||
|
|
||
| const handleClearRange = () => { | ||
| setStartDate(""); |
There was a problem hiding this comment.
Query error state silently falls through to "No data available"
All three data-fetching client components (user-overview-cards.tsx, user-key-trend-chart.tsx, and user-model-breakdown.tsx) share the same pattern: when useQuery throws (e.g., network error, server action returns ok: false), there is no dedicated error UI. The component falls through to the empty-state branch instead.
For user-model-breakdown.tsx:
const { data, isLoading } = useQuery({ ... });
// isError is never destructuredAnd in the render:
{isLoading ? <Skeleton /> : items.length === 0 ? <div>{t("noData")}</div> : <Table />}When the query errors, data is undefined, items is [], and the component silently shows "No data available" — making it impossible for an admin to distinguish "this user has no model usage" from "the request failed".
Consider destructuring isError and error from useQuery and rendering a distinct error message. The same fix applies to user-key-trend-chart.tsx and user-overview-cards.tsx (which returns null on error, leaving the page completely blank).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx
Line: 42-53
Comment:
**Query error state silently falls through to "No data available"**
All three data-fetching client components (`user-overview-cards.tsx`, `user-key-trend-chart.tsx`, and `user-model-breakdown.tsx`) share the same pattern: when `useQuery` throws (e.g., network error, server action returns `ok: false`), there is no dedicated error UI. The component falls through to the empty-state branch instead.
For `user-model-breakdown.tsx`:
```typescript
const { data, isLoading } = useQuery({ ... });
// isError is never destructured
```
And in the render:
```typescript
{isLoading ? <Skeleton /> : items.length === 0 ? <div>{t("noData")}</div> : <Table />}
```
When the query errors, `data` is `undefined`, `items` is `[]`, and the component silently shows "No data available" — making it impossible for an admin to distinguish "this user has no model usage" from "the request failed".
Consider destructuring `isError` and `error` from `useQuery` and rendering a distinct error message. The same fix applies to `user-key-trend-chart.tsx` and `user-overview-cards.tsx` (which returns `null` on error, leaving the page completely blank).
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| export interface ModelBreakdownLabels { | ||
| unknownModel: string; | ||
| modal: { | ||
| requests: string; | ||
| cost: string; | ||
| inputTokens: string; | ||
| outputTokens: string; | ||
| cacheCreationTokens: string; | ||
| cacheReadTokens: string; | ||
| totalTokens: string; | ||
| costPercentage: string; | ||
| cacheHitRate: string; | ||
| cacheTokens: string; | ||
| performanceHigh: string; | ||
| performanceMedium: string; | ||
| performanceLow: string; |
There was a problem hiding this comment.
ModelBreakdownLabels.modal.costPercentage is defined but never consumed
The ModelBreakdownLabels interface includes a costPercentage field in the modal sub-object:
export interface ModelBreakdownLabels {
modal: {
...
costPercentage: string; // ← defined here
...
};
}However, useLabels never maps this field and ModelBreakdownRow never uses a label for the cost percentage — it's always rendered as a computed string ({costPercentage}%) with no i18n label. This means callers (e.g., user-model-breakdown.tsx) are required to provide a costPercentage string that is immediately discarded.
Consider removing this field from the interface to avoid misleading callers:
export interface ModelBreakdownLabels {
modal: {
requests: string;
cost: string;
inputTokens: string;
outputTokens: string;
cacheCreationTokens: string;
cacheReadTokens: string;
totalTokens: string;
// costPercentage removed — not used
cacheHitRate: string;
cacheTokens: string;
performanceHigh: string;
performanceMedium: string;
performanceLow: string;
};
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/analytics/model-breakdown-column.tsx
Line: 29-45
Comment:
**`ModelBreakdownLabels.modal.costPercentage` is defined but never consumed**
The `ModelBreakdownLabels` interface includes a `costPercentage` field in the `modal` sub-object:
```typescript
export interface ModelBreakdownLabels {
modal: {
...
costPercentage: string; // ← defined here
...
};
}
```
However, `useLabels` never maps this field and `ModelBreakdownRow` never uses a label for the cost percentage — it's always rendered as a computed string `({costPercentage}%)` with no i18n label. This means callers (e.g., `user-model-breakdown.tsx`) are required to provide a `costPercentage` string that is immediately discarded.
Consider removing this field from the interface to avoid misleading callers:
```typescript
export interface ModelBreakdownLabels {
modal: {
requests: string;
cost: string;
inputTokens: string;
outputTokens: string;
cacheCreationTokens: string;
cacheReadTokens: string;
totalTokens: string;
// costPercentage removed — not used
cacheHitRate: string;
cacheTokens: string;
performanceHigh: string;
performanceMedium: string;
performanceLow: string;
};
}
```
How can I resolve this? If you propose a fix, please make it concise.| return { | ||
| ...base, | ||
| ...providerFields, | ||
| ...cacheFields, | ||
| ...(modelStatsFormatted !== undefined ? { modelStats: modelStatsFormatted } : {}), | ||
| ...(userModelStatsFormatted !== undefined ? { modelStats: userModelStatsFormatted } : {}), | ||
| }; |
There was a problem hiding this comment.
Two separate modelStats spreads can be unified
modelStatsFormatted and userModelStatsFormatted are mutually exclusive (scope === "provider" vs scope === "user"), so the two spread blocks can never both be truthy. However, spreading the same key modelStats twice makes the intended behavior less obvious and could silently shadow one value if the scoping invariant is ever relaxed:
...(modelStatsFormatted !== undefined ? { modelStats: modelStatsFormatted } : {}),
...(userModelStatsFormatted !== undefined ? { modelStats: userModelStatsFormatted } : {}),Consider unifying them:
const resolvedModelStats = modelStatsFormatted ?? userModelStatsFormatted;
// ...
...(resolvedModelStats !== undefined ? { modelStats: resolvedModelStats } : {}),This makes the mutual-exclusivity explicit and prevents accidental shadowing.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/leaderboard/route.ts
Line: 263-269
Comment:
**Two separate `modelStats` spreads can be unified**
`modelStatsFormatted` and `userModelStatsFormatted` are mutually exclusive (`scope === "provider"` vs `scope === "user"`), so the two spread blocks can never both be truthy. However, spreading the same key `modelStats` twice makes the intended behavior less obvious and could silently shadow one value if the scoping invariant is ever relaxed:
```typescript
...(modelStatsFormatted !== undefined ? { modelStats: modelStatsFormatted } : {}),
...(userModelStatsFormatted !== undefined ? { modelStats: userModelStatsFormatted } : {}),
```
Consider unifying them:
```typescript
const resolvedModelStats = modelStatsFormatted ?? userModelStatsFormatted;
// ...
...(resolvedModelStats !== undefined ? { modelStats: resolvedModelStats } : {}),
```
This makes the mutual-exclusivity explicit and prevents accidental shadowing.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx (1)
35-46:⚠️ Potential issue | 🟠 Major不要把请求失败伪装成“暂无数据”
这里
queryFn在非ok时会抛错，但渲染层只区分isLoading和items.length === 0。一旦接口返回 403/500，页面会直接落到t("noData")，管理员无法判断是真空数据还是请求失败。请单独处理isError/error分支。Also applies to: 124-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx around lines 35 - 46, The current useQuery implementation (queryKey ["user-insights-model-breakdown", userId, appliedRange.start, appliedRange.end] with queryFn calling getUserInsightsModelBreakdown) throws on non-ok responses but the component only checks isLoading and items.length === 0, which masks HTTP errors as "no data"; update the component rendering logic to check useQuery's isError and error first (or surface errors to an error boundary) and render an explicit error state/message when error exists instead of falling through to the empty-data UI; apply the same fix to the other occurrence noted (lines ~124-145) so failed requests (403/500) are distinguishable from genuinely empty results.src/repository/admin-user-insights.ts (1)
39-45:⚠️ Potential issue | 🟠 Major日期筛选边界仍然没有按系统时区计算
这里直接用
::date截断并构造< endDate + 1 day，比较结果会依赖数据库会话时区。实例不是 UTC 时，insights 页面和src/repository/leaderboard.ts对同一日期范围可能看到不同数据。建议复用 leaderboard 的日期窗口换算逻辑，明确按系统时区构造[start, end)边界。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/admin-user-insights.ts` around lines 39 - 45, The date filtering currently truncates with ::date and +1 day which depends on DB session timezone; replace this with the same timezone-aware date-window logic used in src/repository/leaderboard.ts so the [start, end) boundaries are built in the system timezone. In practice, compute the start and end timestamps using the leaderboard helper (reuse its function or logic) to produce explicit timestamp boundaries and push conditions against usageLedger.createdAt using those concrete timestamps (rather than relying on ::date/interval arithmetic) so admin-user-insights.ts yields the same results as leaderboard.ts.src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx (2)
42-49:⚠️ Potential issue | 🟠 Major不要把查询失败渲染成空数据
这里请求失败后会抛错，但渲染层没有
isError分支，最终会直接显示t("noData")。403、服务端异常和真实“没有数据”会被混在一起，排障体验很差。Also applies to: 135-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx around lines 42 - 49, The query currently throws on failure (useQuery(queryFn: getUserInsightsKeyTrend)) but the component only checks for rawData/isLoading and falls back to t("noData"), which mixes network/403/errors with real empty-state; update the render logic to check useQuery's isError and error (add isError, error to the destructure from useQuery) and render a distinct error UI/log the error when isError is true instead of treating it as "no data"; apply the same fix to the other useQuery instance at the other location (lines 135-140) so real errors are surfaced separately from an empty-data state.
81-84:⚠️ Potential issue | 🟠 Major成本趋势现在还是死代码
chartData已经写入了${dk}_cost，但图表始终只渲染${key.dataKey}_calls，头部也没有切换 requests/cost 的入口。这样上线后管理员只能看请求量，PR 目标里的按 Key 成本趋势实际上不可用。Also applies to: 118-130, 179-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx around lines 81 - 84, chartData currently populates both entry[`${dk}_calls`] and entry[`${dk}_cost`] but the chart rendering always uses key.dataKey (calls) and there's no UI to toggle metrics, so implement a metric toggle and wire the chart to use it: add a selectedMetric state (e.g., "calls" | "cost") in the component, render a header control to switch between "requests" and "cost", update the keys/series generation logic that references dk to choose `${dk}_calls` or `${dk}_cost` based on selectedMetric, and ensure the chart rendering uses that chosen dataKey (the place that currently reads key.dataKey) so the graph and header toggle correctly display cost trends when selected; keep existing creation of entry[`${dk}_cost`] and format/labeling for currency if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/actions/admin-user-insights.test.ts`:
- Around line 235-270: The test for getUserInsightsKeyTrend uses an incomplete
mock (only date/cost/requests) and assertions that only check dates, so it won't
detect missing/changed fields in the real DatabaseKeyStatRow contract; update
the test fixtures to return objects shaped like DatabaseKeyStatRow (include
key_id, key_name, api_calls, total_cost, plus date/cost/requests as appropriate)
in both the "returns trend data" and "normalizes Date objects" cases, and add
assertions that those fields exist and have the expected types/values (e.g.,
check key_id/key_name/api_calls/total_cost alongside the date normalization) so
the test validates the real contract used by getUserInsightsKeyTrend.
---
Duplicate comments:
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx:
- Around line 42-49: The query currently throws on failure (useQuery(queryFn:
getUserInsightsKeyTrend)) but the component only checks for rawData/isLoading
and falls back to t("noData"), which mixes network/403/errors with real
empty-state; update the render logic to check useQuery's isError and error (add
isError, error to the destructure from useQuery) and render a distinct error
UI/log the error when isError is true instead of treating it as "no data"; apply
the same fix to the other useQuery instance at the other location (lines
135-140) so real errors are surfaced separately from an empty-data state.
- Around line 81-84: chartData currently populates both entry[`${dk}_calls`] and
entry[`${dk}_cost`] but the chart rendering always uses key.dataKey (calls) and
there's no UI to toggle metrics, so implement a metric toggle and wire the chart
to use it: add a selectedMetric state (e.g., "calls" | "cost") in the component,
render a header control to switch between "requests" and "cost", update the
keys/series generation logic that references dk to choose `${dk}_calls` or
`${dk}_cost` based on selectedMetric, and ensure the chart rendering uses that
chosen dataKey (the place that currently reads key.dataKey) so the graph and
header toggle correctly display cost trends when selected; keep existing
creation of entry[`${dk}_cost`] and format/labeling for currency if needed.
In
`@src/app/`[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx:
- Around line 35-46: The current useQuery implementation (queryKey
["user-insights-model-breakdown", userId, appliedRange.start, appliedRange.end]
with queryFn calling getUserInsightsModelBreakdown) throws on non-ok responses
but the component only checks isLoading and items.length === 0, which masks HTTP
errors as "no data"; update the component rendering logic to check useQuery's
isError and error first (or surface errors to an error boundary) and render an
explicit error state/message when error exists instead of falling through to the
empty-data UI; apply the same fix to the other occurrence noted (lines ~124-145)
so failed requests (403/500) are distinguishable from genuinely empty results.
In `@src/repository/admin-user-insights.ts`:
- Around line 39-45: The date filtering currently truncates with ::date and +1
day which depends on DB session timezone; replace this with the same
timezone-aware date-window logic used in src/repository/leaderboard.ts so the
[start, end) boundaries are built in the system timezone. In practice, compute
the start and end timestamps using the leaderboard helper (reuse its function or
logic) to produce explicit timestamp boundaries and push conditions against
usageLedger.createdAt using those concrete timestamps (rather than relying on
::date/interval arithmetic) so admin-user-insights.ts yields the same results as
leaderboard.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0abe651-c03c-4811-b674-5e2deb44adc7
📒 Files selected for processing (15)
messages/en/dashboard.jsonmessages/ja/dashboard.jsonmessages/ru/dashboard.jsonmessages/zh-CN/dashboard.jsonmessages/zh-TW/dashboard.jsonsrc/actions/admin-user-insights.tssrc/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsxsrc/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsxsrc/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsxsrc/app/[locale]/dashboard/leaderboard/user/[userId]/page.tsxsrc/app/api/leaderboard/route.tssrc/repository/admin-user-insights.tstests/unit/actions/admin-user-insights.test.tstests/unit/api/leaderboard-route.test.tstests/unit/dashboard/user-insights-page.test.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/unit/dashboard/user-insights-page.test.tsx
- tests/unit/api/leaderboard-route.test.ts
- src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx
- messages/zh-TW/dashboard.json
- src/actions/admin-user-insights.ts
- messages/en/dashboard.json
Mock data now matches the actual DatabaseKeyStatRow interface with key_id, key_name, api_calls, and total_cost fields instead of generic cost/requests. Assertions verify the full data contract.
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/unit/actions/admin-user-insights.test.ts (1)
235-280:⚠️ Potential issue | 🟡 Minor继续把 key trend 的返回契约断言补完整。
现在 fixture 已经补了
key_id/key_name/api_calls，但两个成功用例仍然没有锁住total_cost，而且日期归一化用例也没有确认其它字段被原样保留。只要 action 在映射时漏掉total_cost或改错字段名，这里还是会通过。建议补齐的断言
expect(result.ok).toBe(true); if (result.ok) { expect(result.data).toHaveLength(2); expect(result.data[0].date).toBe("2026-03-09"); expect(result.data[0].key_id).toBe(1); expect(result.data[0].key_name).toBe("sk-key-1"); expect(result.data[0].api_calls).toBe(10); + expect(result.data[0].total_cost).toBe(1.5); expect(result.data[1].date).toBe("2026-03-08"); + expect(result.data[1].total_cost).toBe(2.0); } @@ expect(result.ok).toBe(true); if (result.ok) { expect(typeof result.data[0].date).toBe("string"); expect(result.data[0].date).toContain("2026-03-09"); + expect(result.data[0]).toMatchObject({ + key_id: 1, + key_name: "sk-key-1", + api_calls: 10, + total_cost: 1.5, + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/actions/admin-user-insights.test.ts` around lines 235 - 280, The tests for getUserInsightsKeyTrend are missing assertions that total_cost is preserved and that other fields remain unchanged when date objects are normalized; update the "returns trend data for valid request" spec to assert result.data[n].total_cost equals the fixture values (e.g., 1.5 and 2.0) and keep existing key_id/key_name/api_calls checks, and update the "normalizes Date objects to ISO strings" spec to also assert that key_id, key_name, api_calls and total_cost are present and equal to the fixture values (and that date is a string containing "2026-03-09"), ensuring the action didn't drop or rename fields during mapping in getUserInsightsKeyTrend.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/actions/admin-user-insights.test.ts`:
- Around line 113-134: The test fixture function createMockBreakdown must
include a record with model: null to cover the empty-model grouping branch;
update createMockBreakdown to add an entry where model is null (with
requests/cost/token fields populated) and then add assertions in the test to
verify the action preserves/returns the null-model group (e.g., checks for an
entry with model === null and expected aggregated values), ensuring the fixture
shape matches the return type used in admin-user-insights (so types align and
the null branch is exercised).
---
Duplicate comments:
In `@tests/unit/actions/admin-user-insights.test.ts`:
- Around line 235-280: The tests for getUserInsightsKeyTrend are missing
assertions that total_cost is preserved and that other fields remain unchanged
when date objects are normalized; update the "returns trend data for valid
request" spec to assert result.data[n].total_cost equals the fixture values
(e.g., 1.5 and 2.0) and keep existing key_id/key_name/api_calls checks, and
update the "normalizes Date objects to ISO strings" spec to also assert that
key_id, key_name, api_calls and total_cost are present and equal to the fixture
values (and that date is a string containing "2026-03-09"), ensuring the action
didn't drop or rename fields during mapping in getUserInsightsKeyTrend.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01411ad8-49e4-4e90-9660-db0991065792
📒 Files selected for processing (1)
tests/unit/actions/admin-user-insights.test.ts
| function createMockBreakdown() { | ||
| return [ | ||
| { | ||
| model: "claude-sonnet-4-20250514", | ||
| requests: 30, | ||
| cost: 3.5, | ||
| inputTokens: 10000, | ||
| outputTokens: 5000, | ||
| cacheCreationTokens: 2000, | ||
| cacheReadTokens: 8000, | ||
| }, | ||
| { | ||
| model: "claude-opus-4-20250514", | ||
| requests: 20, | ||
| cost: 2.0, | ||
| inputTokens: 8000, | ||
| outputTokens: 3000, | ||
| cacheCreationTokens: 1000, | ||
| cacheReadTokens: 5000, | ||
| }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
把 model: null 这条关键分支也锁进成功用例里。
这组 fixture 现在只覆盖了有模型名的数据，但本次需求明确要求保留空模型分组；而 src/repository/admin-user-insights.ts:9-17 里的返回类型也允许 model: null。如果 action 后续把这类记录过滤掉，这里不会报警。
可直接补上的断言示例
function createMockBreakdown() {
return [
{
model: "claude-sonnet-4-20250514",
requests: 30,
cost: 3.5,
inputTokens: 10000,
outputTokens: 5000,
cacheCreationTokens: 2000,
cacheReadTokens: 8000,
},
{
model: "claude-opus-4-20250514",
requests: 20,
cost: 2.0,
inputTokens: 8000,
outputTokens: 3000,
cacheCreationTokens: 1000,
cacheReadTokens: 5000,
},
+ {
+ model: null,
+ requests: 1,
+ cost: 0.1,
+ inputTokens: 100,
+ outputTokens: 50,
+ cacheCreationTokens: 0,
+ cacheReadTokens: 0,
+ },
];
}
@@
expect(result.ok).toBe(true);
if (result.ok) {
expect(result.data.breakdown).toEqual(breakdown);
+ expect(result.data.breakdown.some((item) => item.model === null)).toBe(true);
expect(result.data.currencyCode).toBe("USD");
}Based on learnings: Applies to **/*.test.{ts,tsx,js,jsx} : All new features must have unit test coverage of at least 80%.
Also applies to: 317-333
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/actions/admin-user-insights.test.ts` around lines 113 - 134, The
test fixture function createMockBreakdown must include a record with model: null
to cover the empty-model grouping branch; update createMockBreakdown to add an
entry where model is null (with requests/cost/token fields populated) and then
add assertions in the test to verify the action preserves/returns the null-model
group (e.g., checks for an entry with model === null and expected aggregated
values), ensuring the fixture shape matches the return type used in
admin-user-insights (so types align and the null branch is exercised).
| @@ -0,0 +1,63 @@ | |||
| "use server"; | |||
There was a problem hiding this comment.
"use server" directive in a repository file exposes unauthenticated DB function
"use server" at the module level marks all exported functions in this file — including getUserModelBreakdown — as callable server actions from client code. Because getUserModelBreakdown contains no authorization check, any client that holds a reference to this function can invoke it to query arbitrary users' model usage data, bypassing the admin-only gate that lives in src/actions/admin-user-insights.ts.
Repository files should never carry "use server". They are plain server-side modules meant to be called from other server components or actions. Remove the directive:
| "use server"; | |
| import { and, desc, eq, gte, lt, sql } from "drizzle-orm"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/repository/admin-user-insights.ts
Line: 1
Comment:
**`"use server"` directive in a repository file exposes unauthenticated DB function**
`"use server"` at the module level marks all exported functions in this file — including `getUserModelBreakdown` — as callable server actions from client code. Because `getUserModelBreakdown` contains no authorization check, any client that holds a reference to this function can invoke it to query arbitrary users' model usage data, bypassing the admin-only gate that lives in `src/actions/admin-user-insights.ts`.
Repository files should never carry `"use server"`. They are plain server-side modules meant to be called from other server components or actions. Remove the directive:
```suggestion
import { and, desc, eq, gte, lt, sql } from "drizzle-orm";
```
How can I resolve this? If you propose a fix, please make it concise.| const modelRows = await db | ||
| .select({ | ||
| userId: usageLedger.userId, | ||
| model: modelField, | ||
| totalRequests: sql<number>`count(*)::double precision`, | ||
| totalCost: sql<string>`COALESCE(sum(${usageLedger.costUsd}), 0)`, | ||
| totalTokens: sql<number>`COALESCE( | ||
| sum( | ||
| ${usageLedger.inputTokens} + | ||
| ${usageLedger.outputTokens} + | ||
| COALESCE(${usageLedger.cacheCreationInputTokens}, 0) + | ||
| COALESCE(${usageLedger.cacheReadInputTokens}, 0) | ||
| )::double precision, | ||
| 0::double precision | ||
| )`, | ||
| }) | ||
| .from(usageLedger) | ||
| .innerJoin(users, and(sql`${usageLedger.userId} = ${users.id}`, isNull(users.deletedAt))) | ||
| .where(and(...whereConditions)) | ||
| .groupBy(usageLedger.userId, modelField) | ||
| .orderBy(desc(sql`sum(${usageLedger.costUsd})`)); |
There was a problem hiding this comment.
Model-stats second query is not scoped to leaderboard users
When includeModelStats is true, the second query groups the entire usageLedger table by (userId, model) using the same date-range and tag/group conditions as the main query — with no constraint limiting it to the user IDs already present in baseEntries. All matching rows across all users are fetched and stored in modelStatsByUser, but only the subset corresponding to baseEntries is ever returned (via the final .map).
If the leaderboard ever gains a LIMIT / top-N boundary, or if tag/group filters are absent and many users match the date range, this query will do substantially more work than necessary — scanning and aggregating data for users whose stats are immediately discarded.
A simple fix is to filter the second query to only the users in baseEntries:
const userIds = baseEntries.map((e) => e.userId);
// Add to whereConditions before the modelRows query:
// import { inArray } from "drizzle-orm"
const modelRows = await db
.select({ ... })
.from(usageLedger)
.innerJoin(users, ...)
.where(and(...whereConditions, inArray(usageLedger.userId, userIds)))
...This guarantees the second query only materialises rows that will actually be used.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/repository/leaderboard.ts
Line: 330-350
Comment:
**Model-stats second query is not scoped to leaderboard users**
When `includeModelStats` is `true`, the second query groups the entire `usageLedger` table by `(userId, model)` using the same date-range and tag/group conditions as the main query — with no constraint limiting it to the user IDs already present in `baseEntries`. All matching rows across all users are fetched and stored in `modelStatsByUser`, but only the subset corresponding to `baseEntries` is ever returned (via the final `.map`).
If the leaderboard ever gains a `LIMIT` / top-N boundary, or if tag/group filters are absent and many users match the date range, this query will do substantially more work than necessary — scanning and aggregating data for users whose stats are immediately discarded.
A simple fix is to filter the second query to only the users in `baseEntries`:
```typescript
const userIds = baseEntries.map((e) => e.userId);
// Add to whereConditions before the modelRows query:
// import { inArray } from "drizzle-orm"
const modelRows = await db
.select({ ... })
.from(usageLedger)
.innerJoin(users, ...)
.where(and(...whereConditions, inArray(usageLedger.userId, userIds)))
...
```
This guarantees the second query only materialises rows that will actually be used.
How can I resolve this? If you propose a fix, please make it concise.| */ | ||
| export async function getUserInsightsKeyTrend( | ||
| targetUserId: number, | ||
| timeRange: string | ||
| ): Promise<ActionResult<DatabaseKeyStatRow[]>> { | ||
| const session = await getSession(); | ||
| if (!session || session.user.role !== "admin") { | ||
| return { ok: false, error: "Unauthorized" }; | ||
| } | ||
|
|
||
| if (!isValidTimeRange(timeRange)) { | ||
| return { | ||
| ok: false, | ||
| error: `Invalid timeRange: must be one of ${VALID_TIME_RANGES.join(", ")}`, | ||
| }; | ||
| } | ||
|
|
||
| const statistics = await getStatisticsWithCache(timeRange, "keys", targetUserId); |
There was a problem hiding this comment.
Inconsistent user-existence validation across server actions
getUserInsightsOverview (line 38) calls findUserById and returns { ok: false, error: "User not found" } if the user does not exist. getUserInsightsKeyTrend and getUserInsightsModelBreakdown skip this check entirely and return empty/default data for a non-existent targetUserId.
While the server component in page.tsx redirects when the user is not found at page-load time, the three client components also call these actions independently on every query-key change. If a user is deleted after the page is first loaded:
- The overview card will show an error state (query throws).
- The key-trend chart and model-breakdown table will silently show "No data available".
This makes it impossible for an admin to distinguish "user deleted mid-session" from "user has no usage data". Adding the same findUserById guard to getUserInsightsKeyTrend and getUserInsightsModelBreakdown would keep the three actions consistent.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/admin-user-insights.ts
Line: 63-80
Comment:
**Inconsistent user-existence validation across server actions**
`getUserInsightsOverview` (line 38) calls `findUserById` and returns `{ ok: false, error: "User not found" }` if the user does not exist. `getUserInsightsKeyTrend` and `getUserInsightsModelBreakdown` skip this check entirely and return empty/default data for a non-existent `targetUserId`.
While the server component in `page.tsx` redirects when the user is not found at page-load time, the three client components also call these actions independently on every query-key change. If a user is deleted after the page is first loaded:
- The overview card will show an error state (query throws).
- The key-trend chart and model-breakdown table will silently show "No data available".
This makes it impossible for an admin to distinguish "user deleted mid-session" from "user has no usage data". Adding the same `findUserById` guard to `getUserInsightsKeyTrend` and `getUserInsightsModelBreakdown` would keep the three actions consistent.
How can I resolve this? If you propose a fix, please make it concise.| if (startDate && !DATE_REGEX.test(startDate)) { | ||
| return { ok: false, error: "Invalid startDate format: use YYYY-MM-DD" }; | ||
| } | ||
| if (endDate && !DATE_REGEX.test(endDate)) { | ||
| return { ok: false, error: "Invalid endDate format: use YYYY-MM-DD" }; | ||
| } |
There was a problem hiding this comment.
getSystemSettings() called twice in the same request
getUserModelBreakdown (in src/repository/admin-user-insights.ts) already calls getSystemSettings() internally to read billingModelSource. The Promise.all here calls it a second time to read currencyDisplay. Both values are available in a single call.
Consider passing the required settings into getUserModelBreakdown as a parameter, or reading getSystemSettings() once at the action layer and forwarding what each function needs. This avoids the double call (and any cache-miss cost) on every breakdown request.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/admin-user-insights.ts
Line: 108-113
Comment:
**`getSystemSettings()` called twice in the same request**
`getUserModelBreakdown` (in `src/repository/admin-user-insights.ts`) already calls `getSystemSettings()` internally to read `billingModelSource`. The `Promise.all` here calls it a second time to read `currencyDisplay`. Both values are available in a single call.
Consider passing the required settings into `getUserModelBreakdown` as a parameter, or reading `getSystemSettings()` once at the action layer and forwarding what each function needs. This avoids the double call (and any cache-miss cost) on every breakdown request.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Summary
Add user leaderboard model drill-down and dedicated user insights page, enabling admins to view per-model usage breakdowns and statistics for individual users.
Problem
The user leaderboard only displayed aggregate data (total requests, tokens, cost). Admins lacked visibility into:
Related Issues:
Follows Pattern From:
Solution
Part 1: Expandable User Rows
Part 2: User Insights Page
/dashboard/leaderboard/user/[userId]Part 3: Shared Component Extraction
ModelBreakdownColumn/Rowfromstatistics-summary-card.tsxinto shared primitiveChanges
Core Changes (Data Layer)
src/repository/leaderboard.ts- AddedUserModelStattype,includeModelStatsplumbing for user scopesrc/repository/admin-user-insights.ts- New:getUserModelBreakdown()with billingModelSource-aware groupingsrc/actions/admin-user-insights.ts- New: 3 admin-only server actions (overview, key trend, model breakdown)src/lib/redis/leaderboard-cache.ts- Cache key isolation for user scopeincludeModelStatssrc/app/api/leaderboard/route.ts- Admin gate forincludeUserModelStats, user modelStats formattingUI Changes
src/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsx- Admin-only expandable user rows, user name Link to insights pagesrc/app/[locale]/dashboard/leaderboard/user/[userId]/- New: server component + 4 client componentssrc/components/analytics/model-breakdown-column.tsx- New: shared i18n-agnostic ModelBreakdownColumn/Row primitivesrc/app/[locale]/my-usage/_components/statistics-summary-card.tsx- Refactored to use shared primitiveSupporting Changes
leaderboard.userInsights.*keys to all 5 languages (en, zh-CN, zh-TW, ja, ru)Testing
Automated Tests
tests/unit/repository/leaderboard-user-model-stats.test.ts(5 tests)tests/unit/api/leaderboard-route.test.ts(11 tests)tests/unit/actions/admin-user-insights.test.ts(11 tests)tests/unit/components/model-breakdown-column.test.tsx(10 tests)tests/unit/dashboard/user-insights-page.test.tsx(6 tests)tests/unit/repository/leaderboard-provider-metrics.test.ts(11 tests - no regressions)Verification
bun run typecheck- Passes (only pre-existing langfuse errors)bun run lint- Passes (only pre-existing warnings)bun run test- 373/374 test files pass (1 pre-existing langfuse failure)Description enhanced by Claude AI
Greptile Summary
This PR adds a user model drill-down to the leaderboard and a new admin-only User Insights page (
/dashboard/leaderboard/user/[userId]), providing per-model usage breakdowns, a key usage trend chart, and overview metric cards. It also extracts a sharedModelBreakdownColumn/Rowcomponent and adds 54 new tests. The overall architecture follows existing patterns from the provider drill-down feature and integrates cleanly with thebillingModelSource-aware model resolution.Key issues found:
"use server"in a repository file (src/repository/admin-user-insights.ts): This marks the exportedgetUserModelBreakdownfunction as a server action, potentially allowing unauthenticated client calls that bypass the admin gate in the actions layer. The directive should be removed.src/repository/leaderboard.ts): WhenincludeModelStatsis enabled, a second DB query fetches(userId, model)aggregates for all matching users rather than only those inbaseEntries. If a leaderboard limit is ever introduced, this will silently over-fetch and discard data.getUserInsightsOverviewreturns a proper "User not found" error, butgetUserInsightsKeyTrendandgetUserInsightsModelBreakdownsilently return empty data for non-existent users.getSystemSettings()called twice ingetUserInsightsModelBreakdown— once insidegetUserModelBreakdownand once in the outerPromise.all.useTranslationscalled unconditionally,costPercentagelabel unused, missing error state for failed queries, duplicatemodelStatsspreads in the route).Confidence Score: 2/5
"use server"directive is removed from the repository file, as it risks exposing an unauthenticated database query as a callable server action."use server"directive onsrc/repository/admin-user-insights.tsis a security concern that needs to be addressed before merge. Several issues from prior review rounds also remain open.src/repository/admin-user-insights.ts(remove"use server"),src/repository/leaderboard.ts(model stats query scoping), and the client components with unhandled error states noted in previous threads.Important Files Changed
"use server"directive, which exposes the unauthenticatedgetUserModelBreakdownfunction as a callable server action from client code.includeModelStatsplumbing for user scope; the second SQL query for model stats is not scoped to only the users inbaseEntries, which is a scalability concern for large deployments.getSystemSettings()call.includeUserModelStatsparameter with proper admin gate andprivate, no-storecache header for sensitive responses; two separatemodelStatsspreads (noted in previous threads) could shadow each other.Number.isIntegeruserId validation, and user existence check before rendering.${dk}_costdata that is never consumed by the Area elements (noted in previous threads). No error state handling for failed queries.useTranslations("myUsage.stats")is always called unconditionally even when external labels are provided (noted in previous threads), andcostPercentagelabel field is defined but never consumed.includeModelStatsand threads the flag through all leaderboard query paths.isAdminto theuseEffectdependency array.Sequence Diagram
sequenceDiagram participant Admin as Admin Browser participant Page as page.tsx (Server) participant OC as UserOverviewCards participant KC as UserKeyTrendChart participant MC as UserModelBreakdown participant SA as actions/admin-user-insights.ts participant Repo as repository/ participant DB as PostgreSQL participant Cache as Redis Cache Admin->>Page: GET /dashboard/leaderboard/user/[userId] Page->>Page: Auth check (admin only) Page->>Repo: findUserById(userId) Repo->>DB: SELECT user DB-->>Page: user or null (redirect if null) Page-->>Admin: Render UserInsightsView Admin->>OC: mount OC->>SA: getUserInsightsOverview(userId) SA->>SA: Auth check SA->>Cache: getOverviewWithCache(userId) Cache->>DB: SELECT overview metrics DB-->>OC: { user, overview, currencyCode } Admin->>KC: mount KC->>SA: getUserInsightsKeyTrend(userId, "7days") SA->>SA: Auth + timeRange validation SA->>Cache: getStatisticsWithCache("7days","keys",userId) Cache->>DB: SELECT key stats DB-->>KC: DatabaseKeyStatRow[] Admin->>MC: mount MC->>SA: getUserInsightsModelBreakdown(userId) SA->>SA: Auth + date validation SA->>Repo: getUserModelBreakdown(userId) Repo->>DB: SELECT model breakdown (billingModelSource-aware) DB-->>MC: { breakdown[], currencyCode } note over Admin,DB: Leaderboard drill-down (API route) Admin->>SA: GET /api/leaderboard?scope=user&includeUserModelStats=1 SA->>SA: Admin gate check SA->>Cache: getLeaderboardWithCache(...) Cache->>DB: 1. Rankings query<br/>2. Model stats query (all matching users) DB-->>Admin: LeaderboardEntry[] with modelStats[]Last reviewed commit: 6af4ae8