fix(openai-path): SSE stream reassembly, tool-call preservation, empty-response detection

[warelik]

Summary

Fixes a class of bugs producing empty completions (finish_reason: stop, completion_tokens: 0) and lost tool calls, especially on long thinking-model (Gemini 3.x) conversations with tool-call history:

• SSE stream reassembly: the streaming path ran JSON.parse on raw network chunks; chunks split mid-event or carrying multiple events were silently dropped, losing content/tool calls and surfacing as empty 200s. Events are now buffered and split on \r?\n\r?\n (CRLF included), tolerant to fragmentation. Applied to all streaming paths (OpenAI chat, OpenAI Responses API, Claude /v1/messages).
• Tool-call preservation: functionCall parts carrying thoughtSignature (mandatory on Gemini 3.x) were filtered out as "thinking" in response translators — pure tool-call answers became empty responses. FunctionCall parts are now always emitted; only genuine thought text is filtered.
• Request translation: functionResponse parts get their real names mapped from the assistant's tool_calls (was hardcoded unknown_function); consecutive same-role contents entries are merged (strict user/model alternation required upstream).
• Empty-upstream detection: genuinely empty terminal completions (no content, no tool calls, completion_tokens: 0) raise a retriable 502 empty_upstream_response feeding the account-switch/retry path instead of silently returning an empty 200. Judgment happens only at completion — thinking-first streams are never aborted mid-flight. Failing browser contexts are disposed after repeated empties instead of being rotated back in; rotation index handling after disposal fixed; account switching guarded against empty pools and in-flight disposal races.
• Non-stream mode: fixed SSE body parsing (was JSON.parse on SSE text — all non-stream requests failed); now covered by the same detection and retry.
• Defaults: immediateSwitchStatusCodes default is now [403, 429, 502, 503] — dead/limited/empty accounts trigger immediate account switch.
• Adds a node:test suite (npm test, 22 tests, no new dependencies): SSE reassembly (LF/CRLF/fragmented/multi-event), empty-detection truth table, tool-call/name mapping, role merge.
• Debug aid: DUMP_EMPTY_UPSTREAM=/path.jsonl env dumps raw payloads of judged-empty responses (inert when unset).

Test plan

• npm test — 22/22 pass (node:test, builtin runner)
• Offline SSE matrix: LF/CRLF, fragmented and multi-event chunks, pure tool-call streams, whitespace/empty stop
• Live: 122-message conversation with 59 tool_call/tool pairs that previously returned empty ~70% of the time now returns tool calls; empty upstreams produce 502 + account switch instead of silent empty 200

[warelik]

@/tmp/comment-228.md

[warelik]

Studio clean-fix publish — current-head evidence (commit faed976)

Scope: Independently reviewed CLEAN Studio fixes for the empty-stream failover gaps. This is a source-only change slice; no live-runtime claim is made and no SINGULARITY claim is made. Studio has no PR CI workflow, so no checks are expected by design.

Commit

• SHA: faed97692e07c56762681b02737c037a321081da
• Subject: fix(openai-path): close terminal empty-stream failover gaps
• Parent: 2517536459364a169979838bc525db98d031f628 (previous PR head)
• Branch: fix/openai-path-stream-toolcalls-empty-responses
• Push: normal fast-forward 2517536..faed976, no force/rebase/reset.

Tracked diff (exactly 5 files, nothing else)

src/auth/AuthSwitcher.js, src/core/FormatConverter.js, src/core/RequestHandler.js, test/formatConverter.test.js, test/requestHandler.test.js — 803 insertions / 122 deletions.

Gates (all run at current head faed976)

Gate	Result
Targeted node --test test/formatConverter.test.js test/requestHandler.test.js	67/67 pass
npm test	83/83 pass
npm run lint	0 errors (1 pre-existing warning in ui/app/pages/StatusPage.vue, not in this diff)
npm run format:check	clean (all files Prettier)
npm run build:ui	success (benign chunk-size warning only)
git diff --check	clean
Stage scope	staged name list == exactly the 5 files above; protected paths excluded

Root causes fixed

1. Terminal empty misjudgment before SSE flush — Claude real stream, OpenAI real stream, and Responses real stream now flush the trailing partial SSE event before judging emptiness at STREAM_END; a fragmented final event that reassembles in the buffer is no longer misjudged as empty.
2. Silent no-op error after headers sent — the old JSON _sendErrorResponse was a silent no-op once headers were sent mid-stream. The flows now emit a protocol-safe SSE error via _sendErrorChunkToClient (JSON error only when headers not yet sent), keeping exactly one auth switch + one error per true-empty stream.
3. Legitimate control finishes misjudged as empty — Gemini SAFETY/RECITATION/BLOCKLIST/PROHIBITED_CONTENT/IMAGE_SAFETY and OpenAI content_filter/safety are preserved as valid terminal results even with zero completion tokens; unknown/OTHER reasons are not exempted.
4. Claude fake-stream aggregate terminal-empty — now judged via _isEmptyUpstreamResponse and routed into the existing single auth-failure + SSE error flow instead of translating an empty stream to the client.
5. Responses initial complete-empty chunk — converted into the existing error/retry flow (one switch only, like the OpenAI chat real-stream path) before the translator can set responseSent.
6. Empty counter not reset on success — AuthSwitcher.resetEmptyJudgmentCountForAuth(index) clears the consecutive empty-judgment counter for the served auth index; all success sites route through the shared _resetFailureStateOnSuccess helper.
7. Responses reasoning effort not mapped — reasoning.effort (and the top-level reasoning_effort alias) now maps through THINKING_LEVEL_MAP to thinkingLevel, matching the chat path; unknown efforts keep includeThoughts only.

Remote PR state (verified via GitHub API)

• headRefOid = faed97692e07c56762681b02737c037a321081da ✅
• mergeable = MERGEABLE, mergeStateStatus = CLEAN ✅
• Checks: none reported — Studio has no pull_request CI by design.

Final tracked-clean status

git status shows zero tracked modifications; the only remaining untracked items are the protected exclusions (.npm-cache/, .serena/, configs/GASauth.zip, lima.REJECTED.yaml) which were never read, staged, or deleted.

[warelik]

Studio final-review publish — current-head evidence

Commit: b3f3b50f3dfdbb6f7113d3185da64152b0144bf6
Head: fix/openai-path-stream-toolcalls-empty-responses → b3f3b50 (was faed976)
Base: main | PR head OID: b3f3b50f3dfdbb6f7113d3185da64152b0144bf6
Mergeability: MERGEABLE / CLEAN | State: OPEN

Gates (at this commit)

• npm test — 87/87 pass, 0 fail
• npm run lint — 0 errors (1 pre-existing vue/no-v-html warning in ui/app/pages/StatusPage.vue, unrelated to this change)
• npm run format:check — All matched files use Prettier code style
• npm run build:ui — pass (chunks built; only a pre-existing >500 kB chunk-size advisory)

Scope — exactly four tracked files in this commit

src/core/RequestHandler.js, src/utils/MessageQueue.js, test/requestHandler.test.js, test/messageQueue.test.js
(4 files changed, +278 / −1. No caches, configs, or artifacts staged.)

Independent review

Reviewer agent-408 verdict: CLEAN — full test suite green, lint/format/build green, diff scope exactly the four files above.

Changes

1. OpenAI chat fake-stream terminal-empty guard — an empty aggregated body now enters the existing single auth-failure + SSE error flow via _handleAuthFailure (exactly one switch, no duplicate) + _handleRequestError, ending the stream with no leaked empty completion; includes upstream correlation dump for the openai-chat-fake-stream case.
2. MessageQueue no-arg default restored — defaultTimeout back to 300000 for no-arg construction; explicit 0 remains unlimited (e.g. stream block-timeout behavior preserved).

Worktree hygiene

Only untracked/gitignored local artifacts remain (configs/GASauth.zip, lima.REJECTED.yaml, caches). No tracked diff beyond this commit.

Please review at your convenience.

[warelik]

Studio review finding — comment-only resolution (commit 57e4f8c)

Resolves the single accepted current-head finding: a P3 defect, independently adjudicated REAL_FIX_REQUIRED, in src/core/RequestHandler.js.

Defect: Three comments at the non-stream correlation-dump call sites claimed the dump records every judged upstream response (empty AND non-empty, judged_empty:false visible). The helper _dumpUpstreamCorrelation intentionally early-returns on non-empty (if (!this._isEmptyUpstreamResponse(rawData)) return), so those comments promised behavior the code does not have.

Contract decision: preserve the low-noise, judged-empty-only behavior. Comments corrected to state exactly what the helper does — records only upstream responses judged empty, gated by DUMP_EMPTY_UPSTREAM; non-empty upstream responses are not recorded.

Deliverable — comment-only delta, zero executable change:

• 3 comment blocks corrected (src/core/RequestHandler.js): 6 insertions / 9 deletions, all lines inside // comments.
• Executable-diff proof: strip all // and /* */ comments from HEAD vs worktree → identical non-blank line content; git diff -U0 classification → removed=9 (comment=9), added=6 (comment=6) ⇒ zero executable diff.
• Helper _dumpUpstreamCorrelation untouched; no behavior or test change.

Gates (run at 57e4f8c):

Gate	Result
npx prettier --check src/core/RequestHandler.js	clean
npx eslint src/core/RequestHandler.js	clean (0 errors)
node --check src/core/RequestHandler.js	pass
npm run format:check	all files Prettier
npm run lint	0 errors (1 pre-existing unrelated vue/no-v-html warning in ui/app/pages/StatusPage.vue)
node --test	87/87 pass, 0 fail

Commit / push state:

• SHA 57e4f8c0a5807bdcdd4baef1f5edb1a66cef9323 — docs(debug): clarify empty-response dumps
• Clean fast-forward b3f3b50..57e4f8c, no amend/reset/rebase/force.
• PR head OID 57e4f8c0… matches local and remote; mergeable = MERGEABLE, state OPEN.
• Staged exactly 1 file (src/core/RequestHandler.js); tracked tree clean afterward; protected exclusions untouched.

Please review at your convenience.

[warelik]

@codex please review the current head 57e4f8c (comment-only doc fix resolving the P3 correlation-dump comment defect)

[chatgpt-codex-connector]

To use Codex here, create a Codex account and connect to github.

[warelik]

Studio P3 comment-only fix — current-head evidence (commit 166551c)

Head: fix/openai-path-stream-toolcalls-empty-responses 57e4f8c -> 166551c7b25a9d1d081360082749b1b03e78d5f7

Resolves the sole current-head ponytail finding (agent-441, P3 REAL_REQUIRED): duplicate identical consecutive comment in src/core/FormatConverter.js (was lines 834-835).

Change (1 deletion, comment-only):

-        // Merge consecutive contents with the same role (Gemini API requires strict role alternation).

Verification (all green, exit 0 unless noted):

• Executable/comment-stripped content byte-identical to 57e4f8c (grep -v ^// compare)
• git diff --check clean; diff = 1 deletion / 0 insertions
• npx prettier --check src/core/FormatConverter.js = 0
• npx eslint src/core/FormatConverter.js = 0
• node --check src/core/FormatConverter.js = 0
• node --test = 87/87 pass, 0 fail (full suite)
• Repo-wide format:check flags only protected/untracked .rev_tmp/comment-studio-57e4f8c.md (outside this change); product file clean
• Pre/post OIDs: 57e4f8c0... -> 166551c7...; local == fork == PR head, tracked tree clean

No service/runtime action. No other comment/refactor edits.

[warelik]

@codex please review the current head 166551c (comment-only: removed duplicate merge comment in src/core/FormatConverter.js)

[chatgpt-codex-connector]

To use Codex here, create a Codex account and connect to github.