Skip to content

fix(db): preserve exact D2 rows during sync reconciliation - #1794

Open
obeattie wants to merge 4 commits into
TanStack:mainfrom
obeattie:fix-query-contributors-err
Open

fix(db): preserve exact D2 rows during sync reconciliation#1794
obeattie wants to merge 4 commits into
TanStack:mainfrom
obeattie:fix-query-contributors-err

Conversation

@obeattie

@obeattie obeattie commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

🎯 Changes

Fixes #1783.

D2 applies an update by removing the old row and adding the new one. The row being removed must exactly match the row previously added. The state diverged like this:

  1. An optimistic insert added one snapshot of the row to D2.
  2. Sync confirmation emitted another insert with the same key. This was correctly skipped so D2 would not count the row twice.
  3. D2 therefore kept the optimistic snapshot, while the collection later described the row using its sync-confirmed snapshot.
  4. A later update tried to remove the collection’s snapshot, but D2 held the optimistic snapshot.
  5. The snapshots represented the same key but were not identical, so D2 threw Query contributors with the same row key are not congruent.

Any differing property can cause this; it is not specific to $synced or $origin. A focused test demonstrates the same mismatch with a normal status property. Changing the hashing would only hide the mismatch by making different rows appear equal. The correct fix is to remove the exact row that was previously added.

I bisected the regression to #1740. That PR added the congruence check which exposed the incorrect removal. This check is correct; the source bookkeeping was not.

The fix remembers the exact row sent to D2 for each key and uses it for later updates and deletes. The same bookkeeping also prevents duplicate inserts.

The regression test covers the original optimistic insert and synchronous sync confirmation, then verifies that the collection and live query each contain exactly one updated row.

✅ Checklist

  • I have tested this code locally with pnpm test.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • Bug Fixes

    • Fixed live queries throwing when optimistically inserted rows are updated after synchronous sync confirmation.
    • Improved handling of row updates, deletions, and duplicate inserts to keep query and collection results consistent.
    • Ensured updates correctly replace previously received row values without stale or duplicate data.
  • Tests

    • Added coverage for optimistic insert updates and accurate row reconciliation.

obeattie and others added 4 commits September 1, 2026 15:19
@coderabbitai

coderabbitai Bot commented Sep 1, 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: Team

Run ID: 0769a2a9-3cfa-48e8-9002-cb8e12f189d0

📥 Commits

Reviewing files that changed from the base of the PR and between 68366ec and 8abd32e.

📒 Files selected for processing (5)
  • .changeset/sixty-nights-refuse.md
  • packages/db/src/query/effect.ts
  • packages/db/src/query/live/collection-subscriber.ts
  • packages/db/src/query/live/utils.ts
  • packages/db/tests/collection-subscriber-duplicate-inserts.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Live query and effect pipelines now track the exact rows contributed to D2. Updates and deletes retract those stored rows. Tests cover row reconciliation and updates to synchronously confirmed optimistic inserts. A patch changeset records the fix.

Changes

Live query reconciliation

Layer / File(s) Summary
Row reconciliation utility
packages/db/src/query/live/utils.ts
prepareChangesForD2 tracks sent rows in a map. It removes duplicate inserts and sets previousValue to the exact previously sent row for updates and deletes.
Pipeline state integration
packages/db/src/query/effect.ts, packages/db/src/query/live/collection-subscriber.ts
Per-source state stores contributed rows. D2 preparation, ordered cursor tracking, truncation, and disposal use the row maps.
Regression validation and release
packages/db/tests/collection-subscriber-duplicate-inserts.test.ts, .changeset/sixty-nights-refuse.md
Tests cover exact-row retraction and updates to synchronously confirmed optimistic inserts. The changeset declares a patch release.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 8abd3

The PR correctly preserves exact rows during live-query reconciliation, preventing duplicate and update failures in normal operation. A bounded recovery risk remains if graph processing fails after bookkeeping is updated, which warrants owner awareness or follow-up but does not block merge under normal checks.

Suggested reviewers: kevin-dp, kyleamathews

Sequence Diagram(s)

sequenceDiagram
  participant Collection
  participant CollectionSubscriber
  participant prepareChangesForD2
  participant D2Pipeline
  Collection->>CollectionSubscriber: update optimistically inserted row
  CollectionSubscriber->>prepareChangesForD2: provide change and sent row map
  prepareChangesForD2->>D2Pipeline: emit update with exact previousValue
  D2Pipeline-->>CollectionSubscriber: process reconciled live query change
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: preserving exact D2 rows during sync reconciliation.
Description check ✅ Passed The description includes the required Changes, Checklist, and Release Impact sections. It explains the defect, fix, regression test, local test status, and changeset.
Linked Issues check ✅ Passed The changes address issue #1783 by tracking exact rows for D2 reconciliation, preventing congruence errors after optimistic inserts and sync confirmation. The regression test covers the required optim…
Out of Scope Changes check ✅ Passed All changed files support the linked issue. The implementation, changeset, and regression tests are directly related to D2 sync reconciliation and live-query consistency.
Full details: Linked Issues check

Explanation

The changes address issue #1783 by tracking exact rows for D2 reconciliation, preventing congruence errors after optimistic inserts and sync confirmation. The regression test covers the required optimistic insert, synchronous confirmation, and later update flow.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ 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.

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.

Calling writeInsert inside a createOptimisticAction throws error

1 participant