feat(leaderboard): 缓存系数列增加 tooltip 与分层上色 - #1389
Conversation
…nt column - Add headerTooltip support to leaderboard table headers with a help icon - Explain cache coefficient: higher value means fewer provider account switches and less noticeable cache degradation - Tiered coloring aligned with cache hit rate: >=0.9 excellent (green), >=0.8 good (yellow), below 0.8 orange - Add i18n copy for all 5 locales and unit tests
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthrough本次变更为排行榜增加缓存系数 Tooltip。缓存系数统一转换为两位小数,并按 0.9、0.8 阈值显示颜色。五种语言新增本地化文案。测试覆盖 Tooltip、排序交互和颜色分级。错误详情中的身份链接改为单行截断。 Changes缓存系数排行榜展示
身份链接布局
Estimated code review effort: 2 (Simple) | ~10 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54245ea0b4
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx (1)
127-145: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win补充 Tooltip 行为和边界值测试。
当前测试在整个容器中查找
[data-slot="tooltip-trigger"]。其他 Tooltip 也可能使测试通过。请将查询范围限定到缓存系数表头。PR 还要求点击帮助图标时不触发表头排序。请模拟点击并断言排序状态未改变。
颜色测试请增加
0.90、0.80和null输入。当前0.95、0.86、0.50未覆盖包含边界和缺失值显示。Also applies to: 147-172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx` around lines 127 - 145, Extend the leaderboard view tests around the cache coefficient column header: scope the tooltip-trigger query to that specific header instead of the whole container, then click the help icon and assert the header sorting state remains unchanged. Expand the color-mapping coverage to include 0.90, 0.80, and null inputs alongside the existing values, verifying the expected boundary and missing-value displays.
🤖 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/app/`[locale]/dashboard/leaderboard/_components/leaderboard-table.tsx:
- Around line 252-266: Update the TooltipTrigger child in the leaderboard header
tooltip block to use a focusable button instead of the span, preserving the
existing onClick propagation stop and styling. Add an accessible label to the
button so keyboard users can identify and open the headerTooltip via Tab.
---
Nitpick comments:
In `@tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx`:
- Around line 127-145: Extend the leaderboard view tests around the cache
coefficient column header: scope the tooltip-trigger query to that specific
header instead of the whole container, then click the help icon and assert the
header sorting state remains unchanged. Expand the color-mapping coverage to
include 0.90, 0.80, and null inputs alongside the existing values, verifying the
expected boundary and missing-value displays.
🪄 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: 8460957e-9adf-4448-905f-20133e005044
📒 Files selected for processing (8)
messages/en/dashboard.jsonmessages/ja/dashboard.jsonmessages/ru/dashboard.jsonmessages/zh-CN/dashboard.jsonmessages/zh-TW/dashboard.jsonsrc/app/[locale]/dashboard/leaderboard/_components/leaderboard-table.tsxsrc/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsxtests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Code Review Summary
I found three user-visible issues in the new cache-coefficient tooltip and tiering changes that should be fixed before merge. The most important are the inaccessible tooltip trigger and the new tier colors disagreeing with the rounded values shown in the UI.
PR Size: S
- Lines changed: 100
- Files changed: 8
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 | 1 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
- [High] [LOGIC-BUG]
src/app/[locale]/dashboard/leaderboard/_components/leaderboard-table.tsx:255— the new tooltip trigger is a plainspan, so keyboard users cannot focus it or open the help text. - [High] [LOGIC-BUG]
src/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsx:116— color tiers are computed from the unrounded value while the UI displaystoFixed(2), so values like8999render as0.90but stay yellow. - [Medium] [COMMENT-INACCURATE]
messages/en/dashboard.json:707— the tooltip text claims the metric means fewer provider account switches, but the underlying formula is a confidence-adjusted cache-effectiveness score and does not measure switches directly.
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.
- Review completed for PR
#1389andsize/Swas applied. - I posted 3 inline comments on diff lines:
src/app/[locale]/dashboard/leaderboard/_components/leaderboard-table.tsx:255— tooltip trigger is not keyboard-focusable.src/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsx:116— tier color can contradict the rounded value shown.messages/en/dashboard.json:707— tooltip copy overstates what the metric measures.
- I also submitted the required summary review with the issue counts and coverage checklist.
If you want, I can now switch from review mode to fix mode and patch those three issues directly.
…n id - Make header tooltip trigger a focusable button with aria-label so keyboard users can open the field explanation (a11y review) - Strengthen cache coefficient tests: scope tooltip query to the column header, assert clicking the help icon does not toggle sorting, cover 0.90/0.80 boundary tiers and null display - Show Canonical Session ID on a single truncated line in the session info drawer; full value stays in the DOM for select-copy and the link navigation is unchanged
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx (2)
143-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win补充 Tooltip 触发器的无障碍断言。
当前断言只检查
[data-slot="tooltip-trigger"]存在。它不能检测触发器退化为不可聚焦元素,或aria-label被删除。请断言触发器为<button>,并断言aria-label非空。依据 PR 目标:跟进提交要求触发器为可聚焦按钮并带有
aria-label。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx` around lines 143 - 149, 增强 leaderboard 缓存系数测试中 coefficientHeader 的 Tooltip 触发器断言:在现有 data-slot 定位后,断言触发器元素为 button,并断言其 aria-label 属性存在且非空,保留现有触发器存在性检查。
176-183: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win验证 Tooltip 内容和本地化文案。
当前点击测试只检查
tbody的行顺序。Tooltip 内容缺失、阈值错误或翻译键错误时,测试仍会通过。触发 Tooltip 后,请断言role="tooltip"出现,并使用当前 locale 的期望消息断言说明文本和两个阈值。依据 PR 目标:Tooltip 需要提供本地化说明和阈值。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx` around lines 176 - 183, Update the click test around the existing MouseEvent dispatch and leaderboard ordering assertion to verify the tooltip contract as well: assert that a role="tooltip" element appears, then check its localized explanatory message and both expected threshold values for the current locale. Preserve the existing default row-order assertion.
🤖 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 `@tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx`:
- Around line 151-164: 在 “does not trigger column sorting when the help icon is
clicked” 相关测试中补充 scope=userCacheHitRate 的 LeaderboardView 覆盖,验证用户排行榜默认路径下缓存系数列的
Tooltip、排序隔离和分级显示行为。复用现有 providerCacheHitRate 测试的断言结构与测试数据模式,确保两种 scope 均得到验证。
---
Nitpick comments:
In `@tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx`:
- Around line 143-149: 增强 leaderboard 缓存系数测试中 coefficientHeader 的 Tooltip
触发器断言:在现有 data-slot 定位后,断言触发器元素为 button,并断言其 aria-label 属性存在且非空,保留现有触发器存在性检查。
- Around line 176-183: Update the click test around the existing MouseEvent
dispatch and leaderboard ordering assertion to verify the tooltip contract as
well: assert that a role="tooltip" element appears, then check its localized
explanatory message and both expected threshold values for the current locale.
Preserve the existing default row-order assertion.
🪄 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: 4b30930c-76ad-4793-9ac4-038c2fc7a11c
📒 Files selected for processing (3)
src/app/[locale]/dashboard/leaderboard/_components/leaderboard-table.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsxtests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/[locale]/dashboard/leaderboard/_components/leaderboard-table.tsx
| it("does not trigger column sorting when the help icon is clicked", async () => { | ||
| fetchMock.mockImplementation(async (input) => { | ||
| const url = String(input); | ||
| if (url.includes("scope=providerCacheHitRate")) { | ||
| return { | ||
| ok: true, | ||
| json: async () => [ | ||
| cacheHitEntry({ providerId: 1, providerName: "high-first", cacheCoefficientBp: 9500 }), | ||
| cacheHitEntry({ providerId: 2, providerName: "low-second", cacheCoefficientBp: 5000 }), | ||
| ], | ||
| } as Response; | ||
| } | ||
| return { ok: true, json: async () => [] } as Response; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'scope=(providerCacheHitRate|userCacheHitRate)' \
tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsxRepository: ding113/claude-code-hub
Length of output: 484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "file_stats"
wc -l tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx
echo
echo "outline"
ast-grep outline tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx || true
echo
echo "scope/usages"
rg -n 'userCacheHitRate|providerCacheHitRate|scope=|cacheCoefficient|cacheCoefficientBp|Leaderboard' tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx
echo
echo "sections"
sed -n '1,220p' tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsxRepository: ding113/claude-code-hub
Length of output: 10217
为 scope=userCacheHitRate 补充 LeaderboardView 测试覆盖。
当前用例只使用 scope=providerCacheHitRate,缓存系数列的 Tooltip、排序隔离和分级显示在未指定 scope 或默认用户排行榜路径下未得到验证。请补充对应的 userCacheHitRate 测试。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx` around
lines 151 - 164, 在 “does not trigger column sorting when the help icon is
clicked” 相关测试中补充 scope=userCacheHitRate 的 LeaderboardView 覆盖,验证用户排行榜默认路径下缓存系数列的
Tooltip、排序隔离和分级显示行为。复用现有 providerCacheHitRate 测试的断言结构与测试数据模式,确保两种 scope 均得到验证。
🧪 测试结果
总体结果: ✅ 所有测试通过 |
变更内容
–ColumnDef新增headerTooltip字段,tooltip 触发处stopPropagation,不影响表头排序验证
bun run typecheck/biome check/bun run build通过bunx vitest run tests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsx:4 passedtests/unit/dashboard/全套 31 个文件 174 个用例通过Greptile Summary
The PR adds localized cache-coefficient guidance and tier-based coloring to both provider leaderboards, while preserving sorting and missing-data behavior.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the previously reported inaccessible tooltip trigger is now a native focusable button.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(leaderboard,logs): address review fe..." | Re-trigger Greptile
Context used: