支持自动复测已自动禁用的 Codex 渠道 - #4215
Open
FlamesCN wants to merge 6 commits into
Open
Conversation
Codex accounts can recover quota after being auto-disabled, but the existing scheduler skipped disabled channels and the stream-test preference only lived in the temporary test modal state. This change persists a per-channel default stream test flag, adds a monitor setting that lets scheduled tests include auto-disabled channels while still skipping manually disabled ones, and wires the existing auto-enable flow so recovered channels can come back online without manual retesting. Constraint: Reuse existing channel settings JSON and monitor settings without schema changes Rejected: Keep recovery manual via per-channel retest | still leaves recovered Codex accounts offline until an operator notices Confidence: high Scope-risk: moderate Reversibility: clean Directive: Manually disabled channels are intentionally excluded from automatic recovery; do not widen this without product review Tested: go test ./controller ./setting/operation_setting Tested: bun x prettier --check src/components/settings/OperationSetting.jsx src/components/table/channels/modals/EditChannelModal.jsx src/components/table/channels/modals/ModelTestModal.jsx src/hooks/channels/useChannelsData.jsx src/pages/Setting/Operation/SettingsMonitoring.jsx Tested: bun run build Not-tested: go test ./... | existing unrelated failures remain in relay/channel/claude and relay/helper (cherry picked from commit 36db011)
Contributor
WalkthroughAdds per-channel test-stream setting and resolution (request override → stored setting → channel-type default), centralizes auto-test skip logic, extends channel-affinity context/override handling, updates frontend UI/hooks to honor stream defaults and new monitoring option, and expands related tests and templates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant API
participant Controller
participant DB
User->>Frontend: open ModelTestModal / click Test
Frontend->>API: request modal data / trigger test (optional ?stream override)
API->>Controller: handle TestChannel request (with optional override)
Controller->>DB: read channel.OtherSettings (test_stream_enabled)
DB-->>Controller: return settings JSON
Controller->>Controller: resolveChannelTestStream(override, storedSetting, channelTypeDefault)
Controller->>API: perform/queue test with resolved stream param
API-->>Frontend: response (started/result)
Frontend->>User: display banner/status based on resolved stream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/components/settings/OperationSetting.jsx (1)
76-82: Comment placement is misleading.The inline comment
/* 签到设置 */(checkin settings) on line 77 now appears to describe the new monitoring setting on line 78, which is confusing. Consider moving the comment to its proper location.📝 Suggested fix
'monitor_setting.auto_test_channel_enabled': false, - 'monitor_setting.auto_test_channel_minutes': 10 /* 签到设置 */, + 'monitor_setting.auto_test_channel_minutes': 10, 'monitor_setting.auto_test_auto_disabled_channels_enabled': true, + + /* 签到设置 */ 'checkin_setting.enabled': false,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/settings/OperationSetting.jsx` around lines 76 - 82, The inline comment "/* 签到设置 */" is placed after 'monitor_setting.auto_test_channel_minutes' making it look like it documents the monitor setting; move that comment to immediately before (or after) the block starting with 'checkin_setting.enabled' so it clearly applies to the checkin settings; e.g., relocate the comment from near 'monitor_setting.auto_test_channel_minutes' to above 'checkin_setting.enabled' (or next to 'checkin_setting.min_quota'/'checkin_setting.max_quota') so the intent for checkin_setting.* is unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/components/settings/OperationSetting.jsx`:
- Around line 76-82: The inline comment "/* 签到设置 */" is placed after
'monitor_setting.auto_test_channel_minutes' making it look like it documents the
monitor setting; move that comment to immediately before (or after) the block
starting with 'checkin_setting.enabled' so it clearly applies to the checkin
settings; e.g., relocate the comment from near
'monitor_setting.auto_test_channel_minutes' to above 'checkin_setting.enabled'
(or next to 'checkin_setting.min_quota'/'checkin_setting.max_quota') so the
intent for checkin_setting.* is unambiguous.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e824883f-0305-4b43-8c3c-ae80c753734d
📒 Files selected for processing (9)
controller/channel-test.gocontroller/channel_test_logic_test.godto/channel_settings.gosetting/operation_setting/monitor_setting.goweb/src/components/settings/OperationSetting.jsxweb/src/components/table/channels/modals/EditChannelModal.jsxweb/src/components/table/channels/modals/ModelTestModal.jsxweb/src/hooks/channels/useChannelsData.jsxweb/src/pages/Setting/Operation/SettingsMonitoring.jsx
Legacy Codex channels without a persisted test_stream_enabled flag were treated as non-stream by the edit form and scheduled auto-tests. After an operator opened Edit Channel to refresh OAuth and saved, the modal wrote back an explicit false value, which kept scheduled recovery probes on /v1/responses failing with "Stream must be set to true". This change treats a missing test_stream_enabled flag as stream-on for Codex channels in both the backend scheduler and the frontend edit/test flows, while still honoring an explicitly saved false value. Constraint: Existing channels may already rely on an explicit false flag, so missing and false must stay distinguishable Rejected: One-off database backfill for current channels | fixes today's rows but not future edits of other legacy Codex channels Confidence: high Scope-risk: narrow Reversibility: clean Directive: Codex defaults should remain compatibility-aware; do not collapse missing and explicit false back into one path Tested: go test ./controller Tested: bun x prettier --check web/src/components/table/channels/modals/EditChannelModal.jsx web/src/hooks/channels/useChannelsData.jsx Tested: bun run build Not-tested: Manual UI verification before deploy
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/src/hooks/channels/useChannelsData.jsx (2)
43-43: ReuseCODEX_CHANNEL_TYPEfor the remaining Codex branch too.This file now has both
CODEX_CHANNEL_TYPEand a separate57literal inupdateChannelBalance. Keeping both makes the Codex-specific behavior easy to desync later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/channels/useChannelsData.jsx` at line 43, The Codex channel type literal (57) is used directly in updateChannelBalance while a constant CODEX_CHANNEL_TYPE is defined; replace the hardcoded 57 in the updateChannelBalance logic with the CODEX_CHANNEL_TYPE constant to avoid drift, updating any conditional checks or comparisons inside updateChannelBalance (and any helper functions it calls) to reference CODEX_CHANNEL_TYPE instead of the numeric literal.
902-923: Keepstreamoptional unless the user explicitly overrides it.Line 922 always sends
stream=true|false, so this UI path no longer lets the backend apply its new fallback chain (request override -> stored setting -> channel default). That recreates a second source of truth in the client and can drift from persisted settings if the row data is stale. Consider tracking an explicit override flag and only appendingstreamwhen the user actually changed the switch.[scratchpad_end] -->♻️ Direction for the request-building change
- const testChannel = async (record, model, endpointType = '', stream) => { - const resolvedStream = - typeof stream === 'boolean' - ? stream - : getChannelDefaultStreamTest(record); + const testChannel = async (record, model, endpointType = '', streamOverride) => { + const hasStreamOverride = typeof streamOverride === 'boolean'; let url = `/api/channel/test/${record.id}?model=${model}`; if (endpointType) { url += `&endpoint_type=${endpointType}`; } - url += `&stream=${resolvedStream ? 'true' : 'false'}`; + if (hasStreamOverride) { + url += `&stream=${streamOverride ? 'true' : 'false'}`; + }You'd then keep the modal's displayed default separate from whether the user has explicitly overridden it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/channels/useChannelsData.jsx` around lines 902 - 923, The testChannel function currently always appends &stream=true|false which forces the client-side resolvedStream and prevents the backend fallback; change testChannel to accept and use an explicit override flag (e.g., streamOverride or streamIsOverridden) and only append the &stream=... query param when that override is true, leaving the param out when undefined so the backend can apply its fallback chain; locate the logic around resolvedStream, the stream parameter, and the URL construction in testChannel and adjust the API.get URL building to conditionally include stream based on the new override flag while keeping existing checks like shouldStopBatchTestingRef, isBatchTesting, and setTestingModels intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/hooks/channels/useChannelsData.jsx`:
- Line 43: The Codex channel type literal (57) is used directly in
updateChannelBalance while a constant CODEX_CHANNEL_TYPE is defined; replace the
hardcoded 57 in the updateChannelBalance logic with the CODEX_CHANNEL_TYPE
constant to avoid drift, updating any conditional checks or comparisons inside
updateChannelBalance (and any helper functions it calls) to reference
CODEX_CHANNEL_TYPE instead of the numeric literal.
- Around line 902-923: The testChannel function currently always appends
&stream=true|false which forces the client-side resolvedStream and prevents the
backend fallback; change testChannel to accept and use an explicit override flag
(e.g., streamOverride or streamIsOverridden) and only append the &stream=...
query param when that override is true, leaving the param out when undefined so
the backend can apply its fallback chain; locate the logic around
resolvedStream, the stream parameter, and the URL construction in testChannel
and adjust the API.get URL building to conditionally include stream based on the
new override flag while keeping existing checks like shouldStopBatchTestingRef,
isBatchTesting, and setTestingModels intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a1079798-c9d6-45f7-bf7c-bfaaf78cdc3c
📒 Files selected for processing (4)
controller/channel-test.gocontroller/channel_test_logic_test.goweb/src/components/table/channels/modals/EditChannelModal.jsxweb/src/hooks/channels/useChannelsData.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- controller/channel_test_logic_test.go
- controller/channel-test.go
…upstreams Channel affinity was pinning requests to dead cache entries and still treating some quota failures as hard stop conditions even when the new retry toggles were enabled. This makes the retry decision reason-aware, invalidates unusable cached channels, wires the new operation settings into the backend and UI, and adds regression coverage for text-only quota errors such as the upstream usage-limit message. Constraint: Upstream providers do not report quota exhaustion with one stable error code or message Rejected: Match only insufficient_quota code/type | misses providers that only return usage-limit text Confidence: high Scope-risk: moderate Reversibility: clean Directive: Quota-failure matching is a compatibility surface; extend it only with observed upstream evidence Tested: go test ./service -run "TestShouldSkipRetryAfterChannelAffinity|TestGetPreferredChannelByAffinity|TestChannelAffinityHitCodexTemplatePassHeadersEffective" Tested: go test ./controller ./middleware ./service ./setting/... Not-tested: Frontend production build before commit
Claude Code cache reuse on the /v1/messages -> responses compatibility path should be driven by channel-affinity rules and override templates, not by hardcoded metadata parsing. This change adds request-header and nested JSON key extraction, exposes the resolved affinity key to override context, and lets templates sync that key into prompt_cache_key and session_id from the console. Constraint: Operators need to adapt to client header/metadata shape changes without shipping a new binary Rejected: Keep parsing metadata.user_id.session_id in convert.go | brittle to client request format changes Rejected: Add a separate Claude-specific settings surface | duplicates existing channel-affinity and param-override controls Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep Claude cache-affinity behavior driven by key_sources plus override templates; avoid reintroducing client-specific parsing in convert paths Tested: go test ./... Tested: bun run build Tested: production smoke test on 38.76.144.165 for /v1/messages -> gpt-5.4 with metadata-only and header-only session keys; second request hit cache in both cases Not-tested: GitHub PR creation via gh CLI (local auth token invalid)
…sabled Codex requests can hit channel affinity with skip-retry enabled, which unintentionally blocked failover when the preferred channel returned a disabled-channel error. This change classifies disabled-channel failures alongside the existing affinity retry exceptions so the configured retry path can continue selecting another healthy channel.\n\nConstraint: Must preserve skip-retry behavior for unrelated affinity failures\nRejected: Disable skip-retry for the Codex affinity template globally | would broaden retries beyond disabled/quota failures and change existing operator intent\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Keep disabled-channel detection aligned with upstream/user-facing error variants before adding new affinity retry exceptions\nTested: go test ./service -run 'TestShouldSkipRetryAfterChannelAffinity(DisabledChannel|Error_)'\nNot-tested: Full relay integration against live Codex channels
… branch This branch predates the newer channel-affinity retry toggles, so backporting the stale-cache, Claude cache-key, and disabled-channel failover fixes left the settings model without the fields those changes expect. Restoring the missing config surface keeps the backported behavior consistent with dev without reopening older logic paths.\n\nConstraint: The clean branch must remain buildable after backporting later channel-affinity fixes\nRejected: Drop the backported retry logic from this branch | would leave the related PR lane without the disabled-channel failover fix the user asked to carry over\nConfidence: medium\nScope-risk: narrow\nReversibility: clean\nDirective: If more dev-era affinity fixes are backported here, keep the settings schema aligned before testing service behavior\nTested: go test ./service -run 'TestShouldSkipRetryAfterChannelAffinity(DisabledChannel|Error_)'\nNot-tested: Broader branch regression suite
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/pages/Setting/Operation/SettingsChannelAffinity.jsx (1)
152-163:⚠️ Potential issue | 🟡 MinorGuard
nested_pathnormalization against non-string valuesLine 156 can throw if
nested_pathis a truthy non-string value from JSON mode input. Coerce before trimming to avoid runtime crashes in the editor/render path.💡 Suggested fix
+const safeTrim = (v) => String(v ?? '').trim(); + const normalizeKeySource = (src) => { - const type = (src?.type || '').trim(); - const key = (src?.key || '').trim(); - const path = (src?.path || '').trim(); - const nestedPath = (src?.nested_path || src?.nestedPath || '').trim(); + const type = safeTrim(src?.type); + const key = safeTrim(src?.key); + const path = safeTrim(src?.path); + const nestedPath = safeTrim(src?.nested_path ?? src?.nestedPath); if (type === 'gjson') { return { type, key: '', path, nested_path: nestedPath }; } return { type, key, path: '', nested_path: nestedPath }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Setting/Operation/SettingsChannelAffinity.jsx` around lines 152 - 163, normalizeKeySource can throw when src.nested_path (or src.nestedPath) is a non-string truthy value because trim() is called directly; update the normalization to guard/coerce nestedPath to a string before trimming (e.g., check typeof or use String(src?.nested_path || src?.nestedPath || '') then .trim()) and return the coerced nested_path in the returned object so normalizeKeySource always returns strings for nested_path.service/channel_affinity.go (1)
751-783:⚠️ Potential issue | 🟠 MajorReason-aware retry overrides are currently bypassed once affinity has been marked used.
MarkChannelAffinityUsed()storesginKeyChannelAffinitySkipRetry = meta.SkipRetry, and this helper returns that flag before it ever looks atRetryOnDisabledChannel/RetryOnChannelQuotaExceeded. In the normal runtime path that means a rule withSkipRetryOnFailure: truewill still suppress failover for disabled/quota failures, so the new toggles never take effect after an affinity hit.Proposed fix
func shouldSkipRetryAfterChannelAffinityFailureWithReason(c *gin.Context, reason channelAffinityFailureReason) bool { if c == nil { return false } - v, ok := c.Get(ginKeyChannelAffinitySkipRetry) - if ok { - b, ok := v.(bool) - if ok { - return b - } - } meta, ok := getChannelAffinityMeta(c) if !ok { return false } - if !meta.SkipRetry { + + skipRetry := meta.SkipRetry + if v, ok := c.Get(ginKeyChannelAffinitySkipRetry); ok { + if b, ok := v.(bool); ok { + skipRetry = b + } + } + if !skipRetry { return false } setting := operation_setting.GetChannelAffinitySetting() if setting == nil { return true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/channel_affinity.go` around lines 751 - 783, The helper shouldSkipRetryAfterChannelAffinityFailureWithReason currently returns the stored ginKeyChannelAffinitySkipRetry flag before consulting reason-specific toggles; change it to first retrieve the operation_setting via GetChannelAffinitySetting(), and if a setting exists apply the reason-specific RetryOnDisabledChannel / RetryOnChannelQuotaExceeded checks to override skipping (return false when retry is allowed for that reason), otherwise fall back to the saved meta.SkipRetry or ginKeyChannelAffinitySkipRetry; reference shouldSkipRetryAfterChannelAffinityFailureWithReason, MarkChannelAffinityUsed and ginKeyChannelAffinitySkipRetry to locate and update the logic so SkipRetry is not a hard short-circuit that prevents the setting toggles from taking effect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/convert.go`:
- Around line 620-623: The current assignment to claudeResponse.Usage only sets
InputTokens/OutputTokens and drops cache-related fields; update the mapping to
populate dto.ClaudeUsage's CacheCreationInputTokens, CacheReadInputTokens and
the cache split fields (the 5m/1h counters) from the source openAIResponse.Usage
(e.g. openAIResponse.Usage.CacheCreationInputTokens,
openAIResponse.Usage.CacheReadInputTokens,
openAIResponse.Usage.CacheReadInputTokens5m/1h or whatever the exact field names
are) so non-stream Claude responses report the same cache usage as the streaming
path — you can still take the address of openAIResponse.Usage (it’s embedded by
value) when assigning to claudeResponse.Usage.
- Around line 429-447: The branch handling OpenAI responses with
len(openAIResponse.Choices) == 0 is currently gated by
info.ClaudeConvertInfo.Done and therefore never runs because
StreamResponseOpenAI2Claude returns before Done is set; fix it by removing or
changing the Done gate so final usage-only chunks are emitted: inside
StreamResponseOpenAI2Claude, when choices==0 detect the final chunk via
info.FinishReason (or the presence of info.ClaudeConvertInfo.Usage) and call
stopOpenBlocks() and append the message_delta (using
buildClaudeUsageFromOpenAIUsage(info.ClaudeConvertInfo.Usage)) and message_stop
to claudeResponses regardless of info.ClaudeConvertInfo.Done, or set
info.ClaudeConvertInfo.Done earlier before returning so the existing append code
executes; update the logic around stopOpenBlocks(), the construction of
dto.ClaudeResponse (Type/message_delta, Usage, Delta.StopReason) and the
subsequent message_stop append accordingly.
- Around line 237-240: Restore the cache-creation split normalization when
populating usage.CacheCreation: when building dto.ClaudeCacheCreationUsage from
oaiUsage, prefer existing
oaiUsage.ClaudeCacheCreation5mTokens/ClaudeCacheCreation1hTokens if they are
present, but if they are missing or zero and oaiUsage.CacheCreationInputTokens
(aggregate) is set, derive the 5m/1h split using the same normalization logic
used elsewhere (i.e., compute Ephemeral5mInputTokens and Ephemeral1hInputTokens
from CacheCreationInputTokens), then assign those normalized values to
Ephemeral5mInputTokens and Ephemeral1hInputTokens on usage.CacheCreation so
cache_creation is not dropped or inconsistent.
---
Outside diff comments:
In `@service/channel_affinity.go`:
- Around line 751-783: The helper
shouldSkipRetryAfterChannelAffinityFailureWithReason currently returns the
stored ginKeyChannelAffinitySkipRetry flag before consulting reason-specific
toggles; change it to first retrieve the operation_setting via
GetChannelAffinitySetting(), and if a setting exists apply the reason-specific
RetryOnDisabledChannel / RetryOnChannelQuotaExceeded checks to override skipping
(return false when retry is allowed for that reason), otherwise fall back to the
saved meta.SkipRetry or ginKeyChannelAffinitySkipRetry; reference
shouldSkipRetryAfterChannelAffinityFailureWithReason, MarkChannelAffinityUsed
and ginKeyChannelAffinitySkipRetry to locate and update the logic so SkipRetry
is not a hard short-circuit that prevents the setting toggles from taking
effect.
In `@web/src/pages/Setting/Operation/SettingsChannelAffinity.jsx`:
- Around line 152-163: normalizeKeySource can throw when src.nested_path (or
src.nestedPath) is a non-string truthy value because trim() is called directly;
update the normalization to guard/coerce nestedPath to a string before trimming
(e.g., check typeof or use String(src?.nested_path || src?.nestedPath || '')
then .trim()) and return the coerced nested_path in the returned object so
normalizeKeySource always returns strings for nested_path.
🪄 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: 213444b3-da57-4d0f-ad15-23db7ef5be61
📒 Files selected for processing (13)
constant/context_key.gocontroller/relay.gomiddleware/distributor.gorelay/common/override.gorelay/common/override_test.gorelay/common/relay_info.goservice/channel_affinity.goservice/channel_affinity_template_test.goservice/convert.gosetting/operation_setting/channel_affinity_setting.goweb/src/components/table/channels/modals/ParamOverrideEditorModal.jsxweb/src/constants/channel-affinity-template.constants.jsweb/src/pages/Setting/Operation/SettingsChannelAffinity.jsx
✅ Files skipped from review due to trivial changes (2)
- constant/context_key.go
- relay/common/override_test.go
Comment on lines
+237
to
+240
| if oaiUsage.ClaudeCacheCreation5mTokens > 0 || oaiUsage.ClaudeCacheCreation1hTokens > 0 { | ||
| usage.CacheCreation = &dto.ClaudeCacheCreationUsage{ | ||
| Ephemeral5mInputTokens: cacheCreation5m, | ||
| Ephemeral1hInputTokens: cacheCreation1h, | ||
| Ephemeral5mInputTokens: oaiUsage.ClaudeCacheCreation5mTokens, | ||
| Ephemeral1hInputTokens: oaiUsage.ClaudeCacheCreation1hTokens, |
Contributor
There was a problem hiding this comment.
Restore cache-creation split normalization here.
CacheCreationInputTokens can be set even when the 5m/1h split is absent or partial. Copying the raw split fields directly can now emit inconsistent Claude usage, or drop cache_creation entirely when only the aggregate count is known. The other Claude usage path still normalizes these values before exposing them.
🔧 Suggested fix
- if oaiUsage.ClaudeCacheCreation5mTokens > 0 || oaiUsage.ClaudeCacheCreation1hTokens > 0 {
+ cacheCreation5m, cacheCreation1h := NormalizeCacheCreationSplit(
+ usage.CacheCreationInputTokens,
+ oaiUsage.ClaudeCacheCreation5mTokens,
+ oaiUsage.ClaudeCacheCreation1hTokens,
+ )
+ if cacheCreation5m > 0 || cacheCreation1h > 0 {
usage.CacheCreation = &dto.ClaudeCacheCreationUsage{
- Ephemeral5mInputTokens: oaiUsage.ClaudeCacheCreation5mTokens,
- Ephemeral1hInputTokens: oaiUsage.ClaudeCacheCreation1hTokens,
+ Ephemeral5mInputTokens: cacheCreation5m,
+ Ephemeral1hInputTokens: cacheCreation1h,
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if oaiUsage.ClaudeCacheCreation5mTokens > 0 || oaiUsage.ClaudeCacheCreation1hTokens > 0 { | |
| usage.CacheCreation = &dto.ClaudeCacheCreationUsage{ | |
| Ephemeral5mInputTokens: cacheCreation5m, | |
| Ephemeral1hInputTokens: cacheCreation1h, | |
| Ephemeral5mInputTokens: oaiUsage.ClaudeCacheCreation5mTokens, | |
| Ephemeral1hInputTokens: oaiUsage.ClaudeCacheCreation1hTokens, | |
| cacheCreation5m, cacheCreation1h := NormalizeCacheCreationSplit( | |
| usage.CacheCreationInputTokens, | |
| oaiUsage.ClaudeCacheCreation5mTokens, | |
| oaiUsage.ClaudeCacheCreation1hTokens, | |
| ) | |
| if cacheCreation5m > 0 || cacheCreation1h > 0 { | |
| usage.CacheCreation = &dto.ClaudeCacheCreationUsage{ | |
| Ephemeral5mInputTokens: cacheCreation5m, | |
| Ephemeral1hInputTokens: cacheCreation1h, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/convert.go` around lines 237 - 240, Restore the cache-creation split
normalization when populating usage.CacheCreation: when building
dto.ClaudeCacheCreationUsage from oaiUsage, prefer existing
oaiUsage.ClaudeCacheCreation5mTokens/ClaudeCacheCreation1hTokens if they are
present, but if they are missing or zero and oaiUsage.CacheCreationInputTokens
(aggregate) is set, derive the 5m/1h split using the same normalization logic
used elsewhere (i.e., compute Ephemeral5mInputTokens and Ephemeral1hInputTokens
from CacheCreationInputTokens), then assign those normalized values to
Ephemeral5mInputTokens and Ephemeral1hInputTokens on usage.CacheCreation so
cache_creation is not dropped or inconsistent.
Comment on lines
429
to
447
| if len(openAIResponse.Choices) == 0 { | ||
| // Some OpenAI-compatible upstreams end with a usage-only SSE chunk. | ||
| oaiUsage := openAIResponse.Usage | ||
| if oaiUsage == nil { | ||
| oaiUsage = info.ClaudeConvertInfo.Usage | ||
| } | ||
| if oaiUsage != nil { | ||
| // no choices | ||
| // 可能为非标准的 OpenAI 响应，判断是否已经完成 | ||
| if info.ClaudeConvertInfo.Done { | ||
| stopOpenBlocks() | ||
| stopReason := stopReasonOpenAI2Claude(info.FinishReason) | ||
| if stopReason == "" { | ||
| stopReason = "end_turn" | ||
| oaiUsage := info.ClaudeConvertInfo.Usage | ||
| if oaiUsage != nil { | ||
| claudeResponses = append(claudeResponses, &dto.ClaudeResponse{ | ||
| Type: "message_delta", | ||
| Usage: buildClaudeUsageFromOpenAIUsage(oaiUsage), | ||
| Delta: &dto.ClaudeMediaMessage{ | ||
| StopReason: common.GetPointer[string](stopReasonOpenAI2Claude(info.FinishReason)), | ||
| }, | ||
| }) | ||
| } | ||
| claudeResponses = append(claudeResponses, &dto.ClaudeResponse{ | ||
| Type: "message_delta", | ||
| Usage: buildClaudeUsageFromOpenAIUsage(oaiUsage), | ||
| Delta: &dto.ClaudeMediaMessage{ | ||
| StopReason: common.GetPointer[string](stopReason), | ||
| }, | ||
| }) | ||
| claudeResponses = append(claudeResponses, &dto.ClaudeResponse{ | ||
| Type: "message_stop", | ||
| }) | ||
| info.ClaudeConvertInfo.Done = true | ||
| } |
Contributor
There was a problem hiding this comment.
This terminal branch is unreachable in the current call flow.
StreamResponseOpenAI2Claude returns immediately when info.ClaudeConvertInfo.Done is already true, and the main caller only flips Done after this function returns. For usage-only final chunks (choices == 0), this condition never passes, so Claude clients can miss the final message_delta/message_stop.
🔧 Suggested fix
if len(openAIResponse.Choices) == 0 {
// no choices
// 可能为非标准的 OpenAI 响应，判断是否已经完成
- if info.ClaudeConvertInfo.Done {
+ if info.FinishReason != "" || info.ClaudeConvertInfo.Usage != nil {
stopOpenBlocks()
oaiUsage := info.ClaudeConvertInfo.Usage
if oaiUsage != nil {
claudeResponses = append(claudeResponses, &dto.ClaudeResponse{
Type: "message_delta",
Usage: buildClaudeUsageFromOpenAIUsage(oaiUsage),
Delta: &dto.ClaudeMediaMessage{
StopReason: common.GetPointer[string](stopReasonOpenAI2Claude(info.FinishReason)),
},
})
}
claudeResponses = append(claudeResponses, &dto.ClaudeResponse{
Type: "message_stop",
})
+ info.ClaudeConvertInfo.Done = true
}
return claudeResponses
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if len(openAIResponse.Choices) == 0 { | |
| // Some OpenAI-compatible upstreams end with a usage-only SSE chunk. | |
| oaiUsage := openAIResponse.Usage | |
| if oaiUsage == nil { | |
| oaiUsage = info.ClaudeConvertInfo.Usage | |
| } | |
| if oaiUsage != nil { | |
| // no choices | |
| // 可能为非标准的 OpenAI 响应，判断是否已经完成 | |
| if info.ClaudeConvertInfo.Done { | |
| stopOpenBlocks() | |
| stopReason := stopReasonOpenAI2Claude(info.FinishReason) | |
| if stopReason == "" { | |
| stopReason = "end_turn" | |
| oaiUsage := info.ClaudeConvertInfo.Usage | |
| if oaiUsage != nil { | |
| claudeResponses = append(claudeResponses, &dto.ClaudeResponse{ | |
| Type: "message_delta", | |
| Usage: buildClaudeUsageFromOpenAIUsage(oaiUsage), | |
| Delta: &dto.ClaudeMediaMessage{ | |
| StopReason: common.GetPointer[string](stopReasonOpenAI2Claude(info.FinishReason)), | |
| }, | |
| }) | |
| } | |
| claudeResponses = append(claudeResponses, &dto.ClaudeResponse{ | |
| Type: "message_delta", | |
| Usage: buildClaudeUsageFromOpenAIUsage(oaiUsage), | |
| Delta: &dto.ClaudeMediaMessage{ | |
| StopReason: common.GetPointer[string](stopReason), | |
| }, | |
| }) | |
| claudeResponses = append(claudeResponses, &dto.ClaudeResponse{ | |
| Type: "message_stop", | |
| }) | |
| info.ClaudeConvertInfo.Done = true | |
| } | |
| if len(openAIResponse.Choices) == 0 { | |
| // no choices | |
| // 可能为非标准的 OpenAI 响应，判断是否已经完成 | |
| if info.FinishReason != "" || info.ClaudeConvertInfo.Usage != nil { | |
| stopOpenBlocks() | |
| oaiUsage := info.ClaudeConvertInfo.Usage | |
| if oaiUsage != nil { | |
| claudeResponses = append(claudeResponses, &dto.ClaudeResponse{ | |
| Type: "message_delta", | |
| Usage: buildClaudeUsageFromOpenAIUsage(oaiUsage), | |
| Delta: &dto.ClaudeMediaMessage{ | |
| StopReason: common.GetPointer[string](stopReasonOpenAI2Claude(info.FinishReason)), | |
| }, | |
| }) | |
| } | |
| claudeResponses = append(claudeResponses, &dto.ClaudeResponse{ | |
| Type: "message_stop", | |
| }) | |
| info.ClaudeConvertInfo.Done = true | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/convert.go` around lines 429 - 447, The branch handling OpenAI
responses with len(openAIResponse.Choices) == 0 is currently gated by
info.ClaudeConvertInfo.Done and therefore never runs because
StreamResponseOpenAI2Claude returns before Done is set; fix it by removing or
changing the Done gate so final usage-only chunks are emitted: inside
StreamResponseOpenAI2Claude, when choices==0 detect the final chunk via
info.FinishReason (or the presence of info.ClaudeConvertInfo.Usage) and call
stopOpenBlocks() and append the message_delta (using
buildClaudeUsageFromOpenAIUsage(info.ClaudeConvertInfo.Usage)) and message_stop
to claudeResponses regardless of info.ClaudeConvertInfo.Done, or set
info.ClaudeConvertInfo.Done earlier before returning so the existing append code
executes; update the logic around stopOpenBlocks(), the construction of
dto.ClaudeResponse (Type/message_delta, Usage, Delta.StopReason) and the
subsequent message_stop append accordingly.
Comment on lines
+620
to
+623
| claudeResponse.Usage = &dto.ClaudeUsage{ | ||
| InputTokens: openAIResponse.PromptTokens, | ||
| OutputTokens: openAIResponse.CompletionTokens, | ||
| } |
Contributor
There was a problem hiding this comment.
Non-stream Claude responses now drop cache usage fields.
This replacement keeps only prompt/completion totals and loses CacheCreationInputTokens, CacheReadInputTokens, and the 5m/1h cache split that the streaming path still preserves. Cached Claude responses will now under-report usage compared with the rest of the relay.
🔧 Suggested fix
- claudeResponse.Usage = &dto.ClaudeUsage{
- InputTokens: openAIResponse.PromptTokens,
- OutputTokens: openAIResponse.CompletionTokens,
- }
+ claudeResponse.Usage = buildClaudeUsageFromOpenAIUsage(&openAIResponse.Usage)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/convert.go` around lines 620 - 623, The current assignment to
claudeResponse.Usage only sets InputTokens/OutputTokens and drops cache-related
fields; update the mapping to populate dto.ClaudeUsage's
CacheCreationInputTokens, CacheReadInputTokens and the cache split fields (the
5m/1h counters) from the source openAIResponse.Usage (e.g.
openAIResponse.Usage.CacheCreationInputTokens,
openAIResponse.Usage.CacheReadInputTokens,
openAIResponse.Usage.CacheReadInputTokens5m/1h or whatever the exact field names
are) so non-stream Claude responses report the same cache usage as the streaming
path — you can still take the address of openAIResponse.Usage (it’s embedded by
value) when assigning to claudeResponse.Usage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
这个改动主要是为了解决 Codex 渠道因为额度或状态波动被自动禁用后，即使后面恢复了，也只能靠人工再测一次才能重新启用的问题。
这次调整了两块：
stream开关，并把它落到渠道设置里，避免只停留在临时测试弹窗状态这样已有的自动启用逻辑就能真正接上，恢复后的 Codex 渠道可以靠定时复测自动回到可用状态，不用再等人手动点一遍。
验证：
go test ./controller ./setting/operation_settingbun x prettier --check src/components/settings/OperationSetting.jsx src/components/table/channels/modals/EditChannelModal.jsx src/components/table/channels/modals/ModelTestModal.jsx src/hooks/channels/useChannelsData.jsx src/pages/Setting/Operation/SettingsMonitoring.jsxbun run build补充说明：
Summary by CodeRabbit
New Features
Settings
Channel Affinity
Tests