Repository navigation
fix(sse): probe one tier down when a reasoning_effort 4xx names no enum - #14895
Merged
diegosouzapw merged 6 commits intoSep 29, 2026
Merged
diegosouzapw merged 6 commits into
diegosouzapw merged 6 commits into
Conversation
The reactive reasoning_effort recovery in base.ts only fires when the 4xx body
advertises the accepted set ("expected one of `low`, `medium`, `high`"), which
parseReasoningEffortEnum reads. Plenty of gateways refuse an out-of-range value
without naming any: opencode.ai/zen/go answers mimo-v2.6-flash and mimo-v2.5-pro
with an opaque `Streaming response failed: [400] Invalid request parameters`
wrapped in the SSE error frame, and recordLearnedReasoningEffort is explicitly
built not to learn from text it cannot map. So the request 400s on every attempt,
forever, for a model whose registry entry simply has not caught up.
When no enum is available, step down exactly one tier and retry. If that probe is
ANSWERED, the ceiling is now proven rather than advertised, so record the whole
accepted prefix and let every later request clamp up front. A probe that is
refused changes nothing: an upstream refusing every tier still surfaces its own
error, and no capability is pinned on a guess.
The ladder stops at `low` deliberately. `minimal` and `none` are not merely
"less thinking" — they are the carriers that switch thinking off, so landing a
request there would silently change its intent instead of repairing it.
The probe rewrites only the effort, across all three carriers the sanitizer
already knows (top-level reasoning_effort, reasoning.effort, output_config.effort),
and never invents a carrier the request did not use.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical correctness issues and retry/accounting concerns remain unresolved.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Adds opaque reasoning_effort 4xx recovery by probing a lower tier and caching successful capability results.
Changes:
- Adds carrier-aware probing and learned-capability helpers.
- Integrates probe retries into
BaseExecutor. - Adds focused tests and changelog documentation.
- Review findings remain around overly broad retries, fingerprint serialization, request accounting, and sparse-tier learning.
| File | Description |
|---|---|
tests/unit/reasoning-effort-opaque-400-probe.test.ts |
Tests probing, learning, failure handling, and carrier preservation. |
open-sse/services/learnedReasoningEffortCaps.ts |
Implements probe selection and capability learning. |
open-sse/executors/base/reasoningEffort.ts |
Adds effort carrier read/write helpers. |
open-sse/executors/base.ts |
Implements opaque-error probing and retry logic. |
changelog.d/fixes/14895-reasoning-effort-opaque-400-probe.md |
Documents the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1613
to
+1617
| } else { | ||
| // No enum in the 4xx body — the upstream told us nothing we can | ||
| // record. Step down one tier and retry; if that retry is accepted we | ||
| // have PROOF of the ceiling (unlike the enum path, which trusts the | ||
| // upstream's own list) and can learn it for every later request. |
Comment on lines
+240
to
+244
| return recordLearnedReasoningEffort( | ||
| provider, | ||
| model, | ||
| REASONING_EFFORT_ORDER.slice(start, idx + 1) | ||
| ); |
…he probed tier
Two review points, both correct.
An opaque 4xx names no accepted set, so nothing in the body says the *effort*
was the cause — it is indistinguishable from an unrelated validation failure
(context too long, a malformed tool schema, a bad image). Running the probe for
every such 400 re-POSTs the real failure with a different body, and if the
upstream happens to answer 2xx, that masks the original error and records a
reasoning cap nothing was ever proven against, permanently lowering quality for
that provider/model in this process. The probe is now opt-in per provider via
OMNIROUTE_REASONING_EFFORT_PROBE_PROVIDERS (comma-separated ids, or *), default
off. Upstreams that name their accepted set keep the unflagged enum path.
A successful probe proves one value and says nothing about the tiers below it: a
model can answer `high` and still refuse `low`, and the learned-cap tests already
cover sparse accepted sets such as {high,max}. Recording the whole
low..probed slice let later low/medium requests bypass clamping and go out
unproven. Only the probed tier is recorded now, which is the conservative
direction — the clamp maps anything higher onto a value the upstream has
acknowledged and leaves anything lower alone.
# Conflicts: # .env.example # open-sse/executors/base.ts
…ningEffortRecovery diegosouzapw#14629 moved the clamp-and-retry chain out of BaseExecutor.execute() so executors that never call super.execute() (commandCode, cliproxyapi, glm) reach it. The probe now lives in that shared helper instead of the removed inline block, so every caller gets it. Still opt-in per provider via OMNIROUTE_REASONING_EFFORT_PROBE_PROVIDERS (off by default), now documented in ENVIRONMENT.md. Adds helper-level tests for a direct caller. Co-authored-by: Moseyuh333 <Moseyuh333@users.noreply.github.com>
diegosouzapw
merged commit Sep 29, 2026
e9993e5
into
diegosouzapw:release/v3.8.51
6 of 7 checks passed
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reasoning_effort4xx that names no accepted set, instead of 400ing on every attempt.The reactive
reasoning_effortrecovery inbase.tsonly fires when the 4xx body advertises the accepted values (expected one of \low`, `medium`, `high`), whichparseReasoningEffortEnumreads. Many gateways refuse an out-of-range value without naming any. Measured live againsthttps://opencode.ai/zen/go/v1/chat/completions` on 2026-09-23,mimo-v2.6-flashandmimo-v2.5-proanswerxhigh/maxwith an opaque:recordLearnedReasoningEffortis explicitly built not to learn from text it cannot map, andsupportsXHighEffort()is opt-out, so a model whose registry entry has not caught up is stuck: every request pays a 400 it can never learn its way out of.When no enum is available, this PR steps down exactly one tier and retries. If that probe is answered, the ceiling is now proven rather than advertised, so the accepted prefix is recorded and every later request clamps up front. A probe that is refused changes nothing — an upstream refusing every tier still surfaces its own error, and no capability is pinned on a guess.
Related Issues
supportedThinkingEffortsfor a specific model family)Validation
npm run lint— not run: eslint in this checkout'snode_modulescrashes on import (pre-existing, unrelated to this diff). CI runs it.release/v3.8.51@7a921299c); focused checks rerun afterwardIsolation proof — the new test file dropped onto the unpatched tip (three source files reverted, test file kept) fails; restored, it passes:
7a921299cThe first test in the file asserts
parseReasoningEffortEnumstill returnsnullfor the opaque body, so the suite cannot silently pass against the enum path once a future upstream starts naming its set.Test runs (
DISABLE_SQLITE_AUTO_BACKUP=true node --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts --import ./tests/_setup/isolateDataDir.ts --test …):reasoning-effort-opaque-400-probe.test.ts(new)reasoning-effort-clamp-and-retry,learned-reasoning-effort-caps,reasoning-effort-learned-capability,reasoning-effort-clamp-direction-consistency,base-executor-sanitize-effort,base-executor-sanitize-effort-max-mapping,base-reasoning-effort-split,effort-tiers-loop-catalog-e2eadaptive-effort,adaptive-effort-wiring,adaptive-effort-model-default-13448,provider-field-stripstsc --noEmit -p open-sse/tsconfig.jsonreports 20 errors, all inservices/compression/omniglyph*andvendor/codex-chatgpt-web(missingtiktoken/omniglyphtype declarations in this checkout's sharednode_modules). None are in a file this PR touches, and both packages are already declared inpackage.json.Writing the tests caught a real design bug before it left the branch:
nextProbeReasoningEffortoriginally steppedminimal → none, whichrecordLearnedProbeReasoningEffortcannot record, so a model answering onminimalwould have learned nothing. The ladder now stops atlow— see below.Tests Added Or Updated
tests/unit/reasoning-effort-opaque-400-probe.test.ts(new, 12 tests) — unit coverage fornextProbeReasoningEffort/recordLearnedProbeReasoningEffort, plus end-to-end coverage of the retry path throughBaseExecutor.executewith a mockedfetch: probe-then-learn, proactive clamp on the next request, failed probe learning nothing, no probe at the floor or without an effort, and thereasoning.effortcarrier.Coverage Notes
Touches
open-sse/executors/base.ts,open-sse/executors/base/reasoningEffort.ts,open-sse/services/learnedReasoningEffortCaps.ts. The new suite drives the changed branch inbase.tsthrough the realexecute()path (same shape as the existingreasoning-effort-clamp-and-retry.test.ts) rather than only unit-testing the helpers, so theif/elsewiring and the "probe failed → surface upstream" path are both executed.Reviewer Notes
fetchbefore it succeeds. After that the learned set applies up front. Providers that already advertise their enum never reach this path.learnedCapsMap — it resets on restart, so the first request after a restart may pay the probe again. This deliberately matchesrecordLearnedReasoningEffortrather than introducing a second store.lowon purpose.minimalandnoneare not merely "less thinking" — they are the carriers that switch thinking off. Landing a request there would silently change its intent rather than repair it, so those tiers are not probed and a request already atlowis left alone.readEffortCarriers/writeEffortValuevia two new thin exports), so it does not invent a carrier the request did not use and does not reshape anything else. Thethinking_budgetandcontext_management400-retry chains are untouched.Maintainer rework (merge-batch 2026-09-28 (release drain))
release/v3.8.51tip (real merge). fix(providers): the 400-recovery chain in BaseExecutor.execute() is unreachable from 59 executors that override execute() #14629/fix(providers): reach the reactive reasoning_effort 400 clamp-and-retry chain from CommandCodeExecutor (#14629) #14774 had extracted the inline clamp-and-retry block this PR edited intoapplyReasoningEffortRecovery()(open-sse/executors/base/reasoningEffortRecovery.ts), whichcommandCode.ts,cliproxyapi.tsandglm.tscall directly. Re-applying the probe inline inBaseExecutor.execute()would have left those callers without it, so the probe now runs inside the shared helper (probeOneTierDown, same{response, body, retried, attempted}contract, sameserializeBodyso Claude-protocol signing still applies).base.tskeeps the tip's version.OMNIROUTE_REASONING_EFFORT_PROBE_PROVIDERSadded todocs/reference/ENVIRONMENT.md(check:env-doc-syncwas red without it). Still off by default.typecheck:coreclean;check:open-sse-typecheck0 errors.check:file-sizereds (chat.ts,default.ts,responseSanitizer.ts,proxyFetch.ts,response-sanitizer.test.ts) are inherited from the tip — none touched here.