Repository navigation
Conversation
|
Hi @innoprej, Thank you for your contribution and for taking the time to submit this pull request. Our team is currently reviewing your changes, and we will reach out if we need any further information. |
kvmilos
left a comment
There was a problem hiding this comment.
Please rebase onto main and move your message list into the reworked FakeTransport so CI can run. GeminiLlmConnectionTest now has LiveConnectionFixture, and FakeTransport was updated to record sends.
Before we merge, could you run a live session through Runner.runLive against a Live model and post what ends up in the session? I'd like to verify that it streams the partial chunks and stores one full-text event per turn. If you can't run it, let us know and we will.
Could you change Closes: #1648 to Part of #1648, or open a follow-up issue for transcription aggregation? #1648 also covers aggregating transcription chunks, which this PR leaves for later, and we don't want that part lost when the issue closes.
| if (streamedParts.size() == parts.size()) { | ||
| serverContent.modelTurn().ifPresent(builder::content); | ||
| } else if (!streamedParts.isEmpty()) { | ||
| builder.content(serverContent.modelTurn().get().toBuilder().parts(streamedParts).build()); |
There was a problem hiding this comment.
Please add a test where a single message has both text and a non-text part, such as [text, executableCode]. No test currently hits this else if branch where only part of the message's parts are kept.
There was a problem hiding this comment.
Added convertToServerResponse_textAndOtherPartInOneMessage_keepsOtherPartAfterFullText for [text, executableCode]: the text comes out in full, then a response with only the executableCode part and the message's role. convertToServerResponse_textAndInlineDataWithTurnComplete_keepsInlineData covers the same branch when the turn ends, with [text, inlineData]. With the else if disabled, these two tests fail and the other tests in the class pass.
| boolean turnComplete = serverContent.turnComplete().orElse(false); | ||
| if (turnComplete || serverContent.interrupted().orElse(false)) { | ||
| streamedParts.removeIf(GeminiLlmConnection::hasText); | ||
| // Unlike in ADK Python, the full text does not carry interrupted; the response below does. |
There was a problem hiding this comment.
Would you match ADK Python here by putting interrupted on the full-text response and dropping the separate marker? I saw the PR description notes you kept interrupted on the message's own response where callers read it today, but matching Python keeps the emitted events consistent across ports.
There was a problem hiding this comment.
Done. When text is pending, the full-text response now carries interrupted, and no response is emitted only to report it again. If the turn also completes or the message has other content such as audio, that response reports it too, as in ADK Python. A transcription that follows the full text gets a response without the flag. With no pending text, the message's own response reports it as before; ADK Python sends a separate response when that message also has audio or a transcription. The remaining differences are listed in the PR description.
In this repo, BaseLlmFlow, SessionJsonConverter, the tokt codecs and the dev UI read the field and none of them relies on a separate event. For external callers, who may have looked for it only on an event without content, I added a note to the PR description.
|
|
||
| private static LlmResponse createServerContentResponse(LiveServerContent serverContent) { | ||
| /** | ||
| * Emits the responses for a server content message. As in ADK Python, text parts are streamed in |
There was a problem hiding this comment.
Nit: please drop the "As in ADK Python" / "Unlike in ADK Python" phrasing from the code and test comments and just state the rule directly. The parity notes in the PR description are enough.
There was a problem hiding this comment.
Done. The code and test comments state the rules directly now, and the parity notes are only in the PR description.
GeminiLlmConnection turned each live server message into responses without aggregating text, and marked a response partial only when its message carried turnComplete: false. Runner.runLive passed every event to appendEvent, while runAsync skips partial events. A live turn's text was therefore stored as one model event per chunk, or, with partial chunks, dropped by InMemorySessionService and stored chunk by chunk by FirestoreSessionService and VertexAiSessionService. Follow ADK Python's live connection and runner: - Only responses that stream text are partial. The connection also collects the text and emits it as one non-partial response before the end of the turn, an interruption or a tool call, and when thought and other text alternate or a part that is neither text nor inline data arrives. Text that arrives with the end of the turn or an interruption is emitted only in full. - The full response reports an interruption that ends the text, and no other response is emitted just to report it. - runLive skips partial events when persisting, as runAsync does. Responses without text are no longer partial when their message carries turnComplete: false. Transcriptions are not aggregated yet.
ca73bf4 to
9435377
Compare
|
Thanks for the review. Updated:
I also ran I couldn't get model text in
With
In this session, no event carried only the interruption. The session ended with 714 stored events and no partial ones, while 15 partial events were streamed. Turn 4 has no full thought because I closed the connection before it ended, which is the pending-text case noted in the PR description. On Vertex AI, in one What this doesn't show:
To cover plain text without a model, I ran the same path with an in-process
Two other observations:
|
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
In live mode, what a turn leaves in the session depends on the session service.
GeminiLlmConnectionturns each server message into responses without aggregating text, and marks a response partial only when its message carriesturnComplete: false.Runner.runLivepasses every event toappendEvent, whilerunAsyncskips partial events. So a turn's text is stored as one model event per chunk, or, when the chunks are partial, dropped byInMemorySessionServiceand stored chunk by chunk byFirestoreSessionServiceandVertexAiSessionService. ADK Python stores it once. #1648 has the details and a reproduction.Solution:
Follow ADK Python's live connection (
gemini_llm_connection.py) and runner (runners.py):GeminiLlmConnection, only the responses that stream text are partial. The connection also collects that text in aPendingTextand emits it as one non-partial response before the response that completes the turn or reports an interruption (:550-559,:600-610), before a tool call (:619-627), and when thought and non-thought text alternate or a part that is neither text nor inline data arrives. Text that arrives in the same message as the end of the turn or an interruption is emitted only in that full response, as ADK Python does.interrupted. No response is emitted only to report the interruption again.turnCompleteand the transcriptions stay on the message's own response. That response also reports the interruption if the turn completes or the message has other content, such as audio, and it reports the interruption when no text is pending. When a message streams text and also carries a transcription, the transcription gets a non-partial response of its own, so that it is still stored.Runner.runLiveskips partial events when persisting, asrunAsyncdoes.mainHelandlowithoutturnComplete, thenturnComplete: trueHel,lo, turn completeHel, partiallo,Hello, turn completeturnComplete: falseon the chunksHel, partiallo, turn completeHel, partiallo,Hello, turn completeHel, thenlowithturnComplete: trueHel,lowith turn completeHel,Hello, turn completeHel, theninterrupted: trueHel, interruptedHel,Helwith interruptedChecking, then a tool callChecking, tool callChecking,Checking, tool callrunLivenow hands only the non-partial events to the session service. Since every text chunk is partial, a turn that completes, is interrupted or calls a tool stores its text once, whether or not the Live API sendsturnComplete: falseon the intermediate messages, the question #1648 left open.Responses that do not stream text, such as audio, transcriptions and interruptions, are no longer partial; on
mainthey were partial whenever their message carriedturnComplete: false. If the Live API sends that flag,InMemorySessionServicenow stores them, as the other session services already did; otherwise nothing changes for them. ADK Python does not mark audio or interruptions partial either. Four differences from ADK Python remain:mainwhen the messages do not carryturnComplete: false. When they do,InMemorySessionServicenow stores it too, and the stored audio is sent back with the history of the next live connection. ADK Python keeps live model media with inline data out of the session in its runner (_should_append_event) and out of the history it sends (send_history). runLive passes partial events to appendEvent and never aggregates text chunks, unlike ADK Python #1648 lists this as a separate difference, and this PR adds neither check.Contentsmerges adjacent ones when it replays them as history. ADK Python streams the chunks as partial responses and stores one finished transcription. I can send that separately.turnComplete, orinterruptedwhen no text is pending.[text, executableCode], the other part's response is not partial here, so it is stored. In ADK Python that response keepspartial: true.Testing Plan
Unit Tests:
./mvnw -pl core clean testwith this change on top of1f1537dc(Windows 11, Microsoft Build of OpenJDK 17.0.19): thedefault-testandbasicexecutions each run 3,315 tests with no failures (30 skipped), and the other five executions pass.mainGeminiLlmConnectionTest.receive_textChunks_streamedAsPartialAndEmittedInFullAtTurnComplete:FakeTransportdeliversHelwithoutturnComplete,lowithturnComplete: false, and an emptyturnComplete: truemain's connection, it gets[text:Hel, partial text:lo, turnComplete]and fails[partial text:Hel, partial text:lo, text:Hello, turnComplete]RunnerTest.runLive_partialEvent_streamedButNotPassedToSessionService, the live counterpart ofrunAsync_partialEvent_streamedButNotPassedToSessionServiceappendEventreceives[partial, final][final]GeminiLlmConnectionTest.convertToServerResponse_*tests for the rules above: text that arrives with the end of the turn, a tool call, a thought switch, inline data (not partial, and does not end the text), another part (ends the text), text with a transcription, an empty text part, and text and another part in one message ([text, executableCode], and[text, inlineData]with the end of the turn)convertToServerResponsehas noPendingTextparameter onmainGeminiLlmConnectionTest.convertToServerResponse_*tests for interruptions: pending text, text that arrives with the interruption, and an interruption with the end of the turn, with inline data, with a transcription, and with no textconvertToServerResponsehas noPendingTextparameter onmainThree existing
GeminiLlmConnectionTesttests now expect the full-text response before the message's own response:withoutInterruptedField_mapsEmptyOptional,withTurnCompleteTrue_mapsPartialFalseandwithContentAndUsageMetadata_emitsMultiple.withInterruptedTrue_mapsInterruptedFieldnow expects one non-partial response with the text andinterrupted. The otherconvertToServerResponsetests only pass a newPendingText.Manual End-to-End (E2E) Tests:
Ran live sessions through
Runner.runLivewithInMemorySessionServiceon the Gemini API and Vertex AI; the results are in the PR conversation. No Live model I could reach returned text inmodelTurn, so the text that went through the connection was the thoughts ofgemini-2.5-flash-native-audio-preview-12-2025: their chunks were streamed as partial events, and the thoughts of each finished turn were stored as one event. Model text without the thought flag was not run live. A run over an in-processGeminiLiveTransportcovered it throughRunner.runLive, and the connection tests cover it with the realGeminiLlmConnectionand its conversion, and the Runner test usesTestLlm's live connection.Checklist
Additional context
runLivenow receive a turn's text twice, as partial chunks and then as one non-partial event, as ADK Python's callers do. A caller that shows the text of every model event needs to skip the partial events or the full one.interruptedis on the event with the full text. A caller that looks for it only on events without content needs to read it there as well.[executableCode, text]or[text, inlineData]in a message that does not end the turn or report an interruption, goes out on the partial response and is not stored, as in ADK Python. Onmain, such a message was stored unless it setturnComplete: false.main, its chunks were stored when they were not partial.FirestoreSessionService.appendEventstill has no partial check of its own, the smaller alignment mentioned in runLive passes partial events to appendEvent and never aggregates text chunks, unlike ADK Python #1648. With this change, neitherrunAsyncnorrunLivepasses it partial events.