Skip to content

fix: stop losing event fields on Firestore session reloads - #1643

Open
innoprej wants to merge 1 commit into
google:mainfrom
innoprej:fix/firestore-session-whole-events
Open

innoprej wants to merge 1 commit into
google:mainfrom
innoprej:fix/firestore-session-whole-events

Conversation

@innoprej

@innoprej innoprej commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

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:

FirestoreSessionService stores only part of each event and rebuilds events from that part when a session is read back. Reloaded events lose their id, invocationId, branch, actions, thought flags and signatures, and function call and response IDs, and user events come back with the user ID as their author. Because Runner reads the session on every run, later requests quote earlier user messages as another agent's content and drop earlier tool results. The function call args are stored as they are, so a tool confirmation request, whose args hold a FunctionCall and a ToolConfirmation, cannot be saved. #1640 has the details and a reproduction.

Solution:

  • eventToMap also stores the whole event as JSON (event.toJson()) in a new rawEvent field, and eventFromMap reads it back with Event.fromJson(). As in fix: stop losing event fields on Vertex AI session reloads #1574 for the Vertex AI session service, the other fields remain the fallback when rawEvent is missing (events stored before this change) or unreadable.
  • The other fields are still written as before, because FirestoreMemoryService searches the same documents by keywords and reads author, timestamp and the text parts from them.
  • The tool call args and results written to those fields are converted to plain values with the ADK object mapper, as FunctionTool does for tool return values. Firestore cannot encode a FunctionCall, a ToolConfirmation or an Optional.

One deliberate difference from ADK Python, noted in the RAW_EVENT_KEY Javadoc: ADK Python stores the event as a map (event_data), and this stores a JSON string, so Firestore's limits on maps (no arrays directly inside arrays, reserved field names, nesting depth) do not apply to this copy of the event. The tool args and results in the other fields are still subject to them, as before. The field is named rawEvent as in #1574; the Java and Python Firestore services use different collection paths, so neither reads the other's events.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

./mvnw -pl contrib/firestore-session-service test: 56 run, 2 failures, both in FirestorePropertiesTest (shouldReturnHardcodedDefaultIfPropertyFileHasNoEntry, shouldLoadDevPropertiesWhenEnvIsSetToDev), which fail only on Windows, the same way on main, and pass in the Ubuntu CI (local run: Microsoft Build of OpenJDK 17.0.19, Windows 11). The four new tests are in FirestoreSessionServiceTest:

Test On main With this change
appendAndGet_returnsEventsAsAppended: a user message, a model event with a signed thought and a function call, and the function response, read back with getSession fails: id and invocationId are null and the user message's author is test-user-id, among other differences passes
appendEvent_withToolConfirmationRequest_storesArgsAsPlainValues fails: originalFunctionCall is stored as a FunctionCall passes
appendEvent_withObjectsInToolResult_storesResultAsPlainValues: {"result": [a record with an Optional]}, which is what FunctionTool returns for a list fails: the result is stored as the records passes
getSession_withUnreadableRawEvent_loadsEventFromOtherFields passes (main ignores rawEvent) passes; guards the fallback

Partial implementations fail them too: writing rawEvent without reading it fails the first test, converting only the args or only the results fails the third or the second, and catching IllegalArgumentException (what #1574 catches around convertValue) instead of the IllegalStateException that Event.fromJson throws fails the fourth.

appendAndGet_withAllPartTypes_serializesAndDeserializesCorrectly now removes rawEvent from the stored document before reading it back, so it keeps covering how events stored before this change are read.

Manual End-to-End (E2E) Tests:

Not run against a real Firestore database or a model. Following the reproduction steps in #1640:

  • the reloaded events, and the agent's next request built from them with Contents, are the same as before the reload;
  • the document that appendEvent stores for the tool confirmation request is accepted by batch().set(...) of a real Firestore client that never commits. On main it throws No properties to serialize found on class com.google.genai.types.AutoValue_FunctionCall.

Checklist

  • I have read the CONTRIBUTING.md document.
  • My pull request contains a single commit.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

  • Events stored before this change have no rawEvent, so they still load from the other fields as before (the user ID as the author, no IDs). Only events appended after the change come back whole.
  • Each event's text, function calls and function responses are now stored twice: in the current fields, which FirestoreMemoryService and older versions of this service read, and in rawEvent. If you would rather keep only what FirestoreMemoryService needs in the current fields (the author, timestamp, text parts and keywords), I can change that, which would leave only the text stored twice.
  • Because rawEvent also holds inline data and repeats the text, function calls and function responses, some events that were saved before can now exceed Firestore's 1 MiB document limit and fail to save: one with inline data such as an uploaded image, which used to be dropped, or one whose text or tool results take up more than about half of the limit. ADK Python keeps inline data in event_data too. With an artifact service, RunConfig.saveInputBlobsAsArtifacts saves the inline data in user messages as artifacts instead.

FirestoreSessionService stored only an event's author, timestamp, and
text, function call, function response and file data parts, and
rebuilt events from those fields alone. Reloaded events lost their ID,
invocation ID, branch, actions, thought flags and signatures, and
function call and response IDs, and user events came back with the
user ID as their author. Function call args were stored as they were,
so a tool confirmation request, whose args hold a FunctionCall and a
ToolConfirmation, could not be saved.

- Events are also stored as JSON in `rawEvent` and read back from it.
  Events stored before this change still load from the other fields,
  which FirestoreMemoryService keeps reading.
- Tool call args and results are stored as plain values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FirestoreSessionService loses event fields on reload, breaking later turns and tool confirmations

1 participant