Conversation
…#876) * feat(providers): add sub-navigation and Options tab to provider form sidebar Add Scheduling, Circuit Breaker, Timeout sub-items under their parent tabs in the desktop sidebar for quick scroll access. Promote Options to a top-level tab. Includes scroll tracking, i18n (5 langs), and 13 tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(providers): add Active Time sub-item under Options tab Add activeTime sub-navigation for quick scroll access to Scheduled Active Time section. Also add variant="highlight" to Options SectionCard for consistent visual styling with other main sections. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(providers): show green ShieldCheck icon when proxy is configured Separate icon with tooltip next to endpoint count on provider list. Visible only when proxyUrl is set. i18n for 5 languages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(providers): fix 7 review issues in sub-nav and form architecture - Derive TAB_CONFIG, TAB_ORDER, NAV_ORDER, PARENT_MAP from NAV_CONFIG (DRY) - Add sub-nav to tablet and mobile breakpoints - Move activeSubTab from useState into form reducer - Compute Options tab status from routing state instead of hardcoding - Lift TooltipProvider from per-item to list container level - Fix RU i18n: singular form for timeout label - Add 8 new tests covering derived constants and responsive sub-nav Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(providers): extract OptionsSection, perf fixes, batch dialog fix - Extract OptionsSection from RoutingSection (963 -> 432 lines) - Throttle scroll handler via requestAnimationFrame - Merge double dispatch into single SET_ACTIVE_NAV action - Derive sectionRefs from NAV_ORDER instead of manual record - Add NAV_BY_ID lookup map for O(1) tablet/mobile nav access - Add excludeTabs prop to FormTabNav, hide Options in batch dialog - Clean up setTimeout/rAF refs on unmount Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(providers): add Options tab to batch edit dialog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(providers): show proxy badge for providers without resolved vendor Move ShieldCheck proxy indicator out of the vendor-specific branch so it renders for all providers with a configured proxyUrl. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(providers): remove dead SET_ACTIVE_SUB_TAB, add status grouping comments Address Gemini Code Assist review findings: - Remove unused SET_ACTIVE_SUB_TAB action type and reducer case (superseded by SET_ACTIVE_NAV) - Add grouping comments to options tab status conditional for readability Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(providers): address CodeRabbit review findings - Fix zh-CN/zh-TW i18n terminology consistency (供应商/供應商 instead of 提供者) - Add activeTimeEnd check to options tab status indicator - Add focus-visible ring to tablet/mobile sub-nav buttons for accessibility Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(providers): use i18n fallback for unknown tag errors and scrollend event for scroll lock - Replace raw reason string fallback with tUI("unknownError") i18n key in tag validation callbacks (routing-section.tsx) - Add "unknownError" key to tagInput namespace in all 5 locales - Use scrollend event with { once: true } + 1000ms fallback timer instead of fixed 500ms setTimeout for scroll lock release (index.tsx) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(providers): add unit tests for OptionsSection component (25 tests) Covers rendering, conditional display by provider type (claude/codex/gemini), batch mode, dispatch actions, active time UI, disabled state, edit mode IDs, and batch-only badges. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(providers): remove stale scrollend listener on rapid tab clicks Store previous scrollend listener in a ref and remove it at the start of each scrollToSection call, preventing premature unlock when multiple smooth scrolls overlap during fast sequential tab clicks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(providers): fix 3 style-level review findings - Remove dead subSectionRefs.options property from OptionsSection (parent div in index.tsx already tracks this ref) - Use filteredNav.find() instead of NAV_BY_ID for tablet/mobile sub-row lookup so excludeTabs is respected; remove unused NAV_BY_ID - Replace non-null assertion with guarded clearTimeout in scrollend handler Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): resolve 3 pre-existing test failures from PR #873 All caused by commit 2e663cd which changed billing model and session API without updating tests/translations: - i18n: add missing `prices.badges.multi` key to ja/ru/zh-TW locales - tests: update cost-calculation expectations to match full-request pricing (all tokens at premium rate when context > 200K threshold) - tests: fix lease-decrement session mock to use getResolvedPricingByBillingSource instead of removed method Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(types): narrow Recharts v3 dataKey type for React Key/ReactNode compat Recharts v3 widened `dataKey` to `string | number | ((obj: any) => any) | undefined`, which is incompatible with React `Key` and `ReactNode`. Wrap with `String()` in 2 files to satisfy tsgo in CI. Pre-existing issue from main, not introduced by this branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: format code (feat-providers-list-3-0732ba6) * ci: retrigger CI after auto-format fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(types): narrow Recharts ValueType for formatter call in chart.tsx Removing the explicit .map() parameter type exposed ValueType (string | number | readonly (string|number)[]) which is too wide for the formatter's (string | number) parameter. Cast item.value. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(types): restore narrowing annotation for Recharts v3 tooltip map Bring back the explicit type annotation on .map() callback but extend dataKey to include ((obj: any) => any) to match Recharts v3 DataKey<any>. This keeps value/name narrowed for formatter compatibility while making the annotation assignable from TooltipPayloadEntry. Replaces the previous approach of removing the annotation entirely, which exposed ValueType and NameType width issues one by one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(types): use inline casts instead of annotation for Recharts v3 compat Remove the .map() parameter annotation entirely (it causes TS2345 due to contravariance — our narrower type is not assignable from TooltipPayloadEntry). Instead, let TS infer the full type and cast only at the two call sites where formatter needs narrower types: item.value as string|number, item.name as string. All other usages of item.dataKey, item.name, item.value are compatible with the wider Recharts v3 types (String() wraps, template literals, ReactNode). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: John Doe <johndoe@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Red: 复现透传请求体回归 * fix: strip transfer-encoding from forwarded upstream requests Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix: preserve original request body bytes for raw passthrough endpoints When bypassForwarderPreprocessing=true and session.request.buffer is available, use the original ArrayBuffer directly instead of JSON.stringify(messageToSend). This preserves whitespace, key ordering, and trailing newlines in the forwarded request body. --------- Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Simplify providerSupportsModel() to treat all provider types uniformly. Previously, claude-* models were hardcoded to only route to claude/claude-auth providers, even when other providers explicitly declared them in allowedModels. Now the logic is: explicit allowedModels/modelRedirects match -> accept; empty allowedModels -> wildcard; otherwise reject. Format compatibility remains enforced by checkFormatProviderTypeCompatibility independently.
- Export providerSupportsModel for direct unit testing - Add table-driven tests (13 cases) covering all provider/model combinations - Add pickRandomProvider path tests verifying format+model check interaction - Add findReusable integration tests for session reuse scenarios - Add invariant comment in findReusable documenting format-safety assumption
In opaque mode, validateAuthToken() rejected raw ADMIN_TOKEN passed via cookie or Authorization header, breaking programmatic API access that worked in legacy/dual mode. Add a constantTimeEqual check for the admin token before returning null, so server-side env secret still authenticates while regular API keys remain correctly rejected.
* feat: add response input rectifier for /v1/responses input normalization
OpenAI Responses API input field supports string shortcut, single object,
and array formats. This rectifier normalizes non-array input to standard
array format before the guard pipeline, with audit trail via special
settings persisted to message_requests.
- Normalize string input to [{role:"user",content:[{type:"input_text",text}]}]
- Wrap single object input (with role/type) into array
- Convert empty string to empty array
- Passthrough array input unchanged
- Default enabled, toggleable via system settings UI
- Broadened format-mapper body detection for string/object input
- Full i18n support (en, zh-CN, zh-TW, ja, ru)
- 14 unit tests covering all normalization paths
* fix: address bugbot review comments on response-input-rectifier PR
- Remove redundant ternary in rectifyResponseInput (both branches "other")
- Revert body-based detection broadening in detectClientFormat to prevent
misrouting /v1/embeddings and other endpoints with string input fields
- Add missing enableBillingHeaderRectifier to returning clause (pre-existing)
- Refine i18n descriptions to specify "single message object" (5 locales)
- Add 4 normalizeResponseInput tests covering settings gate and audit trail
…861) (#886) * feat(leaderboard): add user model drill-down and user insights page (#861) - Add user-level modelStats data layer with null model preservation - Cache key isolation for user scope includeModelStats - Admin-only includeUserModelStats API gate (403 for non-admin) - Extract shared ModelBreakdownColumn/Row UI primitive - Admin-only expandable user rows with per-model breakdown - User name links to /dashboard/leaderboard/user/[userId] - Admin-only user insights page with overview cards, key trend chart, and model breakdown - Server actions for getUserInsightsOverview, KeyTrend, ModelBreakdown - Full i18n support (zh-CN, zh-TW, en, ja, ru) - 54 new tests across 6 test files * chore: format code (feat-861-user-leaderboard-drill-down-81697a4) * fix(leaderboard): resolve bugbot review findings for user insights (#886) - Fix critical bug: overview card labeled "Cache Hit Rate" was showing todayErrorRate; renamed to "Error Rate" with correct i18n (5 locales) - Fix security: Cache-Control now private when allowGlobalUsageView=false - Add date validation (YYYY-MM-DD regex + range check) in getUserInsightsModelBreakdown server action - Normalize key trend date field to ISO string to prevent client crash when cache returns Date objects - Replace hardcoded "OK" button with tCommon("ok") i18n key - Replace hardcoded "Calls" chart label with tStats("requests") - Tighten userId validation to positive integers only - Add NULLIF(TRIM()) to model field for consistency with leaderboard.ts - Use machine-readable error code for 403 response - Strengthen return type from unknown to DatabaseKeyStatRow[] - Update tests: assert errorRate metric, date normalization, date validation, and error code assertions * test: use realistic DatabaseKeyStatRow fields in key trend test mock Mock data now matches the actual DatabaseKeyStatRow interface with key_id, key_name, api_calls, and total_cost fields instead of generic cost/requests. Assertions verify the full data contract. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
- Remove `deletedAt IS NULL` filter from buildWhereConditions: cleanup should delete ALL matching records regardless of soft-delete status - Add `RETURNING 1` to DELETE SQL for driver-agnostic row counting (result.length instead of fragile count/rowCount properties) - Add `FOR UPDATE SKIP LOCKED` to prevent deadlocks with concurrent jobs - Add purgeSoftDeleted: batched hard-delete of soft-deleted records as fallback after main cleanup loop - Add VACUUM ANALYZE after deletions to reclaim disk space (failure is non-fatal) - Update CleanupResult with softDeletedPurged and vacuumPerformed - Pass new fields through API route and show in UI toast - Add i18n keys for 5 languages - 19 tests covering all new behavior
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Wrap DISTINCT ON subquery so the outer ORDER BY sorts by updatedAt DESC, showing recently updated models first in the paginated price table.
…hart colors - Fix chart colors: replace hsl(var(--chart-N)) with var(--chart-N) to resolve oklch incompatibility causing gray fallback - Fix leaderboard username link styling: remove hyperlink appearance while keeping clickability - Add provider breakdown component with repository, server action, and side-by-side layout alongside model breakdown - Add unified filter bar (time preset, key, provider, model) controlling key trend chart and both breakdown panels - Lift filter state to UserInsightsView, refactor child components to accept filter props instead of managing internal state - Extend repository layer with filter params (keyId, providerId, model) for both model and provider breakdown queries - Add i18n keys for all 5 languages (en, zh-CN, zh-TW, ja, ru) - Add comprehensive tests for provider breakdown action and filter utils
#894) * fix(proxy): hedge first-byte timeout failover and clear stale bindings * docs(schema): clarify timeout field comments * fix(proxy): handle hedge launcher failures and endpoint errors * test(validation): add zero timeout disable case
…chain Implements comprehensive observability for hedge (speculative execution) and client abort scenarios: Backend (forwarder.ts): - Record hedge_triggered when threshold timer fires and alternative provider launches - Record hedge_winner when a provider wins the hedge race (first byte received) - Record hedge_loser_cancelled when a provider loses and gets aborted - Record client_abort when client disconnects (replaces generic system_error) Frontend (provider-chain-popover.tsx, LogicTraceTab.tsx): - Add icons and status colors for all 4 new reason types - Correctly count hedge_triggered as informational (not actual request) - Display hedge flow with GitBranch/CheckCircle/XCircle/MinusCircle icons Langfuse (trace-proxy-request.ts): - Add hedge_winner to SUCCESS_REASONS set - Add client_abort to ERROR_REASONS set - Create hedge-trigger event observation with WARNING level i18n: - Add translations for 4 new reasons across 5 languages (en, zh-CN, zh-TW, ja, ru) - Include timeline, description, and reason label translations Tests: - Add 22 new tests covering all new reason types - Test isActualRequest(), getItemStatus(), isSuccessReason(), isErrorReason() Related improvements: - Add isProviderFinalized() utility to detect when provider info is reliable - Show in-progress state in logs table and big-screen for unfinalised requests - Prevent displaying stale provider names during hedge/fallback transitions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… logs table Consolidate the isActualRequest function that was duplicated across three files (provider-chain-formatter, provider-chain-popover, virtualized-logs-table). Export from provider-chain-formatter.ts as the single source of truth. Fixes two bugs in virtualized-logs-table.tsx: - successfulProvider lookup was missing hedge_winner, causing incorrect costMultiplier display for hedge-won requests - Inline isActualRequest was missing hedge_winner, hedge_loser_cancelled, client_abort and other newer reasons, causing incorrect request count and cost badge visibility
When the hedge timer fires and no alternative provider is found, let the sole in-flight request continue instead of aborting it with a 524 error. Only call finishIfExhausted() when all attempts have already completed (edge case).
…tives This commit addresses three issues with hedge race tracking: 1. Decision chain missing hedge participants - Add hedge_launched reason to record alternative provider launches - Record hedge_launched in forwarder.ts when launchedProviderCount > 1 - Add timeline formatting for hedge_launched events 2. Hedge races mislabeled as retries in UI - Add isHedgeRace() and getRetryCount() helper functions - Display "Hedge Race" badge instead of retry count - Update provider-chain-popover and virtualized-logs-table 3. hedge_winner false positives - Fix isActualHedgeWin logic to only check launchedProviderCount - Prevent marking single-provider requests as hedge_winner Additional improvements: - Strengthen addProviderToChain deduplication logic - Add comprehensive JSDoc for getRetryCount design decisions - Add 6 edge case tests (empty chain, incomplete hedge, mixed scenarios) - Test coverage: 54 → 60 tests, all passing i18n: Add hedge_launched translations for 5 languages (en, zh-CN, zh-TW, ja, ru) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove text label from hedge race badge, keeping only the GitBranch icon for a more compact display. The aria-label still contains the full text for accessibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
实现批量编辑供应商时自动预填充已有设置的功能： 核心功能： - 创建 deep-equals.ts 实现深度比较工具 - 创建 analyze-batch-settings.ts 分析批量供应商设置 - 修改 provider-form-context.tsx 在批量模式下预填充表单 分析逻辑： - uniform: 所有供应商值相同时显示该值 - mixed: 供应商值不同时使用默认值 - empty: 所有供应商未设置时使用默认值 测试覆盖： - 单元测试：deepEquals 深度比较（13个测试） - 单元测试：analyzeBatchProviderSettings 分析器（14个测试） - 集成测试：批量编辑预填充（3个测试） 所有测试通过，类型检查通过，构建成功。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 2 implementation: UI enhancements for batch edit prefill Changes: - Created MixedValueIndicator component to show when selected providers have different values - Added i18n translations for mixed value indicators (5 languages: zh-CN, zh-TW, en, ja, ru) - Modified provider-form-context to export batch analysis results - Integrated mixed value indicators in routing-section (priority, weight, cost multiplier, model redirects) - Integrated mixed value indicators in limits-section (all rate limits, circuit breaker settings) - Enhanced LimitCard component to support mixed value display The mixed value indicator appears below fields when: - Batch mode is active - Selected providers have different values for that field - Shows up to 5 different values with "...and N more" for additional values Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add missing translation keys for settings.providers.batchEdit namespace: - Batch operation toolbar (selectedCount, actions, enterMode, etc.) - Edit/delete/reset circuit dialogs (dialog.*) - Preview step with field labels (preview.*, fields.*) - Toast notifications (toast.*) - Undo operations (undo.*) - Confirmation buttons (confirm.*) Covers all UI strings used in provider-batch-actions.tsx, provider-batch-dialog.tsx, provider-batch-preview-step.tsx, and provider-batch-toolbar.tsx components. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…#890) * feat: add costResetAt for soft user limit reset without deleting data Add costResetAt timestamp column to users table that clips all cost calculations to start from reset time instead of all-time. This enables admins to reset a user's rate limits without destroying historical usage data (messageRequest/usageLedger rows are preserved). Key changes: - Schema: new cost_reset_at column on users table - Repository: costResetAt propagated through all select queries, key validation, and statistics aggregation (with per-user batch support) - Rate limiting: all 12 proxy guard checks pass costResetAt; service and lease layers clip time windows accordingly - Auth cache: hydrate costResetAt from Redis cache as Date; invalidate auth cache on reset to avoid stale costResetAt - Actions: resetUserLimitsOnly sets costResetAt + clears cost cache; getUserLimitUsage/getUserAllLimitUsage/getKeyLimitUsage/getMyQuota clip time ranges by costResetAt - UI: edit-user-dialog with separate Reset Limits Only (amber) vs Reset All Statistics (red) with confirmation dialogs - i18n: all 5 languages (en, zh-CN, zh-TW, ja, ru) - Tests: 10 unit tests for resetUserLimitsOnly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: harden costResetAt handling -- repo layer, DRY Redis cleanup, Date validation - Extract resetUserCostResetAt repo function with updatedAt + auth cache invalidation - Extract clearUserCostCache helper to deduplicate Redis cleanup between reset functions - Use instanceof Date checks in lease-service and my-usage for costResetAt validation - Remove dead hasActiveSessions variable in cost-cache-cleanup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: unify costResetAt guards to instanceof Date, add last-reset badge in user edit dialog R3: Replace truthiness checks with `instanceof Date` in 3 places (users.ts clipStart, quotas page). R4: Show last reset timestamp in edit-user-dialog Reset Limits section (5 langs). Add 47 unit tests covering costResetAt across key-quota, redis cleanup, statistics, and auth cache. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: apply costResetAt clipStart to key-quota usage queries Clip all period time ranges by user's costResetAt and replace getTotalUsageForKey with sumKeyTotalCost supporting resetAt parameter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: format branch files with biome, suppress noThenProperty in thenable mock Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR #853 review findings -- guard consistency, window clipping, error handling - keys.ts: eliminate redundant findUserById call, use joined costResetAt + instanceof Date guard - users.ts: handle resetUserCostResetAt return value (false = soft-deleted user) - service.ts: add instanceof Date guard to costResetAt comparison - statistics.ts: fix sumKeyTotalCost/sumUserTotalCost to use max(resetAt, maxAgeCutoff) instead of replacing maxAgeDays; refactor nested ternaries to if-blocks in quota functions - cost-cache-cleanup.ts: wrap pipeline.exec() in try/catch to honor never-throws contract - Update test for pipeline.exec throw now caught inside clearUserCostCache Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: tighten key-quota clipStart assertions with toHaveBeenNthCalledWith Verify each window (5h/daily/weekly/monthly) is clipped individually instead of checking unordered calledWith matches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: repair CI test failures caused by costResetAt feature changes - ja/dashboard.json: replace fullwidth parens with halfwidth - api-key-auth-cache-reset-at.test: override CI env so shouldUseRedisClient() works - key-quota-concurrent-inherit.test: add logger.info mock, sumKeyTotalCost mock, userCostResetAt field - my-usage-concurrent-inherit.test: add logger.info/debug mocks - total-usage-semantics.test: update call assertions for new costResetAt parameter - users-reset-all-statistics.test: mock resetUserCostResetAt, update pipeline.exec error expectations - rate-limit-guard.test: add cost_reset_at: null to expected checkCostLimitsWithLease args Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR #890 review comments for costResetAt feature Critical: - Wrap resetUserAllStatistics DB writes in transaction for atomicity - Change sumKeyTotalCost maxAgeDays from 365 to Infinity for unbounded accumulation from costResetAt - Add costResetAtMs to BudgetLease cache with stale detection Medium: - Add logger.warn to silent Redis scan failure handlers - Add fail-open documentation for costResetAt validation - Fix test mock leak (vi.resetAllMocks + re-establish defaults) Minor: - Rename i18n "Reset Data" to "Reset Options" across 5 languages - Remove brittle source code string assertion tests - Update test assertions for transaction and Infinity changes * chore: format code (fix-user-reset-stats-e5002db) --------- Co-authored-by: John Doe <johndoe@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix: resolve client restriction checkboxes not working in create user dialog Use useRef to track latest nested form state synchronously, preventing stale state overwrites when AccessRestrictionsSection fires multiple onChange calls within the same event handler. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add wildcard glob pattern support for custom client restrictions Patterns with '*' use full-string glob matching (case-insensitive, literal characters). Patterns without '*' retain existing substring matching with normalization for backward compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add popover multi-select for Claude Code sub-client restrictions Adds granular selection of individual Claude Code sub-clients (CLI, VS Code, SDK-TS, SDK-PY, CLI-SDK, GitHub Action) via a popover dropdown on the Claude Code preset row. Auto-consolidates to parent "claude-code" when all 6 sub-clients are selected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: delegate client restrictions UI to shared editor component Remove ~200 lines of duplicated preset/popover logic from AccessRestrictionsSection by reusing ClientRestrictionsEditor. Fixes i18n bug where sub-client "All" label used wrong translation key (presetClients["sub-all"] instead of subClients?.all). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: replace regex glob with linear matcher to prevent ReDoS Replace globToRegex (regex-based, vulnerable to catastrophic backtracking on patterns like *a*a*a*a*c) with globMatch (two-pointer linear algorithm). Remove globCache since regex compilation is no longer needed. Add adversarial tests: consecutive wildcards, regex metacharacters as literals, and a performance guard for pathological patterns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(i18n): correct parentheses and restrict wildcard to client editors - ja/dashboard.json: fullwidth -> halfwidth parentheses (CI test) - zh-TW/dashboard.json: halfwidth -> fullwidth in subClients (CI test) - tag-input.tsx: remove * from DEFAULT_TAG_PATTERN (was leaking to all consumers) - client-restrictions-editor.tsx: explicit validateTag with * for client inputs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ui): disable inactive preset popover and allow dots/slashes in client patterns - Disable sub-client popover button when preset is neither allowed nor blocked - Extend validateTag regex to accept . and / for real-world UA patterns (e.g. my.tool/1.0) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ui): require subClients/nSelected props and document glob anchoring - Make subClients and nSelected required in component interfaces (all callers already provide them; removes hardcoded "All" fallbacks per i18n rules) - Update customHelp in all 10 i18n files to document glob anchoring behavior (use *foo* to match anywhere) - Update test fixtures with required translation props Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: format code (fix-ui-client-restriction-4-8d4ad51) * style: fix formatting and extract shared client tag pattern constant Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: John Doe <johndoe@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
Important Review skippedToo many files! This PR contains 215 files, which is 65 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (215)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-actions
bot
added
size/XL
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 release introduces significant performance and monitoring enhancements, primarily through concurrent provider racing and a new user insights dashboard. It also refines administrative controls with improved UI for client restrictions and provider management, alongside critical bug fixes for API routing and billing accuracy. The update aims to provide a more robust, efficient, and transparent system for managing AI service consumption. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+3144
to
+3158
| launchedProviderIds.add(provider.id); | ||
| launchedProviderCount += 1; | ||
|
|
||
| let endpointSelection: { | ||
| endpointId: number | null; | ||
| baseUrl: string; | ||
| endpointUrl: string; | ||
| }; | ||
| try { | ||
| endpointSelection = await ProxyForwarder.resolveStreamingHedgeEndpoint(session, provider); | ||
| } catch (endpointError) { | ||
| lastError = endpointError as Error; | ||
| await launchAlternative(); | ||
| await finishIfExhausted(); | ||
| return; |
There was a problem hiding this comment.
launchedProviderCount inflated before endpoint resolution succeeds
launchedProviderIds and launchedProviderCount are both incremented at lines 3144–3145, before resolveStreamingHedgeEndpoint is awaited (line 3153). If that call throws (e.g. the vendor-type circuit is open or no endpoint candidates exist), the count is already 1 and the function immediately calls launchAlternative(), which will increment the count to 2.
Later, inside commitWinner, the winning alternative is classified using:
const isActualHedgeWin = launchedProviderCount > 1;Because the count is 2 due to the failed endpoint-resolution step (not a real in-flight network race), the provider chain entry will be logged with reason: "hedge_winner" even though no concurrent race actually took place — it was a plain failover triggered by endpoint selection failure. This can make the audit trail misleading for operators reviewing the decision chain.
Consider only incrementing launchedProviderCount after successful endpoint resolution, or tracking a separate boolean anyRealAttemptLaunched to gate the isActualHedgeWin check:
launchedProviderIds.add(provider.id);
let endpointSelection: { ... };
try {
endpointSelection = await ProxyForwarder.resolveStreamingHedgeEndpoint(session, provider);
} catch (endpointError) {
lastError = endpointError as Error;
await launchAlternative();
await finishIfExhausted();
return;
}
// Only count after a real network attempt is about to launch
launchedProviderCount += 1;Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/forwarder.ts
Line: 3144-3158
Comment:
**`launchedProviderCount` inflated before endpoint resolution succeeds**
`launchedProviderIds` and `launchedProviderCount` are both incremented at lines 3144–3145, *before* `resolveStreamingHedgeEndpoint` is awaited (line 3153). If that call throws (e.g. the vendor-type circuit is open or no endpoint candidates exist), the count is already 1 and the function immediately calls `launchAlternative()`, which will increment the count to 2.
Later, inside `commitWinner`, the winning alternative is classified using:
```typescript
const isActualHedgeWin = launchedProviderCount > 1;
```
Because the count is 2 due to the failed *endpoint-resolution* step (not a real in-flight network race), the provider chain entry will be logged with `reason: "hedge_winner"` even though no concurrent race actually took place — it was a plain failover triggered by endpoint selection failure. This can make the audit trail misleading for operators reviewing the decision chain.
Consider only incrementing `launchedProviderCount` *after* successful endpoint resolution, or tracking a separate boolean `anyRealAttemptLaunched` to gate the `isActualHedgeWin` check:
```typescript
launchedProviderIds.add(provider.id);
let endpointSelection: { ... };
try {
endpointSelection = await ProxyForwarder.resolveStreamingHedgeEndpoint(session, provider);
} catch (endpointError) {
lastError = endpointError as Error;
await launchAlternative();
await finishIfExhausted();
return;
}
// Only count after a real network attempt is about to launch
launchedProviderCount += 1;
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+3531
to
+3538
| const { done, value } = await reader.read(); | ||
| if (done) { | ||
| controller.close(); | ||
| return; | ||
| } | ||
| if (value && value.byteLength > 0) { | ||
| controller.enqueue(value); | ||
| } |
There was a problem hiding this comment.
Zero-length chunk not enqueued and stream not closed
When reader.read() returns a chunk with value.byteLength === 0, neither controller.enqueue nor controller.close is called before pull returns. Per the ReadableStream spec, the runtime will call pull again when the consumer's queue is empty, triggering another await reader.read() round-trip.
In practice, AI streaming providers never emit zero-length chunks after the first valid byte, so this is unlikely to cause problems. However, if any upstream ever produces a burst of empty frames (e.g. keep-alive padding), this could result in a loop of no-op reads that delay real data delivery to the client without any observable error.
A defensive guard would make the intent explicit:
const { done, value } = await reader.read();
if (done) {
controller.close();
return;
}
if (value && value.byteLength > 0) {
controller.enqueue(value);
}
// else: skip empty chunk; pull will be called againThe comment on the else branch would at least document that the silent skip is intentional.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/forwarder.ts
Line: 3531-3538
Comment:
**Zero-length chunk not enqueued and stream not closed**
When `reader.read()` returns a chunk with `value.byteLength === 0`, neither `controller.enqueue` nor `controller.close` is called before `pull` returns. Per the `ReadableStream` spec, the runtime will call `pull` again when the consumer's queue is empty, triggering another `await reader.read()` round-trip.
In practice, AI streaming providers never emit zero-length chunks after the first valid byte, so this is unlikely to cause problems. However, if any upstream ever produces a burst of empty frames (e.g. keep-alive padding), this could result in a loop of no-op reads that delay real data delivery to the client without any observable error.
A defensive guard would make the intent explicit:
```typescript
const { done, value } = await reader.read();
if (done) {
controller.close();
return;
}
if (value && value.byteLength > 0) {
controller.enqueue(value);
}
// else: skip empty chunk; pull will be called again
```
The comment on the `else` branch would at least document that the silent skip is intentional.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
113
to
121
| function providerSupportsModel(provider: Provider, requestedModel: string): boolean { | ||
| const isClaudeModel = requestedModel.startsWith("claude-"); | ||
| const isClaudeProvider = | ||
| provider.providerType === "claude" || provider.providerType === "claude-auth"; | ||
|
|
||
| // Case 1: Claude 模型请求 | ||
| if (isClaudeModel) { | ||
| // 1a. Anthropic 提供商 | ||
| if (isClaudeProvider) { | ||
| // 未设置 allowedModels 或为空数组：允许所有 claude 模型 | ||
| if (!provider.allowedModels || provider.allowedModels.length === 0) { | ||
| return true; | ||
| } | ||
| // 检查白名单 | ||
| return provider.allowedModels.includes(requestedModel); | ||
| } | ||
|
|
||
| // 1b. 非 Anthropic 提供商不支持 Claude 模型调度 | ||
| return false; | ||
| } | ||
|
|
||
| // Case 2: 非 Claude 模型请求（gpt-*, gemini-*, 或其他任意模型） | ||
| // 2a. 优先检查显式声明（支持跨类型代理） | ||
| // 原因：允许 Claude 类型供应商通过 allowedModels/modelRedirects 声明支持非 Claude 模型 | ||
| // 场景：Claude 供应商配置模型重定向，将 gemini-* 请求转发到真实的 Gemini 上游 | ||
| const explicitlyDeclared = !!( | ||
| provider.allowedModels?.includes(requestedModel) || provider.modelRedirects?.[requestedModel] | ||
| ); | ||
|
|
||
| if (explicitlyDeclared) { | ||
| return true; // 显式声明优先级最高，允许跨类型代理 | ||
| } | ||
|
|
||
| // 2b. Anthropic 提供商不支持非声明的非 Claude 模型 | ||
| // 保护机制：防止将非 Claude 模型误路由到 Anthropic API | ||
| if (isClaudeProvider) { | ||
| return false; | ||
| } | ||
|
|
||
| // 2c. 非 Anthropic 提供商（codex, gemini, gemini-cli, openai-compatible） | ||
| // allowedModels 是声明列表，用于调度器匹配提供商 | ||
| // 用户可以手动填写任意模型名称（不限于真实模型），用于声明该提供商"支持"哪些模型 | ||
|
|
||
| // 未设置 allowedModels 或为空数组：接受任意模型（由上游提供商判断） | ||
| // 1. 未设置 allowedModels（null 或空数组）：接受任意模型 | ||
| if (!provider.allowedModels || provider.allowedModels.length === 0) { | ||
| return true; | ||
| } | ||
|
|
||
| // 不在声明列表中且无重定向配置（前面已检查过 explicitlyDeclared） | ||
| return false; | ||
| // 2. 设置了 allowedModels：只按原始请求模型做白名单匹配 | ||
| return provider.allowedModels.includes(requestedModel); | ||
| } |
There was a problem hiding this comment.
modelRedirects no longer participates in provider routing
The previous implementation of providerSupportsModel treated a modelRedirects entry as an explicit declaration that the provider supports a given source model, so it was factored into the routing decision alongside allowedModels:
// Old code
const explicitlyDeclared = !!(
provider.allowedModels?.includes(requestedModel) || provider.modelRedirects?.[requestedModel]
);
if (explicitlyDeclared) return true;With the new simplified logic, modelRedirects is no longer consulted here. A provider that has:
allowedModels: ["gpt-4o"]modelRedirects: { "gpt-3.5-turbo": "gpt-4o" }
…will now return false for a gpt-3.5-turbo request because the source model is absent from allowedModels. Requests that used to be silently redirected will fail to route at all until the operator explicitly adds the redirect source model to allowedModels.
This is noted in the PR's BREAKING CHANGE section, but it is a silent behavioral regression — no error is surfaced to users; requests simply fall through to a 503. Consider adding a debug-level log when a provider is skipped because of model mismatch and a modelRedirects entry exists for the requested model, to help operators detect misconfigured providers:
// After the allowedModels check fails:
if (provider.modelRedirects?.[requestedModel]) {
logger.debug(
"providerSupportsModel: model in modelRedirects but not in allowedModels — skipping provider",
{ providerId: provider.id, requestedModel }
);
}
return false;Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/provider-selector.ts
Line: 113-121
Comment:
**`modelRedirects` no longer participates in provider routing**
The previous implementation of `providerSupportsModel` treated a `modelRedirects` entry as an explicit declaration that the provider supports a given source model, so it was factored into the routing decision alongside `allowedModels`:
```typescript
// Old code
const explicitlyDeclared = !!(
provider.allowedModels?.includes(requestedModel) || provider.modelRedirects?.[requestedModel]
);
if (explicitlyDeclared) return true;
```
With the new simplified logic, `modelRedirects` is no longer consulted here. A provider that has:
- `allowedModels: ["gpt-4o"]`
- `modelRedirects: { "gpt-3.5-turbo": "gpt-4o" }`
…will now return `false` for a `gpt-3.5-turbo` request because the source model is absent from `allowedModels`. Requests that used to be silently redirected will fail to route at all until the operator explicitly adds the redirect source model to `allowedModels`.
This is noted in the PR's BREAKING CHANGE section, but it is a silent behavioral regression — no error is surfaced to users; requests simply fall through to a 503. Consider adding a debug-level log when a provider is skipped because of model mismatch *and* a `modelRedirects` entry exists for the requested model, to help operators detect misconfigured providers:
```typescript
// After the allowedModels check fails:
if (provider.modelRedirects?.[requestedModel]) {
logger.debug(
"providerSupportsModel: model in modelRedirects but not in allowedModels — skipping provider",
{ providerId: provider.id, requestedModel }
);
}
return false;
```
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
这是一个重要的版本发布，引入了多项重大功能，例如供应商并发竞速（hedging）、用户洞察页面和限额重置，同时还包含了大量的 UI/UX 优化和错误修复。这些功能的实现看起来相当稳健且经过深思熟虑。proxy-forwarder.ts 中的并发竞速逻辑虽然复杂，但妥善处理了包括客户端中止在内的各种边缘情况。用于重置用户限额的 costResetAt 功能在不同的用量和配额计算模块中得到了一致的应用。提供商模型支持逻辑的简化，提高了代码的清晰度和正确性。为客户端限制添加的 glob 模式匹配也是一个实用的增强功能。总的来说，这是一个高质量的拉取请求，在各个方面都有实质性的改进。
Note: Security Review did not run due to the size of the PR.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR introduces release v0.6.3 with Provider Hedge Racing, User Insights Dashboard, Soft User Limit Reset, Response Input Rectifier, and GPT Long-Context Pricing. The implementation is well-structured with comprehensive test coverage for all new features.
PR Size: XL
- Lines changed: 26,378 (23,587 additions + 2,791 deletions)
- Files changed: 213
Note: This is an extra-large PR. For future releases, consider splitting into smaller PRs by feature area:
- Core proxy changes (hedge racing, response rectifier) - ~8,000 lines
- User insights dashboard - ~4,000 lines
- User limit management (costResetAt) - ~3,000 lines
- UI improvements and i18n - ~6,000 lines
- Database migrations and schema changes - ~1,000 lines
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean (SQL injection protected via Drizzle parameterized queries, admin routes properly authorized, input validated)
- Error handling - Clean (no empty catch blocks, errors logged or propagated)
- Type safety - Clean (TypeScript used throughout, no unsafe
anyin critical paths) - Documentation accuracy - Clean (comments match implementation)
- Test coverage - Adequate (tests exist for hedge racing, response rectifier, user insights, cost reset)
- Code clarity - Good
Notable Implementation Quality
-
Hedge Racing (
src/app/v1/_lib/proxy/forwarder.ts): Well-implemented concurrent provider racing with proper cleanup of losing attempts and session synchronization. -
Response Input Rectifier (
src/app/v1/_lib/proxy/response-input-rectifier.ts): Clean normalization of/v1/responsesinput formats with proper type guards. -
User Insights Authorization (
src/app/[locale]/dashboard/leaderboard/user/[userId]/page.tsx): Proper admin-only access control with userId validation. -
SQL Safety (
src/repository/admin-user-insights.ts): All queries use Drizzle ORM with parameterized statements, preventing SQL injection. -
Soft Reset Pattern (
src/actions/users.ts): ThecostResetAtmechanism provides a clean way to reset limits without destroying historical data.
Automated review by Claude AI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| > { | ||
| const session = await getSession(); | ||
| if (!session || session.user.role !== "admin") { | ||
| return { ok: false, error: "Unauthorized" }; |
Contributor
There was a problem hiding this comment.
[HIGH] [STANDARD-VIOLATION] Hardcoded ActionResult errors bypass i18n
Evidence: src/actions/admin-user-insights.ts:40 contains return { ok: false, error: "Unauthorized" };; similar hardcoded user-facing strings exist at lines 45, 72, 78, 100, 103, 106, 127, 163.
Why this is a problem: CLAUDE.md requires: i18n Required - All user-facing strings must use i18n (5 languages supported). Never hardcode display text.
Suggested fix:
import { getTranslations } from "next-intl/server";
import { ERROR_CODES } from "@/lib/utils/error-messages";
const tError = await getTranslations("errors");
if (!session || session.user.role !== "admin") {
return {
ok: false,
error: tError("UNAUTHORIZED"),
errorCode: ERROR_CODES.UNAUTHORIZED,
};
}
if (!isValidTimeRange(timeRange)) {
return {
ok: false,
error: tError("INVALID_FORMAT", { field: "timeRange" }),
errorCode: ERROR_CODES.INVALID_FORMAT,
errorParams: { field: "timeRange" },
};
}
| export function UserOverviewCards({ userId }: UserOverviewCardsProps) { | ||
| const t = useTranslations("dashboard.leaderboard.userInsights"); | ||
|
|
||
| const { data, isLoading } = useQuery({ |
Contributor
There was a problem hiding this comment.
[HIGH] [ERROR-NO-USER-FEEDBACK] React Query failures render as blank/"no data"
Evidence: src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx:23-47 throws on failure (if (!result.ok) throw new Error(result.error);) but then hides the error with if (!data) return null;.
Same pattern (throw, but no isError/error render path) in:
src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-key-trend-chart.tsx:41-52(falls through tonoData)src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-model-breakdown.tsx:36-69(falls through tonoData)src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-provider-breakdown.tsx:36-69(falls through tonoData)
Why this is a problem: Network/session errors become indistinguishable from empty datasets, and the UI provides no actionable feedback.
Suggested fix:
const tError = useTranslations("errors");
const { data, isLoading, isError, error } = useQuery({
// ...
});
if (isError) {
return (
<div
role="alert"
className="rounded-lg border border-destructive/50 bg-destructive/5 p-3 text-sm text-destructive"
>
{error instanceof Error ? error.message : tError("OPERATION_FAILED")}
</div>
);
}
| usageCache.set(userId, { data: result.data, timestamp: Date.now() }); | ||
| return result.data; | ||
| }) | ||
| .catch(() => null) |
Contributor
There was a problem hiding this comment.
[MEDIUM] [ERROR-SILENT] getSharedUserLimitUsage swallows request errors
Evidence: src/lib/dashboard/user-limit-usage-cache.ts:59 uses .catch(() => null).
Why this is a problem: Exceptions (network/server) become indistinguishable from legitimate "no data" results, and the root cause is lost (no logging, no propagation). This makes the UI fall back silently and complicates debugging.
Suggested fix:
const request = getUserAllLimitUsage(userId)
.then((result) => {
if (!result.ok || !result.data) {
return null;
}
usageCache.set(userId, { data: result.data, timestamp: Date.now() });
return result.data;
})
.catch((error) => {
console.error("[user-limit-usage-cache] getUserAllLimitUsage failed", error);
return null;
})
.finally(() => {
inFlightUsageRequests.delete(userId);
});
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR is XL and spans proxy core, dashboard UI, settings, and database changes, which makes regression risk high and review/rollback difficult. The most actionable issues are (1) i18n rule violations in new server actions and (2) user-facing failures being rendered as blank/"no data" states due to missing React Query error UI.
PR Size: XL
- Lines changed: 26378
- Files changed: 213
- Split suggestions (recommended):
- Proxy pipeline changes (hedge + rectifiers + response/log handling)
- Dashboard leaderboard/user-insights UI + supporting actions/repositories
- User quota reset/costResetAt + related DB migration
- Provider/settings UI + i18n message updates
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 1 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 1 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- None
High Priority Issues (Should Fix)
- [STANDARD-VIOLATION] Hardcoded ActionResult errors bypass i18n:
src/actions/admin-user-insights.ts:40(and other lines noted inline) - [ERROR-NO-USER-FEEDBACK] User-insights widgets throw React Query errors but render blank/"no data":
src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx:23(and related components) - [ERROR-SILENT] Shared limit-usage cache swallows exceptions:
src/lib/dashboard/user-limit-usage-cache.ts:59
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.
- Applied PR label:
size/XL - Left 3 inline review comments at:
src/actions/admin-user-insights.ts:40src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx:23src/lib/dashboard/user-limit-usage-cache.ts:59
- Submitted PR review summary (comment) with the following body:
Code Review Summary
This PR is XL and spans proxy core, dashboard UI, settings, and database changes, which makes regression risk high and review/rollback difficult. The most actionable issues are (1) i18n rule violations in new server actions and (2) user-facing failures being rendered as blank/"no data" states due to missing React Query error UI.
PR Size: XL
- Lines changed: 26378
- Files changed: 213
- Split suggestions (recommended):
- Proxy pipeline changes (hedge + rectifiers + response/log handling)
- Dashboard leaderboard/user-insights UI + supporting actions/repositories
- User quota reset/costResetAt + related DB migration
- Provider/settings UI + i18n message updates
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 1 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 1 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- None
High Priority Issues (Should Fix)
- [STANDARD-VIOLATION] Hardcoded ActionResult errors bypass i18n:
src/actions/admin-user-insights.ts:40(and other lines noted inline) - [ERROR-NO-USER-FEEDBACK] User-insights widgets throw React Query errors but render blank/"no data":
src/app/[locale]/dashboard/leaderboard/user/[userId]/_components/user-overview-cards.tsx:23(and related components) - [ERROR-SILENT] Shared limit-usage cache swallows exceptions:
src/lib/dashboard/user-limit-usage-cache.ts:59
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
When a hedge winner streams, commitWinner() already logs the provider to the chain with reason "hedge_winner" and updates session binding. The deferred finalization was redundantly calling updateSessionBindingSmart, updateSessionProvider, and addProviderToChain again with reason "retry_success", causing the same provider to appear twice in dashboard logs. Add isHedgeWinner flag to DeferredStreamingFinalization so finalization skips these operations when commitWinner() has already performed them.
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Comment on lines
+2873
to
+2878
|
|
||
| const launchedProviderIds = new Set<number>(); | ||
| let launchedProviderCount = 0; | ||
| let settled = false; | ||
| let winnerCommitted = false; | ||
| let noMoreProviders = false; |
There was a problem hiding this comment.
settleFailure can leave resultPromise permanently unresolved
settleFailure sets settled = true synchronously, then awaits clearSessionProviderBinding. If that Redis call throws (e.g., Redis is unavailable), resolveResult?.({ error }) is never reached, so resultPromise never resolves and the entire hedge call hangs indefinitely.
const settleFailure = async (error: Error) => {
if (settled) return;
settled = true;
await ProxyForwarder.clearSessionProviderBinding(session).catch(() => {}); // swallow to guarantee resolve
resolveResult?.({ error });
};The same silent-swallow pattern already used in abortAttempt (attempt.responseController?.abort(...) inside try/catch) should be applied here. Without it, any Redis outage during a hedge failure leaves the client connection open with no response.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/forwarder.ts
Line: 2873-2878
Comment:
**`settleFailure` can leave `resultPromise` permanently unresolved**
`settleFailure` sets `settled = true` synchronously, then `await`s `clearSessionProviderBinding`. If that Redis call throws (e.g., Redis is unavailable), `resolveResult?.({ error })` is never reached, so `resultPromise` never resolves and the entire hedge call hangs indefinitely.
```typescript
const settleFailure = async (error: Error) => {
if (settled) return;
settled = true;
await ProxyForwarder.clearSessionProviderBinding(session).catch(() => {}); // swallow to guarantee resolve
resolveResult?.({ error });
};
```
The same silent-swallow pattern already used in `abortAttempt` (`attempt.responseController?.abort(...)` inside `try/catch`) should be applied here. Without it, any Redis outage during a hedge failure leaves the client connection open with no response.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+270
to
+284
| logger.info({ | ||
| action: "log_cleanup_soft_delete_purge", | ||
| deletedInBatch: deleted, | ||
| totalPurged, | ||
| }); | ||
|
|
||
| if (deleted === batchSize) { | ||
| await sleep(BATCH_SLEEP_MS); | ||
| } | ||
| } | ||
|
|
||
| return totalPurged; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
VACUUM ANALYZE from application code may conflict with autovacuum
Manually calling VACUUM ANALYZE message_request after each cleanup run is typically unnecessary in production PostgreSQL — autovacuum already reclaims dead tuples after bulk deletes. Triggering it from application code has several downsides:
- It can conflict with a concurrently running autovacuum on the same table, causing one to back off.
- While VACUUM doesn't hold an exclusive lock during the heap scan, it does briefly lock during the analysis phase, which can affect query planning on a busy table.
- If Drizzle's connection pool wraps connections in a transaction (or if a middleware does), the call will throw
ERROR: VACUUM cannot run inside a transaction block— this is already acknowledged in the comment, but the implicit reliance ondb.execute()being outside a transaction is fragile.
Consider removing the runVacuum() call and instead adjusting autovacuum_vacuum_cost_limit / autovacuum_vacuum_scale_factor on the message_request table to make autovacuum respond more aggressively after large deletes. If manual vacuuming is truly needed, a separate out-of-band maintenance job is safer.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/log-cleanup/service.ts
Line: 270-284
Comment:
**`VACUUM ANALYZE` from application code may conflict with autovacuum**
Manually calling `VACUUM ANALYZE message_request` after each cleanup run is typically unnecessary in production PostgreSQL — autovacuum already reclaims dead tuples after bulk deletes. Triggering it from application code has several downsides:
- It can conflict with a concurrently running autovacuum on the same table, causing one to back off.
- While VACUUM doesn't hold an exclusive lock during the heap scan, it does briefly lock during the analysis phase, which can affect query planning on a busy table.
- If Drizzle's connection pool wraps connections in a transaction (or if a middleware does), the call will throw `ERROR: VACUUM cannot run inside a transaction block` — this is already acknowledged in the comment, but the implicit reliance on `db.execute()` being outside a transaction is fragile.
Consider removing the `runVacuum()` call and instead adjusting `autovacuum_vacuum_cost_limit` / `autovacuum_vacuum_scale_factor` on the `message_request` table to make autovacuum respond more aggressively after large deletes. If manual vacuuming is truly needed, a separate out-of-band maintenance job is safer.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+3480
to
+3505
| const exists = mergedProviderChain.some( | ||
| (existing) => | ||
| existing.id === item.id && | ||
| existing.timestamp === item.timestamp && | ||
| existing.reason === item.reason && | ||
| existing.attemptNumber === item.attemptNumber | ||
| ); | ||
| if (!exists) { | ||
| mergedProviderChain.push(item); | ||
| } | ||
| } | ||
| targetState.providerChain = mergedProviderChain; | ||
| targetState.specialSettings = [...sourceState.specialSettings]; | ||
| targetState.originalModelName = sourceState.originalModelName; | ||
| targetState.originalUrlPathname = sourceState.originalUrlPathname; | ||
| targetState.clearResponseTimeout = sourceRuntime.clearResponseTimeout; | ||
| targetState.responseController = sourceRuntime.responseController; | ||
| } | ||
|
|
||
| private static async clearSessionProviderBinding(session: ProxySession): Promise<void> { | ||
| if (!session.sessionId) return; | ||
| await SessionManager.clearSessionProvider(session.sessionId); | ||
| } | ||
|
|
||
| private static async readFirstReadableChunk( | ||
| reader: ReadableStreamDefaultReader<Uint8Array> |
There was a problem hiding this comment.
Shadow session cloning relies on TypeScript-private fields being JS-enumerable
createStreamingShadowSession creates the shadow via:
const shadow = Object.assign(
Object.create(Object.getPrototypeOf(session)) as ProxySession,
session
);This works because TypeScript's private keyword produces regular, enumerable JS properties — so Object.assign copies them. However, if ProxySession ever introduces a field using the native JS #privateField syntax (truly private, non-enumerable), that field would silently not be copied and the shadow would be in a broken state with no compile-time or runtime warning.
The existing code has private readonly endpointPolicy and private originalModelName which are safe today, but this coupling to the TS-private implementation detail is fragile and worth a note or a factory method on ProxySession itself (ProxySession.createShadow(source, provider)) that can be kept in sync when fields change.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/forwarder.ts
Line: 3480-3505
Comment:
**Shadow session cloning relies on TypeScript-private fields being JS-enumerable**
`createStreamingShadowSession` creates the shadow via:
```typescript
const shadow = Object.assign(
Object.create(Object.getPrototypeOf(session)) as ProxySession,
session
);
```
This works because TypeScript's `private` keyword produces regular, enumerable JS properties — so `Object.assign` copies them. However, if `ProxySession` ever introduces a field using the native JS `#privateField` syntax (truly private, non-enumerable), that field would silently not be copied and the shadow would be in a broken state with no compile-time or runtime warning.
The existing code has `private readonly endpointPolicy` and `private originalModelName` which are safe today, but this coupling to the TS-private implementation detail is fragile and worth a note or a factory method on `ProxySession` itself (`ProxySession.createShadow(source, provider)`) that can be kept in sync when fields change.
How can I resolve this? If you propose a fix, please make it concise.- Move launchedProviderCount increment after endpoint resolution to prevent inflated count on resolution failure (hedge_winner misclass) - Add comment documenting intentional zero-length chunk skip - Add error logging to user-limit-usage-cache instead of silent swallow - Add React Query error states to 4 leaderboard user insight components - Add loadError i18n key across all 5 locales - Add trailing newlines to SQL migration files for POSIX compliance - Add test for endpoint resolution failure not inflating hedge count - Add test for cache error logging behavior
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Comment on lines
+135
to
+138
| .innerJoin(providers, eq(usageLedger.finalProviderId, providers.id)) | ||
| .where(and(...conditions)) | ||
| .groupBy(providers.id, providers.name) | ||
| .orderBy(desc(sql`sum(${usageLedger.costUsd})`)); |
There was a problem hiding this comment.
INNER JOIN silently drops null-provider ledger rows
getUserProviderBreakdown uses an INNER JOIN against the providers table. Any usageLedger row where finalProviderId is NULL (e.g. requests that failed before a provider was selected, or legacy rows) is excluded from the query entirely.
This means the sum of all costs in the provider breakdown will be lower than the sum in getUserModelBreakdown for the same user and date range, creating a silent discrepancy on the User Insights dashboard that can confuse admins reviewing the numbers.
Consider using a LEFT JOIN here, grouping the null case under "Unknown provider", to keep the two breakdowns consistent:
.from(usageLedger)
.leftJoin(providers, eq(usageLedger.finalProviderId, providers.id))
// then handle null providers.id in the select / groupByPrompt To Fix With AI
This is a comment left during a code review.
Path: src/repository/admin-user-insights.ts
Line: 135-138
Comment:
**INNER JOIN silently drops null-provider ledger rows**
`getUserProviderBreakdown` uses an `INNER JOIN` against the `providers` table. Any `usageLedger` row where `finalProviderId` is `NULL` (e.g. requests that failed before a provider was selected, or legacy rows) is excluded from the query entirely.
This means the sum of all costs in the provider breakdown will be *lower* than the sum in `getUserModelBreakdown` for the same user and date range, creating a silent discrepancy on the User Insights dashboard that can confuse admins reviewing the numbers.
Consider using a `LEFT JOIN` here, grouping the null case under "Unknown provider", to keep the two breakdowns consistent:
```typescript
.from(usageLedger)
.leftJoin(providers, eq(usageLedger.finalProviderId, providers.id))
// then handle null providers.id in the select / groupBy
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+117
to
+120
| if (filters?.model) { | ||
| conditions.push( | ||
| sql`(${usageLedger.model} ILIKE ${filters.model} OR ${usageLedger.originalModel} ILIKE ${filters.model})` | ||
| ); |
There was a problem hiding this comment.
ILIKE pattern not sanitised — wildcards in model names will match unexpectedly
filters.model is interpolated directly as an ILIKE operand. PostgreSQL ILIKE treats % and _ as wildcards, so a model name that happens to contain either character (e.g. gpt-3.5-turbo contains no such chars, but user-supplied filter inputs might) would match a broader set of rows than intended.
While this is an admin-only endpoint, the broader concern is that the model filter stops being an exact match and starts behaving like a search, which can mislead the breakdown charts. Consider escaping the pattern before passing it to ILIKE:
const escapedModel = filters.model.replace(/[%_\\]/g, "\\$&");
conditions.push(
sql`(${usageLedger.model} ILIKE ${escapedModel} ESCAPE '\\' OR ${usageLedger.originalModel} ILIKE ${escapedModel} ESCAPE '\\')`
);Prompt To Fix With AI
This is a comment left during a code review.
Path: src/repository/admin-user-insights.ts
Line: 117-120
Comment:
**ILIKE pattern not sanitised — wildcards in model names will match unexpectedly**
`filters.model` is interpolated directly as an ILIKE operand. PostgreSQL ILIKE treats `%` and `_` as wildcards, so a model name that happens to contain either character (e.g. `gpt-3.5-turbo` contains no such chars, but user-supplied filter inputs might) would match a broader set of rows than intended.
While this is an admin-only endpoint, the broader concern is that the model filter stops being an exact match and starts behaving like a search, which can mislead the breakdown charts. Consider escaping the pattern before passing it to ILIKE:
```typescript
const escapedModel = filters.model.replace(/[%_\\]/g, "\\$&");
conditions.push(
sql`(${usageLedger.model} ILIKE ${escapedModel} ESCAPE '\\' OR ${usageLedger.originalModel} ILIKE ${escapedModel} ESCAPE '\\')`
);
```
How can I resolve this? If you propose a fix, please make it concise.…n logs table The logs table computed finalProvider as the last entry in the provider chain, which returns the hedge loser (cancelled provider) instead of the actual winner. Add getFinalProviderName() utility with 3-tier priority: hedge_winner > last successful request > last entry fallback. Replace inline chain[length-1].name logic in both virtualized-logs-table and usage-logs-table with the new utility.
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Comment on lines
+2992
to
+3010
|
|
||
| await launchingAlternative; | ||
| }; | ||
|
|
||
| const handleAttemptFailure = async (attempt: StreamingHedgeAttempt, error: Error) => { | ||
| if (settled || winnerCommitted || attempt.settled) return; | ||
|
|
||
| attempt.settled = true; | ||
| if (attempt.thresholdTimer) { | ||
| clearTimeout(attempt.thresholdTimer); | ||
| attempt.thresholdTimer = null; | ||
| } | ||
| attempts.delete(attempt); | ||
| lastError = error; | ||
|
|
||
| const errorCategory = await categorizeErrorAsync(error); | ||
| const statusCode = error instanceof ProxyError ? error.statusCode : undefined; | ||
|
|
||
| if (attempt.endpointAudit.endpointId != null) { |
There was a problem hiding this comment.
Stale chain entry added for loser after async gap
handleAttemptFailure sets attempt.settled = true and removes the attempt from the set, then awaits categorizeErrorAsync. During this async gap another attempt can become the winner (commitWinner sets winnerCommitted = true and calls abortAllAttempts). Because abortAttempt skips already-settled attempts, the loser is not aborted again — but when handleAttemptFailure resumes it still executes session.addProviderToChain(attempt.provider, { reason: "retry_failed" ... }) without re-checking winnerCommitted. This appends a misleading "retry_failed" chain entry for an attempt that was effectively a hedge loser, after the winner entry has already been written.
Add a guard immediately after the async gap:
const errorCategory = await categorizeErrorAsync(error);
const statusCode = error instanceof ProxyError ? error.statusCode : undefined;
if (settled || winnerCommitted) return; // winner committed during async gapPrompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/forwarder.ts
Line: 2992-3010
Comment:
**Stale chain entry added for loser after async gap**
`handleAttemptFailure` sets `attempt.settled = true` and removes the attempt from the set, then `await`s `categorizeErrorAsync`. During this async gap another attempt can become the winner (`commitWinner` sets `winnerCommitted = true` and calls `abortAllAttempts`). Because `abortAttempt` skips already-settled attempts, the loser is not aborted again — but when `handleAttemptFailure` resumes it still executes `session.addProviderToChain(attempt.provider, { reason: "retry_failed" ... })` without re-checking `winnerCommitted`. This appends a misleading `"retry_failed"` chain entry for an attempt that was effectively a hedge loser, after the winner entry has already been written.
Add a guard immediately after the async gap:
```typescript
const errorCategory = await categorizeErrorAsync(error);
const statusCode = error instanceof ProxyError ? error.statusCode : undefined;
if (settled || winnerCommitted) return; // winner committed during async gap
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+2948
to
+2975
| if (launchingAlternative) { | ||
| await launchingAlternative; | ||
| return; | ||
| } | ||
|
|
||
| launchingAlternative = (async () => { | ||
| const alternativeProvider = await ProxyForwarder.selectAlternative( | ||
| session, | ||
| Array.from(launchedProviderIds) | ||
| ); | ||
| if (!alternativeProvider) { | ||
| noMoreProviders = true; | ||
| // No alternative providers available — let in-flight attempt(s) continue. | ||
| // If all attempts already completed, settle with last error. | ||
| if (attempts.size === 0) { | ||
| await finishIfExhausted(); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| await startAttempt(alternativeProvider, false); | ||
| })() | ||
| .catch(async (error) => { | ||
| const normalizedError = error instanceof Error ? error : new Error(String(error)); | ||
|
|
||
| logger.error("ProxyForwarder: Hedge failed to launch alternative provider", { | ||
| error: normalizedError, | ||
| sessionId: session.sessionId ?? null, |
There was a problem hiding this comment.
launchAlternative rejection leaks to concurrent waiters
The IIFE assigned to launchingAlternative calls ProxyForwarder.selectAlternative and startAttempt. If either throws an unexpected exception the resulting rejected promise is stored in launchingAlternative. Any concurrent call that already passed the if (launchingAlternative) guard and is awaiting launchingAlternative will receive that rejection. It then propagates out of launchAlternative(), causing the await launchAlternative() line in handleAttemptFailure to throw — and finishIfExhausted() is never reached. If noMoreProviders is true and all attempts have already been deleted from the set at that point, resultPromise is permanently unresolved and the client connection hangs indefinitely.
Guarding the call-site in handleAttemptFailure is the safest fix:
await launchAlternative().catch((err) => {
logger.warn("ProxyForwarder: launchAlternative threw unexpectedly in handleAttemptFailure", { error: err });
});
await finishIfExhausted();Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/forwarder.ts
Line: 2948-2975
Comment:
**`launchAlternative` rejection leaks to concurrent waiters**
The IIFE assigned to `launchingAlternative` calls `ProxyForwarder.selectAlternative` and `startAttempt`. If either throws an unexpected exception the resulting rejected promise is stored in `launchingAlternative`. Any concurrent call that already passed the `if (launchingAlternative)` guard and is `await`ing `launchingAlternative` will receive that rejection. It then propagates out of `launchAlternative()`, causing the `await launchAlternative()` line in `handleAttemptFailure` to throw — and `finishIfExhausted()` is never reached. If `noMoreProviders` is true and all attempts have already been deleted from the set at that point, `resultPromise` is permanently unresolved and the client connection hangs indefinitely.
Guarding the call-site in `handleAttemptFailure` is the safest fix:
```typescript
await launchAlternative().catch((err) => {
logger.warn("ProxyForwarder: launchAlternative threw unexpectedly in handleAttemptFailure", { error: err });
});
await finishIfExhausted();
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+265
to
280
| ); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| logger.error( | ||
| "[RateLimit] Daily rolling window query failed, fallback to database:", | ||
| error | ||
| ); | ||
| return await RateLimitService.checkCostLimitsFromDatabase(id, type, costLimits); | ||
| return await RateLimitService.checkCostLimitsFromDatabase( | ||
| id, | ||
| type, | ||
| costLimits, | ||
| limits.cost_reset_at | ||
| ); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Redis cache hits for period-based limits ignore costResetAt
When Redis holds a cached cost value for the 5h, daily (rolling), or fixed-period windows the code reads it with a plain parseFloat(value) and never applies the costResetAt clip. The costResetAt filter only applies on the database fallback path (checkCostLimitsFromDatabase). Correctness therefore depends entirely on clearUserCostCache having successfully purged all period keys at reset time.
resetUserLimitsOnly wraps the Redis cleanup in a try/catch that continues on failure (by design — "Continue execution - costResetAt already set in DB"). If Redis is temporarily unavailable during a soft-reset, subsequent requests that hit the warm Redis cache will see the pre-reset accumulated cost and can be incorrectly rate-limited (or under-charged) until the cache key TTL expires. Consider logging a warning that enforcement may be stale when the Redis cleanup fails, so operators can manually trigger a flush if needed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/rate-limit/service.ts
Line: 265-280
Comment:
**Redis cache hits for period-based limits ignore `costResetAt`**
When Redis holds a cached cost value for the 5h, daily (rolling), or fixed-period windows the code reads it with a plain `parseFloat(value)` and never applies the `costResetAt` clip. The `costResetAt` filter only applies on the database fallback path (`checkCostLimitsFromDatabase`). Correctness therefore depends entirely on `clearUserCostCache` having successfully purged all period keys at reset time.
`resetUserLimitsOnly` wraps the Redis cleanup in a `try/catch` that continues on failure (by design — "Continue execution - costResetAt already set in DB"). If Redis is temporarily unavailable during a soft-reset, subsequent requests that hit the warm Redis cache will see the pre-reset accumulated cost and can be incorrectly rate-limited (or under-charged) until the cache key TTL expires. Consider logging a warning that enforcement may be stale when the Redis cleanup fails, so operators can manually trigger a flush if needed.
How can I resolve this? If you propose a fix, please make it concise.Add missing i18n keys referenced in batch edit UI components: - fields.isEnabled: noChange/enable/disable options for batch dropdown - affectedProviders: title and overflow text for provider summaries - batchNotes: codexOnly/claudeOnly/geminiOnly badge labels - undo: singleEditSuccess/singleEditUndone/singleDeleteSuccess/singleDeleteUndone toasts
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
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
2 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.3 introducing provider hedge racing for optimized first-byte timeout handling, user insights dashboard, soft user limit reset, and comprehensive bug fixes and UI improvements.
Problem
This release addresses several key areas:
/v1/responses, auth tokens, and request headersSolution
This release implements multiple features and fixes:
/dashboard/leaderboard/user/[userId]with per-model breakdown and trend chartscostResetAtmechanism allows resetting limits without deleting historical usage data/v1/responsesinput format (string/object to array) before guard pipelineChanges
Core Features
/v1/responsesinput formats (string/object → array)Bug Fixes
UI Improvements
Database Changes
0079_easy_zeigeist.sqlenable_response_input_rectifiersystem setting0080_fresh_clint_barton.sqlcost_reset_atcolumn for soft user limit resetBreaking Changes
claude-*models to routing all models using Anthropic API format. Set model whitelist to maintain previous behaviorTesting
Automated Tests
Manual Testing
Test provider hedge racing with multiple providers
Verify user insights page displays correct per-model statistics
Test soft reset preserves historical data
Test
/v1/responseswith string/object input formatsVerify Anthropic provider model routing with and without whitelist
Related
Description enhanced by Claude AI
Greptile Summary
This release (v0.6.3) introduces Provider Hedge Racing, a User Insights Dashboard, Soft User Limit Reset (
costResetAt), a Response Input Rectifier for/v1/responses, and GPT long-context priority pricing — accompanied by 215 changed files and extensive unit/integration test coverage.Key observations from the review:
forwarder.ts): The core engine is well-structured but carries several known edge cases from prior reviews (unresolvedresultPromiseon Redis failure insettleFailure, stale chain entry after async gap inhandleAttemptFailure,launchAlternativerejection leak). A newly identified issue: whenresolveStreamingHedgeEndpointthrows for any candidate, the catch block silently callslaunchAlternative()with no provider-chain entry recorded, creating an audit gap for operators reviewing why a provider was bypassed.costResetAt/ Soft Reset: The DB path correctly clips the query window bycostResetAt. The Redis period-cache path (5 h rolling, daily rolling, fixed period) still reads a raw accumulated value without applying thecostResetAtclip — a gap already noted in prior review that means stale enforcement can persist up to the cache TTL after a reset if Redis cleanup fails.getUserProviderBreakdown: The INNER JOIN onproviderssilently excludes null-finalProviderIdledger rows, causing the provider breakdown total to diverge from the model breakdown total (prior review). The unescapedILIKEonfilters.modelcan broaden the match unexpectedly (prior review).FOR UPDATE SKIP LOCKEDaddition prevents concurrent-job deadlocks. TheVACUUM ANALYZEcall after bulk deletes is acknowledged as risky inside a transaction block (prior review).readLiveChainBatch: Issues N individual Redis GET calls instead of a single pipeline/MGET, which is suboptimal at scale but not incorrect.providerSupportsModelsimplification removesmodelRedirectsfrom routing decisions — operators with redirect-only configs need to add the source model toallowedModels(documented in PR).Confidence Score: 3/5
settleFailurecan hang on Redis failure, (2)launchAlternativerejection can leaveresultPromiseunresolved, (3) period-based Redis cache hits bypasscostResetAtenforcement, (4)getUserProviderBreakdownINNER JOIN silently drops null-provider rows, and (5) newly identified endpoint-resolution failures instartAttemptproduce no chain entry. The accumulation of open edge cases in the hedge engine lowers confidence.Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant ProxyHandler participant HedgeEngine as sendStreamingWithHedge participant ProviderA as Provider A (initial) participant ProviderB as Provider B (hedge) participant Redis Client->>ProxyHandler: POST /v1/chat/completions (stream=true) ProxyHandler->>HedgeEngine: shouldUseStreamingHedge? → true HedgeEngine->>ProviderA: startAttempt(initial, useOriginalSession=true) ProviderA-->>HedgeEngine: HTTP 200 streaming starts Note over HedgeEngine: firstByteTimeout timer starts alt First byte arrives before timeout ProviderA-->>HedgeEngine: first chunk received HedgeEngine->>HedgeEngine: commitWinner(attemptA, firstChunk) HedgeEngine-->>Client: Response stream (from Provider A) else Timeout fires before first byte HedgeEngine->>HedgeEngine: thresholdTriggered → launchAlternative() HedgeEngine->>ProviderB: startAttempt(alternative, useOriginalSession=false) Note over HedgeEngine: Both A and B racing alt Provider B wins (first chunk arrives) ProviderB-->>HedgeEngine: first chunk received HedgeEngine->>HedgeEngine: commitWinner(attemptB, firstChunk) HedgeEngine->>ProviderA: abortAttempt("hedge_loser") HedgeEngine->>Redis: updateSessionBinding(providerB) HedgeEngine-->>Client: Response stream (from Provider B) else Provider A wins after hedge launched ProviderA-->>HedgeEngine: first chunk received HedgeEngine->>HedgeEngine: commitWinner(attemptA, firstChunk) HedgeEngine->>ProviderB: abortAttempt("hedge_loser") HedgeEngine-->>Client: Response stream (from Provider A) end endComments Outside Diff (1)
src/app/v1/_lib/proxy/forwarder.ts, line 484-493 (link)Endpoint-resolution failure in hedge path leaves no chain entry
When
resolveStreamingHedgeEndpointthrows for any provider (initial or alternative), the catch block silently callslaunchAlternative()and returns without adding a chain entry:launchedProviderIdshas already been updated at this point butlaunchedProviderCountis not incremented, so neitherhandleAttemptFailurenorabortAttemptis called for the failed endpoint resolution. The provider chain will show the alternative's success (or failure) without any trace of why the first candidate was skipped.In the successful case (
commitWinnerfires on the winner), an operator reviewing the chain will see a winner appear with no explanation of which previous endpoint was tried and failed. In the total-failure case (finishIfExhaustedsettles with an error), the chain is empty or only carries the session-reuse entry.Consider recording a chain entry before the early return:
Last reviewed commit: 14740d1