Skip to content

fix(sessions): honor zero recent events in database service#5965

Open
White-Mouse wants to merge 2 commits into
google:mainfrom
White-Mouse:codex/adk-session-zero-events
Open

fix(sessions): honor zero recent events in database service#5965
White-Mouse wants to merge 2 commits into
google:mainfrom
White-Mouse:codex/adk-session-zero-events

Conversation

@White-Mouse
Copy link
Copy Markdown
Contributor

Summary

  • honor GetSessionConfig(num_recent_events=0) in DatabaseSessionService
  • add cross-backend regression coverage for zero recent events in test_get_session_with_config
  • keep existing positive-limit and no-config behavior unchanged

Motivation

GetSessionConfig documents num_recent_events=0 as returning no events. InMemory and Sqlite already preserve that behavior, and ADK call sites use num_recent_events=0 when they only need to check whether a session exists. DatabaseSessionService used a truthy check, so 0 skipped the SQL LIMIT and returned the full event history.

Testing

  • uv run --extra test pytest tests/unittests/sessions/test_session_service.py::test_get_session_with_config -q
  • uv run --extra test pytest tests/unittests/sessions -q
  • uvx pyink --check src/google/adk/sessions/database_session_service.py tests/unittests/sessions/test_session_service.py
  • uv run python -m py_compile src/google/adk/sessions/database_session_service.py tests/unittests/sessions/test_session_service.py
  • git diff --check

@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label Jun 4, 2026
@White-Mouse White-Mouse marked this pull request as ready for review June 4, 2026 12:48
@boyangsvl
Copy link
Copy Markdown
Collaborator

/adk-pr-analyze

@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented Jun 4, 2026

🔍 ADK Pull Request Analysis: PR #5965

Title: fix(sessions): honor zero recent events in database service
Author: @White-Mouse
Status: open
Impact: 8 additions, 1 deletions across 2 files

Executive Summary

  1. Core Objective: Fixes DatabaseSessionService to properly honor GetSessionConfig(num_recent_events=0) by returning an empty event history instead of skipping the limit and returning the full session history.
  2. Justification & Value: Justified Fix - Crucial for database efficiency and consistency across all session service backends (matching InMemory and Sqlite services) when client code only seeks to check if a session exists without loading messages.
  3. Alignment with Principles: Pass - Restores correct contract representation, adheres to zero vs None semantic standards, contains proper target typing, and incorporates high-integrity unit tests.
  4. Recommendation: Approve
Detailed Findings & Analysis

1. Objectives & Impact ("What does it do?")

  • Context & Background: In ADK, session fetching can be parameterized with a GetSessionConfig context. When checking if a session exists without wanting to pull the historical stream, clients request num_recent_events=0.
  • Implementation Mechanism: Previously, database_session_service.py used a truthy check if config and config.num_recent_events:. Because 0 is falsy under Python's default truthy evaluation, the SQL pagination limit stmt = stmt.limit(...) was bypassed, loading and returning the entire event history. The PR switches this to if config and config.num_recent_events is not None:, correctly compiling a LIMIT 0 statement so that zero events are loaded.
  • Affected Surface: This update has zero breaking changes to public APIs. It fixes internal SQL pagination boundaries in database_session_service.py.

2. Justification & Value ("Is it a valid and useful change?")

  • Workspace Verification:
    • Investigated current workspace files: database_session_service.py.
    • Found that the baseline condition did indeed perform a truthy check on config.num_recent_events (which mistakenly interpreted 0 as "no limit").
  • Value Assessment: Highly valuable. This prevents wasteful memory allocation and excessive queries in large-scale production applications using database session services when session existence verification is performed.
  • Alternative Approaches: The chosen approach (is not None) is the cleanest and most standard way to differentiate between an unconfigured limit (None) and a zero-bounded limit (0).
  • Scope & Depth: Systematic Fix & Root Cause. It directly corrects the core logical gap where truthiness was used in place of explicit type bounds.

3. Principle & Style Alignment Checklist ("Does it follow rules?")

  • Public API & Visibility Boundaries:
    • Status: Pass
    • Analysis: No packages or exposed APIs are modified structurally. The public behavior aligns precisely with documented expectations.
  • Code Quality, Typing & Conventions:
    • Status: Pass
    • Analysis: Leverages existing annotations (from __future__ import annotations is correctly present in database_session_service.py). It avoids any typing anti-patterns, keeping the changes highly focused and readable.
  • Robustness & Edge Cases:
    • Status: Pass
    • Analysis: Perfectly discriminates between falsy values (0) and unprovided optional filters (None), preventing unexpected fallbacks.
  • Test Integrity & Quality:
    • Status: Pass
    • Analysis: Incorporates strong verification coverage in test_session_service.py by asserting that num_recent_events=0 yields an empty list of events. Tests follow explicit Arrange-Act-Assert formatting.

Phase Summary & Verification Results

  • Google CLA Verification: PASS (Status SUCCESS confirmed via status checks rollup API).
  • GitHub Workflow Status: PASS (All check runs, including agent-triage-pull-request and formatting/security scanning, succeeded).
  • Recommendation: Fully recommended for immediate approval

@rohityan rohityan self-assigned this Jun 5, 2026
@boyangsvl boyangsvl self-assigned this Jun 5, 2026
@boyangsvl boyangsvl added the ready to pull [Status] This PR is ready to be imported back to Google label Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to pull [Status] This PR is ready to be imported back to Google 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.

4 participants