Conversation
📝 Walkthrough概览此拉取请求为日志仪表板的错误详情、提供者链和使用日志表组件增加了全面的单元测试覆盖，并重构了成本乘数显示逻辑和布局宽度处理，统一了跨多个组件的乘数计算。 变更
预计代码审查工作量🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @YangQing-Lin, 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! 此拉取请求旨在解决日志仪表板中存在的两个主要用户界面问题。首先，它修复了当供应商名称较长且同时显示请求次数徽章和信息图标时，Provider 列中成本倍率徽章溢出单元格并遮挡相邻列的布局问题。其次，它解决了 'costMultiplier' 字段在遇到 'NaN'、'Infinity'、空字符串、'null' 或 'undefined' 等无效值时，UI 会错误地显示 '×NaN' 或 '×Infinity' 的问题。通过引入更健壮的数字校验和布局调整，此更改提升了日志界面的视觉一致性和数据准确性，确保了用户体验的优化。 Highlights
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
|
![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
这个 PR 做得很好，成功解决了两个主要问题：日志表格中 Provider 徽章的溢出问题，以及无效 costMultiplier 值的显示问题。
- 布局修复：在
virtualized-logs-table.tsx和provider-chain-popover.tsx中对 Flexbox 属性（如flex-1,min-w-0,overflow-hidden）的运用非常到位，有效地解决了文本截断和 UI 溢出的问题。 - 逻辑健壮性：在
error-details-dialog.tsx,usage-logs-table.tsx和virtualized-logs-table.tsx中，对costMultiplier的处理逻辑变得更加严谨，通过Number.isFinite等检查，正确地排除了NaN和Infinity等无效值。 - 测试覆盖：为布局变更和倍率逻辑都添加了全面的测试用例，这对于保证代码库的稳定性非常有帮助。
整体来看，这是一次高质量的提交，代码清晰，目标明确。我有一些关于代码可读性和复用性的小建议，请查看具体的评论。
| {(() => { | ||
| if (costMultiplier === "" || costMultiplier == null) return null; | ||
| const multiplier = Number(costMultiplier); | ||
| if (!Number.isFinite(multiplier) || multiplier === 1) return null; | ||
|
|
||
| return ( | ||
| <div className="flex justify-between"> | ||
| <span className="text-muted-foreground"> | ||
| {t("logs.billingDetails.multiplier")}: | ||
| </span> | ||
| <span className="font-mono">{multiplier.toFixed(2)}x</span> | ||
| </div> | ||
| ); | ||
| })()} |
There was a problem hiding this comment.
您好，这部分对 costMultiplier 的校验逻辑非常棒，很严谨。
为了与本次 PR 中其他文件（如 usage-logs-table.tsx）的风格保持一致，并提高代码可读性，建议将这部分逻辑从 IIFE (立即调用函数表达式) 中提取出来。
例如，可以在组件函数体顶部（返回 JSX 之前）定义 multiplier 和 showMultiplier 变量，然后在 JSX 中直接使用它们进行条件渲染。这样可以让 JSX 结构更简洁清晰，也更便于维护。
| const multiplier = | ||
| actualCostMultiplier === "" || actualCostMultiplier == null | ||
| ? null | ||
| : Number(actualCostMultiplier); | ||
| const hasCostBadge = | ||
| multiplier != null && Number.isFinite(multiplier) && multiplier !== 1; |
There was a problem hiding this comment.
这套用于校验 costMultiplier 的逻辑非常健壮，并且在 error-details-dialog.tsx 和 virtualized-logs-table.tsx 中也得到了应用。
为了遵循 DRY (Don't Repeat Yourself) 原则并提高代码的可维护性，可以考虑将这部分逻辑提取到一个共享的工具函数中，例如 getValidMultiplier(costMultiplier)。
这个函数可以接收 costMultiplier 作为参数，并返回一个有效的、不为 1 的 number 或者 null。这样，所有需要此逻辑的组件都可以复用它，使得代码更整洁，未来修改也只需在一处进行。
例如：
// In a utils file
export function getValidMultiplier(costMultiplier: string | number | null | undefined): number | null {
if (costMultiplier === "" || costMultiplier == null) {
return null;
}
const multiplier = Number(costMultiplier);
if (!Number.isFinite(multiplier) || multiplier === 1) {
return null;
}
return multiplier;
}
// In the component
const multiplier = getValidMultiplier(actualCostMultiplier);
const hasCostBadge = multiplier !== null;![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
🤖 Fix all issues with AI agents
In @src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx:
- Around line 516-562: Wrap the test "scrolls to model redirect section when
scrollToRedirect is true" body in a try/finally so that
Element.prototype.scrollIntoView is always restored and vi.useRealTimers()
always called; capture the originalScrollIntoView before overriding, set
Element.prototype.scrollIntoView to scrollIntoViewMock for the test, run the
assertions, and in finally restore Element.prototype.scrollIntoView =
originalScrollIntoView and call vi.useRealTimers(), also ensure
container.remove() happens in the finally block to avoid leaking DOM between
tests.
In @src/app/[locale]/dashboard/logs/_components/error-details-dialog.tsx:
- Around line 539-552: The rendering shows "0.00x" for inputs like " " because
costMultiplier is not trimmed before numeric conversion; update the IIFE around
costMultiplier to first compute const trimmed = costMultiplier?.trim(); treat
trimmed === "" or trimmed == null as no multiplier, then parse const multiplier
= Number(trimmed) and keep the existing checks (Number.isFinite(multiplier) &&
multiplier !== 1) before rendering; reference costMultiplier and multiplier in
the change.
🧹 Nitpick comments (5)
src/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsx (1)
81-116: “不渲染倍率”的断言略宽松，建议补一个更直接的“倍率徽标不存在”断言以防误报当前主要通过
not.toContain("×NaN"/"×Infinity")来侧证；如果未来渲染了其它倍率文本但不包含这些片段，可能漏检。可以考虑额外断言“×”徽标整体不存在（例如定位 badge 容器/特定 class/data-slot，或断言不包含0.20x这类格式）。src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx (1)
345-345: 倍率显示字符与 usage-logs-table.tsx 不一致此处使用
x{multiplier.toFixed(2)}（ASCII 字母 x），而usage-logs-table.tsx第 197 行使用×{multiplier.toFixed(2)}（乘号 U+00D7）。建议统一使用相同的字符以保持一致性。建议修复
- x{multiplier.toFixed(2)} + ×{multiplier.toFixed(2)}或者在
usage-logs-table.tsx中改为使用x。src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsx (1)
182-194: 建议补充 multiplier === 1 的测试用例当前测试覆盖了无效值和有效非 1 值的情况，但缺少对
multiplier === 1的测试。根据业务逻辑，倍率为 1 时不应显示 Badge。建议添加的测试
test("does not render cost multiplier badge when multiplier equals 1", () => { mockIsLoading = false; mockIsError = false; mockError = null; mockHasNextPage = false; mockIsFetchingNextPage = false; mockLogs = [makeLog({ id: 1, costMultiplier: "1" })]; const html = renderToStaticMarkup( <VirtualizedLogsTable filters={{}} autoRefreshEnabled={false} /> ); expect(html).not.toContain("x1.00"); });src/app/[locale]/dashboard/logs/_components/usage-logs-table.tsx (2)
373-377: 倍率显示格式不一致在 Tooltip 中使用
{multiplier.toFixed(2)}x（后缀），而第 197 行的 Badge 中使用×{multiplier.toFixed(2)}（前缀）。同一文件内显示格式不一致可能造成用户困惑。建议统一格式
- {hasCostBadge && multiplier != null ? ( - <div> - {t("logs.billingDetails.multiplier")}: {multiplier.toFixed(2)}x - </div> - ) : null} + {hasCostBadge && multiplier != null ? ( + <div> + {t("logs.billingDetails.multiplier")}: ×{multiplier.toFixed(2)} + </div> + ) : null}
96-114: 可选：考虑提取倍率计算逻辑为共享工具函数此处的倍率计算逻辑与
virtualized-logs-table.tsx中的实现相似。如果未来需要在其他地方复用或修改该逻辑，可以考虑提取为共享的工具函数，例如：// lib/utils/cost-multiplier.ts export function getCostMultiplierInfo(providerChain, costMultiplier) { // ... shared logic return { multiplier, hasCostBadge }; }这是可选的改进建议，当前实现已满足需求。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (8)
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsxsrc/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.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
No emoji characters in any code, comments, or string literals
Files:
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,jsx,js}: All user-facing strings must use i18n (5 languages supported: zh-CN, en, ja, ko, de). Never hardcode display text
Use path alias @/ to map to ./src/
Use Biome for code formatting with configuration: double quotes, trailing commas, 2-space indent, 100 character line width
Prefer named exports over default exports
Use next-intl for internationalization
Use Next.js 16 App Router with Hono for API routes
Files:
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests in tests/unit/, integration tests in tests/integration/, and source-adjacent tests in src/**/*.test.ts
Files:
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use React 19, shadcn/ui, Tailwind CSS, and Recharts for the UI layer
Files:
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
🧠 Learnings (3)
📚 Learning: 2026-01-07T17:05:37.171Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:37.171Z
Learning: Applies to **/*.{tsx,jsx} : Use React 19, shadcn/ui, Tailwind CSS, and Recharts for the UI layer
Applied to files:
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx
📚 Learning: 2026-01-10T06:20:13.376Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 573
File: src/app/[locale]/settings/prices/_components/sync-conflict-dialog.tsx:42-53
Timestamp: 2026-01-10T06:20:13.376Z
Learning: In the claude-code-hub project, model pricing display (in files like `src/app/[locale]/settings/prices/_components/sync-conflict-dialog.tsx`) intentionally uses hardcoded USD currency symbol (`$`) and per-million-token notation (`/M`, `/img`) because the system exclusively tracks LiteLLM pricing in USD and the notation is industry standard. Configurability was deemed unnecessary complexity.
Applied to files:
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsx
📚 Learning: 2026-01-10T06:20:32.687Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 573
File: src/app/[locale]/settings/prices/_components/sync-litellm-button.tsx:118-125
Timestamp: 2026-01-10T06:20:32.687Z
Learning: In `src/app/[locale]/settings/prices/_components/sync-litellm-button.tsx`, the "Cancel" button in the SyncConflictDialog is intentionally designed to call `onConfirm([])`, which triggers `doSync([])` to continue the sync while skipping (not overwriting) conflicting manual prices. This is the desired product behavior to allow users to proceed with LiteLLM sync for non-conflicting models while preserving their manual price entries.
Applied to files:
src/app/[locale]/dashboard/logs/_components/error-details-dialog.tsx
🧬 Code graph analysis (5)
src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx (2)
src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx (1)
ProviderChainPopover(38-131)src/components/ui/badge.tsx (1)
Badge(39-39)
src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsx (2)
src/repository/usage-logs.ts (1)
UsageLogRow(30-62)src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx (1)
VirtualizedLogsTable(51-629)
src/app/[locale]/dashboard/logs/_components/usage-logs-table.tsx (2)
src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx (1)
ProviderChainPopover(38-131)src/lib/utils/provider-chain-formatter.ts (1)
formatProviderSummary(145-194)
src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx (1)
src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx (1)
ProviderChainPopover(38-131)
src/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsx (1)
src/repository/usage-logs.ts (1)
UsageLogRow(30-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Docker Build Test
- GitHub Check: 🌐 API Tests
- GitHub Check: pr-description
- GitHub Check: pr-review
- GitHub Check: check-codex-status
🔇 Additional comments (12)
src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx (1)
55-106: 布局收缩策略（w-full + min-w-0 + flex-1/shrink-0）合理，能稳定避免徽标/图标挤压文本这组调整能确保名称区域可截断且不会把右侧元素（Badge/Info）挤出或撑破布局；同时
aria-label仍保持可读。src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx (1)
1-149: 回归测试覆盖了关键 class（w-full/min-w-0/flex-1），方向正确；但文件落点/扩展名建议确认用例能锁住这次修复的核心布局 contract。另同样需要确认
src/**/**.test.tsx是否符合仓库测试放置约定。As per coding guidelines, tests placement/naming should follow the specified patterns.src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx (1)
1-658: 移除该审查意见 - 测试文件位置和命名符合现有代码规范该文件的位置 (
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx) 和命名遵循了项目既定的惯例。在代码库中，React 组件测试使用.test.tsx扩展名并放置在源代码相邻位置（src/内），而非组件测试使用.test.ts。此模式在整个代码库中一致使用。Likely an incorrect or invalid review comment.
src/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsx (1)
1-184: 文件位置和扩展名符合约定，无需修改该文件位置
src/app/[locale]/dashboard/logs/_components/符合 source-adjacent 测试约定。虽然编码指南提及.test.ts，但 vitest 配置明确支持src/**/*.{test,spec}.tsx模式，且仓库中 18 个.test.tsx文件都是组件测试，包括同目录的其他组件测试文件（如error-details-dialog.test.tsx、provider-chain-popover.test.tsx等）。.test.tsx扩展名对于包含 JSX 语法的组件测试是适当的。src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx (1)
298-334: 布局修复和倍率计算逻辑正确这段修改正确解决了 Provider 单元格溢出问题：
- 外层容器添加
w-full overflow-hidden确保内容不会溢出ProviderChainPopover包裹在flex-1 min-w-0 overflow-hidden的 div 中，允许其收缩- 倍率 Badge 使用
shrink-0保持固定宽度倍率计算逻辑使用
Number()转换和Number.isFinite()检查，正确过滤了 NaN、Infinity 等无效值。src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsx (4)
1-6: 导入顺序和组织测试文件结构良好，导入了必要的测试工具。但建议将
react-dom/server和react-dom/client的导入与 React 核心导入分组。
9-14: 测试状态变量初始化模块级可变变量用于控制 mock 行为是常见做法，但建议在每个测试前重置这些变量以避免测试间状态泄漏。当前在每个测试开头都有重置，这是好的做法。
164-180: 无效倍率测试覆盖全面测试正确覆盖了
null、undefined、空字符串、"NaN"、"Infinity"等无效值，验证这些情况下不会渲染错误的 Badge。
196-240: 滚动测试实现正确测试使用真实 DOM 渲染和
act来验证滚动行为，正确模拟了scrollTop设置和scroll事件分发。对scrollTo的 mock 处理考虑到了 happy-dom 的限制，清理逻辑也很完整。src/app/[locale]/dashboard/logs/_components/usage-logs-table.tsx (3)
96-114: 倍率计算逻辑提取到顶层将倍率计算逻辑从内联 IIFE 提取到
logs.map回调顶部是合理的重构，提高了可读性并允许在多处复用hasCostBadge和multiplier值。逻辑正确处理了空字符串、null、undefined 以及非有限数值的情况。
148-160: Provider 单元格布局修复
ProviderChainPopover包裹在w-full的 div 中，确保其正确占用可用宽度。与virtualized-logs-table.tsx的修改保持一致。
188-199: 倍率 Badge 渲染逻辑正确Badge 根据
multiplier > 1使用不同颜色（橙色表示更贵，绿色表示更便宜），符合直觉的用户体验设计。注意：此处使用
×（乘号），与virtualized-logs-table.tsx中的x不一致，建议统一。
| test("scrolls to model redirect section when scrollToRedirect is true", async () => { | ||
| vi.useFakeTimers(); | ||
| const container = document.createElement("div"); | ||
| document.body.appendChild(container); | ||
|
|
||
| const scrollIntoViewMock = vi.fn(); | ||
| const originalScrollIntoView = Element.prototype.scrollIntoView; | ||
| Object.defineProperty(Element.prototype, "scrollIntoView", { | ||
| value: scrollIntoViewMock, | ||
| configurable: true, | ||
| }); | ||
|
|
||
| const root = createRoot(container); | ||
| await act(async () => { | ||
| root.render( | ||
| <NextIntlClientProvider locale="en" messages={messages} timeZone="UTC"> | ||
| <ErrorDetailsDialog | ||
| externalOpen | ||
| statusCode={200} | ||
| errorMessage={null} | ||
| providerChain={null} | ||
| sessionId={null} | ||
| scrollToRedirect | ||
| originalModel={"m1"} | ||
| currentModel={"m2"} | ||
| /> | ||
| </NextIntlClientProvider> | ||
| ); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| vi.advanceTimersByTime(150); | ||
| }); | ||
|
|
||
| expect(scrollIntoViewMock).toHaveBeenCalled(); | ||
|
|
||
| await act(async () => { | ||
| root.unmount(); | ||
| }); | ||
|
|
||
| Object.defineProperty(Element.prototype, "scrollIntoView", { | ||
| value: originalScrollIntoView, | ||
| configurable: true, | ||
| }); | ||
| vi.useRealTimers(); | ||
| container.remove(); | ||
| }); |
There was a problem hiding this comment.
scrollIntoView + fake timers 建议用 try/finally 包裹，避免失败时污染后续用例
这里修改了 Element.prototype.scrollIntoView 并启用了 vi.useFakeTimers()；若中途断言失败，恢复逻辑不会执行，可能导致后续测试串扰。建议用 try/finally 保证 useRealTimers 与原型恢复必定执行。
🤖 Prompt for AI Agents
In @src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx
around lines 516 - 562, Wrap the test "scrolls to model redirect section when
scrollToRedirect is true" body in a try/finally so that
Element.prototype.scrollIntoView is always restored and vi.useRealTimers()
always called; capture the originalScrollIntoView before overriding, set
Element.prototype.scrollIntoView to scrollIntoViewMock for the test, run the
assertions, and in finally restore Element.prototype.scrollIntoView =
originalScrollIntoView and call vi.useRealTimers(), also ensure
container.remove() happens in the finally block to avoid leaking DOM between
tests.
| {(() => { | ||
| if (costMultiplier === "" || costMultiplier == null) return null; | ||
| const multiplier = Number(costMultiplier); | ||
| if (!Number.isFinite(multiplier) || multiplier === 1) return null; | ||
|
|
||
| return ( | ||
| <div className="flex justify-between"> | ||
| <span className="text-muted-foreground"> | ||
| {t("logs.billingDetails.multiplier")}: | ||
| </span> | ||
| <span className="font-mono">{multiplier.toFixed(2)}x</span> | ||
| </div> | ||
| ); | ||
| })()} |
There was a problem hiding this comment.
空白字符串会被当作 0 渲染为 0.00x，建议一并视为“无倍率”
当前仅排除了 "" | null | undefined，但像 " " 会 Number(" ") === 0 且 isFinite，最终显示 0.00x，通常更像“无效输入”。建议先 trim() 再判断空字符串。
建议修改（仅限本段）
- {(() => {
- if (costMultiplier === "" || costMultiplier == null) return null;
- const multiplier = Number(costMultiplier);
+ {(() => {
+ if (costMultiplier == null) return null;
+ const raw = costMultiplier.trim();
+ if (raw === "") return null;
+ const multiplier = Number(raw);
if (!Number.isFinite(multiplier) || multiplier === 1) return null;
return (
<div className="flex justify-between">
<span className="text-muted-foreground">
{t("logs.billingDetails.multiplier")}:
</span>
<span className="font-mono">{multiplier.toFixed(2)}x</span>
</div>
);
})()}📝 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.
| {(() => { | |
| if (costMultiplier === "" || costMultiplier == null) return null; | |
| const multiplier = Number(costMultiplier); | |
| if (!Number.isFinite(multiplier) || multiplier === 1) return null; | |
| return ( | |
| <div className="flex justify-between"> | |
| <span className="text-muted-foreground"> | |
| {t("logs.billingDetails.multiplier")}: | |
| </span> | |
| <span className="font-mono">{multiplier.toFixed(2)}x</span> | |
| </div> | |
| ); | |
| })()} | |
| {(() => { | |
| if (costMultiplier == null) return null; | |
| const raw = costMultiplier.trim(); | |
| if (raw === "") return null; | |
| const multiplier = Number(raw); | |
| if (!Number.isFinite(multiplier) || multiplier === 1) return null; | |
| return ( | |
| <div className="flex justify-between"> | |
| <span className="text-muted-foreground"> | |
| {t("logs.billingDetails.multiplier")}: | |
| </span> | |
| <span className="font-mono">{multiplier.toFixed(2)}x</span> | |
| </div> | |
| ); | |
| })()} |
🤖 Prompt for AI Agents
In @src/app/[locale]/dashboard/logs/_components/error-details-dialog.tsx around
lines 539 - 552, The rendering shows "0.00x" for inputs like " " because
costMultiplier is not trimmed before numeric conversion; update the IIFE around
costMultiplier to first compute const trimmed = costMultiplier?.trim(); treat
trimmed === "" or trimmed == null as no multiplier, then parse const multiplier
= Number(trimmed) and keep the existing checks (Number.isFinite(multiplier) &&
multiplier !== 1) before rendering; reference costMultiplier and multiplier in
the change.
![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
No significant issues identified in this PR. Given the XL size and that changes span UI layout, multiplier validation, and multiple new tests, consider splitting future work into separate PRs (layout-only, multiplier-only, tests-only) to reduce review load.
PR Size: XL
- Lines changed: 1244
- Files changed: 8
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
- Reviewed PR
#581(8 files, 1244 lines changed) across logic, security, error handling, types, comments, tests, and simplification; no issues met the reporting threshold on diff lines. - Applied label
size/XLviagh pr edit. - Submitted the review summary via
gh pr review --comment(no inline comments posted since no qualifying issues were found).
There was a problem hiding this comment.
Code Review Summary
This PR fixes layout overflow issues in the logs table provider column and improves validation for cost multiplier values to prevent rendering NaN/Infinity. The implementation is clean, well-tested, and follows established patterns in the codebase.
PR Size: XL
- Lines changed: 1244 (1102 additions, 142 deletions)
- Files changed: 8
Note for XL PRs: While this PR touches 8 files, the changes are tightly scoped to a single feature area (logs components) and the bulk of additions are test files. The implementation changes themselves are minimal and focused. No split is recommended.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 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 |
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Comprehensive (new tests for all edge cases)
- Code clarity - Good
Analysis Details
Layout Fixes (provider-chain-popover.tsx, virtualized-logs-table.tsx):
- Added
w-full,min-w-0,flex-1classes to enable proper flex shrinking - Wrapped
ProviderChainPopoverin a shrinkable container to prevent badge overflow - Changes follow established CSS flexbox patterns in the codebase
Multiplier Validation (all 3 table components):
- Replaced
parseFloat(String(...))withNumber()for cleaner type conversion - Added explicit checks for empty string, null, and
Number.isFinite()to filter NaN/Infinity - Consistent validation logic across all components
Test Coverage:
- New test files for
provider-chain-popover,virtualized-logs-table, andusage-logs-table - Tests cover all edge cases: null, undefined, empty string, "NaN", "Infinity", and valid multipliers
- Additional coverage for layout classes, blocked states, and pagination callbacks
Automated review by Claude AI
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
2 participants
PR: Fix logs provider badge overflow + hide invalid cost multiplier
基本信息
devfix/logs-provider-badge-overflow/zh-CN/dashboard/logs（Dashboard -> Logs）背景与问题
修复前截图：
修复后截图：
costMultiplier异常值（如NaN/Infinity/""/null/undefined）会导致 UI 展示×NaN/×Infinity（或详情中出现 multiplier 行），应对齐参考行为：仅在有限数值且不等于 1 时展示倍率。变更概览
1) 修复 Provider 单元格布局溢出
min-w-0、flex-1、overflow-hidden，并保持倍率 Badgeshrink-0，避免被挤出或覆盖相邻列。src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx:289src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx:752) 隐藏非法倍率（避免 NaN/Infinity 渲染）
costMultiplier统一使用Number()转换后用Number.isFinite(...)校验；multiplier !== 1时渲染倍率（Badge/详情行）。src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx:301src/app/[locale]/dashboard/logs/_components/usage-logs-table.tsx:96src/app/[locale]/dashboard/logs/_components/error-details-dialog.tsx:539测试与验证
必跑清单（CLAUDE.md）
bun run build✅bun run lint✅bun run lint:fix✅bun run typecheck✅bun run test✅（Vitest 全量通过）额外验证
.env.local重新跑全量测试：bun --env-file=.env.local run test✅src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx手工验收建议（视觉回归）
/zh-CN/dashboard/logs，构造/筛选一条记录：costMultiplier = "NaN" | "Infinity" | "" | null | undefined×NaN/×Infinity，且详情中不出现 multiplier 行。风险与回滚
风险
overflow-hidden可能裁切某些 focus outline（建议键盘聚焦触发器验证）。回滚
Commit 列表
59a06cb5fix: prevent provider badge overflow in logs table753a5dc2fix: make provider chain trigger shrinkable46eacda6fix: hide invalid cost multiplier badge132f37d5fix: avoid invalid multiplier in usage logsedf23761fix: hide invalid multiplier in error detailsd8d0a897test: add provider chain popover layout regression0be62a59test: cover invalid multiplier rendering branches179db42ftest: cover empty and undefined cost multiplierPR Checklist（提交前自检）
devcostMultiplier非有限值不渲染倍率（列表与详情一致）bun run build/lint/lint:fix/typecheck/test全绿Greptile Overview
Greptile Summary
This PR fixes two UI issues in the logs table:
Changes Overview
1. Layout Overflow Fix
flex-1,min-w-0,w-full, andoverflow-hiddenclasses to provider column containersProviderChainPopoverin a flex container to enable proper truncationmax-w-full shrinktow-full min-w-0for better flex behavior2. Invalid Multiplier Handling
parseFloat(String(...))withNumber()for type conversionNumber.isFinite()to filter NaN/InfinityImplementation Quality
Strengths:
Minor Concern:
costMultiplier = "0"will display×0.00badge, which may or may not be intended behavior. Consider if this should be hidden or displayed differently.Testing
All existing tests pass, and new tests specifically cover:
Confidence Score: 4/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant User participant LogTable as VirtualizedLogsTable/<br/>UsageLogsTable participant Popover as ProviderChainPopover participant Dialog as ErrorDetailsDialog User->>LogTable: View logs with long provider names Note over LogTable: Calculate multiplier validation LogTable->>LogTable: successfulProvider?.costMultiplier ?? log.costMultiplier LogTable->>LogTable: Check: !== "" && != null LogTable->>LogTable: Convert: Number(actualCostMultiplier) LogTable->>LogTable: Validate: isFinite() && !== 1 alt hasCostBadge = true LogTable->>Popover: Pass hasCostBadge=true Note over Popover: Adjust max-width<br/>for badge space Popover->>LogTable: Render with flex-1 min-w-0 LogTable->>User: Display truncated name + Badge else hasCostBadge = false LogTable->>Popover: Pass hasCostBadge=false Popover->>LogTable: Render with wider max-width LogTable->>User: Display truncated name only end User->>Dialog: Click status code for details Dialog->>Dialog: Validate costMultiplier alt Valid and !== 1 Dialog->>User: Show multiplier row in billing details else Invalid/null/empty/1 Dialog->>User: Hide multiplier row end