Skip to content

test(sessions): cover tied-timestamp persistence gaps - #7492

Closed
jabrailkhalil wants to merge 2 commits into
google:mainfrom
jabrailkhalil:test/database-tied-timestamp-regressions
Closed

jabrailkhalil wants to merge 2 commits into
google:mainfrom
jabrailkhalil:test/database-tied-timestamp-regressions

Conversation

@jabrailkhalil

@jabrailkhalil jabrailkhalil commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

Related: #7286. I prepared this tests-only PR following the suggested regression-first approach.

I reproduce two persistence gaps: (timestamp, id) ordering changes a tied-timestamp append history and its recent tail; using event.timestamp as the revision marker lets a stale writer append after another same-timestamp commit.

The file-backed tests use independent service instances, 40 events in forward/reverse ID order, recent limits None/0/1/5/100, and same/increasing-timestamp writer controls. Only the five known failures have strict, assertion-only xfail markers. Production code, schemas, migrations, dependency manifests and tox configuration remain identical to main.

The existing test_get_session_orders_tied_timestamps_by_id[True] still requires reverse-appended IDs to reload in ID order. The new strict xfails record the opposite append-order expectation so the contract decision in #7286 stays explicit. If append order is selected, I will update or remove that existing assertion as part of the fix. The marker-less staleness fallback also selects its latest stored event by (timestamp, id); an append-sequence decision would need to cover that lookup as well as reads.

Current-main integration

I integrated main 12c7bed7661ccf942f0f6d3b62bed22a23ea0a85 by a normal, conflict-free merge. Current head: 97d674a4e2a8f51d63eb7b59a40e0c59d8f4c1f5. The net diff remains 122 added lines in one existing test file. This PR records failures; it does not choose an ordering contract or introduce a migration.

Testing Plan

Fresh Linux validation on the integrated head, 24 October 2026:

Python Passed Skipped Xfailed Xpassed
3.11 21,336 96 23 2
3.12 21,327 97 23 2
3.13 21,327 97 23 2
3.14 21,328 96 23 2
  • The complete tox matrix ran tests/unittests with three pytest workers. I used upstream CI's lancedb install exclusion and a dependency cutoff of 2026-10-24T11:49:14Z; no test exclusions were added. The two existing non-strict xpasses are separate from this PR's strict xfails.
  • The 12 new cases plus two SQLite controls: 9 passed / 5 strict xfailed. With --runxfail, exactly those five DatabaseSessionService assertions fail; there are no setup errors.
  • All-extras mypy comparison in the same environment: 269 errors on main / 269 on head / zero new errors. This is a baseline comparison, not a clean whole-repository mypy claim.
  • uv build passes. A clean Python 3.14 environment installs the built google-adk 2.13.0 wheel with [db]; its actual site-packages import writes four events and state to a SQLite file, closes the writer, and verifies both through an independent reader. No model or provider calls.
  • All pre-commit hooks pass, including the new-file/unit-guide hook and addlicense, on GitHub's generated test merge 14921b39ff8699779b673dc52d75a40678509c10. I verified its tree is identical to PR head 97d674a4e2a8f51d63eb7b59a40e0c59d8f4c1f5 and its parents are the exact main/head above. Tracked source remains unchanged. The new-file check also passes its official --baseline-dir mode against main. The direct branch-history run had identified upstream files across HEAD~1..HEAD; the actual GitHub merge context resolves that range correctly. No waiver, rule change or hook exclusion is set.

The earlier Windows and Workflow replay runs are historical evidence and were not rerun for this base sync. PostgreSQL/MySQL/Spanner and an actual concurrent scheduling race are not claimed. Current-head upstream CI authorization and maintainer review remain separate gates.

I capture fresh-reader ordering, recent-event limits and same-timestamp stale writers with strict xfails while schema alignment remains open. Related: google#7286.
@jabrailkhalil jabrailkhalil changed the title test(sessions): I cover tied-timestamp persistence gaps test(sessions): cover tied-timestamp persistence gaps Oct 15, 2026
@jabrailkhalil
jabrailkhalil marked this pull request as ready for review October 15, 2026 13:42
@jabrailkhalil

Copy link
Copy Markdown
Contributor Author

I integrated current main in 97d674a4 and reran the full tox matrix on Python 3.11–3.14. All four pass; the five strict regression xfails remain explicit. All pre-commit hooks pass on GitHub’s test merge with the identical source tree. The clean wheel/session round-trip passes, and mypy adds no errors against main. This remains tests-only; the ordering decision is still in #7286. Could a maintainer approve the pending workflow runs for this head?

copybara-service Bot pushed a commit that referenced this pull request Oct 28, 2026
@adk-bot

adk-bot commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Thank you @jabrailkhalil for your contribution! 🎉

Your changes have been successfully imported and merged via Copybara in commit 5db54a0.

Closing this PR as the changes are now in the main branch.

@adk-bot adk-bot added the merged [Status] This PR is merged label Oct 28, 2026
@adk-bot adk-bot closed this Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged [Status] This PR is merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants