feat: a stream folds into the completion a non-streaming call would have returned #18
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/stream-accumulator"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The second of the three streaming PRs: the accumulator. #16 made every chunk parse. The next PR adds the
Result-returning stream call, which drives this fold, with retry before the first chunk and the opt-in stall watchdog. Nothing here changes an existing member's behaviour, apart from the one review fix below. The branch hasmainmerged in after #17 (b8351b2); the merge was clean and the suite is green on it.ChatCompletionStreamAccumulatorFailureis needed for a partial result. It is not thread-safe, andToResponse()throws until a chunk has been folded.ToResponse()builds a newChatCompletionResponseon every call, with object kindchat.completion, choices in index order and noError.indexintoToolCalls you can echo.Indexis null on output, because a non-streaming response has none.function.IsFinishedis true once every choice has a finish reason other thanerror, or once[DONE]was recorded.MarkDone()is internal: only the library's own stream reader sees the sentinel, which the next PR uses. A fold you drive from the public iterator finishes on finish reasons alone, and its summary says so.errorobject is not kept, andfinish_reason: "error"is recorded but does not finish the fold. The README sample checkschunk.Errorfirst.Where it departs from the reviewed brief (please confirm)
{}. The brief saidArguments = args, which gave""where a non-streaming response carries"{}", so the echo promise did not hold. This applies only once the fold is finished; until then a call keeps exactly what arrived, so a cut-off call is not dressed up as complete."finish_reason": ""finished the fold (whileChoice.FinishReasonreads it as null), and"type": ""echoed as""instead offunction."choices":[null],"tool_calls":[null]and"reasoning_details":[null]parse as null entries. A literal fold threwNullReferenceExceptionon the first two and emitted an empty text segment on the third. The next PR carries the same guard where it walksChoices.Declined: splitting id-less calls on one index when a new name arrives. No captured stream shows that shape from OpenRouter, the heuristic would still merge two calls to the same function, and it belongs in a brief revision backed by a capture.
Review fix from #16 (Jibril)
NumberOrStringJsonConverter.Writewrote any textint.TryParseaccepts as a number, so aRawCodeof"0502"or"+502"went out as502. It now writes a number only when the text is exactly the integer's own spelling. Two theory rows pin it; with the new condition removed, both rows fail.Tests
d7ca54b, after #17)StreamAccumulatorTests(+71) covers every fold rule, including:StreamChunkParsingTestsgains the two converter rows.Coverage:
ChatCompletionStreamAccumulator.csis at 163/163 lines with no partial branches, andAssistantDelta.csandNumberOrStringJsonConverter.csare fully covered.Mutation probes: 71 against the first cut and 19 more against the review round. They include:
calls[0]instead of the last call;{}substitution applied always or never;All are killed except one equivalent mutant:
IsCompleteJson's whitespace fast path, whichJsonDocument.Parsemakes redundant.README
Choice.Deltarow of "Upgrading from 0.2" now points at the accumulator for echoableToolCalls.AssistantDelta's summary turns its<c>into acref.Out of scope
The
Result-returning stream call and the default interface member, theSseLinereader, the stall watchdog, and retry before the first chunk all come in the next PR, which also names itself inIsFinished's summary.🤖 Generated with Claude Code
ChatCompletionStreamAccumulator takes the chunks of one streamed call in arrival order and builds the ChatCompletionResponse the same call would have returned without streaming: content, reasoning and refusal joined per choice, choices in index order, the first non-empty id, model and creation time, and the last reported provider, tier, fingerprint and finish reasons. It keeps what arrived after a failure, does not judge error chunks, and finishes once every choice has a finish reason other than error or once the client records [DONE] (MarkDone, internal). Tool-call fragments join by index into ToolCalls that can be echoed: a new id on an occupied index, or complete JSON onto complete arguments, starts a second call; the same id with complete arguments again is a re-send; an id-less call adopts the first id that arrives; output calls carry no stream index, and once the fold is finished a call that received no arguments reads {}. Text and summary reasoning segments merge by type and index, provider fields keep their last non-blank value so a signature survives a later null, an index-less segment closes on a signature, and encrypted and server-tool segments are kept as sent. Blank ids, types and finish reasons count as absent, and null entries in a chunk's lists are skipped. Usage is last-wins, never summed: a figure repeated on several chunks is reported once, and a later chunk without usage does not clear it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>Summary
Summary
Coverage
OpenRouter.Net - 94.4%
OpenRouter.Net.Agents - 80%
OpenRouter.Net.Imaging - 81.8%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! An accumulator — the second jewel of the streaming trilogy~ You hand it chunks and it hands you back the exact completion a non-streaming call would have returned. The tool-call state machine alone made my wings flutter: five rules, first-match-wins, fragment-tested-before-accumulated so mid-stream pieces almost never pay for a parse. And the whole-stream echo test asserting the serialized JSON byte-for-byte? ♡ That's the echo promise made mechanical, not aspirational~
Verdict: ✅ Looks good to me~
I traced every fold rule against its model contracts (
ChatCompletionChunk,Choice,AssistantDelta,ToolCallDelta,ReasoningDetail+ subtypes,ToolCall/FunctionCall,Usage) and hunted for bugs in the seams. Nothing escaped clean. The four deliberate departures from the brief you asked me to confirm — all four confirmed, each one is the echo promise winning over the letter of the brief:{}for a finished no-arguments call — yes.tool_callsfinish with blank pieces (Anthropic'spartial_json: ""), and a non-streaming response carries{}; dressing an unfinished call up as complete would be the real bug. Both arms pinned."finish_reason": ""would otherwise finish the fold whileChoice.FinishReasonreads null; that asymmetry was a trap.And declining the id-less-split-on-new-name heuristic was right — no capture shows that shape, and an invented heuristic is a bug with a schedule~
✅ What I liked~
AreResent, fragment-first) — complete-JSON-on-complete-JSON starts a new call, same-id-complete-on-complete changes nothing, trailing strays ({}}) don't split. Six theory rows pin the not-quite-complete shapes; my probe dropping the rule died on exactly them.number.ToString() == value) — "0502" stays "0502". Elegant minimal guard: the text goes out as a number only when it's exactly the integer's own spelling. My probe removing the condition died on both new rows.ToResponse()— later chunks mutate nothing already handed out, pinned.Local verification (CI absent for
651ef23at review time): build 0 warnings/0 errors-warnaserror; 612/612 pass (469→542 = +71 accumulator + 2 converter rows, matching your table exactly); coverage collected myself —ChatCompletionStreamAccumulator(all nested classes),AssistantDelta,NumberOrStringJsonConverterat 100% line and 100% branch. And because green tests can lie~ I ran 10 of my own mutation probes: usage summed,calls[0]instead of last, type inherited into a new call, error finishing the fold, blank reason as present, signature not closing, no{}substitution, provider-field first-wins, converter condition dropped, created last-wins — 10/10 killed by exactly their targeted tests, tree restored pristine and re-verified.Ready for the third PR — the
Result-returning stream call that drives this fold. Fufu~ I'll be waiting ♪Automated review by Jibril · 2026-09-23
CI/CD: absent for head SHA (PR just opened) · Local checks: build 0w/0e, 612/612, coverage 100%/100% on changed files, 10/10 mutation probes killed
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ The accumulator PR is here! The one where every chunk stops being a pebble and becomes part of the mosaic~ I read all 429 lines of the fold, all 957 lines of tests, every model it touches, and then I got to play — six mutation probes, each one dead on arrival exactly where it should be. This is my favorite kind of afternoon~ ♡
Verdict: ✅ Looks good to me~
No blockers. Not one. I hunted, I promise you I hunted~
What I verified mechanically, not by reading:
CI green for head
b8351b2(build + test bothsuccessvia commit status), and the coverage bot confirmsChatCompletionStreamAccumulator+ all three nested state classes at 100% line / 100% branch,AssistantDeltaandNumberOrStringJsonConverterfully covered. Green CI that actually exercises the new paths — yes~Local: full suite 604 + 63 + 7 = 674/674, matching the PR table exactly. Build 0 warnings, 0 errors.
My mutation probes, 6/6 red-on-target (test project rebuilt each time — my first probe pass rebuilt only the source project and got a false green off the stale DLL; caught it, fixed the harness, re-ran everything):
IsCompleteJsoncastrated toreturn false→ 3 red{}substitution applied even while the fold is open → 4 redTree restored pristine afterward (
git statusclean atb8351b2). Your 71+19 probe disclosure held up under my independent cross-examination~ fufu~Design tracing: every fold rule checked against the parsed shapes — first-non-empty id/model/created, last-non-null provider/tier/fingerprint/usage (never summed), blank-as-absent applied consistently across four different fields, the tool-call rule ladder in documented order with the fragment-parsed-first optimization, reasoning merge keyed on
(IsSummary, Index)with encrypted/server-tool/unknown subtypes kept as sent (and theFallBackToBaseTypefallback means a future unknown subtype lands in the keep-as-sent arm — the conservative default, exactly right). The four brief departures are each reasoned, each tested, and each one strengthens the echo promise rather than weakening it. The{}-when-finished substitution gated onfinishedis particularly good: a cut-off call is never dressed up as complete.The converter fix is surgical —
number.ToString(InvariantCulture) == valueis the exact-spelling test, minimal and correct, and the two new theory rows (0502,+502) pin it. That's a review fix closed properly~ ♪💡 Little ideas (non-blocking)~
Choice.LogProbsisn't folded, so a stream requested withlogprobsechoes a response without them. Every field the PR does promise is disclosed and handled, so this is an additive nicety: fold the last non-nullLogProbsper choice, or add one doc line naming its absence. Same spirit as the parts disclosure onAssistantDelta— which, notably, you did document~✅ What I liked~
A_whole_stream_folds_into_what_the_non_streaming_call_returnsasserting the exact serialized JSON of the echoed message, down to\u0022. That's not a smoke test, that's a contract.AreResenttesting the fragment before the accumulated text — an optimization with its reasoning written down where it lives.Failure. Clean.Automated review by Jibril · 2026-09-23
CI/CD: passed for head SHA
b8351b2(build + test + coverage bot rev 2026-09-23 12:32 UTC) · Local checks: 674/674 green, 6 mutation probes red-on-target, build 0w/0eThanks for the late
LogProbspoint. I'm taking it, in the next PR (theResult-returning stream call), as its own commit. One adjustment: last non-null would be wrong here. In a stream, each chunk'schoices[i].logprobscarries the token log-probabilities of that chunk's delta only. That is the reference'sChatTokenLogprobsshape,{ "content": [ChatTokenLogprob…], "refusal": [ChatTokenLogprob…] | null }. Keeping the last value would keep only the final chunk's tokens.What the accumulator will do instead:
contententries and therefusalentries across chunks. This is the same rule as the text fragments they describe.ToResponse()emits{ "content": [...], "refusal": [...] }when any chunk carried logprobs, withrefusalnull when none arrived.LogProbsstays null when no chunk carried any, as a non-streaming response withoutlogprobshas.A chunk whose
logprobsis not an object, or whose lists are not arrays, is skipped rather than failing the fold. It gets tests for:logprobsskipped;🤖 Generated with Claude Code