feat(#3996): stream model-generated media deltas - #4023
Conversation
fc76f6a to
79f0624
Compare
79f0624 to
99aa451
Compare
99aa451 to
1d9a86b
Compare
1d9a86b to
1d2122c
Compare
199777b to
1d2122c
Compare
Extend the shared chat.MessageDelta streaming representation with typed MediaDelta entries (bytes + MIME + name + size) so generated binary media has a provider-agnostic path instead of a Gemini-only side channel. The Gemini adapter captures inline image blobs from response candidates the same way it already captures text/reasoning/tool calls/usage — retaining every blob in a chunk, not just the last — and the run() content gate forwards image-only chunks that previously would have been dropped. The runtime's stream accumulator (pkg/runtime/streaming.go) collects the deltas into streamResult.Media, accumulating before any terminal check so a final chunk that carries both media and a finish reason is never lost. Persistence of the accumulated media ships in the follow-up materialization commits.
1d2122c to
b78c6e1
Compare
aheritier
left a comment
There was a problem hiding this comment.
Review of b78c6e1f (own delta vs gemini-gateway-transport). CI green for this head (10 ✓ / 3 skipped / 0 ✗). Locally: go build ./..., go test -race on pkg/chat, pkg/model/provider/gemini, pkg/runtime/..., golangci-lint all clean; mutation checks confirm the new adapter and handleStream tests are load-bearing (incl. the accumulate-before-terminal-check ordering). Draft PR → comment only.
The streaming mechanics look right: Recv collects every InlineData blob per chunk, the run() gate forwards image-only chunks, and the done branch never re-walks parts so media isn't duplicated; handleStream accumulates before the stop/length/refusal early return. No new event type, so TUI/CLI/JSON/A2A/MCP/API consumers are untouched; other providers leave Media nil and all reads are len()-guarded.
[blocking] Doc comments describe behavior that doesn't exist at this commit
pkg/chat/media.go:9-12links[DocumentSource.ArtifactPath], which doesn't exist here (DocumentSourcehas onlyInlineText/InlineData;ArtifactPathlands in a later PR), and says "Data is never written to session storage" — the approved DB-backed blob design persists these bytes tosession.db.pkg/runtime/streaming.go:216-220says the runtime "materializes the final bytes … (see recordAssistantMessage)". At this headrecordAssistantMessage(loop.go:1173) never readsres.Mediaand skips empty-content turns, andloop.go:875-897warns "returned an empty response" — so an image-only Gemini reply is warned-about and discarded until the follow-up lands.streamResult.Mediahas no reader yet.
Suggest rewording both to forward-looking ("accumulated here; materialization ships in the follow-up") and dropping the dangling godoc link / the never-persisted claim.
[should-fix] PR body / test instructions promise "deltas arrive in order as they are received … without waiting for the complete response", but streaming.go:220 says "Not emitted as a streaming event" and nothing consumes the media at this commit. The only observable here is the unit tests; end-to-end observation needs the materialization PR.
[should-fix] TestHandleStream_MediaOnlyTurnNotTreatedAsEmpty claims a media-only bare-EOF turn is "not misclassified as the 'no output' case", but only asserts Stopped (already true for any no-tool-call turn). The same stream yields FinishReason == "null" because streaming.go:401-405 infers stop only from text length, which loop.go:875-897 then treats as an empty turn. Either extend inference (len(media) > 0 → stop) and assert it, or reword the test. (Masked for real Gemini: hasContent now includes media, so the synthesized done chunk carries stop.)
[optional] No per-turn bound on accumulated media in handleStream — consistent with the plan placing the 20 MiB cap in the persistence layer; noting only.
What
Streams model-generated media deltas through Gemini adaptation, runtime streaming, chat media types, and tests.
Why
Generated media should become available incrementally rather than waiting for the entire response.
Validation
task test; Gemini adapter and runtime streaming tests.Test instructions
Run:
task testInvoke a media-generating model and observe the chat stream. Expected: generated-media deltas arrive in order as they are received, are represented by the chat stream, and do not require waiting for the complete response.