Skip to content

fix(sqlite-persistence): preserve query owner metadata on insert (closes #1618)#1626

Open
kevin-dp wants to merge 4 commits into
TanStack:mainfrom
kevin-dp:fix/persisted-collection-stale-insert-metadata
Open

fix(sqlite-persistence): preserve query owner metadata on insert (closes #1618)#1626
kevin-dp wants to merge 4 commits into
TanStack:mainfrom
kevin-dp:fix/persisted-collection-stale-insert-metadata

Conversation

@kevin-dp

@kevin-dp kevin-dp commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #1618

Problem

With a SQLite-persisted queryCollection, an item inserted while the app is open survives forever after a reload, even once the query stops returning it (the server-side row was deleted). Reported in #1618 (ExpoSQLitePersistence, but it reproduces with any SQLite persistence adapter).

Root cause: in db-sqlite-persistence-core, a sync write of type insert with no per-row metadata unconditionally queues a metadata reset (delete) for that key. During query reconciliation the query collection stamps the row's owners metadata (metadata.row.set) and then writes the metadata-less insert in the same sync transaction — so the insert's reset clobbers the just-written owners. Without persisted owners, the row is orphaned: on reload it belongs to no query and an empty query result can never reclaim it.

Approach (red → green)

  1. First commit — regression test only (expected to fail CI). A behaviour-level test in query-db-collection that inserts a row, reloads from the persisted store, and asserts the row is cleaned up once the query no longer returns it.
  2. Second commit — the fix. Cherry-picked from @tombryden's fix(db-sqlite-persistence-core): rows marked with metadata prior to insert get reset causing stale data #1619: only reset metadata on a metadata-less insert when no metadata write is already queued for that key in the transaction, so an explicitly-stamped owner survives while genuinely metadata-less inserts still clear stale metadata.

This PR is a companion to #1619 that adds the missing end-to-end coverage; fix authorship is preserved via cherry-pick.

🤖 Generated with Claude Code

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes

    • Fixed row metadata handling during inserts so staged metadata within the same transaction is no longer overwritten.
    • Cleared stale row metadata when inserting rows without metadata.
    • Improved persistence behavior so rows dropped by query syncing are removed from live state and persisted storage after reload.
  • Tests

    • Added tests covering metadata preservation vs. stale-metadata cleanup.
    • Added a regression test for deleted rows after rehydration and live query refresh.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f7259808-214f-4bd6-86ef-aa5df4a7c7fc

📥 Commits

Reviewing files that changed from the base of the PR and between f2b9def and 998a16c.

📒 Files selected for processing (1)
  • packages/query-db-collection/tests/query.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/query-db-collection/tests/query.test.ts

📝 Walkthrough

Walkthrough

persisted.ts now avoids overwriting queued row metadata during metadata-less inserts. Tests cover both preserving explicit metadata in the same transaction and clearing stale metadata, plus a reload regression for dropped rows.

Changes

Metadata reset bug fix and tests

Layer / File(s) Summary
Conditional row-metadata delete on insert
packages/db-sqlite-persistence-core/src/persisted.ts, .changeset/small-tables-listen.md
The row-metadata delete for metadata-less inserts is now conditional on there being no queued row-metadata write for the same key in the current transaction. The changeset records the patch release.
Unit and integration tests
packages/db-sqlite-persistence-core/tests/persisted.test.ts, packages/query-db-collection/tests/query.test.ts
Two unit tests cover metadata preservation and stale-metadata clearing, and one integration test covers cleanup of a dropped row after session reload.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • samwillis

Poem

A rabbit hopped by the data-store,
Saw stale metadata lingering no more.
One guarded delete kept truth in place,
And queued writes stayed safe with grace.
🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it does not follow the required template and is missing the Changes, Checklist, and Release Impact sections. Rewrite the PR description using the repository template and add the required sections, including a checklist and release-impact choice.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: preserving query owner metadata on insert for SQLite persistence.
Linked Issues check ✅ Passed The code and tests address #1618 by preserving queued owner metadata and verifying stale rows are cleaned up after reload.
Out of Scope Changes check ✅ Passed The changeset, fix, and regression tests are all directly related to the reported persistence bug, with no obvious unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Jun 29, 2026

Copy link
Copy Markdown
More templates

@tanstack/angular-db

npm i https://pkg.pr.new/@tanstack/angular-db@1626

@tanstack/browser-db-sqlite-persistence

npm i https://pkg.pr.new/@tanstack/browser-db-sqlite-persistence@1626

@tanstack/capacitor-db-sqlite-persistence

npm i https://pkg.pr.new/@tanstack/capacitor-db-sqlite-persistence@1626

@tanstack/cloudflare-durable-objects-db-sqlite-persistence

npm i https://pkg.pr.new/@tanstack/cloudflare-durable-objects-db-sqlite-persistence@1626

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@1626

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@1626

@tanstack/db-sqlite-persistence-core

npm i https://pkg.pr.new/@tanstack/db-sqlite-persistence-core@1626

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@1626

@tanstack/electron-db-sqlite-persistence

npm i https://pkg.pr.new/@tanstack/electron-db-sqlite-persistence@1626

@tanstack/expo-db-sqlite-persistence

npm i https://pkg.pr.new/@tanstack/expo-db-sqlite-persistence@1626

@tanstack/node-db-sqlite-persistence

npm i https://pkg.pr.new/@tanstack/node-db-sqlite-persistence@1626

@tanstack/offline-transactions

npm i https://pkg.pr.new/@tanstack/offline-transactions@1626

@tanstack/powersync-db-collection

npm i https://pkg.pr.new/@tanstack/powersync-db-collection@1626

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@1626

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@1626

@tanstack/react-native-db-sqlite-persistence

npm i https://pkg.pr.new/@tanstack/react-native-db-sqlite-persistence@1626

@tanstack/rxdb-db-collection

npm i https://pkg.pr.new/@tanstack/rxdb-db-collection@1626

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@1626

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@1626

@tanstack/tauri-db-sqlite-persistence

npm i https://pkg.pr.new/@tanstack/tauri-db-sqlite-persistence@1626

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@1626

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@1626

commit: 998a16c

A row inserted while the collection is mounted should be persisted together
with its query owner metadata, so that after a reload the row is removed from
both the live collection and the persisted store once the query no longer
returns it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kevin-dp kevin-dp force-pushed the fix/persisted-collection-stale-insert-metadata branch from 4a646e8 to 64e111a Compare June 29, 2026 13:39

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/query-db-collection/tests/query.test.ts (1)

4981-5000: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract a typed helper for the two session setups instead of using as any.

These two collection-construction blocks are nearly identical, and the casts hide the exact type contract this regression is supposed to lock down. A small helper that takes queryClient, adapter, and per-session overrides would remove the duplication and keep the test on the typed public surface. As per coding guidelines, **/*.{ts,tsx,js}: Extract common logic into utility functions when identical or near-identical code blocks appear in multiple places, and **/*.{ts,tsx}: Avoid using any types; use unknown instead when the type is truly unknown, and provide proper type annotations for return values.

Also applies to: 5024-5036

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/query-db-collection/tests/query.test.ts` around lines 4981 - 5000,
The two collection setup blocks for the reload-insert cleanup test are
duplicating the same construction logic and using casts that obscure the public
type contract. Extract a typed helper around
createCollection/queryCollectionOptions that accepts the shared inputs like
queryClient, adapter, and per-session overrides, then reuse it for both session
setups. Remove the as any casts by adding proper type annotations to the helper
return value and the options object so the test stays on the typed surface and
still exercises the same behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/query-db-collection/tests/query.test.ts`:
- Around line 5039-5052: The test in createLiveQueryCollection is not proving
that persisted hydration happens before the second-session refetch clears the
data. Update the query flow in query.test.ts by deferring the second queryFn
response so the persisted row can be asserted as present after
collection2.stateWhenReady() and before invalidation resolves to an empty
result. Use the existing symbols createLiveQueryCollection,
collection2.stateWhenReady(), secondQueryClient.invalidateQueries(), and
adapter2.rows.has(...) to structure the test so it fails if hydration never
occurs and only passes after the deferred fetch is released.

---

Nitpick comments:
In `@packages/query-db-collection/tests/query.test.ts`:
- Around line 4981-5000: The two collection setup blocks for the reload-insert
cleanup test are duplicating the same construction logic and using casts that
obscure the public type contract. Extract a typed helper around
createCollection/queryCollectionOptions that accepts the shared inputs like
queryClient, adapter, and per-session overrides, then reuse it for both session
setups. Remove the as any casts by adding proper type annotations to the helper
return value and the options object so the test stays on the typed surface and
still exercises the same behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0a72bd3e-a762-4698-b3ec-6c6e3954b83b

📥 Commits

Reviewing files that changed from the base of the PR and between 816b667 and f2b9def.

📒 Files selected for processing (4)
  • .changeset/small-tables-listen.md
  • packages/db-sqlite-persistence-core/src/persisted.ts
  • packages/db-sqlite-persistence-core/tests/persisted.test.ts
  • packages/query-db-collection/tests/query.test.ts

Comment thread packages/query-db-collection/tests/query.test.ts Outdated
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.

ExpoSQLitePersistence + queryCollectionOptions + offline-transactions - stale items after insert

3 participants