Conversation
fix(i18n): refresh server chrome on locale switch
* fix(billing): charge redirected per-request image responses * fix(billing): skip zero per-request usage billing * fix(billing): preserve per-request session error details * fix(billing): tolerate per-request pricing lookup failures * fix(billing): skip per-request billing for fake-200 errors
Merge PR #1120 into dev
The model rankings query was sorting by request count, which contradicted the "Cost Leaderboard" framing and diverged from the user and provider scopes that both order by sum(cost_usd). Switch the ORDER BY to sum(cost_usd) DESC with request count as tiebreaker, matching the provider model sub-rows pattern.
* feat(quota): add total cost limit to provider quotas and update related components * feat(quota): add total quota fields and UI components for total limit * fix(quota): repair total quota compile issues * fix(quota): honor total reset markers
The AbortSignal.any polyfill in forwarder.ts attached `abort` listeners
to source signals (responseController.signal and session.clientAbortSignal)
with `{ once: true }`, but `once: true` only auto-detaches when the abort
event actually fires. When a request completes normally — the common case —
those listeners were never removed and their closures kept holding the
combinedController, the cleanup array, and (transitively) the session and
the request body, preventing GC.
This is the same leak pattern as #1113 fixed for the client abort listener
in `_handleStreamingHedge`; the polyfill branch in
`forwardSingleAttempt` was not covered by that PR.
Extract the polyfill into `combine-abort-signals.ts` returning an explicit
`{ signal, cleanup }` pair. The native `AbortSignal.any` branch returns a
noop cleanup since V8 owns the listener lifecycle.
In `forwardSingleAttempt`:
- Call cleanup in the fetch failure catch path (response-handler never runs).
- Call cleanup in the inner `try { throw fromUpstreamResponse } finally`
path used for HTTP 4xx/5xx errors (response-handler also skipped here).
- Compose cleanup into the `sessionWithTimeout.releaseAgent` callback so
the existing response-handler finally invocation point handles the
successful path without touching every call site. Cleanup is idempotent
via an internal `cleaned` flag.
Add unit tests covering both branches: native delegation, polyfill abort
propagation, listener detachment after explicit cleanup, automatic
cleanup on source abort, and pre-aborted source signals.
Co-authored-by: guangdianclaw <claw@guangdian.studio>
…ng (#1128) SELECT uses COALESCE(sum(costUsd), 0) but ORDER BY used raw sum(costUsd). In PostgreSQL DESC, NULL sorts first, causing models/users/providers with no billing data to rank above those with explicit $0.00 cost despite both returning totalCost=0 to the caller.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough新增“总消费/总配额”维度（limitTotalUsd / costTotal / resetAt/totalCostResetAt），扩展类型、聚合查询与 UI 编辑/展示；将大量服务端翻译调用改为显式 locale；新增 AbortSignal 组合工具并在 proxy forwarder/response 路径中重构并接入 Langfuse 追踪；补充大量测试。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
enhancement
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17cbb8da8e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| // Polyfill 路径上需要主动解绑源信号的 abort listener（response-handler 不会执行）。 | ||
| cleanupCombinedSignal(); |
There was a problem hiding this comment.
In ProxyForwarder's fetch catch path, cleanupCombinedSignal() is called immediately before the HTTP/2→HTTP/1.1 and proxy→direct fallback fetches are attempted. On runtimes using the polyfill path (where AbortSignal.any is unavailable), that cleanup detaches the source listeners from combinedSignal, so the subsequent fallback request no longer reacts to responseController.abort() or session.clientAbortSignal. This can leave fallback requests running past configured response timeouts and ignore client disconnects in standalone environments.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements a "Total Quota" feature for keys and providers, refactors internationalization to use explicit locales in server components, and improves the LanguageSwitcher with router refreshes. It also enhances observability by integrating Langfuse tracing into the proxy error handler and introduces a utility for combining AbortSignals with explicit cleanup. Review feedback correctly identifies a double-billing risk in the per-request billing logic within finalizeRequestStats and a potential null pointer dereference when accessing messageContext.
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
432
to
436
| // 这里用空 usage 只承载 input_cost_per_request，不新增按图、按 token 等语义。 | ||
| return {}; | ||
| } | ||
|
|
||
| type FinalizeDeferredStreamingResult = { |
There was a problem hiding this comment.
UsageMetrics object passed to downstream callers expecting typed fields
When resolveBillableUsageMetricsForCost returns {} (the per-request billing case), this object is passed to maybeSetCodexContext1m(session, provider, billableUsageMetrics.input_tokens) — input_tokens would be undefined. Downstream callers that safely guard on input_tokens are fine, but callers that spread or destructure the object without guards could see unexpected undefined where number is expected. Consider returning an explicit typed sentinel like { input_tokens: 0, output_tokens: 0 } to make the intent clearer and safer for future callers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/response-handler.ts
Line: 432-436
Comment:
**Empty `UsageMetrics` object passed to downstream callers expecting typed fields**
When `resolveBillableUsageMetricsForCost` returns `{}` (the per-request billing case), this object is passed to `maybeSetCodexContext1m(session, provider, billableUsageMetrics.input_tokens)` — `input_tokens` would be `undefined`. Downstream callers that safely guard on `input_tokens` are fine, but callers that spread or destructure the object without guards could see unexpected `undefined` where `number` is expected. Consider returning an explicit typed sentinel like `{ input_tokens: 0, output_tokens: 0 }` to make the intent clearer and safer for future callers.
How can I resolve this? If you propose a fix, please make it concise.![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
This release PR (v0.7.4) contains 8 well-structured bug fixes across i18n, billing, proxy stability, quota management, and dashboard accuracy. The code quality is high with comprehensive test coverage (new test files included for all major changes).
PR Size: XL
- Lines changed: 2,078 (1,865 additions, 213 deletions)
- Files changed: 60
- Note: Size is XL due to 5-language i18n files and related test coverage. The 8 constituent PRs are logically independent and each has been individually reviewed.
Key Changes Reviewed
1. Proxy Stability: combineAbortSignals (PR #1121)
File: src/app/v1/_lib/proxy/combine-abort-signals.ts (new)
Well-designed utility with proper memory leak prevention:
- Idempotent cleanup with
cleanedflag prevents double-cleanup - Native
AbortSignal.anyused when available with NOOP_CLEANUP fallback - Polyfill properly tracks detachers array for listener removal
- Test coverage validates pre-aborted signals and cleanup behavior
Suggested improvement: Consider adding a debug warning if cleanup is called multiple times (idempotent but may indicate logic issues).
2. Per-Request Billing: resolveBillableUsageMetricsForCost (PR #1119)
File: src/app/v1/_lib/proxy/response-handler.ts
Good approach for handling image API billing where token metrics are absent:
- Returns
{}for per-request billing cases (wheninput_cost_per_requestis configured) - Fake-200 detection prevents billing for error responses
- Note: The empty object
{}returned for per-request billing is passed to downstream functions. While current implementations handle this gracefully via optional chaining, future callers should be aware this may not contain expected fields likeinput_tokens.
3. Langfuse Tracing in Error Handler (PR #1120)
File: src/app/v1/_lib/proxy/error-handler.ts, src/lib/langfuse/emit-proxy-trace.ts (new)
Clean extraction of shared emitProxyLangfuseTrace helper:
- Fire-and-forget pattern prevents observability failures from affecting proxy responses
- Proper error suppression with logger warnings
- Error traces now include errorMessage and responseMissing fields
4. Leaderboard COALESCE Fix (PR #1128)
File: src/repository/leaderboard.ts
Critical fix for NULL sorting behavior:
- All 5 ORDER BY clauses now use
COALESCE(sum(costUsd), 0)for consistent NULL handling - Model leaderboard correctly sorts by total cost (with request count as tiebreaker)
5. Total Quota Support (PR #1126)
File: src/actions/providers.ts, src/lib/utils/quota-helpers.ts
New limitTotalUsd field added to provider and key quota management:
- Proper i18n support across all 5 languages
- UI components updated with progress bars for total cost tracking
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 1 | 0 |
| Types | 0 | 0 | 1 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Medium Priority Issues
-
[TYPE-WEAK-INVARIANT] Empty
UsageMetricsobject (response-handler.ts:1544-1607)resolveBillableUsageMetricsForCostreturns{}for per-request billing- Downstream callers that destructure expecting
input_tokens/output_tokenswill getundefined - Suggested fix: Return
{ input_tokens: 0, output_tokens: 0 }for clarity
-
[ERROR-SILENT] Logger-only failure reporting (general pattern)
- Several async operations fail with only logger warnings (e.g., circuit breaker recording failures)
- Consider if these should have alternative telemetry paths for production monitoring
Review Coverage
- Logic and correctness - Clean, all edge cases handled
- Security (OWASP Top 10) - No injection risks, proper input validation
- Error handling - Comprehensive with fallback paths
- Type safety - Strong TypeScript usage, minimal
anytypes - Documentation accuracy - Comments match implementation
- Test coverage - 5222+ tests passing, new tests for all changes
- Code clarity - Well-structured, clear intent
Split Recommendation for Future
For PRs of this size (XL), consider:
- i18n changes (5 language files) could be a separate PR
- Database/quotas changes could be independent
- Proxy changes (forwarder, error-handler, response-handler) could be grouped
Verification
- All builds pass (
bun run build) - All lint checks pass (
bun run lint) - All type checks pass (
bun run typecheck) - All tests pass (
bun run test- 5222+ tests) - No emoji in code (per CLAUDE.md)
- i18n coverage complete (5 languages)
- Test coverage >80% for new features
Automated review by Claude AI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/v1/_lib/proxy/forwarder.ts (1)
2809-2816:⚠️ Potential issue | 🔴 Critical不要在 fallback 之前提前执行
cleanupCombinedSignal()。这里一进入
catch就解绑了 polyfill 路径的源 signal 监听，但后面的 HTTP/2 降级和 proxy-to-direct fallback 仍然复用了同一个combinedSignal。在AbortSignal.any不可用的环境里，第二次请求将不再响应客户端取消或responseTimeout，可能一直挂到上游自己结束。建议只在确认本次请求彻底终止时 cleanup，或者在每次 fallback 前重新组合一组 signal。🤖 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 2809 - 2816, The catch block currently calls cleanupCombinedSignal() immediately, which prematurely removes the polyfill source abort listeners while same combinedSignal is reused for HTTP/2 downgrade and proxy-to-direct fallbacks; change the flow so cleanupCombinedSignal() is only invoked when the request is truly terminated (after all fallbacks and no further retries), or alternatively before each fallback recreate a fresh combinedSignal; ensure you still clear responseTimeoutId early (clearTimeout(responseTimeoutId)) but delay or move cleanupCombinedSignal() to the final termination branch or to the code path that abandons further fallbacks (referencing cleanupCombinedSignal, combinedSignal, and responseTimeoutId).src/app/v1/_lib/proxy/response-handler.ts (1)
3641-3709:⚠️ Potential issue | 🟠 Major无
usage分支也要清理 live chain。这条分支现在会被图片按次计费这类“成功但没有 token usage”的请求正常命中，但这里直接
return null，会跳过下面唯一的deleteLiveChain()。结果是 session observability 里的 live chain 残留，监控/调试界面会继续显示请求中直到 TTL 过期。建议修改
await updateMessageRequestDetails(messageContext.id, { statusCode: statusCode, ...(errorMessage ? { errorMessage } : {}), ttfbMs: session.ttfbMs ?? duration, providerChain: session.getProviderChain(), model: session.getCurrentModel() ?? undefined, actualResponseModel: extractActualResponseModelForProvider( provider.providerType, resolvedIsStream, responseText ), providerId: providerIdForPersistence, context1mApplied: session.getContext1mApplied(), swapCacheTtlApplied: session.provider?.swapCacheTtlBilling ?? false, specialSettings: session.getSpecialSettings() ?? undefined, }); + if (session.sessionId && session.requestSequence != null) { + if (session.shouldTrackSessionObservability()) { + void deleteLiveChain(session.sessionId, session.requestSequence); + } + } return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/response-handler.ts` around lines 3641 - 3709, The branch that returns early when there is no billable usage skips the later live-chain cleanup; call deleteLiveChain(...) before returning to ensure live chain is removed for cases like image-per-request responses. Locate the branch around resolveBillableUsageMetricsForCost / updateRequestCostFromUsage and, just before the early return null, invoke deleteLiveChain with the same session/messageContext identifiers used elsewhere (e.g., deleteLiveChain(session) or deleteLiveChain(messageContext.id) as appropriate), await it and wrap it in try/catch to log any errors so cleanup always runs even on the "no usage" path.src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx (1)
397-400:⚠️ Potential issue | 🟡 Minor“清除所有限额”按钮条件遗漏
costTotal，会导致仅设置总限额时无法显示清除入口。你已在提交/清空 payload 中处理了
limitTotalUsd，但按钮可见性条件没有同步更新，导致功能表现不一致。建议修复
- {(currentQuota?.cost5h.limit || - currentQuota?.costWeekly.limit || - currentQuota?.costMonthly.limit || - (currentQuota?.concurrentSessions.limit ?? 0) > 0) && ( + {(currentQuota?.cost5h.limit || + currentQuota?.costDaily.limit || + currentQuota?.costWeekly.limit || + currentQuota?.costMonthly.limit || + currentQuota?.costTotal.limit || + (currentQuota?.concurrentSessions.limit ?? 0) > 0) && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx around lines 397 - 400, The "Clear all quotas" button visibility check omits the total cost quota so it won't show when only total is set; update the JSX conditional that builds this visibility (the expression referencing currentQuota?.cost5h.limit, costWeekly.limit, costMonthly.limit, and concurrentSessions.limit) to also include the total quota checks—e.g. include currentQuota?.costTotal.limit and/or (currentQuota?.limitTotalUsd ?? 0) > 0—so the button appears when a total-cost quota is present; edit the conditional in edit-key-quota-dialog.tsx where that compound boolean is evaluated to add these checks.src/components/ui/__tests__/language-switcher.test.tsx (1)
1-177:⚠️ Potential issue | 🟡 Minor测试文件路径建议按仓库约定迁移。
当前使用
src/components/ui/__tests__/language-switcher.test.tsx，建议迁移到约定目录（如tests/unit/）或统一为约定的 source-adjacent 形式，避免后续测试发现/分层策略不一致。As per coding guidelines
**/*.test.{ts,tsx}: Unit tests should be located in tests/unit/, integration tests in tests/integration/, and source-adjacent tests in src/**/*.test.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/__tests__/language-switcher.test.tsx` around lines 1 - 177, Move the LanguageSwitcher unit test out of its current source-adjacent location into the canonical unit test directory (keeping the same filename and test contents such as the describe("LanguageSwitcher") block and helper functions render and click), update any import/alias references if they break after moving (LanguageSwitcher, useLocale/usePathname/useRouter mocks), and ensure the test runner config (vitest file patterns) includes the new tests/unit/ path so the test still runs with the existing `@vitest-environment` header.
🧹 Nitpick comments (5)
tests/unit/repository/leaderboard-provider-metrics.test.ts (1)
705-741: 建议把“同成本时按请求数降序”做成行为断言。当前样例中两条记录成本不同，无法真正验证 tie-breaker。建议增加“totalCost 相同、totalRequests 不同”的数据并断言顺序，避免仅靠
orderBy参数结构检查带来的回归漏检。可参考的增强示例
- it("orders by total cost descending with request count as tiebreaker", async () => { + it("orders by total cost descending with request count as tiebreaker", async () => { chainMocks = [ createChainMock([ { - model: "expensive-low-volume", - totalRequests: 5, - totalCost: "50.0", + model: "same-cost-high-req", + totalRequests: 200, + totalCost: "10.0", totalTokens: 1000, successRate: 1.0, }, { - model: "cheap-high-volume", - totalRequests: 200, - totalCost: "1.0", + model: "same-cost-low-req", + totalRequests: 5, + totalCost: "10.0", totalTokens: 100000, successRate: 1.0, }, ]), ]; @@ - expect(result[0].model).toBe("expensive-low-volume"); - expect(result[0].totalCost).toBe(50); - expect(result[1].model).toBe("cheap-high-volume"); - expect(result[1].totalCost).toBe(1); + expect(result[0].model).toBe("same-cost-high-req"); + expect(result[1].model).toBe("same-cost-low-req"); + expect(result[0].totalCost).toBe(10); + expect(result[1].totalCost).toBe(10);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/repository/leaderboard-provider-metrics.test.ts` around lines 705 - 741, The test "orders by total cost descending with request count as tiebreaker" currently uses records with different totalCost so it doesn't exercise the tie-breaker; update the test data in the chainMocks (created via createChainMock) to include two records with identical totalCost (e.g., both "totalCost": "10.0") but different totalRequests (e.g., 5 vs 200), then call findDailyModelLeaderboard() and assert the result ordering uses totalRequests as the tiebreaker (expect result[0].model to be the one with higher totalRequests and result[1] the lower), while retaining the existing assertions that orderBy (chainMocks[0].orderBy) was called with the sum/count args to ensure both behavior and query-shape are verified.src/repository/leaderboard.ts (1)
367-367: 建议补充稳定兜底排序键，避免同分时榜单顺序抖动。当前主排序（以及部分次排序）已合理，但在成本与请求数完全相同的情况下，返回顺序仍可能不稳定。建议在这些查询后追加稳定键（如名称/ID/model）作为最终排序键，降低前端列表抖动与快照波动。
可参考的调整示例
- .orderBy(desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`)); + .orderBy( + desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`), + asc(users.name), + asc(usageLedger.userId), + ); - .orderBy(desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`)); + .orderBy( + desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`), + desc(sql`count(*)`), + asc(modelField), + ); - .orderBy(desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`)); + .orderBy( + desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`), + asc(providers.name), + asc(usageLedger.finalProviderId), + ); - .orderBy(desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`), desc(sql`count(*)`)); + .orderBy( + desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`), + desc(sql`count(*)`), + asc(modelField), + );Also applies to: 407-407, 699-699, 750-750, 1156-1156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/leaderboard.ts` at line 367, 当前多处排行榜查询仅按聚合列（例如 orderBy(desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`))）排序，会导致在聚合值相同时返回顺序不稳定；请在这些 orderBy 链中追加一个稳定兜底排序键（例如实体的唯一标识符/名称或 model id，如 user.id、project.id、model.name 等），把它作为最终的排序条件以保证确定性并减少前端抖动；定位并修改所有出现该模式的函数/表达式（例如包含 orderBy(desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`)) 的查询以及评论中提到的其它相似行），将稳定键追加为最后一个排序键。src/app/v1/_lib/proxy/forwarder.ts (1)
55-55: 改成@/别名导入。这里是新增导入，继续使用相对路径会把同一目录的两种导入风格混在一起。建议直接改成
@/app/v1/_lib/proxy/combine-abort-signals。As per coding guidelines "Use path alias
@/mapped to ./src/ for imports".🤖 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` at line 55, Replace the relative import of combineAbortSignals in forwarder.ts with the project path alias; update the import statement that currently reads import { combineAbortSignals } from "./combine-abort-signals"; to use "@/app/v1/_lib/proxy/combine-abort-signals" so forwarder.ts and combineAbortSignals use the same alias style and comply with the "@/ -> ./src/" guideline.src/types/provider.ts (1)
467-467: 建议收紧totalCostResetAt的可选性，避免null/undefined语义混淆。当前声明为可选会允许字段缺失（
undefined）。如果业务语义是“字段始终存在，null表示无重置时间”，建议改为非可选，提升类型约束并减少漏映射风险。建议修改
- totalCostResetAt?: Date | null; + totalCostResetAt: Date | null;Based on learnings, In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/provider.ts` at line 467, The property totalCostResetAt on the provider type is currently optional which conflates "missing" (undefined) with "explicitly no reset" (null); change the declaration of totalCostResetAt from optional to required with nullable type (i.e., make it totalCostResetAt: Date | null) and update any factory/mapping code, constructors, deserializers, and usages that instantiate or read this type (search for totalCostResetAt occurrences) to explicitly set null when there is no reset time so consumers can rely on the field always being present.tests/unit/settings/prices-layout.test.ts (1)
13-15: 避免对className顺序做强绑定断言。当前断言依赖固定片段/顺序，类名重排（语义不变）会触发误报，建议改为 token 级断言以提升测试稳定性。
可选修改示例
import fs from "node:fs"; import path from "node:path"; import { describe, expect, test } from "vitest"; function readProjectFile(...segments: string[]) { return fs.readFileSync(path.join(process.cwd(), ...segments), "utf8"); } + +function expectClassTokens(source: string, tokens: string[]) { + for (const token of tokens) { + expect(source).toContain(token); + } +} describe("settings prices layout constraints", () => { test("settings content column can shrink inside the centered page container", () => { const source = readProjectFile("src/app/[locale]/settings/layout.tsx"); - expect(source).toContain('className="mx-auto w-full max-w-7xl'); - expect(source).toContain('className="min-w-0 space-y-6"'); + expectClassTokens(source, ["mx-auto", "w-full", "max-w-7xl", "min-w-0", "space-y-6"]); }); test("price table scrolls horizontally inside its settings section", () => { const source = readProjectFile("src/app/[locale]/settings/prices/_components/price-list.tsx"); - expect(source).toMatch(/<div className="[^"]*overflow-x-auto[^"]*overflow-y-hidden[^"]*"/); - expect(source).toMatch(/<table className="[^"]*min-w-\[[^\]]+\][^"]*table-fixed[^"]*"/); + expect(source).toMatch(/<div className="[^"]*overflow-x-auto[^"]*"/); + expect(source).toMatch(/<div className="[^"]*overflow-y-hidden[^"]*"/); + expect(source).toMatch(/<table className="[^"]*min-w-\[[^\]]+\][^"]*"/); + expect(source).toMatch(/<table className="[^"]*table-fixed[^"]*"/); }); });Also applies to: 20-21
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/settings/prices-layout.test.ts` around lines 13 - 15, The tests in prices-layout.test.ts currently assert exact className substrings (uses expect(source).toContain(...)) which binds to class order and will break if classes are reordered; update the assertions that reference the variable source (the two expect lines checking className="mx-auto w-full max-w-7xl" and className="min-w-0 space-y-6") to assert at the token level instead — split the class string into tokens or use a regex/utility that verifies each expected class token exists (e.g., check that "mx-auto", "w-full", "max-w-7xl" are present and that "min-w-0" and "space-y-6" are present) so order changes won’t fail the test.
🤖 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/providers.ts`:
- Around line 1263-1264: In resetProviderTotalUsage, the await
publishProviderCacheInvalidation() call can throw and falsely mark the whole
action as failed after the database reset succeeded; wrap the
publishProviderCacheInvalidation() invocation in a try/catch, log the error
(e.g. using the existing logger or processLogger) and ensure the catch does not
rethrow so the function returns success for the DB reset path; keep the behavior
for real DB errors unchanged and only swallow/log cache-broadcast errors from
publishProviderCacheInvalidation().
In `@src/app/v1/_lib/proxy/error-handler.ts`:
- Around line 252-256: 当前在 ProxyErrorHandler.emitErrorTrace(session, {...})
被调用的位置会早于 getErrorOverrideAsync()，导致上报到 Langfuse 的 trace
仍然是上游原始错误而非最终可能被覆写的状态或响应体；请将对 ProxyErrorHandler.emitErrorTrace 的调用移动到在调用并应用
getErrorOverrideAsync()/错误覆写逻辑之后（即在最终确定 statusCode 和响应体后再调用），确保 emitErrorTrace
使用覆盖后的 statusCode/errorMessage/responseBody，更新引用位置以使用同一 session
变量并保留原有传入字段名（error、errorMessage、statusCode）。
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 3279-3291: The releaseAgent cleanup is assigned on
sessionWithTimeout but isn’t being synced when a shadow attempt wins; update
commitWinner() so syncWinningAttemptSession(session, attempt.session) also
copies the releaseAgent callback (in addition to clearResponseTimeout and
responseController) from the winning attempt.session to the original session,
and ensure any runtime/winner sync fields that carry attempt-specific callbacks
include releaseAgent so the response-handler receives and invokes the correct
cleanup for the winner.
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Around line 392-429: The current code returns usageMetrics immediately
(usageMetrics) before verifying statusCode and detecting fake-200 error
payloads, allowing billing for error responses; move the early-return for
usageMetrics to after the statusCode (statusCode <200 || >=300) and fake-200
detection (responseText + detectUpstreamErrorFromSseOrJsonText) checks or
alternatively add a guard that verifies statusCode is 2xx and detected.isError
is false before returning usageMetrics; keep the resolved pricing logic
(session.getResolvedPricingByBillingSource and hasBillableInputCostPerRequest)
intact and only proceed to billing/return usageMetrics when statusCode is 2xx
and fake-200 detection passed.
In `@src/components/ui/language-switcher.tsx`:
- Around line 82-84: The storedRefreshTarget ternary is inverted so
getPendingLocaleRefreshTarget() is rarely called; change the assignment for
storedRefreshTarget to call getPendingLocaleRefreshTarget() when
activePendingLocaleRefreshTarget is null (i.e., storedRefreshTarget =
activePendingLocaleRefreshTarget === null ? getPendingLocaleRefreshTarget() :
null) so that refreshTarget (pendingLocale ?? activePendingLocaleRefreshTarget
?? storedRefreshTarget) can correctly restore cross-remount pending state; keep
the existing symbols: storedRefreshTarget, activePendingLocaleRefreshTarget,
getPendingLocaleRefreshTarget, refreshTarget, pendingLocale.
In `@tests/unit/i18n/locale-server-translations.test.ts`:
- Around line 33-36: The current per-line split misses multi-line calls like
getTranslations(\n "xxx"; change the detection to run a regex over the full
file content (use a dotall/s flag or pattern that spans lines) to find all
occurrences of getTranslations\(\s*['"] and then for each match compute the line
number by counting newlines in content.slice(0, match.index) + 1; produce the
same `${file}:${lineNumber}: ${lineTrim}` output (use the matched substring or
its first line for the text). Update the code that builds the array (replace the
split/map/filter pipeline) to use this full-content regex search so multi-line
calls are captured while preserving correct line numbers.
---
Outside diff comments:
In
`@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx:
- Around line 397-400: The "Clear all quotas" button visibility check omits the
total cost quota so it won't show when only total is set; update the JSX
conditional that builds this visibility (the expression referencing
currentQuota?.cost5h.limit, costWeekly.limit, costMonthly.limit, and
concurrentSessions.limit) to also include the total quota checks—e.g. include
currentQuota?.costTotal.limit and/or (currentQuota?.limitTotalUsd ?? 0) > 0—so
the button appears when a total-cost quota is present; edit the conditional in
edit-key-quota-dialog.tsx where that compound boolean is evaluated to add these
checks.
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 2809-2816: The catch block currently calls cleanupCombinedSignal()
immediately, which prematurely removes the polyfill source abort listeners while
same combinedSignal is reused for HTTP/2 downgrade and proxy-to-direct
fallbacks; change the flow so cleanupCombinedSignal() is only invoked when the
request is truly terminated (after all fallbacks and no further retries), or
alternatively before each fallback recreate a fresh combinedSignal; ensure you
still clear responseTimeoutId early (clearTimeout(responseTimeoutId)) but delay
or move cleanupCombinedSignal() to the final termination branch or to the code
path that abandons further fallbacks (referencing cleanupCombinedSignal,
combinedSignal, and responseTimeoutId).
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Around line 3641-3709: The branch that returns early when there is no billable
usage skips the later live-chain cleanup; call deleteLiveChain(...) before
returning to ensure live chain is removed for cases like image-per-request
responses. Locate the branch around resolveBillableUsageMetricsForCost /
updateRequestCostFromUsage and, just before the early return null, invoke
deleteLiveChain with the same session/messageContext identifiers used elsewhere
(e.g., deleteLiveChain(session) or deleteLiveChain(messageContext.id) as
appropriate), await it and wrap it in try/catch to log any errors so cleanup
always runs even on the "no usage" path.
In `@src/components/ui/__tests__/language-switcher.test.tsx`:
- Around line 1-177: Move the LanguageSwitcher unit test out of its current
source-adjacent location into the canonical unit test directory (keeping the
same filename and test contents such as the describe("LanguageSwitcher") block
and helper functions render and click), update any import/alias references if
they break after moving (LanguageSwitcher, useLocale/usePathname/useRouter
mocks), and ensure the test runner config (vitest file patterns) includes the
new tests/unit/ path so the test still runs with the existing
`@vitest-environment` header.
---
Nitpick comments:
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Line 55: Replace the relative import of combineAbortSignals in forwarder.ts
with the project path alias; update the import statement that currently reads
import { combineAbortSignals } from "./combine-abort-signals"; to use
"@/app/v1/_lib/proxy/combine-abort-signals" so forwarder.ts and
combineAbortSignals use the same alias style and comply with the "@/ -> ./src/"
guideline.
In `@src/repository/leaderboard.ts`:
- Line 367: 当前多处排行榜查询仅按聚合列（例如
orderBy(desc(sql`COALESCE(sum(${usageLedger.costUsd}),
0)`))）排序，会导致在聚合值相同时返回顺序不稳定；请在这些 orderBy 链中追加一个稳定兜底排序键（例如实体的唯一标识符/名称或 model id，如
user.id、project.id、model.name
等），把它作为最终的排序条件以保证确定性并减少前端抖动；定位并修改所有出现该模式的函数/表达式（例如包含
orderBy(desc(sql`COALESCE(sum(${usageLedger.costUsd}), 0)`))
的查询以及评论中提到的其它相似行），将稳定键追加为最后一个排序键。
In `@src/types/provider.ts`:
- Line 467: The property totalCostResetAt on the provider type is currently
optional which conflates "missing" (undefined) with "explicitly no reset"
(null); change the declaration of totalCostResetAt from optional to required
with nullable type (i.e., make it totalCostResetAt: Date | null) and update any
factory/mapping code, constructors, deserializers, and usages that instantiate
or read this type (search for totalCostResetAt occurrences) to explicitly set
null when there is no reset time so consumers can rely on the field always being
present.
In `@tests/unit/repository/leaderboard-provider-metrics.test.ts`:
- Around line 705-741: The test "orders by total cost descending with request
count as tiebreaker" currently uses records with different totalCost so it
doesn't exercise the tie-breaker; update the test data in the chainMocks
(created via createChainMock) to include two records with identical totalCost
(e.g., both "totalCost": "10.0") but different totalRequests (e.g., 5 vs 200),
then call findDailyModelLeaderboard() and assert the result ordering uses
totalRequests as the tiebreaker (expect result[0].model to be the one with
higher totalRequests and result[1] the lower), while retaining the existing
assertions that orderBy (chainMocks[0].orderBy) was called with the sum/count
args to ensure both behavior and query-shape are verified.
In `@tests/unit/settings/prices-layout.test.ts`:
- Around line 13-15: The tests in prices-layout.test.ts currently assert exact
className substrings (uses expect(source).toContain(...)) which binds to class
order and will break if classes are reordered; update the assertions that
reference the variable source (the two expect lines checking className="mx-auto
w-full max-w-7xl" and className="min-w-0 space-y-6") to assert at the token
level instead — split the class string into tokens or use a regex/utility that
verifies each expected class token exists (e.g., check that "mx-auto", "w-full",
"max-w-7xl" are present and that "min-w-0" and "space-y-6" are present) so order
changes won’t fail the test.
🪄 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: 041949ba-af89-4d9d-86a3-ccf832ab3c19
📒 Files selected for processing (60)
messages/en/quota.jsonmessages/ja/quota.jsonmessages/ru/quota.jsonmessages/zh-CN/quota.jsonmessages/zh-TW/quota.jsonsrc/actions/key-quota.tssrc/actions/keys.tssrc/actions/providers.tssrc/app/[locale]/dashboard/_components/dashboard-header.tsxsrc/app/[locale]/dashboard/_components/dashboard-sections.tsxsrc/app/[locale]/dashboard/_components/user/key-limit-usage.tsxsrc/app/[locale]/dashboard/audit-logs/page.tsxsrc/app/[locale]/dashboard/availability/page.tsxsrc/app/[locale]/dashboard/layout.tsxsrc/app/[locale]/dashboard/leaderboard/page.tsxsrc/app/[locale]/dashboard/my-quota/page.tsxsrc/app/[locale]/dashboard/providers/page.tsxsrc/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsxsrc/app/[locale]/dashboard/quotas/keys/_components/keys-quota-client.tsxsrc/app/[locale]/dashboard/quotas/keys/_components/keys-quota-manager.tsxsrc/app/[locale]/dashboard/quotas/layout.tsxsrc/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsxsrc/app/[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsxsrc/app/[locale]/dashboard/quotas/providers/_components/providers-quota-manager.tsxsrc/app/[locale]/dashboard/quotas/providers/page.tsxsrc/app/[locale]/dashboard/quotas/users/page.tsxsrc/app/[locale]/dashboard/rate-limits/page.tsxsrc/app/[locale]/settings/_lib/nav-items.tssrc/app/[locale]/settings/client-versions/page.tsxsrc/app/[locale]/settings/config/page.tsxsrc/app/[locale]/settings/error-rules/page.tsxsrc/app/[locale]/settings/layout.tsxsrc/app/[locale]/settings/logs/page.tsxsrc/app/[locale]/settings/prices/_components/price-list.tsxsrc/app/[locale]/settings/prices/page.tsxsrc/app/[locale]/settings/providers/page.tsxsrc/app/[locale]/settings/request-filters/page.tsxsrc/app/[locale]/settings/sensitive-words/page.tsxsrc/app/[locale]/settings/status-page/page.tsxsrc/app/[locale]/status/[slug]/page.tsxsrc/app/[locale]/usage-doc/layout.tsxsrc/app/v1/_lib/proxy/combine-abort-signals.tssrc/app/v1/_lib/proxy/error-handler.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/response-handler.tssrc/components/ui/__tests__/language-switcher.test.tsxsrc/components/ui/language-switcher.tsxsrc/lib/langfuse/emit-proxy-trace.tssrc/lib/langfuse/trace-proxy-request.tssrc/lib/utils/quota-helpers.tssrc/repository/leaderboard.tssrc/types/provider.tstests/integration/billing-model-source.test.tstests/unit/actions/providers-usage.test.tstests/unit/i18n/locale-server-translations.test.tstests/unit/langfuse/langfuse-trace.test.tstests/unit/proxy/combine-abort-signals.test.tstests/unit/proxy/error-handler-langfuse-trace.test.tstests/unit/repository/leaderboard-provider-metrics.test.tstests/unit/settings/prices-layout.test.ts
| vi.mock("@/repository/statistics", () => ({ | ||
| sumProviderCostInTimeRange: (providerId: number, startTime: Date, endTime: Date) => | ||
| sumProviderCostInTimeRangeMock(providerId, startTime, endTime), | ||
| sumProviderTotalCost: (providerId: number) => sumProviderTotalCostMock(providerId), |
Contributor
There was a problem hiding this comment.
[MEDIUM] [TEST-INCOMPLETE] sumProviderTotalCost mock drops resetAt, so total-limit semantics are untestable
Why this is a problem: The production helper is sumProviderTotalCost(providerId, resetAt), where resetAt controls the “total cost since reset” window. This mock only forwards providerId, so tests cannot assert that getProviderLimitUsage* passes provider.totalCostResetAt correctly, and would still pass even if that argument were accidentally omitted/changed.
Suggested fix:
vi.mock("@/repository/statistics", () => ({
sumProviderCostInTimeRange: (providerId: number, startTime: Date, endTime: Date) =>
sumProviderCostInTimeRangeMock(providerId, startTime, endTime),
sumProviderTotalCost: (providerId: number, resetAt?: Date | null) =>
sumProviderTotalCostMock(providerId, resetAt),
}));
// In at least one test (getProviderLimitUsage and batch):
const totalCostResetAt = new Date(nowMs - 60_000);
findProviderByIdMock.mockResolvedValue({
...mockProvider,
totalCostResetAt,
limitTotalUsd: 500,
});
const result = await getProviderLimitUsage(1);
expect(sumProviderTotalCostMock).toHaveBeenCalledWith(1, totalCostResetAt);
if (result.ok) expect(result.data.limitTotalUsd.current).toBe(0);
Contributor
There was a problem hiding this comment.
Code Review Summary
This is an XL release PR touching proxy billing/tracing, quota limits, and multiple dashboard/settings routes. Review focused on new/modified logic and accompanying tests; one test gap was found that could allow regressions in total-usage reset semantics.
PR Size: XL
- Lines changed: 2078
- Files changed: 60
- Split suggestion (XL): Consider keeping each of the 8 included fixes as its own PR (i18n refresh, per-request billing, Langfuse error traces, total quota, abort-signal cleanup, leaderboard ordering, NULL ordering, pricing layout) and merge via a release branch/tag to reduce review surface.
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 | 1 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
None.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
- Identified current PR as
#1130(release v0.7.4) and applied labelsize/XL. - Left 1 inline review comment (Medium severity) at
tests/unit/actions/providers-usage.test.ts:41about thesumProviderTotalCostmock dropping theresetAtargument (tests can’t validate total-usage reset semantics); included a concrete code fix + assertion snippet. - Submitted the required PR review summary via
gh pr review --comment(includes XL split suggestion and issue table).
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/v1/_lib/proxy/response-handler.ts (1)
3641-3709:⚠️ Potential issue | 🟠 Major无 usage 的按次计费分支漏掉了 live-chain 清理。
这次把“成功但没有 usage 的按次计费响应”纳入了这个早退分支，但 Line 3709 直接返回，跳过了后面正常路径里的
deleteLiveChain(...)。结果这类请求会一直残留在 live-chain / requesting 视图里，仪表盘状态会挂住。建议修改
await updateMessageRequestDetails(messageContext.id, { statusCode: statusCode, ...(errorMessage ? { errorMessage } : {}), ttfbMs: session.ttfbMs ?? duration, providerChain: session.getProviderChain(), model: session.getCurrentModel() ?? undefined, actualResponseModel: extractActualResponseModelForProvider( provider.providerType, resolvedIsStream, responseText ), providerId: providerIdForPersistence, context1mApplied: session.getContext1mApplied(), swapCacheTtlApplied: session.provider?.swapCacheTtlBilling ?? false, specialSettings: session.getSpecialSettings() ?? undefined, }); + + if (session.sessionId && session.requestSequence != null) { + if (session.shouldTrackSessionObservability()) { + void deleteLiveChain(session.sessionId, session.requestSequence); + } + } + return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/response-handler.ts` around lines 3641 - 3709, The early-return path for per-request billing when billablePerRequestUsage is falsy skips cleaning up the live-chain, causing requests to remain in the live-chain/requesting view; update the branch surrounding resolveBillableUsageMetricsForCost / the block that sets perRequestCostUsd to ensure deleteLiveChain(...) (the same cleanup used in the normal completion path) is invoked with the appropriate identifiers (e.g., messageContext.id and session) before the function returns, and then return null as before so live-chain entries are removed for "successful but no usage" per-request billing cases.
🧹 Nitpick comments (2)
src/actions/providers.ts (1)
2905-2922: 批量总额度查询可考虑做聚合降本这里在每个 provider 循环内新增了一次
sumProviderTotalCost聚合查询，provider 数量大时会线性放大数据库压力。建议后续在 repository 层增加批量总额聚合接口，再一次性回填。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/providers.ts` around lines 2905 - 2922, The per-provider call to sumProviderTotalCost inside the Promise.all loop causes N DB aggregations when iterating providers; implement a batched aggregation in the repository (e.g., add a new method like sumTotalCostForProviderIds(providerIds, resetAt?) that returns total costs keyed by providerId), call that once before looping/providers Promise.all, and replace individual sumProviderTotalCost(provider.id, ...) usages by looking up the precomputed value for each provider.id; keep existing calls to RateLimitService.getCurrentCost and sumProviderCostInTimeRange unchanged.tests/unit/actions/providers-usage.test.ts (1)
206-212: 建议再补一层返回值断言，覆盖limitTotalUsd映射现在主要校验了调用参数，建议顺带断言
limitTotalUsd.current/limit/resetAt，避免“参数传对但返回组装出错”的漏检。可选补充示例
it("should pass total cost reset time to sumProviderTotalCost", async () => { const { getProviderLimitUsage } = await import("@/actions/providers"); + sumProviderTotalCostMock.mockResolvedValueOnce(123.4); - await getProviderLimitUsage(1); + const result = await getProviderLimitUsage(1); expect(sumProviderTotalCostMock).toHaveBeenCalledWith(1, mockProvider.totalCostResetAt); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.data.limitTotalUsd.current).toBe(123.4); + expect(result.data.limitTotalUsd.limit).toBe(mockProvider.limitTotalUsd); + expect(result.data.limitTotalUsd.resetAt).toEqual(mockProvider.totalCostResetAt); + } }); it("should pass each provider total reset time to sumProviderTotalCost", async () => { const { getProviderLimitUsageBatch } = await import("@/actions/providers"); + sumProviderTotalCostMock.mockResolvedValueOnce(11.1).mockResolvedValueOnce(22.2); - await getProviderLimitUsageBatch(mockProviders); + const result = await getProviderLimitUsageBatch(mockProviders); expect(sumProviderTotalCostMock).toHaveBeenCalledWith(1, mockProviders[0].totalCostResetAt); expect(sumProviderTotalCostMock).toHaveBeenCalledWith(2, mockProviders[1].totalCostResetAt); + expect(result.get(1)?.limitTotalUsd.current).toBe(11.1); + expect(result.get(2)?.limitTotalUsd.current).toBe(22.2); });Also applies to: 455-462
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/actions/providers-usage.test.ts` around lines 206 - 212, Add an assertion to verify the returned structure's mapping for limitTotalUsd after calling getProviderLimitUsage: call getProviderLimitUsage(1) as already done, then assert that the returned object's limitTotalUsd.current, limitTotalUsd.limit and limitTotalUsd.resetAt match the expected values (e.g., values derived from sumProviderTotalCostMock return and mockProvider.totalCostResetAt). Locate this in the test around getProviderLimitUsage and sumProviderTotalCostMock usage and add checks that the function's return value correctly maps the sumProviderTotalCost result into limitTotalUsd.current/limit and uses mockProvider.totalCostResetAt for resetAt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/v1/_lib/proxy/error-handler.ts`:
- Around line 252-275: The database log call in finalizeErrorResponse persists
the original statusCode instead of the possibly overridden finalResponse.status,
causing mismatched status codes; update the call to
ProxyErrorHandler.logErrorToDatabase(session, logErrorMessage,
finalResponse.status, null) (or otherwise pass finalResponse.status) so the
persisted status matches the traced/returned response, ensuring consistency
between ProxyErrorHandler.emitErrorTrace, the response returned by
finalizeErrorResponse, and updateMessageRequestDetails.
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 3288-3300: releaseAgent currently combines agent release and
cleanupCombinedSignal but hedge loser paths in abortAttempt and runAttempt never
call it, causing agent refs and polyfill listeners to leak; make
sessionWithTimeout.releaseAgent (and any attemptRuntime.releaseAgent) explicitly
idempotent (guard with a cleaned flag) and then add calls to
attempt.session.releaseAgent?.() (or attemptRuntime.releaseAgent?.()) in all
loser/early-return/cancel branches inside abortAttempt and runAttempt so every
loser attempt always invokes the unified cleanup callback.
---
Outside diff comments:
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Around line 3641-3709: The early-return path for per-request billing when
billablePerRequestUsage is falsy skips cleaning up the live-chain, causing
requests to remain in the live-chain/requesting view; update the branch
surrounding resolveBillableUsageMetricsForCost / the block that sets
perRequestCostUsd to ensure deleteLiveChain(...) (the same cleanup used in the
normal completion path) is invoked with the appropriate identifiers (e.g.,
messageContext.id and session) before the function returns, and then return null
as before so live-chain entries are removed for "successful but no usage"
per-request billing cases.
---
Nitpick comments:
In `@src/actions/providers.ts`:
- Around line 2905-2922: The per-provider call to sumProviderTotalCost inside
the Promise.all loop causes N DB aggregations when iterating providers;
implement a batched aggregation in the repository (e.g., add a new method like
sumTotalCostForProviderIds(providerIds, resetAt?) that returns total costs keyed
by providerId), call that once before looping/providers Promise.all, and replace
individual sumProviderTotalCost(provider.id, ...) usages by looking up the
precomputed value for each provider.id; keep existing calls to
RateLimitService.getCurrentCost and sumProviderCostInTimeRange unchanged.
In `@tests/unit/actions/providers-usage.test.ts`:
- Around line 206-212: Add an assertion to verify the returned structure's
mapping for limitTotalUsd after calling getProviderLimitUsage: call
getProviderLimitUsage(1) as already done, then assert that the returned object's
limitTotalUsd.current, limitTotalUsd.limit and limitTotalUsd.resetAt match the
expected values (e.g., values derived from sumProviderTotalCostMock return and
mockProvider.totalCostResetAt). Locate this in the test around
getProviderLimitUsage and sumProviderTotalCostMock usage and add checks that the
function's return value correctly maps the sumProviderTotalCost result into
limitTotalUsd.current/limit and uses mockProvider.totalCostResetAt for resetAt.
🪄 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: 72c732e6-0184-4b3e-9f2d-af58fe6bd202
📒 Files selected for processing (10)
src/actions/providers.tssrc/app/v1/_lib/proxy/error-handler.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/response-handler.tssrc/components/ui/__tests__/language-switcher.test.tsxsrc/components/ui/language-switcher.tsxtests/unit/actions/providers-usage.test.tstests/unit/i18n/locale-server-translations.test.tstests/unit/proxy/error-handler-langfuse-trace.test.tstests/unit/proxy/proxy-forwarder-provider-session-release.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/unit/i18n/locale-server-translations.test.ts
- src/components/ui/tests/language-switcher.test.tsx
- tests/unit/proxy/error-handler-langfuse-trace.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integration/billing-model-source.test.ts (1)
1177-1288: 建议抽取负向场景的公共 Arrange 逻辑，减少重复维护成本。Line 1178-1219、Line 1222-1267、Line 1270-1315 三段在 mock/session 初始化上高度重复，后续改动容易漏改。
可选重构示例
+async function setupImageNoBillingCase(options: { + priceLookup: "zero" | "throws" | "normal"; + responseFactory: () => Response; + sessionId: string; + messageId: number; +}) { + invalidateSystemSettingsCache(); + const originalModel = "gpt-image-2"; + const redirectedModel = "gpt-image-2-all"; + + vi.mocked(getSystemSettings).mockResolvedValue(makeSystemSettings("original")); + vi.mocked(findLatestPriceByModel).mockImplementation(async (modelName: string) => { + if (options.priceLookup === "throws") throw new Error("pricing db unavailable"); + if (options.priceLookup === "zero") { + return makePriceRecord(modelName, { input_cost_per_request: 0 }, "manual"); + } + return makePriceRecord(modelName, { input_cost_per_request: 0.01 }, "manual"); + }); + + vi.mocked(updateMessageRequestCostWithBreakdown).mockResolvedValue(undefined); + vi.mocked(SessionManager.updateSessionUsage).mockResolvedValue(undefined); + vi.mocked(RateLimitService.trackCost).mockResolvedValue(undefined); + + const session = createSession({ + originalModel, + redirectedModel, + sessionId: options.sessionId, + messageId: options.messageId, + requestPath: "/v1/images/edits", + providerOverrides: { providerType: "openai", url: "https://api.openai.com/v1" }, + }); + + const clientResponse = await ProxyResponseHandler.dispatch(session, options.responseFactory()); + const responseText = await clientResponse.text(); + await drainAsyncTasks(); + return { clientResponse, responseText }; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/billing-model-source.test.ts` around lines 1177 - 1288, The three tests duplicate the same arrange/setup logic (mocks for getSystemSettings, findLatestPriceByModel, updateMessageRequestDetails, updateMessageRequestDuration, SessionManager.storeSessionResponse, RateLimitService.trackUserDailyCost, SessionTracker.refreshSession, updateMessageRequestCostWithBreakdown, SessionManager.updateSessionUsage, RateLimitService.trackCost plus createSession and createImageEditResponseWithoutUsage calls); extract that into a single helper (e.g., prepareImageEditTest or setupImageEditMocks) that accepts parameters for originalModel, redirectedModel, sessionId, messageId, requestPath and an optional price/mock override, move the repeated vi.mocked(...) and createSession/createImageEditResponseWithoutUsage calls into it, and call this helper from each test to reduce duplication while preserving current mock behaviors and return any objects the tests need (session, clientResponse, responseText) so tests remain unchanged except for invoking the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/integration/billing-model-source.test.ts`:
- Around line 1177-1288: The three tests duplicate the same arrange/setup logic
(mocks for getSystemSettings, findLatestPriceByModel,
updateMessageRequestDetails, updateMessageRequestDuration,
SessionManager.storeSessionResponse, RateLimitService.trackUserDailyCost,
SessionTracker.refreshSession, updateMessageRequestCostWithBreakdown,
SessionManager.updateSessionUsage, RateLimitService.trackCost plus createSession
and createImageEditResponseWithoutUsage calls); extract that into a single
helper (e.g., prepareImageEditTest or setupImageEditMocks) that accepts
parameters for originalModel, redirectedModel, sessionId, messageId, requestPath
and an optional price/mock override, move the repeated vi.mocked(...) and
createSession/createImageEditResponseWithoutUsage calls into it, and call this
helper from each test to reduce duplication while preserving current mock
behaviors and return any objects the tests need (session, clientResponse,
responseText) so tests remain unchanged except for invoking the helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42ff00b6-9860-4c4a-aa3c-521a10d46368
📒 Files selected for processing (1)
tests/integration/billing-model-source.test.ts
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/v1/_lib/proxy/error-handler.ts (1)
205-219: 建议将限流分支也收敛到统一收尾路径，避免后续口径漂移。当前限流分支仍单独执行 trace+写库，和其他错误分支的
finalizeErrorResponse流程不同。虽然现在可用，但后续若修改收尾逻辑，容易出现“返回状态/trace/DB”不一致。建议至少改为基于response.status记录，并补充最终响应体采样，或直接复用统一收尾函数。可选改法（最小化改动）
const response = ProxyErrorHandler.buildRateLimitResponse(error); + let responseText: string | undefined; + try { + responseText = await response.clone().text(); + } catch { + responseText = undefined; + } ProxyErrorHandler.emitErrorTrace(session, { error, errorMessage: logErrorMessage, - statusCode, + statusCode: response.status, + responseText, }); await ProxyErrorHandler.logErrorToDatabase( session, logErrorMessage, - statusCode, + response.status, rateLimitMetadata );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/error-handler.ts` around lines 205 - 219, The rate-limit branch currently calls ProxyErrorHandler.buildRateLimitResponse, ProxyErrorHandler.emitErrorTrace and ProxyErrorHandler.logErrorToDatabase separately which diverges from the common finalizeErrorResponse flow; change it so the rate-limit branch creates the same response object (use buildRateLimitResponse to produce response), then call the unified ProxyErrorHandler.finalizeErrorResponse(session, response, { rateLimitMetadata }) or at minimum have finalizeErrorResponse be responsible for emitting traces and DB writes (use response.status to decide logging semantics and include sampling of the final response body), ensuring emitErrorTrace/logErrorToDatabase are invoked via finalizeErrorResponse rather than independently.src/app/v1/_lib/proxy/forwarder.ts (1)
55-55: 这里继续用了相对导入，和仓库的@/约定不一致。既然这次已经新增了导入，建议直接改成
@/app/v1/_lib/proxy/combine-abort-signals，避免同一模块继续扩散两套导入风格。As per coding guidelines, "Use path alias
@/mapped to ./src/ for imports".🤖 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` at line 55, The import in forwarder.ts currently uses a relative path for combineAbortSignals; update the import to use the project path alias (e.g., change the "./combine-abort-signals" import for combineAbortSignals to the "@/..." alias form used across the repo) so the module uses the unified "@/..." style instead of a relative path.tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts (1)
549-610: 建议再补一个“先 abort、后回填releaseAgent”的竞态用例。现在两个 mocked attempt 都是在返回
Response之前同步写入runtime.releaseAgent，所以这里只验证了“loser 已经拿到 runtime”的路径。更容易回归的是abortAttempt()先跑、runAttempt()到后面才回填releaseAgent的场景；如果没有这个用例，上面的清理泄漏很难被测出来。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts` around lines 549 - 610, Add a new unit test that reproduces the race where abortAttempt() runs before runAttempt() writes runtime.releaseAgent: in ProxyForwarder.send test, mock the second doForward (or one attempt) so it sets runtime.responseController but delays assigning runtime.releaseAgent (e.g., await a small timer or Promise.resolve().then) until after you trigger controller.abort()/abortAttempt(); then later let the delayed assignment run and ensure the delayed releaseAgent is invoked and cleanup still happens (assert mocks.releaseProviderSession called for the cancelled provider, and that releaseInitialAgent/releaseLoserAgent are called the expected number of times). Reference symbols: doForward mock implementations, runtime.releaseAgent, controller.abort()/responseController, abortAttempt/runAttempt behavior, and ProxyForwarder.send/ModelRedirector.apply to locate where to insert the new scenario.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 3419-3432: The bug is that releaseAttemptAgent sets
attempt.agentReleased = true before attempt.releaseAgent may actually be
attached, causing late-arriving losers to be skipped; modify releaseAttemptAgent
to first read attempt.releaseAgent into a local (e.g., const release =
attempt.releaseAgent), if release is undefined do not set agentReleased or call
it, and only set attempt.agentReleased = true after successfully invoking
release (inside the try block after calling release()), so that we don't
short-circuit future releases; also apply the same pattern to the other similar
blocks (the other releaseAttemptAgent occurrences and any code paths at/around
abortAttempt() and doForward()).
---
Nitpick comments:
In `@src/app/v1/_lib/proxy/error-handler.ts`:
- Around line 205-219: The rate-limit branch currently calls
ProxyErrorHandler.buildRateLimitResponse, ProxyErrorHandler.emitErrorTrace and
ProxyErrorHandler.logErrorToDatabase separately which diverges from the common
finalizeErrorResponse flow; change it so the rate-limit branch creates the same
response object (use buildRateLimitResponse to produce response), then call the
unified ProxyErrorHandler.finalizeErrorResponse(session, response, {
rateLimitMetadata }) or at minimum have finalizeErrorResponse be responsible for
emitting traces and DB writes (use response.status to decide logging semantics
and include sampling of the final response body), ensuring
emitErrorTrace/logErrorToDatabase are invoked via finalizeErrorResponse rather
than independently.
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Line 55: The import in forwarder.ts currently uses a relative path for
combineAbortSignals; update the import to use the project path alias (e.g.,
change the "./combine-abort-signals" import for combineAbortSignals to the
"@/..." alias form used across the repo) so the module uses the unified "@/..."
style instead of a relative path.
In `@tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts`:
- Around line 549-610: Add a new unit test that reproduces the race where
abortAttempt() runs before runAttempt() writes runtime.releaseAgent: in
ProxyForwarder.send test, mock the second doForward (or one attempt) so it sets
runtime.responseController but delays assigning runtime.releaseAgent (e.g.,
await a small timer or Promise.resolve().then) until after you trigger
controller.abort()/abortAttempt(); then later let the delayed assignment run and
ensure the delayed releaseAgent is invoked and cleanup still happens (assert
mocks.releaseProviderSession called for the cancelled provider, and that
releaseInitialAgent/releaseLoserAgent are called the expected number of times).
Reference symbols: doForward mock implementations, runtime.releaseAgent,
controller.abort()/responseController, abortAttempt/runAttempt behavior, and
ProxyForwarder.send/ModelRedirector.apply to locate where to insert the new
scenario.
🪄 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: ecb0470e-151d-4f33-ad9b-7b5ab1ed11ca
📒 Files selected for processing (4)
src/app/v1/_lib/proxy/error-handler.tssrc/app/v1/_lib/proxy/forwarder.tstests/unit/proxy/error-handler-langfuse-trace.test.tstests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/proxy/error-handler-langfuse-trace.test.ts
Comment on lines
+3419
to
+3432
| const releaseAttemptAgent = (attempt: StreamingHedgeAttempt) => { | ||
| if (attempt.agentReleased) return; | ||
| attempt.agentReleased = true; | ||
| try { | ||
| attempt.releaseAgent?.(); | ||
| } catch (releaseError) { | ||
| logger.debug("ProxyForwarder: hedge attempt releaseAgent failed", { | ||
| error: releaseError instanceof Error ? releaseError.message : String(releaseError), | ||
| sessionId: attempt.session.sessionId ?? null, | ||
| providerId: attempt.provider.id, | ||
| providerName: attempt.provider.name, | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
agentReleased 置位过早，会漏掉“先取消、后回填 releaseAgent”的 loser。
如果某个 hedge loser 在 abortAttempt() 执行时还没走到 Line 3577 回填 attempt.releaseAgent，这里会先把 agentReleased 设成 true，但实际什么都没释放。等 doForward() 之后再把真正的 releaseAgent 挂上来时，后面的 releaseAttemptAgent() 就直接短路了，这类晚到的 loser 会继续持有 agent ref 和 combined-signal listener。
建议修改
const releaseAttemptAgent = (attempt: StreamingHedgeAttempt) => {
- if (attempt.agentReleased) return;
- attempt.agentReleased = true;
+ if (attempt.agentReleased || !attempt.releaseAgent) return;
+ attempt.agentReleased = true;
try {
- attempt.releaseAgent?.();
+ attempt.releaseAgent();
} catch (releaseError) {
logger.debug("ProxyForwarder: hedge attempt releaseAgent failed", {
error: releaseError instanceof Error ? releaseError.message : String(releaseError),
sessionId: attempt.session.sessionId ?? null,
providerId: attempt.provider.id,
providerName: attempt.provider.name,
});
}
};Also applies to: 3434-3473, 3575-3598
🤖 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 3419 - 3432, The bug is that
releaseAttemptAgent sets attempt.agentReleased = true before
attempt.releaseAgent may actually be attached, causing late-arriving losers to
be skipped; modify releaseAttemptAgent to first read attempt.releaseAgent into a
local (e.g., const release = attempt.releaseAgent), if release is undefined do
not set agentReleased or call it, and only set attempt.agentReleased = true
after successfully invoking release (inside the try block after calling
release()), so that we don't short-circuit future releases; also apply the same
pattern to the other similar blocks (the other releaseAttemptAgent occurrences
and any code paths at/around abortAttempt() and doForward()).
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Line 55: The import of combineAbortSignals in forwarder.ts uses a relative
path; update it to use the repository alias (mapped to ./src/) to match project
guidelines—replace the relative import with the alias form pointing to the same
module (e.g., import from "@/app/v1/_lib/proxy/combine-abort-signals") so
combineAbortSignals continues to be referenced correctly without changing the
symbol name.
- Around line 3297-3302: sessionWithTimeout.releaseAgent is being overwritten on
each doForward which can drop the previous release callback and leak agent
refcounts and combined-signal listeners; instead of overwriting, compose the new
release into the existing one (or push handlers to a small array) so both
previous and current cleanup run: capture the previous
sessionWithTimeout.releaseAgent into a variable, then set
sessionWithTimeout.releaseAgent to a function that first invokes the captured
previousRelease (if any) and then runs the new logic that calls
pool.releaseAgent(agentCacheKeyToRelease, agentDispatcherIdToRelease) and
cleanupCombinedSignal(); reference doForward, sessionWithTimeout.releaseAgent,
pool.releaseAgent, agentCacheKeyToRelease, agentDispatcherIdToRelease and
cleanupCombinedSignal in the change.
🪄 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: 86addaa6-8fe7-41d6-9d82-b5a3ba929890
📒 Files selected for processing (2)
src/app/v1/_lib/proxy/forwarder.tstests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts
| import { rectifyBillingHeader } from "./billing-header-rectifier"; | ||
| import { bindClientAbortListener } from "./client-abort-listener"; | ||
| import { deriveClientSafeUpstreamErrorMessage } from "./client-error-message"; | ||
| import { combineAbortSignals } from "./combine-abort-signals"; |
There was a problem hiding this comment.
新增 import 路径与仓库别名规范不一致
这里新增的是相对路径导入，和仓库统一的 @/ 别名约定不一致，建议改为别名路径以保持一致性和可维护性。
建议修改
-import { combineAbortSignals } from "./combine-abort-signals";
+import { combineAbortSignals } from "@/app/v1/_lib/proxy/combine-abort-signals";As per coding guidelines **/*.{ts,tsx,js,jsx}: Use path alias @/ mapped to ./src/ for imports.
📝 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
| import { combineAbortSignals } from "./combine-abort-signals"; | |
| import { combineAbortSignals } from "@/app/v1/_lib/proxy/combine-abort-signals"; |
🤖 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` at line 55, The import of
combineAbortSignals in forwarder.ts uses a relative path; update it to use the
repository alias (mapped to ./src/) to match project guidelines—replace the
relative import with the alias form pointing to the same module (e.g., import
from "@/app/v1/_lib/proxy/combine-abort-signals") so combineAbortSignals
continues to be referenced correctly without changing the symbol name.
Comment on lines
+3297
to
+3302
| sessionWithTimeout.releaseAgent = () => { | ||
| if (pool && agentCacheKeyToRelease && agentDispatcherIdToRelease) { | ||
| pool.releaseAgent(agentCacheKeyToRelease, agentDispatcherIdToRelease); | ||
| }; | ||
| } | ||
| } | ||
| cleanupCombinedSignal(); | ||
| }; |
There was a problem hiding this comment.
避免 releaseAgent 被覆盖导致重试路径泄漏
sessionWithTimeout.releaseAgent 每次 doForward 都会重写。当前在“doForward 成功返回后、但外层内容校验判定失败并重试”的路径里，旧回调可能在 response-handler 介入前被覆盖，导致上一轮 agent refcount 与 combined-signal listener 都没释放。
这会在连续 fake-200/空响应重试时累积泄漏。
建议修改
const agentCacheKeyToRelease = proxyConfig?.cacheKey ?? directConnectionCacheKey;
const agentDispatcherIdToRelease = proxyConfig?.dispatcherId ?? directConnectionDispatcherId;
const pool = agentCacheKeyToRelease && agentDispatcherIdToRelease ? getGlobalAgentPool() : null;
+ const previousReleaseAgent = sessionWithTimeout.releaseAgent;
+ if (previousReleaseAgent) {
+ try {
+ previousReleaseAgent();
+ } catch (releaseError) {
+ logger.debug("ProxyForwarder: previous releaseAgent failed before overwrite", {
+ error: releaseError instanceof Error ? releaseError.message : String(releaseError),
+ providerId: provider.id,
+ providerName: provider.name,
+ });
+ }
+ }
sessionWithTimeout.releaseAgent = () => {
if (pool && agentCacheKeyToRelease && agentDispatcherIdToRelease) {
pool.releaseAgent(agentCacheKeyToRelease, agentDispatcherIdToRelease);
}
cleanupCombinedSignal();
};🤖 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 3297 - 3302,
sessionWithTimeout.releaseAgent is being overwritten on each doForward which can
drop the previous release callback and leak agent refcounts and combined-signal
listeners; instead of overwriting, compose the new release into the existing one
(or push handlers to a small array) so both previous and current cleanup run:
capture the previous sessionWithTimeout.releaseAgent into a variable, then set
sessionWithTimeout.releaseAgent to a function that first invokes the captured
previousRelease (if any) and then runs the new logic that calls
pool.releaseAgent(agentCacheKeyToRelease, agentDispatcherIdToRelease) and
cleanupCombinedSignal(); reference doForward, sessionWithTimeout.releaseAgent,
pool.releaseAgent, agentCacheKeyToRelease, agentDispatcherIdToRelease and
cleanupCombinedSignal in the change.
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
4 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.7.4 - A maintenance release containing 8 bug fixes and improvements across i18n, billing, proxy stability, quota management, and dashboard accuracy.
Included PRs
1. fix(i18n): refresh server chrome on locale switch (#1116)
Problem: After switching locale, server-rendered navigation headings and descriptions could display stale translations while client components updated correctly.
Solution: Pass
[locale]route param explicitly to all server-sidegetTranslationscalls and add a locale-switch refresh guard inLanguageSwitcherto re-fetch the RSC tree after locale navigation.Files:
language-switcher.tsx,dashboard-header.tsx,settings/layout.tsx, and 18 other route pages2. fix(billing): charge redirected per-request image responses (#1119)
Problem: Image API requests (OpenAI Images API) without token usage data were bypassing billing when model redirects occurred with per-request pricing configured.
Solution: Introduced
resolveBillableUsageMetricsForCost()to synthesize billable usage metrics for successful responses without token usage wheninput_cost_per_requestis configured.Files:
response-handler.ts,billing-model-source.test.ts3. fix: trace proxy errors in Langfuse (#1120)
Problem: Proxy errors (rate limits, network errors, upstream errors) were not being traced in Langfuse, making debugging difficult.
Solution: Extracted shared
emitProxyLangfuseTracehelper and wired it intoProxyErrorHandlerto emit traces for all error paths before database persistence.Files:
emit-proxy-trace.ts,error-handler.ts,response-handler.ts,error-handler-langfuse-trace.test.ts4. fix(dashboard): order model leaderboard by total cost (#1127)
Problem: Model leaderboard was sorting by request count instead of cost, causing expensive low-volume models to rank below cheap high-volume ones.
Solution: Changed
ORDER BYfromcount(*)tosum(cost_usd) DESCwithcount(*)as tiebreaker.Files:
leaderboard.ts,leaderboard-provider-metrics.test.ts5. fix total quota (#1126)
Problem: The quota system lacked a lifetime/total cost limit, preventing administrators from setting hard budget caps.
Solution: Added
limitTotalUsd(total cost limit) support for both providers and API keys, with UI components and i18n translations for all 5 languages.Files:
providers.ts,quota-helpers.ts,provider-quota-list-item.tsx,keys-quota-client.tsx,edit-key-quota-dialog.tsx,messages/*/quota.json6. fix(proxy): clean up combined abort signal listeners (#1121)
Problem: Memory leak in
AbortSignal.anypolyfill where abort listeners were never detached on normal request completion, preventing GC of session and request body.Solution: Extracted
combine-abort-signals.tshelper returning explicit{ signal, cleanup }pair; integrated cleanup into all exit paths.Files:
combine-abort-signals.ts,forwarder.ts,combine-abort-signals.test.ts7. fix(leaderboard): ORDER BY 添加 COALESCE 以统一 NULL 排序行为 (#1128)
Problem: Leaderboard queries used
COALESCE(sum(costUsd), 0)in SELECT but baresum(costUsd)in ORDER BY, causing NULL groups to sort first in PostgreSQL DESC.Solution: Added
COALESCE(sum(costUsd), 0)to all 5 leaderboard ORDER BY clauses for consistent NULL handling.Files:
leaderboard.ts8. fix(settings): constrain pricing table layout (#1129)
Problem: Wide pricing tables could overflow the centered max-width container, breaking horizontal layout on smaller screens.
Solution: Added
min-w-0to content area and enabled horizontal scrolling on pricing table withoverflow-x-auto.Files:
settings/layout.tsx,price-list.tsx,prices-layout.test.tsBreaking Changes
None. All changes are backward-compatible bug fixes.
Testing
bun run build- Production buildbun run lint- Biome lintingbun run typecheck- TypeScript type checkingbun run test- Unit and integration tests (5222+ tests)Migration
No migration required. Deploy and restart services.
Release PR for v0.7.4
Greptile Summary
This release bundles 8 targeted bug fixes: an AbortSignal listener memory leak in the proxy, per-request image billing for redirect scenarios, Langfuse tracing for error paths, leaderboard sort-order corrections (cost vs. count, COALESCE NULL handling), a lifetime quota (
limitTotalUsd) for providers and API keys, an i18n stale-RSC refresh guard, and a pricing-table layout overflow fix. All changes are backward-compatible and covered by new unit/integration tests.Confidence Score: 5/5
Safe to merge; all changes are backward-compatible bug fixes with thorough test coverage. The single finding is a Langfuse observability gap that does not affect billing correctness.
Only P2 findings present (incomplete costUsd in one Langfuse trace within finalizeRequestStats). Billing DB writes, quota enforcement, leaderboard ordering, and the memory-leak fix are all correct. 5000+ tests pass.
src/app/v1/_lib/proxy/response-handler.ts — the perRequestCostUsd computed in finalizeRequestStats is not forwarded to the existing emitProxyLangfuseTrace at the end of the function.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Proxy Request] --> B{Response Type} B -->|SSE / Streaming| C[processStreamingResponse] B -->|Non-Streaming| D[processNonStreamingResponse] B -->|Gemini Passthrough| E[Gemini passthrough handler] C --> F[finalizeDeferredStreamingFinalization] D --> G[resolveBillableUsageMetricsForCost] E --> G F --> G G -->|has token usage| H[return usageMetrics] G -->|no token usage + per-request price| I[return sentinel 0,0] G -->|no token usage + no price config| J[return null - no billing] H --> K[updateRequestCostFromUsage] I --> K J --> L[skip billing] K --> M[trackCostToRedis] M --> N[emitProxyLangfuseTrace] B -->|Error path| O[ProxyErrorHandler] O --> P[emitErrorTrace via emitProxyLangfuseTrace] P --> Q[logErrorToDatabase] A --> R[combineAbortSignals] R -->|native AbortSignal.any| S[NOOP cleanup] R -->|polyfill| T[explicit cleanup via releaseAgent]Prompt To Fix All With AI
Reviews (5): Last reviewed commit: "fix(proxy): release late hedge loser age..." | Re-trigger Greptile