feat: implement DeepSeek V4 reasoning suffix handling and tests - #4428
Conversation
WalkthroughThe PR refactors reasoning and thinking suffix parsing across OpenAI and DeepSeek channels. It introduces shared parsing functions in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
relay/channel/deepseek/adaptor.go (2)
96-99: Clarify theinfo.ChannelMeta != nilgate — or drop it.The fallback to
info.UpstreamModelNameis conditioned oninfo.ChannelMeta != nil. It’s not obvious whyChannelMetagates trustingUpstreamModelName; typicallyUpstreamModelNameis authoritative regardless. If the intent is just “guard against a stub/zeroinfoin tests,” theinfo.UpstreamModelName != ""check already covers that. Please either add a short comment explaining the intent or simplify the guard.Also applies to: 122-126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/deepseek/adaptor.go` around lines 96 - 99, The current guard uses info.ChannelMeta != nil before trusting info.UpstreamModelName, which is confusing; either remove the ChannelMeta nil-check so modelName is set when info != nil && info.UpstreamModelName != "" (making UpstreamModelName authoritative), or keep the check but add a brief comment above the block explaining why ChannelMeta must be present to trust UpstreamModelName (e.g., tests create stub infos lacking ChannelMeta). Update both occurrences where modelName is set before calling reasoning.ParseDeepSeekV4ThinkingSuffix to follow the same approach and ensure consistent behavior.
94-150: Optional: extract the common model-name/info update boilerplate.
applyDeepSeekV4OpenAIThinkingSuffixandapplyDeepSeekV4ClaudeThinkingSuffixshare the samemodelNameresolution andinfopropagation. A tiny helper keeps the two request-type branches focused on their payload-specific mutation.♻️ Suggested shape
func resolveDeepSeekV4Suffix(info *relaycommon.RelayInfo, modelOnRequest string) (base, thinkingType, effort string, ok bool) { name := modelOnRequest if info != nil && info.ChannelMeta != nil && info.UpstreamModelName != "" { name = info.UpstreamModelName } return reasoning.ParseDeepSeekV4ThinkingSuffix(name) } func propagateDeepSeekV4Meta(info *relaycommon.RelayInfo, baseModel, effort string) { if info == nil { return } if info.ChannelMeta != nil { info.UpstreamModelName = baseModel } info.ReasoningEffort = effort }Then each
apply*only performs the request-type-specific marshalling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/deepseek/adaptor.go` around lines 94 - 150, Both applyDeepSeekV4OpenAIThinkingSuffix and applyDeepSeekV4ClaudeThinkingSuffix duplicate the same model-name resolution and info propagation logic; extract that boilerplate into helpers like resolveDeepSeekV4Suffix(info *relaycommon.RelayInfo, modelOnRequest string) (base, thinkingType, effort string, ok bool) and propagateDeepSeekV4Meta(info *relaycommon.RelayInfo, baseModel, effort string) to centralize: have each apply* call resolveDeepSeekV4Suffix instead of repeating the name/select logic and call propagateDeepSeekV4Meta to update info.UpstreamModelName and info.ReasoningEffort, leaving each apply* to only handle request-specific marshaling and field assignment.setting/reasoning/suffix.go (1)
38-51: Nit: unreachabledefaultbranch.
TrimEffortSuffixWithSuffixesis called withDeepSeekV4EffortSuffixes = ["-minimal", "-max"], so whenok == truethe returnedsuffixcan only be"minimal"or"max". Thedefault:arm is dead code. It’s harmless, but you can simplify:♻️ Proposed simplification
- switch suffix { - case "minimal": - return baseModel, "disabled", "", true - case "max": - return baseModel, "enabled", "max", true - default: - return modelName, "", "", false - } + if suffix == "minimal" { + return baseModel, "disabled", "", true + } + return baseModel, "enabled", "max", true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/reasoning/suffix.go` around lines 38 - 51, The switch in ParseDeepSeekV4ThinkingSuffix contains an unreachable default branch because TrimEffortSuffixWithSuffixes is called with DeepSeekV4EffortSuffixes (only "minimal" or "max"), so simplify the control flow: in ParseDeepSeekV4ThinkingSuffix (after calling TrimEffortSuffixWithSuffixes and the prefix check) replace the switch with explicit handling for "minimal" and "max" (e.g., if/else or a switch without a default) and return the same values for those two cases, removing the dead default branch; keep references to TrimEffortSuffixWithSuffixes and DeepSeekV4EffortSuffixes 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 `@relay/channel/deepseek/adaptor.go`:
- Around line 96-99: The current guard uses info.ChannelMeta != nil before
trusting info.UpstreamModelName, which is confusing; either remove the
ChannelMeta nil-check so modelName is set when info != nil &&
info.UpstreamModelName != "" (making UpstreamModelName authoritative), or keep
the check but add a brief comment above the block explaining why ChannelMeta
must be present to trust UpstreamModelName (e.g., tests create stub infos
lacking ChannelMeta). Update both occurrences where modelName is set before
calling reasoning.ParseDeepSeekV4ThinkingSuffix to follow the same approach and
ensure consistent behavior.
- Around line 94-150: Both applyDeepSeekV4OpenAIThinkingSuffix and
applyDeepSeekV4ClaudeThinkingSuffix duplicate the same model-name resolution and
info propagation logic; extract that boilerplate into helpers like
resolveDeepSeekV4Suffix(info *relaycommon.RelayInfo, modelOnRequest string)
(base, thinkingType, effort string, ok bool) and propagateDeepSeekV4Meta(info
*relaycommon.RelayInfo, baseModel, effort string) to centralize: have each
apply* call resolveDeepSeekV4Suffix instead of repeating the name/select logic
and call propagateDeepSeekV4Meta to update info.UpstreamModelName and
info.ReasoningEffort, leaving each apply* to only handle request-specific
marshaling and field assignment.
In `@setting/reasoning/suffix.go`:
- Around line 38-51: The switch in ParseDeepSeekV4ThinkingSuffix contains an
unreachable default branch because TrimEffortSuffixWithSuffixes is called with
DeepSeekV4EffortSuffixes (only "minimal" or "max"), so simplify the control
flow: in ParseDeepSeekV4ThinkingSuffix (after calling
TrimEffortSuffixWithSuffixes and the prefix check) replace the switch with
explicit handling for "minimal" and "max" (e.g., if/else or a switch without a
default) and return the same values for those two cases, removing the dead
default branch; keep references to TrimEffortSuffixWithSuffixes and
DeepSeekV4EffortSuffixes intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7d2913a-babc-4511-8cc4-1f342ac18b64
📒 Files selected for processing (4)
relay/channel/deepseek/adaptor.gorelay/channel/deepseek/constants.gorelay/channel/openai/adaptor.gosetting/reasoning/suffix.go
There was a problem hiding this comment.
🧹 Nitpick comments (5)
setting/reasoning/suffix_test.go (1)
55-63: Test name doesn't match what's asserted.
TestTrimEffortSuffixKeepsGeneralSuffixesonly asserts the happy path where-maxis successfully trimmed — it never exercises a case where a "general" (non-effort) trailing token is kept intact. Consider renaming to reflect what's actually verified (e.g.TestTrimEffortSuffixTrimsMaxSuffix), or extend with a negative case like"claude-opus-4-7"whereokshould befalse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/reasoning/suffix_test.go` around lines 55 - 63, The test name TestTrimEffortSuffixKeepsGeneralSuffixes is misleading because it only verifies that TrimEffortSuffix trims a "-max" effort suffix; either rename the test to reflect the behavior (e.g., TestTrimEffortSuffixTrimsMaxSuffix) or add a second assertion case that passes a non-effort trailing token (e.g., "claude-opus-4-7") and asserts ok == false and model equals the original string; update the test around TrimEffortSuffix accordingly so the name matches the assertions.relay/channel/deepseek/adaptor_test.go (2)
181-195: URL assertion looks good, but won't catch base-URL variants.The test hard-codes
https://api.deepseek.comasChannelBaseUrl. IfGetRequestURLever normalizes trailing slashes or concatenates paths differently (e.g. a base URL ending with/), this test wouldn't exercise it. Consider adding one more case with a trailing slash to pin down the joining behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/deepseek/adaptor_test.go` around lines 181 - 195, Add a second sub-case to TestGetRequestURLUsesClaudeUpstreamForClaudeRelayFormat that exercises ChannelBaseUrl with a trailing slash (e.g. "https://api.deepseek.com/") and asserts GetRequestURL on (&Adaptor{}).GetRequestURL(info) still returns the same normalized Claude messages URL ("https://api.deepseek.com/anthropic/v1/messages"); ensure you reuse the existing RelayInfo setup and only change ChannelBaseUrl to cover URL-joining behavior and trailing-slash normalization.
63-96: Consider also assertinginfo.ReasoningEffortin the ignored-suffix path.In
TestConvertOpenAIRequestDeepSeekV4ThinkingSuffixIgnoredthe test verifies thatconvertedRequest.ReasoningEffortis preserved as"client-effort", but doesn't checkinfo.ReasoningEffort. SinceapplyDeepSeekV4OpenAIThinkingSuffixalso mutatesinfo.ReasoningEfforton the success path, adding a guard here would lock in the "no-op on unsupported suffix" contract against future regressions.Proposed addition
if convertedRequest.ReasoningEffort != "client-effort" { t.Fatalf("ReasoningEffort = %q, want client-effort", convertedRequest.ReasoningEffort) } + if info.ReasoningEffort != "" { + t.Fatalf("info.ReasoningEffort = %q, want empty", info.ReasoningEffort) + } assertRawThinkingType(t, convertedRequest.THINKING, "client")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/deepseek/adaptor_test.go` around lines 63 - 96, The test TestConvertOpenAIRequestDeepSeekV4ThinkingSuffixIgnored currently asserts convertedRequest.ReasoningEffort but not the upstream info.ReasoningEffort; add an assertion that info.ReasoningEffort remains unchanged (e.g., "client-effort") after calling (&Adaptor{}).ConvertOpenAIRequest so the ignored-suffix path of applyDeepSeekV4OpenAIThinkingSuffix is validated; locate the test loop in TestConvertOpenAIRequestDeepSeekV4ThinkingSuffixIgnored and add a check for info.ReasoningEffort alongside the existing assertions for convertedRequest.ReasoningEffort and convertedRequest.Model.setting/reasoning/suffix.go (2)
30-36: Return order is inverted relative to sibling helper.
ParseOpenAIReasoningEffortFromModelSuffixreturns(effort, baseModel)whileParseDeepSeekV4ThinkingSuffixreturns(baseModel, thinkingType, effort, ok)— different positional conventions for the same concept. Not a bug (callers use named assignments), but easy to misuse. Consider aligning to(baseModel, effort, ok)for consistency, or at least document the ordering in the function doc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/reasoning/suffix.go` around lines 30 - 36, Change ParseOpenAIReasoningEffortFromModelSuffix to return values in the same positional order as the sibling helper: (baseModel, effort, ok) instead of (effort, baseModel). Update the function signature and the return statement to propagate the ok value returned by TrimEffortSuffixWithSuffixes and return baseModel first, then effort, then ok; reference TrimEffortSuffixWithSuffixes and OpenAIEffortSuffixes to locate the call site to adjust.
38-51: Unreachabledefaultbranch in switch.
TrimEffortSuffixWithSuffixesonly returnsok=truewhensuffixis one ofDeepSeekV4EffortSuffixes("none"/"max"after the leading-is stripped), so thedefaultarm on line 48-50 can never be hit. It's harmless as defensive code, but worth noting if someone ever extendsDeepSeekV4EffortSuffixes— the new suffix would silently fall intodefaultand be reported as unsupported instead of failing loudly. A short comment clarifying the invariant (or apanic/test to guard it) would make the intent explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/reasoning/suffix.go` around lines 38 - 51, The switch in ParseDeepSeekV4ThinkingSuffix has an unreachable default because TrimEffortSuffixWithSuffixes only returns ok=true when suffix is one of DeepSeekV4EffortSuffixes; update ParseDeepSeekV4ThinkingSuffix to either (preferred) add a short comment above the switch documenting the invariant that TrimEffortSuffixWithSuffixes guarantees suffix ∈ DeepSeekV4EffortSuffixes and thus the default is defensive/unreachable, or (if you want fail-fast) replace the default branch with a panic or logging fatal that references the unexpected suffix to surface future changes; reference ParseDeepSeekV4ThinkingSuffix, TrimEffortSuffixWithSuffixes, and DeepSeekV4EffortSuffixes when applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@relay/channel/deepseek/adaptor_test.go`:
- Around line 181-195: Add a second sub-case to
TestGetRequestURLUsesClaudeUpstreamForClaudeRelayFormat that exercises
ChannelBaseUrl with a trailing slash (e.g. "https://api.deepseek.com/") and
asserts GetRequestURL on (&Adaptor{}).GetRequestURL(info) still returns the same
normalized Claude messages URL
("https://api.deepseek.com/anthropic/v1/messages"); ensure you reuse the
existing RelayInfo setup and only change ChannelBaseUrl to cover URL-joining
behavior and trailing-slash normalization.
- Around line 63-96: The test
TestConvertOpenAIRequestDeepSeekV4ThinkingSuffixIgnored currently asserts
convertedRequest.ReasoningEffort but not the upstream info.ReasoningEffort; add
an assertion that info.ReasoningEffort remains unchanged (e.g., "client-effort")
after calling (&Adaptor{}).ConvertOpenAIRequest so the ignored-suffix path of
applyDeepSeekV4OpenAIThinkingSuffix is validated; locate the test loop in
TestConvertOpenAIRequestDeepSeekV4ThinkingSuffixIgnored and add a check for
info.ReasoningEffort alongside the existing assertions for
convertedRequest.ReasoningEffort and convertedRequest.Model.
In `@setting/reasoning/suffix_test.go`:
- Around line 55-63: The test name TestTrimEffortSuffixKeepsGeneralSuffixes is
misleading because it only verifies that TrimEffortSuffix trims a "-max" effort
suffix; either rename the test to reflect the behavior (e.g.,
TestTrimEffortSuffixTrimsMaxSuffix) or add a second assertion case that passes a
non-effort trailing token (e.g., "claude-opus-4-7") and asserts ok == false and
model equals the original string; update the test around TrimEffortSuffix
accordingly so the name matches the assertions.
In `@setting/reasoning/suffix.go`:
- Around line 30-36: Change ParseOpenAIReasoningEffortFromModelSuffix to return
values in the same positional order as the sibling helper: (baseModel, effort,
ok) instead of (effort, baseModel). Update the function signature and the return
statement to propagate the ok value returned by TrimEffortSuffixWithSuffixes and
return baseModel first, then effort, then ok; reference
TrimEffortSuffixWithSuffixes and OpenAIEffortSuffixes to locate the call site to
adjust.
- Around line 38-51: The switch in ParseDeepSeekV4ThinkingSuffix has an
unreachable default because TrimEffortSuffixWithSuffixes only returns ok=true
when suffix is one of DeepSeekV4EffortSuffixes; update
ParseDeepSeekV4ThinkingSuffix to either (preferred) add a short comment above
the switch documenting the invariant that TrimEffortSuffixWithSuffixes
guarantees suffix ∈ DeepSeekV4EffortSuffixes and thus the default is
defensive/unreachable, or (if you want fail-fast) replace the default branch
with a panic or logging fatal that references the unexpected suffix to surface
future changes; reference ParseDeepSeekV4ThinkingSuffix,
TrimEffortSuffixWithSuffixes, and DeepSeekV4EffortSuffixes when applying the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2170eb1d-cfdd-4c35-8851-8fc3f12a251c
📒 Files selected for processing (4)
relay/channel/deepseek/adaptor_test.gorelay/channel/deepseek/constants.gosetting/reasoning/suffix.gosetting/reasoning/suffix_test.go
✅ Files skipped from review due to trivial changes (1)
- relay/channel/deepseek/constants.go
|
签个名呗大佬 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
setting/reasoning/suffix.go (1)
38-51: Unreachabledefaultbranch in switch.
suffixis guaranteed to be either"none"or"max"becauseTrimEffortSuffixWithSuffixesonly returns a non-empty suffix when it matches one ofDeepSeekV4EffortSuffixes({"-none", "-max"}). Thedefaultcase at Lines 48–49 is dead code. Not a bug, but you could drop it or convert the switch to a plainif/elsefor clarity.♻️ Proposed simplification
- switch suffix { - case "none": - return baseModel, "disabled", "", true - case "max": - return baseModel, "enabled", "max", true - default: - return modelName, "", "", false - } + if suffix == "none" { + return baseModel, "disabled", "", true + } + return baseModel, "enabled", "max", true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/reasoning/suffix.go` around lines 38 - 51, The switch in ParseDeepSeekV4ThinkingSuffix has an unreachable default because TrimEffortSuffixWithSuffixes only yields suffixes from DeepSeekV4EffortSuffixes ("-none","-max"); update ParseDeepSeekV4ThinkingSuffix to remove the dead default branch and simplify the logic by replacing the switch with an if/else that checks suffix == "none" and else assumes "max", keeping the existing early-return guard that uses TrimEffortSuffixWithSuffixes and the "deepseek-v4-" prefix check.setting/reasoning/suffix_test.go (1)
5-123: LGTM — good table-driven coverage for both parsers.Positive and negative cases are well represented, including the tricky "embedded but not suffix" case (
gpt-5.1-codex-max) and unsupported-suffix rejections for DeepSeek v4. One optional addition: a test fordeepseek-v4-none/deepseek-v4-max(no model family segment between the prefix and the effort suffix), which exercises theHasPrefix(baseModel, "deepseek-v4-")guard — the trimmed basedeepseek-v4lacks the trailing dash and should be rejected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setting/reasoning/suffix_test.go` around lines 5 - 123, Add tests for the edge cases "deepseek-v4-none" and "deepseek-v4-max" to exercise the HasPrefix(baseModel, \"deepseek-v4-\") guard in ParseDeepSeekV4ThinkingSuffix; ensure each new test case asserts that the function returns the original model unchanged (wantBaseModel equals input) with empty thinkingType/effort and ok==false so the trimmed base lacking the trailing dash is rejected.relay/channel/deepseek/adaptor_test.go (1)
13-195: LGTM — thorough adaptor-level coverage.Good pairing of positive/ignored cases across both OpenAI and Claude conversion paths, plus a targeted check that Claude relay format routes to
/anthropic/v1/messages. The "ignored" tests correctly pre-populate client-suppliedTHINKING/Thinking/OutputConfigvalues and verify the adaptor leaves them untouched when the suffix doesn't match, which is the most important invariant.Optional: consider adding one negative case for
GetRequestURLthat exercises a non-ClaudeRelayFormat(e.g., default →/v1/chat/completionsor/beta/completions) so the routing switch is covered in both directions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/deepseek/adaptor_test.go` around lines 13 - 195, Add a companion negative test to cover the non-Claude routing branch of GetRequestURL: create a test (e.g., TestGetRequestURLUsesDefaultUpstreamForNonClaudeRelayFormat) that constructs a relaycommon.RelayInfo with RelayFormat not equal to types.RelayFormatClaude (and set RelayMode to relayconstant.RelayModeChatCompletions or RelayModeCompletions), provide ChannelMeta.ChannelBaseUrl and RequestURLPath, call (&Adaptor{}).GetRequestURL(info), assert no error, and assert the returned URL points to the non-Claude upstream path (e.g., base + /v1/chat/completions or the expected default completions path); this will mirror TestGetRequestURLUsesClaudeUpstreamForClaudeRelayFormat but verify the other switch branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@relay/channel/deepseek/adaptor_test.go`:
- Around line 13-195: Add a companion negative test to cover the non-Claude
routing branch of GetRequestURL: create a test (e.g.,
TestGetRequestURLUsesDefaultUpstreamForNonClaudeRelayFormat) that constructs a
relaycommon.RelayInfo with RelayFormat not equal to types.RelayFormatClaude (and
set RelayMode to relayconstant.RelayModeChatCompletions or
RelayModeCompletions), provide ChannelMeta.ChannelBaseUrl and RequestURLPath,
call (&Adaptor{}).GetRequestURL(info), assert no error, and assert the returned
URL points to the non-Claude upstream path (e.g., base + /v1/chat/completions or
the expected default completions path); this will mirror
TestGetRequestURLUsesClaudeUpstreamForClaudeRelayFormat but verify the other
switch branch.
In `@setting/reasoning/suffix_test.go`:
- Around line 5-123: Add tests for the edge cases "deepseek-v4-none" and
"deepseek-v4-max" to exercise the HasPrefix(baseModel, \"deepseek-v4-\") guard
in ParseDeepSeekV4ThinkingSuffix; ensure each new test case asserts that the
function returns the original model unchanged (wantBaseModel equals input) with
empty thinkingType/effort and ok==false so the trimmed base lacking the trailing
dash is rejected.
In `@setting/reasoning/suffix.go`:
- Around line 38-51: The switch in ParseDeepSeekV4ThinkingSuffix has an
unreachable default because TrimEffortSuffixWithSuffixes only yields suffixes
from DeepSeekV4EffortSuffixes ("-none","-max"); update
ParseDeepSeekV4ThinkingSuffix to remove the dead default branch and simplify the
logic by replacing the switch with an if/else that checks suffix == "none" and
else assumes "max", keeping the existing early-return guard that uses
TrimEffortSuffixWithSuffixes and the "deepseek-v4-" prefix check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aca89e1d-8a1f-4e44-aabd-3f12b75dafec
📒 Files selected for processing (6)
relay/channel/deepseek/adaptor.gorelay/channel/deepseek/adaptor_test.gorelay/channel/deepseek/constants.gorelay/channel/openai/adaptor.gosetting/reasoning/suffix.gosetting/reasoning/suffix_test.go
✅ Files skipped from review due to trivial changes (1)
- relay/channel/deepseek/constants.go
🚧 Files skipped from review as they are similar to previous changes (2)
- relay/channel/openai/adaptor.go
- relay/channel/deepseek/adaptor.go
好了，用错电脑commit 了 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/deepseek/adaptor.go`:
- Around line 94-150: The helpers applyDeepSeekV4OpenAIThinkingSuffix and
applyDeepSeekV4ClaudeThinkingSuffix currently unconditionally overwrite
reasoning fields; change them to merge instead: in
applyDeepSeekV4OpenAIThinkingSuffix only set request.THINKING.Type when
request.THINKING is nil or its Type is empty (preserve existing budget_tokens
and other keys) and only set request.ReasoningEffort when it is empty; in
applyDeepSeekV4ClaudeThinkingSuffix only set request.Thinking.Type when
request.Thinking is nil or its Type is empty (preserve existing BudgetTokens),
and instead of setting request.OutputConfig = nil or replacing it entirely,
update or clear only the "effort" entry (preserve other OutputConfig fields) —
keep updating info.UpstreamModelName and info.ReasoningEffort as before. Ensure
marshaling logic respects existing non-empty fields and avoids discarding
caller-supplied BudgetTokens or other custom fields.
- Around line 36-39: The current branch silently returns convertedRequest when
the type assertion to *dto.ClaudeRequest fails, which skips the DeepSeek V4
"-none"/"-max" suffix handling; update ConvertClaudeRequest usage so the suffix
is handled deterministically: either perform the "-none"/"-max" model-name
normalization on the original req.Model before calling
claude.Adaptor.ConvertClaudeRequest, or if you must inspect the adaptor result,
replace the silent fallthrough in the claudeRequest type-assertion block with an
explicit error return (or fallback that extracts the model field) so that
dto.ClaudeRequest, convertedRequest, or the original request always goes through
the suffix-stripping logic (referencing ConvertClaudeRequest, convertedRequest,
claudeRequest, and dto.ClaudeRequest).
🪄 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: c680aa6d-bb7d-4497-9809-7ee94321ec86
📒 Files selected for processing (4)
relay/channel/deepseek/adaptor.gorelay/channel/deepseek/constants.gorelay/channel/openai/adaptor.gosetting/reasoning/suffix.go
✅ Files skipped from review due to trivial changes (1)
- relay/channel/deepseek/constants.go
| claudeRequest, ok := convertedRequest.(*dto.ClaudeRequest) | ||
| if !ok { | ||
| return convertedRequest, nil | ||
| } |
There was a problem hiding this comment.
Silent fallthrough may leave the -none/-max suffix unhandled.
If claude.Adaptor.ConvertClaudeRequest ever returns a non-*dto.ClaudeRequest payload (e.g. a future refactor returns a raw []byte/map), this branch returns the converted value unchanged and the DeepSeek V4 suffix handling is silently skipped. The -none/-max-suffixed model name would then be forwarded upstream, producing an obscure "invalid model" error on /anthropic/v1/messages rather than a clear local error.
Consider either asserting the type or applying the suffix handling on the raw req.Model before calling the claude adaptor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/deepseek/adaptor.go` around lines 36 - 39, The current branch
silently returns convertedRequest when the type assertion to *dto.ClaudeRequest
fails, which skips the DeepSeek V4 "-none"/"-max" suffix handling; update
ConvertClaudeRequest usage so the suffix is handled deterministically: either
perform the "-none"/"-max" model-name normalization on the original req.Model
before calling claude.Adaptor.ConvertClaudeRequest, or if you must inspect the
adaptor result, replace the silent fallthrough in the claudeRequest
type-assertion block with an explicit error return (or fallback that extracts
the model field) so that dto.ClaudeRequest, convertedRequest, or the original
request always goes through the suffix-stripping logic (referencing
ConvertClaudeRequest, convertedRequest, claudeRequest, and dto.ClaudeRequest).
| func applyDeepSeekV4OpenAIThinkingSuffix(info *relaycommon.RelayInfo, request *dto.GeneralOpenAIRequest) error { | ||
| modelName := request.Model | ||
| if info != nil && info.ChannelMeta != nil && info.UpstreamModelName != "" { | ||
| modelName = info.UpstreamModelName | ||
| } | ||
| baseModel, thinkingType, effort, ok := reasoning.ParseDeepSeekV4ThinkingSuffix(modelName) | ||
| if !ok { | ||
| return nil | ||
| } | ||
| thinking, err := common.Marshal(map[string]string{ | ||
| "type": thinkingType, | ||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("error marshalling thinking: %w", err) | ||
| } | ||
| request.Model = baseModel | ||
| request.THINKING = thinking | ||
| request.ReasoningEffort = effort | ||
| if info != nil { | ||
| if info.ChannelMeta != nil { | ||
| info.UpstreamModelName = baseModel | ||
| } | ||
| info.ReasoningEffort = effort | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func applyDeepSeekV4ClaudeThinkingSuffix(info *relaycommon.RelayInfo, request *dto.ClaudeRequest) error { | ||
| modelName := request.Model | ||
| if info != nil && info.ChannelMeta != nil && info.UpstreamModelName != "" { | ||
| modelName = info.UpstreamModelName | ||
| } | ||
| baseModel, thinkingType, effort, ok := reasoning.ParseDeepSeekV4ThinkingSuffix(modelName) | ||
| if !ok { | ||
| return nil | ||
| } | ||
| request.Model = baseModel | ||
| request.Thinking = &dto.Thinking{Type: thinkingType} | ||
| if effort == "" { | ||
| request.OutputConfig = nil | ||
| } else { | ||
| outputConfig, err := common.Marshal(map[string]string{ | ||
| "effort": effort, | ||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("error marshalling output_config: %w", err) | ||
| } | ||
| request.OutputConfig = outputConfig | ||
| } | ||
| if info != nil { | ||
| if info.ChannelMeta != nil { | ||
| info.UpstreamModelName = baseModel | ||
| } | ||
| info.ReasoningEffort = effort | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Consider preserving caller-supplied Thinking / THINKING / OutputConfig fields.
Both helpers unconditionally clobber reasoning-related request fields whenever the suffix matches:
applyDeepSeekV4OpenAIThinkingSuffix(L103-111) replacesrequest.THINKINGwith{"type": thinkingType}and overwritesrequest.ReasoningEffort, even if the caller already setTHINKINGwithbudget_tokensor a customReasoningEffort.applyDeepSeekV4ClaudeThinkingSuffix(L131-142) replacesrequest.Thinkingwith&dto.Thinking{Type: ...}(dropsBudgetTokensetc.), and for the-nonecase setsrequest.OutputConfig = nil, silently discarding anything the caller put there; for-maxit overwritesOutputConfigwith just{"effort":"max"}.
If the intended semantic is "the suffix is the sole signal and overrides client input", this is fine but worth documenting. Otherwise, prefer merging (only set fields that are currently zero/nil, e.g. preserve existing BudgetTokens, only clear OutputConfig.effort rather than the whole struct).
♻️ Example: preserve existing Thinking fields
- request.Model = baseModel
- request.Thinking = &dto.Thinking{Type: thinkingType}
+ request.Model = baseModel
+ if request.Thinking == nil {
+ request.Thinking = &dto.Thinking{}
+ }
+ request.Thinking.Type = thinkingType📝 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.
| func applyDeepSeekV4OpenAIThinkingSuffix(info *relaycommon.RelayInfo, request *dto.GeneralOpenAIRequest) error { | |
| modelName := request.Model | |
| if info != nil && info.ChannelMeta != nil && info.UpstreamModelName != "" { | |
| modelName = info.UpstreamModelName | |
| } | |
| baseModel, thinkingType, effort, ok := reasoning.ParseDeepSeekV4ThinkingSuffix(modelName) | |
| if !ok { | |
| return nil | |
| } | |
| thinking, err := common.Marshal(map[string]string{ | |
| "type": thinkingType, | |
| }) | |
| if err != nil { | |
| return fmt.Errorf("error marshalling thinking: %w", err) | |
| } | |
| request.Model = baseModel | |
| request.THINKING = thinking | |
| request.ReasoningEffort = effort | |
| if info != nil { | |
| if info.ChannelMeta != nil { | |
| info.UpstreamModelName = baseModel | |
| } | |
| info.ReasoningEffort = effort | |
| } | |
| return nil | |
| } | |
| func applyDeepSeekV4ClaudeThinkingSuffix(info *relaycommon.RelayInfo, request *dto.ClaudeRequest) error { | |
| modelName := request.Model | |
| if info != nil && info.ChannelMeta != nil && info.UpstreamModelName != "" { | |
| modelName = info.UpstreamModelName | |
| } | |
| baseModel, thinkingType, effort, ok := reasoning.ParseDeepSeekV4ThinkingSuffix(modelName) | |
| if !ok { | |
| return nil | |
| } | |
| request.Model = baseModel | |
| request.Thinking = &dto.Thinking{Type: thinkingType} | |
| if effort == "" { | |
| request.OutputConfig = nil | |
| } else { | |
| outputConfig, err := common.Marshal(map[string]string{ | |
| "effort": effort, | |
| }) | |
| if err != nil { | |
| return fmt.Errorf("error marshalling output_config: %w", err) | |
| } | |
| request.OutputConfig = outputConfig | |
| } | |
| if info != nil { | |
| if info.ChannelMeta != nil { | |
| info.UpstreamModelName = baseModel | |
| } | |
| info.ReasoningEffort = effort | |
| } | |
| return nil | |
| } | |
| func applyDeepSeekV4ClaudeThinkingSuffix(info *relaycommon.RelayInfo, request *dto.ClaudeRequest) error { | |
| modelName := request.Model | |
| if info != nil && info.ChannelMeta != nil && info.UpstreamModelName != "" { | |
| modelName = info.UpstreamModelName | |
| } | |
| baseModel, thinkingType, effort, ok := reasoning.ParseDeepSeekV4ThinkingSuffix(modelName) | |
| if !ok { | |
| return nil | |
| } | |
| request.Model = baseModel | |
| if request.Thinking == nil { | |
| request.Thinking = &dto.Thinking{} | |
| } | |
| request.Thinking.Type = thinkingType | |
| if effort == "" { | |
| request.OutputConfig = nil | |
| } else { | |
| outputConfig, err := common.Marshal(map[string]string{ | |
| "effort": effort, | |
| }) | |
| if err != nil { | |
| return fmt.Errorf("error marshalling output_config: %w", err) | |
| } | |
| request.OutputConfig = outputConfig | |
| } | |
| if info != nil { | |
| if info.ChannelMeta != nil { | |
| info.UpstreamModelName = baseModel | |
| } | |
| info.ReasoningEffort = effort | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/deepseek/adaptor.go` around lines 94 - 150, The helpers
applyDeepSeekV4OpenAIThinkingSuffix and applyDeepSeekV4ClaudeThinkingSuffix
currently unconditionally overwrite reasoning fields; change them to merge
instead: in applyDeepSeekV4OpenAIThinkingSuffix only set request.THINKING.Type
when request.THINKING is nil or its Type is empty (preserve existing
budget_tokens and other keys) and only set request.ReasoningEffort when it is
empty; in applyDeepSeekV4ClaudeThinkingSuffix only set request.Thinking.Type
when request.Thinking is nil or its Type is empty (preserve existing
BudgetTokens), and instead of setting request.OutputConfig = nil or replacing it
entirely, update or clear only the "effort" entry (preserve other OutputConfig
fields) — keep updating info.UpstreamModelName and info.ReasoningEffort as
before. Ensure marshaling logic respects existing non-empty fields and avoids
discarding caller-supplied BudgetTokens or other custom fields.
|
@HynoR 佬，我现在的版本是
是我的姿势不对吗？ |
只做了openai接口，a社转换没适配🌚,晚点pr |
能麻烦截个图看你是怎么接deepseek的吗，这边复现一下找找最佳方案 |
|
okok提个issue，我这几天修 |
感谢佬的付出，已提 issue #4562 |
3 participants
Important
📝 变更描述 / Description
适配deepseek v4 思考后缀，支持 -max (max reasoning) 和 -none （关闭思考） 两种后缀
适配deepseek的 claude 渠道，和 kimi 一样走 /anthropic/v1/message路径
参考的 openai 方法
对写代码的用户和翻译来说，这样按模型名称来区分思考深度更方便。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
截图测试里用的 minimal ，代码实际版本是 none
v4-flash-none (no reasoning)
v4-flash (high reasoning)
v4-flash (max reasoning)
Summary by CodeRabbit
New Features
deepseek-v4-flashanddeepseek-v4-prowith configurable reasoning effort levels (none, max, and default).Bug Fixes
Refactor