perf(dashboard): lazy-load usage data on users page for instant first render#818
Conversation
… render Split getUsersBatch into two phases: - getUsersBatchCore: returns users + keys only (2 queries, renders immediately) - getUsersUsageBatch: returns usage/statistics per key (lazy-loaded in background) Repository optimizations: - Add findKeysStatisticsBatchFromKeys to eliminate redundant findKeyListBatch call - Hybrid cursor pagination (keyset for NOT NULL columns, offset fallback for nullable) - Bound searchUsersForFilter to 200 results - Clamp page limit to [1, 200], validate userIds batch size (max 500) Client changes: - useInfiniteQuery calls getUsersBatchCore for instant table render - useQueries fires per-page usage queries independently - Merge usage data into allUsers memo as it arrives Reduces initial page load from 7 slow queries to 2 fast queries.
📝 WalkthroughWalkthrough将用户列表分页从数值光标改为字符串并引入键集/偏移混合分页；将密钥统计批处理函数拆分并重命名，新增按需加载使用数据的核心与使用API；客户端改为先拉取核心用户页，再并行延迟加载每页键使用统计；相关测试与仓库查询逻辑同步更新。 Changes
预计代码审查工作量🎯 4 (复杂) | ⏱️ ~45 分钟 🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ding113, 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 enhances the performance of the user dashboard by refactoring data loading mechanisms. It introduces a two-phase approach where core user and key information renders immediately, while heavier usage statistics are fetched and displayed asynchronously. This change also upgrades the pagination strategy to a more efficient hybrid cursor-based method, ensuring consistent performance regardless of the dataset size. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
Bot
added
size/M
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
| } | ||
| return merged; | ||
| }, [usageQueries]); |
There was a problem hiding this comment.
usageQueries array reference changes on every render. Memo should depend on query states instead.
| }, [usageQueries]); | |
| }, [usageQueries.map(q => q.data).join()]); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/users/users-page-client.tsx
Line: 228
Comment:
`usageQueries` array reference changes on every render. Memo should depend on query states instead.
```suggestion
}, [usageQueries.map(q => q.data).join()]);
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/user-repository-search-users-for-filter.test.ts (1)
40-46:⚠️ Potential issue | 🟡 Minor测试名称 "returns all users without limit" 与当前实现不一致。
searchUsersForFilter现在始终应用.limit(200)，但测试名称仍暗示无限制返回。此外，未验证limitMock是否被调用以及调用参数（200）是否正确。建议更新测试名称并增加断言
- test("returns all users without limit", async () => { + test("returns users capped at 200", async () => { const { searchUsersForFilter } = await import("@/repository/user"); const result = await searchUsersForFilter(); expect(result).toEqual(resolvedRows); + expect(limitMock).toHaveBeenCalledWith(200); });注意：
limitMock需要从 mock 作用域中导出或提升为模块级变量以便在测试中访问。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/user-repository-search-users-for-filter.test.ts` around lines 40 - 46, Rename the test "returns all users without limit" to reflect that searchUsersForFilter applies a limit (e.g., "applies default limit when no limit provided") and add assertions that the mock limit function (limitMock) is called and called with 200; update the test to import searchUsersForFilter and assert expect(limitMock).toHaveBeenCalled() and expect(limitMock).toHaveBeenCalledWith(200). If limitMock is currently scoped inside the mock definition, lift it to module scope or export it from the mock so the test can access it.
🧹 Nitpick comments (4)
src/repository/user.ts (1)
295-297: DESC 排序使用原始 SQL 而非 Drizzle 的desc()函数。ASC 使用了 Drizzle 的
asc(sortColumn)，但 DESC 使用了sql`${sortColumn} DESC`。建议保持一致性，导入并使用desc函数。建议统一使用 Drizzle 排序函数
-import { and, asc, eq, isNull, type SQL, sql } from "drizzle-orm"; +import { and, asc, desc, eq, isNull, type SQL, sql } from "drizzle-orm";- const orderByClause = sortOrder === "asc" ? asc(sortColumn) : sql`${sortColumn} DESC`; + const orderByClause = sortOrder === "asc" ? asc(sortColumn) : desc(sortColumn);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/user.ts` around lines 295 - 297, The code mixes Drizzle helpers and raw SQL for ordering: asc(sortColumn) is used but DESC is produced via sql`${sortColumn} DESC`; update the orderByClause to use Drizzle's desc() for consistency by importing desc from Drizzle and replacing the raw SQL branch so orderByClause uses asc(sortColumn) when sortOrder === "asc" and desc(sortColumn) when sortOrder === "desc" (referencing sortOrder, sortColumn, asc, desc, and orderByClause to locate the change).src/app/[locale]/dashboard/users/users-page-client.tsx (1)
205-228:usageByKeyId的useMemo依赖[usageQueries]，但useQueries每次渲染返回新数组引用，导致 memo 每次都重新计算。代码注释声称"只在实际数据变化时产生新对象"，但实际上
usageQueries是一个不稳定的引用。虽然Object.assign本身开销不大，但每次产生的新usageByKeyId对象会进一步导致allUsersmemo 也在每次渲染时重新计算，对大量用户场景下有不必要的开销。一种常见的解决方式是基于实际数据内容来推导稳定的依赖：
建议使用稳定的依赖值
+ // Derive a stable fingerprint from the actual query data + const usageDataFingerprint = useMemo(() => { + return usageQueries + .map((q) => (q.data?.ok ? q.dataUpdatedAt : 0)) + .join(","); + }, [usageQueries]); + const usageByKeyId = useMemo(() => { const merged: Record<number, KeyUsageData> = {}; for (const query of usageQueries) { if (query.data?.ok) { Object.assign(merged, query.data.data.usageByKeyId); } } return merged; - }, [usageQueries]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [usageDataFingerprint]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/users/users-page-client.tsx around lines 205 - 228, The memo for usageByKeyId depends on the unstable usageQueries array reference from useQueries, causing unnecessary recomputation; change the useMemo dependency to a stable derived value from the queries' actual results (for example compute deps = usageQueries.map(q => q.data ? q.data : null) or usageQueries.map(q => q.data?.data?.usageByKeyId ?? null) and use [deps] as the dependency) and keep the existing merging logic inside the useMemo (referencing usageByKeyId, usageQueries, useMemo, and getUsersUsageBatch) so usageByKeyId only recalculates when the contained query data actually changes.src/actions/users.ts (2)
630-747:getUsersBatchCore与getUsersBatch的UserDisplay构建逻辑高度重复。两个函数中 user →
UserDisplay的映射代码几乎完全相同（约 60 行），仅 usage 字段的来源不同。建议提取为共享的辅助函数，接受可选的 usage/statistics lookup 参数，以减少未来维护时两处不同步的风险。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/users.ts` around lines 630 - 747, getUsersBatchCore and getUsersBatch duplicate the user → UserDisplay mapping; extract that logic into a single helper (e.g., buildUserDisplay or mapUserToDisplay) that accepts a User object, its keys (from keysMap), locale/t translations, and an optional usage/statistics lookup (usageMap or a callback) so getUsersBatchCore can pass defaults and getUsersBatch can pass real usage; update getUsersBatchCore and getUsersBatch to call this helper (keeping symbols: getUsersBatchCore, getUsersBatch, UserDisplay, keysMap) to eliminate the ~60-line duplication while preserving all existing fields and formatting behavior.
793-798: 依赖顺序正确，但需注意inArray(usageLedger.key, keyStrings)的查询性能
Promise.all并行获取keysMap和usageMap，然后findKeysStatisticsBatchFromKeys(keysMap)串行执行，这是正确的依赖顺序。总计 5 个 DB 查询（findKeyListBatch+findKeyUsageTodayBatch+ 统计中的 3 个：today call counts、last usage LATERAL、model stats）。
usageMap和statisticsMap数据不重复——usageMap汇总每个 key 的总成本/token，statisticsMap提供每个 model 的详细分解。需注意
_findKeysStatisticsBatchInternal中inArray(usageLedger.key, keyStrings)的查询性能。当sanitizedIds接近 500 且每个用户有多个 key 时，keyStrings 列表会很大。虽然存在idx_usage_ledger_key_created_at复合索引可帮助优化，但在极端情况下仍应监控查询性能表现。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/users.ts` around lines 793 - 798, The DB batching is correct but the _findKeysStatisticsBatchInternal query uses inArray(usageLedger.key, keyStrings) which can produce a very large IN list and hurt performance when sanitizedIds (and thus keyStrings) is large; replace or mitigate the large IN-list by paginating/chunking keyStrings or switching to a join-based approach (e.g., temporary table, CTE, or EXISTS/INNER JOIN against a values/table of keys) inside findKeysStatisticsBatchFromKeys/_findKeysStatisticsBatchInternal, and ensure indexes like idx_usage_ledger_key_created_at are still used; update findKeyListBatch, findKeyUsageTodayBatch and findKeysStatisticsBatchFromKeys call sites only if needed to support the new join/temporary-key-table strategy.
🤖 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/repository/user.ts`:
- Around line 163-173: The parseKeysetCursor function currently casts parsed.id
with Number(parsed.id) which can yield NaN and later produce invalid SQL; update
parseKeysetCursor to validate that parsed.id is a finite integer before
returning: after JSON.parse ensure "v" exists and is a stringable value and that
Number(parsed.id) yields a finite integer (use Number(parsed.id) into a variable
like idNum and check Number.isInteger(idNum) && Number.isFinite(idNum)); if
validation fails return null instead of returning {v, id}; keep returning { v:
String(parsed.v), id: idNum } when valid.
---
Outside diff comments:
In `@tests/unit/user-repository-search-users-for-filter.test.ts`:
- Around line 40-46: Rename the test "returns all users without limit" to
reflect that searchUsersForFilter applies a limit (e.g., "applies default limit
when no limit provided") and add assertions that the mock limit function
(limitMock) is called and called with 200; update the test to import
searchUsersForFilter and assert expect(limitMock).toHaveBeenCalled() and
expect(limitMock).toHaveBeenCalledWith(200). If limitMock is currently scoped
inside the mock definition, lift it to module scope or export it from the mock
so the test can access it.
---
Nitpick comments:
In `@src/actions/users.ts`:
- Around line 630-747: getUsersBatchCore and getUsersBatch duplicate the user →
UserDisplay mapping; extract that logic into a single helper (e.g.,
buildUserDisplay or mapUserToDisplay) that accepts a User object, its keys (from
keysMap), locale/t translations, and an optional usage/statistics lookup
(usageMap or a callback) so getUsersBatchCore can pass defaults and
getUsersBatch can pass real usage; update getUsersBatchCore and getUsersBatch to
call this helper (keeping symbols: getUsersBatchCore, getUsersBatch,
UserDisplay, keysMap) to eliminate the ~60-line duplication while preserving all
existing fields and formatting behavior.
- Around line 793-798: The DB batching is correct but the
_findKeysStatisticsBatchInternal query uses inArray(usageLedger.key, keyStrings)
which can produce a very large IN list and hurt performance when sanitizedIds
(and thus keyStrings) is large; replace or mitigate the large IN-list by
paginating/chunking keyStrings or switching to a join-based approach (e.g.,
temporary table, CTE, or EXISTS/INNER JOIN against a values/table of keys)
inside findKeysStatisticsBatchFromKeys/_findKeysStatisticsBatchInternal, and
ensure indexes like idx_usage_ledger_key_created_at are still used; update
findKeyListBatch, findKeyUsageTodayBatch and findKeysStatisticsBatchFromKeys
call sites only if needed to support the new join/temporary-key-table strategy.
In `@src/app/`[locale]/dashboard/users/users-page-client.tsx:
- Around line 205-228: The memo for usageByKeyId depends on the unstable
usageQueries array reference from useQueries, causing unnecessary recomputation;
change the useMemo dependency to a stable derived value from the queries' actual
results (for example compute deps = usageQueries.map(q => q.data ? q.data :
null) or usageQueries.map(q => q.data?.data?.usageByKeyId ?? null) and use
[deps] as the dependency) and keep the existing merging logic inside the useMemo
(referencing usageByKeyId, usageQueries, useMemo, and getUsersUsageBatch) so
usageByKeyId only recalculates when the contained query data actually changes.
In `@src/repository/user.ts`:
- Around line 295-297: The code mixes Drizzle helpers and raw SQL for ordering:
asc(sortColumn) is used but DESC is produced via sql`${sortColumn} DESC`; update
the orderByClause to use Drizzle's desc() for consistency by importing desc from
Drizzle and replacing the raw SQL branch so orderByClause uses asc(sortColumn)
when sortOrder === "asc" and desc(sortColumn) when sortOrder === "desc"
(referencing sortOrder, sortColumn, asc, desc, and orderByClause to locate the
change).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (5)
src/actions/users.tssrc/app/[locale]/dashboard/users/users-page-client.tsxsrc/repository/key.tssrc/repository/user.tstests/unit/user-repository-search-users-for-filter.test.ts
| function parseKeysetCursor(raw: string): KeysetCursor | null { | ||
| try { | ||
| const parsed = JSON.parse(raw); | ||
| if (typeof parsed === "object" && parsed !== null && "v" in parsed && "id" in parsed) { | ||
| return { v: String(parsed.v), id: Number(parsed.id) }; | ||
| } | ||
| } catch { | ||
| // not JSON - treat as offset | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
parseKeysetCursor 未校验 id 是否为有效整数，可能导致 SQL 异常。
Number(parsed.id) 在输入非数值时会返回 NaN，后续 seek 条件（如 (col, id) > (d, NaN)）可能生成无效 SQL 或产生不可预期的查询结果。
建议增加 id 有效性校验
function parseKeysetCursor(raw: string): KeysetCursor | null {
try {
const parsed = JSON.parse(raw);
if (typeof parsed === "object" && parsed !== null && "v" in parsed && "id" in parsed) {
- return { v: String(parsed.v), id: Number(parsed.id) };
+ const id = Number(parsed.id);
+ if (!Number.isFinite(id) || !Number.isInteger(id)) return null;
+ return { v: String(parsed.v), id };
}
} catch {
// not JSON - treat as offset
}
return null;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repository/user.ts` around lines 163 - 173, The parseKeysetCursor
function currently casts parsed.id with Number(parsed.id) which can yield NaN
and later produce invalid SQL; update parseKeysetCursor to validate that
parsed.id is a finite integer before returning: after JSON.parse ensure "v"
exists and is a stringable value and that Number(parsed.id) yields a finite
integer (use Number(parsed.id) into a variable like idNum and check
Number.isInteger(idNum) && Number.isFinite(idNum)); if validation fails return
null instead of returning {v, id}; keep returning { v: String(parsed.v), id:
idNum } when valid.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly optimizes the users dashboard by implementing lazy-loading for usage and statistics data, reducing initial page render time through a two-phase fetching mechanism. It also introduces hybrid cursor pagination for efficient handling of large datasets. However, the new cursor parsing logic has a medium-severity Denial of Service (DoS) vulnerability due to a lack of input size validation on the pagination cursor, which could allow a malicious user to crash the server. Additionally, there are issues with the keyset pagination logic, including a fallback bug when switching sort orders and potential data consistency issues from timestamp precision loss in the cursor.
| if (cursor && useKeyset) { | ||
| const keyset = parseKeysetCursor(cursor); | ||
| if (keyset) { | ||
| if (sortBy === "createdAt") { | ||
| const d = new Date(keyset.v); | ||
| if (!Number.isNaN(d.getTime())) { | ||
| if (sortOrder === "asc") { | ||
| conditions.push(sql`(${sortColumn}, ${users.id}) > (${d}, ${keyset.id})`); | ||
| } else { | ||
| conditions.push( | ||
| sql`(${sortColumn} < ${d} OR (${sortColumn} = ${d} AND ${users.id} > ${keyset.id}))` | ||
| ); | ||
| } | ||
| } | ||
| } else { | ||
| if (sortOrder === "asc") { | ||
| conditions.push(sql`(${sortColumn}, ${users.id}) > (${keyset.v}, ${keyset.id})`); | ||
| } else { | ||
| conditions.push( | ||
| sql`(${sortColumn} < ${keyset.v} OR (${sortColumn} = ${keyset.v} AND ${users.id} > ${keyset.id}))` | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } else if (cursor) { | ||
| // Offset fallback for nullable columns | ||
| offset = Math.max(Number(cursor) || 0, 0); | ||
| } |
There was a problem hiding this comment.
There is a logic error in the keyset pagination fallback. If useKeyset is true but the cursor is not a valid JSON keyset (e.g., it's a numeric string from a previous session or a different sort column), the code enters the first block, parseKeysetCursor returns null, and the offset remains 0. This effectively resets the pagination to the first page instead of falling back to offset-based pagination. The fallback logic should be moved outside the if (keyset) block or handled more gracefully.
| function parseKeysetCursor(raw: string): KeysetCursor | null { | ||
| try { | ||
| const parsed = JSON.parse(raw); | ||
| if (typeof parsed === "object" && parsed !== null && "v" in parsed && "id" in parsed) { | ||
| return { v: String(parsed.v), id: Number(parsed.id) }; | ||
| } | ||
| } catch { | ||
| // not JSON - treat as offset | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The function parseKeysetCursor uses JSON.parse() on the raw input string, which is derived from the user-provided cursor parameter. A malicious user can provide a very large JSON string (a "JSON bomb") that can cause the server to consume excessive CPU and memory during parsing, leading to a Denial of Service (DoS). There is no size validation on the input before it is parsed.
function parseKeysetCursor(raw: string): KeysetCursor | null {
if (raw.length > 1024) { // Add a reasonable size limit
return null;
}
try {
const parsed = JSON.parse(raw);
if (typeof parsed === "object" && parsed !== null && "v" in parsed && "id" in parsed) {
return { v: String(parsed.v), id: Number(parsed.id) };
}
} catch {
// not JSON - treat as offset
}
return null;
}| const d = new Date(keyset.v); | ||
| if (!Number.isNaN(d.getTime())) { | ||
| if (sortOrder === "asc") { | ||
| conditions.push(sql`(${sortColumn}, ${users.id}) > (${d}, ${keyset.id})`); | ||
| } else { | ||
| conditions.push( | ||
| sql`(${sortColumn} < ${d} OR (${sortColumn} = ${d} AND ${users.id} > ${keyset.id}))` | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Using Date.toISOString() for keyset pagination on a timestamp with time zone column in PostgreSQL can lead to duplicates or skipped rows. JS Date objects and toISOString() only provide millisecond precision, whereas PostgreSQL timestamps typically have microsecond precision. When comparing col = d or using row constructors, the equality check will likely fail for records with non-zero microseconds, causing the pagination to skip the tie-breaker logic or re-include the last row of the previous page. Consider using date_trunc('milliseconds', sortColumn) in the comparison or selecting the timestamp as text with full precision for the cursor.
| const usageByKeyId = useMemo(() => { | ||
| const merged: Record<number, KeyUsageData> = {}; | ||
| for (const query of usageQueries) { | ||
| if (query.data?.ok) { | ||
| Object.assign(merged, query.data.data.usageByKeyId); | ||
| } | ||
| } | ||
| return merged; | ||
| }, [usageQueries]); |
There was a problem hiding this comment.
The usageByKeyId memoization depends on the usageQueries array. In TanStack Query (especially versions prior to v5 or without the combine option), useQueries returns a new array reference on every render. This causes the merge logic and the subsequent allUsers memo to re-run on every render of the component, even if the data hasn't changed. If you are using TanStack Query v5, consider using the combine property in useQueries to produce a stable merged result.
…print useQueries returns a new array reference every render, causing useMemo to recompute unconditionally. Use dataUpdatedAt timestamps as a stable dependency so the memo only recomputes when actual query data arrives.
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
| if (useKeyset) { | ||
| const lastRow = usersToReturn[usersToReturn.length - 1]; | ||
| const rawValue = sortBy === "name" ? lastRow.name : lastRow.createdAt; | ||
| nextCursor = encodeKeysetCursor(rawValue, lastRow.id); |
There was a problem hiding this comment.
hardcoded ternary only handles name, assumes all other keyset columns are createdAt — breaks if KEYSET_SORT_COLUMNS expands
| if (useKeyset) { | |
| const lastRow = usersToReturn[usersToReturn.length - 1]; | |
| const rawValue = sortBy === "name" ? lastRow.name : lastRow.createdAt; | |
| nextCursor = encodeKeysetCursor(rawValue, lastRow.id); | |
| const lastRow = usersToReturn[usersToReturn.length - 1]; | |
| const rawValue = sortColumn; // use the already-resolved sortColumn reference | |
| nextCursor = encodeKeysetCursor(rawValue, lastRow.id); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/repository/user.ts
Line: 370-373
Comment:
hardcoded ternary only handles `name`, assumes all other keyset columns are `createdAt` — breaks if `KEYSET_SORT_COLUMNS` expands
```suggestion
const lastRow = usersToReturn[usersToReturn.length - 1];
const rawValue = sortColumn; // use the already-resolved sortColumn reference
nextCursor = encodeKeysetCursor(rawValue, lastRow.id);
```
How can I resolve this? If you propose a fix, please make it concise.| if (sortBy === "createdAt") { | ||
| const d = new Date(keyset.v); | ||
| if (!Number.isNaN(d.getTime())) { | ||
| if (sortOrder === "asc") { | ||
| conditions.push(sql`(${sortColumn}, ${users.id}) > (${d}, ${keyset.id})`); | ||
| } else { | ||
| conditions.push( | ||
| sql`(${sortColumn} < ${d} OR (${sortColumn} = ${d} AND ${users.id} > ${keyset.id}))` | ||
| ); | ||
| } | ||
| } | ||
| } else { | ||
| if (sortOrder === "asc") { | ||
| conditions.push(sql`(${sortColumn}, ${users.id}) > (${keyset.v}, ${keyset.id})`); | ||
| } else { | ||
| conditions.push( | ||
| sql`(${sortColumn} < ${keyset.v} OR (${sortColumn} = ${keyset.v} AND ${users.id} > ${keyset.id}))` | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
keyset cursor parsing branches on sortBy === "createdAt" to parse date vs string, but fails to validate that the parsed value type matches the column type before using it in the WHERE clause
If a malformed cursor with a string value is sent for createdAt sort, new Date(keyset.v) could parse successfully but produce an invalid date, or if a numeric string is sent for name sort, it bypasses validation. Add validation that keyset.v matches expected format.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/repository/user.ts
Line: 306-325
Comment:
keyset cursor parsing branches on `sortBy === "createdAt"` to parse date vs string, but fails to validate that the parsed value type matches the column type before using it in the WHERE clause
If a malformed cursor with a string value is sent for `createdAt` sort, `new Date(keyset.v)` could parse successfully but produce an invalid date, or if a numeric string is sent for `name` sort, it bypasses validation. Add validation that `keyset.v` matches expected format.
How can I resolve this? If you propose a fix, please make it concise.| if (sortOrder === "asc") { | ||
| conditions.push(sql`(${sortColumn}, ${users.id}) > (${keyset.v}, ${keyset.id})`); | ||
| } else { | ||
| conditions.push( | ||
| sql`(${sortColumn} < ${keyset.v} OR (${sortColumn} = ${keyset.v} AND ${users.id} > ${keyset.id}))` | ||
| ); | ||
| } |
There was a problem hiding this comment.
for string columns like name, this assumes the column value can be directly compared with the string from the cursor, but doesn't handle potential SQL injection if keyset.v contains special characters
While drizzle-orm should handle parameterization, explicitly validating or sanitizing the cursor value would make this more defensive.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/repository/user.ts
Line: 318-324
Comment:
for string columns like `name`, this assumes the column value can be directly compared with the string from the cursor, but doesn't handle potential SQL injection if `keyset.v` contains special characters
While drizzle-orm should handle parameterization, explicitly validating or sanitizing the cursor value would make this more defensive.
How can I resolve this? If you propose a fix, please make it concise.| const pageUserIds = useMemo( | ||
| () => (data?.pages ?? []).map((page) => page.users.map((u) => u.id)), | ||
| [data] | ||
| ); |
There was a problem hiding this comment.
the pageUserIds memo flattens all loaded pages into a 2D array, but this creates a new array reference on EVERY pagination event (when data.pages changes)
This triggers all usageQueries to re-create their query descriptors even when only a new page is appended. Consider a more stable structure like a Map keyed by page index.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/users/users-page-client.tsx
Line: 200-203
Comment:
the `pageUserIds` memo flattens all loaded pages into a 2D array, but this creates a new array reference on EVERY pagination event (when `data.pages` changes)
This triggers all `usageQueries` to re-create their query descriptors even when only a new page is appended. Consider a more stable structure like a Map keyed by page index.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| if (Object.keys(usageByKeyId).length === 0) return coreUsers; | ||
| return coreUsers.map((user) => { | ||
| const hasUsageForAnyKey = user.keys.some((k) => k.id in usageByKeyId); | ||
| if (!hasUsageForAnyKey) return user; | ||
| return { |
There was a problem hiding this comment.
check hasUsageForAnyKey before cloning the entire user object, but then clone the user anyway if true — inefficient for users with many keys where only some have usage data
| if (Object.keys(usageByKeyId).length === 0) return coreUsers; | |
| return coreUsers.map((user) => { | |
| const hasUsageForAnyKey = user.keys.some((k) => k.id in usageByKeyId); | |
| if (!hasUsageForAnyKey) return user; | |
| return { | |
| return coreUsers.map((user) => { | |
| const keysWithUsage = user.keys.map((key) => { | |
| const usage = usageByKeyId[key.id]; | |
| return usage ? { ...key, ...usage } : key; | |
| }); | |
| return keysWithUsage === user.keys ? user : { ...user, keys: keysWithUsage }; | |
| }); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/users/users-page-client.tsx
Line: 237-241
Comment:
check `hasUsageForAnyKey` before cloning the entire user object, but then clone the user anyway if true — inefficient for users with many keys where only some have usage data
```suggestion
return coreUsers.map((user) => {
const keysWithUsage = user.keys.map((key) => {
const usage = usageByKeyId[key.id];
return usage ? { ...key, ...usage } : key;
});
return keysWithUsage === user.keys ? user : { ...user, keys: keysWithUsage };
});
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| const sanitizedIds = Array.from(new Set(userIds)).filter((id) => Number.isInteger(id)); | ||
| if (sanitizedIds.length === 0) { | ||
| return { ok: true, data: { usageByKeyId: {} } }; | ||
| } |
There was a problem hiding this comment.
filters out non-integer IDs but doesn't check for negative integers or zero, which could cause unexpected database queries or errors
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/users.ts
Line: 781-784
Comment:
filters out non-integer IDs but doesn't check for negative integers or zero, which could cause unexpected database queries or errors
How can I resolve this? If you propose a fix, please make it concise.| // Instead of N*3 queries (one per user for keys, usage, statistics), | ||
| // we now do 3 batch queries total | ||
| const userIds = users.map((u) => u.id); | ||
| const [keysMap, usageMap, statisticsMap] = await Promise.all([ | ||
| const [keysMap, usageMap] = await Promise.all([ | ||
| findKeyListBatch(userIds), | ||
| findKeyUsageTodayBatch(userIds), | ||
| findKeysWithStatisticsBatch(userIds), | ||
| ]); |
There was a problem hiding this comment.
changed to call findKeysStatisticsBatchFromKeys after awaiting keysMap instead of running in parallel with findKeyUsageTodayBatch — adds sequential latency
The original parallel structure was better for performance. The new function accepts a keysMap to avoid redundant findKeyListBatch, but you're still fetching keys here, so the optimization doesn't apply. Revert to parallel or pass keysMap properly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/users.ts
Line: 239-245
Comment:
changed to call `findKeysStatisticsBatchFromKeys` after awaiting `keysMap` instead of running in parallel with `findKeyUsageTodayBatch` — adds sequential latency
The original parallel structure was better for performance. The new function accepts a keysMap to avoid redundant `findKeyListBatch`, but you're still fetching keys here, so the optimization doesn't apply. Revert to parallel or pass keysMap properly.
How can I resolve this? If you propose a fix, please make it concise.![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR implements lazy-loading of usage data on the users page to improve initial render performance. The approach is sound: splitting data fetching into a fast "core" phase (users + keys) and a background "usage" phase. However, there are several issues that need to be addressed before merging.
PR Size: M
- Lines changed: 458 (424 additions, 34 deletions)
- Files changed: 5
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 | 1 | 0 | 0 | 0 |
| Comments/Docs | 0 | 2 | 0 | 0 |
| Tests | 0 | 2 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
[TYPE-MISMATCH] UserListBatchFilters.cursor type mismatch in src/repository/user.ts:11
The interface declares cursor?: number but the implementation now receives JSON-encoded keyset cursors (strings). This is a breaking type contract issue.
Current:
export interface UserListBatchFilters {
/** Offset pagination cursor */
cursor?: number;Fix:
export interface UserListBatchFilters {
/** Cursor for pagination (JSON-encoded keyset or numeric offset) */
cursor?: string;Also update UserListBatchResult.nextCursor on line 40 from number | null to string | null.
High Priority Issues (Should Fix)
-
[COMMENT-OUTDATED]
src/repository/user.ts:10-11andsrc/repository/user.ts:152The JSDoc comments describe "Offset pagination cursor" and "Offset-based pagination" but the implementation uses hybrid cursor pagination. Update comments to reflect the new keyset cursor approach.
-
[TEST-MISSING] Missing tests for
getUsersBatchCoreandgetUsersUsageBatchinsrc/actions/users.tsPer CLAUDE.md: "All new features must have unit test coverage of at least 80%". These new server actions lack any test coverage for:
- Permission checks (admin only)
- Batch size validation (max 500)
- Empty state handling
- Usage data aggregation logic
Add tests in
tests/unit/users-actions.test.ts.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Proper error logging and user feedback
- Type safety - One type mismatch identified
- Documentation accuracy - Two outdated comments
- Test coverage - Insufficient for new functions
- Code clarity - Good, with minor simplification opportunities
Automated review by Claude AI
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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]/dashboard/users/users-page-client.tsx:
- Around line 205-214: The current usageQueries queryFn returns the ActionResult
from getUsersUsageBatch, so failures with ok: false are swallowed and React
Query never enters error state; update the query function used in usageQueries
(the mapping over pageUserIds) to inspect the returned ActionResult from
getUsersUsageBatch and when result.ok is false either throw an Error (with the
result error message/details) so TanStack Query marks the query as error and
triggers retries, or at minimum emit a console.warn with the error details
before returning; ensure you reference getUsersUsageBatch and the usageQueries
mapping over pageUserIds so the change applies to each queryFn.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (1)
src/app/[locale]/dashboard/users/users-page-client.tsx
| const usageQueries = useQueries({ | ||
| queries: isAdmin | ||
| ? pageUserIds.map((ids) => ({ | ||
| queryKey: ["users-usage", ids], | ||
| queryFn: () => getUsersUsageBatch(ids), | ||
| enabled: ids.length > 0, | ||
| staleTime: 60_000, | ||
| refetchOnWindowFocus: false, | ||
| })) | ||
| : [], |
There was a problem hiding this comment.
getUsersUsageBatch 失败时错误被完全吞没
queryFn 直接返回 ActionResult，当 ok: false 时不会抛出异常。TanStack Query 规定：只有 queryFn 抛出异常或返回被拒绝的 Promise，Query 才会进入 error 状态。这导致：
query.isError永远为false，React Query 的默认重试（3 次）不会触发；- 使用数据静默返回全 0（
coreUsers初始值），既无控制台警告也无 UI 提示，给调试带来困难。
若这是有意为之的降级策略，建议至少在失败时添加一条 console.warn，以便排查。
建议修改
- queryFn: () => getUsersUsageBatch(ids),
+ queryFn: async () => {
+ const result = await getUsersUsageBatch(ids);
+ if (!result.ok) {
+ console.warn("[users-usage] Failed to load usage batch:", result.error);
+ }
+ return result;
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const usageQueries = useQueries({ | |
| queries: isAdmin | |
| ? pageUserIds.map((ids) => ({ | |
| queryKey: ["users-usage", ids], | |
| queryFn: () => getUsersUsageBatch(ids), | |
| enabled: ids.length > 0, | |
| staleTime: 60_000, | |
| refetchOnWindowFocus: false, | |
| })) | |
| : [], | |
| const usageQueries = useQueries({ | |
| queries: isAdmin | |
| ? pageUserIds.map((ids) => ({ | |
| queryKey: ["users-usage", ids], | |
| queryFn: async () => { | |
| const result = await getUsersUsageBatch(ids); | |
| if (!result.ok) { | |
| console.warn("[users-usage] Failed to load usage batch:", result.error); | |
| } | |
| return result; | |
| }, | |
| enabled: ids.length > 0, | |
| staleTime: 60_000, | |
| refetchOnWindowFocus: false, | |
| })) | |
| : [], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/dashboard/users/users-page-client.tsx around lines 205 -
214, The current usageQueries queryFn returns the ActionResult from
getUsersUsageBatch, so failures with ok: false are swallowed and React Query
never enters error state; update the query function used in usageQueries (the
mapping over pageUserIds) to inspect the returned ActionResult from
getUsersUsageBatch and when result.ok is false either throw an Error (with the
result error message/details) so TanStack Query marks the query as error and
triggers retries, or at minimum emit a console.warn with the error details
before returning; ensure you reference getUsersUsageBatch and the usageQueries
mapping over pageUserIds so the change applies to each queryFn.
| if (!Number.isNaN(d.getTime())) { | ||
| if (sortOrder === "asc") { | ||
| conditions.push(sql`(${sortColumn}, ${users.id}) > (${d}, ${keyset.id})`); | ||
| } else { |
There was a problem hiding this comment.
[CRITICAL] [LOGIC-BUG] Keyset pagination for createdAt can duplicate/skip rows due to timestamp precision mismatch.
Evidence: src/repository/user.ts:310-314
conditions.push(sql`(${sortColumn}, ${users.id}) > (${d}, ${keyset.id})`);
// ...
sql`(${sortColumn} < ${d} OR (${sortColumn} = ${d} AND ${users.id} > ${keyset.id}))`Why this is a problem: created_at is timestamp with time zone (microsecond precision), but the cursor is encoded/decoded via JS Date (toISOString() / new Date(...), millisecond precision). When the cursor loses sub-ms precision, the seek condition can include rows from the previous page (ASC) or skip rows (DESC), breaking pagination correctness.
Suggested fix (truncate DB timestamp to milliseconds in keyset comparisons to match JS encoding precision):
if (sortBy === "createdAt") {
const d = new Date(keyset.v);
if (!Number.isNaN(d.getTime())) {
const truncCol = sql`date_trunc('milliseconds', ${sortColumn})`;
if (sortOrder === "asc") {
conditions.push(sql`(${truncCol}, ${users.id}) > (${d}, ${keyset.id})`);
} else {
conditions.push(
sql`(${truncCol} < ${d} OR (${truncCol} = ${d} AND ${users.id} > ${keyset.id}))`
);
}
}
}
| ? pageUserIds.map((ids) => ({ | ||
| queryKey: ["users-usage", ids], | ||
| queryFn: () => getUsersUsageBatch(ids), | ||
| enabled: ids.length > 0, |
There was a problem hiding this comment.
[HIGH] [ERROR-NO-USER-FEEDBACK] Background usage fetch errors are silently dropped, leaving usage columns stuck at default 0/null with no UI signal.
Evidence: src/app/[locale]/dashboard/users/users-page-client.tsx:209-228
queryFn: () => getUsersUsageBatch(ids),
...
if (query.data?.ok) {
Object.assign(merged, query.data.data.usageByKeyId);
}Why this is a problem: getUsersUsageBatch returns an ActionResult. When it returns { ok: false, ... }, React Query still treats the request as a success, and the merge step simply skips it. That masks backend failures and can mislead admins into thinking usage is actually zero.
Suggested fix (treat ok: false as a query error + surface it once):
import { toast } from "sonner";
const usageQueries = useQueries({
queries: isAdmin
? pageUserIds.map((ids) => ({
queryKey: ["users-usage", ids],
queryFn: async () => {
const result = await getUsersUsageBatch(ids);
if (!result.ok) throw new Error(result.error);
return result.data;
},
retry: false,
onError: (err) => {
toast.error(err instanceof Error ? err.message : tCommon("error"));
},
enabled: ids.length > 0,
staleTime: 60_000,
refetchOnWindowFocus: false,
}))
: [],
});
const usageByKeyId = useMemo(() => {
const merged: Record<number, KeyUsageData> = {};
for (const query of usageQueries) {
if (query.data) Object.assign(merged, query.data.usageByKeyId);
}
return merged;
}, [usageDataVersion]);
| return { | ||
| ok: false, | ||
| error: tError("BATCH_SIZE_EXCEEDED"), | ||
| errorCode: ERROR_CODES.INVALID_FORMAT, |
There was a problem hiding this comment.
[MEDIUM] [LOGIC-BUG] BATCH_SIZE_EXCEEDED i18n message is missing the required {max} interpolation parameter.
Evidence: src/actions/users.ts:785-790
if (sanitizedIds.length > 500) {
return {
ok: false,
error: tError("BATCH_SIZE_EXCEEDED"),
errorCode: ERROR_CODES.INVALID_FORMAT,
};
}Why this is a problem: the translation string for BATCH_SIZE_EXCEEDED contains {max} (e.g. messages/en/errors.json), so returning it without params produces a broken user-facing message.
Suggested fix:
const MAX_BATCH_SIZE = 500;
...
if (sanitizedIds.length > MAX_BATCH_SIZE) {
return {
ok: false,
error: tError("BATCH_SIZE_EXCEEDED", { max: MAX_BATCH_SIZE }),
errorCode: ERROR_CODES.INVALID_FORMAT,
};
}
| } catch (error) { | ||
| logger.error("Failed to fetch user batch core data:", error); | ||
| const message = error instanceof Error ? error.message : "Failed to fetch user batch core data"; | ||
| return { ok: false, error: message, errorCode: ERROR_CODES.INTERNAL_ERROR }; |
There was a problem hiding this comment.
[MEDIUM] [STANDARD-VIOLATION] Server-action catch blocks return raw error.message / hardcoded English strings to the client.
Evidence: src/actions/users.ts:742-746, src/actions/users.ts:828-833
const message = error instanceof Error ? error.message : "Failed to fetch user batch core data";
return { ok: false, error: message, errorCode: ERROR_CODES.INTERNAL_ERROR };Why this is a problem: these error strings are surfaced to the UI (the client wraps result.error in new Error(...)). Returning raw exception messages bypasses i18n and can leak internal details (DB errors, stack-adjacent messages) into the dashboard.
Suggested fix (log the real error, return a localized generic message):
} catch (error) {
logger.error("Failed to fetch user batch core data:", error);
const tError = await getTranslations("errors");
return {
ok: false,
error: tError("INTERNAL_ERROR"),
errorCode: ERROR_CODES.INTERNAL_ERROR,
};
}
There was a problem hiding this comment.
Code Review Summary
This PR introduces two-phase loading for the Users dashboard and adds hybrid cursor pagination (keyset for name/createdAt, OFFSET fallback elsewhere). The performance goal is clear, but there are correctness risks in the new keyset cursor logic for createdAt ordering and multiple error paths that either fail silently or surface non-localized/internal messages.
PR Size: M
- Lines changed: 458
- Files changed: 5
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 1 | 0 | 1 | 0 |
| Security | 0 | 0 | 1 | 0 |
| Error Handling | 0 | 1 | 1 | 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)
src/repository/user.ts:310Keyset pagination forcreatedAtuses ms-precision cursors against microsecond DB timestamps, which can duplicate/skip rows at page boundaries (confidence 97).
High Priority Issues (Should Fix)
src/app/[locale]/dashboard/users/users-page-client.tsx:209Background usage fetch drops{ ok: false }results with no UI feedback, masking backend failures and leaving usage at defaults (confidence 90).src/actions/users.ts:785tError("BATCH_SIZE_EXCEEDED")is missing the{ max }param, producing broken i18n output (confidence 90).src/actions/users.ts:744Catch blocks return rawerror.message/ hardcoded English strings to the UI, bypassing i18n and potentially leaking internal details (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
There was a problem hiding this comment.
Applied PR size label size/M (458 lines changed across 5 files) and submitted the review on PR #818.
Inline comments posted (with file+line evidence + concrete fixes):
src/repository/user.tskeyset pagination forcreatedAtcan duplicate/skip rows due to microsecond DB timestamps vs millisecond JSDatecursor encoding.src/app/[locale]/dashboard/users/users-page-client.tsxusage lazy-load errors are silently dropped (no user feedback, defaults remain 0/null).src/actions/users.tstError("BATCH_SIZE_EXCEEDED")missing{ max }interpolation param.src/actions/users.tscatch blocks return rawerror.message/ hardcoded English strings to the UI (non-i18n + potential info leak).
1 participant
Summary
getUsersBatchinto two phases:getUsersBatchCore(users + keys only, 2 fast queries) andgetUsersUsageBatch(usage/statistics, lazy-loaded in background)name,createdAt), OFFSET fallback for nullable columnsfindKeyListBatchcall via newfindKeysStatisticsBatchFromKeysfunctionsearchUsersForFilterbounded to 200 results, page limit clamped to [1, 200],getUsersUsageBatchvalidates batch size (max 500)Before
usage_ledgeraggregates (14-17s)After
useQueriesname/createdAtsort avoids sequential scan at high offsetsRelated Issues
Files Changed
src/repository/key.tsfindKeysStatisticsBatchFromKeys+ shared_findKeysStatisticsBatchInternalsrc/repository/user.ts.limit(200)on search, limit clampingsrc/actions/users.tsgetUsersBatchCore,getUsersUsageBatch,KeyUsageDatatype, batch validationsrc/app/[locale]/dashboard/users/users-page-client.tsxuseQueries, usage merge memotests/unit/user-repository-search-users-for-filter.test.ts.limit()Test Plan
bun run typecheck- no type errorsbun run build- production build succeedsbun run lint- clean (only pre-existing warning)bun run test- 3240/3240 passed/dashboard/users, verify table renders immediately with user/key dataDescription enhanced by Claude AI
Greptile Summary
Optimized dashboard users page with two-phase data loading:
getUsersBatchCoreimmediately returns users + keys (2 fast queries), whilegetUsersUsageBatchlazy-loads usage/statistics in the background viauseQueries. Hybrid pagination now uses keyset cursors forname/createdAtsorts (O(1) seek) and falls back to OFFSET for nullable columns.Key improvements:
usage_ledgeraggregates (14-17s → instant)findKeyListBatchin old code via newfindKeysStatisticsBatchFromKeyshelperdataUpdatedAtfingerprint to prevent memo thrashingIssues found:
namecheck, assumes all other keyset columns arecreatedAt— breaks extensibilitygetUsers()and oldgetUsersBatch()changed to sequentialawaitinstead of parallelPromise.allfor statistics fetch — degrades performance benefit of the new helperpageUserIdsmemo creates new array reference on every pagination, forcinguseQueriesto regenerate descriptorsConfidence Score: 3/5
getUsers()/getUsersBatch()undermines the benefit of the new batch helper. These issues are non-critical but reduce robustness and performance in edge cases.src/repository/user.ts(keyset cursor logic) and verify DESC sort pagination manually per test planImportant Files Changed
Sequence Diagram
sequenceDiagram participant Client as Users Page Client participant Core as getUsersBatchCore participant Repo as findUserListBatch participant Usage as getUsersUsageBatch participant DB as Database Client->>Core: Request page (cursor, limit) Core->>Repo: findUserListBatch(filters) alt Keyset cursor (name/createdAt) Repo->>DB: SELECT users WHERE (col, id) > (cursor.v, cursor.id) else Offset cursor (nullable columns) Repo->>DB: SELECT users OFFSET cursor LIMIT end DB-->>Repo: Users batch + nextCursor Repo-->>Core: {users, nextCursor, hasMore} Core->>DB: findKeyListBatch(userIds) DB-->>Core: keysMap Core-->>Client: {users with keys, usage=0} Note over Client: Render table instantly Client->>Usage: getUsersUsageBatch(userIds) [background] Usage->>DB: findKeyListBatch(userIds) DB-->>Usage: keysMap Usage->>DB: findKeyUsageTodayBatch(userIds) DB-->>Usage: usageMap Usage->>DB: findKeysStatisticsBatchFromKeys(keysMap) DB-->>Usage: statisticsMap Usage-->>Client: {usageByKeyId} Note over Client: Merge usage into displayed usersLast reviewed commit: a5a49d1