Skip to content

Conversation

@nimanthadilz
Copy link
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):

Problem:
Currenlty, there is no way to limit the number of events fetched from the session store when using Runner.run_async method. In some cases, this could cause to fetch a large num. of events from the session store.

Solution:
The BaseSessionService.get_session method accepts a config parameter of type GetSessionConfig. But this is not used inside Runner.run_async. This PR updates RunConfig to include an optional field to pass a GetSessionConfig object and use it in run_async, run_debug & run_live.

Testing Plan

Added new unit tests to verify that new optional field is properly passed down when calling get_session from Runner.

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.
image

Manual End-to-End (E2E) Tests:

  1. Create a simple agent.
  2. Use SqliteSessionService as the session service.
  3. When running the agent using Runner.run_async, pass a RunConfig including a get_session_config.
  4. Use this patch and test the agent with debug logs enabled for aiosqlite package.
  5. You would see that the SQL statement executed has a LIMIT clause matching the config you passed.
2025-11-22 16:19:16,321 - aiosqlite - DEBUG - operation functools.partial(<bound method Connection._execute_fetchall of <Connection(Thread-2, started 19364)>>, 'SELECT event_data FROM events WHERE app_name=? AND user_id=? AND session_id=? AND timestamp >= ? ORDER BY timestamp DESC LIMIT ?', ['jokes_agent', 'nimantha', '9789', 1763788716.0, 2]) completed

If you want a quick setup to test this change, I have a GitHub repo here.

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.

@adk-bot adk-bot added the core [Component] This issue is related to the core interface and implementation label Nov 22, 2025
@nimanthadilz nimanthadilz marked this pull request as ready for review November 22, 2025 10:55
@ryanaiagent ryanaiagent self-assigned this Nov 25, 2025
@ryanaiagent
Copy link
Collaborator

Hi @nimanthadilz ,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.

@ryanaiagent ryanaiagent added the needs-review [Status] The PR is awaiting review from the maintainer label Nov 30, 2025
@ryanaiagent
Copy link
Collaborator

Hi @wyf7107 , can you please review this

@ryanaiagent ryanaiagent added services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc and removed core [Component] This issue is related to the core interface and implementation labels Dec 3, 2025
app_name=self.app_name,
user_id=user_id,
session_id=session_id,
config=run_config.get_session_config if run_config else None,
Copy link

@thomasErich135 thomasErich135 Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To maintain the same logic, I recommend adding this code at the beginning of the run_debug method and removing the ternary check from the config parameter assignment

run_config = run_config or RunConfig()

) -> None:
"""Rewinds the session to before the specified invocation."""
session = await self.session_service.get_session(
app_name=self.app_name, user_id=user_id, session_id=session_id

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To maintain the same logic, I recommend passing the config parameter to the get_session method.

  1. Update the rewind_async signature to include: run_config: Optional[RunConfig] = None.
  2. Initialize the config before the get_session call: run_config = run_config or RunConfig()
  3. Pass the config in the get_session call: config=run_config.get_session_config

# Check both calls had config=None
for call in self.mock_session_service.get_session.call_args_list:
assert call.kwargs["config"] is None

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend adding unit tests for rewind_async and run_live methods

@ryanaiagent
Copy link
Collaborator

Hi @nimanthadilz , Thank you for your patience here. I apologize for the delay in getting to this review; I know this has been sitting for a while. This PR has merge conflicts that require changes from your end. Could you please rebase your branch with the latest main branch to address these? Once this is complete, please let us know so we can proceed with the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review [Status] The PR is awaiting review from the maintainer services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow limiting num. of Session events fetched when calling Runner.run_async

4 participants