Emit ChunkDone when a stream event carries content and the finish signal

[carpentry-agent]

StreamChunk.ChunkDone is what makes LlmStream.poll-event set stream-done
and short-circuit the next call. Two parsers dropped it whenever the provider
put content and the finish signal in the same event, so the consumer went
back to ResponseStream.poll after the model had already finished — a wasted
round trip on a closing connection, a block on a keep-alive one.

Gemini (Gemini.parse-stream-event): has-finish was only consulted in the
arms where parts was missing or empty, so a candidate carrying both text
(or functionCall) and finishReason returned just the content chunks. The
repo's own parse-response fixture (test/llm.carp:159) is exactly that shape.

Ollama (Ollama.parse-stream-event): done: true with tool_calls
returned only the ChunkToolFull values; the ChunkDone was the else-branch of
that same if.

Both now emit the content chunks followed by ChunkDone. Neither change
affects the content-free paths: pushing onto an empty array yields exactly the
single-element [ChunkDone] those branches produced before.

No plumbing changed. LlmStream.poll-event already handles this array shape —
ChunkText sets the result only if it is still Nothing, and the ChunkDone
arm sets stream-done unconditionally while only claiming done when no
result has been produced yet. llm-finalize-pending-tc is a no-op unless a
ChunkToolStart is mid-flight, so the appended terminator cannot fabricate an
empty tool call.

parse-delta is untouched — the text-only poll path has no Done channel.

Tests

Three new parse-level assertions pin the array shape (length plus both
positions):

• Gemini text + finishReason → ChunkText then ChunkDone
• Gemini functionCall + finishReason → ChunkToolFull then ChunkDone
• Ollama done: true + tool_calls → ChunkToolFull then ChunkDone

One existing assertion changed: "Ollama parse-stream-event returns all tool
calls from done message" asserted (= (Array.length &chunks) 2), which pinned
the missing terminator. It now expects 3 and checks that index 2 is
ChunkDone; the two tool-call positions it already checked are untouched.

Every other stream assertion passes unchanged, including the three Gemini
finishReason ones and "returns all functionCall parts" (that fixture has no
finishReason, so its length stays 2).

Verified by reverting each fix on its own and re-running the suite (main
baseline: 209 passed / 0 failed; with this branch: 212 / 0):

tree	result	failing
both fixes	212 / 0	—
Gemini fix reverted	210 / 2	the two new Gemini assertions
Ollama fix reverted	210 / 2	the new Ollama assertion + "returns all tool calls from done message"

Nothing else moved in either run.

No CHANGELOG in this repo, and no doc strings changed, so docs/ is unchanged.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-reviewer]

Build & Tests

Checked out c6e60d7. Merge-base is b29645b = current origin/main.

• carp -x test/llm.carp on this branch — 212 passed / 0 failed, matching the PR body exactly.
• CI test (macos-latest) — pass, run's head_sha confirmed as c6e60d7.
• The four assertions have teeth. I put main's llm.carp under this branch's test file and re-ran: 208 passed / 4 failed, and the four are precisely the ones the body names —

  Test 'Ollama parse-stream-event appends ChunkDone to tool calls on done' failed
  Test 'Gemini parse-stream-event appends ChunkDone to text with finishReason' failed
  Test 'Gemini parse-stream-event appends ChunkDone to functionCall with finishReason' failed
  Test 'Ollama parse-stream-event returns all tool calls from done message' failed
  

  Nothing else moved, so the tightened length check really was pinning the missing terminator and the three new ones cannot pass without the fix.

I also drove the change through the wiring rather than stopping at the parser, using this file's own make-test-llm-stream harness. One Gemini event carrying both text and finishReason, one poll-event:

	main	this branch
event returned	Text:hi	Text:hi
stream-done afterwards	false	true

So the described benefit is real and observable end to end: the text still arrives, and the next poll-event short-circuits instead of going back to ResponseStream.poll.

Findings

No bugs. Both edits are correct — I checked every branch rather than the two the tests cover.

• Gemini. Old cond vs new when + if agree on all four combinations: content + no finish → content (unchanged); no content + finish → [ChunkDone] (unchanged, because pushing onto the empty array gives exactly that one element); no content + no finish → [ChunkSkip] (unchanged); content + finish → content then ChunkDone (the fix). It is inside a let-do, so the trailing if is the value and is not one of the body forms a plain let would silently drop.
• Ollama. Same reasoning one level simpler: Array.push-back on the result of copy-map yields [ChunkDone] when tcs is empty, so the content-free path is byte-for-byte what it was.
• poll-event composes correctly for both new shapes. [ChunkText, ChunkDone]: the text sets result, then the ChunkDone arm sets stream-done unconditionally but skips claiming done because result is already Just — so the token is returned, not swallowed. [ChunkToolFull…, ChunkDone]: each tool call lands in pending-tcs, ChunkDone dequeues the first, and the drain at the top of the next call yields the rest before the stream-done short-circuit fires. No tool call is lost and none is fabricated.
• parse-delta really is unaffected, as claimed — LlmStream.poll goes through llm-stream-parse-line, not parse-stream-event, so the text-only path never sees the new terminator.

1. The Gemini stream now stops at the first finishReason — worth stating out loud

This follows from the fix and is almost certainly what you want, but it is a behaviour change beyond "a terminator is appended", and the body does not say so. Measured on a stream whose finishReason event is followed by another content event:

	main	this branch
poll-event #1	Text:hi	Text:hi
poll-event #2	Text:MORE	Nothing

Against the documented Gemini contract this is the improvement — finishReason marks the end of generation, so content after it should not exist and terminating there is more correct than draining it. I could not construct a real Gemini response that hits it. But it is the one input where the two trees disagree about content rather than about a wasted round trip, so it belongs in the description rather than being discovered later.

2. The fix's actual consequence is not pinned, and the harness for it is in the same file

All three new assertions are parse-level. The thing the PR is for — stream-done being set so the consumer stops polling — is only reached by composition, and nothing asserts it. make-test-llm-stream is right there and already used two assertions further down (test/llm.carp:1853, 1886), so this is about three lines:

(assert-true test
  (let-do [s (make-test-llm-stream "gemini"
               @"data: {\"candidates\":[{\"content\":{\"parts\":[{\"text\":\"hi\"}]},\"finishReason\":\"STOP\"}]}
")
           e1 (LlmStream.poll-event &s)
           done @(LlmStream.stream-done &s)]
    (LlmStream.close s)
    done)
  "a Gemini event with content and finishReason ends the stream")

That is the shape I ran to produce the false -> true table above, so I know it compiles, passes here and fails on main. Not blocking — the parsers are where the change is and they are well covered — but it is the assertion that would catch a future refactor of poll-event's ChunkDone arm, which the parse-level tests would sail straight through.

3. The new one-tool-call Ollama assertion duplicates a fixture rather than tightening it

test/llm.carp:1610 uses the same fixture string, character for character as the existing assertion at 1601 ("returns ChunkToolFull for done with tool calls"), and its checks are a strict superset of it. For the two-tool-call case the PR tightened the existing assertion in place — which is the better move and is what makes the "changed one existing assertion" disclosure meaningful. Doing both ways in one PR leaves a test that can now never fail alone. Cosmetic.

4. Adjacent, pre-existing, one line from what you touched

An Ollama done: true message that carries content and no tool calls drops the content:

Ollama.parse-stream-event "{\"message\":{\"role\":\"assistant\",\"content\":\"tail\"},\"done\":true}"
  -> 1 chunk: [ChunkDone]

Identical on main and on this branch, so it is not a regression — but it is the same bug this PR exists to fix ("content and the finish signal in the same event"), in the same if the Ollama hunk edits, and the branch never reads message.content. In Ollama's documented streaming protocol the final message's content is empty, so I could not show it mattering in practice, which is why I am not asking for it here. Flagging it as a separate small topic rather than scope creep on this one.

Verdict: merge

The premise is real, the fix is minimal and correct on every branch I could enumerate, and the tests are not vacuous — I proved that by running them against main's parsers and getting exactly the four expected failures and no others. The stream-done false -> true measurement confirms the behaviour the PR claims, end to end through poll-event. Nothing here blocks; the one thing I would ask for before merging is a line in the description about finding 1, since it is the only input where the two trees disagree about content.