refactor(firestore) : comprehensive refactoring for FirestoreSessionService

[Dumeng]

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

N/A

2. Or, if no issue exists, describe the change:

If applicable, please follow the issue templates to provide as much detail as
possible.

Problem:
This Pull Request introduces comprehensive refactoring, performance optimizations, and robustness improvements to the FirestoreSessionService. It simplifies the core implementation, aligns state handling logic with other session services, fixes side-effects in transactions, and significantly improves read performance for bulk operations.

Solution:

• Optimized Batch Retrieval: Updated list_sessions to leverage Firestore's native client.get_all(...) method. This fetches multiple user state documents in a single optimized RPC request rather than individual sequential reads, significantly reducing read latency and overhead.

• Aligned State Handling: Refactored state management logic to match the established, robust patterns found in DatabaseSessionService.
  Transaction Safety: Removed unsafe side-effects inside transaction callbacks within append_event to ensure idempotency and reliability during automatic Firestore transaction retries.

• Revision Tracking: Explicitly initialized session revision tracking to 0 upon session creation to guarantee monotonic tracking consistency.

• Simplified Implementation: Streamlined internal data extraction and simplified redundant record lookups for improved long-term maintainability.

• Import Standards: Resorted and updated import statements to strictly adhere to the ADK project's relative import conventions and code style guide.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required
for all PRs that are not small documentation or typo fixes.

Unit Tests:

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

Please include a summary of passed pytest results.

Manual End-to-End (E2E) Tests:

Please provide instructions on how to manually test your changes, including any
necessary setup or configuration. Please provide logs or screenshots to help
reviewers better understand the fix.

Checklist

• I have read the CONTRIBUTING.md document.
• 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

Add any other context or screenshots about the feature request here.

[rohityan]

Hi @Dumeng, Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share.

[rohityan]

Hi @xuanyang15 , can you please review this.

[xuanyang15]

@DeanChensj Could you please help review?

[DeanChensj]

@gemini-cli /review

[github-actions]

🤖 Hi @DeanChensj, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

[github-actions]

🤖 I'm sorry @DeanChensj, but I was unable to process your request. Please see the logs for more details.

[github-actions]

The changes in this PR significantly improve the efficiency of the FirestoreSessionService by using concurrent fetches (asyncio.gather) and batching user state retrievals (get_all). The refactoring of imports and type hints also makes the code cleaner.

I've reviewed the logic for session creation, retrieval, and event appending, and it looks solid. The use of revisions for optimistic concurrency control is correctly implemented.

Great work!

[Dumeng]

Hi team, any update?

[DeanChensj]

Hello @Dumeng! Thank you very much for contributing this pull request to improve ADK. I've conducted a thorough architectural and style review of your implementation.

The performance optimizations, especially using Firestore's native client.get_all for bulk fetches in list_sessions and incorporating asyncio.gather in get_session, are stellar enhancements. Furthermore, removing the side-effects on in-memory session.state inside transaction callbacks is a great improvement for robustness.

However, there is a critical regression introduced by changing the initial revision count without aligning the local update marker.

Here is the feedback and the suggested change to fix this:

🔴 Major Concerns / Blocks

Stale Session Exception on First Event Append (Revision Mismatch)

• Target Code: firestore_session_service.py:L204-L210 and firestore_session_service.py:L275
• Issue: In create_session, you changed the initialized document revision from 1 to 0:

  session_data = {
      ...
      "revision": 0,
  }

  However, the return block of create_session (which is unchanged in the PR) still sets the in-memory update marker to "1":

  session._storage_update_marker = "1"

  When the newly created session is returned, it carries the update marker "1". On the very first append_event call, the transaction reads the document from Firestore, retrieves "revision": 0, and executes the optimistic concurrency check:

  if session._storage_update_marker != str(current_revision):
      raise ValueError(_STALE_SESSION_ERROR_MESSAGE)

  Since "1" != "0" is True, every newly created session will immediately throw a ValueError and crash on the first event append.
• Suggested Correction: Align the local _storage_update_marker with the new initialized revision of "0". Please update lines 267–276 of create_session as follows:

  local_now = datetime.now(timezone.utc).timestamp()
  session = Session(
      id=session_id,
      app_name=app_name,
      user_id=user_id,
      state=merged_state,
      events=[],
      last_update_time=local_now,
  )
  session._storage_update_marker = "0"
  return session

[Dumeng]

Stale Session Exception on First Event Append (Revision Mismatch)

This issue has been fixed

[DeanChensj]

Could you fix the failing tests?