Skip to content

Commit 7452413

Browse files
damianmomotgooglecopybara-github
authored andcommitted
fix: revert "Suppress empty-text-only chunks from streaming responses while preserving carried metadata"
PiperOrigin-RevId: 918876807
1 parent 384a0c5 commit 7452413

2 files changed

Lines changed: 16 additions & 266 deletions

File tree

core/src/main/java/com/google/adk/models/Gemini.java

Lines changed: 16 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,21 @@ public Flowable<LlmResponse> generateContent(LlmRequest llmRequest, boolean stre
226226
() ->
227227
processRawResponses(
228228
Flowable.fromFuture(streamFuture).flatMapIterable(iterable -> iterable)))
229-
.filter(Gemini::shouldEmit);
229+
.filter(
230+
llmResponse ->
231+
llmResponse
232+
.content()
233+
.flatMap(Content::parts)
234+
.map(
235+
parts ->
236+
!parts.isEmpty()
237+
&& parts.stream()
238+
.anyMatch(
239+
p ->
240+
p.functionCall().isPresent()
241+
|| p.functionResponse().isPresent()
242+
|| p.text().isPresent()))
243+
.orElse(false));
230244
} else {
231245
logger.debug("Sending generateContent request to model {}", effectiveModelName);
232246
return Flowable.fromFuture(
@@ -284,28 +298,7 @@ static Flowable<LlmResponse> processRawResponses(Flowable<GenerateContentRespons
284298
responsesToEmit.add(aggregatedTextResponse);
285299
accumulatedText.setLength(0);
286300
}
287-
if (isEmptyTextOnlyResponse(currentProcessedLlmResponse)) {
288-
// Strip the empty-text content while preserving any carried metadata
289-
// (`usageMetadata`, `finishReason`, `modelVersion`, etc.) by emitting a
290-
// content-less response marked as `partial`. This handles the trailing
291-
// `{parts:[{text:""}], finishReason:STOP}` chunk emitted by some Gemini
292-
// preview models (e.g. 3.1-flash-lite) after a function call: keeping
293-
// the chunk as-is would propagate it as a non-partial event whose
294-
// Event#finalResponse() returns true and prematurely terminate
295-
// BaseLlmFlow#run before the function response is sent back to the
296-
// model; dropping it entirely would lose the carried metadata. If the
297-
// chunk carries no useful metadata at all, suppress it outright.
298-
LlmResponse metadataOnly =
299-
currentProcessedLlmResponse.toBuilder()
300-
.content((Content) null)
301-
.partial(true)
302-
.build();
303-
if (hasUsefulMetadata(metadataOnly)) {
304-
responsesToEmit.add(metadataOnly);
305-
}
306-
} else {
307-
responsesToEmit.add(currentProcessedLlmResponse);
308-
}
301+
responsesToEmit.add(currentProcessedLlmResponse);
309302
}
310303
logger.debug("Responses to emit: {}", responsesToEmit);
311304
return Flowable.fromIterable(responsesToEmit);
@@ -365,67 +358,6 @@ private static LlmResponse thinkingResponseFromText(String accumulatedThoughtTex
365358
.build();
366359
}
367360

368-
/**
369-
* Returns true if {@code response} should be emitted downstream by the streaming pipeline.
370-
*
371-
* <p>Drops chunks that carry neither semantic content (i.e. they are an empty-text-only response
372-
* per {@link #isEmptyTextOnlyResponse}) nor any useful metadata (per {@link #hasUsefulMetadata}).
373-
*
374-
* <p>Package-private for testing.
375-
*/
376-
static boolean shouldEmit(LlmResponse response) {
377-
return !isEmptyTextOnlyResponse(response) || hasUsefulMetadata(response);
378-
}
379-
380-
/**
381-
* Returns true if {@code response} carries any non-content metadata that should be propagated
382-
* downstream (e.g. {@code usageMetadata}, {@code finishReason}, transcriptions, grounding or
383-
* error info). Inspects only top-level {@link LlmResponse} fields; the response's content/parts
384-
* are intentionally not considered here.
385-
*/
386-
private static boolean hasUsefulMetadata(LlmResponse response) {
387-
return response.usageMetadata().isPresent()
388-
|| response.finishReason().isPresent()
389-
|| response.errorCode().isPresent()
390-
|| response.groundingMetadata().isPresent()
391-
|| response.inputTranscription().isPresent()
392-
|| response.outputTranscription().isPresent();
393-
}
394-
395-
/**
396-
* Returns true if {@code response} consists of exactly one {@link Part} whose only meaningful
397-
* payload is an empty text string (i.e. {@code parts:[{text:""}]}). Such a chunk can be safely
398-
* dropped from the streaming aggregator because it carries no semantic content for the agent
399-
* pipeline. A part is considered to carry semantic content if any of its non-text payloads
400-
* ({@code functionCall}, {@code functionResponse}, {@code inlineData}, {@code executableCode},
401-
* {@code codeExecutionResult}, {@code fileData}, {@code thoughtSignature}, {@code videoMetadata},
402-
* {@code toolCall}, {@code toolResponse}) is present.
403-
*/
404-
private static boolean isEmptyTextOnlyResponse(LlmResponse response) {
405-
return response
406-
.content()
407-
.flatMap(Content::parts)
408-
.map(
409-
parts -> {
410-
if (parts.size() != 1) {
411-
return false;
412-
}
413-
Part part = parts.get(0);
414-
return part.text().map(String::isEmpty).orElse(false)
415-
&& part.functionCall().isEmpty()
416-
&& part.functionResponse().isEmpty()
417-
&& part.inlineData().isEmpty()
418-
&& part.executableCode().isEmpty()
419-
&& part.codeExecutionResult().isEmpty()
420-
&& part.fileData().isEmpty()
421-
&& part.thoughtSignature().isEmpty()
422-
&& part.videoMetadata().isEmpty()
423-
&& part.toolCall().isEmpty()
424-
&& part.toolResponse().isEmpty();
425-
})
426-
.orElse(false);
427-
}
428-
429361
@Override
430362
public BaseLlmConnection connect(LlmRequest llmRequest) {
431363
if (!apiClient.vertexAI()) {

core/src/test/java/com/google/adk/models/GeminiTest.java

Lines changed: 0 additions & 182 deletions
Original file line numberDiff line numberDiff line change
@@ -63,81 +63,6 @@ public void processRawResponses_withTextChunks_emitsPartialResponses() {
6363
isFunctionCallResponse());
6464
}
6565

66-
// Regression test for b/513501918. gemini-3.1-flash-lite emits an extra trailing chunk after a
67-
// function call: `{parts:[{text:""}], finishReason:STOP}`. That chunk must not be propagated as
68-
// a non-partial event because BaseLlmFlow#run would treat it as the final response and
69-
// terminate the loop before the function response is sent back to the model. The chunk's
70-
// metadata (e.g. `finishReason`, `usageMetadata`) is preserved by emitting it on a content-less
71-
// partial response instead of dropping the chunk entirely.
72-
@Test
73-
public void
74-
processRawResponses_functionCallThenEmptyTextWithStop_emitsFunctionCallAndMetadataOnlyPartial() {
75-
Flowable<GenerateContentResponse> rawResponses =
76-
Flowable.just(
77-
toResponse(Part.fromFunctionCall("test_function", ImmutableMap.of())),
78-
toResponseWithText("", FinishReason.Known.STOP));
79-
80-
Flowable<LlmResponse> llmResponses = Gemini.processRawResponses(rawResponses);
81-
82-
assertLlmResponses(
83-
llmResponses,
84-
isFunctionCallResponse(),
85-
isContentlessPartialWithFinishReason(FinishReason.Known.STOP));
86-
}
87-
88-
// Same as above but with `usageMetadata` on the trailing empty chunk: the metadata must survive
89-
// on the emitted content-less partial.
90-
@Test
91-
public void
92-
processRawResponses_functionCallThenEmptyTextWithUsageMetadata_preservesUsageMetadata() {
93-
GenerateContentResponseUsageMetadata metadata = createUsageMetadata(5, 10, 15);
94-
Flowable<GenerateContentResponse> rawResponses =
95-
Flowable.just(
96-
toResponse(Part.fromFunctionCall("test_function", ImmutableMap.of())),
97-
toResponseWithText("", FinishReason.Known.STOP, metadata));
98-
99-
Flowable<LlmResponse> llmResponses = Gemini.processRawResponses(rawResponses);
100-
101-
assertLlmResponses(
102-
llmResponses, isFunctionCallResponse(), isContentlessPartialWithUsageMetadata(metadata));
103-
}
104-
105-
// Same as above but without a finishReason or usageMetadata: the trailing empty chunk carries no
106-
// useful payload and must be suppressed entirely.
107-
@Test
108-
public void processRawResponses_functionCallThenEmptyText_doesNotEmitExtraEmptyResponse() {
109-
Flowable<GenerateContentResponse> rawResponses =
110-
Flowable.just(
111-
toResponse(Part.fromFunctionCall("test_function", ImmutableMap.of())),
112-
toResponseWithText(""));
113-
114-
Flowable<LlmResponse> llmResponses = Gemini.processRawResponses(rawResponses);
115-
116-
assertLlmResponses(llmResponses, isFunctionCallResponse());
117-
}
118-
119-
// Combined scenario: leading partial text, then a function call, then the trailing empty-text
120-
// chunk with STOP. Accumulated text must still be flushed, the function call must still be
121-
// emitted, and the trailing chunk must surface only its metadata on a content-less partial.
122-
@Test
123-
public void
124-
processRawResponses_textThenFunctionCallThenEmptyTextWithStop_emitsTextFunctionCallAndMetadata() {
125-
Flowable<GenerateContentResponse> rawResponses =
126-
Flowable.just(
127-
toResponseWithText("Thinking..."),
128-
toResponse(Part.fromFunctionCall("test_function", ImmutableMap.of())),
129-
toResponseWithText("", FinishReason.Known.STOP));
130-
131-
Flowable<LlmResponse> llmResponses = Gemini.processRawResponses(rawResponses);
132-
133-
assertLlmResponses(
134-
llmResponses,
135-
isPartialTextResponse("Thinking..."),
136-
isFinalTextResponse("Thinking..."),
137-
isFunctionCallResponse(),
138-
isContentlessPartialWithFinishReason(FinishReason.Known.STOP));
139-
}
140-
14166
@Test
14267
public void processRawResponses_textAndStopReason_emitsPartialThenFinalText() {
14368
Flowable<GenerateContentResponse> rawResponses =
@@ -250,93 +175,6 @@ public void processRawResponses_thoughtChunksAndStop_includeUsageMetadata() {
250175
isFinalThoughtResponseWithUsageMetadata("Thinking deeply", metadata2));
251176
}
252177

253-
// Test cases for the shouldEmit filter applied by generateContent after processRawResponses.
254-
// shouldEmit drops chunks that are empty-text-only AND carry no useful metadata; everything else
255-
// is forwarded. processRawResponses normally already strips empty-text-only chunks, so shouldEmit
256-
// is defense-in-depth, but it must still behave correctly when fed any LlmResponse directly.
257-
258-
@Test
259-
public void shouldEmit_emptyTextOnlyResponseWithNoMetadata_returnsFalse() {
260-
LlmResponse response =
261-
LlmResponse.builder()
262-
.content(Content.builder().role("model").parts(Part.fromText("")).build())
263-
.build();
264-
265-
assertThat(Gemini.shouldEmit(response)).isFalse();
266-
}
267-
268-
@Test
269-
public void shouldEmit_emptyTextOnlyResponseWithFinishReason_returnsTrue() {
270-
LlmResponse response =
271-
LlmResponse.builder()
272-
.content(Content.builder().role("model").parts(Part.fromText("")).build())
273-
.finishReason(new FinishReason(FinishReason.Known.STOP))
274-
.build();
275-
276-
assertThat(Gemini.shouldEmit(response)).isTrue();
277-
}
278-
279-
@Test
280-
public void shouldEmit_emptyTextOnlyResponseWithUsageMetadata_returnsTrue() {
281-
LlmResponse response =
282-
LlmResponse.builder()
283-
.content(Content.builder().role("model").parts(Part.fromText("")).build())
284-
.usageMetadata(createUsageMetadata(5, 10, 15))
285-
.build();
286-
287-
assertThat(Gemini.shouldEmit(response)).isTrue();
288-
}
289-
290-
@Test
291-
public void shouldEmit_nonEmptyTextResponse_returnsTrue() {
292-
LlmResponse response =
293-
LlmResponse.builder()
294-
.content(Content.builder().role("model").parts(Part.fromText("hello")).build())
295-
.build();
296-
297-
assertThat(Gemini.shouldEmit(response)).isTrue();
298-
}
299-
300-
@Test
301-
public void shouldEmit_functionCallResponse_returnsTrue() {
302-
LlmResponse response =
303-
LlmResponse.builder()
304-
.content(
305-
Content.builder()
306-
.role("model")
307-
.parts(Part.fromFunctionCall("test_function", ImmutableMap.of()))
308-
.build())
309-
.build();
310-
311-
assertThat(Gemini.shouldEmit(response)).isTrue();
312-
}
313-
314-
@Test
315-
public void shouldEmit_contentlessResponse_returnsTrue() {
316-
// A response with no content at all is not an empty-text-only response, so it should pass
317-
// through regardless of metadata. This is the shape emitted by processRawResponses after it
318-
// strips empty-text content while preserving metadata.
319-
LlmResponse response = LlmResponse.builder().build();
320-
321-
assertThat(Gemini.shouldEmit(response)).isTrue();
322-
}
323-
324-
@Test
325-
public void shouldEmit_multiPartResponseWithEmptyTextPart_returnsTrue() {
326-
// Only single-part empty-text responses are considered "empty-text-only". A multi-part response
327-
// is treated as carrying semantic content and must always pass through.
328-
LlmResponse response =
329-
LlmResponse.builder()
330-
.content(
331-
Content.builder()
332-
.role("model")
333-
.parts(Part.fromText(""), Part.fromText("hello"))
334-
.build())
335-
.build();
336-
337-
assertThat(Gemini.shouldEmit(response)).isTrue();
338-
}
339-
340178
@Test
341179
public void processRawResponses_thoughtAndTextWithStop_onlyFinalTextIncludesUsageMetadata() {
342180
GenerateContentResponseUsageMetadata metadata1 = createUsageMetadata(5, 5, 10);
@@ -394,26 +232,6 @@ private static Predicate<LlmResponse> isFunctionCallResponse() {
394232
};
395233
}
396234

397-
private static Predicate<LlmResponse> isContentlessPartialWithFinishReason(
398-
FinishReason.Known expectedFinishReason) {
399-
return response -> {
400-
assertThat(response.partial()).hasValue(true);
401-
assertThat(response.content()).isEmpty();
402-
assertThat(response.finishReason().map(fr -> fr.knownEnum())).hasValue(expectedFinishReason);
403-
return true;
404-
};
405-
}
406-
407-
private static Predicate<LlmResponse> isContentlessPartialWithUsageMetadata(
408-
GenerateContentResponseUsageMetadata expectedMetadata) {
409-
return response -> {
410-
assertThat(response.partial()).hasValue(true);
411-
assertThat(response.content()).isEmpty();
412-
assertThat(response.usageMetadata()).hasValue(expectedMetadata);
413-
return true;
414-
};
415-
}
416-
417235
private static Predicate<LlmResponse> isEmptyResponse() {
418236
return response -> {
419237
assertThat(response.partial()).isEmpty();

0 commit comments

Comments
 (0)