Skip to content

feat: improve Claude Responses cache affinity support - #4981

Closed
Rebybyx wants to merge 3 commits into
QuantumNous:mainfrom
Rebybyx:dev/Rebybyx/20260518
Closed

feat: improve Claude Responses cache affinity support#4981
Rebybyx wants to merge 3 commits into
QuantumNous:mainfrom
Rebybyx:dev/Rebybyx/20260518

Conversation

@Rebybyx

@Rebybyx Rebybyx commented May 20, 2026

Copy link
Copy Markdown

⚠️ 提交说明 / PR Notice

Important

This description is manually organized around the code changes and runtime behavior. It is not a raw AI-generated dump.

📝 变更描述 / Description

This PR improves the Claude-compatible -> OpenAI Responses relay path and the cache-affinity behavior around it.

Main changes:

  • Add a direct Claude Messages -> OpenAI Responses conversion path, avoiding the intermediate Claude -> Chat -> Responses conversion when Responses compatibility is enabled.
  • Add prompt cache key derivation for Claude-compatible requests and /v1/responses requests.
  • Preserve server-side web_search semantics across Claude/Responses compatibility flows instead of degrading it into a normal function tool.
  • Add claude_prompt_cache_key as a channel-affinity key source, so Claude/Codex-style traces can stay on the same successful channel.
  • Add scheduled channel test exclusion settings, allowing selected channel IDs to be skipped by automatic scheduled tests while keeping manual tests unaffected.

Why it works:

  • Claude system/messages are converted directly into Responses input items, which keeps the upstream prompt shape more stable.
  • Cache keys are derived from explicit prompt_cache_key, stable metadata, headers, or Claude cache-control prefix information where available.
  • Channel affinity can reuse the derived cache key and request metadata, so related traces can prefer a previously successful channel.
  • Scheduled testing now reads an explicit excluded-channel list before automatic channel checks.

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue,避免将设计取舍、理解偏差或预期不一致直接归类为 bug
  • ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
  • ⚡ 性能优化 / 重构 (Refactor)
  • 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

#4150 is related to configurable Claude Code cache affinity. This PR is broader and also includes Claude -> Responses direct conversion, Responses prompt cache key derivation, server-side web_search preservation, and scheduled channel test exclusions.

✅ 提交前检查项 / Checklist

  • 人工确认: 我已亲自整理并撰写此描述,没有直接粘贴未经处理的 AI 输出。
  • 非重复提交: 我已搜索现有的 IssuesPRs,确认不是重复提交。
  • Bug fix 说明: 若此 PR 标记为 Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核结果。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

📸 运行证明 / Proof of Work

Usage examples

Model mapping

A Claude-compatible model can be mapped to the actual upstream Responses model. For example:

  • Original model: cc-gpt-5.5
  • Upstream model: gpt-5.5

ChatCompletions -> Responses compatibility

Channels can opt into the Responses compatibility path with a policy like:

{
  "enabled": true,
  "all_channels": false,
  "channel_ids": [1],
  "model_patterns": ["^cc-gpt-5[.]5$"]
}

Channel affinity

Example Claude CLI trace rule:

  • Model pattern: ^claude-.*$
  • Path pattern: /v1/messages
  • Key sources:
    • gjson: metadata.user_id
    • claude_prompt_cache_key

The UI now exposes claude_prompt_cache_key as a key source option.

OpenAI Responses prompt cache key

For /v1/responses, the relay can derive a prompt cache key from stable request metadata or headers when the request does not already provide prompt_cache_key.

Scheduled channel test exclusions

The monitoring settings page now supports excluding specific channel IDs from scheduled channel tests. Manual channel tests are not affected.

Local verification

Passed after removing the out-of-scope plugin usage endpoint:

go test ./controller ./relay ./relay/channel/codex ./relay/channel/openai ./relay/common ./service ./service/openaicompat ./setting/operation_setting

Previously passed before this scope cleanup:

cd web/default && bun run build
cd web/classic && bun run build

Known baseline note:

The broader relay test suite has existing failures on origin/main in relay/channel/claude and relay/helper; this PR did not introduce those failures.

Screenshots

Screenshots show:
image

  • Model mapping from cc-gpt-5.5 to gpt-5.5.
1715432e6b7b78e76395a4f2a1d7c017
  • ChatCompletions -> Responses compatibility policy JSON.
e4db708835b26cdbb65ccb21a33de4cf
  • Channel affinity rules using metadata.user_id and claude_prompt_cache_key.
ebd4074b1da6d06bc23a0ed186d1802e
  • The claude_prompt_cache_key option in the rule editor key-source dropdown.
ee7cd064fcf4d4a70f35a085e697b899

Summary by CodeRabbit

  • New Features

    • Exclude specific channels from automatic scheduled tests via configurable excluded channel IDs.
    • Improved web-search and multi-backend responses support, plus Anthropic(Claude) -> Responses conversion and prompt-cache key support.
  • UI & Settings

    • New textarea in monitoring settings for excluded channel IDs with validation and i18n strings.
    • New "Claude prompt cache key" option in channel affinity settings.
  • Bug Fixes

    • Better channel-test error handling and correct web-search usage counting.
  • API

    • Added read-only token usage endpoint.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 559199b5-91d8-4a00-a99e-ae3fb77299a0

📥 Commits

Reviewing files that changed from the base of the PR and between ee71b27 and 585e2f9.

📒 Files selected for processing (13)
  • controller/channel-test.go
  • relay/chat_completions_via_responses.go
  • relay/claude_handler.go
  • relay/common/relay_info.go
  • relay/responses_handler.go
  • router/api-router.go
  • web/classic/src/i18n/locales/en.json
  • web/classic/src/i18n/locales/zh.json
  • web/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsx
  • web/default/src/features/system-settings/integrations/monitoring-settings-section.tsx
  • web/default/src/features/system-settings/operations/index.tsx
  • web/default/src/features/system-settings/operations/section-registry.tsx
  • web/default/src/features/system-settings/types.ts
💤 Files with no reviewable changes (4)
  • web/default/src/features/system-settings/operations/index.tsx
  • web/default/src/features/system-settings/types.ts
  • web/default/src/features/system-settings/operations/section-registry.tsx
  • web/default/src/features/system-settings/integrations/monitoring-settings-section.tsx
✅ Files skipped from review due to trivial changes (2)
  • web/classic/src/i18n/locales/en.json
  • web/classic/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (6)
  • relay/common/relay_info.go
  • controller/channel-test.go
  • relay/responses_handler.go
  • router/api-router.go
  • web/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsx
  • relay/chat_completions_via_responses.go

Walkthrough

This PR implements Claude↔OpenAI Responses conversion and relay plumbing (tools, streaming, prompt-cache keys), normalizes web-search tool variants, adds channel-test auto-filtering with excluded-ID config, and wires frontend settings and i18n for excluded channel IDs and channel-affinity prompt-cache sources.

Changes

Claude-Responses Relay System

Layer / File(s) Summary
DTOs and built-in tool constants
dto/openai_request.go, dto/openai_response.go
ToolCallRequest extended with ID and SearchContextSize; ResponsesOutput gains Action *ResponsesOutputAction; BuildInToolWebSearch constant added.
Codex adaptor normalization
relay/channel/codex/adaptor.go
Normalize tools JSON by mapping preview web-search tool type to canonical web_search before conversion.
Responses→Chat streaming & aggregation
relay/channel/openai/chat_via_responses.go, relay/channel/openai/relay_responses.go
Add non-stream SSE aggregation, shared conversion to client format, assistant-text extraction helper, Claude web-search streaming handling, and updated finish-reason logic.
Chat/Claude upstream routing
relay/chat_completions_via_responses.go, relay/claude_handler.go
Detect upstream streaming from Content-Type; route to stream/non-stream handlers accordingly; add claudeMessagesViaResponses and forceClaudeResponsesUpstreamStream for Claude→Responses flows.
Claude→Responses conversion service
service/claude_responses.go
New ClaudeToResponsesRequest mapping system/messages/tools/controls into OpenAI Responses request, including tool_choice/reasoning mapping and optional prompt-cache-key injection.
Convert helpers & Responses→Claude mapping
service/convert.go
Prompt-cache-key wiring in Claude→OpenAI path, extract tool conversion helpers, and Responses→Claude conversion preserving web_search server_tool semantics and streaming SSE events.
Prompt-cache key derivation
service/openai_responses_prompt_cache.go, service/claude_prompt_cache.go
New modules to derive/stash prompt-cache keys for OpenAI Responses and Claude (user_id preference, normalized payload hashing, slugified sources).
Channel affinity extraction
service/channel_affinity.go, setting/operation_setting/channel_affinity_setting.go
Support openai_responses_prompt_cache_key and claude_prompt_cache_key sources and request-path detection; docs and rule templates updated to include claude_prompt_cache_key.
Responses handler prompt-cache application
relay/responses_handler.go
Apply automatic OpenAI Responses prompt-cache key derivation before converting outbound Responses requests.
Channel test auto-filtering
controller/channel-test.go, setting/operation_setting/monitor_setting.go
Add AutoTestChannelExcludedIDs setting with parser; shouldSkipAutomaticChannelTest and testAllChannels(useAutomaticTestFilter) skip nil/excluded/manually disabled channels; manual vs automatic call-site differences.
API usage token route
router/api-router.go
New /api/usage group and GET /token route with CORS and critical rate limiting, protected by read-only token auth.
Frontend: monitoring settings (excluded IDs)
web/classic/.../SettingsMonitoring.jsx, web/default/.../monitoring-settings-section.tsx
Add textarea UI, validation, normalization, Zod schema, defaults wiring, and submit normalization for monitor_setting.auto_test_channel_excluded_ids.
Frontend: channel affinity UI & types
web/classic/.../SettingsChannelAffinity.jsx, web/default/.../channel-affinity/*
Add claude_prompt_cache_key key-source type, disable parameter editing for it, update rule-editor helpers, templates, and TypeScript types.
Frontend: settings defaults and i18n
web/classic/..., web/default/..., web/*/i18n/locales/*.json
Add operations default for excluded IDs and i18n strings (EN/ZH) for labels, input hints, and format errors.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • seefs001
  • Calcium-Ion
  • creamlike1024

Poem

🐰 A rabbit bytes through Responses and code,

Claude and OpenAI walk the same road,
Cache keys hashed and web-search calls hop,
Channel tests skip the ones we should drop,
Small paws tidy settings before we go.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.51% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: improve Claude Responses cache affinity support' accurately reflects the main objective of adding Claude → Responses conversion, prompt cache key derivation, and cache affinity improvements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🧹 Nitpick comments (1)
web/default/src/i18n/locales/en.json (1)

1437-1437: ⚡ Quick win

Use hierarchical i18n keys for the newly added monitoring strings.

These new entries use literal sentence keys instead of semantic hierarchical keys. Please switch to scoped keys (for example under monitoring.autoTest.*) and update the corresponding t(...) lookups.

💡 Suggested key shape
- "Enter positive channel IDs separated by commas or line breaks": "Enter positive channel IDs separated by commas or line breaks",
- "Excluded channel IDs": "Excluded channel IDs",
- "Scheduled tests skip these channel IDs. Leave empty to test all eligible channels.": "Scheduled tests skip these channel IDs. Leave empty to test all eligible channels.",
+ "monitoring.autoTest.excludedChannelIds.placeholder": "Enter positive channel IDs separated by commas or line breaks",
+ "monitoring.autoTest.excludedChannelIds.label": "Excluded channel IDs",
+ "monitoring.autoTest.excludedChannelIds.description": "Scheduled tests skip these channel IDs. Leave empty to test all eligible channels.",

As per coding guidelines, use “hierarchical and semantically clear translation key names such as dashboard.overview.title and maintain naming consistency”.

Also applies to: 1532-1532, 3463-3463

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/default/src/i18n/locales/en.json` at line 1437, Summary: The new i18n
entry uses a full sentence as the key instead of a hierarchical semantic key;
replace it with a scoped key and update code to use the new key. Fix: in en.json
replace the literal key "Enter positive channel IDs separated by commas or line
breaks" with a hierarchical key such as
"monitoring.autoTest.enterPositiveChannelIds" (and do the same for the other
similar literal keys flagged), then update all t(...) lookups that reference the
literal sentence to use the new hierarchical key string, ensuring naming
consistency under the monitoring.autoTest namespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@controller/channel-test.go`:
- Around line 874-882: The helper shouldSkipAutomaticChannelTest currently
always skips channels with Status == common.ChannelStatusManuallyDisabled, which
prevents manual TestAllChannels when useAutomaticTestFilter is false; change
shouldSkipAutomaticChannelTest to accept a useAutomaticTestFilter bool and only
apply the channel.Status == common.ChannelStatusManuallyDisabled check when
useAutomaticTestFilter is true, update all callers (including the manual
TestAllChannels path that invokes shouldSkipAutomaticChannelTest) to pass the
appropriate flag, and ensure the nil and excludedChannelIDs checks remain
unchanged.

In `@controller/usage_stats.go`:
- Around line 131-138: The response object is using usedUSD for weekUsed and
totalUsed which is inconsistent with period-specific fields (todayUsed uses
todayStats.Quota); update weekUsed to quotaToUSD(weekStats.Quota) and
weekUsedFormatted to formatUSD(quotaToUSD(weekStats.Quota)), and update
totalUsed to quotaToUSD(totalStats.Quota) and totalUsedFormatted to
formatUSD(quotaToUSD(totalStats.Quota)); keep other fields (e.g., usedUSD,
analyticsTotalUsed) unchanged unless you intend to rename them to clarify they
represent cumulative account/subscription usage.

In `@dto/openai_request.go`:
- Around line 230-234: Change the optional scalar struct fields ID and
SearchContextSize from plain string to pointer types (e.g., *string) and keep
`omitempty` on their json tags so omitted vs explicit-empty values are preserved
during relay conversion; update the struct that contains the fields ID, Type,
Function, Custom, SearchContextSize (the OpenAI request DTO) to use pointer
types for those optional scalars and adjust any callers/constructors to handle
nil vs non-nil values accordingly.

In `@relay/channel/openai/chat_via_responses.go`:
- Around line 590-591: The call to helper.ClaudeData(c, *claudeResp) currently
ignores any returned error; change it to capture the error (err :=
helper.ClaudeData(...)) and handle it by logging the error and terminating
further parsing/streaming (e.g., return the error or break out of the event loop
and close the connection) so upstream event processing stops on downstream write
failures; update the surrounding function in chat_via_responses.go to propagate
or handle that error appropriately using the c and claudeResp symbols.
- Around line 229-243: readResponsesStreamFinal only accumulates text from
"response.output_text.delta", so if upstream sends text via
"response.output_item.delta" or only a terminal "response.output_item.done" you
end up with an empty synthesized body; update the switch inside
readResponsesStreamFinal to also handle "response.output_item.delta" (append
streamResp.Delta to outputText) and handle "response.output_item.done" (append
any final piece of text from streamResp.Content/Delta or streamResp.Response
choice text into outputText) before using outputText as the stream-final
fallback, using the existing variables streamResp, outputText and finalResp to
aggregate those pieces.

In `@router/api-router.go`:
- Around line 284-288: The new v1UsageRoute group lacks rate limiting which
exposes the public GET /v1/usage/stats (controller.GetUsageStats) to
brute-force/DoS; add the same middleware used by the existing /api/usage route
by calling v1UsageRoute.Use(middleware.CriticalRateLimit()) (or the appropriate
rate limit middleware) when constructing the group so the /v1/usage route is
protected.

In `@service/channel_affinity.go`:
- Around line 329-343: The current extraction returns the literal "null" when
res represents JSON null; change the logic in the switch that inspects
res.Type/res.Raw so that JSON null is considered missing: treat gjson.Null
(and/or res.Raw == "null") as empty and do not return it, allowing the fallback
that checks src.Path == "prompt_cache_key" and calls
BuildOpenAIResponsesPromptCacheKeyFromContext(c).Key to execute; update the
conditional around res.String()/res.Raw (the switch handling res.Type and the
subsequent if) to explicitly exclude gjson.Null or the "null" raw value before
returning.

In `@service/claude_prompt_cache.go`:
- Around line 39-44: extractClaudeMetadataUserID currently returns a raw user ID
and the code in the Claude prompt-cache branch returns that raw value as
ClaudePromptCacheKeyResult.Key; instead of forwarding the raw ID, compute a
cryptographic hash (e.g., SHA‑256 hex) of the userID and return that hashed
string as Key while leaving Source as "metadata.user_id" and OK true. Update the
branch that constructs ClaudePromptCacheKeyResult for
extractClaudeMetadataUserID to replace the plain userID with the hashed
representation so upstream/cache paths never see the raw identifier.

In `@service/claude_responses.go`:
- Around line 367-370: The switch is matching strings.TrimSpace(effort) but
returns the original effort, so inputs like " high " can pass the case yet still
emit an invalid token; change the code to first normalize the token (e.g.,
normalized := strings.ToLower(strings.TrimSpace(effort))) and switch on that
normalized value, and return the normalized string in the matching cases
(reference the effort variable and the switch block in claude_responses.go),
leaving the existing "max"/default handling intact.

In `@web/classic/src/i18n/locales/en.json`:
- Line 2820: Update the English translation for the key "定时测试排除渠道 ID 格式不正确" so
the message reads naturally in English; change "Invalid scheduled-test excluded
channel ID format" to use "scheduled test" (e.g., "Invalid scheduled test
excluded channel ID format") to remove the awkward hyphen and improve clarity in
the UI.

In `@web/classic/src/pages/Setting/Operation/SettingsChannelAffinity.jsx`:
- Around line 73-74: The new option 'claude_prompt_cache_key' was added to the
key source list but validateKeySources currently treats it as invalid,
preventing saves; update the validation logic in SettingsChannelAffinity.jsx so
validateKeySources recognizes 'claude_prompt_cache_key' as a valid source (and
any related allow-list or switch handling used by the save routine), ensuring
the same symbol name ('claude_prompt_cache_key') is accepted wherever key
sources are validated before saving.

In
`@web/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsx`:
- Around line 115-119: The placeholder string in getKeySourceInputPlaceholder is
hardcoded and must be localized; update getKeySourceInputPlaceholder to not
return raw user-facing strings but either accept a translation function (t:
TFunction) as a parameter or return stable translation keys (e.g.,
'placeholder.noParam', 'placeholder.metadataConversationId',
'placeholder.userId') and then call t(...) from the React component that uses
getKeySourceInputPlaceholder; update callers of getKeySourceInputPlaceholder
(where it's used in the rule-editor-dialog UI) to pass useTranslation().t or to
translate the returned keys before rendering so all user-facing text uses t().

In
`@web/default/src/features/system-settings/integrations/monitoring-settings-section.tsx`:
- Around line 49-57: The hardcoded Zod error message in channelIdListString (the
z.string().refine call) must be replaced with a localized message supplied via
i18n; refactor this by creating a schema factory or a small helper that accepts
the t function (from useTranslation) and returns the zod string schema with the
refine error message set to t('...') (or map Zod errors to localized strings),
then use that factory where channelIdListString is currently defined so all
validation messages call t(...) instead of the hardcoded English string.

---

Nitpick comments:
In `@web/default/src/i18n/locales/en.json`:
- Line 1437: Summary: The new i18n entry uses a full sentence as the key instead
of a hierarchical semantic key; replace it with a scoped key and update code to
use the new key. Fix: in en.json replace the literal key "Enter positive channel
IDs separated by commas or line breaks" with a hierarchical key such as
"monitoring.autoTest.enterPositiveChannelIds" (and do the same for the other
similar literal keys flagged), then update all t(...) lookups that reference the
literal sentence to use the new hierarchical key string, ensuring naming
consistency under the monitoring.autoTest namespace.
🪄 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: ec6a8453-acac-4917-b283-674457a4cb60

📥 Commits

Reviewing files that changed from the base of the PR and between 2d1ca15 and b23b23a.

📒 Files selected for processing (36)
  • controller/channel-test.go
  • controller/usage_stats.go
  • dto/openai_request.go
  • dto/openai_response.go
  • relay/channel/codex/adaptor.go
  • relay/channel/openai/chat_via_responses.go
  • relay/channel/openai/relay_responses.go
  • relay/chat_completions_via_responses.go
  • relay/claude_handler.go
  • relay/common/relay_info.go
  • relay/responses_handler.go
  • router/api-router.go
  • service/channel_affinity.go
  • service/claude_prompt_cache.go
  • service/claude_responses.go
  • service/convert.go
  • service/openai_responses_prompt_cache.go
  • service/openaicompat/chat_to_responses.go
  • service/text_quota.go
  • setting/operation_setting/channel_affinity_setting.go
  • setting/operation_setting/monitor_setting.go
  • web/classic/src/components/settings/OperationSetting.jsx
  • web/classic/src/i18n/locales/en.json
  • web/classic/src/i18n/locales/zh-CN.json
  • web/classic/src/i18n/locales/zh.json
  • web/classic/src/pages/Setting/Operation/SettingsChannelAffinity.jsx
  • web/classic/src/pages/Setting/Operation/SettingsMonitoring.jsx
  • web/default/src/features/system-settings/general/channel-affinity/constants.ts
  • web/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsx
  • web/default/src/features/system-settings/general/channel-affinity/types.ts
  • web/default/src/features/system-settings/integrations/monitoring-settings-section.tsx
  • web/default/src/features/system-settings/operations/index.tsx
  • web/default/src/features/system-settings/operations/section-registry.tsx
  • web/default/src/features/system-settings/types.ts
  • web/default/src/i18n/locales/en.json
  • web/default/src/i18n/locales/zh.json

Comment thread controller/channel-test.go
Comment thread controller/usage_stats.go Outdated
Comment on lines +131 to +138
"weekUsed": usedUSD,
"weekUsedFormatted": formatUSD(usedUSD),
"weekCalls": weekStats.Calls,
"weekTokens": weekStats.Tokens,
"totalUsed": usedUSD,
"totalUsedFormatted": formatUSD(usedUSD),
"analyticsTotalUsed": usedUSD,
"analyticsTotalUsedFormatted": formatUSD(usedUSD),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

weekUsed and totalUsed values are inconsistent with their period-specific counterparts.

todayUsed correctly uses todayStats.Quota (the actual quota consumed today), but weekUsed and totalUsed both use usedUSD (subscription/wallet cumulative used quota) rather than weekStats.Quota and totalStats.Quota respectively. Meanwhile, weekCalls/weekTokens and totalCalls/totalTokens do use the period-specific stats.

This semantic inconsistency may confuse API consumers expecting period-specific usage. Consider either:

  1. Using quotaToUSD(weekStats.Quota) for weekUsed and quotaToUSD(totalStats.Quota) for totalUsed, or
  2. Renaming these fields to clarify they represent cumulative subscription/account usage (e.g., accountUsed)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controller/usage_stats.go` around lines 131 - 138, The response object is
using usedUSD for weekUsed and totalUsed which is inconsistent with
period-specific fields (todayUsed uses todayStats.Quota); update weekUsed to
quotaToUSD(weekStats.Quota) and weekUsedFormatted to
formatUSD(quotaToUSD(weekStats.Quota)), and update totalUsed to
quotaToUSD(totalStats.Quota) and totalUsedFormatted to
formatUSD(quotaToUSD(totalStats.Quota)); keep other fields (e.g., usedUSD,
analyticsTotalUsed) unchanged unless you intend to rename them to clarify they
represent cumulative account/subscription usage.

Comment thread dto/openai_request.go
Comment on lines +230 to +234
ID string `json:"id,omitempty"`
Type string `json:"type"`
Function FunctionRequest `json:"function,omitempty"`
Custom json.RawMessage `json:"custom,omitempty"`
SearchContextSize string `json:"search_context_size,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use pointer types for new optional scalar tool fields.

Line 230 and Line 234 add optional scalar request fields as plain string, which drops omitted-vs-explicit-empty semantics during relay conversion.

Proposed fix
 type ToolCallRequest struct {
-	ID                string          `json:"id,omitempty"`
+	ID                *string         `json:"id,omitempty"`
 	Type              string          `json:"type"`
 	Function          FunctionRequest `json:"function,omitempty"`
 	Custom            json.RawMessage `json:"custom,omitempty"`
-	SearchContextSize string          `json:"search_context_size,omitempty"`
+	SearchContextSize *string         `json:"search_context_size,omitempty"`
 }
As per coding guidelines `**/{dto,relay/convert}/**/*.go`: “use pointer types with `omitempty` for optional scalar fields … to preserve explicit zero values.”
📝 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
ID string `json:"id,omitempty"`
Type string `json:"type"`
Function FunctionRequest `json:"function,omitempty"`
Custom json.RawMessage `json:"custom,omitempty"`
SearchContextSize string `json:"search_context_size,omitempty"`
ID *string `json:"id,omitempty"`
Type string `json:"type"`
Function FunctionRequest `json:"function,omitempty"`
Custom json.RawMessage `json:"custom,omitempty"`
SearchContextSize *string `json:"search_context_size,omitempty"`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dto/openai_request.go` around lines 230 - 234, Change the optional scalar
struct fields ID and SearchContextSize from plain string to pointer types (e.g.,
*string) and keep `omitempty` on their json tags so omitted vs explicit-empty
values are preserved during relay conversion; update the struct that contains
the fields ID, Type, Function, Custom, SearchContextSize (the OpenAI request
DTO) to use pointer types for those optional scalars and adjust any
callers/constructors to handle nil vs non-nil values accordingly.

Comment on lines +229 to +243
case "response.output_text.delta":
outputText.WriteString(streamResp.Delta)
case "response.completed":
if streamResp.Response != nil {
finalResp = streamResp.Response
}
case "response.error", "response.failed":
if streamResp.Response != nil {
if oaiErr := streamResp.Response.GetOpenAIError(); oaiErr != nil && oaiErr.Type != "" {
return nil, types.WithOpenAIError(*oaiErr, http.StatusInternalServerError)
}
}
return nil, types.NewOpenAIError(fmt.Errorf("responses stream error: %s", streamResp.Type), types.ErrorCodeBadResponse, http.StatusInternalServerError)
default:
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Aggregate response.output_item.done message text in stream-final fallback.

readResponsesStreamFinal only appends response.output_text.delta. If upstream emits message text only via response.output_item.done and response.completed is missing, the synthesized response body becomes empty.

💡 Suggested patch
 		switch streamResp.Type {
 		case "response.created":
 			if streamResp.Response != nil {
@@
 		case "response.output_text.delta":
 			outputText.WriteString(streamResp.Delta)
+		case "response.output_item.done":
+			if streamResp.Item != nil && streamResp.Item.Type == "message" {
+				text := responsesOutputItemText(streamResp.Item)
+				if delta := stringDeltaFromPrefix(outputText.String(), text); delta != "" {
+					outputText.WriteString(delta)
+				}
+			}
 		case "response.completed":
 			if streamResp.Response != nil {
 				finalResp = streamResp.Response
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@relay/channel/openai/chat_via_responses.go` around lines 229 - 243,
readResponsesStreamFinal only accumulates text from
"response.output_text.delta", so if upstream sends text via
"response.output_item.delta" or only a terminal "response.output_item.done" you
end up with an empty synthesized body; update the switch inside
readResponsesStreamFinal to also handle "response.output_item.delta" (append
streamResp.Delta to outputText) and handle "response.output_item.done" (append
any final piece of text from streamResp.Content/Delta or streamResp.Response
choice text into outputText) before using outputText as the stream-final
fallback, using the existing variables streamResp, outputText and finalResp to
aggregate those pieces.

Comment on lines +590 to +591
_ = helper.ClaudeData(c, *claudeResp)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle helper.ClaudeData write errors instead of ignoring them.

Ignoring this error can keep parsing upstream events after downstream write failures, causing inconsistent stream termination behavior.

💡 Suggested patch
-					_ = helper.ClaudeData(c, *claudeResp)
+					if err := helper.ClaudeData(c, *claudeResp); err != nil {
+						streamErr = types.NewOpenAIError(err, types.ErrorCodeBadResponse, http.StatusInternalServerError)
+						sr.Stop(streamErr)
+						return
+					}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_ = helper.ClaudeData(c, *claudeResp)
}
if err := helper.ClaudeData(c, *claudeResp); err != nil {
streamErr = types.NewOpenAIError(err, types.ErrorCodeBadResponse, http.StatusInternalServerError)
sr.Stop(streamErr)
return
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@relay/channel/openai/chat_via_responses.go` around lines 590 - 591, The call
to helper.ClaudeData(c, *claudeResp) currently ignores any returned error;
change it to capture the error (err := helper.ClaudeData(...)) and handle it by
logging the error and terminating further parsing/streaming (e.g., return the
error or break out of the event loop and close the connection) so upstream event
processing stops on downstream write failures; update the surrounding function
in chat_via_responses.go to propagate or handle that error appropriately using
the c and claudeResp symbols.

Comment on lines +367 to +370
switch strings.TrimSpace(effort) {
case "low", "medium", "high", "xhigh":
return effort
case "max":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Return normalized effort token from the switch.

Line 367 matches on strings.TrimSpace(effort) but Line 369 returns the original effort, so values like " high " can pass the switch and still emit an invalid token.

Proposed fix
 func mapClaudeResponsesReasoningEffort(effort string) string {
-	switch strings.TrimSpace(effort) {
+	normalized := strings.TrimSpace(effort)
+	switch normalized {
 	case "low", "medium", "high", "xhigh":
-		return effort
+		return normalized
 	case "max":
 		return "xhigh"
 	default:
 		return ""
 	}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
switch strings.TrimSpace(effort) {
case "low", "medium", "high", "xhigh":
return effort
case "max":
func mapClaudeResponsesReasoningEffort(effort string) string {
normalized := strings.TrimSpace(effort)
switch normalized {
case "low", "medium", "high", "xhigh":
return normalized
case "max":
return "xhigh"
default:
return ""
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@service/claude_responses.go` around lines 367 - 370, The switch is matching
strings.TrimSpace(effort) but returns the original effort, so inputs like " high
" can pass the case yet still emit an invalid token; change the code to first
normalize the token (e.g., normalized :=
strings.ToLower(strings.TrimSpace(effort))) and switch on that normalized value,
and return the normalized string in the matching cases (reference the effort
variable and the switch block in claude_responses.go), leaving the existing
"max"/default handling intact.

"自动禁用关键词": "Automatic disable keywords",
"自动禁用状态码": "Auto-disable status codes",
"自动禁用状态码格式不正确": "Invalid auto-disable status code format",
"定时测试排除渠道 ID 格式不正确": "Invalid scheduled-test excluded channel ID format",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use natural wording in validation message

“scheduled-test” reads awkwardly in English UI. Prefer “scheduled test” for clarity.

✏️ Proposed text fix
-    "定时测试排除渠道 ID 格式不正确": "Invalid scheduled-test excluded channel ID format",
+    "定时测试排除渠道 ID 格式不正确": "Invalid excluded channel ID format for scheduled tests",
📝 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
"定时测试排除渠道 ID 格式不正确": "Invalid scheduled-test excluded channel ID format",
"定时测试排除渠道 ID 格式不正确": "Invalid excluded channel ID format for scheduled tests",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/classic/src/i18n/locales/en.json` at line 2820, Update the English
translation for the key "定时测试排除渠道 ID 格式不正确" so the message reads naturally in
English; change "Invalid scheduled-test excluded channel ID format" to use
"scheduled test" (e.g., "Invalid scheduled test excluded channel ID format") to
remove the awkward hyphen and improve clarity in the UI.

Comment on lines +73 to 74
{ label: 'claude_prompt_cache_key', value: 'claude_prompt_cache_key' },
{ label: 'gjson', value: 'gjson' },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

New key source type is currently unsavable in classic UI.

Line 73 adds claude_prompt_cache_key, but validateKeySources later classifies it as invalid, so users can select it but cannot save rules.

Suggested fix
 const normalizeKeySource = (src) => {
   const type = (src?.type || '').trim();
   const key = (src?.key || '').trim();
   const path = (src?.path || '').trim();
 
+  if (type === 'claude_prompt_cache_key') {
+    return { type, key: '', path: '' };
+  }
+
   if (type === 'gjson') {
     return { type, key: '', path };
   }
 
   return { type, key, path: '' };
 };
@@
   for (const x of xs) {
-    if (
+    if (x.type === 'claude_prompt_cache_key') {
+      continue;
+    } else if (
       x.type === 'context_int' ||
       x.type === 'context_string' ||
       x.type === 'request_header'
     ) {
       if (!x.key) return { ok: false, message: 'Key 不能为空' };
@@
                   const src = normalizeKeySource(
                     editingRule?.key_sources?.[idx],
                   );
                   const isGjson = src.type === 'gjson';
+                  const noParam = src.type === 'claude_prompt_cache_key';
 
                   return (
                     <Input
-                      placeholder={
-                        isGjson ? 'metadata.conversation_id' : 'X-Affinity-Key'
-                      }
+                      placeholder={noParam ? '无需参数' : (isGjson ? 'metadata.conversation_id' : 'X-Affinity-Key')}
                       aria-label={t('Key 或 Path')}
-                      value={isGjson ? src.path : src.key}
+                      value={noParam ? '' : (isGjson ? src.path : src.key)}
+                      disabled={noParam}
                       onChange={(value) =>
+                        noParam ? undefined :
                         updateKeySource(
                           idx,
                           isGjson ? { path: value } : { key: value },
                         )
                       }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/classic/src/pages/Setting/Operation/SettingsChannelAffinity.jsx` around
lines 73 - 74, The new option 'claude_prompt_cache_key' was added to the key
source list but validateKeySources currently treats it as invalid, preventing
saves; update the validation logic in SettingsChannelAffinity.jsx so
validateKeySources recognizes 'claude_prompt_cache_key' as a valid source (and
any related allow-list or switch handling used by the save routine), ensuring
the same symbol name ('claude_prompt_cache_key') is accepted wherever key
sources are validated before saving.

Comment on lines +115 to +119
function getKeySourceInputPlaceholder(src: KeySource): string {
if (src.type === 'claude_prompt_cache_key') return 'No parameter required'
if (src.type === 'gjson') return 'metadata.conversation_id'
return 'user_id'
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Translate the new placeholder text.

Line 116 hardcodes a user-facing string (No parameter required) and bypasses i18n.

Suggested fix
-function getKeySourceInputPlaceholder(src: KeySource): string {
-  if (src.type === 'claude_prompt_cache_key') return 'No parameter required'
+function getKeySourceInputPlaceholder(
+  src: KeySource,
+  translate: (key: string) => string
+): string {
+  if (src.type === 'claude_prompt_cache_key')
+    return translate('No parameter required')
   if (src.type === 'gjson') return 'metadata.conversation_id'
   return 'user_id'
 }
@@
-                    placeholder={getKeySourceInputPlaceholder(src)}
+                    placeholder={getKeySourceInputPlaceholder(src, t)}

As per coding guidelines: web/default/**/*.{tsx,ts} requires all user-facing text content to support i18n using t() from useTranslation() in React components.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@web/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsx`
around lines 115 - 119, The placeholder string in getKeySourceInputPlaceholder
is hardcoded and must be localized; update getKeySourceInputPlaceholder to not
return raw user-facing strings but either accept a translation function (t:
TFunction) as a parameter or return stable translation keys (e.g.,
'placeholder.noParam', 'placeholder.metadataConversationId',
'placeholder.userId') and then call t(...) from the React component that uses
getKeySourceInputPlaceholder; update callers of getKeySourceInputPlaceholder
(where it's used in the rule-editor-dialog UI) to pass useTranslation().t or to
translate the returned keys before rendering so all user-facing text uses t().

Comment on lines +49 to +57
const channelIdListString = z.string().refine((value) => {
const trimmed = value.trim()
if (!trimmed) return true
return trimmed
.split(/[,\s;]+/)
.filter(Boolean)
.every((item) => /^[1-9]\d*$/.test(item))
}, 'Enter positive channel IDs separated by commas or line breaks')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Localize the new excluded-channel-ID validation message.

The new Zod error string is hardcoded in English, so non-English locales won’t get translated validation feedback. Please source this message from i18n (t(...)) via a schema factory or localized error mapping.

As per coding guidelines web/default/**/*.{tsx,ts}: “All user-facing text content must support i18n using the t() function from useTranslation() in React components”.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@web/default/src/features/system-settings/integrations/monitoring-settings-section.tsx`
around lines 49 - 57, The hardcoded Zod error message in channelIdListString
(the z.string().refine call) must be replaced with a localized message supplied
via i18n; refactor this by creating a schema factory or a small helper that
accepts the t function (from useTranslation) and returns the zod string schema
with the refine error message set to t('...') (or map Zod errors to localized
strings), then use that factory where channelIdListString is currently defined
so all validation messages call t(...) instead of the hardcoded English string.

@Rebybyx

Rebybyx commented May 22, 2026

Copy link
Copy Markdown
Author

@seefs001 你好,这个 PR 现在 CodeRabbit 检查已通过,最新 review 也没有新的 actionable comments,目前只卡在 required approving review。方便的时候可以帮忙看一下吗?我是第一次向这个项目提交 PR,如果有什么需要做的请告知我,谢谢!

CodeRabbit checks are passing, and the PR is currently blocked only by the required approving review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant