Skip to content

fix(sessions): keep integers outside 64 bits exact in SqliteSessionService state - #7577

Open
Somay-kousis wants to merge 2 commits into
google:mainfrom
Somay-kousis:fix/sqlite-state-bigint
Open

Somay-kousis wants to merge 2 commits into
google:mainfrom
Somay-kousis:fix/sqlite-state-bigint

Conversation

@Somay-kousis

@Somay-kousis Somay-kousis commented Oct 16, 2026 •

Copy link
Copy Markdown

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:

_MERGE_STATE_SQL rebuilds the whole state row in SQLite on every state change. SQLite parses an integer outside the signed 64-bit range as REAL, so json_each reports it with type = 'integer' but a REAL value, and json_group_object writes that REAL back. A 64-bit hash stored in state becomes a float, both when it's written and on any later write to another key. #7426 fixed floats in the same SQL and noted this case as not covered.

Solution:

For a value whose type is integer but whose SQL value is REAL, copy the element's original JSON text with SQLite's -> operator instead of the parsed value. The lookup runs only for those values, so merging ordinary state stays linear in the key count. Nothing else in the merge changes. -> needs SQLite 3.38+. On older SQLite the expression is NULL, so the merge falls back to today's behavior: no error, and such an integer still reads back rounded there.

The delta now binds once as ?1, since the merge reads it in more places.

Testing Plan

Unit Tests:

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

Two new tests in tests/unittests/sessions/test_session_service.py, run on every session backend and for plain, user: and app: keys: test_large_integers_survive_unrelated_state_delta (big ints in the initial state, then an unrelated delta) and test_large_integer_state_delta_is_stored_exactly (big ints written by a delta). They use 2**64 - 1 and -(2**70) - 1. test_sqlite_state_merge_scales_linearly_with_ordinary_state checks that a one-key merge into 20k ordinary keys takes under 30x the time of 2k keys (about 10x with the fix).

Before the fix, only the SQLite-backed services fail:

FAILED tests/unittests/sessions/test_session_service.py::test_large_integers_survive_unrelated_state_delta[sqlite-]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integers_survive_unrelated_state_delta[sqlite-user:]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integers_survive_unrelated_state_delta[sqlite-app:]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integers_survive_unrelated_state_delta[per_agent_database-]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integers_survive_unrelated_state_delta[per_agent_database-user:]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integers_survive_unrelated_state_delta[per_agent_database-app:]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integer_state_delta_is_stored_exactly[sqlite-]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integer_state_delta_is_stored_exactly[sqlite-user:]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integer_state_delta_is_stored_exactly[sqlite-app:]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integer_state_delta_is_stored_exactly[per_agent_database-]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integer_state_delta_is_stored_exactly[per_agent_database-user:]
FAILED tests/unittests/sessions/test_session_service.py::test_large_integer_state_delta_is_stored_exactly[per_agent_database-app:]
12 failed, 24 passed, 350 deselected in 3.27s

After the fix:

$ pytest tests/unittests/sessions/test_session_service.py -k large_integer
36 passed, 350 deselected in 11.86s

$ pytest tests/unittests/sessions
640 passed in 17.39s

With sqlite3.sqlite_version_info forced to (3, 37, 2), the merge takes the NULL path and test_session_service.py gives 12 failed, 374 passed: the same 12 as main, no SQL errors. pyink, isort and mypy pass on the changed files.

Manual End-to-End (E2E) Tests:

The script from the issue runs an LlmAgent with a fake model and three tools on a Runner with SqliteSessionService, reloading the session after each turn.

Before:

expected                     18364758544493064720 (int)
after remember_fingerprint   state['fingerprint'] = 1.8364758544493064e+19 (float)
after count_visit            state['fingerprint'] = 1.8364758544493064e+19 (float)
after check_fingerprint      state['fingerprint'] = 1.8364758544493064e+19 (float)
check_fingerprint tool saw: {'matches': False, 'value': '1.8364758544493064e+19'}

After:

expected                     18364758544493064720 (int)
after remember_fingerprint   state['fingerprint'] = 18364758544493064720 (int)
after count_visit            state['fingerprint'] = 18364758544493064720 (int)
after check_fingerprint      state['fingerprint'] = 18364758544493064720 (int)
check_fingerprint tool saw: {'matches': True, 'value': '18364758544493064720'}

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

N/A

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request addresses an issue where SQLite parses integers outside the signed 64-bit range as REAL, leading to precision loss. It introduces a check using the -> operator (available in SQLite 3.38+) to retrieve the exact JSON text representation of large integers and preserve them as exact integers. It also updates the parameter binding in _update_session_state_in_db to use ?1 for the delta JSON. Additionally, unit tests have been added to verify that large integers are stored exactly and survive unrelated state updates. I have no feedback to provide as there are no review comments.

@BichengWang BichengWang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Guard raw JSON extraction inside each UNION arm

Reviewed dbc39ca2ae354ca75b9b8ba64c9ddd64b0962e90 against current upstream origin/main at e4067dbcc8192615b69d8ca1fc1de17743a71676.

In src/google/adk/sessions/sqlite_session_service.py:84-87, the new source -> fullkey projections run for every key before the outer integer guard. Each lookup searches the source object, so merging an unrelated delta now takes quadratic time even when every value is an ordinary small integer and no value needs this fix. This affects persisted session, user, and app state on subsequent writes.

I reproduced this offline with the exact _MERGE_STATE_SQL assignments extracted from that upstream baseline and the PR head, using state = {f'k{i}': i for i in range(n)} and the single-key delta {'other': 1}. Median of five executions on Python 3.13.16 / SQLite 3.53.4:

State keys JSON bytes Upstream main PR head
1,000 12,780 0.22 ms 3.23 ms
5,000 72,780 1.47 ms 67.70 ms
10,000 147,780 2.28 ms 246.49 ms
20,000 317,780 4.13 ms 1023.51 ms

The public SqliteSessionService.append_event path at this head also reproduces the growth: median of three unrelated single-key appends was 46.60 / 177.13 / 622.04 ms for 5k / 10k / 20k state keys, respectively (Python 3.12.15 / SQLite 3.53.1, temporary local database).

Could the extraction be guarded in each inner SELECT, for example CASE WHEN type = 'integer' AND typeof(value) = 'real' THEN source -> fullkey END, so normal values never perform the lookup? Applying only that guard to the SQL in memory reduced the 20k-key case to 7.07 ms, with identical resulting state. All 24 additional SQL round trips for positive/negative out-of-range integers, delta/retained-state paths, and ordinary/dotted/quoted/backslash/Unicode/empty keys passed. A scaling regression check for ordinary state would cover the missing case.

Validation: the PR's new tests passed unchanged (pytest tests/unittests/sessions/test_session_service.py -k large_integer: 36 passed). A broader offline sessions run excluding test_vertex_ai_session_service.py had 578 passed and two failures caused by missing optional Spanner/Agent Platform dependencies; the excluded Vertex test module also could not collect without Google API Core. Full tox was not run. No source or test files were modified.

@Somay-kousis

Copy link
Copy Markdown
Author

@BichengWang you're right, thanks for measuring it. Every key paid for the -> lookup. Moved the type check inside each arm in f094042, so only an integer that SQLite read as REAL does the lookup.

Same setup as yours (one-key delta into n small-int keys, median of 5, Python 3.12.13 / SQLite 3.53.4):

State keys main dbc39ca f094042
1,000 0.46 ms 6.85 ms 0.68 ms
5,000 2.35 ms 163.35 ms 3.39 ms
10,000 4.71 ms 637.42 ms 6.82 ms
20,000 9.51 ms 2491.81 ms 13.72 ms

Added test_sqlite_state_merge_scales_linearly_with_ordinary_state for the missing case. It times the merge at 2k and 20k ordinary keys and checks the ratio stays under 30. It's about 10 with the guard and about 100 on dbc39ca, where it fails. The big-int tests pass unchanged, and tests/unittests/sessions gives 641 passed.

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.

SqliteSessionService turns state integers outside 64 bits into floats

3 participants