Repository navigation
fix(chat): preserve suffix reasoning intent across model attempts - #13720
diegosouzapw merged 6 commits into
Conversation
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>
44a6280 to
0dfc9d6
Compare
|
Thanks for this — the suffix-effort/retry analysis is sharp, and the incidental fix (the Two things are blocking green right now, both mechanical and both ours to fix in-place: the
|
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
b9dd80c
into
cheaper-inference:release/v3.8.51
…eaper-inference#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>
Reconcile the discovered-tier routing with the tip's Codex reasoning work: - applyCodexReasoningSelection now owns the force-rule (cheaper-inference#13556) and enabled:false handling plus the reasoning key whitelist (cheaper-inference#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 cheaper-inference#14593). - Thread runtimeModelInfo alongside resolvedThinkingEffort (cheaper-inference#13720). Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…s-version consent #16810 gated the internal cross-version family hop behind `x-omniroute-allow-cross-version-fallback: true`; without it an empty-content answer from claude-sonnet-4.6 is returned as 502 instead of being recovered on claude-sonnet-4.5. The three "empty family fallback preserves only explicit effort" cases (from #13720) exercise exactly that hop and never sent the header, so they failed with `502 !== 200` after 24s. Send the opt-in; the effort propagation they assert is unchanged. Refs #15306
…port, route-guard locale proof, parseModel alias, family-fallback consent (#17007) * test(sse): align the parseModel cache-key expectation with the #16811 Opus 4.5 contract #16811 made the provider-qualified `anthropic/claude-opus-4.5` alias keep its `anthropic` prefix and resolve to the dashed wire ID (`claude-opus-4-5`), leaving only the bare alias on the dated snapshot. It updated its own regression test but not this sibling from #16341, which still expected `provider: null`, `claude-opus-4-5-20251101`, `isAlias: true`. The compat on/off results still differ, which is what the cache-key test is about. Refs #15306 * test(docs): bound the route-guard locale proof by the current paragraph order #16691 moved the deny-list test reference ahead of the regex companion constant in docs/security/ROUTE_GUARD_TIERS.md; the mirrors followed, but this test (from #16504) still searched for that reference AFTER `SPAWN_CAPABLE_PATTERN_ANCESTORS` and reported "missing localized policy proof" for all 66 locales. The window now runs from the Kiro auto-import deny-list entry to the regex companion and asserts the deny-list proof inside it; all 66 mirrors pass and the English-fallback and hash checks are unchanged. Refs #15306 * fix(mcp): keep apiKeyPolicy and next/headers out of the MCP server bundle's import time `git bisect` on tests/unit/build/mcp-bundle-startup.test.ts lands on #16810, which made src/shared/utils/modelFallbackAuthorization.ts import `resolveEnforcedPolicyKey` from apiKeyPolicy. That module is reached from open-sse (modelFamilyFallbackPolicy), so the MCP server bundle started carrying apiKeyPolicy → apiAuth → `next/headers` (esbuild hoists the external import; plain Node cannot resolve it) and, once that is out of the way, an async init cycle around sse/services/auth that leaves the import with an unsettled top-level await (exit 13). #16499 then opened a second path into apiKeyPolicy through keyConnectionScope → comboTargetKeyPolicy on the embeddings service. Load the three edges lazily, at the call sites that are already async: modelFallbackAuthorization and keyConnectionScope import apiKeyPolicy / comboTargetKeyPolicy inside the functions that use them, and apiAuth imports `next/headers` only when it actually falls back to the request cookie store. The bundle imports again on Node 24 and the request paths are unchanged. Refs #15306 * test(chat): opt the family-fallback effort cases into the #16810 cross-version consent #16810 gated the internal cross-version family hop behind `x-omniroute-allow-cross-version-fallback: true`; without it an empty-content answer from claude-sonnet-4.6 is returned as 502 instead of being recovered on claude-sonnet-4.5. The three "empty family fallback preserves only explicit effort" cases (from #13720) exercise exactly that hop and never sent the header, so they failed with `502 !== 200` after 24s. Send the opt-in; the effort propagation they assert is unchanged. Refs #15306
Problem
Resolved model suffixes such as
-maxand-nonecould 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.