feat: 修复排行榜指标并增加请求缓存详情 - #1383
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthrough本次变更新增请求级缓存指标，支持 Prefix ID 与物理 Session ID，更新日志详情和导出展示，并调整模型排行榜成功率及缓存系数逻辑。 Changes缓存与会话可观测性
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1fb5905b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx (1)
400-511: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win将 CachePerformance 展示到所有 Logic Trace 请求流程。
LogicTraceTab只在summary中有条件地渲染CachePerformance；Discovery 模式直接返回DiscoveryTraceView，普通决策链、Affinity Hit、replay/重试/hedge 也未渲染。如果需求是统一展示缓存性能指标，需将CachePerformance拆到 Session Reuse 流程之外，或为 Discovery/普通流程添加独立展示区域。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx around lines 400 - 511, 将 CachePerformance 从 isSessionReuseFlow 的 StepCard 内容中提取为所有 Logic Trace 流程共享的展示区域，确保 DiscoveryTraceView、普通决策链、Affinity Hit 以及 replay/重试/hedge 流程也能显示缓存性能指标；复用现有缓存指标属性和紧凑布局，避免仅在 session reuse 分支渲染。
🧹 Nitpick comments (2)
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx (1)
91-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winbuildLogsFilterHref 与 SummaryTab.tsx 中的实现完全重复。
此函数体与
SummaryTab.tsx第 110-114 行新增的buildLogsFilterHref完全相同。两处独立维护同一段逻辑，后续修改容易只改一处而遗漏另一处。
提取为共享工具函数，放在该目录下的公共文件中，两个标签页共同引用。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx around lines 91 - 105, 提取 LogicTraceTab 中的 buildLogsFilterHref 为该目录下的共享工具函数，并移除两处重复实现；更新 LogicTraceTab 和 SummaryTab 共同引用该函数，保持现有 sessionId 查询参数及返回路径行为不变。src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx (1)
108-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winbuildLogsFilterHref 与 LogicTraceTab.tsx 中的实现完全重复。
此函数体与
LogicTraceTab.tsx第 101-105 行新增的buildLogsFilterHref完全相同。两处独立维护同一段逻辑，后续修改容易只改一处而遗漏另一处。
提取为共享工具函数，放在该目录下的公共文件中，两个标签页共同引用。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx around lines 108 - 128, Extract the duplicated buildLogsFilterHref logic from SummaryTab and LogicTraceTab into a shared utility in their common directory, then import and use that utility in both tabs. Preserve the existing sessionId query construction and generated dashboard logs URL exactly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/instrumentation.ts`:
- Around line 294-314: Update describeSchedulerError so the errorCause value
truncates Error causes’ message to 500 characters, matching the existing
non-Error cause handling; preserve the current undefined behavior when no cause
is present.
In `@src/repository/leaderboard.ts`:
- Around line 787-807: Align the cache-coefficient lookup in the leaderboard
model rows with the model field used by getProviderModelCacheCoefficients for
redirected data. Update modelCacheKey and the corresponding lookup in the second
affected block (around the additional cacheCoefficientBp assignment) to use the
same original-model source and normalization as the coefficient aggregation,
preventing valid coefficients from becoming null.
---
Outside diff comments:
In
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx:
- Around line 400-511: 将 CachePerformance 从 isSessionReuseFlow 的 StepCard
内容中提取为所有 Logic Trace 流程共享的展示区域，确保 DiscoveryTraceView、普通决策链、Affinity Hit 以及
replay/重试/hedge 流程也能显示缓存性能指标；复用现有缓存指标属性和紧凑布局，避免仅在 session reuse 分支渲染。
---
Nitpick comments:
In
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx:
- Around line 91-105: 提取 LogicTraceTab 中的 buildLogsFilterHref
为该目录下的共享工具函数，并移除两处重复实现；更新 LogicTraceTab 和 SummaryTab 共同引用该函数，保持现有 sessionId
查询参数及返回路径行为不变。
In
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx:
- Around line 108-128: Extract the duplicated buildLogsFilterHref logic from
SummaryTab and LogicTraceTab into a shared utility in their common directory,
then import and use that utility in both tabs. Preserve the existing sessionId
query construction and generated dashboard logs URL exactly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ee58996-7802-4e89-b1d8-68213de5238f
📒 Files selected for processing (37)
messages/en/dashboard.jsonmessages/ja/dashboard.jsonmessages/ru/dashboard.jsonmessages/zh-CN/dashboard.jsonmessages/zh-TW/dashboard.jsonsrc/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsxsrc/app/[locale]/dashboard/leaderboard/_components/success-rate-display.tssrc/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/CachePerformance.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/CachePerformance.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/index.tssrc/app/[locale]/dashboard/logs/_components/error-details-dialog/index.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/types.tssrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/instrumentation.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/schemas/usage-logs.tssrc/lib/cache-effectiveness/request-metrics.tssrc/lib/cache-effectiveness/service.tssrc/lib/usage-logs/export/columns.tssrc/repository/leaderboard.tssrc/repository/provider-cache-effectiveness.tssrc/repository/usage-logs.tstests/unit/api/leaderboard-route.test.tstests/unit/dashboard/leaderboard-success-rate-display.test.tstests/unit/lib/cache-effectiveness/request-metrics.test.tstests/unit/lib/cache-effectiveness/service.test.tstests/unit/repository/leaderboard-cache-coefficient.test.tstests/unit/repository/leaderboard-provider-metrics.test.tstests/unit/repository/usage-logs-sessionid-filter.test.tstests/unit/usage-logs/export-csv.test.tstests/unit/usage-logs/export-xlsx.test.ts
![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 XL PR adds substantial leaderboard, usage-log, export, and i18n surface area. I found two high-confidence logic issues in the new code paths: one in the new model-level cache-coefficient aggregation, and one in the new ledger-backed session-identity mapping.
PR Size: XL
- Lines changed: 1494
- Files changed: 37
- Split suggestion: Separate the leaderboard/cache-effectiveness backend work from the logs/export/session-identity UI contract changes.
- Split suggestion: Keep the translation catalog updates in a follow-up PR so behavioral review is not mixed with 100+ locale-string edits.
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 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- None.
High Priority Issues (Should Fix)
src/repository/provider-cache-effectiveness.ts:204— the new model-level cache-coefficient query slices scheduler rollups bywindow_end, which can pull traffic from outside the requested daily/custom leaderboard window.src/repository/usage-logs.ts:661— ledger-backed rows hard-codesessionIdentityKindtonull, so the new Prefix ID / Session ID split breaks for historical or ledger-only data.
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.
- Reviewed PR
#1383, applied thesize/XLlabel, posted 2 inline comments, and submitted the summary review. - Reported
src/repository/provider-cache-effectiveness.ts:204for model-level cache coefficients leaking scheduler windows across daily/custom leaderboard boundaries. - Reported
src/repository/usage-logs.ts:661for hard-codingsessionIdentityKindtonullon ledger-backed rows, which breaks the new Prefix ID / Session ID split for historical/ledger-only data. - Summary review is now on the PR with the required size stats, severity table, split suggestions, and coverage checklist.
|
Review follow-up for 3ae38cf:
Validation: 847 test files / 8243 tests passed, plus lint, lint:fix, typecheck, OpenAPI check/lint, production build, and git diff --check. |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ae38cf0c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const modelCacheCoefficientsPromise = | ||
| billingModelSource === "redirected" | ||
| ? getProviderModelCacheCoefficients(resolveLeaderboardWindow(period, timezone, dateRange)) | ||
| : Promise.resolve(new Map()); |
There was a problem hiding this comment.
When the usage-ledger model query rejects or remains pending while this already-started coefficient query rejects, execution has not reached the later await, so the coefficient rejection can be reported as unhandled. registerCrashDiagnostics exits the process on unhandled rejections, meaning a database failure while loading this leaderboard can terminate the entire replica rather than only fail the request. Attach handlers immediately, such as by awaiting both operations through Promise.all; the same pattern is repeated in the cache-hit branch at lines 889-925.
Useful? React with 👍 / 👎.
| <Link | ||
| href={buildLogsFilterHref(identity.value)} | ||
| className="text-xs font-mono break-all underline-offset-2 hover:underline" |
There was a problem hiding this comment.
For a client-supplied physical session ID beginning with pfx: or sid:, buildPublicSessionIdentity replaces the canonical identity with a sid:<hash> value, so this list contains both the hash and the raw physical ID. The new link filters using that raw value, but buildUsageLogConditions intentionally treats reserved values as canonical-only and omits the physical session_id comparison; clicking the displayed physical ID therefore returns no matching logs. Link reserved source rows through their canonical identity or add an explicitly disambiguated physical-session filter.
Useful? React with 👍 / 👎.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
1 participant
摘要
Date直接交给 Drizzle/postgres.js 编码导致聚合持续失败的问题，恢复provider_cache_effectiveness数据产出。billing_model_source=redirected下展示最终执行模型的成功率；仅在没有可计数 outcome 时返回不可用。sessionId筛选同时支持 canonical identity 和 physical Session ID 的精确过滤。关键实现
::timestamptz，避免 postgres.js 的Date参数编码错误。providerId + normalized redirected model聚合；original 模型口径保持null。cause的 name/code/message，避免再次丢失真实底层错误。验证
bun run lintbun run lint:fixbun run typecheckbun run test：847 个 test files 通过，2 个跳过；8233 个 tests 通过，13 个跳过bun run buildgit diff --checkProduction build 仅保留仓库既有的 Edge Runtime Node API warnings。
部署说明
部署后仍需执行一次有界历史回填，并连续观察至少两个 5 分钟调度周期，确认聚合表持续增长、排行榜 provider/model 系数恢复且不再出现 cache effectiveness tick failure。
Greptile Summary
The PR restores cache-effectiveness aggregation and expands request-level cache, model-success, and session-identity reporting across repository, API, dashboard, and export paths.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the previously reported ledger fallback identity loss is corrected across all relevant query and mapping paths, and ledger population and backfill preserve the required identity kind.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR MR[message_request identity and cache fields] --> Trigger[Ledger trigger/backfill] Trigger --> UL[usage_ledger] MR --> Repo[Usage log repository] UL --> Repo Repo --> API[Usage-log API] Repo --> Export[CSV/XLSX export] API --> UI[Summary, Logic Trace, and tables]Reviews (2): Last reviewed commit: "fix: address cache metric review finding..." | Re-trigger Greptile
Context used (3)