From 593818a338a1a610152611a77d9cfc8734778d31 Mon Sep 17 00:00:00 2001 From: Kevin De Porre Date: Mon, 13 Jul 2026 12:26:47 +0200 Subject: [PATCH 1/8] feat(db): republish ordered live queries on an order-only move (RFC #1623 phase 4) An `orderBy` live query that reorders its rows without changing any projected row value (an "order-only move") was swallowed by the collection's value-diff: `.values()`/`.entries()` re-sorted, but no change event fired, so subscribers kept the stale order. This is the last universal expected-fail in the cross-adapter conformance suite (issue #1601). Phase 4 of the live-query platform RFC calls for an explicit layout-revision contract rather than a forged row `update`. This does that: - The live-query flush captures the retracted side of each change and, after commit, detects an order-only move (value deep-equal, `orderByIndex` moved) and publishes a first-class empty layout-change notification via a new `CollectionChangesManager.emitLayoutChangeEvent()`. - The shared observer snapshot gains `layoutRevision`, which increments on any visible membership, ordering, or order-only-move change. All five adapters pick this up through their existing wholesale re-read, so the `order-only-move` conformance scenario is removed from UNIVERSAL_EXPECTED_FAIL and now passes on React, Vue, Svelte, Solid, and Angular. Distinct from PR #1601 (v-anton), which fixes the same bug via a forced row `update`; this uses the RFC's layout-revision approach instead. Co-Authored-By: Claude Opus 4.8 --- .changeset/live-query-order-only-move.md | 13 ++ packages/db/src/collection/changes.ts | 13 ++ packages/db/src/live-query-observer.ts | 21 +++ .../query/live/collection-config-builder.ts | 43 +++++++ packages/db/src/query/live/types.ts | 6 + packages/db/tests/conformance/suite.ts | 2 +- .../tests/live-query-order-only-move.test.ts | 120 ++++++++++++++++++ 7 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 .changeset/live-query-order-only-move.md create mode 100644 packages/db/tests/live-query-order-only-move.test.ts diff --git a/.changeset/live-query-order-only-move.md b/.changeset/live-query-order-only-move.md new file mode 100644 index 0000000000..b391621da3 --- /dev/null +++ b/.changeset/live-query-order-only-move.md @@ -0,0 +1,13 @@ +--- +'@tanstack/db': patch +--- + +fix(db): republish ordered live queries on an order-only move + +An `orderBy` live query that reordered its rows without changing any projected +row value (an "order-only move") previously emitted nothing, so `useLiveQuery` +kept rendering the stale order. The live-query collection now publishes an +explicit layout-change notification when this happens, and the shared live-query +observer snapshot exposes a `layoutRevision` that increments on any visible +membership, ordering, or order-only-move change. All five framework adapters +pick this up via their existing wholesale re-read. diff --git a/packages/db/src/collection/changes.ts b/packages/db/src/collection/changes.ts index dc07cd3f18..dc79a04437 100644 --- a/packages/db/src/collection/changes.ts +++ b/packages/db/src/collection/changes.ts @@ -60,6 +60,19 @@ export class CollectionChangesManager< } } + /** + * Notify subscribers that the visible layout (row order) changed without any + * row value changing — e.g. an order-only move in an `orderBy` live query. + * Emits an empty batch directly (bypassing the empty-array check) so ordered + * consumers re-read the now re-sorted collection. This is a first-class layout + * signal, deliberately not a forged row `update`. + */ + public emitLayoutChangeEvent(): void { + for (const subscription of this.changeSubscriptions) { + subscription.emitEvents([]) + } + } + /** * Enriches a change message with virtual properties ($synced, $origin, $key, $collectionId). * Uses the "add-if-missing" pattern to preserve virtual properties from upstream collections. diff --git a/packages/db/src/live-query-observer.ts b/packages/db/src/live-query-observer.ts index 8387b38381..44fe2df183 100644 --- a/packages/db/src/live-query-observer.ts +++ b/packages/db/src/live-query-observer.ts @@ -22,6 +22,14 @@ export interface LiveQuerySnapshot< data: T | ReadonlyArray | undefined /** The underlying collection, or `undefined` when disabled. */ collection: Collection | undefined + /** + * Monotonic counter bumped whenever the visible layout changes — membership, + * ordering, or an order-only move. Lets consumers detect a reorder that + * changed no row value (which `data`/`state` identity alone can't express on + * its own once row values are structurally shared). Increments in lockstep + * with snapshot identity for enabled queries. + */ + layoutRevision: number status: CollectionStatus | `disabled` isLoading: boolean isReady: boolean @@ -70,6 +78,7 @@ const DISABLED_SNAPSHOT: LiveQuerySnapshot = { state: undefined, data: undefined, collection: undefined, + layoutRevision: 0, status: `disabled`, isLoading: false, isReady: true, @@ -89,6 +98,8 @@ class LiveQueryObserverImpl< private cachedVersion = -1 private cachedStatus: CollectionStatus | undefined private cachedSnapshot: LiveQuerySnapshot = DISABLED_SNAPSHOT + private layoutRevision = 0 + private lastLayoutSignature: string | undefined private readonly listeners = new Set>() private collectionUnsub: (() => void) | null = null // Bumped on each attach. `onFirstReady` can't be unsubscribed, so a callback @@ -124,6 +135,15 @@ class LiveQueryObserverImpl< let stateCache: Map | null = null let dataCache: Array | null = null + // Bump the layout revision when the ordered key sequence changed — + // membership, ordering, or an order-only move all shift it. `\u0000` + // can't appear in a stringified key, so it's a safe separator. + const layoutSignature = entries.map(([key]) => String(key)).join(`\u0000`) + if (layoutSignature !== this.lastLayoutSignature) { + this.lastLayoutSignature = layoutSignature + this.layoutRevision++ + } + this.cachedSnapshot = { get state() { if (!stateCache) stateCache = new Map(entries) @@ -134,6 +154,7 @@ class LiveQueryObserverImpl< return singleResult ? dataCache[0] : dataCache }, collection, + layoutRevision: this.layoutRevision, status: collection.status, ...getLiveQueryStatusFlags(collection.status), isEnabled: true, diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index a6a51b4788..faf09baa59 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -11,6 +11,7 @@ import { } from '../../errors.js' import { transactionScopedScheduler } from '../../scheduler.js' import { getActiveTransaction } from '../../transactions.js' +import { deepEquals } from '../../utils.js' import { CollectionSubscriber } from './collection-subscriber.js' import { getCollectionBuilder } from './collection-registry.js' import { LIVE_QUERY_INTERNAL } from './internal.js' @@ -799,6 +800,11 @@ export class CollectionConfigBuilder< existing.orderByIndex = changes.orderByIndex } } + // Keep the retracted (old) side for order-only-move detection. + if (changes.deletes > 0) { + existing.previousValue = changes.previousValue + existing.previousOrderByIndex = changes.previousOrderByIndex + } } else { merged.set(customKey, { ...changes }) } @@ -811,6 +817,17 @@ export class CollectionConfigBuilder< begin() changesToApply.forEach(this.applyChanges.bind(this, config)) commit() + // An order-only move (the row's projected value is unchanged but its + // `orderByIndex` moved) is swallowed by the collection's value-diff, so + // `commit()` emits nothing even though `.values()`/`.entries()` are now + // re-sorted. Publish an explicit layout-change notification so ordered + // consumers re-read — a first-class signal, not a forged row `update`. + if (hasOrderOnlyMove(changesToApply)) { + const changesManager = (config.collection as any)._changes as { + emitLayoutChangeEvent: () => void + } + changesManager.emitLayoutChangeEvent() + } } pendingChanges = new Map() @@ -2315,6 +2332,10 @@ function accumulateChanges( } if (multiplicity < 0) { changes.deletes += Math.abs(multiplicity) + // Remember the retracted (old) value + position so the flush can tell an + // order-only move apart from a real value change. + changes.previousValue = value + changes.previousOrderByIndex = orderByIndex } else if (multiplicity > 0) { changes.inserts += multiplicity // Update value to the latest version for this key @@ -2326,3 +2347,25 @@ function accumulateChanges( acc.set(key, changes) return acc } + +/** + * Detect whether any accumulated change is an "order-only move": a row that was + * updated in place (both retracted and re-inserted) whose `orderByIndex` moved + * but whose projected value is deep-equal to before. The collection's value-diff + * emits nothing for these, so the flush must publish a layout notification. + * Rows whose value actually changed are ignored — they emit a normal `update`. + */ +function hasOrderOnlyMove(changesToApply: Map>): boolean { + for (const changes of changesToApply.values()) { + if ( + changes.inserts > 0 && + changes.deletes > 0 && + changes.orderByIndex !== changes.previousOrderByIndex && + changes.previousValue !== undefined && + deepEquals(changes.previousValue, changes.value) + ) { + return true + } + } + return false +} diff --git a/packages/db/src/query/live/types.ts b/packages/db/src/query/live/types.ts index 118015bd6f..307397698e 100644 --- a/packages/db/src/query/live/types.ts +++ b/packages/db/src/query/live/types.ts @@ -16,6 +16,12 @@ export type Changes = { inserts: number value: T orderByIndex: string | undefined + // Captured from the retract side of a change so the flush can detect an + // "order-only move": a row whose projected value is unchanged but whose + // `orderByIndex` moved. Such a move is swallowed by the collection's + // value-diff, so it needs an explicit layout notification. + previousValue?: T + previousOrderByIndex?: string | undefined } export type SyncState = { diff --git a/packages/db/tests/conformance/suite.ts b/packages/db/tests/conformance/suite.ts index ba4c53360b..4f612f4ebf 100644 --- a/packages/db/tests/conformance/suite.ts +++ b/packages/db/tests/conformance/suite.ts @@ -39,7 +39,7 @@ const ISSUES: Array = [ ] /** Keys that are expected to fail on ALL adapters (core gaps, not adapter drift). */ -const UNIVERSAL_EXPECTED_FAIL = new Set([`order-only-move`]) +const UNIVERSAL_EXPECTED_FAIL = new Set([]) export function runSuite(rawDriver: LiveQueryDriver) { const { ops } = rawDriver diff --git a/packages/db/tests/live-query-order-only-move.test.ts b/packages/db/tests/live-query-order-only-move.test.ts new file mode 100644 index 0000000000..a3362f1161 --- /dev/null +++ b/packages/db/tests/live-query-order-only-move.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from 'vitest' +import { createCollection } from '../src/collection/index.js' +import { createLiveQueryCollection } from '../src/query/live-query-collection.js' +import { createLiveQueryObserver } from '../src/live-query-observer.js' +import { mockSyncCollectionOptions } from './utils.js' + +interface Person { + id: string + name: string + age: number +} + +const SEED: Array = [ + { id: `1`, name: `Alice`, age: 30 }, + { id: `2`, name: `Bob`, age: 20 }, + { id: `3`, name: `Carol`, age: 40 }, +] + +let seq = 0 +function makeSource(data: Array = SEED) { + return createCollection( + mockSyncCollectionOptions({ + id: `order-only-move-${seq++}`, + getKey: (p) => p.id, + initialData: data, + }), + ) +} + +/** Live query ordered by `age` (NOT projected), selecting only `{ id, name }`. */ +async function makeOrderedByAge(source: ReturnType) { + const lq = createLiveQueryCollection((q) => + q + .from({ p: source }) + .orderBy(({ p }) => p.age, `asc`) + .select(({ p }) => ({ id: p.id, name: p.name })), + ) + await lq.preload() + return lq +} + +const flush = () => new Promise((r) => setTimeout(r, 0)) + +describe(`order-only move (RFC #1623 phase 4)`, () => { + it(`republishes the ordered result when a row moves but its value is unchanged`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver<{ id: string; name: string }, string>( + lq as any, + ) + + let notifications = 0 + observer.subscribe(() => { + notifications++ + }) + + const before = observer.getSnapshot() + expect((before.data as Array).map((r) => r.id)).toEqual([`2`, `1`, `3`]) + const revBefore = before.layoutRevision + + // Move Bob (age 20 -> 99) to the end. The projected `{ id, name }` is + // identical, so the collection's value-diff emits no row change — only the + // layout notification should republish the new order. + source.utils.begin() + source.utils.write({ type: `update`, value: { id: `2`, name: `Bob`, age: 99 } }) + source.utils.commit() + await flush() + + const after = observer.getSnapshot() + expect((after.data as Array).map((r) => r.id)).toEqual([`1`, `3`, `2`]) + expect(after.layoutRevision).toBeGreaterThan(revBefore) + expect(notifications).toBeGreaterThan(0) + observer.dispose() + }) + + it(`does not bump the layout revision when nothing about the layout changes`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver<{ id: string; name: string }, string>( + lq as any, + ) + observer.subscribe(() => {}) + + const revBefore = observer.getSnapshot().layoutRevision + + // Update a row's `age` in a way that keeps its sort position (20 -> 21, + // still the youngest) and does not change the projected value. + source.utils.begin() + source.utils.write({ type: `update`, value: { id: `2`, name: `Bob`, age: 21 } }) + source.utils.commit() + await flush() + + // Order is unchanged (`2` still first), so the layout revision is stable. + const after = observer.getSnapshot() + expect((after.data as Array).map((r) => r.id)).toEqual([`2`, `1`, `3`]) + expect(after.layoutRevision).toBe(revBefore) + observer.dispose() + }) + + it(`bumps the layout revision on membership changes too`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver<{ id: string; name: string }, string>( + lq as any, + ) + observer.subscribe(() => {}) + + const revBefore = observer.getSnapshot().layoutRevision + + source.utils.begin() + source.utils.write({ type: `insert`, value: { id: `4`, name: `Dan`, age: 10 } }) + source.utils.commit() + await flush() + + const after = observer.getSnapshot() + expect((after.data as Array).map((r) => r.id)).toEqual([`4`, `2`, `1`, `3`]) + expect(after.layoutRevision).toBeGreaterThan(revBefore) + observer.dispose() + }) +}) From 2ea8f1ed86c78d49947fb383f8435536b053ea0c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 13 Jul 2026 10:28:23 +0000 Subject: [PATCH 2/8] ci: apply automated fixes --- .../query/live/collection-config-builder.ts | 4 +- .../tests/live-query-order-only-move.test.ts | 49 +++++++++++++------ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index faf09baa59..49ad01e772 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -2355,7 +2355,9 @@ function accumulateChanges( * emits nothing for these, so the flush must publish a layout notification. * Rows whose value actually changed are ignored — they emit a normal `update`. */ -function hasOrderOnlyMove(changesToApply: Map>): boolean { +function hasOrderOnlyMove( + changesToApply: Map>, +): boolean { for (const changes of changesToApply.values()) { if ( changes.inserts > 0 && diff --git a/packages/db/tests/live-query-order-only-move.test.ts b/packages/db/tests/live-query-order-only-move.test.ts index a3362f1161..0b9d24562e 100644 --- a/packages/db/tests/live-query-order-only-move.test.ts +++ b/packages/db/tests/live-query-order-only-move.test.ts @@ -45,9 +45,10 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { it(`republishes the ordered result when a row moves but its value is unchanged`, async () => { const source = makeSource() const lq = await makeOrderedByAge(source) - const observer = createLiveQueryObserver<{ id: string; name: string }, string>( - lq as any, - ) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) let notifications = 0 observer.subscribe(() => { @@ -55,14 +56,21 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { }) const before = observer.getSnapshot() - expect((before.data as Array).map((r) => r.id)).toEqual([`2`, `1`, `3`]) + expect((before.data as Array).map((r) => r.id)).toEqual([ + `2`, + `1`, + `3`, + ]) const revBefore = before.layoutRevision // Move Bob (age 20 -> 99) to the end. The projected `{ id, name }` is // identical, so the collection's value-diff emits no row change — only the // layout notification should republish the new order. source.utils.begin() - source.utils.write({ type: `update`, value: { id: `2`, name: `Bob`, age: 99 } }) + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 99 }, + }) source.utils.commit() await flush() @@ -76,9 +84,10 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { it(`does not bump the layout revision when nothing about the layout changes`, async () => { const source = makeSource() const lq = await makeOrderedByAge(source) - const observer = createLiveQueryObserver<{ id: string; name: string }, string>( - lq as any, - ) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) observer.subscribe(() => {}) const revBefore = observer.getSnapshot().layoutRevision @@ -86,7 +95,10 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { // Update a row's `age` in a way that keeps its sort position (20 -> 21, // still the youngest) and does not change the projected value. source.utils.begin() - source.utils.write({ type: `update`, value: { id: `2`, name: `Bob`, age: 21 } }) + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 21 }, + }) source.utils.commit() await flush() @@ -100,20 +112,29 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { it(`bumps the layout revision on membership changes too`, async () => { const source = makeSource() const lq = await makeOrderedByAge(source) - const observer = createLiveQueryObserver<{ id: string; name: string }, string>( - lq as any, - ) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) observer.subscribe(() => {}) const revBefore = observer.getSnapshot().layoutRevision source.utils.begin() - source.utils.write({ type: `insert`, value: { id: `4`, name: `Dan`, age: 10 } }) + source.utils.write({ + type: `insert`, + value: { id: `4`, name: `Dan`, age: 10 }, + }) source.utils.commit() await flush() const after = observer.getSnapshot() - expect((after.data as Array).map((r) => r.id)).toEqual([`4`, `2`, `1`, `3`]) + expect((after.data as Array).map((r) => r.id)).toEqual([ + `4`, + `2`, + `1`, + `3`, + ]) expect(after.layoutRevision).toBeGreaterThan(revBefore) observer.dispose() }) From d84fb7b8d13139517943606b6174f2d6aa3a1e9c Mon Sep 17 00:00:00 2001 From: Kevin De Porre Date: Mon, 13 Jul 2026 16:51:02 +0200 Subject: [PATCH 3/8] refactor(db): compare key sequence directly for layoutRevision + fix doc Addresses independent review of the layoutRevision contract: - The join-with-separator signature could collide: a key value equal to the concatenation of neighboring keys around the separator produces the same string as two separate keys, so a real layout change (a membership change whose combined key spans the separator) was missed. Compare the ordered key sequence directly instead - collision-free, and it avoids materializing a large string on every snapshot rebuild (a new key array is only allocated when the layout actually moved). Adds a regression test. - Correct the layoutRevision doc comment: it is NOT in lockstep with snapshot identity (a value-only update yields a new snapshot but the same layoutRevision). Co-Authored-By: Claude Opus 4.8 --- packages/db/src/live-query-observer.ts | 41 +++++++++++++------ packages/db/tests/live-query-observer.test.ts | 23 +++++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/packages/db/src/live-query-observer.ts b/packages/db/src/live-query-observer.ts index 44fe2df183..cd742a7d5b 100644 --- a/packages/db/src/live-query-observer.ts +++ b/packages/db/src/live-query-observer.ts @@ -23,11 +23,14 @@ export interface LiveQuerySnapshot< /** The underlying collection, or `undefined` when disabled. */ collection: Collection | undefined /** - * Monotonic counter bumped whenever the visible layout changes — membership, - * ordering, or an order-only move. Lets consumers detect a reorder that - * changed no row value (which `data`/`state` identity alone can't express on - * its own once row values are structurally shared). Increments in lockstep - * with snapshot identity for enabled queries. + * Monotonic counter bumped whenever the visible layout (the ordered key + * sequence) changes — membership, ordering, or an order-only move. Lets + * consumers detect a reorder that changed no row value (which `data`/`state` + * identity alone can't express once row values are structurally shared). + * + * It is NOT in lockstep with snapshot identity: a value-only update produces a + * new snapshot while `layoutRevision` stays put. A `layoutRevision` change + * always accompanies a new snapshot, but not vice versa. */ layoutRevision: number status: CollectionStatus | `disabled` @@ -99,7 +102,7 @@ class LiveQueryObserverImpl< private cachedStatus: CollectionStatus | undefined private cachedSnapshot: LiveQuerySnapshot = DISABLED_SNAPSHOT private layoutRevision = 0 - private lastLayoutSignature: string | undefined + private lastLayoutKeys: Array | undefined private readonly listeners = new Set>() private collectionUnsub: (() => void) | null = null // Bumped on each attach. `onFirstReady` can't be unsubscribed, so a callback @@ -135,12 +138,26 @@ class LiveQueryObserverImpl< let stateCache: Map | null = null let dataCache: Array | null = null - // Bump the layout revision when the ordered key sequence changed — - // membership, ordering, or an order-only move all shift it. `\u0000` - // can't appear in a stringified key, so it's a safe separator. - const layoutSignature = entries.map(([key]) => String(key)).join(`\u0000`) - if (layoutSignature !== this.lastLayoutSignature) { - this.lastLayoutSignature = layoutSignature + // Bump the layout revision when the ordered key sequence changes + // (membership, ordering, or an order-only move). Compare the key sequence + // directly rather than via a serialized signature: a joined-with-separator + // signature can collide when a key value equals the concatenation of + // neighboring keys around the separator. Comparing keys also avoids + // materializing a large string on every rebuild; a new key array is only + // allocated when the layout actually moved. + const prevKeys = this.lastLayoutKeys + let layoutChanged = + prevKeys === undefined || prevKeys.length !== entries.length + if (!layoutChanged) { + for (let i = 0; i < entries.length; i++) { + if (prevKeys![i] !== entries[i]![0]) { + layoutChanged = true + break + } + } + } + if (layoutChanged) { + this.lastLayoutKeys = entries.map(([key]) => key) this.layoutRevision++ } diff --git a/packages/db/tests/live-query-observer.test.ts b/packages/db/tests/live-query-observer.test.ts index 1035a11ef8..7ec98ff257 100644 --- a/packages/db/tests/live-query-observer.test.ts +++ b/packages/db/tests/live-query-observer.test.ts @@ -189,4 +189,27 @@ describe(`createLiveQueryObserver`, () => { expect(observer.getSnapshot().status).toBe(`ready`) observer.dispose() }) + + it(`bumps layoutRevision on a membership change that a joined key signature would collide on`, () => { + // Two keys "a","b" vs a single key "a\u0000b" join to the same string under + // any separator that can appear in a key. The revision compares the key + // sequence directly, so it must still register the membership change. + const source = makeSource([ + { id: `a`, name: `A` }, + { id: `b`, name: `B` }, + ]) + const observer = createLiveQueryObserver(source as any) + observer.subscribe(() => {}) + + const revBefore = observer.getSnapshot().layoutRevision + + source.utils.begin() + source.utils.write({ type: `delete`, value: { id: `a`, name: `A` } }) + source.utils.write({ type: `delete`, value: { id: `b`, name: `B` } }) + source.utils.write({ type: `insert`, value: { id: `a\u0000b`, name: `AB` } }) + source.utils.commit() + + expect(observer.getSnapshot().layoutRevision).not.toBe(revBefore) + observer.dispose() + }) }) From d7628551f219cab27ddd542bffda40c8ee7f5aad Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 13 Jul 2026 14:52:20 +0000 Subject: [PATCH 4/8] ci: apply automated fixes --- packages/db/tests/live-query-observer.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/db/tests/live-query-observer.test.ts b/packages/db/tests/live-query-observer.test.ts index 7ec98ff257..eb07e0c3d8 100644 --- a/packages/db/tests/live-query-observer.test.ts +++ b/packages/db/tests/live-query-observer.test.ts @@ -206,7 +206,10 @@ describe(`createLiveQueryObserver`, () => { source.utils.begin() source.utils.write({ type: `delete`, value: { id: `a`, name: `A` } }) source.utils.write({ type: `delete`, value: { id: `b`, name: `B` } }) - source.utils.write({ type: `insert`, value: { id: `a\u0000b`, name: `AB` } }) + source.utils.write({ + type: `insert`, + value: { id: `a\u0000b`, name: `AB` }, + }) source.utils.commit() expect(observer.getSnapshot().layoutRevision).not.toBe(revBefore) From 43f1a7d6f4664b745378eab8baa72e6f238dc511 Mon Sep 17 00:00:00 2001 From: Kevin De Porre Date: Wed, 15 Jul 2026 10:10:38 +0200 Subject: [PATCH 5/8] test(db): add failing regressions for Kyle's review findings Two gaps in the order-only-move handling, reproduced as failing tests (to be fixed in a follow-up commit): 1. A commit containing both an ordinary value update and an order-only move publishes twice (commit's row batch + the separate empty layout event), where exactly one publication is expected. 2. Ordered child collections produced by `includes` don't consume the insertion-side order metadata or publish a layout-only move, so an ordered child stays in its old order after a child order-only move. Co-Authored-By: Claude Opus 4.8 --- .../tests/live-query-order-only-move.test.ts | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/packages/db/tests/live-query-order-only-move.test.ts b/packages/db/tests/live-query-order-only-move.test.ts index 0b9d24562e..50a6b61109 100644 --- a/packages/db/tests/live-query-order-only-move.test.ts +++ b/packages/db/tests/live-query-order-only-move.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { createCollection } from '../src/collection/index.js' import { createLiveQueryCollection } from '../src/query/live-query-collection.js' import { createLiveQueryObserver } from '../src/live-query-observer.js' +import { eq } from '../src/query/builder/functions.js' import { mockSyncCollectionOptions } from './utils.js' interface Person { @@ -138,4 +139,103 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { expect(after.layoutRevision).toBeGreaterThan(revBefore) observer.dispose() }) + + // Kyle's review issue 1: a commit containing both an ordinary value update + // and an order-only move must publish exactly once — the ordinary publication + // already carries the final values and ordering, so the separate layout event + // is redundant. + it(`publishes a mixed value update and order-only move exactly once`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) + + let notifications = 0 + observer.subscribe(() => notifications++) + notifications = 0 // exclude subscribeChanges' initial-state publication + + source.utils.begin() + source.utils.write({ + type: `update`, + value: { id: `1`, name: `Alicia`, age: 30 }, + }) + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 99 }, + }) + source.utils.commit() + + const after = observer.getSnapshot() + expect( + (after.data as Array).map(({ id, name }) => [id, name]), + ).toEqual([ + [`1`, `Alicia`], + [`3`, `Carol`], + [`2`, `Bob`], + ]) + expect(notifications).toBe(1) + observer.dispose() + }) + + // Kyle's review issue 2: an ordered child collection produced by `includes` + // must consume the insertion-side order metadata and publish its move when the + // projected child value is unchanged. + it(`publishes an ordered included child move exactly once`, async () => { + const parents = createCollection( + mockSyncCollectionOptions<{ id: string }>({ + id: `order-only-parents-${seq++}`, + getKey: ({ id }) => id, + initialData: [{ id: `p1` }], + }), + ) + const children = createCollection( + mockSyncCollectionOptions<{ + id: string + parentId: string + name: string + position: number + }>({ + id: `order-only-children-${seq++}`, + getKey: ({ id }) => id, + initialData: [ + { id: `c1`, parentId: `p1`, name: `One`, position: 1 }, + { id: `c2`, parentId: `p1`, name: `Two`, position: 2 }, + ], + }), + ) + const lq = createLiveQueryCollection((q) => + q.from({ parent: parents }).select(({ parent }) => ({ + id: parent.id, + children: q + .from({ child: children }) + .where(({ child }) => eq(child.parentId, parent.id)) + .orderBy(({ child }) => child.position) + .select(({ child }) => ({ id: child.id, name: child.name })), + })), + ) + await lq.preload() + + const childCollection = (lq.get(`p1`) as any).children + let notifications = 0 + const subscription = childCollection.subscribeChanges( + () => notifications++, + { includeInitialState: false }, + ) + + children.utils.begin() + children.utils.write({ + type: `update`, + value: { id: `c1`, parentId: `p1`, name: `One`, position: 3 }, + }) + children.utils.commit() + + expect([...childCollection.values()].map(({ id }: any) => id)).toEqual([ + `c2`, + `c1`, + ]) + expect(notifications).toBe(1) + subscription.unsubscribe() + }) }) From c9ec751d83b2bc9d5b92dd344cc9743236fc6e68 Mon Sep 17 00:00:00 2001 From: Kevin De Porre Date: Wed, 15 Jul 2026 10:18:18 +0200 Subject: [PATCH 6/8] fix(db): coalesce layout publications and cover ordered includes children Addresses Kyle's review of the order-only-move handling: 1. A commit containing both an ordinary value update and an order-only move published twice: commit() emitted the row batch and then the separate layout event fired redundantly. Replace hasOrderOnlyMove with needsLayoutOnlyPublication, which fires the layout event only when the commit published nothing else (any real insert/delete/value-changed update already notifies subscribers, who re-read the re-sorted collection). 2. Ordered child collections produced by includes did not reorder on an order-only child move: - The child accumulate replaced value on the insert side but left the retracted orderByIndex, so the child collection re-sorted against a stale index. Update orderByIndex on insert and capture the retract side (both the single-level and nested-includes accumulate blocks). - The child flush committed without a layout-only publication when the projected child value was unchanged. Publish one through the same mechanism (emitLayoutChange) when the child commit published nothing else. Co-Authored-By: Claude Opus 4.8 --- .../query/live/collection-config-builder.ts | 75 ++++++++++++++----- 1 file changed, 56 insertions(+), 19 deletions(-) diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index 49ad01e772..0db2687de7 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -822,11 +822,10 @@ export class CollectionConfigBuilder< // `commit()` emits nothing even though `.values()`/`.entries()` are now // re-sorted. Publish an explicit layout-change notification so ordered // consumers re-read — a first-class signal, not a forged row `update`. - if (hasOrderOnlyMove(changesToApply)) { - const changesManager = (config.collection as any)._changes as { - emitLayoutChangeEvent: () => void - } - changesManager.emitLayoutChangeEvent() + // Only when the commit published nothing else: any real row change + // already notifies subscribers, who re-read the re-sorted collection. + if (needsLayoutOnlyPublication(changesToApply)) { + emitLayoutChange(config.collection) } } pendingChanges = new Map() @@ -910,9 +909,14 @@ export class CollectionConfigBuilder< if (multiplicity < 0) { existing.deletes += Math.abs(multiplicity) + existing.previousValue = childResult + existing.previousOrderByIndex = _orderByIndex } else if (multiplicity > 0) { existing.inserts += multiplicity existing.value = childResult + if (_orderByIndex !== undefined) { + existing.orderByIndex = _orderByIndex + } } byChild.set(childKey, existing) @@ -1337,9 +1341,14 @@ function setupNestedPipelines( if (multiplicity < 0) { existing.deletes += Math.abs(multiplicity) + existing.previousValue = childResult + existing.previousOrderByIndex = _orderByIndex } else if (multiplicity > 0) { existing.inserts += multiplicity existing.value = childResult + if (_orderByIndex !== undefined) { + existing.orderByIndex = _orderByIndex + } } byChild.set(childKey, existing) @@ -2002,6 +2011,13 @@ function flushIncludesState( } } entry.syncMethods.commit() + // Same order-only-move handling as the parent flush: a child row that + // moved without a value change is swallowed by the value-diff, so + // publish a layout-only notification when the child commit published + // nothing else. + if (needsLayoutOnlyPublication(childChanges)) { + emitLayoutChange(entry.syncMethods.collection) + } } // Update routing index for nested includes @@ -2349,25 +2365,46 @@ function accumulateChanges( } /** - * Detect whether any accumulated change is an "order-only move": a row that was - * updated in place (both retracted and re-inserted) whose `orderByIndex` moved - * but whose projected value is deep-equal to before. The collection's value-diff - * emits nothing for these, so the flush must publish a layout notification. - * Rows whose value actually changed are ignored — they emit a normal `update`. + * Decide whether a flush needs a standalone layout-change publication. + * + * An "order-only move" — a row updated in place whose `orderByIndex` moved but + * whose projected value is deep-equal to before — is swallowed by the value-diff + * and needs an explicit layout notification. But that notification is only + * needed when the same `commit()` published nothing else: any real change + * (insert, delete, or a value-changed update) already notifies subscribers, who + * re-read the re-sorted collection, so a second layout event would be redundant. + * + * Returns true iff there is at least one order-only move AND no change that the + * commit itself publishes. */ -function hasOrderOnlyMove( +function needsLayoutOnlyPublication( changesToApply: Map>, ): boolean { + let layoutMoved = false + let commitPublishes = false for (const changes of changesToApply.values()) { - if ( - changes.inserts > 0 && - changes.deletes > 0 && - changes.orderByIndex !== changes.previousOrderByIndex && - changes.previousValue !== undefined && - deepEquals(changes.previousValue, changes.value) + const isUpdate = changes.inserts > 0 && changes.deletes > 0 + if (!isUpdate) { + // A net insert or delete always publishes a row change. + commitPublishes = true + } else if ( + changes.previousValue === undefined || + !deepEquals(changes.previousValue, changes.value) ) { - return true + // In-place update whose value changed — publishes a normal `update`. + commitPublishes = true + } else if (changes.orderByIndex !== changes.previousOrderByIndex) { + // Value unchanged, position moved — swallowed by the value-diff. + layoutMoved = true } } - return false + return layoutMoved && !commitPublishes +} + +/** Emit a layout-only change notification on a live-query collection. */ +function emitLayoutChange(collection: Collection): void { + const changesManager = (collection as any)._changes as { + emitLayoutChangeEvent: () => void + } + changesManager.emitLayoutChangeEvent() } From ff339d8c3bf297827e6fd67d2b7a7c39c2b9bc29 Mon Sep 17 00:00:00 2001 From: Kevin De Porre Date: Wed, 15 Jul 2026 10:34:54 +0200 Subject: [PATCH 7/8] test(db): guard order-only moves in deeply-nested ordered includes The includes flush is recursive, so the order-only-move handling must hold beyond one level. Adds a two-level ordered-includes regression (org -> teams -> members): moving a grandchild whose projected value is unchanged must re-sort its collection and publish exactly once. Verified red when the child-flush layout publication is removed. Co-Authored-By: Claude Opus 4.8 --- .../tests/live-query-order-only-move.test.ts | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/packages/db/tests/live-query-order-only-move.test.ts b/packages/db/tests/live-query-order-only-move.test.ts index 50a6b61109..442ec77fe7 100644 --- a/packages/db/tests/live-query-order-only-move.test.ts +++ b/packages/db/tests/live-query-order-only-move.test.ts @@ -238,4 +238,83 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { expect(notifications).toBe(1) subscription.unsubscribe() }) + + // The includes flush is recursive, so the order-only-move handling must hold + // at depth, not just one level. Two levels of ordered includes + // (org -> teams -> members); move a grandchild whose projected value is + // unchanged and assert its collection re-sorts and publishes exactly once. + it(`publishes an ordered move in a deeply-nested included child exactly once`, async () => { + const orgs = createCollection( + mockSyncCollectionOptions<{ id: string }>({ + id: `order-only-orgs-${seq++}`, + getKey: ({ id }) => id, + initialData: [{ id: `o1` }], + }), + ) + const teams = createCollection( + mockSyncCollectionOptions<{ id: string; orgId: string; position: number }>( + { + id: `order-only-teams-${seq++}`, + getKey: ({ id }) => id, + initialData: [{ id: `t1`, orgId: `o1`, position: 1 }], + }, + ), + ) + const members = createCollection( + mockSyncCollectionOptions<{ + id: string + teamId: string + name: string + position: number + }>({ + id: `order-only-members-${seq++}`, + getKey: ({ id }) => id, + initialData: [ + { id: `m1`, teamId: `t1`, name: `One`, position: 1 }, + { id: `m2`, teamId: `t1`, name: `Two`, position: 2 }, + ], + }), + ) + const lq = createLiveQueryCollection((q) => + q.from({ org: orgs }).select(({ org }) => ({ + id: org.id, + teams: q + .from({ team: teams }) + .where(({ team }) => eq(team.orgId, org.id)) + .orderBy(({ team }) => team.position) + .select(({ team }) => ({ + id: team.id, + members: q + .from({ member: members }) + .where(({ member }) => eq(member.teamId, team.id)) + .orderBy(({ member }) => member.position) + .select(({ member }) => ({ id: member.id, name: member.name })), + })), + })), + ) + await lq.preload() + + const teamCollection = (lq.get(`o1`) as any).teams + const memberCollection = (teamCollection.get(`t1`)).members + let notifications = 0 + const subscription = memberCollection.subscribeChanges( + () => notifications++, + { includeInitialState: false }, + ) + + // Move m1 behind m2 (position 1 -> 3); projected { id, name } unchanged. + members.utils.begin() + members.utils.write({ + type: `update`, + value: { id: `m1`, teamId: `t1`, name: `One`, position: 3 }, + }) + members.utils.commit() + + expect([...memberCollection.values()].map(({ id }: any) => id)).toEqual([ + `m2`, + `m1`, + ]) + expect(notifications).toBe(1) + subscription.unsubscribe() + }) }) From 4634ea5d2a69efb5f6b8d854c8009faa43029127 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 15 Jul 2026 08:36:41 +0000 Subject: [PATCH 8/8] ci: apply automated fixes --- .../tests/live-query-order-only-move.test.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/db/tests/live-query-order-only-move.test.ts b/packages/db/tests/live-query-order-only-move.test.ts index 442ec77fe7..a4fb2700ff 100644 --- a/packages/db/tests/live-query-order-only-move.test.ts +++ b/packages/db/tests/live-query-order-only-move.test.ts @@ -252,13 +252,15 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { }), ) const teams = createCollection( - mockSyncCollectionOptions<{ id: string; orgId: string; position: number }>( - { - id: `order-only-teams-${seq++}`, - getKey: ({ id }) => id, - initialData: [{ id: `t1`, orgId: `o1`, position: 1 }], - }, - ), + mockSyncCollectionOptions<{ + id: string + orgId: string + position: number + }>({ + id: `order-only-teams-${seq++}`, + getKey: ({ id }) => id, + initialData: [{ id: `t1`, orgId: `o1`, position: 1 }], + }), ) const members = createCollection( mockSyncCollectionOptions<{ @@ -295,7 +297,7 @@ describe(`order-only move (RFC #1623 phase 4)`, () => { await lq.preload() const teamCollection = (lq.get(`o1`) as any).teams - const memberCollection = (teamCollection.get(`t1`)).members + const memberCollection = teamCollection.get(`t1`).members let notifications = 0 const subscription = memberCollection.subscribeChanges( () => notifications++,