Conversation
* fix: run reactive rectifiers during streaming hedge * fix: address hedge race conditions and audit accuracy in forwarder - Add attempt.settled guard in .then() callback to prevent stale response processing - Clear thresholdTimer before rectifier retry to avoid spurious hedge triggers - Use requestAttemptCount instead of sequence for accurate retry chain entries - Merge specialSettings on hedge winner sync to preserve rectifier audit data Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nfig (#954) Update proxy URL placeholder and description across all 5 languages (en, zh-CN, zh-TW, ja, ru) to include: - Complete list of supported protocols (http, https, socks5, socks4) - Authenticated proxy format: http://user:password@host:port - URL encoding reminder for special characters in passwords (e.g. # as %23) Previously the description only showed "Supported formats:" without listing them, and the placeholder had no authentication example.
* fix: stabilize usage log virtual scrolling * fix: address usage log PR review comments * fix: address follow-up PR review issues * fix: polish my-usage log table review follow-ups * fix: stabilize my-usage load-more callback
* feat: add codex priority billing source setting * fix: address codex priority billing review comments
* chore: upgrade tsgo and gate build flows * chore: keep caret deps and harden release gate
…keys (#965) Amp-Thread-ID: https://ampcode.com/threads/T-019d157d-b1be-73fa-b113-2eb4f14d6aa4 Co-authored-by: Amp <amp@ampcode.com>
* fix: 模型列表端点返回所有可用供应商的模型而非仅第一个 修复 #956 之前 `/v1/models` 端点复用了代理请求的 `selectProviderByType()`， 该方法每种 providerType 只选择一个最优供应商，导致同类型下其他 供应商的模型被遗漏。 改为直接从 `findAllProviders()` 获取全量供应商，按分组、启用状态、 providerType 和调度时间窗口过滤后，从所有匹配供应商并行获取模型 列表并去重聚合。跳过健康检查（熔断/费用限制），因为模型列表的 职责是展示可用模型，实时可用性由代理请求时的守卫链处理。 * refactor: 合并两步过滤为单次 filter 调用 采纳 gemini-code-assist 建议，消除中间变量 visibleProviders， 将分组过滤与类型/启用/调度过滤合并为一次 filter。
* fix: harden usage logs csv export Amp-Thread-ID: https://ampcode.com/threads/T-019d1590-be48-7749-987d-936bc6abd5f7 Co-authored-by: Amp <amp@ampcode.com> * fix: address review feedback on usage logs export - Replace hardcoded Chinese error strings with English messages - Add 5-minute polling timeout to prevent UI hanging on stuck jobs - Check Redis CSV write result and fail job if persist fails - Add clarifying comment on setTimeout for self-hosted server Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: increase export polling timeout from 5 to 10 minutes Align client-side deadline with backend's 15-minute TTL to avoid prematurely aborting large exports that legitimately run longer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: restore legacy user search API compatibility Amp-Thread-ID: https://ampcode.com/threads/T-019d1607-8336-7525-aab4-dd247d6a8d93 Co-authored-by: Amp <amp@ampcode.com> * fix: address user search review feedback Amp-Thread-ID: https://ampcode.com/threads/T-019d1679-0b36-75ba-803e-7c414bac6d0f Co-authored-by: Amp <amp@ampcode.com> --------- Co-authored-by: Amp <amp@ampcode.com>
📝 WalkthroughWalkthrough引入 Codex Priority 计费源字段与迁移（DB、类型、校验、缓存与 UI），将使用日志分页从页码改为光标并添加 Redis 异步导出作业，统一提供商组解析工具，重构多处虚拟化/导出/代理计费与测试，且更新 CI、依赖与本地忽略规则。（50 字内） Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
github-actions
bot
added
enhancement
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
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 introduces several enhancements and bug fixes across the application. It focuses on improving billing accuracy, enhancing user experience, and ensuring code stability. The changes include adding a new system setting for Codex Priority billing, improving usage logs export, enhancing provider group management, updating dependencies, and addressing various bugs and code improvements. Highlights
Ignored Files
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
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
1188
to
1238
| @@ -1077,19 +1216,23 @@ const { route: getMyUsageLogsRoute, handler: getMyUsageLogsHandler } = createAct | |||
| cacheTtlApplied: z.string().nullable(), | |||
| }) | |||
| ), | |||
| total: z.number(), | |||
| page: z.number(), | |||
| pageSize: z.number(), | |||
| nextCursor: z | |||
| .object({ | |||
| createdAt: z.string(), | |||
| id: z.number().int(), | |||
| }) | |||
| .nullable(), | |||
| hasMore: z.boolean(), | |||
| currencyCode: z.string(), | |||
| billingModelSource: z.enum(["original", "redirected"]), | |||
| }), | |||
| description: "获取当前会话的使用日志（仅返回自己的数据）", | |||
| summary: "获取我的使用日志", | |||
| description: "获取当前会话的使用日志批量数据（仅返回自己的数据，游标分页）", | |||
| summary: "批量获取我的使用日志", | |||
| tags: ["使用日志"], | |||
| allowReadOnlyAccess: true, | |||
| } | |||
| ); | |||
| app.openapi(getMyUsageLogsRoute, getMyUsageLogsHandler); | |||
| app.openapi(getMyUsageLogsBatchRoute, getMyUsageLogsBatchHandler); | |||
|
|
|||
| const { route: getMyAvailableModelsRoute, handler: getMyAvailableModelsHandler } = | |||
| createActionRoute("my-usage", "getMyAvailableModels", myUsageActions.getMyAvailableModels, { | |||
There was a problem hiding this comment.
The /api/actions/my-usage/getMyUsageLogs endpoint is being replaced with /api/actions/my-usage/getMyUsageLogsBatch, but the old path is simply removed — no redirect, no alias, no deprecation handler. Any external tool or script already calling the old endpoint will immediately start receiving 404 responses after this deploy.
The schema change is also non-trivial: callers using page/pageSize request params will need to switch to cursor/limit, and the response drops total, page, pageSize in favour of nextCursor/hasMore.
Consider either keeping the old route alive as a deprecated alias, or documenting a migration path explicitly in the release notes so integrators have time to adapt.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/actions/[...route]/route.ts
Line: 1188-1238
Comment:
**Breaking API endpoint rename without backward-compat shim**
The `/api/actions/my-usage/getMyUsageLogs` endpoint is being replaced with `/api/actions/my-usage/getMyUsageLogsBatch`, but the old path is simply removed — no redirect, no alias, no deprecation handler. Any external tool or script already calling the old endpoint will immediately start receiving 404 responses after this deploy.
The schema change is also non-trivial: callers using `page`/`pageSize` request params will need to switch to `cursor`/`limit`, and the response drops `total`, `page`, `pageSize` in favour of `nextCursor`/`hasMore`.
Consider either keeping the old route alive as a deprecated alias, or documenting a migration path explicitly in the release notes so integrators have time to adapt.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+832
to
+836
| systemSettings.codexPriorityBillingSource === "requested" | ||
| ? systemSettings.codexPriorityBillingSource | ||
| : "requested"; | ||
|
|
||
| if (billingModelSource !== systemSettings.billingModelSource) { |
There was a problem hiding this comment.
hasUsableBillingSettings() returns false when both the live DB read and the in-memory cache miss, causing getResolvedPricingByBillingSource() to return null and skip cost calculation entirely. Previously the code fell back to "redirected" and still computed a cost.
In practice this corner case is rare (requires a fresh startup with no prior successful DB read and a DB outage), but when it occurs the request is silently billed at $0 with no indication in the usage log.
A warning is already emitted, but consider whether it's preferable to bill with the safe default ("redirected") rather than drop the billing record altogether. At minimum, the log message should be at error level to make this visible in monitoring.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/session.ts
Line: 832-836
Comment:
**Pricing silently skipped when billing settings fall back to defaults**
`hasUsableBillingSettings()` returns `false` when both the live DB read and the in-memory cache miss, causing `getResolvedPricingByBillingSource()` to return `null` and skip cost calculation entirely. Previously the code fell back to `"redirected"` and still computed a cost.
In practice this corner case is rare (requires a fresh startup with no prior successful DB read *and* a DB outage), but when it occurs the request is silently billed at `$0` with no indication in the usage log.
A warning is already emitted, but consider whether it's preferable to bill with the safe default (`"redirected"`) rather than drop the billing record altogether. At minimum, the log message should be at `error` level to make this visible in monitoring.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+120
to
+140
| function hasExplicitPaginationParams( | ||
| params?: GetUsersBatchParams, | ||
| normalizedParams = normalizeUserListParams(params) | ||
| ): boolean { | ||
| return Boolean( | ||
| normalizedParams.cursor !== undefined || | ||
| normalizedParams.limit !== undefined || | ||
| params?.page !== undefined || | ||
| params?.offset !== undefined | ||
| ); | ||
| } | ||
|
|
||
| function hasSearchOrFilterOverrides(normalizedParams: GetUsersBatchParams): boolean { | ||
| return Boolean( | ||
| normalizedParams.searchTerm || | ||
| (normalizedParams.tagFilters?.length ?? 0) > 0 || | ||
| (normalizedParams.keyGroupFilters?.length ?? 0) > 0 || | ||
| normalizedParams.statusFilter || | ||
| normalizedParams.sortBy || | ||
| normalizedParams.sortOrder | ||
| ); |
There was a problem hiding this comment.
loadAllUsersForAdmin is unbounded and has no safety cap
When getUsers() is called without explicit pagination (the default from the web UI and the legacy API), the function iterates through every page at USER_LIST_MAX_LIMIT = 200 rows until the table is exhausted. For an installation with, say, 20 000 users that is 100 sequential DB round-trips and the full result set held in memory before the response is serialised.
The performance impact can be significant, and the function has no timeout, max-rows guard, or early-abort mechanism.
For the legacy-compat API path (no pagination params), consider adding a hard cap (e.g. 2 000 rows) and logging a warning when the cap is hit, or redirecting callers to the paginated getUsersBatch endpoint instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/users.ts
Line: 120-140
Comment:
**`loadAllUsersForAdmin` is unbounded and has no safety cap**
When `getUsers()` is called without explicit pagination (the default from the web UI and the legacy API), the function iterates through every page at `USER_LIST_MAX_LIMIT = 200` rows until the table is exhausted. For an installation with, say, 20 000 users that is 100 sequential DB round-trips and the full result set held in memory before the response is serialised.
The performance impact can be significant, and the function has no timeout, max-rows guard, or early-abort mechanism.
For the legacy-compat API path (no pagination params), consider adding a hard cap (e.g. 2 000 rows) and logging a warning when the cap is hit, or redirecting callers to the paginated `getUsersBatch` endpoint instead.
How can I resolve this? If you propose a fix, please make it concise.![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 is a substantial release PR that merges numerous features, bug fixes, and tooling upgrades. The changes are extensive and well-executed, touching upon proxy reliability, UI stability, security, and configuration. Key improvements include a more robust and scalable CSV export feature using background jobs, cursor-based pagination for usage logs to support virtual scrolling, and support for Unicode in provider group names. The codebase has also seen significant refactoring for better consistency and maintainability, particularly around provider group parsing. I've identified one medium-severity concern related to potential performance issues when fetching all users for an admin, which could be addressed to further improve scalability.
Comment on lines
323
to
+330
| if (isAdmin) { | ||
| users = await findUserList(); // 管理员可以看到所有用户 | ||
| if (hasExplicitPaginationParams(params, normalizedParams)) { | ||
| users = (await findUserListBatch(normalizedParams)).users; | ||
| } else if (hasSearchOrFilterOverrides(normalizedParams)) { | ||
| users = await loadAllUsersForAdmin(normalizedParams); | ||
| } else { | ||
| users = await loadAllUsersForAdmin(); | ||
| } |
Contributor
There was a problem hiding this comment.
The getUsers function, when called by an admin without explicit pagination parameters, fetches all users from the database via the loadAllUsersForAdmin helper. While this is done in pages, it accumulates all user objects in memory before returning. On instances with a very large number of users (e.g., tens of thousands), this could lead to high memory consumption and potential performance issues on the server.
Consider if the client can be adapted to always use pagination, or if this function should enforce a limit even when fetching "all" users to prevent unbounded memory usage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/api/actions/[...route]/route.ts (1)
1176-1235:⚠️ Potential issue | 🟠 Major保留
getMyUsageLogs兼容入口，否则现有 API 客户端会直接断这里把公开 action 名改成了
my-usage/getMyUsageLogsBatch，但这个文件里没有继续注册旧的my-usage/getMyUsageLogs。对外脚本或 SDK 升级后会直接 404，这是公开 API 的破坏性变更。至少需要保留一个过渡别名，并在适配层把旧的页码参数转换成新的 cursor 语义。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/actions/`[...route]/route.ts around lines 1176 - 1235, You removed the public action name getMyUsageLogs and replaced it with my-usage/getMyUsageLogsBatch causing breaking changes; restore backward compatibility by re-registering the legacy route name "my-usage/getMyUsageLogs" (using the same handler as getMyUsageLogsBatchHandler or a thin adapter) and implement an adapter that converts legacy pagination params (page, pageSize or limit/offset) into the new cursor shape (createdAt,id) and limit before calling myUsageActions.getMyUsageLogsBatch; reference getMyUsageLogsBatchRoute, getMyUsageLogsBatchHandler and myUsageActions.getMyUsageLogsBatch when locating where to add the alias and adapter logic.tests/unit/actions/usage-logs-export-retry-count.test.ts (1)
1-1:⚠️ Potential issue | 🟠 Major恢复 real timers，避免这个用例把时钟模式泄漏到别的文件
这个用例切到了 fake timers，但文件里没有在结束后切回 real timers。因为这里只在
beforeEach里复位，文件跑完以后不会再自动恢复，同 worker 的后续测试可能继续运行在 fake timer 模式。建议修改
-import { beforeEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";describe("Usage logs CSV export retryCount", () => { beforeEach(() => { vi.resetModules(); vi.clearAllMocks(); vi.useRealTimers(); exportStatusStore.clear(); exportCsvStore.clear(); getSessionMock.mockResolvedValue({ user: { id: 1, role: "admin" } }); findUsageLogsWithDetailsMock.mockResolvedValue({ logs: [], total: 0, summary: createSummary(), }); findUsageLogsBatchMock.mockResolvedValue({ logs: [], nextCursor: null, hasMore: false }); findUsageLogsStatsMock.mockResolvedValue(createSummary()); }); + + afterEach(() => { + vi.useRealTimers(); + });Also applies to: 154-168, 326-362
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/actions/usage-logs-export-retry-count.test.ts` at line 1, The test file switches to fake timers but never restores real timers, which can leak fake timer mode to other tests; add a teardown that calls vi.useRealTimers() (e.g., in an afterEach or afterAll) so that any use of vi.useFakeTimers() in tests is paired with vi.useRealTimers() and does not affect other suites—place the call alongside existing beforeEach/describe/test blocks and reference vitest's vi to restore timers.
🧹 Nitpick comments (15)
messages/ja/settings/providers/form/sections.json (1)
104-107: 建议在占位符中补一个 SOCKS 示例以保持说明一致。Line 104 明确支持
socks5:///socks4://，但 Line 107 仅给了 HTTP 示例；可追加一个 SOCKS 示例，降低误配概率。💡 可选文案调整
- "placeholder": "例: http://proxy.example.com:8080 または http://user:pass@proxy:8080" + "placeholder": "例: http://proxy.example.com:8080、socks5://127.0.0.1:1080 または http://user:pass@proxy:8080"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/ja/settings/providers/form/sections.json` around lines 104 - 107, The placeholder currently only shows an HTTP example while the formats string documents support for socks4/5; update the "placeholder" value to include a SOCKS example (e.g., add a socks5://... or socks4://... example) so it matches the "formats" entry—locate the "placeholder" key in the same object that contains "formats", "label", and "optional" and amend its text to include a SOCKS example.src/app/[locale]/settings/providers/_components/forms/provider-form.legacy.tsx (1)
889-889: 确认validateTag={() => true}的意图。
validateTag现在始终返回true，意味着onInvalidTag回调仅在duplicate/empty/too_long等内置检查失败时触发，不再执行自定义格式校验。表单提交时仍有总长度校验（lines 428-433），请确认此行为符合预期。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form.legacy.tsx at line 889, The validateTag prop currently returns true unconditionally which bypasses any custom format validation and leaves only the built-in duplicate/empty/too_long checks and the separate total-length check in the form submission; change validateTag on the ProviderForm component to implement the intended custom validation (or remove it to rely solely on built-ins) and ensure onInvalidTag is still triggered for custom failures—locate the validateTag={() => true} usage and replace it with the proper validation function that checks your required tag format (or document that intentional bypass) and keep the existing total-length check behavior in the submit flow.src/app/[locale]/my-usage/_components/usage-logs-table.tsx (1)
143-149: 潜在的重复重试 UI当
errorMessage和onLoadMore同时存在时，重试按钮会同时出现在状态栏（行 143-149）和 loader row（行 204-210）中。请确认这是预期设计，还是应该只在其中一处显示。♻️ 建议：移除 loader row 中的重试 UI，仅保留状态栏中的
{errorMessage && onLoadMore ? ( - <div className="flex items-center gap-3 text-xs text-destructive"> - <span>{errorMessage}</span> - <Button size="sm" variant="outline" onClick={onLoadMore}> - {tCommon("retry")} - </Button> - </div> + <span className="text-xs text-destructive">{errorMessage}</span> ) : ( <Loader2 className="h-4 w-4 animate-spin text-muted-foreground" /> )}Also applies to: 204-210
🤖 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 143 - 149, The error retry UI is rendered twice when both errorMessage and onLoadMore exist; update the component so only the status-bar location (the branch that renders {errorMessage && onLoadMore ? ...} around the status row) shows the Button, and remove the duplicate retry UI from the loader row (the alternate branch that renders the loader row error+Button). Concretely, keep the existing conditional that renders the Button next to the status text (references: errorMessage, onLoadMore) and delete or disable the duplicate Button/Span in the loader-row rendering (the loader row JSX that currently also checks errorMessage/onLoadMore) so the retry control appears only once.src/actions/usage-logs.ts (1)
230-242: 进度回调频繁访问 Redis每次批处理后都会从 Redis 读取并写入 job 状态。对于大型导出（数十万行），这可能产生显著的 Redis 操作开销。
♻️ 建议：降低进度更新频率
考虑每 N 批或每 N 秒更新一次进度，而非每批都更新：
+ let lastProgressUpdate = Date.now(); + const PROGRESS_UPDATE_INTERVAL_MS = 2000; + const csv = await buildUsageLogsExportCsv(filters, async (progress) => { + const now = Date.now(); + if (now - lastProgressUpdate < PROGRESS_UPDATE_INTERVAL_MS && progress.progressPercent < 100) { + return; + } + lastProgressUpdate = now; + const currentJob = await usageLogsExportStatusStore.get(jobId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/usage-logs.ts` around lines 230 - 242, The progress callback passed into buildUsageLogsExportCsv currently reads and writes usageLogsExportStatusStore on every batch (using jobId), causing heavy Redis load for large exports; modify the callback to throttle updates by aggregating progress and only calling usageLogsExportStatusStore.get/set when a threshold is reached (e.g., every N batches or every N seconds), track the lastSentTime/lastSentBatch and pendingProgress inside the scope where the callback is created, ensure you still send a final update when export completes, and keep references to jobId and usageLogsExportStatusStore so you update the same job record..github/workflows/dev.yml (1)
65-69: LGTM!在 CI 流程中添加
bun run typecheck是良好的改进，可以在早期捕获类型错误。这与 CLAUDE.md 中"提交前运行 typecheck"的指南一致。可选建议：根据项目指南，
bun run lint也是推荐的提交前检查项。如果希望在 CI 中获得更完整的代码质量检查，可以考虑在后续添加 lint 步骤。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/dev.yml around lines 65 - 69, The workflow step "Install dependencies, type check, and format code" currently runs "bun install", "bun run typecheck", and "bun run format"; optionally add "bun run lint" (per CLAUDE.md guidance) to that step to run linting in CI—insert the "bun run lint" command in the sequence (e.g., after "bun run typecheck" and before "bun run format") so the job runs install → typecheck → lint → format.src/app/[locale]/dashboard/_components/user/forms/user-form.tsx (1)
220-230:onInvalidTag处理器可能变为无效代码。由于
validateTag={() => true}始终返回true，onInvalidTag回调可能永远不会被触发。如果TagInputField组件的内部逻辑仅在validateTag返回false时调用onInvalidTag，则此处理器成为死代码。建议添加注释说明此设计意图，或者如果确认不再需要，可以移除
onInvalidTag处理器以提高代码清晰度。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/forms/user-form.tsx around lines 220 - 230, The onInvalidTag handler is likely dead because validateTag={() => true} always returns true; either remove onInvalidTag to avoid dead code or implement real validation in validateTag and keep onInvalidTag for fallback behavior. Concretely, either (A) delete the onInvalidTag block and its messages/toast usage from the TagInputField props (references: validateTag, onInvalidTag, TagInputField, tUI, toast.error), or (B) replace validateTag={() => true} with a proper validator that checks empty/duplicate/too_long/invalid_format/max_tags (using the same message keys) so that TagInputField will call onInvalidTag when validation fails; if you choose to keep the always-true validator, add a short comment above validateTag explaining it intentionally disables onInvalidTag to make intent explicit.src/types/css.d.ts (1)
1-1: CSS 模块声明正确。标准的环境模块声明，允许 TypeScript 编译 CSS 文件导入。可以考虑更明确的类型定义（如下），但当前实现也是可接受的标准做法。
可选：更明确的类型定义
-declare module "*.css"; +declare module "*.css" { + const content: { [className: string]: string }; + export default content; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/css.d.ts` at line 1, The current module declaration declare module "*.css"; is acceptable but to make types more explicit replace it with a typed CSS module declaration: declare module "*.css" { const classes: { [key: string]: string }; export default classes; } — update the module declaration (the "*.css" module) to export a mapping of class names to strings (or alternatively export const style names) so imports like import styles from "./x.css" have proper typing.src/app/[locale]/dashboard/_components/user/forms/add-key-form.tsx (1)
127-135: 建议在分组变更时统一写回规范化结果。当前仅在“多组且包含 default”时才使用解析结果；其他场景会把原始输入（空格/全角逗号等）直接写回。建议统一基于
parseProviderGroups的结果回写，保持存储格式一致。♻️ 可选重构示例
const handleProviderGroupChange = useCallback( (newValue: string) => { const groups = parseProviderGroups(newValue); - if (groups.length > 1 && groups.includes(PROVIDER_GROUP.DEFAULT)) { - const withoutDefault = groups.filter((g) => g !== PROVIDER_GROUP.DEFAULT); - form.setValue("providerGroup", withoutDefault.join(",")); - } else { - form.setValue("providerGroup", newValue); - } + const normalized = + groups.length > 1 && groups.includes(PROVIDER_GROUP.DEFAULT) + ? groups.filter((g) => g !== PROVIDER_GROUP.DEFAULT) + : groups; + form.setValue("providerGroup", normalized.join(",")); }, [form] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/forms/add-key-form.tsx around lines 127 - 135, handleProviderGroupChange currently only normalizes input when there are multiple groups including PROVIDER_GROUP.DEFAULT, but otherwise writes the raw newValue back; change it to always use the normalized array from parseProviderGroups before calling form.setValue so stored providerGroup is consistent. Specifically, call parseProviderGroups(newValue) into groups, compute the joined string (e.g., groups.join(",")) but if groups contains PROVIDER_GROUP.DEFAULT and groups.length>1 filter it out first, then call form.setValue("providerGroup", normalizedString); update the logic inside handleProviderGroupChange to always derive the value from parseProviderGroups rather than using newValue directly.src/app/[locale]/dashboard/users/users-page-client.tsx (1)
37-43: 建议同步更新splitTags注释描述。实现已切到
parseProviderGroups，注释仍写“逗号分割”，建议改成“复用共享分组解析器”，避免后续误解。📝 建议修改
-/** - * Split comma-separated tags into an array of trimmed, non-empty strings. - * This matches the server-side providerGroup handling in provider-selector.ts - */ +/** + * Normalize provider-group tags via shared parser to keep client/server behavior aligned. + */ function splitTags(value?: string | null): string[] { return parseProviderGroups(value); }🤖 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 37 - 43, 注释中描述 splitTags 为“逗号分割标签”已不正确；当前实现直接复用 parseProviderGroups。请更新 splitTags 上方的注释把“逗号分割”改为说明“复用共享分组解析器 parseProviderGroups 以返回已修剪且非空的标签数组”，并可提及这与服务端 providerGroup 解析保持一致（引用函数名 splitTags 和 parseProviderGroups 以便定位）。src/app/[locale]/dashboard/_components/user/forms/edit-key-form.tsx (2)
186-193: 建议在 Line 192 写回规范化后的分组字符串。当前已先用
parseProviderGroups(newValue)解析，但非default分支仍写回原始newValue。建议统一写回解析结果，避免同义输入（空格/分隔符差异）造成值不规范。♻️ 建议修改
const handleProviderGroupChange = useCallback( (newValue: string) => { const groups = parseProviderGroups(newValue); if (groups.length > 1 && groups.includes(PROVIDER_GROUP.DEFAULT)) { const withoutDefault = groups.filter((g) => g !== PROVIDER_GROUP.DEFAULT); form.setValue("providerGroup", withoutDefault.join(",")); } else { - form.setValue("providerGroup", newValue); + form.setValue("providerGroup", groups.join(",")); } }, [form] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/forms/edit-key-form.tsx around lines 186 - 193, The handler currently parses groups via parseProviderGroups(newValue) but in the else branch writes the original newValue back, causing inconsistent formatting; always normalize by using the parsed groups array (filtered as needed) and set form.setValue("providerGroup", parsedGroups.join(",")) so both branches write the canonical string—use parseProviderGroups, PROVIDER_GROUP.DEFAULT, and form.setValue("providerGroup", ...) to locate and update the logic.
265-275:validateTag={() => true}下的部分错误映射可简化。这里恒返回
true后，invalid_format这类依赖自定义校验的分支基本不会触发。建议删除不可达映射，或补一行注释说明“故意保留”。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/_components/user/forms/edit-key-form.tsx around lines 265 - 275, The onInvalidTag handler contains mappings for reasons like "invalid_format" and "too_long" while validateTag is hardcoded to always return true; either remove the unreachable mapping keys from the messages object (so messages only contains errors that can actually occur) or explicitly document why those keys are kept (add a comment near validateTag and onInvalidTag explaining they are intentionally preserved for future custom validation). Update references to validateTag and onInvalidTag accordingly so the code and comments stay consistent.src/app/[locale]/dashboard/rate-limits/_components/rate-limit-filters.tsx (1)
88-94: 建议为 providers 加载添加相同的取消守卫模式。用户列表加载已正确添加了
cancelled标志，但供应商列表加载（Lines 88-94）缺少相同的保护，可能导致组件卸载后的状态更新警告。建议的改进
// 加载供应商列表 React.useEffect(() => { - getProviders().then((providerList) => { - setProviders(providerList); - setLoadingProviders(false); - }); + let cancelled = false; + + void getProviders() + .then((providerList) => { + if (!cancelled) { + setProviders(providerList); + } + }) + .finally(() => { + if (!cancelled) { + setLoadingProviders(false); + } + }); + + return () => { + cancelled = true; + }; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/rate-limits/_components/rate-limit-filters.tsx around lines 88 - 94, The providers loading effect (React.useEffect) should use the same cancellation guard pattern as the users loader to avoid updating state after unmount: inside the effect declare a local let cancelled = false, call getProviders().then(providerList => { if (!cancelled) { setProviders(providerList); setLoadingProviders(false); } }); and return a cleanup function that sets cancelled = true; reference the existing getProviders, setProviders, setLoadingProviders and the React.useEffect to locate where to add this guard.src/components/ui/__tests__/provider-group-tag-input.test.tsx (1)
1-10: 建议把这两个用例拆回各自源码旁边。这个文件同时覆盖
TagInput和ProviderGroupSelect，但放在src/components/ui/__tests__下，后续改动时很难从源码附近发现对应测试。更合适的是分别放到各自组件旁边，保持 source-adjacent。As per coding guidelines, "Source-adjacent tests should be placed in
src/**/*.test.tsalongside source files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/__tests__/provider-group-tag-input.test.tsx` around lines 1 - 10, The test file combines two component specs (TagInput and ProviderGroupSelect) in a shared ui __tests__ folder; split them into two source-adjacent tests: create provider-group-select.test.tsx next to the ProviderGroupSelect component and tag-input.test.tsx next to the TagInput component, copy the relevant tests into their respective files, update imports inside each new test to reference the local component path and keep the top-line vitest environment and any setup/teardown (act, createRoot, vi) needed, and remove the original combined test file so each component’s tests live beside its source (ensuring any shared mocks/helpers are moved or re-exported as needed).src/app/[locale]/my-usage/_components/usage-logs-table.test.tsx (1)
89-125: 这组用例现在覆盖不到真正的交互回归。
renderToStaticMarkup只能验证静态 HTML；retry、复制按钮、onLoadMore这些本次新增的交互路径都不会真正执行。也就是说，就算事件绑定断掉，这三条用例还是会通过。建议至少补一条 happy-dom 交互用例，实际点击重试或复制按钮并断言回调/提示被触发。As per coding guidelines, "All new features must have unit test coverage of at least 80%".
🤖 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 around lines 89 - 125, The current tests use renderToStaticMarkup so interactive paths (retry/button copy/onLoadMore) aren’t exercised; add at least one DOM-enabled test using happy-dom (or `@testing-library/react` with a happy-dom environment) that mounts UsageLogsTable, simulates user events (click the retry control to assert onLoadMore is called and click the model-copy button to assert clipboard write or a callback/tooltip appears), and verifies the UI updates or callbacks fire; target the UsageLogsTable component and its props (logs, onLoadMore, errorMessage) when adding the new interaction test.src/app/v1/_lib/proxy/provider-selector.ts (1)
51-53:parseGroupString已变成纯透传包装，可选地直接移除以降低维护成本现在该函数不再承载独立逻辑，后续可直接使用
parseProviderGroups，减少一层间接调用和注释维护负担。可选重构示例
-function parseGroupString(groupString: string): string[] { - return parseProviderGroups(groupString); -} +const parseGroupString = parseProviderGroups;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/provider-selector.ts` around lines 51 - 53, parseGroupString is now a trivial passthrough to parseProviderGroups; remove the wrapper to reduce indirection and update callers to call parseProviderGroups directly. Delete the parseGroupString function from provider-selector.ts, replace any references to parseGroupString with parseProviderGroups, and adjust imports/exports so parseProviderGroups is exported/available where needed (or re-export it if the module API must remain unchanged).
🤖 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/actions/users.ts`:
- Around line 390-393: Extract the "allow expose full key" check into a shared
predicate and reuse it in getUsers(), getUsersBatch(), and getUsersBatchCore():
create a helper (e.g., canExposeFullKey(session, targetUser) or reuse
canUserManageKey) that returns session.key.canLoginWebUi && (isAdmin ||
session.user.id === targetUser.id) (ensure isAdmin is obtainable where used),
then replace the inline checks so these three functions only include fullKey and
set canCopy: true when that helper returns true—also remove any other
unconditional returns of fullKey in these handlers to centralize the permission
logic.
In `@src/app/`[locale]/dashboard/_components/rate-limit-top-users.tsx:
- Around line 41-45: The repository call searchUsersForFilterRepository has a
hard-coded .limit(200) which truncates leaderboard results; update the code so
the component can get complete or paginated results: either (A) change
searchUsersForFilterRepository to accept limit/offset (or page/size) parameters
and pass them through from the searchUsers() wrapper used by the component (so
setUsers receives a paginated/complete result), or (B) if only top-N is
intended, add an explicit UI/prop or comment in rate-limit-top-users.tsx that
this component intentionally shows the top 200; locate symbols
searchUsersForFilterRepository, searchUsers(), and setUsers to implement the
parameterized pagination or to add the explicit top-200 annotation/prop and
update types accordingly.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-filters.tsx:
- Around line 193-194: The export currently uses the applied prop `filters`
instead of the panel's in-edit state `localFilters`, causing exported CSVs to
reflect stale criteria; update the export call to use
`sanitizeFilters(localFilters)` and pass that to `startUsageLogsExport`, or
alternately disable the export button / show a warning when `localFilters`
differs from `filters` (compare the two before enabling export) so users cannot
export until they apply changes; touch the code around `exportFilters`,
`sanitizeFilters`, `startUsageLogsExport`, `filters` and `localFilters` to
implement the chosen fix.
In `@src/app/`[locale]/my-usage/_components/usage-logs-section.test.tsx:
- Around line 76-103: The test currently uses
mocks.useInfiniteQuery.mockReturnValue so the hook's queryFn never runs and
getMyUsageLogsBatch isn't exercised; change the test to mock useInfiniteQuery
with a mockImplementation that captures the queryFn argument, invokes it (e.g.,
await queryFn({ pageParam: undefined }) or queryFn({ pageParam: null }) to
simulate the initial fetch) so that getMyUsageLogsBatch is executed, then return
a realistic shape ({ data: { pages: [{ logs: [], nextCursor: null, hasMore:
false }] }, fetchNextPage: vi.fn(), hasNextPage: false, ... }) and finally
assert that getMyUsageLogsBatch was called and getMyUsageLogs was not called;
reference mocks.useInfiniteQuery, getMyUsageLogsBatch, and getMyUsageLogs when
updating the test.
In `@src/app/`[locale]/my-usage/_components/usage-logs-section.tsx:
- Around line 85-109: handleApply and handleReset unconditionally call
query.refetch(), which will re-fetch all cached pages and amplify load when the
user is deep-scrolled; update those handlers (handleApply and handleReset) to
avoid unconditional refetching by either: 1) comparing new filter values to
current appliedFilters and only calling query.reset() / query.refetch() when
they actually changed, or 2) short-circuiting with the isBrowsingHistory flag
before calling query.refetch(), or 3) invoking query.refetch({ refetchPage: ()
=> true }) (or a tighter refetchPage predicate) to control which pages are
refreshed; apply the change in the handlers that currently call query.refetch()
so useInfiniteQuery (query) isn’t forced to re-fetch all pages unnecessarily.
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 3081-3093: runAttempt is stripping
provider.firstByteTimeoutStreamingMs (setting it to 0) before calling
ProxyForwarder.doForward, which prevents doForward from enforcing per-attempt
first-byte deadlines; combined with the rectifier path that clears
thresholdTimer without re-arming it on retries, this lets stuck attempts hang
until transport timeouts. Fix by preserving and passing the original
provider.firstByteTimeoutStreamingMs into doForward (don’t force it to 0) or
explicitly propagate the intended deadline value, and update the rectifier retry
logic to re-install the thresholdTimer when retrying an attempt so each retry
honors the per-attempt first-byte timeout; check runAttempt,
ProxyForwarder.doForward, rectifier, firstByteTimeoutStreamingMs and
thresholdTimer (also adjust the identical logic around the other occurrence at
the 3251-3256 block).
In `@src/app/v1/_lib/proxy/session.ts`:
- Around line 745-748: The current early return in the ProxySession pricing path
treats a missing/failed billing config as "non-billable" by doing return null;
instead, change the logic in the method containing hasUsableBillingSettings
(class ProxySession) to use a fallback billing source when the primary config
read fails (e.g., detect a default/fallback source on this.billingSettings or
call a new resolvePricingWithFallback), so pricing resolution still produces a
cost record and allows usage/quotas to be charged; if no safe fallback exists,
propagate an explicit failure (fail-closed) to the caller rather than returning
null. Ensure you update any callers that expect null to handle the fallback
failure path and keep logger.warn("[ProxySession] Billing settings
unavailable…") but include which fallback was used.
In `@src/repository/usage-logs.ts`:
- Around line 582-588: The startTime/endTime branches use truthy checks and
silently ignore 0 values; update the conditions that push into ledgerConditions
to explicitly check for !== undefined (or !== null) instead of truthiness so
filters.startTime === 0 and filters.endTime === 0 are honored; locate the if
blocks referencing filters.startTime, filters.endTime and usageLedger.createdAt
and replace the truthy guards with explicit undefined checks to match the
earlier condition construction.
In `@src/repository/user.ts`:
- Around line 249-250: SQL 的 regexp_split_to_array 调用与 parseProviderGroups
的分隔语义不一致：regexp_split_to_array(... '\\s*[,，]+\\s*') 未包含换行符，导致 keyGroupFilters 基于
users.providerGroup 的匹配会漏掉以换行分隔的历史/导入数据。请修改该 SQL 表达式（在 user.ts 中使用
regexp_split_to_array 的位置）使分隔模式与 parseProviderGroups 一致，包含 '\n' 和
'\r'（并保留逗号与中文逗号的处理），确保 users.providerGroup 的拆分能匹配 parseProviderGroups 的行为，从而修复
keyGroupFilters 的漏匹配问题。
In `@tests/api/api-openapi-spec.test.ts`:
- Around line 249-252: The test currently only checks limitSchema?.maximum and
can pass if the limit property is missing; update the assertions to first assert
the presence of the limit schema (e.g., assert that schema?.properties?.limit or
limitSchema is defined) and only then assert that limitSchema?.maximum is
undefined so the test fails if the limit field is absent; reference the existing
symbols limitSchema and schema?.properties?.limit and keep the pageSchema
assertion as-is.
In `@tests/integration/billing-model-source.test.ts`:
- Around line 93-96: The beforeEach currently resets cloudPriceSyncRequests and
invalidates cache but does not clear Jest mock call histories; add mock clearing
to avoid accumulated calls affecting later tests by calling either
jest.clearAllMocks() or explicitly clearing updateMessageRequestCost.mockClear()
and RateLimitService.trackCost.mockClear() (or their mockReset() if you also
need to reset implementations) inside the same beforeEach so assertions on
updateMessageRequestCost.mock.calls.length and
RateLimitService.trackCost.mock.calls.length start from zero.
In `@tests/unit/dashboard-logs-export-progress-ui.test.tsx`:
- Line 9: The test enables fake timers and overwrites
globalThis.URL.createObjectURL, globalThis.URL.revokeObjectURL, and
HTMLAnchorElement.prototype.click but never restores them; add an afterEach that
calls vi.useRealTimers() and restores the original globals saved in beforeEach
(e.g., origCreateObjectURL, origRevokeObjectURL, origAnchorClick) and also calls
vi.restoreAllMocks() or vi.resetAllMocks() to fully clean up state so subsequent
tests (including worker tests) aren't polluted.
In `@tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts`:
- Around line 877-878: The tests set persistent mock implementations using
mocks.pickRandomProviderWithExclusion and mocks.categorizeErrorAsync returning
ProxyErrorCategory.NON_RETRYABLE_CLIENT_ERROR with mockResolvedValue(...), which
are not cleared by the current beforeEach that calls vi.clearAllMocks(); change
those calls to mockResolvedValueOnce(...) (for example on
mocks.pickRandomProviderWithExclusion and mocks.categorizeErrorAsync) so each
test gets its own one-off return, or alternatively update the test setup to use
vi.resetAllMocks() in the beforeEach so persistent mock implementations are
reset between tests.
---
Outside diff comments:
In `@src/app/api/actions/`[...route]/route.ts:
- Around line 1176-1235: You removed the public action name getMyUsageLogs and
replaced it with my-usage/getMyUsageLogsBatch causing breaking changes; restore
backward compatibility by re-registering the legacy route name
"my-usage/getMyUsageLogs" (using the same handler as getMyUsageLogsBatchHandler
or a thin adapter) and implement an adapter that converts legacy pagination
params (page, pageSize or limit/offset) into the new cursor shape (createdAt,id)
and limit before calling myUsageActions.getMyUsageLogsBatch; reference
getMyUsageLogsBatchRoute, getMyUsageLogsBatchHandler and
myUsageActions.getMyUsageLogsBatch when locating where to add the alias and
adapter logic.
In `@tests/unit/actions/usage-logs-export-retry-count.test.ts`:
- Line 1: The test file switches to fake timers but never restores real timers,
which can leak fake timer mode to other tests; add a teardown that calls
vi.useRealTimers() (e.g., in an afterEach or afterAll) so that any use of
vi.useFakeTimers() in tests is paired with vi.useRealTimers() and does not
affect other suites—place the call alongside existing beforeEach/describe/test
blocks and reference vitest's vi to restore timers.
---
Nitpick comments:
In @.github/workflows/dev.yml:
- Around line 65-69: The workflow step "Install dependencies, type check, and
format code" currently runs "bun install", "bun run typecheck", and "bun run
format"; optionally add "bun run lint" (per CLAUDE.md guidance) to that step to
run linting in CI—insert the "bun run lint" command in the sequence (e.g., after
"bun run typecheck" and before "bun run format") so the job runs install →
typecheck → lint → format.
In `@messages/ja/settings/providers/form/sections.json`:
- Around line 104-107: The placeholder currently only shows an HTTP example
while the formats string documents support for socks4/5; update the
"placeholder" value to include a SOCKS example (e.g., add a socks5://... or
socks4://... example) so it matches the "formats" entry—locate the "placeholder"
key in the same object that contains "formats", "label", and "optional" and
amend its text to include a SOCKS example.
In `@src/actions/usage-logs.ts`:
- Around line 230-242: The progress callback passed into buildUsageLogsExportCsv
currently reads and writes usageLogsExportStatusStore on every batch (using
jobId), causing heavy Redis load for large exports; modify the callback to
throttle updates by aggregating progress and only calling
usageLogsExportStatusStore.get/set when a threshold is reached (e.g., every N
batches or every N seconds), track the lastSentTime/lastSentBatch and
pendingProgress inside the scope where the callback is created, ensure you still
send a final update when export completes, and keep references to jobId and
usageLogsExportStatusStore so you update the same job record.
In `@src/app/`[locale]/dashboard/_components/user/forms/add-key-form.tsx:
- Around line 127-135: handleProviderGroupChange currently only normalizes input
when there are multiple groups including PROVIDER_GROUP.DEFAULT, but otherwise
writes the raw newValue back; change it to always use the normalized array from
parseProviderGroups before calling form.setValue so stored providerGroup is
consistent. Specifically, call parseProviderGroups(newValue) into groups,
compute the joined string (e.g., groups.join(",")) but if groups contains
PROVIDER_GROUP.DEFAULT and groups.length>1 filter it out first, then call
form.setValue("providerGroup", normalizedString); update the logic inside
handleProviderGroupChange to always derive the value from parseProviderGroups
rather than using newValue directly.
In `@src/app/`[locale]/dashboard/_components/user/forms/edit-key-form.tsx:
- Around line 186-193: The handler currently parses groups via
parseProviderGroups(newValue) but in the else branch writes the original
newValue back, causing inconsistent formatting; always normalize by using the
parsed groups array (filtered as needed) and set form.setValue("providerGroup",
parsedGroups.join(",")) so both branches write the canonical string—use
parseProviderGroups, PROVIDER_GROUP.DEFAULT, and form.setValue("providerGroup",
...) to locate and update the logic.
- Around line 265-275: The onInvalidTag handler contains mappings for reasons
like "invalid_format" and "too_long" while validateTag is hardcoded to always
return true; either remove the unreachable mapping keys from the messages object
(so messages only contains errors that can actually occur) or explicitly
document why those keys are kept (add a comment near validateTag and
onInvalidTag explaining they are intentionally preserved for future custom
validation). Update references to validateTag and onInvalidTag accordingly so
the code and comments stay consistent.
In `@src/app/`[locale]/dashboard/_components/user/forms/user-form.tsx:
- Around line 220-230: The onInvalidTag handler is likely dead because
validateTag={() => true} always returns true; either remove onInvalidTag to
avoid dead code or implement real validation in validateTag and keep
onInvalidTag for fallback behavior. Concretely, either (A) delete the
onInvalidTag block and its messages/toast usage from the TagInputField props
(references: validateTag, onInvalidTag, TagInputField, tUI, toast.error), or (B)
replace validateTag={() => true} with a proper validator that checks
empty/duplicate/too_long/invalid_format/max_tags (using the same message keys)
so that TagInputField will call onInvalidTag when validation fails; if you
choose to keep the always-true validator, add a short comment above validateTag
explaining it intentionally disables onInvalidTag to make intent explicit.
In `@src/app/`[locale]/dashboard/rate-limits/_components/rate-limit-filters.tsx:
- Around line 88-94: The providers loading effect (React.useEffect) should use
the same cancellation guard pattern as the users loader to avoid updating state
after unmount: inside the effect declare a local let cancelled = false, call
getProviders().then(providerList => { if (!cancelled) {
setProviders(providerList); setLoadingProviders(false); } }); and return a
cleanup function that sets cancelled = true; reference the existing
getProviders, setProviders, setLoadingProviders and the React.useEffect to
locate where to add this guard.
In `@src/app/`[locale]/dashboard/users/users-page-client.tsx:
- Around line 37-43: 注释中描述 splitTags 为“逗号分割标签”已不正确；当前实现直接复用
parseProviderGroups。请更新 splitTags 上方的注释把“逗号分割”改为说明“复用共享分组解析器 parseProviderGroups
以返回已修剪且非空的标签数组”，并可提及这与服务端 providerGroup 解析保持一致（引用函数名 splitTags 和
parseProviderGroups 以便定位）。
In `@src/app/`[locale]/my-usage/_components/usage-logs-table.test.tsx:
- Around line 89-125: The current tests use renderToStaticMarkup so interactive
paths (retry/button copy/onLoadMore) aren’t exercised; add at least one
DOM-enabled test using happy-dom (or `@testing-library/react` with a happy-dom
environment) that mounts UsageLogsTable, simulates user events (click the retry
control to assert onLoadMore is called and click the model-copy button to assert
clipboard write or a callback/tooltip appears), and verifies the UI updates or
callbacks fire; target the UsageLogsTable component and its props (logs,
onLoadMore, errorMessage) when adding the new interaction test.
In `@src/app/`[locale]/my-usage/_components/usage-logs-table.tsx:
- Around line 143-149: The error retry UI is rendered twice when both
errorMessage and onLoadMore exist; update the component so only the status-bar
location (the branch that renders {errorMessage && onLoadMore ? ...} around the
status row) shows the Button, and remove the duplicate retry UI from the loader
row (the alternate branch that renders the loader row error+Button). Concretely,
keep the existing conditional that renders the Button next to the status text
(references: errorMessage, onLoadMore) and delete or disable the duplicate
Button/Span in the loader-row rendering (the loader row JSX that currently also
checks errorMessage/onLoadMore) so the retry control appears only once.
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form.legacy.tsx:
- Line 889: The validateTag prop currently returns true unconditionally which
bypasses any custom format validation and leaves only the built-in
duplicate/empty/too_long checks and the separate total-length check in the form
submission; change validateTag on the ProviderForm component to implement the
intended custom validation (or remove it to rely solely on built-ins) and ensure
onInvalidTag is still triggered for custom failures—locate the validateTag={()
=> true} usage and replace it with the proper validation function that checks
your required tag format (or document that intentional bypass) and keep the
existing total-length check behavior in the submit flow.
In `@src/app/v1/_lib/proxy/provider-selector.ts`:
- Around line 51-53: parseGroupString is now a trivial passthrough to
parseProviderGroups; remove the wrapper to reduce indirection and update callers
to call parseProviderGroups directly. Delete the parseGroupString function from
provider-selector.ts, replace any references to parseGroupString with
parseProviderGroups, and adjust imports/exports so parseProviderGroups is
exported/available where needed (or re-export it if the module API must remain
unchanged).
In `@src/components/ui/__tests__/provider-group-tag-input.test.tsx`:
- Around line 1-10: The test file combines two component specs (TagInput and
ProviderGroupSelect) in a shared ui __tests__ folder; split them into two
source-adjacent tests: create provider-group-select.test.tsx next to the
ProviderGroupSelect component and tag-input.test.tsx next to the TagInput
component, copy the relevant tests into their respective files, update imports
inside each new test to reference the local component path and keep the top-line
vitest environment and any setup/teardown (act, createRoot, vi) needed, and
remove the original combined test file so each component’s tests live beside its
source (ensuring any shared mocks/helpers are moved or re-exported as needed).
In `@src/types/css.d.ts`:
- Line 1: The current module declaration declare module "*.css"; is acceptable
but to make types more explicit replace it with a typed CSS module declaration:
declare module "*.css" { const classes: { [key: string]: string }; export
default classes; } — update the module declaration (the "*.css" module) to
export a mapping of class names to strings (or alternatively export const style
names) so imports like import styles from "./x.css" have proper typing.
🪄 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: 7dd2c3ee-1a75-4660-9092-4b2212d5cdce
📒 Files selected for processing (110)
.dockerignore.github/workflows/dev.yml.github/workflows/release.ymlCLAUDE.mddrizzle/0085_busy_ken_ellis.sqldrizzle/meta/0085_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/dashboard.jsonmessages/en/settings/config.jsonmessages/en/settings/providers/form/sections.jsonmessages/ja/dashboard.jsonmessages/ja/settings/config.jsonmessages/ja/settings/providers/form/sections.jsonmessages/ru/dashboard.jsonmessages/ru/settings/config.jsonmessages/ru/settings/providers/form/sections.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/config.jsonmessages/zh-CN/settings/providers/form/sections.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/config.jsonmessages/zh-TW/settings/providers/form/sections.jsonpackage.jsonsrc/actions/keys.tssrc/actions/my-usage.tssrc/actions/providers.tssrc/actions/request-filters.tssrc/actions/system-config.tssrc/actions/usage-logs.tssrc/actions/users.tssrc/app/[locale]/dashboard/_components/rate-limit-top-users.tsxsrc/app/[locale]/dashboard/_components/user/forms/add-key-form.tsxsrc/app/[locale]/dashboard/_components/user/forms/edit-key-form.tsxsrc/app/[locale]/dashboard/_components/user/forms/key-edit-section.tsxsrc/app/[locale]/dashboard/_components/user/forms/provider-group-select.tsxsrc/app/[locale]/dashboard/_components/user/forms/user-form.tsxsrc/app/[locale]/dashboard/_components/user/key-row-item.tsxsrc/app/[locale]/dashboard/_components/user/user-key-table-row.tsxsrc/app/[locale]/dashboard/_components/user/utils/provider-group.tssrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-filters.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/[locale]/dashboard/rate-limits/_components/rate-limit-filters.tsxsrc/app/[locale]/dashboard/users/users-page-client.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.test.tsxsrc/app/[locale]/my-usage/_components/usage-logs-table.tsxsrc/app/[locale]/my-usage/page.tsxsrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/app/[locale]/settings/providers/_components/batch-edit/analyze-batch-settings.tssrc/app/[locale]/settings/providers/_components/batch-edit/provider-batch-toolbar.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form.legacy.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsxsrc/app/[locale]/settings/providers/_components/provider-manager.tsxsrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/app/api/actions/[...route]/route.tssrc/app/api/admin/system-config/route.tssrc/app/v1/_lib/models/available-models.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session.tssrc/components/form/form-field.tsxsrc/components/ui/__tests__/provider-group-tag-input.test.tsxsrc/drizzle/schema.tssrc/hooks/use-virtualized-infinite-list.tssrc/lib/config/index.tssrc/lib/config/system-settings-cache.tssrc/lib/provider-patch-contract.tssrc/lib/request-filter-engine.tssrc/lib/utils/provider-group.test.tssrc/lib/utils/provider-group.tssrc/lib/utils/special-settings.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.test.tssrc/repository/_shared/transformers.tssrc/repository/leaderboard.tssrc/repository/provider.tssrc/repository/system-config.tssrc/repository/usage-logs.tssrc/repository/user.tssrc/types/css.d.tssrc/types/special-settings.tssrc/types/system-config.tstests/api/api-actions-integrity.test.tstests/api/api-endpoints.test.tstests/api/api-openapi-spec.test.tstests/api/my-usage-readonly.test.tstests/api/users-search-users-compat.test.tstests/integration/billing-model-source.test.tstests/unit/actions/my-usage-date-range-dst.test.tstests/unit/actions/system-config-save.test.tstests/unit/actions/usage-logs-export-retry-count.test.tstests/unit/app/v1/url.test.tstests/unit/dashboard-logs-export-progress-ui.test.tsxtests/unit/lib/config/system-settings-cache.test.tstests/unit/lib/utils/special-settings.test.tstests/unit/models/available-models-gemini-key.test.tstests/unit/proxy/available-models.test.tstests/unit/proxy/proxy-forwarder-hedge-first-byte.test.tstests/unit/proxy/session.test.tstests/unit/repository/system-config-update-missing-columns.test.tstests/unit/repository/usage-logs-sessionid-filter.test.tstests/unit/users-action-get-users-compat.test.tstests/unit/users-action-search-users-for-filter.test.tstsconfig.json
src/app/[locale]/dashboard/_components/rate-limit-top-users.tsx
Outdated
Show resolved
Hide resolved
src/app/[locale]/dashboard/logs/_components/usage-logs-filters.tsx
Outdated
Show resolved
Hide resolved
![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 is a release PR (v0.6.6) combining 9 feature and bugfix PRs into main. The changes span proxy reliability, UI stability, security hardening, billing configuration, and tooling upgrades.
PR Size: XL
- Lines changed: 10,533 (8,965 additions, 1,568 deletions)
- Files changed: 110
Key Changes Reviewed
| Area | Change | Assessment |
|---|---|---|
| Cursor Pagination | Migration from page/pageSize to cursor-based pagination for usage logs | Well-implemented with proper fallback to ledger mode |
| Legacy API Compat | getUsersBatch/searchUsers with backward compat for page/offset/query/keyword | Properly implemented with parameter normalization |
| Unicode Provider Groups | Support for Chinese commas and newlines as separators | Clean implementation with reusable splitProviderGroupValue |
| Readonly Key Security | Blocks fullKey exposure for canLoginWebUi=false sessions | Properly implemented and tested |
| Proxy Pipeline | Reactive rectifiers during streaming hedge | Well-structured refactor with shared helper |
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 |
Notable Positive Observations
- Security Fix Validated: The readonly key security fix properly ensures
fullKeyis undefined andcanCopyis false for sessions withcanLoginWebUi=false - Test Coverage: New test files exist for key features:
tests/api/users-search-users-compat.test.ts- Legacy API compatibilitytests/api/my-usage-readonly.test.ts- Updated for cursor pagination
- Backward Compatibility: Legacy parameters (page/offset/query/keyword) are properly normalized in the new batch endpoints
- Error Handling: All catch blocks properly log errors and return user-friendly messages
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Claude AI
src/app/v1/_lib/proxy/forwarder.ts
Outdated
| try { | ||
| attemptRuntime.responseController?.abort(new Error("hedge_loser")); | ||
| } catch { | ||
| // ignore |
Contributor
There was a problem hiding this comment.
[Critical] [ERROR-SILENT] Hedge cleanup swallows abort/cancel failures
Why this is a problem: In the hedged streaming path, abort() / response.body.cancel() failures are currently swallowed (catch {} and .catch(() => {})). If this starts failing (invalid state, already-closed stream, etc.), we lose the signal that hedging cleanup is breaking, which makes leaked readers / stuck streams much harder to debug in the proxy’s most critical path.
Suggested fix:
try {
attemptRuntime.responseController?.abort(new Error("hedge_loser"));
} catch (abortError) {
logger.debug("ProxyForwarder: hedge loser abort failed", {
error: abortError instanceof Error ? abortError.message : String(abortError),
sessionId: attempt.session.sessionId ?? null,
providerId: attempt.provider.id,
providerName: attempt.provider.name,
});
}
const cancelPromise = response.body?.cancel("hedge_loser");
cancelPromise?.catch((cancelError) => {
logger.debug("ProxyForwarder: hedge loser body cancel failed", {
error: cancelError instanceof Error ? cancelError.message : String(cancelError),
sessionId: attempt.session.sessionId ?? null,
providerId: attempt.provider.id,
providerName: attempt.provider.name,
});
});
| } | ||
| }) | ||
| .catch(() => { | ||
| if (!cancelled) { |
Contributor
There was a problem hiding this comment.
[High] [ERROR-SILENT] User list load failures are swallowed (renders as empty data)
Why this is a problem: The current catch(() => { ...setUsers([]) }) converts a real failure (network/server/runtime) into a “no data” UI state with no signal. That hides outages/regressions and can mislead admins.
Suggested fix (log + show an i18n error state instead of silently falling back):
const [loadError, setLoadError] = React.useState(false);
void searchUsers()
.then((result) => {
if (!cancelled) {
setUsers(result.ok ? result.data : []);
setLoadError(!result.ok);
}
})
.catch((error) => {
console.error("RateLimitTopUsers: failed to load users", error);
if (!cancelled) {
setUsers([]);
setLoadError(true);
}
});
// Render: loadError ? t("loadError") : ...
| } | ||
| }) | ||
| .catch(() => { | ||
| if (!cancelled) { |
Contributor
There was a problem hiding this comment.
[High] [ERROR-SILENT] User list load failures are swallowed (renders as empty data)
Why this is a problem: catch(() => setUsers([])) turns an actual failure into an empty-result UI state. That hides outages and makes debugging much harder.
Suggested fix (capture the error, log it, and surface an i18n error state):
const [loadError, setLoadError] = React.useState(false);
void searchUsers()
.then((result) => {
if (!cancelled) {
setUsers(result.ok ? result.data : []);
setLoadError(!result.ok);
}
})
.catch((error) => {
console.error("RateLimitFilters: failed to load users", error);
if (!cancelled) {
setUsers([]);
setLoadError(true);
}
});
// Render: loadError ? t("loadError") : ...
| if (!startResult.ok) { | ||
| setExportStatus(null); | ||
| toast.error(startResult.error || t("logs.filters.exportError")); | ||
| return; |
Contributor
There was a problem hiding this comment.
[High] [STANDARD-VIOLATION] User-facing errors bypass i18n (raw server strings in toasts)
Why this is a problem: This displays startResult.error directly to the user. The related server actions return hardcoded message strings (mixed languages) and don’t consistently provide errorCode, so non-zh locales will see untranslated error text. It also skips the existing error-code i18n flow (ActionResult.errorCode + getErrorMessage).
Suggested fix (prefer i18n via errorCode, fall back to a translated generic message):
const tErrors = useTranslations("errors");
const message = startResult.errorCode
? getErrorMessage(tErrors, startResult.errorCode, startResult.errorParams)
: t("logs.filters.exportError");
toast.error(message);Server-side: set errorCode for these failures (e.g. ERROR_CODES.UNAUTHORIZED, ERROR_CODES.NOT_FOUND, ERROR_CODES.OPERATION_FAILED) and reserve raw error strings for logs/debug.
Contributor
There was a problem hiding this comment.
Code Review Summary
XL PR (~10.5k lines across 110 files) that touches critical proxy streaming/hedging logic and multiple dashboard flows. Highest risk items in the diff are swallowed errors in new/modified async paths and user-facing toasts that display raw server error strings (bypassing the existing i18n error-code system).
PR Size: XL
- Lines changed: 10533
- Files changed: 110
- Split suggestions:
-
- Proxy hedging/forwarding changes (
src/app/v1/_lib/proxy/*+ related tests)
- Proxy hedging/forwarding changes (
-
- Usage logs export pipeline + UI (
src/actions/usage-logs.ts,src/app/[locale]/dashboard/logs/*,tests/unit/dashboard-logs-export-progress-ui.test.tsx)
- Usage logs export pipeline + UI (
-
- System settings + migration (
src/repository/system-config.ts,src/app/v1/_lib/proxy/session.ts,drizzle/*)
- System settings + migration (
-
- UI/provider grouping + translations (
src/app/[locale]/**,src/lib/utils/provider-group.ts,messages/**)
- UI/provider grouping + translations (
-
- CI workflow updates (
.github/workflows/*)
- CI workflow updates (
-
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 1 | 2 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
src/app/v1/_lib/proxy/forwarder.ts:3099[Critical] [ERROR-SILENT] Hedge cleanup swallows abort/cancel failures (Confidence: 97)
High Priority Issues (Should Fix)
src/app/[locale]/dashboard/_components/rate-limit-top-users.tsx:47[High] [ERROR-SILENT] Swallowed user-load failures render as empty data (Confidence: 88)src/app/[locale]/dashboard/rate-limits/_components/rate-limit-filters.tsx:72[High] [ERROR-SILENT] Swallowed user-load failures render as empty data (Confidence: 88)src/app/[locale]/dashboard/logs/_components/usage-logs-filters.tsx:201[High] [STANDARD-VIOLATION] Raw server error strings shown in toasts (i18n bypass) (Confidence: 90)
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.
Reviewed PR #968 (release v0.6.6), calculated size as XL, and applied the size/XL label.
Posted inline review comments on diff lines for the highest-confidence issues:
src/app/v1/_lib/proxy/forwarder.ts:3099— swallowed abort/cancel failures in hedge cleanup (catch {}/.catch(() => {}))src/app/[locale]/dashboard/_components/rate-limit-top-users.tsx:47— swallowed user-load failures render as empty datasrc/app/[locale]/dashboard/rate-limits/_components/rate-limit-filters.tsx:72— same swallowed user-load failure patternsrc/app/[locale]/dashboard/logs/_components/usage-logs-filters.tsx:201— raw server error strings shown in toasts (bypasses i18n error-code flow)
Submitted the required PR review summary (including XL split suggestions) via gh pr review.
10 tasks
* fix: address review feedback for release v0.6.6 Amp-Thread-ID: https://ampcode.com/threads/T-019d18a9-9c99-7379-879c-3f50eb6612fe Co-authored-by: Amp <amp@ampcode.com> * fix: address remaining review nits for PR 968 Amp-Thread-ID: https://ampcode.com/threads/T-019d18a9-9c99-7379-879c-3f50eb6612fe Co-authored-by: Amp <amp@ampcode.com> * fix: address follow-up review findings on PR 970 Amp-Thread-ID: https://ampcode.com/threads/T-019d18a9-9c99-7379-879c-3f50eb6612fe Co-authored-by: Amp <amp@ampcode.com> * fix: address second-round PR review feedback Amp-Thread-ID: https://ampcode.com/threads/T-019d18a9-9c99-7379-879c-3f50eb6612fe Co-authored-by: Amp <amp@ampcode.com> --------- Co-authored-by: Amp <amp@ampcode.com>
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Comment on lines
+169
to
+175
| }; | ||
| } | ||
|
|
||
| async function buildUsageLogsExportCsv( | ||
| filters: Omit<UsageLogFilters, "page" | "pageSize">, | ||
| onProgress?: ( | ||
| progress: Pick<UsageLogsExportStatus, "processedRows" | "totalRows" | "progressPercent"> |
There was a problem hiding this comment.
findUsageLogsWithDetails({ ...filters, page: 1, pageSize: 1 }) executes a full COUNT(*) with all the multi-table JOINs and returns one actual data row, purely to seed the progress-bar estimate. findUsageLogsStats is purpose-built for counting and is already called as the fallback — calling it first (or directly) would be lighter-weight:
Suggested change
| }; | |
| } | |
| async function buildUsageLogsExportCsv( | |
| filters: Omit<UsageLogFilters, "page" | "pageSize">, | |
| onProgress?: ( | |
| progress: Pick<UsageLogsExportStatus, "processedRows" | "totalRows" | "progressPercent"> | |
| const stats = await findUsageLogsStats(filters); | |
| let estimatedTotalRows = stats.totalRequests; |
The existing if (estimatedTotalRows === 0) fallback block below can then be removed. The actual export data is unaffected (it uses findUsageLogsBatch), so only the initial progress estimate changes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/usage-logs.ts
Line: 169-175
Comment:
**Redundant initial query just for count estimation**
`findUsageLogsWithDetails({ ...filters, page: 1, pageSize: 1 })` executes a full COUNT(*) with all the multi-table JOINs and returns one actual data row, purely to seed the progress-bar estimate. `findUsageLogsStats` is purpose-built for counting and is already called as the fallback — calling it first (or directly) would be lighter-weight:
```suggestion
const stats = await findUsageLogsStats(filters);
let estimatedTotalRows = stats.totalRequests;
```
The existing `if (estimatedTotalRows === 0)` fallback block below can then be removed. The actual export data is unaffected (it uses `findUsageLogsBatch`), so only the initial progress estimate changes.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+227
to
+233
| argsMapper: (body) => { | ||
| const searchTerm = [body.searchTerm, body.query, body.keyword] | ||
| .map((value: string | undefined) => value?.trim()) | ||
| .find((value): value is string => Boolean(value)); | ||
|
|
||
| return [searchTerm]; | ||
| }, |
There was a problem hiding this comment.
limit field silently dropped by argsMapper
The route schema uses .passthrough(), which allows a caller to include a limit field in the request body, and the underlying searchUsers action accepts one. However, the argsMapper only extracts searchTerm and returns [searchTerm] — limit is never forwarded. Callers expecting to control result-set size through this endpoint will always get the 200-row default.
If that cap is intentional for the legacy compatibility endpoint, a brief comment would help future readers understand why limit is omitted:
Suggested change
| argsMapper: (body) => { | |
| const searchTerm = [body.searchTerm, body.query, body.keyword] | |
| .map((value: string | undefined) => value?.trim()) | |
| .find((value): value is string => Boolean(value)); | |
| return [searchTerm]; | |
| }, | |
| argsMapper: (body) => { | |
| const searchTerm = [body.searchTerm, body.query, body.keyword] | |
| .map((value: string | undefined) => value?.trim()) | |
| .find((value): value is string => Boolean(value)); | |
| // Limit is intentionally omitted for the legacy compat endpoint; | |
| // callers needing a custom limit should use searchUsersForFilter directly. | |
| return [searchTerm]; | |
| }, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/actions/[...route]/route.ts
Line: 227-233
Comment:
**`limit` field silently dropped by `argsMapper`**
The route schema uses `.passthrough()`, which allows a caller to include a `limit` field in the request body, and the underlying `searchUsers` action accepts one. However, the `argsMapper` only extracts `searchTerm` and returns `[searchTerm]` — `limit` is never forwarded. Callers expecting to control result-set size through this endpoint will always get the 200-row default.
If that cap is intentional for the legacy compatibility endpoint, a brief comment would help future readers understand why `limit` is omitted:
```suggestion
argsMapper: (body) => {
const searchTerm = [body.searchTerm, body.query, body.keyword]
.map((value: string | undefined) => value?.trim())
.find((value): value is string => Boolean(value));
// Limit is intentionally omitted for the legacy compat endpoint;
// callers needing a custom limit should use searchUsersForFilter directly.
return [searchTerm];
},
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/actions/users.ts (1)
488-516:⚠️ Potential issue | 🟠 Major
searchUsers()在省略limit时会静默截断大实例的用户列表。这里会落到仓储层默认的 200 条上限，但这个 PR 里的新调用点已经把
searchUsers()当成通用“加载用户选项”的接口用了。用户数一旦超过 200，后面的账号就不会出现在筛选/选择 UI 中，而且没有任何提示。建议把limit变成显式参数，或至少在空searchTerm时使用更高的默认值。Also applies to: 524-529
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/users.ts` around lines 488 - 516, The function searchUsersForFilter currently allows limit to be omitted which causes silent truncation by the repository default (200); update searchUsersForFilter (and the analogous call around lines 524-529) to make limit explicit or to apply a larger default when searchTerm is empty: either change the function signature to require limit, or compute an effectiveLimit = limit ?? (searchTerm ? normalizeSearchUsersLimit(limit) : LARGER_DEFAULT) and pass that to searchUsersForFilterRepository; ensure you use the existing helper normalizeSearchUsersLimit (or replace it with a new normalizeWithEmptySearchDefault) and, if possible, surface/log when results are truncated.tests/api/api-actions-integrity.test.ts (1)
147-156:⚠️ Potential issue | 🟡 Minor把
my-usage也纳入标签归类断言。这里新增了
getMyUsageLogsBatch，但本文件后面的moduleTagMapping仍然没有/api/actions/my-usage/前缀，所以这组路径现在只校验“存在”，不校验 tag 是否挂到了正确分组。OpenAPI 一旦误标到别的标签，这个测试仍会放过。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/api/api-actions-integrity.test.ts` around lines 147 - 156, The test lists new endpoints under "/api/actions/my-usage/..." but the moduleTagMapping used later doesn't include the "/api/actions/my-usage/" prefix, so the new getMyUsageLogsBatch entry only checks existence and not correct tag grouping; update moduleTagMapping in tests/api/api-actions-integrity.test.ts to include a mapping key for the "/api/actions/my-usage/" prefix (or change the existing my-usage entry to use the full "/api/actions/my-usage/" prefix) and ensure the tag asserted for that key matches the intended "my-usage" tag so the test verifies endpoints are grouped under the correct tag.
🧹 Nitpick comments (1)
src/app/v1/_lib/proxy/forwarder.ts (1)
3710-3718:specialSettings合并逻辑改为去重而非覆盖，避免丢失审计记录。使用
JSON.stringify作为去重 key 是实用的做法。但需注意：如果同一 setting 对象在不同代码路径以不同属性顺序创建，可能导致重复。目前的 setting 构造都在固定代码路径中完成，暂无此风险。如果后续出现属性顺序不一致的场景，可考虑改用基于type+providerId+attemptNumber等关键字段的组合 key。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/forwarder.ts` around lines 3710 - 3718, 当前对 specialSettings 的去重使用 JSON.stringify 可能在属性顺序不一致时误判重复；在合并 sourceState.specialSettings 到 targetState.specialSettings 时，请改为用确定性的组合 key（例如 setting.type + '|' + setting.providerId + '|' + setting.attemptNumber 或其他唯一标识字段）来构建 existingKeys，而非 JSON.stringify，保持变量名 existingKeys、merged、targetState.specialSettings 和 sourceState.specialSettings 不变以便定位；如果某些字段可能不存在，请在构建 key 时提供安全默认值。
🤖 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/actions/my-usage.ts`:
- Around line 509-541: In mapMyUsageLogEntries, add the per-request audit fields
to each MyUsageLogEntry: include an actualServiceTier field (use
log.actualServiceTier if present, fallback to log.serviceTier, else null) and a
cacheHitPercentage field (compute as null when both cacheReadInputTokens and
cacheCreationInputTokens are null or sum to 0; otherwise set to
(cacheReadInputTokens / (cacheReadInputTokens + cacheCreationInputTokens)) * 100
as a Number). Update the returned object in mapMyUsageLogEntries to populate
these fields (ensure types default to null when missing) and then sync the
MyUsageLogEntry response schema and any UI/rendering layers to consume
actualServiceTier and cacheHitPercentage.
In `@src/actions/users.ts`:
- Around line 149-168: The loadAllUsersForAdmin loop can spin indefinitely if
findUserListBatch keeps returning hasMore=true with an unchanged nextCursor;
update the loadAllUsersForAdmin function to break/throw when progress stalls by
checking if page.nextCursor === cursor and stop the loop, and also add an
iteration cap (like getUsersWithQuotas) e.g., a MAX_ITERATIONS constant and
increment a counter each loop to abort after the limit; ensure you propagate or
log an explanatory error (or return partial results) when aborting so callers of
loadAllUsersForAdmin can handle the failure.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-filters.tsx:
- Around line 236-239: The code currently passes the raw server error
(statusResult.data.error) directly to toast.error when statusResult.data.status
=== "failed"; replace that with a localized message resolution: call
getErrorMessage(statusResult.data.errorCode) first (if errorCode exists), fall
back to t("logs.filters.exportError"), and pass the resulting string into
toast.error (still optionally append the raw server message only for debugging
if needed). Update the failure branch that references statusResult,
statusResult.data.error, toast.error and t to use this resolved message instead
of the unlocalized server string.
In `@src/app/`[locale]/dashboard/quotas/users/page.tsx:
- Around line 21-44: The loop currently silently truncates at
MAX_USERS_FOR_QUOTAS (2000) without signaling truncation; modify the logic
around collectedUsers, getUsersBatch, and result.data.hasMore so that when
collectedUsers.length >= MAX_USERS_FOR_QUOTAS and the latest batch's
result.data.hasMore is true you set a clear truncated flag (e.g., isTruncated =
true) or attach a "truncated" marker to the returned payload instead of only
relying on iterations/MAX_ITERATIONS; implement this by checking after pushing
result.data.users whether collectedUsers.length >= MAX_USERS_FOR_QUOTAS &&
result.data.hasMore and then break the loop and surface that flag (or log a
console.warn with that context) so callers/UI can show “results truncated”
rather than silently dropping users.
In `@src/app/`[locale]/my-usage/_components/usage-logs-section.tsx:
- Around line 167-180: When applying or resetting filters, also clear the
browsing-history flag so refetchInterval isn't permanently disabled: in
handleApply (around the logic that compares and calls
setAppliedFilters(nextFilters)) call setIsBrowsingHistory(false) after updating
applied filters (or before returning early if no change), and in handleReset
(after setDraftFilters({}) and when you call setAppliedFilters({})) call
setIsBrowsingHistory(false) as well; locate the handlers handleApply and
handleReset and the state setter setIsBrowsingHistory to make these changes.
In `@src/app/v1/_lib/proxy/session.ts`:
- Around line 745-750: The warning branch is dead because
hasUsableBillingSettings() inspects cachedBillingModelSource which
loadBillingSettings() always sets to "original" or "redirected"; change the
condition to detect when the code is using the fallback billing settings by
checking billingSettingsSource instead of cachedBillingModelSource. Update the
if in the ProxySession session code to use this.billingSettingsSource (e.g., if
(this.billingSettingsSource !== "original" && this.billingSettingsSource !==
"redirected") or compare against the known default/fallback string) so the
logger.warn runs when loadBillingSettings() fell back; keep the same log payload
(billingSettingsSource and cachedBillingModelSource) and leave the
hasUsableBillingSettings() implementation unchanged.
In `@tests/unit/dashboard-logs-export-progress-ui.test.tsx`:
- Around line 200-237: The test name ("exports the applied filters instead of
unapplied local draft filters") conflicts with the assertion which expects
startUsageLogsExportMock to be called with the draft sessionId ("draft-session")
while the component is rendered with applied filters ({ sessionId:
"applied-session" }); update either the test name to indicate it verifies
exporting draft/local filters or change the test to expect the applied filters
by modifying the rendered UsageLogsFilters props or the final expect call;
specifically, for the current assertion keep the test name referencing "draft
filters" or change expect(startUsageLogsExportMock).toHaveBeenCalledWith to
expect { sessionId: "applied-session" } to match the test title and the filters
prop passed to UsageLogsFilters.
---
Outside diff comments:
In `@src/actions/users.ts`:
- Around line 488-516: The function searchUsersForFilter currently allows limit
to be omitted which causes silent truncation by the repository default (200);
update searchUsersForFilter (and the analogous call around lines 524-529) to
make limit explicit or to apply a larger default when searchTerm is empty:
either change the function signature to require limit, or compute an
effectiveLimit = limit ?? (searchTerm ? normalizeSearchUsersLimit(limit) :
LARGER_DEFAULT) and pass that to searchUsersForFilterRepository; ensure you use
the existing helper normalizeSearchUsersLimit (or replace it with a new
normalizeWithEmptySearchDefault) and, if possible, surface/log when results are
truncated.
In `@tests/api/api-actions-integrity.test.ts`:
- Around line 147-156: The test lists new endpoints under
"/api/actions/my-usage/..." but the moduleTagMapping used later doesn't include
the "/api/actions/my-usage/" prefix, so the new getMyUsageLogsBatch entry only
checks existence and not correct tag grouping; update moduleTagMapping in
tests/api/api-actions-integrity.test.ts to include a mapping key for the
"/api/actions/my-usage/" prefix (or change the existing my-usage entry to use
the full "/api/actions/my-usage/" prefix) and ensure the tag asserted for that
key matches the intended "my-usage" tag so the test verifies endpoints are
grouped under the correct tag.
---
Nitpick comments:
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 3710-3718: 当前对 specialSettings 的去重使用 JSON.stringify
可能在属性顺序不一致时误判重复；在合并 sourceState.specialSettings 到 targetState.specialSettings
时，请改为用确定性的组合 key（例如 setting.type + '|' + setting.providerId + '|' +
setting.attemptNumber 或其他唯一标识字段）来构建 existingKeys，而非 JSON.stringify，保持变量名
existingKeys、merged、targetState.specialSettings 和 sourceState.specialSettings
不变以便定位；如果某些字段可能不存在，请在构建 key 时提供安全默认值。
🪄 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: 1ecd542b-a031-495e-bf48-b26d80b256a6
📒 Files selected for processing (32)
.github/workflows/dev.ymlmessages/ja/settings/providers/form/sections.jsonsrc/actions/my-usage.tssrc/actions/usage-logs.tssrc/actions/users.tssrc/app/[locale]/dashboard/_components/rate-limit-top-users.tsxsrc/app/[locale]/dashboard/_components/user/forms/add-key-form.tsxsrc/app/[locale]/dashboard/_components/user/forms/edit-key-form.tsxsrc/app/[locale]/dashboard/_components/user/forms/user-form.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-filters.tsxsrc/app/[locale]/dashboard/quotas/users/page.tsxsrc/app/[locale]/dashboard/rate-limits/_components/rate-limit-filters.tsxsrc/app/[locale]/dashboard/users/users-page-client.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/api/actions/[...route]/route.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/session.tssrc/repository/usage-logs.tssrc/repository/user.tstests/api/api-actions-integrity.test.tstests/api/api-openapi-spec.test.tstests/integration/billing-model-source.test.tstests/unit/actions/my-usage-date-range-dst.test.tstests/unit/actions/usage-logs-export-retry-count.test.tstests/unit/dashboard-logs-export-progress-ui.test.tsxtests/unit/proxy/proxy-forwarder-hedge-first-byte.test.tstests/unit/proxy/session.test.tstests/unit/repository/usage-logs-slim-pagination.test.tstests/unit/users-action-search-users-for-filter.test.ts
✅ Files skipped from review due to trivial changes (1)
- src/app/[locale]/dashboard/_components/user/forms/edit-key-form.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- .github/workflows/dev.yml
- src/app/[locale]/dashboard/users/users-page-client.tsx
- src/app/[locale]/dashboard/_components/rate-limit-top-users.tsx
- src/app/[locale]/my-usage/_components/usage-logs-section.test.tsx
- tests/unit/users-action-search-users-for-filter.test.ts
- src/app/v1/_lib/proxy/provider-selector.ts
- tests/unit/actions/my-usage-date-range-dst.test.ts
- src/repository/usage-logs.ts
Comment on lines
+509
to
+541
| function mapMyUsageLogEntries( | ||
| result: Pick<UsageLogSlimBatchResult, "logs">, | ||
| billingModelSource: BillingModelSource | ||
| ): MyUsageLogEntry[] { | ||
| return result.logs.map((log) => { | ||
| const modelRedirect = | ||
| log.originalModel && log.model && log.originalModel !== log.model | ||
| ? `${log.originalModel} → ${log.model}` | ||
| : null; | ||
|
|
||
| const billingModel = | ||
| (billingModelSource === "original" ? log.originalModel : log.model) ?? null; | ||
|
|
||
| return { | ||
| id: log.id, | ||
| createdAt: log.createdAt, | ||
| model: log.model, | ||
| billingModel, | ||
| anthropicEffort: log.anthropicEffort ?? null, | ||
| modelRedirect, | ||
| inputTokens: log.inputTokens ?? 0, | ||
| outputTokens: log.outputTokens ?? 0, | ||
| cost: log.costUsd ? Number(log.costUsd) : 0, | ||
| statusCode: log.statusCode, | ||
| duration: log.durationMs, | ||
| endpoint: log.endpoint, | ||
| cacheCreationInputTokens: log.cacheCreationInputTokens ?? null, | ||
| cacheReadInputTokens: log.cacheReadInputTokens ?? null, | ||
| cacheCreation5mInputTokens: log.cacheCreation5mInputTokens ?? null, | ||
| cacheCreation1hInputTokens: log.cacheCreation1hInputTokens ?? null, | ||
| cacheTtlApplied: log.cacheTtlApplied ?? null, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
共享日志映射仍然没有带出请求级审计字段。
这层现在同时服务 getMyUsageLogs() 和 getMyUsageLogsBatch()，但返回值里依然没有“实际 service tier”以及“单次请求 cache-hit 百分比”。这样下游 API 和表格最多只能看到原始 cache token 数，仍然无法满足这次 PR 里“请求级审计可见性”的目标。建议在这个 mapper 里统一补齐字段，再同步更新响应 schema 和展示层。
🤖 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 509 - 541, In mapMyUsageLogEntries, add
the per-request audit fields to each MyUsageLogEntry: include an
actualServiceTier field (use log.actualServiceTier if present, fallback to
log.serviceTier, else null) and a cacheHitPercentage field (compute as null when
both cacheReadInputTokens and cacheCreationInputTokens are null or sum to 0;
otherwise set to (cacheReadInputTokens / (cacheReadInputTokens +
cacheCreationInputTokens)) * 100 as a Number). Update the returned object in
mapMyUsageLogEntries to populate these fields (ensure types default to null when
missing) and then sync the MyUsageLogEntry response schema and any UI/rendering
layers to consume actualServiceTier and cacheHitPercentage.
Comment on lines
+149
to
+168
| async function loadAllUsersForAdmin(baseParams?: GetUsersBatchParams): Promise<User[]> { | ||
| const users: User[] = []; | ||
| const normalizedBaseParams = normalizeUserListParams(baseParams); | ||
| let cursor = normalizedBaseParams.cursor; | ||
|
|
||
| while (true) { | ||
| const page = await findUserListBatch({ | ||
| ...normalizedBaseParams, | ||
| cursor, | ||
| limit: USER_LIST_MAX_LIMIT, | ||
| }); | ||
|
|
||
| users.push(...page.users); | ||
|
|
||
| if (!page.hasMore || !page.nextCursor) { | ||
| return users; | ||
| } | ||
|
|
||
| cursor = page.nextCursor; | ||
| } |
There was a problem hiding this comment.
给全量拉取循环加上终止保护。
这里没有任何“游标未前进 / 最大迭代次数”的保护。只要 findUserListBatch() 某次持续返回 hasMore = true 且 nextCursor 不前进，这个 Server Action 就会一直循环，把管理页请求挂死。建议至少检测 nextCursor === cursor 并中断，同时加一个和 getUsersWithQuotas() 类似的迭代上限。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/users.ts` around lines 149 - 168, The loadAllUsersForAdmin loop
can spin indefinitely if findUserListBatch keeps returning hasMore=true with an
unchanged nextCursor; update the loadAllUsersForAdmin function to break/throw
when progress stalls by checking if page.nextCursor === cursor and stop the
loop, and also add an iteration cap (like getUsersWithQuotas) e.g., a
MAX_ITERATIONS constant and increment a counter each loop to abort after the
limit; ensure you propagate or log an explanatory error (or return partial
results) when aborting so callers of loadAllUsersForAdmin can handle the
failure.
Comment on lines
+236
to
+239
| if (statusResult.data.status === "failed") { | ||
| toast.error(statusResult.data.error || t("logs.filters.exportError")); | ||
| return; | ||
| } |
There was a problem hiding this comment.
服务端错误消息未经 i18n 处理
当 statusResult.data.status === "failed" 时，直接显示 statusResult.data.error，这是服务端返回的原始错误消息（可能是中文或英文），会在非对应语言环境下显示未翻译的文本。
建议统一使用 i18n 回退消息，或在服务端返回 errorCode 后通过 getErrorMessage 转换：
if (statusResult.data.status === "failed") {
- toast.error(statusResult.data.error || t("logs.filters.exportError"));
+ toast.error(t("logs.filters.exportError"));
return;
}📝 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.
Suggested change
| if (statusResult.data.status === "failed") { | |
| toast.error(statusResult.data.error || t("logs.filters.exportError")); | |
| return; | |
| } | |
| if (statusResult.data.status === "failed") { | |
| toast.error(t("logs.filters.exportError")); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-filters.tsx around
lines 236 - 239, The code currently passes the raw server error
(statusResult.data.error) directly to toast.error when statusResult.data.status
=== "failed"; replace that with a localized message resolution: call
getErrorMessage(statusResult.data.errorCode) first (if errorCode exists), fall
back to t("logs.filters.exportError"), and pass the resulting string into
toast.error (still optionally append the raw server message only for debugging
if needed). Update the failure branch that references statusResult,
statusResult.data.error, toast.error and t to use this resolved message instead
of the unlocalized server string.
Comment on lines
+21
to
+44
| const collectedUsers: UserDisplay[] = []; | ||
| const MAX_USERS_FOR_QUOTAS = 2000; | ||
| const MAX_ITERATIONS = Math.ceil(MAX_USERS_FOR_QUOTAS / 200) + 1; | ||
| let cursor: string | undefined; | ||
| let iterations = 0; | ||
|
|
||
| while (collectedUsers.length < MAX_USERS_FOR_QUOTAS && iterations < MAX_ITERATIONS) { | ||
| iterations += 1; | ||
| const result = await getUsersBatch({ cursor, limit: 200 }); | ||
| if (!result.ok) { | ||
| throw new Error(result.error); | ||
| } | ||
|
|
||
| collectedUsers.push(...result.data.users); | ||
| if (!result.data.hasMore || !result.data.nextCursor) { | ||
| break; | ||
| } | ||
|
|
||
| cursor = result.data.nextCursor; | ||
| } | ||
|
|
||
| if (iterations >= MAX_ITERATIONS) { | ||
| console.warn("getUsersWithQuotas: reached max iterations, results may be incomplete"); | ||
| } |
There was a problem hiding this comment.
这里的 2000 用户上限会静默丢数据。
循环一旦因为 collectedUsers.length < MAX_USERS_FOR_QUOTAS 变成 false 就会直接退出，但下面只在 iterations >= MAX_ITERATIONS 时告警。也就是说，用户数超过 2000 时这里会悄悄截断结果，totalCount、配额汇总和后续列表都会变成不完整数据，而且这条 warning 还不会触发。要么把页面改成真正分页/虚拟化，要么至少在命中上限且 hasMore 仍为 true 时显式标记“结果已截断”。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/dashboard/quotas/users/page.tsx around lines 21 - 44, The
loop currently silently truncates at MAX_USERS_FOR_QUOTAS (2000) without
signaling truncation; modify the logic around collectedUsers, getUsersBatch, and
result.data.hasMore so that when collectedUsers.length >= MAX_USERS_FOR_QUOTAS
and the latest batch's result.data.hasMore is true you set a clear truncated
flag (e.g., isTruncated = true) or attach a "truncated" marker to the returned
payload instead of only relying on iterations/MAX_ITERATIONS; implement this by
checking after pushing result.data.users whether collectedUsers.length >=
MAX_USERS_FOR_QUOTAS && result.data.hasMore and then break the loop and surface
that flag (or log a console.warn with that context) so callers/UI can show
“results truncated” rather than silently dropping users.
Comment on lines
167
to
+180
| const handleApply = () => { | ||
| loadLogs({ ...draftFilters, page: 1 }); | ||
| const nextFilters = { ...draftFilters }; | ||
| if (JSON.stringify(nextFilters) === JSON.stringify(appliedFilters)) { | ||
| return; | ||
| } | ||
| setAppliedFilters(nextFilters); | ||
| }; | ||
|
|
||
| const handleReset = () => { | ||
| setDraftFilters({ page: 1 }); | ||
| loadLogs({ page: 1 }); | ||
| setDraftFilters({}); | ||
| if (Object.keys(appliedFilters).length === 0) { | ||
| return; | ||
| } | ||
| setAppliedFilters({}); |
There was a problem hiding this comment.
在应用/重置筛选时同步清掉 isBrowsingHistory。
refetchInterval 完全受 isBrowsingHistory 控制，但这里把查询切回第一页时没有把它重置为 false。用户先下钻历史页，再改筛选或点重置后，会继续停留在“历史浏览中”状态，自动刷新会被一直关掉，直到子表格再次显式回调一次 false。
建议修改
const handleApply = () => {
const nextFilters = { ...draftFilters };
if (JSON.stringify(nextFilters) === JSON.stringify(appliedFilters)) {
return;
}
+ setIsBrowsingHistory(false);
setAppliedFilters(nextFilters);
};
const handleReset = () => {
setDraftFilters({});
if (Object.keys(appliedFilters).length === 0) {
return;
}
+ setIsBrowsingHistory(false);
setAppliedFilters({});
};Based on learnings: 这里的自动刷新本来就明确依赖 !isBrowsingHistory，所以这个状态一旦残留为 true，新查询结果也会被一并停掉轮询。
📝 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.
Suggested change
| const handleApply = () => { | |
| loadLogs({ ...draftFilters, page: 1 }); | |
| const nextFilters = { ...draftFilters }; | |
| if (JSON.stringify(nextFilters) === JSON.stringify(appliedFilters)) { | |
| return; | |
| } | |
| setAppliedFilters(nextFilters); | |
| }; | |
| const handleReset = () => { | |
| setDraftFilters({ page: 1 }); | |
| loadLogs({ page: 1 }); | |
| setDraftFilters({}); | |
| if (Object.keys(appliedFilters).length === 0) { | |
| return; | |
| } | |
| setAppliedFilters({}); | |
| const handleApply = () => { | |
| const nextFilters = { ...draftFilters }; | |
| if (JSON.stringify(nextFilters) === JSON.stringify(appliedFilters)) { | |
| return; | |
| } | |
| setIsBrowsingHistory(false); | |
| setAppliedFilters(nextFilters); | |
| }; | |
| const handleReset = () => { | |
| setDraftFilters({}); | |
| if (Object.keys(appliedFilters).length === 0) { | |
| return; | |
| } | |
| setIsBrowsingHistory(false); | |
| setAppliedFilters({}); | |
| }; |
🤖 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.tsx around lines
167 - 180, When applying or resetting filters, also clear the browsing-history
flag so refetchInterval isn't permanently disabled: in handleApply (around the
logic that compares and calls setAppliedFilters(nextFilters)) call
setIsBrowsingHistory(false) after updating applied filters (or before returning
early if no change), and in handleReset (after setDraftFilters({}) and when you
call setAppliedFilters({})) call setIsBrowsingHistory(false) as well; locate the
handlers handleApply and handleReset and the state setter setIsBrowsingHistory
to make these changes.
Comment on lines
+745
to
750
| if (!this.hasUsableBillingSettings()) { | ||
| logger.warn("[ProxySession] Billing settings unavailable, using fallback billing source", { | ||
| billingSettingsSource: this.billingSettingsSource, | ||
| fallbackBillingModelSource: this.cachedBillingModelSource, | ||
| }); | ||
| } |
There was a problem hiding this comment.
警告条件永远不会触发（死代码）
hasUsableBillingSettings() 检查 cachedBillingModelSource 是否为 "original" 或 "redirected"，但 loadBillingSettings() 在所有分支（包括第 879 行的默认兜底）都会将其设置为这两个值之一。因此 !this.hasUsableBillingSettings() 永远为 false，这段警告代码永远不会执行。
如果意图是在使用默认兜底配置时发出警告，条件应该检查 billingSettingsSource：
建议修改
- if (!this.hasUsableBillingSettings()) {
+ if (this.billingSettingsSource === "default") {
logger.warn("[ProxySession] Billing settings unavailable, using fallback billing source", {
billingSettingsSource: this.billingSettingsSource,
fallbackBillingModelSource: this.cachedBillingModelSource,
});
}📝 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.
Suggested change
| if (!this.hasUsableBillingSettings()) { | |
| logger.warn("[ProxySession] Billing settings unavailable, using fallback billing source", { | |
| billingSettingsSource: this.billingSettingsSource, | |
| fallbackBillingModelSource: this.cachedBillingModelSource, | |
| }); | |
| } | |
| if (this.billingSettingsSource === "default") { | |
| logger.warn("[ProxySession] Billing settings unavailable, using fallback billing source", { | |
| billingSettingsSource: this.billingSettingsSource, | |
| fallbackBillingModelSource: this.cachedBillingModelSource, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/v1/_lib/proxy/session.ts` around lines 745 - 750, The warning branch
is dead because hasUsableBillingSettings() inspects cachedBillingModelSource
which loadBillingSettings() always sets to "original" or "redirected"; change
the condition to detect when the code is using the fallback billing settings by
checking billingSettingsSource instead of cachedBillingModelSource. Update the
if in the ProxySession session code to use this.billingSettingsSource (e.g., if
(this.billingSettingsSource !== "original" && this.billingSettingsSource !==
"redirected") or compare against the known default/fallback string) so the
logger.warn runs when loadBillingSettings() fell back; keep the same log payload
(billingSettingsSource and cachedBillingModelSource) and leave the
hasUsableBillingSettings() implementation unchanged.
Comment on lines
+200
to
+237
| test("exports the applied filters instead of unapplied local draft filters", async () => { | ||
| startUsageLogsExportMock.mockResolvedValue({ ok: true, data: { jobId: "job-2" } }); | ||
| getUsageLogsExportStatusMock.mockResolvedValueOnce({ | ||
| ok: true, | ||
| data: { | ||
| jobId: "job-2", | ||
| status: "completed", | ||
| processedRows: 1, | ||
| totalRows: 1, | ||
| progressPercent: 100, | ||
| }, | ||
| }); | ||
| downloadUsageLogsExportMock.mockResolvedValue({ ok: true, data: "\uFEFFTime,User\n" }); | ||
|
|
||
| const { container, unmount } = renderWithIntl( | ||
| <UsageLogsFilters | ||
| isAdmin={true} | ||
| providers={[]} | ||
| initialKeys={[]} | ||
| filters={{ sessionId: "applied-session" }} | ||
| onChange={() => {}} | ||
| onReset={() => {}} | ||
| /> | ||
| ); | ||
|
|
||
| await actClick(container.querySelector("[data-testid='request-filters']")); | ||
|
|
||
| const exportButton = Array.from(container.querySelectorAll("button")).find( | ||
| (button) => (button.textContent || "").trim() === "Export" | ||
| ); | ||
|
|
||
| await actClick(exportButton ?? null); | ||
| await flushPromises(); | ||
|
|
||
| expect(startUsageLogsExportMock).toHaveBeenCalledWith({ sessionId: "draft-session" }); | ||
|
|
||
| unmount(); | ||
| }); |
There was a problem hiding this comment.
测试名称与断言逻辑矛盾
测试名称为 "exports the applied filters instead of unapplied local draft filters"（导出已应用的过滤器而非未应用的草稿），但第 234 行的断言却期望调用参数为 { sessionId: "draft-session" }（草稿值）而非 { sessionId: "applied-session" }（已应用值）。
测试名称与断言逻辑完全相反。根据当前代码实现（使用 localFilters），如果期望的行为是导出草稿过滤器，请修正测试名称：
- test("exports the applied filters instead of unapplied local draft filters", async () => {
+ test("exports draft local filters rather than applied filters", async () => {如果期望的行为是导出已应用的过滤器，则需要修改代码实现和断言。
📝 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.
Suggested change
| test("exports the applied filters instead of unapplied local draft filters", async () => { | |
| startUsageLogsExportMock.mockResolvedValue({ ok: true, data: { jobId: "job-2" } }); | |
| getUsageLogsExportStatusMock.mockResolvedValueOnce({ | |
| ok: true, | |
| data: { | |
| jobId: "job-2", | |
| status: "completed", | |
| processedRows: 1, | |
| totalRows: 1, | |
| progressPercent: 100, | |
| }, | |
| }); | |
| downloadUsageLogsExportMock.mockResolvedValue({ ok: true, data: "\uFEFFTime,User\n" }); | |
| const { container, unmount } = renderWithIntl( | |
| <UsageLogsFilters | |
| isAdmin={true} | |
| providers={[]} | |
| initialKeys={[]} | |
| filters={{ sessionId: "applied-session" }} | |
| onChange={() => {}} | |
| onReset={() => {}} | |
| /> | |
| ); | |
| await actClick(container.querySelector("[data-testid='request-filters']")); | |
| const exportButton = Array.from(container.querySelectorAll("button")).find( | |
| (button) => (button.textContent || "").trim() === "Export" | |
| ); | |
| await actClick(exportButton ?? null); | |
| await flushPromises(); | |
| expect(startUsageLogsExportMock).toHaveBeenCalledWith({ sessionId: "draft-session" }); | |
| unmount(); | |
| }); | |
| test("exports draft local filters rather than applied filters", async () => { | |
| startUsageLogsExportMock.mockResolvedValue({ ok: true, data: { jobId: "job-2" } }); | |
| getUsageLogsExportStatusMock.mockResolvedValueOnce({ | |
| ok: true, | |
| data: { | |
| jobId: "job-2", | |
| status: "completed", | |
| processedRows: 1, | |
| totalRows: 1, | |
| progressPercent: 100, | |
| }, | |
| }); | |
| downloadUsageLogsExportMock.mockResolvedValue({ ok: true, data: "\uFEFFTime,User\n" }); | |
| const { container, unmount } = renderWithIntl( | |
| <UsageLogsFilters | |
| isAdmin={true} | |
| providers={[]} | |
| initialKeys={[]} | |
| filters={{ sessionId: "applied-session" }} | |
| onChange={() => {}} | |
| onReset={() => {}} | |
| /> | |
| ); | |
| await actClick(container.querySelector("[data-testid='request-filters']")); | |
| const exportButton = Array.from(container.querySelectorAll("button")).find( | |
| (button) => (button.textContent || "").trim() === "Export" | |
| ); | |
| await actClick(exportButton ?? null); | |
| await flushPromises(); | |
| expect(startUsageLogsExportMock).toHaveBeenCalledWith({ sessionId: "draft-session" }); | |
| unmount(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/dashboard-logs-export-progress-ui.test.tsx` around lines 200 -
237, The test name ("exports the applied filters instead of unapplied local
draft filters") conflicts with the assertion which expects
startUsageLogsExportMock to be called with the draft sessionId ("draft-session")
while the component is rendered with applied filters ({ sessionId:
"applied-session" }); update either the test name to indicate it verifies
exporting draft/local filters or change the test to expect the applied filters
by modifying the rendered UsageLogsFilters props or the final expect call;
specifically, for the current assertion keep the test name referencing "draft
filters" or change expect(startUsageLogsExportMock).toHaveBeenCalledWith to
expect { sessionId: "applied-session" } to match the test title and the filters
prop passed to UsageLogsFilters.
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
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
3 participants
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
Release v0.6.6 merges the
devbranch intomain, containing 9 feature and bugfix PRs spanning proxy reliability, UI stability, security hardening, billing configuration, and tooling upgrades.Included Changes
Features
http://user:pass@host:port)Bug Fixes
/v1/modelsnow returns models from ALL available providers, not just first per type (fixes #956)fullKeyexposure for sessions withcanLoginWebUi=false/api/actions/users/getUsersBatchand/api/actions/users/searchUsersendpointsTooling
@typescript/native-preview, gate dev/build flows withbun run typecheckDatabase Migration
0085_busy_ken_ellis.sqlcodex_priority_billing_sourcecolumn tosystem_settingstableBreaking Changes
None. All changes are backward compatible.
Key Areas Impacted
Testing
All component PRs passed:
bun run typecheckbun run buildbun run testbun run lintComponent PRs
Description enhanced by Claude AI
Greptile Summary
This release PR (v0.6.6) merges 9 component PRs covering proxy reliability, security hardening, UI virtualisation, billing configuration, and API compatibility. The changes are broad but well-scoped, with solid test coverage for the new behaviours.
Highlights:
fullKeyis now gated behindcanExposeFullKey()in every path that builds key display objects (getUsers,getUsersBatch,getUsersBatchCore). Readonly API keys (canLoginWebUi=false) can no longer retrieve plain-text API keys for any user, including their own. The implementation is consistent across all three code paths. ✅loadBillingSettings()now has a proper three-level fallback (live DB → in-memory cache → hardcoded defaults). The previous concern about pricing falling back to $0 on a cold startup with a DB outage is fully resolved — the default path always setscachedBillingModelSource = "redirected". ✅/v1/modelsnow fetches from all matching enabled providers concurrently viaPromise.all. EachfetchModelsFromProvidercall wraps its own error and returns[]on failure, so a single unresponsive provider does not block the others. ✅ReactiveRectifierRetryStateis now tracked on eachStreamingHedgeAttempt, so thinking-signature and thinking-budget rectifiers fire correctly during hedge retries without double-applying. ✅escapeCsvField) and admin/user filter isolation are correctly applied. ThesetTimeout(() => { void runUsageLogsExportJob(...) }, 0)fire-and-forget pattern is explicitly documented as only safe for long-lived Bun processes, which matches the deployment target. ✅getMyUsageLogs(page/pageSize) is preserved alongside the new cursor-basedgetMyUsageLogsBatch.getUsersBatchandsearchUsersroutes are added. The prior concern about a breaking rename is resolved. ✅Minor findings:
buildUsageLogsExportCsvmakes an initialfindUsageLogsWithDetails({pageSize:1})call purely to seed the progress estimate, whenfindUsageLogsStatsis purpose-built for counting and avoids the heavy multi-table JOIN. Non-blocking style suggestion.searchUsersroute'sargsMappersilently dropslimit; callers cannot control result-set size through this legacy compatibility endpoint (always defaults to 200). Possibly intentional, but worth a clarifying comment.Confidence Score: 4/5
limitdrop insearchUsersroute) but neither affects correctness or security.src/actions/usage-logs.tsandsrc/app/api/actions/[...route]/route.tsbut both are well-tested.Important Files Changed
findUsageLogsWithDetails({pageSize:1})call is used only to seed the progress estimate whenfindUsageLogsStatswould be lighter-weight.loadBillingSettings()helper with a proper three-level fallback (live DB → in-memory cache → hardcoded defaults). The previous concern about billing falling back to $0 is fully addressed — the default path now returns "redirected"/"requested" sohasUsableBillingSettings()is always true after the load.ReactiveRectifierRetryState, preventing duplicate retries. The newaddSpecialSettingForPersistencehelper safely deduplicates settings between owner and persist sessions.Promise.allacross all matched providers; eachfetchModelsFromProviderwraps in try/catch and returns[]on error, so a single failing provider does not block the rest.canExposeFullKey()which gatesfullKeyexposure onsession.key.canLoginWebUi. Legacy API compatibility restored with proper normalization ofpage/offset/query/keywordparams throughnormalizeUserListParams.getUsersBatch,searchUsers, andgetMyUsageLogsBatchroutes for legacy compat. The oldgetMyUsageLogs(page/pageSize) route is preserved alongside the new cursor-based batch route, addressing the prior breaking-change concern. ThesearchUsersroute's argsMapper only extractssearchTerm, notlimit, so result-set size is capped at the 200 default regardless of what the caller sends.resolveCodexPriorityBillingDecision(). The newbillingSourcePreference/resolvedFromfields are stored in the special-settings record and thehasPriorityServiceTierSpecialSettingguard correctly excludes these new-style records to avoid double-counting.buildNextCursorOrThrowthat converts the previously silentnullreturn (whenhasMore=truebutcreatedAtRawis missing) into a hard error — a good defensive change that surfaces data-integrity violations instead of silently causing infinite pagination loops.normalizeProviderGroupTagfor provider storage andparseProviderGroupsnow acceptsunknowninstead ofstring. Consistent with DB filter regex update inuser.ts.codex_priority_billing_sourcecolumn gracefully before migration runs. The progressive fallback (full → without new column → minimal fields) is clean and tested by the newsystem-config-update-missing-columns.test.ts.Sequence Diagram
sequenceDiagram participant Client participant Action as usage-logs action participant Redis participant DB Client->>Action: startUsageLogsExport(filters) Action->>Action: resolveUsageLogFiltersForSession() Action->>Redis: set job {status:"queued", ownerUserId} Action-->>Client: {jobId} Note over Action,DB: setTimeout(0) — fire and forget Action->>DB: findUsageLogsStats(filters) [initial count] loop cursor-based batch fetch Action->>DB: findUsageLogsBatch({cursor, limit:500}) DB-->>Action: {logs, nextCursor, hasMore} Action->>Action: buildCsvRows() + escapeCsvField() Action->>Redis: update job progress end Action->>Redis: set csv data (TTL 15 min) Action->>Redis: set job {status:"completed"} Client->>Action: getUsageLogsExportStatus(jobId) Action->>Redis: get job (ownership check) Redis-->>Action: job record Action-->>Client: {status, progressPercent} Client->>Action: downloadUsageLogsExport(jobId) Action->>Redis: get csv data Redis-->>Action: csv string Action-->>Client: CSV contentComments Outside Diff (1)
src/repository/usage-logs.ts, line 460-476 (link)buildNextCursorOrThrowthrows on data inconsistency, breaking CSV export mid-flightPreviously a missing
createdAtRawwhenhasMoreistruesilently returnednull, causing pagination to stop. The newbuildNextCursorOrThrowhelper throws instead, which will propagate up throughbuildUsageLogsExportCsvand mark the background export job asfailed.If any existing row in the DB has a
NULLcreatedAt, any export that reaches that row will fail without producing partial output. Given that the old code treated this as non-fatal, there may already be such rows in production.Consider logging the inconsistency and returning
null(stopping pagination gracefully) rather than throwing, or add a guard that nulls-outcreatedAtRawcoalesces in the query itself.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix: address PR #968 review feedback (#9..." | Re-trigger Greptile