Skip to content

fix: list each app once in SQL persister list_app_ids, newest save first - #942

Open
sangkyoonnam wants to merge 1 commit into
apache:mainfrom
sangkyoonnam:fix/list-app-ids-distinct-latest
Open

sangkyoonnam wants to merge 1 commit into
apache:mainfrom
sangkyoonnam:fix/list-app-ids-distinct-latest

Conversation

@sangkyoonnam

Copy link
Copy Markdown

list_app_ids on the SQL persisters now returns each app once, ordered by that app's most recent save. Today the Postgres persisters can return the same app once per save, and the SQLite persisters can put an older app first.

The Postgres query is SELECT DISTINCT app_id, created_at ... ORDER BY created_at DESC, so it keeps one row per distinct (app_id, created_at) pair. Saving a-app seq 0, z-app seq 0, a-app seq 1, a-app seq 2 in separate transactions returns ['a-app', 'a-app', 'z-app', 'a-app'] on main, for both psycopg2 and asyncpg. The SQLite query is SELECT DISTINCT app_id ... ORDER BY created_at DESC, which sorts by whichever row SQLite keeps for each app. Saving a, z, a at distinct timestamps returns ['z-app', 'a-app']. The youtube-to-social-media-post notebook picks the app to resume with persister.list_app_ids("")[0], so on SQLite that can resume the wrong app. This came up while reviewing #920.

Changes

  • The SQLite, aiosqlite, psycopg2 and asyncpg persisters use GROUP BY app_id ORDER BY MAX(created_at) DESC.

How I tested this

  • One new test per persister saves a, z, a with fixed created_at values and asserts ['a-app', 'z-app']. All four fail on main and pass here. The Postgres tests use their own partition key, so rows from the existing tests don't leak in.
  • The Postgres tests ran against a local postgres:15 container with BURR_CI_INTEGRATION_TESTS=true: 13 passed.
  • pytest tests --ignore=tests/integrations/persisters/test_postgresql.py --ignore=tests/integrations/test_bip0042_bedrock.py: 682 passed, 4 skipped.
  • pre-commit run on the changed files passes.

Notes

  • Redis and MongoDB already return unique IDs and aren't changed here. Redis orders by sequence_id rather than save time, so ordering still differs across persisters.
  • SQLite created_at has one-second resolution, so apps saved within the same second can still come back in either order.
  • MAX(created_at) per app isn't served by the existing created_at index. On an in-memory SQLite table with 100,000 saves across 100 apps, the new query takes about 21 ms against 3 ms for the old one (19 ms after ANALYZE). A (partition_key, app_id, created_at) index could help large tables, but I haven't measured it and left the schema alone.
  • I wrote this with Claude Code and reviewed and ran it myself.

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added area/core Application, State, Graph, Actions area/storage Persisters, state storage area/integrations External integrations (LLMs, frameworks) labels Oct 4, 2026

This branch has not been deployed

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

Labels

area/core Application, State, Graph, Actions area/integrations External integrations (LLMs, frameworks) area/storage Persisters, state storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant