fix: stabilize usage log virtual scrolling#959
Merged
Conversation
📝 WalkthroughWalkthrough将使用日志检索从基于页码的分页切换为基于游标的批量（cursor/limit）分页，端到端替换后端仓储、动作、API 路由、前端列表与虚拟化钩子，并新增/调整多处测试以覆盖新行为与错误路径。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 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 |
Contributor
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 significantly improves the user experience and performance of the 'My Usage' logs section by transitioning from a paginated approach to a cursor-based infinite scrolling system. This change addresses previous data truncation issues and leverages virtualization to handle large log datasets efficiently. The update involves comprehensive refactoring across both frontend components and backend API actions to support the new data fetching and rendering paradigm. 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. Footnotes
|
github-actions
bot
added
bug
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring to implement infinite virtual scrolling for usage logs, which addresses page truncation and improves performance. The changes span from the database layer to the UI, introducing cursor-based pagination and a reusable useVirtualizedInfiniteList hook. The code is cleaner and more efficient. I have a couple of minor suggestions to improve the React useEffect dependency management for better performance and adherence to best practices.
src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx
Outdated
Show resolved
Hide resolved
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/actions/my-usage.ts (1)
521-548:⚠️ Potential issue | 🟠 Major先校验
cursor，再下推到仓储层。这里目前只裁剪了
limit，但cursor.createdAt/cursor.id完全信任调用方。只要传入非法日期字符串或非整数 id，后面的 SQL 元组比较和::timestamptz强转就会把一个坏请求升级成 500。建议在 action 边界做运行时校验，并对无效 cursor 直接返回可预期的业务错误。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/my-usage.ts` around lines 521 - 548, In getMyUsageLogsBatch the incoming filters.cursor is implicitly trusted before calling findUsageLogsForKeyBatch which can cause server errors when cursor.createdAt is an invalid date string or cursor.id is not an integer; validate the cursor at the action boundary: if filters.cursor is present ensure cursor.createdAt parses to a valid Date/ISO timestamptz and cursor.id is a positive integer (or null/undefined if not provided), and if validation fails return a controlled ActionResult error (e.g. { ok: false, error: "Invalid cursor" }) instead of calling findUsageLogsForKeyBatch; keep existing limit trimming but do not pass malformed cursor values through to findUsageLogsForKeyBatch.src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx (1)
115-139:⚠️ Potential issue | 🟠 Major每次自动刷新都会重新拉取所有已缓存的分页数据。
当前已移除了
maxPages限制，但refetchInterval仍然启用。TanStack Query 在无限查询上触发周期性刷新时，会自动重新拉取所有当前缓存的页面（以维护数据一致性，避免光标陈旧导致重复或缺失）。随着用户向下滚动加载更多页面，每次自动刷新的成本会线性增长，最终导致性能问题。建议限制轮询仅作用于首页，或在启用轮询时使用refetchPage选项仅刷新第一页，或显式设置maxPages来限制缓存大小。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx around lines 115 - 139, The infinite query currently re-fetches all cached pages on each auto-refresh, causing linear cost as more pages load; update the useInfiniteQuery call to limit polling to only the first page when shouldPoll is true (or set a max cached pages). Concretely, keep the existing queryFn/getNextPageParam but add the refetchPage option like refetchPage: (page, index) => index === 0 when shouldPoll is enabled (or alternatively implement logic to stop polling after N pages by enforcing a maxPages policy), ensuring autoRefreshIntervalMs/refetchInterval still controls timing but only triggers a refetch of the first page instead of all pages; reference useInfiniteQuery, refetchInterval, refetchPage, shouldPoll, and autoRefreshIntervalMs in your change.
🧹 Nitpick comments (2)
src/app/[locale]/my-usage/_components/usage-logs-table.tsx (1)
89-97: 没有依赖数组的useEffect可能导致不必要的执行Line 93-97 的
useEffect没有依赖数组，会在每次渲染时执行。虽然通过 ref 比较 (previousResetKeyRef.current === resolvedResetKey) 进行了门控，但这种模式可能在高频渲染场景下产生额外开销。建议添加
resolvedResetKey作为依赖项，使 effect 仅在 key 变化时触发：♻️ 建议的优化
useEffect(() => { if (previousResetKeyRef.current === resolvedResetKey) return; previousResetKeyRef.current = resolvedResetKey; resetScrollPosition(); - }); + }, [resolvedResetKey, resetScrollPosition]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/my-usage/_components/usage-logs-table.tsx around lines 89 - 97, The useEffect that checks previousResetKeyRef.current against resolvedResetKey and calls resetScrollPosition runs on every render because it lacks a dependency array; update the effect in usage-logs-table.tsx to include resolvedResetKey (and any stable functions like resetScrollPosition if not stable) in its dependency array so it only runs when resolvedResetKey changes, e.g. change the effect that references previousResetKeyRef, resolvedResetKey, and resetScrollPosition to useEffect(..., [resolvedResetKey]) and preserve the existing ref-based guard.src/app/[locale]/my-usage/_components/usage-logs-section.test.tsx (1)
75-108: 测试覆盖率可能不足当前测试仅验证了
useInfiniteQuery被调用且getMyUsageLogs未被调用。根据编码指南，新功能需要至少 80% 的单元测试覆盖率。建议考虑补充以下测试场景：
- 筛选条件变化时的行为（日期范围、模型、端点筛选）
- 错误状态处理
- 加载更多数据的交互
resetScrollKey变化时的滚动重置行为🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/my-usage/_components/usage-logs-section.test.tsx around lines 75 - 108, Add more unit tests to raise coverage for UsageLogsSection: extend the existing test suite that mocks useInfiniteQuery/getMyUsageLogs/getMyAvailableModels/getMyAvailableEndpoints and add tests that (1) simulate changing filter props/controls (date range, model and endpoint filters) and assert the component calls useInfiniteQuery with updated params and renders filtered state, (2) mock useInfiniteQuery returning isError/error and assert error UI is shown, (3) mock pages with more data and simulate the "load more" interaction to ensure fetchNextPage is called and new logs render, and (4) change the resetScrollKey prop (or trigger the resetScrollKey behavior) and assert the scroll/visible list resets; reuse the same mock setup and the UsageLogsSection render/unmount pattern to keep tests focused and deterministic.
🤖 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/app/`[locale]/my-usage/_components/usage-logs-section.tsx:
- Around line 85-109: The infinite query is accumulating unlimited pages while
auto-refresh is enabled, causing memory/performance issues; update the
useInfiniteQuery setup (queryKey ["my-usage-logs-batch", appliedFilters],
useInfiniteQuery call, getMyUsageLogsBatch, BATCH_SIZE, getNextPageParam) to (1)
enforce a maxPages constant and trim cached pages in an onSuccess handler (use
queryClient.setQueryData to slice pages to the newest maxPages) and (2) stop
global refetching of every cached page by replacing the current refetchInterval
logic with a targeted refresh: when autoRefreshSeconds is set and
!isBrowsingHistory, run a periodic task (useEffect + setInterval) that calls
queryClient.fetchQuery for the same queryKey supplying cursor undefined (to
reload only the first page) and clear the interval on cleanup; this combination
limits cached pages and only refreshes the first-screen data during polling.
In `@src/app/api/actions/`[...route]/route.ts:
- Around line 1096-1097: The OpenAPI fields summary and description in the route
registration are hardcoded; replace them with i18n lookups (use the project's
i18n helper) instead of literal strings so the text is localized for
zh-CN/zh-TW/en/ja/ru. Locate the summary and description assignments in
src/app/api/actions/[...route]/route.ts and swap the literals for calls to the
i18n API (e.g., i18n.t or the project's translation function) using descriptive
keys like "api.actions.batchGetMyUsage.summary" and
"api.actions.batchGetMyUsage.description", and add corresponding entries to the
i18n resource files for all supported languages. Ensure the keys are
exported/used consistently where OpenAPI metadata is built (summary and
description fields) so Swagger/Scalar shows translated text.
- Around line 1043-1046: You removed the public action name
my-usage/getMyUsageLogs when adding my-usage/getMyUsageLogsBatch; restore
backwards compatibility by registering an alias route named
"my-usage/getMyUsageLogs" that delegates to the existing handler
(getMyUsageLogsBatchHandler) or a thin wrapper that calls
myUsageActions.getMyUsageLogsBatch and emits a deprecation warning; update the
createActionRoute usages (the const tuple with getMyUsageLogsBatchRoute and
getMyUsageLogsBatchHandler) to also create or export the legacy route/handler
alias so existing clients continue to work while you run a deprecation window.
---
Outside diff comments:
In `@src/actions/my-usage.ts`:
- Around line 521-548: In getMyUsageLogsBatch the incoming filters.cursor is
implicitly trusted before calling findUsageLogsForKeyBatch which can cause
server errors when cursor.createdAt is an invalid date string or cursor.id is
not an integer; validate the cursor at the action boundary: if filters.cursor is
present ensure cursor.createdAt parses to a valid Date/ISO timestamptz and
cursor.id is a positive integer (or null/undefined if not provided), and if
validation fails return a controlled ActionResult error (e.g. { ok: false,
error: "Invalid cursor" }) instead of calling findUsageLogsForKeyBatch; keep
existing limit trimming but do not pass malformed cursor values through to
findUsageLogsForKeyBatch.
In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx:
- Around line 115-139: The infinite query currently re-fetches all cached pages
on each auto-refresh, causing linear cost as more pages load; update the
useInfiniteQuery call to limit polling to only the first page when shouldPoll is
true (or set a max cached pages). Concretely, keep the existing
queryFn/getNextPageParam but add the refetchPage option like refetchPage: (page,
index) => index === 0 when shouldPoll is enabled (or alternatively implement
logic to stop polling after N pages by enforcing a maxPages policy), ensuring
autoRefreshIntervalMs/refetchInterval still controls timing but only triggers a
refetch of the first page instead of all pages; reference useInfiniteQuery,
refetchInterval, refetchPage, shouldPoll, and autoRefreshIntervalMs in your
change.
---
Nitpick comments:
In `@src/app/`[locale]/my-usage/_components/usage-logs-section.test.tsx:
- Around line 75-108: Add more unit tests to raise coverage for
UsageLogsSection: extend the existing test suite that mocks
useInfiniteQuery/getMyUsageLogs/getMyAvailableModels/getMyAvailableEndpoints and
add tests that (1) simulate changing filter props/controls (date range, model
and endpoint filters) and assert the component calls useInfiniteQuery with
updated params and renders filtered state, (2) mock useInfiniteQuery returning
isError/error and assert error UI is shown, (3) mock pages with more data and
simulate the "load more" interaction to ensure fetchNextPage is called and new
logs render, and (4) change the resetScrollKey prop (or trigger the
resetScrollKey behavior) and assert the scroll/visible list resets; reuse the
same mock setup and the UsageLogsSection render/unmount pattern to keep tests
focused and deterministic.
In `@src/app/`[locale]/my-usage/_components/usage-logs-table.tsx:
- Around line 89-97: The useEffect that checks previousResetKeyRef.current
against resolvedResetKey and calls resetScrollPosition runs on every render
because it lacks a dependency array; update the effect in usage-logs-table.tsx
to include resolvedResetKey (and any stable functions like resetScrollPosition
if not stable) in its dependency array so it only runs when resolvedResetKey
changes, e.g. change the effect that references previousResetKeyRef,
resolvedResetKey, and resetScrollPosition to useEffect(..., [resolvedResetKey])
and preserve the existing ref-based guard.
🪄 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
Run ID: 72e09b51-a088-463c-a8d0-4c7bb9b2efd8
📒 Files selected for processing (13)
src/actions/my-usage.tssrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/[locale]/my-usage/_components/usage-logs-section.test.tsxsrc/app/[locale]/my-usage/_components/usage-logs-section.tsxsrc/app/[locale]/my-usage/_components/usage-logs-table.tsxsrc/app/[locale]/my-usage/page.tsxsrc/app/api/actions/[...route]/route.tssrc/hooks/use-virtualized-infinite-list.tssrc/repository/usage-logs.tstests/api/api-actions-integrity.test.tstests/api/my-usage-readonly.test.tstests/unit/actions/my-usage-date-range-dst.test.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR migrates my-usage logs from page-based to cursor-based pagination with TanStack Virtual infinite scrolling, and extracts a reusable useVirtualizedInfiniteList hook. The implementation is solid with proper error handling on the main data fetching path.
PR Size: XL
- Lines changed: 1457 (828 additions, 629 deletions)
- Files changed: 13
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 (cursor values parameterized via Drizzle sql template tag)
- Error handling - Clean (TanStack Query handles errors with isError/error state)
- Type safety - Clean (proper TypeScript interfaces for batch results and cursors)
- Documentation accuracy - Clean
- Test coverage - Good (new tests for hook usage, regression test for maxPages, updated DST tests)
- Code clarity - Good (extracted shared hook reduces duplication)
Notes
- The cursor-based pagination SQL uses Drizzle's
sqltemplate tag for proper parameterization:sql\(...createdAt, ...id) < (${cursor.createdAt}::timestamptz, ${cursor.id})`` - The
maxPagesremoval in dashboard logs is validated by a regression test - Helper queries (getMyAvailableModels/getMyAvailableEndpoints) silently fail on error, but this is pre-existing behavior and non-critical (dropdowns just show loading state)
Automated review by Claude AI
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR replaces page-based my-usage logs with cursor-based batching + virtualized infinite scrolling and removes cached-page capping on dashboard logs.
Two issues need attention: errors can become invisible after initial data loads, and the new cursor schema allows non-integer ids.
PR Size: XL
- Lines changed: 1457
- Files changed: 13
- For XL PRs: consider splitting into (1) backend cursor pagination + action route changes, (2) UI virtualization + UX changes, (3) test/regression updates.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 0 | 0 |
| Types | 0 | 0 | 1 | 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)
- [HIGH] [ERROR-NO-USER-FEEDBACK]
UsageLogsTableonly renderserrorMessagewhenlogs.length === 0, so failures after initial load can be silent. (Confidence: 88) - [MEDIUM] [TYPE-MISSING-VALIDATION] Cursor
idingetMyUsageLogsBatchschema usesz.number()instead ofz.number().int(). (Confidence: 85)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
- Identified open PR
#959(“fix: stabilize usage log virtual scrolling”) and labeled itsize/XL(13 files, 1457 lines changed). - Posted 2 inline review comments on diff lines:
src/app/[locale]/my-usage/_components/usage-logs-table.tsx:124— [HIGH] [ERROR-NO-USER-FEEDBACK]errorMessageis only rendered whenlogs.length === 0, so refetch/fetch-more failures can become invisible once any data is loaded.src/app/api/actions/[...route]/route.ts:1060— [MEDIUM] [TYPE-MISSING-VALIDATION] cursoridisz.number()(allows floats); suggestedz.number().int()for cursor/nextCursor ids.
- Submitted the required “Code Review Summary” via
gh pr review --commenton PR#959.
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx (1)
123-147:⚠️ Potential issue | 🟠 Major无界 infinite query 上的定时轮询会导致恢复时重抓成本爆炸
虽然代码已通过
!isHistoryBrowsing防止在用户浏览历史时轮询，但当用户从历史页面回到最新时（showScrollToTop变为false），shouldPoll恢复为true，refetchInterval会启动。此时 TanStack Query 会顺序重抓所有已缓存页面。由于移除了
maxPages限制，已加载页数可无界增长。用户深滚加载大量历史页后再回到最新，Line 142-146 会按累积的页数进行全量重抓，网络和渲染成本会线性放大。建议采用以下方案之一：
- 设置
maxPages: N（如 3-5）限制缓存页数- 拆分为独立查询来处理最新数据刷新，保持历史页单独管理
- 在轮询恢复前使用
queryClient.setQueryData手动裁剪为仅保留第一页🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx around lines 123 - 147, The infinite query can re-fetch every cached page when polling resumes; fix it by limiting cached pages before polling restarts: use useQueryClient() and in the refetchInterval callback (inside the useInfiniteQuery for queryKey ["usage-logs-batch", filters]) detect when shouldPoll would return an interval and call queryClient.setQueryData for that queryKey to trim data.pages to the most recent N (e.g., 3–5) and adjust data.pageParams accordingly, then return autoRefreshIntervalMs; alternatively implement a separate queryKey for "latest" vs historical pages, but the immediate fix is to trim the cached pages in refetchInterval to bound re-fetch cost.
🧹 Nitpick comments (2)
tests/unit/repository/usage-logs-sessionid-filter.test.ts (1)
115-186: 建议抽取测试行构造器，减少重复字段带来的维护成本。两段 case 的 mock 行结构高度重复；后续字段变更时需要多处同步，容易漏改。建议抽一个最小工厂函数。
可选重构示例（去重 mock row）
+function makeBatchRow(id: number, createdAt: string) { + return { + id, + createdAt: new Date(createdAt), + createdAtRaw: null, + sessionId: null, + requestSequence: null, + userName: "u", + keyName: "k", + providerName: "p", + model: "m", + originalModel: "m", + endpoint: "/v1/messages", + statusCode: 200, + inputTokens: 1, + outputTokens: 1, + cacheCreationInputTokens: 0, + cacheReadInputTokens: 0, + cacheCreation5mInputTokens: 0, + cacheCreation1hInputTokens: 0, + cacheTtlApplied: null, + costUsd: "0.01", + costMultiplier: null, + durationMs: 10, + ttfbMs: 5, + errorMessage: null, + providerChain: null, + blockedBy: null, + blockedReason: null, + userAgent: null, + messagesCount: null, + context1mApplied: null, + swapCacheTtlApplied: null, + specialSettings: null, + }; +} + - createThenableQuery([ - { ...大量重复字段..., id: 101, createdAt: new Date("2026-03-21T00:00:00Z"), createdAtRaw: null }, - { ...大量重复字段..., id: 100, createdAt: new Date("2026-03-20T00:00:00Z"), createdAtRaw: null }, - ]) + createThenableQuery([ + makeBatchRow(101, "2026-03-21T00:00:00Z"), + makeBatchRow(100, "2026-03-20T00:00:00Z"), + ])Also applies to: 207-248
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/repository/usage-logs-sessionid-filter.test.ts` around lines 115 - 186, The two mock rows in the selectMock for createThenableQuery are nearly identical and should be generated by a small factory to reduce duplication; add a helper function (e.g., makeUsageLogRow or usageLogRowFactory) adjacent to the test that returns a base row object (populating id, createdAt, sessionId, userName, keyName, providerName, model, endpoint, statusCode, tokens, costUsd, durationMs, etc.), then call that factory and override only the differing fields when building the array passed to createThenableQuery in the selectMock; update other similar mocks (the block referenced by selectMock and createThenableQuery and the repeated block pointed out in the comment) to use the same factory.src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx (1)
152-190: 修正滚动重置键的哈希语义，避免属性顺序导致的误触发当前代码用
JSON.stringify(filters)生成滚动重置键，但 TanStack Query 的对象 queryKey 使用的是自定义哈希（对键排序后序列化），与属性顺序无关。如果 filters 内容相同但属性插入顺序不同，标准JSON.stringify会产生不同的字符串，从而误触发resetScrollPosition()，导致用户突然被拉回页面顶部。建议用 TanStack Query 的哈希函数生成 reset key，或对 filters 对象进行规范化处理后再序列化。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx around lines 152 - 190, The reset-key should use a stable, order-insensitive hash instead of JSON.stringify to avoid spurious resets; replace the JSON.stringify(filters) used for filtersResetKey with a canonical hash (e.g., TanStack Query's stableHash/stableStringify) or by normalizing the filters (sort object keys recursively) before serializing, keeping the rest of the flow (previousFiltersResetKeyRef, handleFiltersReset, resetScrollPosition, and useVirtualizedInfiniteList) unchanged so identical filter contents produce the same key regardless of property order.
🤖 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/app/`[locale]/my-usage/_components/usage-logs-table.tsx:
- Around line 218-223: The clickable <span> used to copy model names is not
focusable or keyboard-accessible; update the usage in usage-logs-table.tsx to
make it accessible: either replace the <span> with a native <button> (preserving
className and calling handleCopyModel(log.model!)) or keep the element but add
role="button", tabIndex={0}, an onKeyDown handler that triggers handleCopyModel
on Enter/Space, and an appropriate aria-label. Ensure you reference and update
the interactive element that calls handleCopyModel and keep visual styling
identical.
- Around line 128-129: The current UI only shows errorMessage when logs.length
=== 0, which hides subsequent-page load errors; update the rendering logic in
usage-logs-table.tsx (affecting the earlier error-check and the loader row
handling at the other occurrence) so that when errorMessage is set and
logs.length > 0 you render the error inline in the table status/loader row
(instead of only showing the spinner) and expose a retry affordance that calls
the existing onLoadMore handler; specifically, detect errorMessage alongside
logs in the loader/StatusRow code paths and replace/augment the spinner with a
small error text and a retry button that invokes onLoadMore.
- Around line 150-165: The issue is that the inner scroll container (the div
with ref={parentRef} and onScroll={handleScroll}) uses overflow-auto which
allows independent horizontal scrolling and causes header/body desync; change
that container to only scroll vertically (e.g., overflow-y-auto and
overflow-x-hidden or overflow-x-visible) so the outer wrapper (the element with
overflow-x-auto and min-w) remains responsible for horizontal scrolling; update
the class on the element that currently reads "h-[520px] overflow-auto" to use
vertical-only scrolling and keep the ref and handleScroll as-is to preserve
scroll handling and sticky header alignment.
---
Outside diff comments:
In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx:
- Around line 123-147: The infinite query can re-fetch every cached page when
polling resumes; fix it by limiting cached pages before polling restarts: use
useQueryClient() and in the refetchInterval callback (inside the
useInfiniteQuery for queryKey ["usage-logs-batch", filters]) detect when
shouldPoll would return an interval and call queryClient.setQueryData for that
queryKey to trim data.pages to the most recent N (e.g., 3–5) and adjust
data.pageParams accordingly, then return autoRefreshIntervalMs; alternatively
implement a separate queryKey for "latest" vs historical pages, but the
immediate fix is to trim the cached pages in refetchInterval to bound re-fetch
cost.
---
Nitpick comments:
In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx:
- Around line 152-190: The reset-key should use a stable, order-insensitive hash
instead of JSON.stringify to avoid spurious resets; replace the
JSON.stringify(filters) used for filtersResetKey with a canonical hash (e.g.,
TanStack Query's stableHash/stableStringify) or by normalizing the filters (sort
object keys recursively) before serializing, keeping the rest of the flow
(previousFiltersResetKeyRef, handleFiltersReset, resetScrollPosition, and
useVirtualizedInfiniteList) unchanged so identical filter contents produce the
same key regardless of property order.
In `@tests/unit/repository/usage-logs-sessionid-filter.test.ts`:
- Around line 115-186: The two mock rows in the selectMock for
createThenableQuery are nearly identical and should be generated by a small
factory to reduce duplication; add a helper function (e.g., makeUsageLogRow or
usageLogRowFactory) adjacent to the test that returns a base row object
(populating id, createdAt, sessionId, userName, keyName, providerName, model,
endpoint, statusCode, tokens, costUsd, durationMs, etc.), then call that factory
and override only the differing fields when building the array passed to
createThenableQuery in the selectMock; update other similar mocks (the block
referenced by selectMock and createThenableQuery and the repeated block pointed
out in the comment) to use the same factory.
🪄 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
Run ID: 5e2db829-162d-4f50-a2b1-3b4a063de1a3
📒 Files selected for processing (4)
src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/[locale]/my-usage/_components/usage-logs-table.tsxsrc/repository/usage-logs.tstests/unit/repository/usage-logs-sessionid-filter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/repository/usage-logs.ts
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
1 similar comment
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/app/[locale]/my-usage/_components/usage-logs-table.tsx (2)
217-225:⚠️ Potential issue | 🟡 Minor把模型名复制控件改成原生按钮。
Line 220-225 现在只有鼠标点击路径，键盘和辅助技术用户没法触发复制。这个交互本身就是按钮语义，用
button type="button"最稳妥。可行修复
- {log.model ? ( - <span - className="cursor-pointer hover:underline truncate" - onClick={() => handleCopyModel(log.model!)} - > - {log.model} - </span> + {log.model ? ( + <button + type="button" + className="max-w-full cursor-pointer truncate border-0 bg-transparent p-0 text-left hover:underline" + onClick={() => handleCopyModel(log.model!)} + > + {log.model} + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/my-usage/_components/usage-logs-table.tsx around lines 217 - 225, The model name copy control is currently a clickable span which is not keyboard/AT accessible; replace the span used to display the model (the element rendering {log.model} with onClick={() => handleCopyModel(log.model!)}) with a semantic <button type="button"> that preserves the same className, onClick handler (handleCopyModel), and visual styles (cursor, hover:underline, truncate) and add an appropriate aria-label (e.g., `Copy model name`) so keyboard and assistive-technology users can trigger the copy action.
152-166:⚠️ Potential issue | 🟠 Major让外层容器独占横向滚动。
Line 166 这里继续使用
overflow-auto，会让表体拿到独立横向滚动条；窄屏时表头还留在外层容器里，列会失去同步。内层容器应只负责纵向滚动。可行修复
- <div ref={parentRef} className="h-[520px] overflow-auto" onScroll={handleScroll}> + <div + ref={parentRef} + className="h-[520px] overflow-y-auto overflow-x-hidden" + onScroll={handleScroll} + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/my-usage/_components/usage-logs-table.tsx around lines 152 - 166, The inner scroll container (the element with ref={parentRef} and onScroll={handleScroll}) is using "overflow-auto", which allows it to create its own horizontal scrollbar and desync the header; change its class to only allow vertical scrolling (e.g., replace "overflow-auto" with "overflow-y-auto" and ensure no "overflow-x-*" is present) so the outer wrapper (the div with "overflow-x-auto") remains the single horizontal scroller and the header stays aligned with the body.
🧹 Nitpick comments (1)
src/app/[locale]/my-usage/_components/usage-logs-table.test.tsx (1)
64-64: 这里不要再用相对路径导入src内模块。Line 64 的
./usage-logs-table绕开了仓库统一的@/别名约定，后续移动目录时也更容易漏改。直接走别名即可。可行修复
-import { UsageLogsTable } from "./usage-logs-table"; +import { UsageLogsTable } from "@/app/[locale]/my-usage/_components/usage-logs-table";As per coding guidelines, "Use path alias
@/to reference files in./src/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/my-usage/_components/usage-logs-table.test.tsx at line 64, 当前文件使用相对路径导入 UsageLogsTable ("./usage-logs-table")，绕开了仓库约定的 "@/“ 别名；请将该 import 改为使用项目别名（使用 "@/..." 形式指向同一模块），确保导入语句仍引用同一符号 UsageLogsTable，并同时检查该测试文件中其它相对 src 内的导入也统一替换为 "@/..." 别名以避免未来移动或重构出错。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/`[locale]/my-usage/_components/usage-logs-table.tsx:
- Around line 217-225: The model name copy control is currently a clickable span
which is not keyboard/AT accessible; replace the span used to display the model
(the element rendering {log.model} with onClick={() =>
handleCopyModel(log.model!)}) with a semantic <button type="button"> that
preserves the same className, onClick handler (handleCopyModel), and visual
styles (cursor, hover:underline, truncate) and add an appropriate aria-label
(e.g., `Copy model name`) so keyboard and assistive-technology users can trigger
the copy action.
- Around line 152-166: The inner scroll container (the element with
ref={parentRef} and onScroll={handleScroll}) is using "overflow-auto", which
allows it to create its own horizontal scrollbar and desync the header; change
its class to only allow vertical scrolling (e.g., replace "overflow-auto" with
"overflow-y-auto" and ensure no "overflow-x-*" is present) so the outer wrapper
(the div with "overflow-x-auto") remains the single horizontal scroller and the
header stays aligned with the body.
---
Nitpick comments:
In `@src/app/`[locale]/my-usage/_components/usage-logs-table.test.tsx:
- Line 64: 当前文件使用相对路径导入 UsageLogsTable ("./usage-logs-table")，绕开了仓库约定的 "@/“
别名；请将该 import 改为使用项目别名（使用 "@/..." 形式指向同一模块），确保导入语句仍引用同一符号
UsageLogsTable，并同时检查该测试文件中其它相对 src 内的导入也统一替换为 "@/..." 别名以避免未来移动或重构出错。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 439f4dca-7f51-4b72-88f2-d64979d97dfc
📒 Files selected for processing (4)
src/app/[locale]/my-usage/_components/usage-logs-table.test.tsxsrc/app/[locale]/my-usage/_components/usage-logs-table.tsxsrc/app/api/actions/[...route]/route.tstests/api/api-actions-integrity.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/api/api-actions-integrity.test.ts
- src/app/api/actions/[...route]/route.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/[locale]/my-usage/_components/usage-logs-table.tsx (1)
104-108: 清理 effect 的依赖可能导致意外行为。当
onHistoryBrowsingChange回调引用变化时，cleanup 函数会执行并调用onHistoryBrowsingChange?.(false)，这可能导致在组件仍挂载时误报"不在浏览历史"状态。建议使用 ref 存储回调，或移除依赖项（如果回调始终稳定）：
建议修复
+ const onHistoryBrowsingChangeRef = useRef(onHistoryBrowsingChange); + onHistoryBrowsingChangeRef.current = onHistoryBrowsingChange; useEffect(() => { return () => { - onHistoryBrowsingChange?.(false); + onHistoryBrowsingChangeRef.current?.(false); }; - }, [onHistoryBrowsingChange]); + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/my-usage/_components/usage-logs-table.tsx around lines 104 - 108, The effect's cleanup currently depends on onHistoryBrowsingChange so when that callback's identity changes the cleanup will run and call onHistoryBrowsingChange?.(false) unexpectedly; fix by keeping a stable ref to the callback instead of listing it in the dependency array: create a ref (e.g., historyChangeRef) updated whenever onHistoryBrowsingChange changes, leave the effect's dependency array empty (or only include truly stable values), and in the cleanup call historyChangeRef.current?.(false); alternatively, if onHistoryBrowsingChange is truly stable, remove it from the deps and document that assumption.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/`[locale]/my-usage/_components/usage-logs-table.tsx:
- Around line 104-108: The effect's cleanup currently depends on
onHistoryBrowsingChange so when that callback's identity changes the cleanup
will run and call onHistoryBrowsingChange?.(false) unexpectedly; fix by keeping
a stable ref to the callback instead of listing it in the dependency array:
create a ref (e.g., historyChangeRef) updated whenever onHistoryBrowsingChange
changes, leave the effect's dependency array empty (or only include truly stable
values), and in the cleanup call historyChangeRef.current?.(false);
alternatively, if onHistoryBrowsingChange is truly stable, remove it from the
deps and document that assumption.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d71f5cf-627e-4779-82cd-c755ad2b229e
📒 Files selected for processing (2)
src/app/[locale]/my-usage/_components/usage-logs-table.test.tsxsrc/app/[locale]/my-usage/_components/usage-logs-table.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/[locale]/my-usage/_components/usage-logs-table.test.tsx
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
5 tasks
Merged
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes usage log virtual scrolling truncation and stability issues by removing the
maxPagescap from dashboard logs and migrating my-usage logs from page-based pagination to cursor-based TanStack Query + TanStack Virtual infinite scrolling.Problem
maxPages: 5was capping cached pages, causing deep scroll to lose access to the latest rows)Related Work:
useVirtualizedInfiniteListhook rather than creating a separate component, improving code reuseSolution
maxPageslimit from the infinite query in the dashboard logs table to allow full page retentionuseVirtualizedInfiniteListhook that consolidates virtual list logic for both dashboard and my-usage tablespage,pageSize) to cursor-based pagination (cursor,limit) with propernextCursor/hasMoreresponse structureChanges
Core Changes
findUsageLogsForKeySlimwithfindUsageLogsForKeyBatchusing cursor-based paginationgetMyUsageLogswithgetMyUsageLogsBatchaction (cursor-based)UI Changes
maxPageslimit, use new hookAPI Changes
getMyUsageLogsroute withgetMyUsageLogsBatch(cursor-based)Testing
Automated Tests
usage-logs-section.test.tsx)maxPagesis not capped in dashboard logsmy-usage-date-range-dst.test.ts)Manual Testing
Checklist
Description enhanced by Claude AI
Greptile Summary
This PR fixes virtual-scrolling reliability in both the dashboard logs table and the my-usage logs section by (1) removing the
maxPages: 5cap that caused deep-scroll truncation on the dashboard, and (2) migrating my-usage logs from page-based pagination to cursor-based TanStack Query infinite scroll backed by a newfindUsageLogsForKeyBatchrepository function.Key changes:
src/repository/usage-logs.ts: ReplacesfindUsageLogsForKeySlim(OFFSET pagination +COUNT(*)) withfindUsageLogsForKeyBatch(keyset/cursor pagination). AddsbuildNextCursorOrThrowhelper — addressing the prior review concern thathasMoreandnextCursorcould silently drift, now enforced with a hard throw.src/hooks/use-virtualized-infinite-list.ts: New shared hook that consolidates scroll tracking,fetchNextPagetriggering, scroll-to-top state, and scroll-position reset for both the dashboard and my-usage virtual tables.src/app/[locale]/my-usage/_components/usage-logs-section.tsx: Replaces manualsetIntervalpolling + page state with TanStack QueryuseInfiniteQuery; auto-refresh suppression wired throughisBrowsingHistorystate from the child table.src/app/[locale]/my-usage/_components/usage-logs-table.tsx: Converts from a<Table>with prev/next buttons to a TanStack Virtual infinite scroll list matching the dashboard implementation.maxPagesabsence, cursor integer schema, DST date-range correctness,buildNextCursorOrThrowthrow path, and new component-level tests for the my-usage section and table.Confidence Score: 4/5
Important Files Changed
rowVirtualizerRefsync pattern forresetScrollPosition, configurable thresholds. No issues found.findUsageLogsForKeySlim(page-based, with expensive COUNT(*)) withfindUsageLogsForKeyBatch(cursor-based). AddsbuildNextCursorOrThrowhelper that ensureshasMoreandnextCursorare always consistent — addressing the prior thread concern about silent pagination truncation. Both batch paths (message-request and ledger fallback) correctly thread the cursor condition.<Table>to a TanStack Virtual infinite-scroll list.handleLoadMoreis correctly wrapped inuseCallback. Minor:errorMessageis shown redundantly in both a top banner and the status row; the plain banner adds no actionable affordance.isBrowsingHistorywired throughonHistoryBrowsingChangecorrectly suppresses auto-refresh when scrolled down.query.refetchfallback is unnecessary dead code.getMyUsageLogs(page+total) withgetMyUsageLogsBatch(cursor+hasMore). ExtractsmapMyUsageLogEntriesto deduplicate mapping logic. Clean migration with no regressions visible.maxPages: 5cap (core fix), delegates virtual-list plumbing touseVirtualizedInfiniteList, and switches from JSON-stringify comparison for filter resets to theuseEffectEventpattern. TheisHistoryBrowsingflag now controls auto-refresh suppression cleanly.isLogsLoading/logsDatastate and the correspondinggetMyUsageLogscall.getMyUsageLogstogetMyUsageLogsBatch, schema migrated from page/pageSize to cursor/limit.cursor.idand responsenextCursor.idboth declared asz.number().int(), verified by new integrity test.Sequence Diagram
sequenceDiagram participant UI as UsageLogsSection participant Hook as useVirtualizedInfiniteList participant TQ as TanStack Query participant Action as getMyUsageLogsBatch participant DB as findUsageLogsForKeyBatch UI->>TQ: useInfiniteQuery(queryKey, queryFn, getNextPageParam) TQ->>Action: queryFn({ pageParam: undefined }) Action->>DB: cursor=undefined, limit=20 DB-->>Action: { logs, nextCursor, hasMore } Action-->>TQ: page[0] UI->>Hook: useVirtualizedInfiniteList({ itemCount, hasNextPage, fetchNextPage }) Note over Hook: User scrolls near bottom Hook->>TQ: fetchNextPage() TQ->>Action: queryFn({ pageParam: nextCursor }) Action->>DB: cursor={createdAt, id}, limit=20 DB-->>Action: { logs, nextCursor, hasMore } Action-->>TQ: page[1] TQ-->>UI: data.pages = [page0, page1, ...] Note over UI: Filter change UI->>TQ: setAppliedFilters (new queryKey) TQ->>Action: queryFn({ pageParam: undefined }) Hook->>Hook: resetScrollPosition()Comments Outside Diff (4)
src/app/[locale]/my-usage/_components/usage-logs-section.tsx, line 596 (link)latestPagedata?.pages[0]is the first page fetched (the most-recent log batch in the DESC-ordered query). As the user scrolls, TanStack Query appends older data aspages[1],pages[2], etc. Sopages[0]is actually the first/initial page, not the "latest" one in the sense of "last appended". The name may confuse future readers who equate "latest" with "most recently fetched". A name likefirstPageorinitialPagewould be clearer.…and update the
currencyCodeprop reference accordingly:Prompt To Fix With AI
src/app/[locale]/my-usage/_components/usage-logs-section.tsx, line 593 (link)refetchquery.refetchis always a function in TanStack Query — it is part of the stableUseInfiniteQueryResultcontract and will never beundefined. The?? (async (): Promise<unknown> => undefined)fallback is unreachable and adds noise. Consider simplifying to:Prompt To Fix With AI
src/app/[locale]/my-usage/_components/usage-logs-table.tsx, line 1277-1296 (link)errorMessagerendered twice when stale logs are presentWhen
errorMessageis set andonLoadMoreis provided, the message appears in two adjacent places: a plain red banner (line 1277) and again in the status row with a retry button (lines 1281-1295). The banner has no actionable affordance, so the only useful version is the status-row entry with the retry button.The top banner should either be removed (let the status row own the error state) or changed to only show when
!onLoadMore(i.e. truly unrecoverable), so the user doesn't see the same string twice:Prompt To Fix With AI
src/app/[locale]/my-usage/_components/usage-logs-section.tsx, line 593 (link)refetchfallbackquery.refetchis always present on a TanStack Query result — it is part of the return type and is neverundefined. The?? (async (): Promise<unknown> => undefined)fallback is dead code and just adds noise.Prompt To Fix With AI
Prompt To Fix All With AI
Last reviewed commit: "fix: stabilize my-us..."