Repository navigation
fix(reasoning): preserve compatible response state - #10574
diegosouzapw merged 12 commits into
Conversation
|
Live continuation verification update:
Current focused validation:
The PR body now separates plaintext and opaque transports, includes the Chat/Responses streaming matrix, and distinguishes completed live |
bfa4a48 to
1060683
Compare
|
This is a solid, well-scoped fix — the plaintext-vs-opaque reasoning transport split and the target-local reject/drop behavior for Combo fallback are correctly threaded through both call sites (chatCore.ts and executors/codex.ts), and the cross-layer wiring (Zod schema, DB-backed combo config, i18n'd dashboard toggle with a drop-mode warning) is complete. CI is fully green. Given the size of the diff, it may be worth one more focused pass over the transport-compatibility matrix in responsesInputPolicy.ts before merging, but I found no blocking issues. |
…onses-reasoning-transport # Conflicts: # open-sse/handlers/chatCore.ts # open-sse/handlers/responseTranslator.ts # src/sse/handlers/chat.ts # src/sse/handlers/chatHelpers.ts # tests/unit/chatcore-translation-paths.test.ts
|
Post-merge regression repair is pushed in
Refreshed Quality Gates run: https://github.com/diegosouzapw/OmniRoute/actions/runs/32342839553 The PR-specific Codex failure is absent from the refreshed failure list. Build, Vitest, ESLint, merge integrity, and change classification pass. The run remains red on the previously identified |
…asoningTransportFallback (+2)
…eadfix # Conflicts: # config/quality/file-size-baseline.json
769ab62
into
diegosouzapw:release/v3.8.50
…stream content With a Responses-API upstream (e.g. grok-cli) serving a Chat Completions client, translate mode collects the streamed assistant content from the raw upstream events. No branch recognised Responses events, so the generic fallback appended every string `delta` and `text` field: reasoning deltas, the `response.reasoning_text.done` snapshot, output deltas, the `response.output_text.done` snapshot and even function_call_arguments deltas. The collected content became "Think hard.Think hard.Hello worldHello world" while the client itself received the correct stream. That content is not only the call-log responseBody: chatCore also feeds it to the semantic cache, memory extraction and reasoning replay, which now receive the real answer. Only `response.output_text.delta` now contributes to content; every other Responses event is kept out of the generic fallback. Reasoning is left to the response translator, which already stores the replayable reasoning text on `output_item.done` (diegosouzapw#10574), so it is not collected twice. Usage estimation still counts every string delta.
Preserves authentic plaintext reasoning continuations across Chat Completions and Responses (streaming + non-streaming), applying one target-aware reasoning transport policy before protocol translation. Fixes diegosouzapw#10550. Validated in an isolated worktree boarded onto origin/release/v3.8.50 (0 conflicts, 39 files): - 446/446 focused node:test tests pass (chat-route-coverage, chatcore-translation-paths, combo-attempt-body-isolation-7847, combo-config, executor-codex, kimi-coding-translator, moonshot-k3, reasoning-cache, response-sanitizer, responses-handler, responses-translation-fixes, strip-reasoning-blobs-agentic-context-1599, translator-openai-responses-req). - 12/12 vitest tests pass (edit-connection-modal-free-models.test.tsx). - check-changelog-integrity: OK. - typecheck:core: clean. - check-complexity / check-cognitive-complexity: OK, both under baseline. - file-size: chatHelpers.ts crossed the frozen cap by +2 lines (irreducible reasoningTransportFallback option threading) — rebaselined 1017->1019 with justification, pushed to the PR branch (fix-in-place), re-validated after a base-drift re-merge against the latest release tip. Co-authored-by: jackjinke <jackjinke@users.noreply.github.com>
Summary
summary/summary_textseparate. Summaries never participate in compatibility decisions. Summaries attached to active reasoning are preserved; orphan summaries are removed from explicitly stateless (store: false) input.drop: remove only incompatible active reasoning for the current target attempt, without mutating the original request or later targets. Explicitskipremains strict fallthrough behavior.Fixes #10550.
Before / After
Before, reasoning compatibility was coupled to Responses routing and missing provider metadata failed closed. This rejected portable plaintext for unannotated targets such as OpenCode Go, while Chat input and stream mode could follow different paths. Combo also defaulted to skipping the target.
After, both Chat and Responses inputs are inspected before request translation and before streaming/non-streaming handling diverges:
plaintextopaqueornoneopaqueplaintext, ornonestoreunset ortruestore: falseProvider-generated state is never synthesized or cross-converted. Rejected requests remain unmodified, and target-local drop clones only the affected Chat messages or Responses items.
Validation
Latest post-merge repair suite:
The broader focused reasoning suite previously reported:
Covered paths include:
reasoning_textreplayAdditional checks passed:
npm run typecheck:corenpm run check:changelog-integritygit diff --checkFocused coverage for
reasoningInputPolicy.tsafter the repair reports 86.72% statements/lines, 85.60% branches, and 90.90% functions. The earlier safe explicit-file coverage run on the broader branch reported 65.79% statements/lines, 66.49% branches, and 78.46% functions. Repository-wide coverage is intentionally left to CI because the root test glob includes a lifecycle test that can terminate listeners on port 20128.npm run check:open-sse-typecheckstill reports the unchanged release-branchGlmExecutor.buildHeadersmismatch atopen-sse/executors/index.ts:239; this PR does not widen the frozen baseline.Risk
The compatibility decision is internal and target-scoped. Plaintext remains portable by default; only provider-bound opaque state needs explicit metadata. Direct single-target calls still return
400for genuinely incompatible active state. Combo drop never changes capability filtering, effort, budgets, provider thinking controls, or the original fallback body. Stateless input removes only reasoning records with no active plaintext or opaque continuation state.