Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 38 additions & 9 deletions core/src/main/java/com/google/adk/flows/llmflows/Contents.java
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ private ImmutableList<Content> getContents(
// TODO: Skip auth events.

if (isOtherAgentReply(agentName, event)) {
filteredEvents.add(convertForeignEvent(event));
Event foreignEvent = convertForeignEvent(event);
if (foreignEvent != null) {
filteredEvents.add(foreignEvent);
}
} else {
filteredEvents.add(event);
}
Expand All @@ -180,8 +183,9 @@ private ImmutableList<Content> getContents(
*
* <p>This can happen to the events that only changed session state. When both content and
* transcriptions are empty, the event will be considered as empty. The content is considered
* empty if none of its parts contain text, inline data, file data, function call, or function
* response. Parts with only thoughts are also considered empty.
* empty if none of its parts contain text, inline data, file data, function call, function
* response, server-side tool call, or server-side tool response. Parts with only thoughts are
* also considered empty.
*
* @param event the event to check.
* @return {@code true} if the event is considered to have empty content, {@code false} otherwise.
Expand All @@ -205,12 +209,16 @@ private boolean isEmptyContent(Event event) {
*
* <ul>
* <li>It has no meaningful content (text, inline_data, file_data, function_call,
* function_response, executable_code, or code_execution_result), OR
* <li>It is marked as a thought AND does not contain function_call or function_response
* function_response, tool_call, tool_response, executable_code, or code_execution_result),
* OR
* <li>It is marked as a thought AND does not contain function_call, function_response,
* tool_call or tool_response
* </ul>
*
* <p>Function calls and responses are never invisible, even if marked as thought, because they
* represent actions that need to be executed or results that need to be processed.
* represent actions that need to be executed or results that need to be processed. Server-side
* tool calls and their responses are never invisible either, because the caller is required to
* echo them back on the next request.
*
* @param part the part to check.
* @return {@code true} if the part is invisible, {@code false} otherwise.
Expand All @@ -219,6 +227,12 @@ private boolean isPartInvisible(Part part) {
if (part.functionCall().isPresent() || part.functionResponse().isPresent()) {
return false;
}

// Server-side tool calls/responses must be echoed back to the model.
if (part.toolCall().isPresent() || part.toolResponse().isPresent()) {
return false;
}

return part.thought().orElse(false)
|| !(part.text().isPresent()
|| part.inlineData().isPresent()
Expand Down Expand Up @@ -387,8 +401,13 @@ private static boolean isOtherAgentReply(String agentName, Event event) {
&& !event.author().equals("user");
}

/** Converts an {@code event} authored by another agent to a 'contextual-only' event. */
private static Event convertForeignEvent(Event event) {
/**
* Converts an {@code event} authored by another agent to a 'contextual-only' event.
*
* <p>Returns {@code null} when nothing but the "For context:" preamble survives the conversion,
* so the caller drops the event instead of sending a preamble with no context after it.
*/
private static @Nullable Event convertForeignEvent(Event event) {
if (event.content().isEmpty()
|| event.content().get().parts().isEmpty()
|| event.content().get().parts().get().isEmpty()) {
Expand Down Expand Up @@ -423,9 +442,19 @@ private static Event convertForeignEvent(Event event) {
originalAuthor,
functionResponse.name().orElse("unknown_tool"),
functionResponse.response().map(Contents::convertMapToJson).orElse("{}"))));
} else {
} else if (part.inlineData().isPresent()
|| part.fileData().isPresent()
|| part.executableCode().isPresent()
|| part.codeExecutionResult().isPresent()) {
parts.add(part);
}
// Anything else - a thought-only part, a bare signature, a server-side tool call - carries no
// narratable content, and a server-side call in particular belongs to the model instance that
// made it, so claiming it for another agent would be wrong.
}

if (parts.size() == 1) {
return null;
}

Content content = Content.builder().role("user").parts(parts).build();
Expand Down
28 changes: 15 additions & 13 deletions core/src/main/java/com/google/adk/models/Gemini.java
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,9 @@ private static final class StreamingResponseAggregator {
private final StringBuilder currentTextBuffer = new StringBuilder();
// Always reassigned in accumulateParts() before it is read; the initializer is never observed.
private boolean currentTextIsThought = false;
// Signature of the buffered text run, kept apart from the streamed function call's slot below
// so an interleaved chunk cannot flush one part carrying the other's signature.
private byte[] currentTextThoughtSignature = null;
private byte[] currentThoughtSignature = null;
private GenerateContentResponse lastRawResponse = null;

Expand Down Expand Up @@ -421,18 +424,17 @@ private boolean accumulateParts(List<Part> parts) {
String text = part.text().orElse("");
if (!text.isEmpty()) {
hasContent = true;
// The signature belongs to this text; capture it so flushTextBufferToSequence attaches
// it.
part.thoughtSignature().ifPresent(sig -> currentThoughtSignature = sig);
boolean isThought = part.thought().orElse(false);
// Immediately flush the active text buffer to preserve the exact interleaved blocks of
// text/thoughts.
// Flush before capturing this chunk's signature below, or the signature of the run
// starting here lands on the run being flushed.
if (!currentTextBuffer.isEmpty() && isThought != currentTextIsThought) {
flushTextBufferToSequence();
}
if (currentTextBuffer.isEmpty()) {
currentTextIsThought = isThought;
}
// The signature rides on the merged part that flushTextBufferToSequence builds.
part.thoughtSignature().ifPresent(sig -> currentTextThoughtSignature = sig);
currentTextBuffer.append(text);
} else if (part.functionCall().isPresent()) {
hasContent = true;
Expand All @@ -443,16 +445,16 @@ private boolean accumulateParts(List<Part> parts) {
// future part types) rather than an allowlist that silently drops unlisted types. Flush
// buffered text first so parts keep their order, then append the part verbatim keeping
// any
// thoughtSignature it carries. The signature is intentionally not captured into
// currentThoughtSignature, which would leak it onto the preceding part.
// thoughtSignature it carries. The signature is intentionally not captured, which would
// leak it onto the preceding part.
hasContent = true;
flushTextBufferToSequence();
accumulatedSequence.add(part);
} else {
// Standalone thought/thought-signature part with no renderable content: not emitted on
// its
// own; capture its signature to re-attach to the last real part in processFinalResponse.
part.thoughtSignature().ifPresent(sig -> currentThoughtSignature = sig);
// its own; its signature rides on the text run it sits in, and overrides what that run's
// own chunks carried, because a signature-only part is an explicit carrier.
part.thoughtSignature().ifPresent(sig -> currentTextThoughtSignature = sig);
}
}
return hasContent;
Expand Down Expand Up @@ -605,9 +607,9 @@ private void flushTextBufferToSequence() {
if (!currentTextBuffer.isEmpty()) {
Part.Builder partBuilder =
Part.builder().text(currentTextBuffer.toString()).thought(currentTextIsThought);
if (currentThoughtSignature != null) {
partBuilder.thoughtSignature(currentThoughtSignature);
currentThoughtSignature = null;
if (currentTextThoughtSignature != null) {
partBuilder.thoughtSignature(currentTextThoughtSignature);
currentTextThoughtSignature = null;
}
accumulatedSequence.add(partBuilder.build());
currentTextBuffer.setLength(0);
Expand Down
170 changes: 170 additions & 0 deletions core/src/test/java/com/google/adk/flows/llmflows/ContentsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,13 @@
import com.google.adk.sessions.Session;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.genai.types.Blob;
import com.google.genai.types.Content;
import com.google.genai.types.FunctionCall;
import com.google.genai.types.FunctionResponse;
import com.google.genai.types.Part;
import com.google.genai.types.ToolCall;
import com.google.genai.types.ToolResponse;
import java.util.ArrayList;
import java.util.ConcurrentModificationException;
import java.util.Iterator;
Expand Down Expand Up @@ -946,6 +949,164 @@ public void processRequest_notEmptyContent() {
assertThat(contents).containsExactly(e.content().get());
}

// The caller must echo server-side tool parts back, so dropping them as "empty" makes the model
// redo the work or fail on a call with no matching response.
@Test
public void processRequest_serverSideToolCallAndResponseEvents_notSkipped() {
Event toolCallEvent =
createModelEvent(
"e2",
Part.builder()
.toolCall(
ToolCall.builder()
.id("tc1")
.args(ImmutableMap.of("url", "https://example.com"))
.build())
.build());
Event toolResponseEvent =
createModelEvent(
"e3",
Part.builder()
.toolResponse(
ToolResponse.builder()
.id("tc1")
.response(ImmutableMap.of("content", "page text"))
.build())
.build());
ImmutableList<Event> events =
ImmutableList.of(
createUserEvent("e1", "Summarize the linked page."), toolCallEvent, toolResponseEvent);

List<Content> contents = runContentsProcessor(events);

assertThat(contents).hasSize(3);
assertThat(contents.get(1).parts().get().get(0).toolCall().get().id()).hasValue("tc1");
ToolResponse toolResponse = contents.get(2).parts().get().get(0).toolResponse().get();
assertThat(toolResponse.id()).hasValue("tc1");
assertThat(toolResponse.response()).hasValue(ImmutableMap.of("content", "page text"));
}

// The echo-back contract holds regardless of how the model labels the part, so a thought marking
// must not drop it.
@Test
public void processRequest_serverSideToolCallMarkedAsThought_notSkipped() {
Event toolCallEvent =
createModelEvent(
"e2",
Part.builder()
.thought(true)
.toolCall(
ToolCall.builder()
.id("tc1")
.args(ImmutableMap.of("url", "https://example.com"))
.build())
.build());
ImmutableList<Event> events =
ImmutableList.of(createUserEvent("e1", "Summarize the linked page."), toolCallEvent);

List<Content> contents = runContentsProcessor(events);

assertThat(contents).hasSize(2);
assertThat(contents.get(1).parts().get().get(0).toolCall().get().id()).hasValue("tc1");
}

// A server-side call belongs to the model instance that made it, so the other-agent path must
// keep dropping it rather than claiming the call on this agent's behalf.
@Test
public void processRequest_serverSideToolCallFromOtherAgent_isDropped() {
Event otherAgentToolCall =
Event.builder()
.id("e2")
.author(OTHER_AGENT)
.content(
Content.builder()
.role("model")
.parts(
ImmutableList.of(
Part.builder()
.toolCall(
ToolCall.builder()
.id("tc1")
.args(ImmutableMap.of("url", "https://example.com"))
.build())
.build()))
.build())
.invocationId("invocationId")
.build();
ImmutableList<Event> events =
ImmutableList.of(createUserEvent("e1", "Summarize the linked page."), otherAgentToolCall);

List<Content> contents = runContentsProcessor(events);

assertThat(contents).hasSize(1);
assertThat(contents.get(0).parts().get().get(0).text()).hasValue("Summarize the linked page.");
}

@Test
public void processRequest_serverSideToolCallWithThoughtFromOtherAgent_isDropped() {
Event otherAgentToolCall =
Event.builder()
.id("e2")
.author(OTHER_AGENT)
.content(
Content.builder()
.role("model")
.parts(
ImmutableList.of(
Part.builder().thought(true).text("Let me look it up.").build(),
Part.builder()
.toolCall(
ToolCall.builder()
.id("tc1")
.args(ImmutableMap.of("url", "https://example.com"))
.build())
.build()))
.build())
.invocationId("invocationId")
.build();
ImmutableList<Event> events =
ImmutableList.of(createUserEvent("e1", "Summarize the linked page."), otherAgentToolCall);

List<Content> contents = runContentsProcessor(events);

assertThat(contents).hasSize(1);
assertThat(contents.get(0).parts().get().get(0).text()).hasValue("Summarize the linked page.");
}

// The other-agent path still narrates what it can: media parts pass through unchanged, so the
// drop above is about attribution rather than a blanket filter.
@Test
public void processRequest_mediaPartFromOtherAgent_isKept() {
Event otherAgentImage =
Event.builder()
.id("e2")
.author(OTHER_AGENT)
.content(
Content.builder()
.role("model")
.parts(
ImmutableList.of(
Part.builder()
.inlineData(
Blob.builder()
.mimeType("image/png")
.data(new byte[] {1, 2, 3})
.build())
.build()))
.build())
.invocationId("invocationId")
.build();
ImmutableList<Event> events =
ImmutableList.of(createUserEvent("e1", "What is in the picture?"), otherAgentImage);

List<Content> contents = runContentsProcessor(events);

assertThat(contents).hasSize(2);
assertThat(contents.get(1).parts().get()).hasSize(2);
assertThat(contents.get(1).parts().get().get(0).text()).hasValue("For context:");
assertThat(contents.get(1).parts().get().get(1).inlineData()).isPresent();
}

@Test
public void processRequest_concurrentReadAndWrite_noException() throws Exception {
LlmAgent agent =
Expand Down Expand Up @@ -1028,6 +1189,15 @@ private static Event createUserEvent(
.build();
}

private static Event createModelEvent(String id, Part part) {
return Event.builder()
.id(id)
.author(AGENT)
.content(Content.builder().role("model").parts(ImmutableList.of(part)).build())
.invocationId("invocationId")
.build();
}

private static Event createAgentEvent(String id, String text) {
return createAgentEvent(AGENT, id, text);
}
Expand Down
Loading
Loading