fix(db): includes orderBy ignored after optimistic update on child collection

[comet08]

Closes #1444

🎯 Changes

When a child collection item is updated, the D2 graph emits a (-1, +1) pair. If the delete (-1) message arrives first, an accumulator entry is created with the old orderByIndex. The subsequent insert (+1) message updated value correctly but never updated orderByIndex — so the child collection's comparator kept using the stale positional index.

This caused incorrect sort order immediately after optimistic updates on child collections, with ordering only correcting itself after server confirmation.

The parent pipeline (accumulateChanges) already handled this correctly. This fix applies the same orderByIndex update logic to the child pipeline accumulator and the nested pipeline accumulator for consistency.

✅ 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

  • Resolved an issue where sort ordering in child collections included within parent queries was not properly preserved during child collection updates.

• Tests

  • Added comprehensive test coverage validating that sort ordering in nested child collections remains stable following both optimistic client-side and server-backed updates.

[coderabbitai]

📝 Walkthrough

Walkthrough

This patch fixes a bug where orderBy is lost in included child collections during optimistic updates. The fix prevents orderByIndex from being overwritten with undefined during change aggregation in both direct and nested includes paths, backed by a comprehensive test validating the correction persists through optimistic and server-confirmed updates.

Changes

OrderBy Preservation in Included Collections

Layer / File(s)	Summary
OrderByIndex conditional assignment in includes packages/db/src/query/live/collection-config-builder.ts	Conditionally assigns existing.orderByIndex only when _orderByIndex is defined in both setupIncludesOutput (lines 897–899) and setupNestedPipelines (lines 1294–1296), preventing the loss of existing ordering during multiplicity aggregation.
OrderBy ordering validation test packages/db/tests/query/includes.test.ts	New test suite validates that included child collection ordering is preserved through optimistic client-side updates (task position swaps) and subsequent server confirmation, checking the task id sequence before and after each phase.
Changeset documentation .changeset/twelve-jars-lead.md	Marks @tanstack/db as a patch release and documents the fix for preserving orderBy in includes after child-collection updates.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

• samwillis

Poem

🐰 A rabbit's tale of order restored:
When children update with hope renewed,
Their sorting was lost—a sorting world!
But a simple guard, undefined reviewed,
Now keeps the orderBy bright and praised. 🌿✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately describes the main change: fixing orderBy behavior in includes after optimistic updates on child collections.
Description check	✅ Passed	The description provides thorough context including the root cause, the fix strategy, testing confirmation, and changeset generation. All required template sections are addressed.
Linked Issues check	✅ Passed	The PR directly addresses issue #1444 by fixing the orderByIndex accumulation logic in child and nested pipeline accumulators to match parent pipeline behavior, resolving the reported sorting issue after optimistic updates.
Out of Scope Changes check	✅ Passed	All changes are focused on fixing the orderByIndex preservation issue in includes, with a corresponding test added and changeset generated. No out-of-scope modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/db/tests/query/includes.test.ts (1)

5372-5451: ⚡ Quick win

Add a nested-includes regression for the second modified path.

This suite covers the direct child include flow, but it never exercises the nested buffer path changed in setupNestedPipelines(). A status -> tasks -> subtasks optimistic reorder case would close the gap and protect the second branch touched in this PR. As per coding guidelines, Always add unit tests that reproduce a bug before fixing it to ensure the bug is fixed and prevent regression and Test corner cases including: ... async race conditions.

🤖 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/db/tests/query/includes.test.ts` around lines 5372 - 5451, The test
suite exercises optimistic reorders for a single child include but doesn't hit
the nested buffer path in setupNestedPipelines(); add a new spec that mirrors
the existing `orderBy in includes after child collection update` test but with a
nested include (status -> tasks -> subtasks) to exercise the second branch:
create mockSyncCollectionOptions and createCollection for statuses, tasks, and
subtasks, build a live query via createLiveQueryCollection that selects statuses
with tasks which in turn include subtasks ordered by position, preload the live
query, perform an optimistic swap of subtask positions using
subtasks.update(...), assert the in-memory order changes immediately, then use
the tasks/subtasksOptions.utils.begin/write/commit flow to simulate server
confirmation and finally assert the order remains correct; ensure test names and
helper functions mirror existing patterns (e.g., getSubtaskOrder analogous to
getTaskOrder) so setupNestedPipelines()’s nested pipeline path is exercised.

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.

Nitpick comments:
In `@packages/db/tests/query/includes.test.ts`:
- Around line 5372-5451: The test suite exercises optimistic reorders for a
single child include but doesn't hit the nested buffer path in
setupNestedPipelines(); add a new spec that mirrors the existing `orderBy in
includes after child collection update` test but with a nested include (status
-> tasks -> subtasks) to exercise the second branch: create
mockSyncCollectionOptions and createCollection for statuses, tasks, and
subtasks, build a live query via createLiveQueryCollection that selects statuses
with tasks which in turn include subtasks ordered by position, preload the live
query, perform an optimistic swap of subtask positions using
subtasks.update(...), assert the in-memory order changes immediately, then use
the tasks/subtasksOptions.utils.begin/write/commit flow to simulate server
confirmation and finally assert the order remains correct; ensure test names and
helper functions mirror existing patterns (e.g., getSubtaskOrder analogous to
getTaskOrder) so setupNestedPipelines()’s nested pipeline path is exercised.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 26dee569-e9b0-41fa-9ee8-e20d3607d68d

📥 Commits

Reviewing files that changed from the base of the PR and between 4d1abde and b395264.

📒 Files selected for processing (3)

• .changeset/twelve-jars-lead.md
• packages/db/src/query/live/collection-config-builder.ts
• packages/db/tests/query/includes.test.ts