fix(models): surface DeepSeek V4 proxy max effort and wire mapping - #3892
fix(models): surface DeepSeek V4 proxy max effort and wire mapping#3892innocarpe wants to merge 3 commits into
Conversation
2b5e0c4 to
d8e1777
Compare
|
Rebased onto latest Head: |
d8e1777 to
9ca8273
Compare
|
Rebased onto latest |
yazzang-homelab
left a comment
There was a problem hiding this comment.
Independent architect review. Closes the actual gap in #3858 rather than the symptom, and the diagnostic half is genuinely wired through instead of being a dead export.
Verified end to end:
- The real bug was the matcher, not the ladder.
model.name.includes("deepseek-v4")missed namespaced custom proxies whose id iscline-pass/deepseek-v4-flashbut whosenamecarries the operator's own label.isDeepSeekV4FamilyModelnow checksgetCanonicalModelId(model.id)orname, which is what makes:maxstop being clamped on exactly the configuration that reported it. Effort.MaxandDEFAULT_REASONING_EFFORTS_WITH_XHIGH_AND_MAXalready existed and are already used for Anthropic Opus (model-thinking.ts:695-697). This reuses an established ladder rather than inventing a DeepSeek-specific one, somaxneeds no new plumbing in selectors or session state.- The diagnostic reaches a user.
resolveWireReasoningEffort→resolveThinkingEffortResolution(thinking.ts:158) →formatSelectorWithEffortDiagnostics(thinking.ts:173) →model-selector.ts:1449. I checked because a "surface the wire value" change that stops at an exported helper is the common way this kind of issue gets half-fixed. - The doc comment is honest about its own limits: "request-side effective value only. Providers that accept the request without echoing an effort do not confirm backend-normalized effort." That is the correct claim — the alternative would have been a diagnostic that quietly implies the backend agreed.
hasMapEntryvsremappedare distinct fields, so identity passthrough (max→maxvia an explicit map entry) is distinguishable from no entry at all. That distinction is the whole point for an operator debugging a proxy, and it would have been easy to collapse into one boolean.- CHANGELOG entries are under
## [Unreleased]in bothpackages/ai(line 8) andpackages/coding-agent(line 13). Worth stating explicitly — several currently-open PRs have entries misfiled into already-released sections.
One non-blocking risk: substring matching on model ids.
canonicalId.includes("deepseek-v4") || name.includes("deepseek-v4")
This matches anything containing the substring — my-deepseek-v4-lookalike, or a proxy label a user happens to type. A false positive silently grants a model the max level and the DeepSeek effort ladder. The blast radius is small (a wider effort ladder, not a wrong wire value), and the previous code was strictly looser since it matched on name alone, so this is not a regression. But if the id space ever gets adversarial, anchoring on a segment boundary (/ or - delimited) would be tighter than includes.
Second, smaller: isDeepSeekV4FamilyModel is generic over TApi but resolveWireReasoningEffort takes Model<"openai-completions">, and thinking.ts:158 reaches it through a cast (model as Model<"openai-completions">). The call site is guarded by an api check upstream, so it holds today, but the cast is where it would break silently if a non-completions DeepSeek route is ever added. A narrowing type guard instead of a cast would make that a compile error rather than a runtime surprise.
gajae.pr-review-verdict.v1 merge-approved sha256:f18ae6ff3f8159d97d6e8d36d3968b80a32bd41e reviewer:architect evidence:read of model-thinking.ts:27-48,690-700, openai-completions-compat.ts:314+, thinking.ts:140-175, model-selector.ts:1449 at this head
Custom openai-compatible DeepSeek V4 proxies clamped profile :max to GJC xhigh and hid the request-side wire value, so operators could not tell whether the gateway ran high, max, or a default. Lore-id: 3858wire Constraint: preserve namespaced wire model ids on API requests Constraint: wire diagnostics are request-side only, not backend-confirmed Constraint: formatClampedModelSelector stays pure for persistence Rejected: HUD always shows provider wire string | confuses GJC level UX and aliases Rejected: fail closed on unmapped custom efforts | breaks valid generic OpenAI proxies Confidence: high Scope-risk: narrow Reversibility: easy Tested: Yeachan-Heo#3858 effort clamp/wire diagnostics + namespaced leaf inheritance Not-tested: live ClinePass gateway effort echo (provider does not return it)
DeepSeek V4 family thinking max is now first-class max (Yeachan-Heo#3858), so the Ollama cloud alias deepseek-v4-pro:cloud inherits max rather than xhigh. Update the canonical-equivalence assertion to match. Lore-id: 3858ci01 Constraint: no production behavior change -- test expectation only Confidence: high Scope-risk: narrow Reversibility: easy Tested: model-registry Ollama cloud aliases + Yeachan-Heo#3856/Yeachan-Heo#3858 filters
…base Rebase onto post-Yeachan-Heo#3883 dev left comment/assert conflict markers in model-registry and its namespaced DeepSeek V4 test. Keep the Yeachan-Heo#3858 thinking/max + reasoningEffortMap assertions and the expanded leaf-meta comment. Lore-id: 3858reba Constraint: no production behavior change beyond completing the rebase resolution Confidence: high Scope-risk: narrow Reversibility: easy Tested: conflict-marker scan on PR files Not-tested: live ClinePass gateway
f18ae6f to
3cd8341
Compare
|
Rebased onto latest |
|
Closing during the emergency maintenance freeze. This PR is not in the retained critical or maintainer-owned set. Do not open a replacement PR unless a maintainer explicitly directs it. — |
Summary
cline-pass/deepseek-v4-flash) inherit bundled leaf metadata (thinking +reasoningEffortMap) while keeping the wire id.maxas a first-class thinking level so profile:maxis not silently rewritten toxhigh.resolveWireReasoningEffortand model-preset preview diagnostics that show request-sidewire=/ clamp notes without claiming backend confirmation.Fixes #3858
Test plan
bun test packages/ai/test/issue-3858-deepseek-effort-clamp.test.tsbun test packages/coding-agent/test/issue-3858-effort-diagnostics.test.tsbun test packages/coding-agent/test/model-registry.test.ts -t "3856|3858|namespaced custom"bun --cwd=packages/ai run checkbun --cwd=packages/coding-agent run checkmodel-thinking,deepseek-reasoning-content,openai-completions-compat(84 pass)Risks
xhighandmaxin GJC levels; both map to providermax. HUD can show either while wire ismax.