Repository navigation
fix(sse): provider-response summary format bugs (dashboard Provider Response panel) - #10037
Conversation
1ce45d0 to
04d95dc
Compare
04d95dc to
121ae54
Compare
The dashboard's "Provider Response" panel showed a stale, incomplete snapshot for long streamed responses. Root cause: open-sse/utils/stream.ts reconstructed the summary from buildStreamSummaryFromEvents(providerPayloadCollector.getEvents(), ...) -- but getEvents() only returns whatever survived the collector's maxEvents/maxBytes cap, so once a stream exceeded it (easy with a reasoning + tool-calling model), everything after the cutoff (final finish_reason, tool_calls, rest of reasoning_content, usage) was silently dropped from the reconstruction, even though the client actually received the correct, complete response. Fix: streamPayloadCollector.ts's per-format summary builders (buildOpenAISummary/buildResponsesSummary/buildClaudeSummary/ buildGeminiSummary) are now also available as incremental reducers (createXReducer: ingest one chunk at a time, finalize at the end). createStructuredSSECollector accepts a format + fallbackModel and feeds the reducer on every push() -- including chunks that get dropped from the retained event array once the cap is hit -- via a new getSummary() method. stream.ts's error-path call site now uses collector.getSummary() instead of reconstructing from the (possibly truncated) getEvents(). Extracted from a squashed commit (originally authored alongside a conversation-tracking continuation fix in the same commit) -- only the files relevant to this SSE-summary bug are included here (stream.ts/streamPayloadCollector.ts + their test); the unrelated conversationTracker.ts continuation fix stays with the conversation- tracking PR it belongs to. Test plan: - New TDD regression tests in tests/unit/stream-payload-collector.test.ts, confirmed failing before the fix and passing after.
… provider's providerPayloadCollector (dashboard "Provider Response" panel) was keyed on sourceFormat (the CLIENT's wire format) instead of targetFormat (the PROVIDER's — see createSSEStream's own @PARAM doc: "targetFormat - Provider format", "sourceFormat - Client format"). Whenever a request translates between two different formats — e.g. a Responses-API client routed to a plain-OpenAI-chat-completions upstream, the common OpenClaw/opencode-zen shape — the reducer picked for sourceFormat could never recognize the provider's actual raw event shape, so it stayed stuck at its empty initial state. The dashboard's "Provider Response" panel showed a permanently empty `output: []` while "Client Response" (built from separately-accumulated state, unaffected by this bug) correctly showed full content — reading as if the two panels simply disagreed about the same request. Confirmed live via a wire-level pcap capture (scripts/sre/tcp-close- analyzer.py) cross-referenced against the dashboard log (1786032832181-1c6275): the actual response was complete and correct: this was purely a logging/summary bug, never a wire-format bug. Fix is mode-aware: TRANSLATE mode uses targetFormat (the provider's true format); PASSTHROUGH mode keeps sourceFormat, since passthrough has no separate provider/client format split — nothing gets translated there, and real passthrough callers (createPassthroughStreamWithLogger) don't even pass targetFormat. New regression test reproduces the exact live scenario (Responses-API source, OpenAI target, real chat.completion.chunk deltas) and asserts the provider summary reflects them — confirmed it fails with the old `sourceFormat`-keyed code (reproducing the live `output: []`-style symptom) and passes with the fix. Co-authored-by: Markus Hartung <markus.hartung@gmail.com>
createSSEStream's providerPayloadCollector.build() falls back to the
synthesized responseBody as the "Provider Response" dashboard summary
whenever sourceFormat/targetFormat isn't OPENAI_RESPONSES (in both the
passthrough and translate branches) -- but responseBody is built purely
for the client and never carries an `object` field at all, so the
summary ended up with `object: undefined` instead of the expected
"chat.completion", even though everything else (choices, usage) was
correct.
Caught by this PR's own new regression test ("createSSEStream translate
mode: providerPayload summary reflects the PROVIDER's format, not the
client's") -- the code itself was unchanged by the rebase (applied
cleanly from the original commit), so this was a latent gap in the
original fix, not a rebase regression.
Fix: stamp `object: "chat.completion"` on a shallow copy used only for
the provider summary in both branches; responseBody itself (sent to the
client elsewhere) stays untouched.
Verified: tests/unit/stream-utils.test.ts 51/52 passing (the one
remaining failure is an unrelated, pre-existing v3.6.6-era test,
confirmed present and failing identically on a pristine
upstream/release/v3.8.50 checkout -- base-red inherited: cheaper-inference#9985).
typecheck/lint clean (pre-existing unrelated errors elsewhere in the
file, confirmed identical to upstream).
121ae54 to
47b250f
Compare
|
Reviewed via TDD probe against a fresh worktree off release/v3.8.50 (merge-base 1 commit behind tip, no base-drift). All three claimed bugs verified independently: reverting the prod diff makes the corresponding new regression test fail with the exact symptom described (getSummary is not a function for bug 1; providerPayload.object undefined instead of 'chat.completion' for bugs 2+3 combined). Full suite green hermetically: stream-utils.test.ts 52/52, stream-payload-collector*.test.ts 19/19, plus the untouched sibling request-log-payloads.test.ts (still exercises getEvents()) unaffected at 9/9. The apparent -366 line deletions are reformatting noise, not real removals -- confirmed buildOpenAISummary/buildResponsesSummary/buildClaudeSummary/buildGeminiSummary all still exist, still internal-only, no lost callers. Raw eslint shows 17 hits but they match config/quality/eslint-suppressions.json's frozen counts exactly (1 pre-existing no-restricted-syntax + 16 pre-existing no-explicit-any) -- zero new violations, Note: the base-red that was failing this PR's CI (#9985) was drained today — base-reds PR #10213 just merged into |
4bda225
into
cheaper-inference:release/v3.8.50
|
Merged — thanks @hartmark! All three Provider Response summary bugs verified independently; the live per-format reducer is a real improvement over the capped getEvents() reconstruction. |
…esponse panel) (cheaper-inference#10037) * fix(sse): provider-response summary reconstructed from truncated events The dashboard's "Provider Response" panel showed a stale, incomplete snapshot for long streamed responses. Root cause: open-sse/utils/stream.ts reconstructed the summary from buildStreamSummaryFromEvents(providerPayloadCollector.getEvents(), ...) -- but getEvents() only returns whatever survived the collector's maxEvents/maxBytes cap, so once a stream exceeded it (easy with a reasoning + tool-calling model), everything after the cutoff (final finish_reason, tool_calls, rest of reasoning_content, usage) was silently dropped from the reconstruction, even though the client actually received the correct, complete response. Fix: streamPayloadCollector.ts's per-format summary builders (buildOpenAISummary/buildResponsesSummary/buildClaudeSummary/ buildGeminiSummary) are now also available as incremental reducers (createXReducer: ingest one chunk at a time, finalize at the end). createStructuredSSECollector accepts a format + fallbackModel and feeds the reducer on every push() -- including chunks that get dropped from the retained event array once the cap is hit -- via a new getSummary() method. stream.ts's error-path call site now uses collector.getSummary() instead of reconstructing from the (possibly truncated) getEvents(). Extracted from a squashed commit (originally authored alongside a conversation-tracking continuation fix in the same commit) -- only the files relevant to this SSE-summary bug are included here (stream.ts/streamPayloadCollector.ts + their test); the unrelated conversationTracker.ts continuation fix stays with the conversation- tracking PR it belongs to. Test plan: - New TDD regression tests in tests/unit/stream-payload-collector.test.ts, confirmed failing before the fix and passing after. * fix(sse): provider-response summary used the client's format, not the provider's providerPayloadCollector (dashboard "Provider Response" panel) was keyed on sourceFormat (the CLIENT's wire format) instead of targetFormat (the PROVIDER's — see createSSEStream's own @PARAM doc: "targetFormat - Provider format", "sourceFormat - Client format"). Whenever a request translates between two different formats — e.g. a Responses-API client routed to a plain-OpenAI-chat-completions upstream, the common OpenClaw/opencode-zen shape — the reducer picked for sourceFormat could never recognize the provider's actual raw event shape, so it stayed stuck at its empty initial state. The dashboard's "Provider Response" panel showed a permanently empty `output: []` while "Client Response" (built from separately-accumulated state, unaffected by this bug) correctly showed full content — reading as if the two panels simply disagreed about the same request. Confirmed live via a wire-level pcap capture (scripts/sre/tcp-close- analyzer.py) cross-referenced against the dashboard log (1786032832181-1c6275): the actual response was complete and correct: this was purely a logging/summary bug, never a wire-format bug. Fix is mode-aware: TRANSLATE mode uses targetFormat (the provider's true format); PASSTHROUGH mode keeps sourceFormat, since passthrough has no separate provider/client format split — nothing gets translated there, and real passthrough callers (createPassthroughStreamWithLogger) don't even pass targetFormat. New regression test reproduces the exact live scenario (Responses-API source, OpenAI target, real chat.completion.chunk deltas) and asserts the provider summary reflects them — confirmed it fails with the old `sourceFormat`-keyed code (reproducing the live `output: []`-style symptom) and passes with the fix. Co-authored-by: Markus Hartung <markus.hartung@gmail.com> * fix(sse): stamp object: chat.completion on the provider-summary fallback createSSEStream's providerPayloadCollector.build() falls back to the synthesized responseBody as the "Provider Response" dashboard summary whenever sourceFormat/targetFormat isn't OPENAI_RESPONSES (in both the passthrough and translate branches) -- but responseBody is built purely for the client and never carries an `object` field at all, so the summary ended up with `object: undefined` instead of the expected "chat.completion", even though everything else (choices, usage) was correct. Caught by this PR's own new regression test ("createSSEStream translate mode: providerPayload summary reflects the PROVIDER's format, not the client's") -- the code itself was unchanged by the rebase (applied cleanly from the original commit), so this was a latent gap in the original fix, not a rebase regression. Fix: stamp `object: "chat.completion"` on a shallow copy used only for the provider summary in both branches; responseBody itself (sent to the client elsewhere) stays untouched. Verified: tests/unit/stream-utils.test.ts 51/52 passing (the one remaining failure is an unrelated, pre-existing v3.6.6-era test, confirmed present and failing identically on a pristine upstream/release/v3.8.50 checkout -- base-red inherited: cheaper-inference#9985). typecheck/lint clean (pre-existing unrelated errors elsewhere in the file, confirmed identical to upstream). --------- Co-authored-by: Markus Hartung <markus.hartung@gmail.com>
Summary
Extracted from PR #9439 (agentic conversation tracking) — these are general SSE/dashboard bugs affecting the "Provider Response" panel for any streamed request, not specific to conversation tracking. Splitting them out for independent, faster review.
Three related bugs, fixed in sequence:
Stale/truncated summary reconstruction (fix(backend): dashboard log viewer shows stale provider response for long streamed responses #9315).
open-sse/utils/stream.tsreconstructed the dashboard's "Provider Response" summary frombuildStreamSummaryFromEvents(providerPayloadCollector.getEvents(), ...)— butgetEvents()only returns whatever survived the collector'smaxEvents/maxBytescap. Once a stream exceeded it (easy with a reasoning + tool-calling model), everything after the cutoff (finalfinish_reason,tool_calls, rest ofreasoning_content,usage) was silently dropped from the reconstruction, even though the client actually received the correct, complete response.Fix:
streamPayloadCollector.ts's per-format summary builders are now also available as incremental reducers (ingest one chunk at a time, finalize at the end).createStructuredSSECollectoraccepts aformat/fallbackModeland feeds the reducer on everypush()— including chunks later dropped from the retained event array — via a newgetSummary()method.Client format used instead of provider format.
providerPayloadCollectorwas keyed onsourceFormat(the CLIENT's wire format) instead oftargetFormat(the PROVIDER's). Whenever a request translates between two different formats — e.g. a Responses-API client routed to a plain-OpenAI-chat-completions upstream (the common OpenClaw/opencode-zen combo shape) — the reducer picked forsourceFormatcould never recognize the provider's actual raw event shape, so it never left its empty initial state. The dashboard's "Provider Response" panel permanently showedoutput: []while "Client Response" correctly showed full content, reading as if the two panels simply disagreed about the same request.Confirmed live via a wire-level pcap capture cross-referenced against a dashboard log entry: the actual response was complete and correct — this was purely a logging/summary bug, never a wire-format bug.
Missing
objectmarker on the fallback summary. TheresponseBody-as-summary fallback path (used when the format doesn't need the events-derived summary) never setobject: "chat.completion"at all, caught by this PR's own new regression test.Test plan
tests/unit/stream-payload-collector.test.tsandtests/unit/stream-utils.test.ts— confirmed failing against the pre-fix code, passing after.tests/unit/stream-utils.test.ts+tests/unit/stream-payload-collector.test.ts+tests/unit/stream-payload-collector-9315-truncated-provider-response.test.ts— 70/71 passing (the one remaining failure, "passthrough splits mixed reasoning and content deltas", is a pre-existing v3.6.6-era test confirmed failing identically on a pristineupstream/release/v3.8.50checkout — unrelated to this change).npx tsc --noEmit— clean for all changed files (pre-existing unrelated errors elsewhere confirmed identical to upstream, zero diff at those lines).npm run lint— clean.