Skip to content

fix(chat): preserve suffix reasoning intent across model attempts - #13720

Merged
diegosouzapw merged 6 commits into
diegosouzapw:release/v3.8.51from
HouMinXi:fix/suffix-effort-propagation
Sep 17, 2026
Merged

diegosouzapw merged 6 commits into
diegosouzapw:release/v3.8.51from
HouMinXi:fix/suffix-effort-propagation

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Resolved model suffixes such as -max and -none could be dropped between model resolution and chat dispatch. Passing the value through alone is insufficient: after a handler selects a replacement model, an effort already injected into the shared request body looks like explicit client intent and leaks into the replacement request.

Change

Keep suffix provenance separate from the mutable request. Resolve defaults and dependent parameter constraints for each handler model attempt, including credential refresh and server-tool follow-ups. Explicit client reasoning fields remain authoritative even when model constraints remove their wire representation.

Include translated reasoning intent in context-aware deduplication hashes. Concurrent requests with different effort settings must not share a response. The existing two-argument hash interface and tenant namespace remain unchanged.

Verification

The post-commit batch passed 268 tests, with zero failures or skipped tests. Core TypeScript checking and formatting checks on all eight changed files passed. All 39 exact-site mutations triggered test assertion failures before restoring the source.

Local HTTP tests cover same-account retries, actual two-account fallback, model replacement, tool follow-ups, refreshed credentials, and overlapping high/none requests in both arrival orders. Equal high/high requests still deduplicate. Eleven baseline/candidate first-request wire cases also matched.

Three consecutive code-review cycles completed without a confirmed finding. This was one review context using three lenses per cycle, not independent-model consensus. The commit hook subsequently reformatted one existing call and added a trailing object comma; that non-behavioral delta was checked separately and the tests rerun.

Scope and known limitations

No deployment or production model probe is included. This fixes handler-level attempts, not provider-native target remapping or payload rules that rewrite model identity.

Two existing pipeline recovery defects were reproduced separately on the unchanged base: model-unavailable fallback can resend the old model, and non-streaming context recovery can return before fallback. Six corresponding baseline-failing cases remain separate diagnostic artifacts; they are not skipped tests in the passing batch.

Full-repository lint is not claimed green: 11 inherited unused-symbol diagnostics remain in the handlers. The existing unrelated stream file-size check also fails on the base. Neither was folded into this change.

Carry resolved suffix effort through dispatch without treating a derived
value as explicit client input. Prepare reasoning defaults and dependent
parameter constraints for each handler attempt so a replacement model
does not inherit the original model's suffix.

Keep explicit reasoning choices in context-aware request hashes to avoid
sharing concurrent responses across different effort settings. Preserve
the legacy hash interface and tenant namespace.

Exercise retries, credential refresh, tool follow-ups, replacement models
and overlapping requests with local HTTP and targeted regression tests.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
…tion

Keep synchronous per-attempt normalization together so payload preparation
stays within the function size and complexity limits without changing its
ordering or explicit reasoning semantics. Condense redundant provider
selection comments to retain the formatted file within its size ceiling.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Signed-off-by: Minxi Hou <houminxi@gmail.com>
@HouMinXi
HouMinXi force-pushed the fix/suffix-effort-propagation branch from 44a6280 to 0dfc9d6 Compare September 15, 2026 11:54
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for this — the suffix-effort/retry analysis is sharp, and the incidental fix (the
credential-refresh retry used to skip prepareUpstreamBody entirely, not just reasoning
defaults — payload rules and prompt_cache_key too) is a nice catch beyond the stated scope.
The new test suite is genuinely thorough (27 HTTP-harness scenarios, including real concurrent
requests over a live socket) and the dedup-hash extension has an explicit anti-spoof test,
which we appreciated.

Two things are blocking green right now, both mechanical and both ours to fix in-place: the
complexity-ratchets gate flags 2 new violations in the new prepareUpstreamBody normalization
chain (needs splitting into a helper), and file-size flags src/sse/handlers/chat.ts growing
past its frozen cap — mostly from the pre-commit hook's own Prettier reformat of an
already-over-100-col line you mentioned in the PR body. We'll handle both plus a rebase (which
also clears an unrelated check:secrets finding that turned out to be our branch trailing the
current tip on a file this PR doesn't touch, not something in your diff).

⚠️ base-red inherited: #12732 covers most of the remaining red (ESLint, docs env-var sync,
agent-skills-sync, a golden-snapshot test, and a pre-existing modelTestRunner.ts TS2741) —
confirmed identical on the current base tip, unrelated to this PR.

diegosouzapw and others added 3 commits September 16, 2026 04:18
The release tip independently grew src/sse/handlers/chatHelpers.ts from
1164 to 1213 lines while the frozen cap sat at 1214; this PR's own +3
lines (threading resolvedThinkingEffort through resolveModelOrError and
executeChatWithBreaker) push the merged result to 1217, past the cap.
Owner-approved exception for this file only, with the measured growth
breakdown recorded in the baseline entry.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
# Conflicts:
#	config/quality/file-size-baseline.json
#	open-sse/handlers/chatCore.ts
@diegosouzapw
diegosouzapw merged commit b9dd80c into diegosouzapw:release/v3.8.51 Sep 17, 2026
3 of 7 checks passed
@HouMinXi
HouMinXi deleted the fix/suffix-effort-propagation branch September 19, 2026 13:08
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…egosouzapw#13720)

* chat/suffix-effort: keep reasoning intent tied to each model attempt

Carry resolved suffix effort through dispatch without treating a derived
value as explicit client input. Prepare reasoning defaults and dependent
parameter constraints for each handler attempt so a replacement model
does not inherit the original model's suffix.

Keep explicit reasoning choices in context-aware request hashes to avoid
sharing concurrent responses across different effort settings. Preserve
the legacy hash interface and tenant namespace.

Exercise retries, credential refresh, tool follow-ups, replacement models
and overlapping requests with local HTTP and targeted regression tests.

Signed-off-by: Minxi Hou <houminxi@gmail.com>

* chat/upstream-body: separate normalization from async payload preparation

Keep synchronous per-attempt normalization together so payload preparation
stays within the function size and complexity limits without changing its
ordering or explicit reasoning semantics. Condense redundant provider
selection comments to retain the formatted file within its size ceiling.

Signed-off-by: Minxi Hou <houminxi@gmail.com>

* changelog: record the suffix-effort propagation fix

Signed-off-by: Minxi Hou <houminxi@gmail.com>

* fix(quality): rebaseline file-size cap for chatHelpers.ts growth

The release tip independently grew src/sse/handlers/chatHelpers.ts from
1164 to 1213 lines while the frozen cap sat at 1214; this PR's own +3
lines (threading resolvedThinkingEffort through resolveModelOrError and
executeChatWithBreaker) push the merged result to 1217, past the cap.
Owner-approved exception for this file only, with the measured growth
breakdown recorded in the baseline entry.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
diegosouzapw added a commit to TPOHH/OmniRoute that referenced this pull request Sep 29, 2026
Reconcile the discovered-tier routing with the tip's Codex reasoning work:
- applyCodexReasoningSelection now owns the force-rule (diegosouzapw#13556) and
  enabled:false handling plus the reasoning key whitelist (diegosouzapw#13643), so
  the block removed from codex.ts is not lost.
- Legacy caps use the tip's alias sets via getCodexAliasEffortCap.
- Discovery uses readCodexReasoningMetadata as the single source for
  supportedThinkingEfforts/defaultThinkingEffort (drops the duplicate
  helper added with diegosouzapw#14593).
- Thread runtimeModelInfo alongside resolvedThinkingEffort (diegosouzapw#13720).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
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.

2 participants