Skip to content

fix(tests): realign two more stream-utils passthrough cases with the #10017 control-line rules - #10796

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.50from
MichaelYcJo:fix/release-v3.8.50-basereds-stream-utils
Aug 20, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.50from
MichaelYcJo:fix/release-v3.8.50-basereds-stream-utils

Conversation

@MichaelYcJo

Copy link
Copy Markdown
Contributor

Follow-up to #10726 (merged). Two more stream-utils cases are red on release/v3.8.50 for the same reason the ones in that PR were: #10473 changed what passthrough forwards and only added new coverage, leaving these asserting the framing it replaced.

Test-only change.

The two cases

forwards data only after the complete SSE event boundary — the fixture emits response.output_text.delta against a responses-model, so it is an OpenAI Responses scenario. event: framing is forwarded for exactly that format and for Claude Messages (stream.ts:833), so the case now declares clientResponseFormat and passes on its own terms.

preserves event metadata in a single SSE event — this one cannot be fixed by declaring a format. : comments, id: and retry: are dropped for every client format, since none of the three supported SSE protocols define them (streamHelpers.ts → remember). The expectation now pins what the case is actually about — the surviving lines stay inside ONE event instead of being split apart by blank lines — and asserts the dropped lines are gone.

Verification

node --import tsx/esm --test tests/unit/stream-utils.test.ts
  base 3d7ed7aa8 : 4 failed | 48 passed
  this branch    : 2 failed | 50 passed

npm run test:vitest on the same base is already fully green (368/368).

Two failures in the same file are NOT addressed here

They are left alone deliberately — both look like behaviour changes rather than stale expectations, and getting them wrong would affect billing and tool routing rather than just a test.

passthrough merges Claude usage chunks and restores mapped tool names — the fixture's message_start carries usage: { input_tokens: 6 }, but the completion payload reports prompt_tokens: 33. An upstream-reported input count appears to be replaced by an estimate. If that is intended, the case needs a new expectation; if not, the estimate is overriding real usage data and the fix belongs in the usage merge path, not in this test.

translate mode converts Claude SSE into OpenAI chunks and completion payload — fails in the same file and likely shares that cause.

Also still red on this base and untouched here: tests/unit/translator-tool-call-shim.test.ts → streaming: Read suppresses raw pages delta and emits cleaned input at finish. The shim itself is correct (/etc/hosts is not a PDF, so sanitizeReadArgs deletes pages), but the streaming path through openaiToClaudeResponse emits the uncleaned input — one delta is produced, so the shim is reached, yet "pages" survives in partial_json.

Happy to take any of these in a follow-up if the intended behaviour is confirmed.

⚠️ base-red inherited: #9985 — release/v3.8.50 was already red when this branch was cut.

…ntrol-line rules

Two passthrough cases still asserted the pre-#10017 framing and have been red on
release/v3.8.50 since. #10473 stopped forwarding upstream SSE control lines, but
only added new coverage — it left these two behind.

The event-boundary case is an OpenAI Responses scenario (its fixture emits
response.output_text.delta against a responses-model), so it now declares
clientResponseFormat accordingly; `event:` framing is forwarded exactly for that
format and for Claude Messages.

The metadata case cannot be fixed by declaring a format: `:` comments, `id:` and
`retry:` are dropped for every client format, since none of the three supported
SSE protocols define them. Its expectation now pins what the case is actually
about — the surviving lines stay inside ONE event instead of being split by blank
lines — and asserts the dropped lines are gone.
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Verified independently: rebuilt the passthrough SSE prefix-buffer logic in open-sse/utils/streamHelpers.ts (createSSEEventPrefixBuffer/remember) and stream.ts against these two test cases, and ran node --import tsx/esm --test tests/unit/stream-utils.test.ts on the base tip and on this branch in an isolated worktree — base fails 4, this branch fails only the 2 you flagged as deliberately untouched (usage-merge, translate-mode). The new assertions match current behavior exactly (control-line drop is unconditional per remember(); event: forwarding gates on clientResponseFormat), so this isn't a weakened assertion, it's a genuine realignment. No overlap with #10795's file set. Merging as-is.

@diegosouzapw
diegosouzapw merged commit 85aa84e into cheaper-inference:release/v3.8.50 Aug 20, 2026
5 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…heaper-inference#10017 control-line rules (cheaper-inference#10796)

Merged via merge-train (release/v3.8.50, batch1 2026-08-20) — static gates (typecheck/file-size/complexity/cognitive/changelog) green on the combined tree; test:unit reds observed in the boarded run were verified pre-existing on the pure release tip (unrelated flake), not caused by this PR. Thanks for the contribution!
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