From b81f680ca0d9f2646f3e803673aa6a5e7df5d93f Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 08:10:46 -0600 Subject: [PATCH 1/9] perf(db): order nullish keys and restore large sync batches by merge - SortedMap orders runtime undefined/null keys first, so binary search finds them; this removes the runtimeKeyCount immediate-path fallback. - Deferred restore removes dirty keys by membership and merges the sorted dirty keys in one pass (O(n + k log n)) instead of re-sorting every key. - Remove SortedMap's unused forEach and its public export. - Keyed MultiSet.consolidate uses nested Maps instead of string ids. - Collection commit loop drops a never-read set and computes virtual props only for deletes and updates. - Add a named load-subset replay for the failed include demand trace. Co-authored-by: Isaac --- docs/contributing/oracle-coverage.md | 2 +- docs/reference/classes/SortedMap.md | 318 ------------------ docs/reference/index.md | 1 - packages/db-ivm/src/multiset.ts | 55 ++- packages/db-ivm/src/utils.ts | 20 -- packages/db/src/SortedMap.ts | 154 +++++---- packages/db/src/collection/state.ts | 32 +- packages/db/src/index.ts | 1 - packages/db/tests/SortedMap-oracle.test.ts | 272 ++++++++++++--- ...ad-subset-replay-refinement-oracle.test.ts | 19 ++ 10 files changed, 385 insertions(+), 489 deletions(-) delete mode 100644 docs/reference/classes/SortedMap.md diff --git a/docs/contributing/oracle-coverage.md b/docs/contributing/oracle-coverage.md index 347a6b065a..160a493795 100644 --- a/docs/contributing/oracle-coverage.md +++ b/docs/contributing/oracle-coverage.md @@ -1064,7 +1064,7 @@ generic visitor behavior outside the named collectors. | Index suggestions | [index suggestion oracle](https://github.com/TanStack/db/blob/main/packages/db/tests/index-suggestion-oracle.test.ts) | A public filtered live-query Collection checks manual/default, eager, threshold, matching-index, and unrelated-index cases against the documented collection-size suggestion policy. A bounded ordered-query decision table separately classifies no field index, a null-placement mismatch, matching but order-ineligible indexes, and usable candidates in either registration order. It compares the development warning after preload; the matching-index selection owner below checks the chosen reader and ordered request. Two cyclic locale-options cells require the diagnostic to preserve valid query construction with or without a field index, and a custom-collation cell requires remediation to preserve comparator reference identity. The original manual-silence/eager-noise implementation failed two expected observations. The issue #2190 baseline failed four diagnostic cells, including a first-unusable/second-usable candidate order; the first formatter draft failed all three non-JSON-safe remediation cells. The owner does not judge repeated-query warning volume, slow-query timing, production-build suppression, every comparison-option mismatch, or every query shape. It runs in `@tanstack/db`'s `test:oracles` campaign. | | Minified DB public API | [consumer bundle check](https://github.com/TanStack/db/blob/main/scripts/test-minified-db.mjs) | CI builds `@tanstack/db`, then bundles its built `dist` entry with db-ivm inlined through esbuild `minify: true`. The build renames TypeScript-private members from `packages/db/mangle-cache.json`, so this lane runs against the renamed output that consumers install. It rejects a `dist` that is older than `src`, the cache, or the package's build configuration. `pnpm --filter @tanstack/db test:dist` also runs five db test files against the built `dist`, chosen for coverage of the modules with the most renamed members. `pnpm check:mangle` fails when a cached name is used other than as a private member in db src, is read by another package, appears as a string, or has a short name that collides with a source identifier ([private-member mangling review](oracle-reviews/code-weight-private-member-mangling.md)). It checks every exported error class's current public `name`, the built-in `BasicIndex` resolver name in index metadata and its event, complete query rows (after removing the four documented virtual fields) against an independent array filter/sort/projection, and one live update. The `new.target.name`, public-member mangle, and extra-output-field calibration modes fail at the intended observations. This fixed slice does not replace the unminified generated oracles, exercise framework adapters, or establish stable names for custom index resolvers. | | Boundary refinements | [cleanup/restart](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-cleanup-restart-oracle.test.ts), [idle cleanup](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-idle-cleanup.test.ts), [issue #1891 async-cleanup review](oracle-reviews/issue-1891-async-cleanup.md), [issue #1891 cleanup-start follow-up](oracle-reviews/issue-1891-cleanup-start-followup.md), [metadata publication](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-metadata-publication-oracle.property.test.ts), [row metadata composition](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-row-metadata-composition-oracle.test.ts), [nested-begin metadata rebuild review](oracle-reviews/2026-10-06-nested-begin-metadata-rebuild.md), [state mutation round 4](oracle-reviews/2026-10-06-state-mutation-round-4.md), [state retention](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-state-retention-oracle.property.test.ts), [acquisition cells](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-subscription-lifecycle-oracle.test.ts), [D2 source reconciliation](https://github.com/TanStack/db/blob/main/packages/db/tests/d2-source-reconciliation-oracle.property.test.ts), [top-K support windows](https://github.com/TanStack/db/blob/main/packages/db-ivm/tests/operators/topk-support-window-oracle.test.ts), [nested Query work](https://github.com/TanStack/db/blob/main/packages/query-db-collection/tests/includes-work-counter-oracle.test.ts) | Explicit lifecycle products, independent source maps and weighted relations, exact publication cuts, support/multiplicity, and value-plus-work observations. These refine the larger subsystem models; they do not replace them. | -| Small structures and test mechanics | [SortedMap](https://github.com/TanStack/db/blob/main/packages/db/tests/SortedMap-oracle.test.ts), [cleanup queue](https://github.com/TanStack/db/blob/main/packages/db/tests/cleanup-queue-oracle.property.test.ts), [live-query GC clock](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-gc-clock.test.ts), [guarded replay](https://github.com/TanStack/db/blob/main/packages/db/tests/oracle-replay.test.ts) | Map/full-sort and elapsed appointment-list models with executed target/seed/path checks. SortedMap also checks deferred bulk writes, array-movement work, scan reuse, live iterators, runtime `undefined` keys, and public single-transaction and queued-transaction Collection sync paths. It does not settle the contract for comparator-observed values mutated after publication or every legal iterator/mutation interleaving. Wall-clock steps do not move due times, and a focused public witness checks source subscription release. Callback-reentrant scheduling, OS suspend/resume, environments without `performance.now()`, and fake/real Performance clock replacement while GC is pending remain outside this coverage. Tests must drain or reset pending GC before switching timer providers. | +| Small structures and test mechanics | [SortedMap](https://github.com/TanStack/db/blob/main/packages/db/tests/SortedMap-oracle.test.ts), [cleanup queue](https://github.com/TanStack/db/blob/main/packages/db/tests/cleanup-queue-oracle.property.test.ts), [live-query GC clock](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-gc-clock.test.ts), [guarded replay](https://github.com/TanStack/db/blob/main/packages/db/tests/oracle-replay.test.ts) | Map/full-sort and elapsed appointment-list models with executed target/seed/path checks. SortedMap promises what Collection state uses, not full Map parity (maintainer decision). The model is a Map plus a full sort; key order is total over Collection keys (`undefined`, `null`, strings, numbers). Generated histories mix deferred and immediate sets and deletes, clear, deferred bursts of 17 to 41 keys, runtime `undefined` and `null` keys, and prunes that delete entries with deferOrder while iterating, as settled transactions are pruned; a prune must yield the order it began with. Declared reach requires deletes of placed runtime keys, observations with runtime keys, burst restorations, and an entry yielded after a prune deleted one. Focused witnesses check splice movement for deferred inserts, deletes and comparator reorders, a key iterator opened before a deferred restore, rewritten or deleted rows whose value was mutated in place, scan reuse, and public single-transaction and queued-transaction Collection sync paths. The movement counter sees splices only, so it rejects per-key splicing but does not bound the restore's single filter and merge pass. Not promised: other mutation during iteration (immediate writes, clear), where a row mutated in place without a write sorts, NaN and -0 keys, a comparator that throws, and keys of other runtime types (booleans, bigints). `forEach` was removed; SortedMap is not exported. Wall-clock steps do not move due times, and a focused public witness checks source subscription release. Callback-reentrant scheduling, OS suspend/resume, environments without `performance.now()`, and fake/real Performance clock replacement while GC is pending remain outside this coverage. Tests must drain or reset pending GC before switching timer providers. | The index-suggestion oracle's PR #2196 extension checks a matching local custom-collation index against the full-source request and retained warning, diff --git a/docs/reference/classes/SortedMap.md b/docs/reference/classes/SortedMap.md deleted file mode 100644 index 714c93a2d4..0000000000 --- a/docs/reference/classes/SortedMap.md +++ /dev/null @@ -1,318 +0,0 @@ ---- -id: SortedMap -title: SortedMap ---- - -Defined in: [packages/db/src/SortedMap.ts:8](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L8) - -A Map implementation that keeps its entries sorted based on a comparator function - -## Type Parameters - -### TKey - -`TKey` *extends* `string` \| `number` - -The type of keys in the map (must be string | number) - -### TValue - -`TValue` - -The type of values in the map - -## Constructors - -### Constructor - -```ts -new SortedMap(comparator?): SortedMap; -``` - -Defined in: [packages/db/src/SortedMap.ts:21](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L21) - -Creates a new SortedMap instance - -#### Parameters - -##### comparator? - -(`a`, `b`) => `number` - -Optional function to compare values for sorting. - If not provided, entries are sorted by key only. - -#### Returns - -`SortedMap`\<`TKey`, `TValue`\> - -## Accessors - -### size - -#### Get Signature - -```ts -get size(): number; -``` - -Defined in: [packages/db/src/SortedMap.ts:201](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L201) - -Gets the number of key-value pairs in the map - -##### Returns - -`number` - -## Methods - -### \[iterator\]() - -```ts -iterator: IterableIterator<[TKey, TValue]>; -``` - -Defined in: [packages/db/src/SortedMap.ts:210](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L210) - -Default iterator that returns entries in sorted order - -#### Returns - -`IterableIterator`\<\[`TKey`, `TValue`\]\> - -An iterator for the map's entries - -*** - -### clear() - -```ts -clear(): void; -``` - -Defined in: [packages/db/src/SortedMap.ts:191](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L191) - -Removes all key-value pairs from the map - -#### Returns - -`void` - -*** - -### delete() - -```ts -delete(key, deferOrder): boolean; -``` - -Defined in: [packages/db/src/SortedMap.ts:158](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L158) - -Removes a key-value pair from the map - -#### Parameters - -##### key - -`TKey` - -The key to remove - -##### deferOrder - -`boolean` = `false` - -Defer ordering until restoreOrder or the next ordered read - -#### Returns - -`boolean` - -True if the key was found and removed, false otherwise - -*** - -### entries() - -```ts -entries(): IterableIterator<[TKey, TValue]>; -``` - -Defined in: [packages/db/src/SortedMap.ts:222](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L222) - -Returns an iterator for the map's entries in sorted order - -#### Returns - -`IterableIterator`\<\[`TKey`, `TValue`\]\> - -An iterator for the map's entries - -*** - -### forEach() - -```ts -forEach(callbackfn): void; -``` - -Defined in: [packages/db/src/SortedMap.ts:253](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L253) - -Executes a callback function for each key-value pair in the map in sorted order - -#### Parameters - -##### callbackfn - -(`value`, `key`, `map`) => `void` - -Function to execute for each entry - -#### Returns - -`void` - -*** - -### get() - -```ts -get(key): TValue | undefined; -``` - -Defined in: [packages/db/src/SortedMap.ts:147](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L147) - -Gets a value by its key - -#### Parameters - -##### key - -`TKey` - -The key to look up - -#### Returns - -`TValue` \| `undefined` - -The value associated with the key, or undefined if not found - -*** - -### has() - -```ts -has(key): boolean; -``` - -Defined in: [packages/db/src/SortedMap.ts:184](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L184) - -Checks if a key exists in the map - -#### Parameters - -##### key - -`TKey` - -The key to check - -#### Returns - -`boolean` - -True if the key exists, false otherwise - -*** - -### keys() - -```ts -keys(): IterableIterator; -``` - -Defined in: [packages/db/src/SortedMap.ts:231](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L231) - -Returns an iterator for the map's keys in sorted order - -#### Returns - -`IterableIterator`\<`TKey`\> - -An iterator for the map's keys - -*** - -### restoreOrder() - -```ts -restoreOrder(): void; -``` - -Defined in: [packages/db/src/SortedMap.ts:88](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L88) - -Restore the ordered view after a synchronous batch of deferred writes. - -#### Returns - -`void` - -*** - -### set() - -```ts -set( - key, - value, - deferOrder): this; -``` - -Defined in: [packages/db/src/SortedMap.ts:108](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L108) - -Sets a key-value pair in the map and maintains sort order - -#### Parameters - -##### key - -`TKey` - -The key to set - -##### value - -`TValue` - -The value to associate with the key - -##### deferOrder - -`boolean` = `false` - -Defer ordering until restoreOrder or the next ordered read - -#### Returns - -`this` - -This SortedMap instance for chaining - -*** - -### values() - -```ts -values(): IterableIterator; -``` - -Defined in: [packages/db/src/SortedMap.ts:241](https://github.com/TanStack/db/blob/main/packages/db/src/SortedMap.ts#L241) - -Returns an iterator for the map's values in sorted order - -#### Returns - -`IterableIterator`\<`TValue`\> - -An iterator for the map's values diff --git a/docs/reference/index.md b/docs/reference/index.md index da78b354f4..52ee991c11 100644 --- a/docs/reference/index.md +++ b/docs/reference/index.md @@ -99,7 +99,6 @@ title: "@tanstack/db" - [SetWindowFindOneLimitError](classes/SetWindowFindOneLimitError.md) - [SetWindowReentrancyError](classes/SetWindowReentrancyError.md) - [SetWindowRequiresOrderByError](classes/SetWindowRequiresOrderByError.md) -- [SortedMap](classes/SortedMap.md) - [SourceCollectionIdReuseError](classes/SourceCollectionIdReuseError.md) - [StorageError](classes/StorageError.md) - [StorageKeyRequiredError](classes/StorageKeyRequiredError.md) diff --git a/packages/db-ivm/src/multiset.ts b/packages/db-ivm/src/multiset.ts index 68448ec37b..feb8904598 100644 --- a/packages/db-ivm/src/multiset.ts +++ b/packages/db-ivm/src/multiset.ts @@ -1,4 +1,4 @@ -import { chunkedArrayPush, getStringId } from './utils.js' +import { chunkedArrayPush } from './utils.js' import { hash } from './hashing/index.js' export type MultiSetArray = Array<[T, number]> @@ -94,21 +94,7 @@ export class MultiSet { (typeof data[0] === `string` || typeof data[0] === `number`), ) ) { - const referenceIds = new Map() - return consolidateBy(inner, (data) => { - const [key, value] = data as [string | number, unknown] - // Length prefixes keep user text from crossing component boundaries. - let valueId: string - if (Array.isArray(value) && value.length === 2) { - const firstId = getStringId(value[0], referenceIds) - const secondId = getStringId(value[1], referenceIds) - valueId = `tuple_${firstId.length}:${firstId}${secondId}` - } else { - valueId = `leaf_${getStringId(value, referenceIds)}` - } - const keyId = getStringId(key, referenceIds) - return `${keyId.length}:${keyId}${valueId}` - }) + return consolidateKeyed(inner) } // Unkeyed data of a single primitive type is its own identity, and the @@ -142,6 +128,43 @@ export class MultiSet { } } +/** + * Consolidates `[key, value]` records. `Map` keys compare by SameValueZero, + * which is the identity the keyed rule asks for: primitives by value (`-0` + * equals `0`, `NaN` equals `NaN`) and objects by reference. A join tuple is + * looked up element by element, and leaves and tuples use separate maps. + */ +function consolidateKeyed(inner: MultiSetArray): MultiSet { + const leaves = new Map>() + const tuples = new Map>>() + const entries: MultiSetArray = [] + for (const [data, multiplicity] of inner) { + const [key, value] = data as [string | number, unknown] + let entry: [T, number] | undefined + if (Array.isArray(value) && value.length === 2) { + let byFirst = tuples.get(key) + if (!byFirst) tuples.set(key, (byFirst = new Map())) + let bySecond = byFirst.get(value[0]) + if (!bySecond) byFirst.set(value[0], (bySecond = new Map())) + entry = bySecond.get(value[1]) + if (!entry) { + bySecond.set(value[1], (entry = [data, 0])) + entries.push(entry) + } + } else { + let byValue = leaves.get(key) + if (!byValue) leaves.set(key, (byValue = new Map())) + entry = byValue.get(value) + if (!entry) { + byValue.set(value, (entry = [data, 0])) + entries.push(entry) + } + } + entry[1] += multiplicity + } + return new MultiSet(entries.filter(([, multiplicity]) => multiplicity !== 0)) +} + /** * Sums multiplicities of records with the same identity, keeping the first * record seen for each identity (or its Map key, which writes -0 as 0, when diff --git a/packages/db-ivm/src/utils.ts b/packages/db-ivm/src/utils.ts index fc35619e43..6fce8c7ce8 100644 --- a/packages/db-ivm/src/utils.ts +++ b/packages/db-ivm/src/utils.ts @@ -37,26 +37,6 @@ export function binarySearch( return low } -/** - * A string identity within one consolidation: primitives by value, objects, - * functions, and symbols by identity. - */ -export function getStringId( - value: unknown, - referenceIds: Map, -): string { - if (value === null) return `null` - if (value === undefined) return `undefined` - const type = typeof value - if (type === `object` || type === `function` || type === `symbol`) { - const reference = value as object | symbol - let id = referenceIds.get(reference) - if (id === undefined) referenceIds.set(reference, (id = referenceIds.size)) - return `ref_${id}` - } - return `${type}_${String(value)}` -} - export type HRange = [number, number] // half-open [start, end[ i.e. end is exclusive /** diff --git a/packages/db/src/SortedMap.ts b/packages/db/src/SortedMap.ts index dbcd830e57..4e380ee985 100644 --- a/packages/db/src/SortedMap.ts +++ b/packages/db/src/SortedMap.ts @@ -1,5 +1,19 @@ import { compareKeys } from '@tanstack/db-ivm' +/** + * Grouped Collections can produce nullish keys at runtime, which compareKeys + * does not order. Placing them first makes key order total, so binary search + * finds every key. + */ +function compareMapKeys(left: unknown, right: unknown): number { + if (left == null || right == null) { + if (left === right) return 0 + if (left == null && right == null) return left === undefined ? -1 : 1 + return left == null ? -1 : 1 + } + return compareKeys(left as string | number, right as string | number) +} + /** * A Map implementation that keeps its entries sorted based on a comparator function * @template TKey - The type of keys in the map (must be string | number) @@ -9,8 +23,9 @@ export class SortedMap { private map: Map private sortedKeys: Array private comparator: ((a: TValue, b: TValue) => number) | undefined - private orderDirty = false - private runtimeKeyCount = 0 + // Keys set or deleted with deferOrder since the last restoreOrder. A dirty + // key can still hold a slot in sortedKeys that its old value ordered. + private dirtyKeys = new Set() /** * Creates a new SortedMap instance @@ -33,18 +48,23 @@ export class SortedMap { * * @param key - The key to find position for (used as tie-breaker or primary sort when no comparator) * @param value - The value to compare against (only used if comparator is provided) + * @param keys - The ordered keys to search * @returns The index where the key should be inserted */ - private indexOf(key: TKey, value: TValue): number { + private indexOf( + key: TKey, + value: TValue, + keys: Array = this.sortedKeys, + ): number { let left = 0 - let right = this.sortedKeys.length + let right = keys.length // Fast path: no comparator means sort by key only if (!this.comparator) { while (left < right) { const mid = Math.floor((left + right) / 2) - const midKey = this.sortedKeys[mid]! - const keyComparison = compareKeys(key, midKey) + const midKey = keys[mid]! + const keyComparison = compareMapKeys(key, midKey) if (keyComparison < 0) { right = mid } else if (keyComparison > 0) { @@ -59,7 +79,7 @@ export class SortedMap { // With comparator: sort by value first, then key as tie-breaker while (left < right) { const mid = Math.floor((left + right) / 2) - const midKey = this.sortedKeys[mid]! + const midKey = keys[mid]! const midValue = this.map.get(midKey)! const valueComparison = this.comparator(value, midValue) @@ -69,7 +89,7 @@ export class SortedMap { left = mid + 1 } else { // Values are equal, use key as tie-breaker for deterministic ordering - const keyComparison = compareKeys(key, midKey) + const keyComparison = compareMapKeys(key, midKey) if (keyComparison < 0) { right = mid } else if (keyComparison > 0) { @@ -84,17 +104,46 @@ export class SortedMap { return left } - /** Restore the ordered view after a synchronous batch of deferred writes. */ + /** + * Restore the ordered view after a synchronous batch of deferred writes. + * Dirty keys leave their old slots by Set membership, so a deleted value or + * one a sync source mutated in place cannot strand a slot. The dirty keys + * still present are sorted and merged into the rest in one pass: + * O(n + k log n) for k dirty keys. The merge rewrites sortedKeys in place, + * so an open key iterator reads the restored order. + */ restoreOrder(): void { - if (!this.orderDirty) return - this.sortedKeys.length = 0 - for (const key of this.map.keys()) this.sortedKeys.push(key) - this.sortedKeys.sort((left, right) => { - const byValue = - this.comparator?.(this.map.get(left)!, this.map.get(right)!) ?? 0 - return byValue || compareKeys(left, right) - }) - this.orderDirty = false + if (this.dirtyKeys.size === 0) return + const keys = this.sortedKeys + const kept = keys.filter((key) => !this.dirtyKeys.has(key)) + // Array sort moves undefined elements last without comparing them, so + // sort wrapped keys. + const placed = [...this.dirtyKeys] + .filter((key) => this.map.has(key)) + .map((key) => [key] as const) + .sort( + ([left], [right]) => + (this.comparator?.(this.map.get(left)!, this.map.get(right)!) ?? 0) || + compareMapKeys(left, right), + ) + keys.length = 0 + let next = 0 + for (const [key] of placed) { + const index = this.indexOf(key, this.map.get(key)!, kept) + while (next < index) keys.push(kept[next++]!) + keys.push(key) + } + while (next < kept.length) keys.push(kept[next++]!) + this.dirtyKeys.clear() + } + + /** Removes a key's slot while its placed value is still mapped. */ + private unplace(key: TKey): void { + // Binary search misses a value that a sync source mutated in place after + // it was placed, so fall back to a scan. + let index = this.indexOf(key, this.map.get(key)!) + if (this.sortedKeys[index] !== key) index = this.sortedKeys.indexOf(key) + this.sortedKeys.splice(index, 1) } /** @@ -111,30 +160,17 @@ export class SortedMap { this.map.set(key, value) return this } - // Grouped Collections can produce nullish keys at runtime. compareKeys - // is not a total order there, so retain the existing binary-insert path. - const runtimeKey = typeof key !== `string` && typeof key !== `number` - if (deferOrder && this.runtimeKeyCount === 0 && !runtimeKey) { + if (deferOrder) { this.map.set(key, value) - this.orderDirty = true + this.dirtyKeys.add(key) return this } this.restoreOrder() - const exists = this.map.has(key) - if (exists) { - // Need to remove the old key from the sorted keys array - const oldValue = this.map.get(key)! - const oldIndex = this.indexOf(key, oldValue) - this.sortedKeys.splice(oldIndex, 1) - } - - // Insert the new key at the correct position - const index = this.indexOf(key, value) - this.sortedKeys.splice(index, 0, key) - + // The old slot leaves before the search, since an in-place mutation can + // misorder it. + if (this.map.has(key)) this.unplace(key) + this.sortedKeys.splice(this.indexOf(key, value), 0, key) this.map.set(key, value) - if (runtimeKey && !exists) this.runtimeKeyCount++ - return this } @@ -156,23 +192,14 @@ export class SortedMap { * @returns True if the key was found and removed, false otherwise */ delete(key: TKey, deferOrder = false): boolean { - if (deferOrder && this.runtimeKeyCount === 0) { + if (deferOrder) { const deleted = this.map.delete(key) - this.orderDirty ||= deleted + if (deleted) this.dirtyKeys.add(key) return deleted } this.restoreOrder() - if (this.map.has(key)) { - const oldValue = this.map.get(key) - const index = this.indexOf(key, oldValue!) - this.sortedKeys.splice(index, 1) - const deleted = this.map.delete(key) - if (deleted && typeof key !== `string` && typeof key !== `number`) - this.runtimeKeyCount-- - return deleted - } - - return false + if (this.map.has(key)) this.unplace(key) + return this.map.delete(key) } /** @@ -191,8 +218,7 @@ export class SortedMap { clear(): void { this.map.clear() this.sortedKeys = [] - this.orderDirty = false - this.runtimeKeyCount = 0 + this.dirtyKeys.clear() } /** @@ -208,10 +234,7 @@ export class SortedMap { * @returns An iterator for the map's entries */ *[Symbol.iterator](): IterableIterator<[TKey, TValue]> { - this.restoreOrder() - for (const key of this.sortedKeys) { - yield [key, this.map.get(key)!] as [TKey, TValue] - } + for (const key of this.keys()) yield [key, this.map.get(key)!] } /** @@ -239,23 +262,6 @@ export class SortedMap { * @returns An iterator for the map's values */ *values(): IterableIterator { - this.restoreOrder() - for (const key of this.sortedKeys) { - yield this.map.get(key)! - } - } - - /** - * Executes a callback function for each key-value pair in the map in sorted order - * - * @param callbackfn - Function to execute for each entry - */ - forEach( - callbackfn: (value: TValue, key: TKey, map: Map) => void, - ): void { - this.restoreOrder() - for (const key of this.sortedKeys) { - callbackfn(this.map.get(key)!, key, this.map) - } + for (const key of this.keys()) yield this.map.get(key)! } } diff --git a/packages/db/src/collection/state.ts b/packages/db/src/collection/state.ts index fb2a78a5e5..fef8d6b65e 100644 --- a/packages/db/src/collection/state.ts +++ b/packages/db/src/collection/state.ts @@ -1095,7 +1095,6 @@ export class CollectionStateManager< // First collect all keys that will be affected by sync operations const changedKeys = new Set() - const syncedInsertedOrUpdatedKeys = new Set() const firstSyncOperations = new Map< TKey, OptimisticChangeMessage @@ -1106,7 +1105,6 @@ export class CollectionStateManager< changedKeys.add(key) if (!firstSyncOperations.has(key)) firstSyncOperations.set(key, operation) - if (operation.type !== `delete`) syncedInsertedOrUpdatedKeys.add(key) } for (const [key] of transaction.rowMetadataWrites) { changedKeys.add(key) @@ -1310,6 +1308,16 @@ export class CollectionStateManager< ? syncPreviousValue : currentVisibleState.get(key) const newVisibleValue = this.get(key) // This returns the new derived state + + if (previousVisibleValue === undefined) { + if (newVisibleValue === undefined) continue + // Subscribers last saw this key absent, whatever a completed + // optimistic request held. Batching composes a buffered delete with + // this insert into an update. + events.push({ type: `insert`, key, value: newVisibleValue }) + continue + } + // Only deletes and updates describe the previous row. const previousVirtualProps = this.preSyncVirtualState.get(key) ?? this.getVirtualPropsSnapshotForState(key, { @@ -1317,10 +1325,6 @@ export class CollectionStateManager< optimisticUpserts: previousOptimisticUpserts, optimisticDeletes: previousOptimisticDeletes, }) - const nextVirtualProps = this.getVirtualPropsSnapshotForState(key) - const virtualChanged = - previousVirtualProps.$synced !== nextVirtualProps.$synced || - previousVirtualProps.$origin !== nextVirtualProps.$origin const withPreviousVirtualProps = (value: TOutput) => enrichRowWithVirtualProps( value, @@ -1330,20 +1334,18 @@ export class CollectionStateManager< () => previousVirtualProps.$origin, ) - if (previousVisibleValue === undefined) { - if (newVisibleValue === undefined) continue - // Subscribers last saw this key absent, whatever a completed - // optimistic request held. Batching composes a buffered delete with - // this insert into an update. - events.push({ type: `insert`, key, value: newVisibleValue }) - } else if (newVisibleValue === undefined) { + if (newVisibleValue === undefined) { events.push({ type: `delete`, key, value: withPreviousVirtualProps(previousVisibleValue), }) - } else if ( - virtualChanged || + continue + } + const nextVirtualProps = this.getVirtualPropsSnapshotForState(key) + if ( + previousVirtualProps.$synced !== nextVirtualProps.$synced || + previousVirtualProps.$origin !== nextVirtualProps.$origin || !deepEquals(previousVisibleValue, newVisibleValue) ) { events.push({ diff --git a/packages/db/src/index.ts b/packages/db/src/index.ts index 51a4976462..1b607fe966 100644 --- a/packages/db/src/index.ts +++ b/packages/db/src/index.ts @@ -4,7 +4,6 @@ import * as IR from './query/ir.js' export * from './collection/index.js' -export * from './SortedMap' export * from './transactions' export * from './client.js' export { withCollectionConfigFactory } from './client.js' diff --git a/packages/db/tests/SortedMap-oracle.test.ts b/packages/db/tests/SortedMap-oracle.test.ts index 032f8ae39d..4649c5e077 100644 --- a/packages/db/tests/SortedMap-oracle.test.ts +++ b/packages/db/tests/SortedMap-oracle.test.ts @@ -4,25 +4,37 @@ import { SortedMap } from '../src/SortedMap' import { createCollection } from '../src/collection/index.js' import { createTransaction } from '../src/transactions.js' import { oraclePropertyOptions, oracleRuns } from './oracle-config' +import { createOracleReach } from './oracle-reach' import type { SyncConfig } from '../src/types.js' /** - * SortedMap is an ordered view over ordinary map semantics. + * SortedMap is the ordered map behind Collection state: synced rows, ordered + * by the Collection's compare option or by key, and active transactions. It + * promises what those callers use, not every Map behavior. * - * The contract has two independent parts. Keys still have Map ownership: - * setting a key replaces its value, deleting reports whether the key existed, - * and clearing removes everything. Iteration then presents the surviving - * entries in either key order or the caller's value order. Equal values use - * key order as a stable tie-breaker, so no entry disappears inside a tie. - * Deferred writes may postpone ordering within one synchronous batch, but - * point reads remain current and ordered reads restore the same view. + * Keys have Map ownership: setting a key replaces its value, deleting reports + * whether the key existed, and clearing removes everything. Iteration presents + * the surviving entries in either key order or the caller's value order. Equal + * values use key order as a stable tie-breaker, so no entry disappears inside + * a tie. Key order is total over the keys Collections produce: `undefined`, + * then `null` (grouped Collections can produce these at runtime even though + * the key type excludes them), then strings, then numbers. Deferred writes may + * postpone ordering within one synchronous batch, but point reads remain + * current and ordered reads restore the same view. + * + * One mutation during iteration is promised, because pruning settled + * transactions relies on it: deleting entries with deferOrder while iterating + * yields each entry of the order the iteration began with exactly once. Other + * mutation during iteration, NaN and -0 keys, values mutated in place without + * a write, and a comparator that throws are not promised; the coverage map + * records these limits. * * The reference model is deliberately dull: a native Map owns the values and * a fresh full sort derives every observation. The production structure may - * update its ordered index incrementally, but it must refine that recomputation - * after every generated command. The driver observes size, point lookup, every - * iterator, and forEach; checking only the final values would miss stale keys, - * lost ties, and inconsistent views of the same state. + * update its ordered index incrementally, but it must refine that + * recomputation after every generated command. The driver observes size, + * point lookup, and every iterator; checking only the final values would miss + * stale keys, lost ties, and inconsistent views of the same state. */ type Order = `key` | `ascending` | `descending` @@ -50,28 +62,66 @@ async function countDisplacedSlots( return displaced } +type RuntimeKey = undefined | null +type ModelKey = number | RuntimeKey + +const isRuntimeKey = (key: unknown): key is RuntimeKey => key == null +const asKey = (key: ModelKey) => key as number + +// A burst writes more keys than one small batch holds, in an order derived +// from its seed value, so ordered restoration has many pending keys to place. +function burstEntries(seed: number): Array<[number, number]> { + const count = 17 + (seed + 3) * 4 + const keys = Array.from({ length: count }, (_, index) => 10 + index) + if (seed % 2 !== 0) keys.reverse() + return keys.map((key) => [key, ((key * 7) % 5) - 2]) +} + +const probeKeys: Array = [ + -3, + -2, + -1, + 0, + 1, + 2, + 3, + undefined, + null, + 10, + 26, + 50, +] + +// The promised total key order: undefined, null, then numbers (the generated +// keys hold no strings). +function compareModelKeys(a: ModelKey, b: ModelKey): number { + const rank = (key: ModelKey) => (key === undefined ? 0 : key === null ? 1 : 2) + return rank(a) - rank(b) || (rank(a) === 2 ? a! - b! : 0) +} + +function modelOrder(model: Map, order: Order) { + return [...model].sort( + ([ka, va], [kb, vb]) => + (order === `key` ? 0 : order === `ascending` ? va - vb : vb - va) || + compareModelKeys(ka, kb), + ) +} + function expectMap( actual: SortedMap, - model: Map, + model: Map, order: Order, ) { // Recompute from a plain Map; do not copy SortedMap's binary insertion logic. - const expected = [...model].sort( - ([ka, va], [kb, vb]) => - (order === `key` ? 0 : order === `ascending` ? va - vb : vb - va) || - ka - kb, - ) + const expected = modelOrder(model, order) expect(actual.size).toBe(model.size) expect([...actual]).toEqual(expected) expect([...actual.entries()]).toEqual(expected) expect([...actual.keys()]).toEqual(expected.map(([key]) => key)) expect([...actual.values()]).toEqual(expected.map(([, value]) => value)) - const visited: Array<[number, number]> = [] - actual.forEach((value, key) => visited.push([key, value])) - expect(visited).toEqual(expected) - for (let key = -3; key <= 3; key++) { - expect(actual.has(key)).toBe(model.has(key)) - expect(actual.get(key)).toBe(model.get(key)) + for (const key of probeKeys) { + expect(actual.has(asKey(key))).toBe(model.has(key)) + expect(actual.get(asKey(key))).toBe(model.get(key)) } } @@ -86,19 +136,43 @@ describe.each([`key`, `ascending`, `descending`] as const)( it.each([20260912, undefined])( `agrees with full recomputation (seed %s)`, async (seed) => { + const reach = createOracleReach( + property, + [ + `placed-runtime-key-deleted`, + `runtime-key-observed`, + `deferred-burst-observed`, + `entry-yielded-after-pruning`, + ], + { minimum: 5 }, + ) await fc.assert( fc.asyncProperty( fc.array( fc.record({ - kind: fc.constantFrom(`set`, `set`, `delete`, `clear`), - key: fc.integer({ min: -3, max: 3 }), + kind: fc.constantFrom( + `set`, + `set`, + `set`, + `delete`, + `delete`, + `delete`, + `clear`, + `burst`, + `prune`, + ), + key: fc.oneof( + { arbitrary: fc.integer({ min: -3, max: 3 }), weight: 3 }, + { arbitrary: fc.constantFrom(undefined, null), weight: 3 }, + ), value: fc.integer({ min: -3, max: 3 }), deferOrder: fc.boolean(), observe: fc.boolean(), }), - { maxLength: 40 }, + { minLength: 6, maxLength: 40 }, ), (actions) => { + reach.beginHistory() const actual = new SortedMap( order === `key` ? undefined @@ -106,33 +180,95 @@ describe.each([`key`, `ascending`, `descending`] as const)( ? (a, b) => a - b : (a, b) => b - a, ) - const model = new Map() - expectMap(actual, model, order) + const model = new Map() + // Keys written with deferOrder since the last ordered read or + // ordering write; an ordered read must place all of them. + const deferred = new Set() + // Keys whose ordered slot reflects their current value: written + // without deferral, or read in order since their last write. + const placed = new Set() + const observe = () => { + for (const key of model.keys()) placed.add(key) + if (deferred.size > 16) reach.reach(`deferred-burst-observed`) + if ([...model.keys()].some(isRuntimeKey)) + reach.reach(`runtime-key-observed`) + expectMap(actual, model, order) + deferred.clear() + } + observe() for (const action of actions) { + const key = action.key if (action.kind === `set`) { expect( - actual.set(action.key, action.value, action.deferOrder), + actual.set(asKey(key), action.value, action.deferOrder), ).toBe(actual) - model.set(action.key, action.value) + model.set(key, action.value) + if (action.deferOrder) { + deferred.add(key) + placed.delete(key) + } else { + deferred.clear() + for (const modelKey of model.keys()) placed.add(modelKey) + } } else if (action.kind === `delete`) { - expect(actual.delete(action.key, action.deferOrder)).toBe( - model.delete(action.key), + if (isRuntimeKey(key) && placed.has(key)) + reach.reach(`placed-runtime-key-deleted`) + expect(actual.delete(asKey(key), action.deferOrder)).toBe( + model.delete(key), ) + placed.delete(key) + if (action.deferOrder) deferred.add(key) + else { + deferred.clear() + for (const modelKey of model.keys()) placed.add(modelKey) + } + } else if (action.kind === `burst`) { + for (const [burstKey, value] of burstEntries(action.value)) { + actual.set(burstKey, value, true) + model.set(burstKey, value) + deferred.add(burstKey) + placed.delete(burstKey) + } + } else if (action.kind === `prune`) { + // Iterate in order, deleting each entry whose value has the + // action's parity with deferOrder, as settled transactions + // are pruned. The iteration yields the order it began with. + const expected = modelOrder(model, order) + const yielded: Array<[number, number]> = [] + let pruned = false + for (const [entryKey, value] of actual) { + if (pruned) reach.reach(`entry-yielded-after-pruning`) + yielded.push([entryKey, value]) + if ((value - action.value) % 2 === 0) { + expect(actual.delete(entryKey, true)).toBe(true) + pruned = true + } + } + expect(yielded).toEqual(expected) + for (const [entryKey, value] of expected) { + if ((value - action.value) % 2 !== 0) continue + model.delete(entryKey) + deferred.add(entryKey) + placed.delete(entryKey) + } } else { actual.clear() model.clear() + deferred.clear() + placed.clear() } expect(actual.size).toBe(model.size) - expect(actual.has(action.key)).toBe(model.has(action.key)) - expect(actual.get(action.key)).toBe(model.get(action.key)) + expect(actual.has(asKey(key))).toBe(model.has(key)) + expect(actual.get(asKey(key))).toBe(model.get(key)) if ( action.kind === `clear` || - !action.deferOrder || + ((action.kind === `set` || action.kind === `delete`) && + !action.deferOrder) || action.observe ) - expectMap(actual, model, order) + observe() } - expectMap(actual, model, order) + observe() return Promise.resolve() }, ), @@ -190,6 +326,7 @@ describe.each([`key`, `ascending`, `descending`] as const)( ], }, ) + reach.assertReached() }, ) }, @@ -342,6 +479,60 @@ describe(`SortedMap`, () => { ) }) + it(`keeps ordered-key movement bounded during deferred bulk deletes and reorders`, async () => { + // A bulk commit can remove or reorder many existing keys, not only add + // new ones. Moving every later key once per removed or reordered key is + // quadratic; the bound allows a constant number of passes. + const size = 2048 + const map = new SortedMap((a, b) => a - b) + for (let key = 0; key < size; key++) map.set(key, key, true) + map.restoreOrder() + const deleted = await countDisplacedSlots(() => { + for (let key = 0; key < size; key += 2) map.delete(key, true) + map.restoreOrder() + }) + expect(deleted).toBeLessThan(size * 64) + const reordered = await countDisplacedSlots(() => { + for (let key = 1; key < size; key += 4) map.set(key, -key, true) + map.restoreOrder() + }) + expect(reordered).toBeLessThan(size * 64) + const expected = Array.from( + { length: size / 2 }, + (_, index) => 2 * index + 1, + ) + .map((key) => [key, key % 4 === 1 ? -key : key] as const) + .sort(([, a], [, b]) => a - b) + .map(([key]) => key) + expect([...map.keys()]).toEqual(expected) + }) + + it(`keeps Map ownership when a placed comparator value was mutated in place`, () => { + // A sync source may reuse a row object it already wrote. Where the mutated + // row sorts is not promised, but deleting or rewriting it must affect only + // its own key: a binary search guided by the stale order would otherwise + // remove a neighbor. + for (const deferOrder of [false, true]) { + const map = new SortedMap( + (left, right) => left.rank - right.rank, + ) + const rows = [`a`, `b`, `c`, `d`, `e`].map((key, rank) => { + const row = { rank } + map.set(key, row) + return row + }) + rows[1]!.rank = 10 + expect(map.delete(`b`, deferOrder)).toBe(true) + map.restoreOrder() + expect([...map.keys()]).toEqual([`a`, `c`, `d`, `e`]) + rows[3]!.rank = -10 + map.set(`d`, { rank: 1.5 }, deferOrder) + map.restoreOrder() + expect([...map.keys()]).toEqual([`a`, `d`, `c`, `e`]) + expect(map.size).toBe(4) + } + }) + it(`orders mutable comparator values at the batch checkpoint`, () => { const map = new SortedMap( (left, right) => left.rank - right.rank, @@ -608,11 +799,6 @@ describe(`SortedMap`, () => { // Test values() const values = Array.from(map.values()) expect(values).toEqual([1, 2, 3]) - - // Test forEach - const forEachResults: Array = [] - map.forEach((value) => forEachResults.push(value)) - expect(forEachResults).toEqual([1, 2, 3]) }) it(`orders string keys when no custom comparator is provided`, () => { diff --git a/packages/db/tests/query/load-subset-replay-refinement-oracle.test.ts b/packages/db/tests/query/load-subset-replay-refinement-oracle.test.ts index 2550b374e0..12c9b3ad81 100644 --- a/packages/db/tests/query/load-subset-replay-refinement-oracle.test.ts +++ b/packages/db/tests/query/load-subset-replay-refinement-oracle.test.ts @@ -2223,6 +2223,25 @@ describe(`Ordered windows across truncate replays`, () => { }, failureState: replayDemandStates[1], }, + { + // The random campaign's trace from #2224 (also reported as #2239). With + // includes and no joined source, the failed load is new include demand + // issued during the marker replay, so the terminal error is fatal. + name: `a failed include demand during a marker replay`, + history: [ + { type: `hold` }, + { type: `truncateMarkers` }, + { type: `settle`, ok: false }, + ], + scenario: { + ...ordinaryOrderedScenario, + limit: 1, + joined: `none`, + markerMode: `on-demand`, + includes: true, + }, + failureState: replayDemandStates[0], + }, { name: `two window moves while a load is held`, history: [ From 7789b9977e3172b23e0e4d6d1252e8ec25421b72 Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 08:12:58 -0600 Subject: [PATCH 2/9] chore: add changeset for SortedMap merge restore Co-authored-by: Isaac --- .changeset/sortedmap-merge-restore.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/sortedmap-merge-restore.md diff --git a/.changeset/sortedmap-merge-restore.md b/.changeset/sortedmap-merge-restore.md new file mode 100644 index 0000000000..6376570388 --- /dev/null +++ b/.changeset/sortedmap-merge-restore.md @@ -0,0 +1,10 @@ +--- +'@tanstack/db': minor +'@tanstack/db-ivm': patch +--- + +Breaking: `@tanstack/db` no longer exports `SortedMap`, and `SortedMap` no longer has `forEach`. `SortedMap` is the internal ordered map of Collection state. + +- Ordered Collection views now keep rows with a runtime `undefined` or `null` key. Grouped Collections can produce these keys. Before, ordered reads could skip or lose these rows. +- When one publication applies 512 or more sync operations, the Collection now merges the changed keys into the existing order. Before, it sorted all rows again. With 50,000 rows, a `compare` option, and 600 operations in each sync transaction, the load takes 181 ms instead of 1,589 ms. +- Keyed `MultiSet.consolidate` groups entries in nested maps. It no longer encodes each value as a string. From a2f7f133575f32698e8db05bc4bfe36559c6ab07 Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 08:25:11 -0600 Subject: [PATCH 3/9] fix: update mangle cache and stop wrapping removed SortedMap forEach Co-authored-by: Isaac --- packages/db/mangle-cache.json | 6 +++--- .../query/bucket-facade-rollback-oracle.property.test.ts | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/db/mangle-cache.json b/packages/db/mangle-cache.json index 977b4d2d2b..74166632e1 100644 --- a/packages/db/mangle-cache.json +++ b/packages/db/mangle-cache.json @@ -136,7 +136,6 @@ "leases": "cw", "pendingActivity": "cx", "reconcileStalePublishedChanges": "cy", - "runtimeKeyCount": "cz", "trackTruncateReplayParticipant": "cB", "_changes": "cC", "canCompleteInitialLoad": "cD", @@ -220,7 +219,6 @@ "filterAndFlipChanges": "d0", "getPreloadError": "d1", "graphCache": "d3", - "orderDirty": "d4", "recordRequestFailure": "d5", "refreshDetachedState": "d6", "releaseDemandAt": "d7", @@ -413,5 +411,7 @@ "ssrQueryWaiters": "bL", "includesRuntimeCache": "ch", "failSsrQueryWaiters": "cA", - "gcGraceUntil": "aP" + "gcGraceUntil": "aP", + "dirtyKeys": "bT", + "unplace": "cz" } diff --git a/packages/db/tests/query/bucket-facade-rollback-oracle.property.test.ts b/packages/db/tests/query/bucket-facade-rollback-oracle.property.test.ts index 8b9b0a00a3..d4620917b6 100644 --- a/packages/db/tests/query/bucket-facade-rollback-oracle.property.test.ts +++ b/packages/db/tests/query/bucket-facade-rollback-oracle.property.test.ts @@ -100,7 +100,7 @@ import type { BucketRow } from '../../src/query/live/materialized-pipeline.js' * While a hold settles after a failed flush, a restoring sync update can * still publish the value the facade already shows; that permitted false * positive is the only event a failed flush may publish. - * The work counter wraps the iteration, `forEach`, `get` and `has` methods of + * The work counter wraps the iteration, `get` and `has` methods of * each facade's stored rows during the flush and records which facades the * flush read. A pinned case covers nested facades across two edges: the child * edge commits, then the parent edge throws. Pinned cases also cover a @@ -590,13 +590,12 @@ class Driver { return original(...args) } } - // Every read path of the stored rows: iteration, forEach, and lookups. + // Every read path of the stored rows: iteration and lookups. for (const method of [ Symbol.iterator, `entries`, `keys`, `values`, - `forEach`, `get`, `has`, ]) { From 33cec3897ecb148722fbbd3a58aaeef90676a253 Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 09:15:18 -0600 Subject: [PATCH 4/9] fix(db): key grouped rows by the Collection key type - A single-field group whose value is null, undefined, a boolean, or a bigint gets a string key in the reserved normalized-key namespace, so grouped keys are always strings or numbers. - sync.write rejects keys that are not strings or numbers with the same check that mutations use; key error messages show bigints. - SortedMap drops its nullish key order and wrapped sort and compares keys with compareKeys. - New grouped result key oracle; the SortedMap oracle adds in-place row changes and a comparator-call work law for restores, and its example tests move to SortedMap.test.ts. Co-authored-by: Isaac --- .changeset/sortedmap-merge-restore.md | 6 +- docs/contributing/oracle-coverage.md | 3 +- packages/db/src/SortedMap.ts | 27 +- packages/db/src/collection/mutations.ts | 12 +- packages/db/src/collection/sync.ts | 3 + packages/db/src/errors.ts | 18 +- packages/db/src/query/compiler/group-by.ts | 10 +- packages/db/src/utils/comparison.ts | 2 +- packages/db/tests/SortedMap-oracle.test.ts | 932 +++++------------- packages/db/tests/SortedMap.test.ts | 516 ++++++++++ packages/db/tests/collection.test.ts | 40 + packages/db/tests/oracle-config.ts | 1 + packages/db/tests/query/group-by.test.ts | 9 +- .../group-result-key-oracle.property.test.ts | 291 ++++++ 14 files changed, 1156 insertions(+), 714 deletions(-) create mode 100644 packages/db/tests/SortedMap.test.ts create mode 100644 packages/db/tests/query/group-result-key-oracle.property.test.ts diff --git a/.changeset/sortedmap-merge-restore.md b/.changeset/sortedmap-merge-restore.md index 6376570388..7e308e8d1d 100644 --- a/.changeset/sortedmap-merge-restore.md +++ b/.changeset/sortedmap-merge-restore.md @@ -3,8 +3,10 @@ '@tanstack/db-ivm': patch --- -Breaking: `@tanstack/db` no longer exports `SortedMap`, and `SortedMap` no longer has `forEach`. `SortedMap` is the internal ordered map of Collection state. +Breaking: Collection keys now always match the declared key type, a string or a number. -- Ordered Collection views now keep rows with a runtime `undefined` or `null` key. Grouped Collections can produce these keys. Before, ordered reads could skip or lose these rows. +- A grouped live query that groups by one field now gives a `null`, `undefined`, boolean, or bigint group value a string key. Before, the key was the raw value, so `$key` was `null` for the `null` group. Read a group's key from its `$key`. String and number group values keep their keys. +- `sync.write` throws `UndefinedKeyError` or `InvalidKeyError` when `getKey` returns a key that is not a string or a number. Optimistic mutations already threw these errors. For a bigint key, the development message now shows the key instead of failing with a `TypeError`. +- `@tanstack/db` no longer exports `SortedMap`, and `SortedMap` no longer has `forEach`. `SortedMap` is the internal ordered map of Collection state. - When one publication applies 512 or more sync operations, the Collection now merges the changed keys into the existing order. Before, it sorted all rows again. With 50,000 rows, a `compare` option, and 600 operations in each sync transaction, the load takes 181 ms instead of 1,589 ms. - Keyed `MultiSet.consolidate` groups entries in nested maps. It no longer encodes each value as a string. diff --git a/docs/contributing/oracle-coverage.md b/docs/contributing/oracle-coverage.md index 160a493795..6f30babd5b 100644 --- a/docs/contributing/oracle-coverage.md +++ b/docs/contributing/oracle-coverage.md @@ -68,6 +68,7 @@ that test identifiers must copy production's private data structures. | Indexed predicate filtering | Complete for bounded dotted-path collision | An independent row filter checks direct predicates and public callback predicates for a dotted scalar field beside a nested path, with and without an index. Selected-field ordering checks the same distinction through `$selected`. | | WHERE predicate publication | Complete for bounded predicate and sync-transaction grammar | The contract, Kleene reference evaluator, snapshot and change-history grammar, three subscriber consumers plus direct snapshot, and per-commit key-set refinement check are literate. A fixed same-key update checks live-row and direct-subscriber payloads; a focused descriptor boundary checks stored-row prefilter safety. Generated cleanup and restart histories for filtered subscribers remain open. | | Joined result keys | Complete for bounded two-source key grammar | The contract, nested-loop pair model, delimiter-, number-like, infinite, and `NaN` key grammar, public join driver, and per-checkpoint pair and key-count check are literate. Joins over subqueries, more than two sources, custom `getKey`, and optimistic mutations remain outside this owner. | +| Grouped result keys | [Grouped result key oracle](https://github.com/TanStack/db/blob/main/packages/db/tests/query/group-result-key-oracle.property.test.ts) | A grouped live-query Collection gives each group one key of the Collection key type, a string or a number, whatever the group value. Maintainer decision (2026-10-27): grouped keys match the declared key type rather than widening it to nullish keys. Group values come from strings, numbers, `NaN`, `null`, `undefined`, booleans, a bigint, strings that print like those values, and strings in the reserved `\u0000tanstack-db:` namespace; histories move, delete, and insert rows. After preload and each synced change, the (group value, count) pairs equal an independent partition by type and value, every key is a string or number, no two groups share a key, and a group that survives a change keeps its key. Reach (minimum 5, per campaign of 80): a nullish group published (32 to 38), a group value published beside a string that prints like it (18 to 21), and a key kept across a change (49 to 53). RED on the parent commit: keys `undefined`, `null`, `false`, and `10n`. Mutants that encode with `String(value)` (lookalike keys collide) or that encode only nullish values (boolean keys) fail. Encoding only the internal D2 key is equivalent in this domain, because the Collection keys rows by the public group key. A focused witness in `collection.test.ts` checks that `sync.write` rejects `undefined`, `null`, boolean, and bigint keys before staging them. Not covered: Dates, `-0`, objects, and multi-field groups, whose values the aggregate select oracle owns; real sync adapters, since the source is a controlled mock. | | D2 Index storage | Complete for bounded prefix grammar | The contract, plain-`Map` multiset model, prefixed and unprefixed value grammar, `Index` driver, and per-addition `get`/`has` check are literate. Compaction, presence tracking, and structural payloads remain outside this owner. | | Pooled live queries | Complete for bounded eq-filter grammar | The contract, independent `eq` model, live-query Collection second formulation, sync/optimistic/mount/cleanup-restart grammar, observer driver, and per-step refinement check are literate. On-demand and persisted sources, `DbClient`, Suspense, and every clause beyond `eq` conjuncts keep the live-query Collection. | | Live-query deferred acquisition | Complete for bounded synchronous grammar | The law, independent model, history grammar over source states, shapes, and depth, real-Collection driver, and per-step refinement check are literate. Pooled views and DbClient stream preloads have their own blocks. The grammar also adds a peer consumer starting the source and a source truncate before the first subscriber; pinned blocks cover a subscriber that outlives cleanup, reads that wait for readiness, and release of two independent descriptor sources when one start fails. Asynchronous sources, several live-query consumers, and general failure histories stay open. | @@ -1064,7 +1065,7 @@ generic visitor behavior outside the named collectors. | Index suggestions | [index suggestion oracle](https://github.com/TanStack/db/blob/main/packages/db/tests/index-suggestion-oracle.test.ts) | A public filtered live-query Collection checks manual/default, eager, threshold, matching-index, and unrelated-index cases against the documented collection-size suggestion policy. A bounded ordered-query decision table separately classifies no field index, a null-placement mismatch, matching but order-ineligible indexes, and usable candidates in either registration order. It compares the development warning after preload; the matching-index selection owner below checks the chosen reader and ordered request. Two cyclic locale-options cells require the diagnostic to preserve valid query construction with or without a field index, and a custom-collation cell requires remediation to preserve comparator reference identity. The original manual-silence/eager-noise implementation failed two expected observations. The issue #2190 baseline failed four diagnostic cells, including a first-unusable/second-usable candidate order; the first formatter draft failed all three non-JSON-safe remediation cells. The owner does not judge repeated-query warning volume, slow-query timing, production-build suppression, every comparison-option mismatch, or every query shape. It runs in `@tanstack/db`'s `test:oracles` campaign. | | Minified DB public API | [consumer bundle check](https://github.com/TanStack/db/blob/main/scripts/test-minified-db.mjs) | CI builds `@tanstack/db`, then bundles its built `dist` entry with db-ivm inlined through esbuild `minify: true`. The build renames TypeScript-private members from `packages/db/mangle-cache.json`, so this lane runs against the renamed output that consumers install. It rejects a `dist` that is older than `src`, the cache, or the package's build configuration. `pnpm --filter @tanstack/db test:dist` also runs five db test files against the built `dist`, chosen for coverage of the modules with the most renamed members. `pnpm check:mangle` fails when a cached name is used other than as a private member in db src, is read by another package, appears as a string, or has a short name that collides with a source identifier ([private-member mangling review](oracle-reviews/code-weight-private-member-mangling.md)). It checks every exported error class's current public `name`, the built-in `BasicIndex` resolver name in index metadata and its event, complete query rows (after removing the four documented virtual fields) against an independent array filter/sort/projection, and one live update. The `new.target.name`, public-member mangle, and extra-output-field calibration modes fail at the intended observations. This fixed slice does not replace the unminified generated oracles, exercise framework adapters, or establish stable names for custom index resolvers. | | Boundary refinements | [cleanup/restart](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-cleanup-restart-oracle.test.ts), [idle cleanup](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-idle-cleanup.test.ts), [issue #1891 async-cleanup review](oracle-reviews/issue-1891-async-cleanup.md), [issue #1891 cleanup-start follow-up](oracle-reviews/issue-1891-cleanup-start-followup.md), [metadata publication](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-metadata-publication-oracle.property.test.ts), [row metadata composition](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-row-metadata-composition-oracle.test.ts), [nested-begin metadata rebuild review](oracle-reviews/2026-10-06-nested-begin-metadata-rebuild.md), [state mutation round 4](oracle-reviews/2026-10-06-state-mutation-round-4.md), [state retention](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-state-retention-oracle.property.test.ts), [acquisition cells](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-subscription-lifecycle-oracle.test.ts), [D2 source reconciliation](https://github.com/TanStack/db/blob/main/packages/db/tests/d2-source-reconciliation-oracle.property.test.ts), [top-K support windows](https://github.com/TanStack/db/blob/main/packages/db-ivm/tests/operators/topk-support-window-oracle.test.ts), [nested Query work](https://github.com/TanStack/db/blob/main/packages/query-db-collection/tests/includes-work-counter-oracle.test.ts) | Explicit lifecycle products, independent source maps and weighted relations, exact publication cuts, support/multiplicity, and value-plus-work observations. These refine the larger subsystem models; they do not replace them. | -| Small structures and test mechanics | [SortedMap](https://github.com/TanStack/db/blob/main/packages/db/tests/SortedMap-oracle.test.ts), [cleanup queue](https://github.com/TanStack/db/blob/main/packages/db/tests/cleanup-queue-oracle.property.test.ts), [live-query GC clock](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-gc-clock.test.ts), [guarded replay](https://github.com/TanStack/db/blob/main/packages/db/tests/oracle-replay.test.ts) | Map/full-sort and elapsed appointment-list models with executed target/seed/path checks. SortedMap promises what Collection state uses, not full Map parity (maintainer decision). The model is a Map plus a full sort; key order is total over Collection keys (`undefined`, `null`, strings, numbers). Generated histories mix deferred and immediate sets and deletes, clear, deferred bursts of 17 to 41 keys, runtime `undefined` and `null` keys, and prunes that delete entries with deferOrder while iterating, as settled transactions are pruned; a prune must yield the order it began with. Declared reach requires deletes of placed runtime keys, observations with runtime keys, burst restorations, and an entry yielded after a prune deleted one. Focused witnesses check splice movement for deferred inserts, deletes and comparator reorders, a key iterator opened before a deferred restore, rewritten or deleted rows whose value was mutated in place, scan reuse, and public single-transaction and queued-transaction Collection sync paths. The movement counter sees splices only, so it rejects per-key splicing but does not bound the restore's single filter and merge pass. Not promised: other mutation during iteration (immediate writes, clear), where a row mutated in place without a write sorts, NaN and -0 keys, a comparator that throws, and keys of other runtime types (booleans, bigints). `forEach` was removed; SortedMap is not exported. Wall-clock steps do not move due times, and a focused public witness checks source subscription release. Callback-reentrant scheduling, OS suspend/resume, environments without `performance.now()`, and fake/real Performance clock replacement while GC is pending remain outside this coverage. Tests must drain or reset pending GC before switching timer providers. | +| Small structures and test mechanics | [SortedMap](https://github.com/TanStack/db/blob/main/packages/db/tests/SortedMap-oracle.test.ts), [cleanup queue](https://github.com/TanStack/db/blob/main/packages/db/tests/cleanup-queue-oracle.property.test.ts), [live-query GC clock](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-gc-clock.test.ts), [guarded replay](https://github.com/TanStack/db/blob/main/packages/db/tests/oracle-replay.test.ts) | Map/full-sort and elapsed appointment-list models with executed target/seed/path checks. SortedMap promises what Collection state uses, not full Map parity (maintainer decision). Keys are strings or numbers: `sync.write` and mutations throw `InvalidKeyError` or `UndefinedKeyError` for any other key, and grouped live queries key nullish, boolean, and bigint group values with strings (maintainer decision, 2026-10-27). The model is a Map of numbers plus a full sort; production stores row objects so the driver can change one in place. Generated histories mix deferred and immediate sets and deletes, clear, deferred bursts of 17 to 41 keys, prunes that delete entries with deferOrder while iterating, and in-place changes of a deferred or a placed row. A prune must yield the order it began with. A row changed before restore sorts by its current value; once a placed row changes in place, comparator order downgrades to membership (each key once, current value) for the rest of the history, and a later write or delete affects only its own key. At each ordered read with a comparator, restoring k deferred keys among n others makes at most k * (ceil(log2 k) + ceil(log2 (n + 1)) + 2) comparator calls, and none with nothing deferred. Declared reach (minimum 5, per campaign of 100): burst restorations, an entry yielded after a prune deleted one, a restore whose bound is below n (21 to 29), a changed placed row later written (19 to 23), and a deferred row changed before restore (9 to 18). Mutants that re-sort every key (108 calls against a bound of 7), re-sort the kept keys with the dirty keys appended (34 against 7), re-sort clean scans, drop the scan fallback for a changed row, keep dirty slots, apply deferred deletes immediately, or place a deferred row by its value at the write each fail. Supplemental examples in `SortedMap.test.ts` pin splice movement at scale, a key iterator opened before a deferred restore, scan reuse, and public single-transaction and queued-transaction Collection sync paths. Not promised: other mutation during iteration (immediate writes, clear), the order of rows once a placed row changed in place, keys outside the key type, NaN and -0 keys, a comparator that throws, and the work of key order without a comparator, which the public API cannot observe. `forEach` was removed; SortedMap is not exported. Wall-clock steps do not move due times, and a focused public witness checks source subscription release. Callback-reentrant scheduling, OS suspend/resume, environments without `performance.now()`, and fake/real Performance clock replacement while GC is pending remain outside this coverage. Tests must drain or reset pending GC before switching timer providers. | The index-suggestion oracle's PR #2196 extension checks a matching local custom-collation index against the full-source request and retained warning, diff --git a/packages/db/src/SortedMap.ts b/packages/db/src/SortedMap.ts index 4e380ee985..3be4c1aaeb 100644 --- a/packages/db/src/SortedMap.ts +++ b/packages/db/src/SortedMap.ts @@ -1,19 +1,5 @@ import { compareKeys } from '@tanstack/db-ivm' -/** - * Grouped Collections can produce nullish keys at runtime, which compareKeys - * does not order. Placing them first makes key order total, so binary search - * finds every key. - */ -function compareMapKeys(left: unknown, right: unknown): number { - if (left == null || right == null) { - if (left === right) return 0 - if (left == null && right == null) return left === undefined ? -1 : 1 - return left == null ? -1 : 1 - } - return compareKeys(left as string | number, right as string | number) -} - /** * A Map implementation that keeps its entries sorted based on a comparator function * @template TKey - The type of keys in the map (must be string | number) @@ -64,7 +50,7 @@ export class SortedMap { while (left < right) { const mid = Math.floor((left + right) / 2) const midKey = keys[mid]! - const keyComparison = compareMapKeys(key, midKey) + const keyComparison = compareKeys(key, midKey) if (keyComparison < 0) { right = mid } else if (keyComparison > 0) { @@ -89,7 +75,7 @@ export class SortedMap { left = mid + 1 } else { // Values are equal, use key as tie-breaker for deterministic ordering - const keyComparison = compareMapKeys(key, midKey) + const keyComparison = compareKeys(key, midKey) if (keyComparison < 0) { right = mid } else if (keyComparison > 0) { @@ -116,19 +102,16 @@ export class SortedMap { if (this.dirtyKeys.size === 0) return const keys = this.sortedKeys const kept = keys.filter((key) => !this.dirtyKeys.has(key)) - // Array sort moves undefined elements last without comparing them, so - // sort wrapped keys. const placed = [...this.dirtyKeys] .filter((key) => this.map.has(key)) - .map((key) => [key] as const) .sort( - ([left], [right]) => + (left, right) => (this.comparator?.(this.map.get(left)!, this.map.get(right)!) ?? 0) || - compareMapKeys(left, right), + compareKeys(left, right), ) keys.length = 0 let next = 0 - for (const [key] of placed) { + for (const key of placed) { const index = this.indexOf(key, this.map.get(key)!, kept) while (next < index) keys.push(kept[next++]!) keys.push(key) diff --git a/packages/db/src/collection/mutations.ts b/packages/db/src/collection/mutations.ts index 4f87f8810e..1d5a3151da 100644 --- a/packages/db/src/collection/mutations.ts +++ b/packages/db/src/collection/mutations.ts @@ -9,7 +9,6 @@ import { createTransaction, getActiveTransaction } from '../transactions' import { DeleteKeyNotFoundError, DuplicateKeyError, - InvalidKeyError, InvalidSchemaError, KeyUpdateNotAllowedError, MissingDeleteHandlerError, @@ -20,8 +19,8 @@ import { NoKeysPassedToUpdateError, SchemaMustBeSynchronousError, SchemaValidationError, - UndefinedKeyError, UpdateKeyNotFoundError, + assertCollectionKey, } from '../errors' import type { Collection, CollectionImpl } from './index.js' import type { StandardSchemaV1 } from '@standard-schema/spec' @@ -182,14 +181,7 @@ export class CollectionMutationsManager< } public generateGlobalKey(key: any, item: any): string { - if (typeof key !== `string` && typeof key !== `number`) { - // Preserve specific error for undefined keys - if (typeof key === `undefined`) { - throw new UndefinedKeyError(item) - } - throw new InvalidKeyError(key, item) - } - + assertCollectionKey(key, item) return `KEY::${JSON.stringify([this.id, typeof key, String(key)])}` } diff --git a/packages/db/src/collection/sync.ts b/packages/db/src/collection/sync.ts index 103c847f55..168de61095 100644 --- a/packages/db/src/collection/sync.ts +++ b/packages/db/src/collection/sync.ts @@ -10,6 +10,7 @@ import { SyncRowReusedWithoutPreviousValueError, SyncTransactionAlreadyCommittedError, SyncTransactionAlreadyCommittedWriteError, + assertCollectionKey, } from '../errors' import { createDeferred } from '../deferred' import { codedMessage, devBuild } from '../error-message' @@ -242,6 +243,8 @@ export class CollectionSyncManager< `key` in messageWithOptionalKey ? messageWithOptionalKey.key : this.config.getKey(messageWithOptionalKey.value) + // Ordered Collection state needs keys of the declared key type. + assertCollectionKey(key, messageWithOptionalKey) let messageType = messageWithOptionalKey.type diff --git a/packages/db/src/errors.ts b/packages/db/src/errors.ts index 6f6b002db6..8321c5e929 100644 --- a/packages/db/src/errors.ts +++ b/packages/db/src/errors.ts @@ -207,11 +207,16 @@ export class CollectionOperationError extends TanStackDBError { } } +// Development messages show a bigint key or field instead of failing to +// serialize it. +const showBigint = (_key: string, value: unknown) => + typeof value === `bigint` ? `${value}n` : value + export class UndefinedKeyError extends CollectionOperationError { constructor(item: any) { super( devBuild() && process.env.NODE_ENV !== `production` - ? `An object was created without a defined key: ${JSON.stringify(item)}` + ? `An object was created without a defined key: ${JSON.stringify(item, showBigint)}` : codedMessage(14, { item }), ) } @@ -221,12 +226,21 @@ export class InvalidKeyError extends CollectionOperationError { constructor(key: unknown, item: unknown) { super( devBuild() && process.env.NODE_ENV !== `production` - ? `getKey returned an invalid key type. Expected string or number, but got ${key === null ? `null` : typeof key}: ${JSON.stringify(key)}. Item: ${JSON.stringify(item)}` + ? `getKey returned an invalid key type. Expected string or number, but got ${key === null ? `null` : typeof key}: ${JSON.stringify(key, showBigint)}. Item: ${JSON.stringify(item, showBigint)}` : codedMessage(15, { key, type: key === null ? `null` : typeof key }), ) } } +/** Throws unless a key has the Collection key type: a string or a number. */ +export function assertCollectionKey(key: unknown, item: unknown): void { + if (typeof key !== `string` && typeof key !== `number`) { + throw key === undefined + ? new UndefinedKeyError(item) + : new InvalidKeyError(key, item) + } +} + export class DuplicateKeyError extends CollectionOperationError { constructor(key: string | number) { super( diff --git a/packages/db/src/query/compiler/group-by.ts b/packages/db/src/query/compiler/group-by.ts index d41bfcfbcf..1a15fb7b34 100644 --- a/packages/db/src/query/compiler/group-by.ts +++ b/packages/db/src/query/compiler/group-by.ts @@ -14,6 +14,7 @@ import { } from '../ir.js' import { isRefProxy } from '../builder/ref-proxy-identity.js' import { deepEquals, isTemporal } from '../../utils.js' +import { normalizedKey } from '../../utils/comparison.js' import { AggregateFunctionNotInSelectError, NonAggregateExpressionNotInGroupByError, @@ -95,13 +96,20 @@ function createPublicGroupKey(values: Array): unknown { const identities = values.map(getEqualityValueIdentity) if (identities.length === 1) { const identity = identities[0] + if (typeof identity === `string` || typeof identity === `number`) { + return identity + } + // Collection keys are strings or numbers. Another primitive group value, + // such as `null`, `undefined`, or a boolean, keys its group in the + // namespace that `normalizeValue` reserves. No string group value + // occupies that namespace, so the key cannot collide with one. if ( identity == null || (typeof identity !== `object` && typeof identity !== `function` && typeof identity !== `symbol`) ) { - return identity + return normalizedKey(typeof identity, String(identity)) } } return serializeValue(identities) diff --git a/packages/db/src/utils/comparison.ts b/packages/db/src/utils/comparison.ts index 1a4d79bac0..e8e2c01b03 100644 --- a/packages/db/src/utils/comparison.ts +++ b/packages/db/src/utils/comparison.ts @@ -224,7 +224,7 @@ function areUint8ArraysEqual(a: Uint8Array, b: Uint8Array): boolean { } const NORMALIZED_KEY_PREFIX = `\u0000tanstack-db:` -function normalizedKey(kind: string, value: string): string { +export function normalizedKey(kind: string, value: string): string { return `${NORMALIZED_KEY_PREFIX}${kind}:${value}` } diff --git a/packages/db/tests/SortedMap-oracle.test.ts b/packages/db/tests/SortedMap-oracle.test.ts index 4649c5e077..bf3cd24f73 100644 --- a/packages/db/tests/SortedMap-oracle.test.ts +++ b/packages/db/tests/SortedMap-oracle.test.ts @@ -1,11 +1,8 @@ -import { describe, expect, it, vi } from 'vitest' +import { describe, expect, it } from 'vitest' import fc from 'fast-check' import { SortedMap } from '../src/SortedMap' -import { createCollection } from '../src/collection/index.js' -import { createTransaction } from '../src/transactions.js' import { oraclePropertyOptions, oracleRuns } from './oracle-config' import { createOracleReach } from './oracle-reach' -import type { SyncConfig } from '../src/types.js' /** * SortedMap is the ordered map behind Collection state: synced rows, ordered @@ -16,57 +13,49 @@ import type { SyncConfig } from '../src/types.js' * whether the key existed, and clearing removes everything. Iteration presents * the surviving entries in either key order or the caller's value order. Equal * values use key order as a stable tie-breaker, so no entry disappears inside - * a tie. Key order is total over the keys Collections produce: `undefined`, - * then `null` (grouped Collections can produce these at runtime even though - * the key type excludes them), then strings, then numbers. Deferred writes may - * postpone ordering within one synchronous batch, but point reads remain - * current and ordered reads restore the same view. + * a tie. Keys are strings or numbers, the Collection key type; a sync write + * or mutation with another key throws before it reaches SortedMap. Deferred + * writes may postpone ordering within one synchronous batch, but point reads + * remain current and ordered reads restore the same view. * - * One mutation during iteration is promised, because pruning settled - * transactions relies on it: deleting entries with deferOrder while iterating - * yields each entry of the order the iteration began with exactly once. Other - * mutation during iteration, NaN and -0 keys, values mutated in place without - * a write, and a comparator that throws are not promised; the coverage map + * Three further laws come from Collection state's use: + * + * - A sync source may change a row object in place. A row changed before + * ordering is restored sorts by its current value. A row changed after it + * was placed may leave the comparator order wrong, but every view still + * holds each key once with its current value, and a later write or delete + * of that row affects only its own key. + * - Deleting entries with deferOrder while iterating yields each entry of the + * order the iteration began with exactly once. Pruning settled transactions + * relies on this. + * - Restoring the order after k deferred writes over n other keys makes at + * most k * (ceil(log2 k) + ceil(log2 (n + 1)) + 2) comparator calls, and an + * ordered read with nothing deferred makes none. A large sync publication + * depends on this; re-sorting or comparing every key would exceed it once n + * is large beside k. + * + * Not promised: other mutation during iteration (immediate writes, clear), + * the order of rows once a placed row changed in place, keys outside the key + * type, NaN and -0 keys, a comparator that throws, and the work of key order + * without a comparator, which the public API cannot observe. The coverage map * records these limits. * - * The reference model is deliberately dull: a native Map owns the values and - * a fresh full sort derives every observation. The production structure may - * update its ordered index incrementally, but it must refine that - * recomputation after every generated command. The driver observes size, - * point lookup, and every iterator; checking only the final values would miss - * stale keys, lost ties, and inconsistent views of the same state. + * The reference model is deliberately dull: a native Map of numbers owns the + * values, and a fresh full sort derives every observation. The production + * structure may update its ordered index incrementally, but it must refine + * that recomputation after every generated command. The driver observes size, + * point lookup, every iterator, and comparator calls at each ordered read; + * checking only the final values would miss stale keys, lost ties, + * inconsistent views of the same state, and work that grows with the map. */ type Order = `key` | `ascending` | `descending` -async function countDisplacedSlots( - action: () => void | Promise, -): Promise { - const splice = Array.prototype.splice - let displaced = 0 - const spy = vi.spyOn(Array.prototype, `splice`).mockImplementation(function ( - this: Array, - start: number, - deleteCount?: number, - ...items: Array - ) { - const removed = deleteCount ?? this.length - start - displaced += Math.max(0, this.length - start - removed) - return splice.call(this, start, removed, ...items) - }) - try { - await action() - } finally { - spy.mockRestore() - } - return displaced -} - -type RuntimeKey = undefined | null -type ModelKey = number | RuntimeKey +// Production stores row objects so the driver can change one in place. The +// model stores each row's rank as a number and never shares an object. +type Row = { rank: number } -const isRuntimeKey = (key: unknown): key is RuntimeKey => key == null -const asKey = (key: ModelKey) => key as number +type ModelKey = number // A burst writes more keys than one small batch holds, in an order derived // from its seed value, so ordered restoration has many pending keys to place. @@ -77,27 +66,10 @@ function burstEntries(seed: number): Array<[number, number]> { return keys.map((key) => [key, ((key * 7) % 5) - 2]) } -const probeKeys: Array = [ - -3, - -2, - -1, - 0, - 1, - 2, - 3, - undefined, - null, - 10, - 26, - 50, -] +const probeKeys: Array = [-3, -2, -1, 0, 1, 2, 3, 10, 26, 50] -// The promised total key order: undefined, null, then numbers (the generated -// keys hold no strings). -function compareModelKeys(a: ModelKey, b: ModelKey): number { - const rank = (key: ModelKey) => (key === undefined ? 0 : key === null ? 1 : 2) - return rank(a) - rank(b) || (rank(a) === 2 ? a! - b! : 0) -} +// The generated keys are numbers, which key order sorts ascending. +const compareModelKeys = (a: ModelKey, b: ModelKey) => a - b function modelOrder(model: Map, order: Order) { return [...model].sort( @@ -107,24 +79,113 @@ function modelOrder(model: Map, order: Order) { ) } +// The work law's bound for restoring k deferred keys among n others. +function restoreBound(k: number, n: number): number { + if (k === 0) return 0 + return k * (Math.ceil(Math.log2(k)) + Math.ceil(Math.log2(n + 1)) + 2) +} + +const byKey = (entries: Array<[ModelKey, number]>) => + [...entries].sort(([a], [b]) => compareModelKeys(a, b)) + +// Compare one observed view with the model. Without the order promise, the +// view must still hold each key once with its current value. +function expectView( + actual: Array<[ModelKey, number]>, + expected: Array<[ModelKey, number]>, + orderPromised: boolean, +) { + if (orderPromised) expect(actual).toEqual(expected) + else expect(byKey(actual)).toEqual(byKey(expected)) +} + function expectMap( - actual: SortedMap, + actual: SortedMap, model: Map, order: Order, + orderPromised = true, ) { // Recompute from a plain Map; do not copy SortedMap's binary insertion logic. const expected = modelOrder(model, order) + const ranks = (entries: Iterable<[number, Row]>) => + // A stale slot reads no row; keep it visible to the comparison. + [...entries].map(([key, row]): [ModelKey, number] => [ + key, + (row as Row | undefined)?.rank as number, + ]) expect(actual.size).toBe(model.size) - expect([...actual]).toEqual(expected) - expect([...actual.entries()]).toEqual(expected) - expect([...actual.keys()]).toEqual(expected.map(([key]) => key)) - expect([...actual.values()]).toEqual(expected.map(([, value]) => value)) + expectView(ranks(actual), expected, orderPromised) + expectView(ranks(actual.entries()), expected, orderPromised) + const values = [...actual.values()] + expectView( + [...actual.keys()].map((key, index) => [ + key, + (values[index] as Row | undefined)?.rank as number, + ]), + expected, + orderPromised, + ) for (const key of probeKeys) { - expect(actual.has(asKey(key))).toBe(model.has(key)) - expect(actual.get(asKey(key))).toBe(model.get(key)) + expect(actual.has(key)).toBe(model.has(key)) + expect(actual.get(key)?.rank).toBe(model.get(key)) } } +type Action = { + kind: `set` | `delete` | `clear` | `burst` | `prune` | `mutate` + key: ModelKey + value: number + deferOrder: boolean + observe: boolean +} + +const fixedStep = ( + kind: Action[`kind`], + key: ModelKey, + value = 0, + deferOrder = false, + observe = false, +): Action => ({ kind, key, value, deferOrder, observe }) + +/** + * Fixed regressions replay through the same model, driver, and comparison. + * + * - Deferred writes, an immediate rewrite of a tie, a deferred delete, and + * clear. + * - A placed row changed in place, then neighbours deleted and rewritten. A + * binary search guided by the stale order would remove a neighbour. + * - A deferred key deleted before a large deferred batch. + */ +const fixedHistories: Array> = [ + [ + fixedStep(`set`, 1, 3, true), + fixedStep(`set`, -1, 3, true), + fixedStep(`set`, 1, -3, false, true), + fixedStep(`delete`, -1, 0, true), + fixedStep(`clear`, 0, 0, false, true), + fixedStep(`set`, -1, 2, true, true), + ], + [ + fixedStep(`set`, -2, 0), + fixedStep(`set`, -1, 1), + fixedStep(`set`, 0, 2), + fixedStep(`set`, 1, 3), + fixedStep(`set`, 2, 3), + fixedStep(`mutate`, -1, 3), + fixedStep(`delete`, -1, 0, false, true), + fixedStep(`mutate`, 1, -3), + fixedStep(`set`, 1, 1, false, true), + fixedStep(`delete`, 0, 0, true, true), + ], + [ + fixedStep(`set`, -3, 0, true), + fixedStep(`set`, 1, 0, true, true), + fixedStep(`delete`, -3, 0, true), + fixedStep(`burst`, 0, 3), + fixedStep(`set`, 2, 0, false, true), + ], +] + describe.each([`key`, `ascending`, `descending`] as const)( `SortedMap %s history`, (order) => { @@ -133,16 +194,23 @@ describe.each([`key`, `ascending`, `descending`] as const)( ascending: `sorted-map.ascending`, descending: `sorted-map.descending`, }[order] + const comparatorStates = + order === `key` + ? [] + : ([ + `restore-bound-below-order-size`, + `placed-row-changed-then-written`, + `deferred-row-changed-before-restore`, + ] as const) it.each([20260912, undefined])( `agrees with full recomputation (seed %s)`, async (seed) => { const reach = createOracleReach( property, [ - `placed-runtime-key-deleted`, - `runtime-key-observed`, `deferred-burst-observed`, `entry-yielded-after-pruning`, + ...comparatorStates, ], { minimum: 5 }, ) @@ -160,11 +228,10 @@ describe.each([`key`, `ascending`, `descending`] as const)( `clear`, `burst`, `prune`, + `mutate`, + `mutate`, ), - key: fc.oneof( - { arbitrary: fc.integer({ min: -3, max: 3 }), weight: 3 }, - { arbitrary: fc.constantFrom(undefined, null), weight: 3 }, - ), + key: fc.integer({ min: -3, max: 3 }), value: fc.integer({ min: -3, max: 3 }), deferOrder: fc.boolean(), observe: fc.boolean(), @@ -173,93 +240,130 @@ describe.each([`key`, `ascending`, `descending`] as const)( ), (actions) => { reach.beginHistory() - const actual = new SortedMap( + // The driver counts comparator calls to check the work law. + let comparatorCalls = 0 + const actual = new SortedMap( order === `key` ? undefined - : order === `ascending` - ? (a, b) => a - b - : (a, b) => b - a, + : (a, b) => { + comparatorCalls++ + return order === `ascending` + ? a.rank - b.rank + : b.rank - a.rank + }, ) const model = new Map() - // Keys written with deferOrder since the last ordered read or - // ordering write; an ordered read must place all of them. + // Keys written or deleted with deferOrder since the last ordered + // read or immediate write; the next ordered read restores them. const deferred = new Set() - // Keys whose ordered slot reflects their current value: written - // without deferral, or read in order since their last write. - const placed = new Set() + // Rows changed in place after they were placed. Once one exists, + // comparator order is no longer promised in this history. + const changedPlaced = new Set() + let orderPromised = true + const restored = () => { + deferred.clear() + } const observe = () => { - for (const key of model.keys()) placed.add(key) if (deferred.size > 16) reach.reach(`deferred-burst-observed`) - if ([...model.keys()].some(isRuntimeKey)) - reach.reach(`runtime-key-observed`) - expectMap(actual, model, order) - deferred.clear() + if (order !== `key`) { + // The first ordered read restores the deferred keys. + const k = [...deferred].filter((key) => model.has(key)).length + const bound = restoreBound(k, model.size - k) + if (k > 0 && bound < model.size - k) + reach.reach(`restore-bound-below-order-size`) + comparatorCalls = 0 + void [...actual.keys()] + expect(comparatorCalls).toBeLessThanOrEqual(bound) + } + restored() + expectMap(actual, model, order, orderPromised) + } + const written = (key: ModelKey) => { + if (changedPlaced.delete(key)) + reach.reach(`placed-row-changed-then-written`) } observe() for (const action of actions) { const key = action.key if (action.kind === `set`) { + written(key) expect( - actual.set(asKey(key), action.value, action.deferOrder), + actual.set(key, { rank: action.value }, action.deferOrder), ).toBe(actual) model.set(key, action.value) if (action.deferOrder) { deferred.add(key) - placed.delete(key) - } else { - deferred.clear() - for (const modelKey of model.keys()) placed.add(modelKey) - } + } else restored() } else if (action.kind === `delete`) { - if (isRuntimeKey(key) && placed.has(key)) - reach.reach(`placed-runtime-key-deleted`) - expect(actual.delete(asKey(key), action.deferOrder)).toBe( + written(key) + expect(actual.delete(key, action.deferOrder)).toBe( model.delete(key), ) - placed.delete(key) if (action.deferOrder) deferred.add(key) - else { - deferred.clear() - for (const modelKey of model.keys()) placed.add(modelKey) + else restored() + } else if (action.kind === `mutate`) { + // Change a stored row object without a write, as a sync + // source that reuses its row objects does. The target is a + // deferred row when deferOrder is set, else a placed row. + const targets = [...model.keys()].filter( + (candidate) => + deferred.has(candidate) === action.deferOrder, + ) + if (targets.length > 0) { + const target = targets[(action.key + 3) % targets.length]! + actual.get(target)!.rank = action.value + model.set(target, action.value) + if (order !== `key`) { + if (action.deferOrder) + reach.reach(`deferred-row-changed-before-restore`) + else { + changedPlaced.add(target) + orderPromised = false + } + } } } else if (action.kind === `burst`) { for (const [burstKey, value] of burstEntries(action.value)) { - actual.set(burstKey, value, true) + written(burstKey) + actual.set(burstKey, { rank: value }, true) model.set(burstKey, value) deferred.add(burstKey) - placed.delete(burstKey) } } else if (action.kind === `prune`) { // Iterate in order, deleting each entry whose value has the // action's parity with deferOrder, as settled transactions // are pruned. The iteration yields the order it began with. const expected = modelOrder(model, order) - const yielded: Array<[number, number]> = [] + const yielded: Array<[ModelKey, number]> = [] let pruned = false - for (const [entryKey, value] of actual) { + for (const [entryKey, row] of actual) { if (pruned) reach.reach(`entry-yielded-after-pruning`) - yielded.push([entryKey, value]) - if ((value - action.value) % 2 === 0) { + yielded.push([entryKey, row.rank]) + if ((row.rank - action.value) % 2 === 0) { expect(actual.delete(entryKey, true)).toBe(true) pruned = true } } - expect(yielded).toEqual(expected) + expectView(yielded, expected, orderPromised) + // Iteration restored the order before the deletes. + restored() for (const [entryKey, value] of expected) { if ((value - action.value) % 2 !== 0) continue + written(entryKey) model.delete(entryKey) deferred.add(entryKey) - placed.delete(entryKey) } } else { actual.clear() model.clear() deferred.clear() - placed.clear() + changedPlaced.clear() + // No slot survives clear, so order is promised again. + orderPromised = true } expect(actual.size).toBe(model.size) - expect(actual.has(asKey(key))).toBe(model.has(key)) - expect(actual.get(asKey(key))).toBe(model.get(key)) + expect(actual.has(key)).toBe(model.has(key)) + expect(actual.get(key)?.rank).toBe(model.get(key)) if ( action.kind === `clear` || ((action.kind === `set` || action.kind === `delete`) && @@ -276,65 +380,20 @@ describe.each([`key`, `ascending`, `descending`] as const)( ...(seed === undefined ? oraclePropertyOptions(100, property) : { seed, numRuns: oracleRuns(100) }), - examples: [ - [ - [ - { - kind: `set`, - key: 1, - value: 3, - deferOrder: true, - observe: false, - }, - { - kind: `set`, - key: -1, - value: 3, - deferOrder: true, - observe: false, - }, - { - kind: `set`, - key: 1, - value: -3, - deferOrder: false, - observe: true, - }, - { - kind: `delete`, - key: -1, - value: 0, - deferOrder: true, - observe: false, - }, - { - kind: `clear`, - key: 0, - value: 0, - deferOrder: false, - observe: true, - }, - { - kind: `set`, - key: -1, - value: 2, - deferOrder: true, - observe: true, - }, - ], - ], - ], + examples: fixedHistories.map((history) => [history]), }, ) + if (process.env.TANSTACK_DB_ORACLE_REACH_REPORT) + console.log(property, seed ?? `random`, [...reach.counts()]) reach.assertReached() }, ) }, ) -it(`rejects wrong default ordering, missing tied entries, and stale overwrites`, () => { - const byValue = new SortedMap((a, b) => a - b) - byValue.set(1, 3).set(-1, 4) +it(`rejects wrong default ordering, missing tied entries, stale overwrites, and lost keys`, () => { + const byValue = new SortedMap((a, b) => a.rank - b.rank) + byValue.set(1, { rank: 3 }).set(-1, { rank: 4 }) expect(() => expectMap( byValue, @@ -345,8 +404,8 @@ it(`rejects wrong default ordering, missing tied entries, and stale overwrites`, `key`, ), ).toThrow() - const missingTie = new SortedMap((a, b) => a - b) - missingTie.set(1, 3) + const missingTie = new SortedMap((a, b) => a.rank - b.rank) + missingTie.set(1, { rank: 3 }) expect(() => expectMap( missingTie, @@ -357,507 +416,34 @@ it(`rejects wrong default ordering, missing tied entries, and stale overwrites`, `ascending`, ), ).toThrow() - const stale = new SortedMap() - stale.set(1, 3) + const stale = new SortedMap() + stale.set(1, { rank: 3 }) expect(() => expectMap(stale, new Map([[1, 2]]), `key`)).toThrow() -}) - -describe(`SortedMap`, () => { - it(`keeps ordered-key movement bounded during descending bulk inserts`, async () => { - // This work counter records the array elements displaced by splice. The - // old ordered-key array moves every earlier key on each descending insert. - // It is a diagnostic for this work class, not a wall-clock threshold. - async function displacedSlots(size: number) { - const map = new SortedMap() - const displaced = await countDisplacedSlots(() => { - for (let key = size - 1; key >= 0; key--) map.set(key, key, true) - }) - return { map, displaced } - } - const small = await displacedSlots(512) - const large = await displacedSlots(2048) - expect(large.displaced).toBeLessThan(2048 * 64) - if (small.displaced > 0) - expect(large.displaced).toBeLessThan(small.displaced * 8) - large.map.restoreOrder() - expect([...large.map.keys()]).toEqual( - Array.from({ length: 2048 }, (_, key) => key), - ) - }) - - it.each([`key`, `ascending`, `descending`] as const)( - `keeps exact order after deferred %s inserts, reorders, and deletes`, - (order) => { - const map = new SortedMap( - order === `key` - ? undefined - : order === `ascending` - ? (a, b) => a - b - : (a, b) => b - a, - ) - const model = new Map() - for (let key = 255; key >= 0; key--) { - map.set(key, key % 17, true) - model.set(key, key % 17) - } - expectMap(map, model, order) - for (let key = 0; key < 256; key += 5) { - map.set(key, -key, true) - model.set(key, -key) - } - expectMap(map, model, order) - for (let key = 0; key < 256; key += 7) { - expect(map.delete(key, true)).toBe(model.delete(key)) - } - expectMap(map, model, order) - }, - ) - - it(`keeps a default-order key iterator live across set and delete`, () => { - const map = new SortedMap() - map.set(1, 1).set(3, 3) - const pending = map.keys() - map.set(2, 2, true) - map.restoreOrder() - expect([...pending]).toEqual([1, 2, 3]) - - const running = map.keys() - expect(running.next().value).toBe(1) - map.delete(2, true) - map.restoreOrder() - expect([...running]).toEqual([3]) - - const resumed = map.keys() - expect(resumed.next().value).toBe(1) - map.set(2, 2, true) - map.restoreOrder() - expect([...resumed]).toEqual([2, 3]) - }) - - it(`keeps mixed string, number, and NaN keys distinct in default order`, () => { - const map = new SortedMap() - for (const key of [2, `2`, Number.NaN, -1, `a`, `0`, 0]) { - map.set(key, 1, true) - } - expect([...map.keys()]).toEqual([`0`, `2`, `a`, -1, 0, 2, Number.NaN]) - map.delete(`2`, true) - map.set(Number.NaN, 2, true) - expect([...map.keys()]).toEqual([`0`, `a`, -1, 0, 2, Number.NaN]) - expect(map.size).toBe(6) - expect(map.get(Number.NaN)).toBe(2) - }) - - it(`retains runtime undefined keys and does not resort clean scans`, () => { - const compare = vi.fn((left: number, right: number) => left - right) - const map = new SortedMap(compare) - const absent = undefined as unknown as string - map.set(`a`, 2, true) - map.set(absent, 1, true) - map.restoreOrder() - expect([...map.keys()]).toEqual([absent, `a`]) - compare.mockClear() - for (let scan = 0; scan < 20; scan++) { - expect([...map.keys()]).toEqual([absent, `a`]) - } - expect(compare).not.toHaveBeenCalled() - expect(map.delete(absent, true)).toBe(true) - map.restoreOrder() - expect([...map.keys()]).toEqual([`a`]) - }) - - it(`preserves insertion behavior around transient nullish numeric keys`, () => { - const map = new SortedMap() - const absent = undefined as unknown as number - map.set(absent, 0, true) - map.set(1, 0, true) - expect([...map.keys()]).toEqual([absent, 1]) - expect(map.delete(absent, true)).toBe(true) - for (let key = 128; key > 1; key--) map.set(key, 0, true) - map.restoreOrder() - expect([...map.keys()]).toEqual( - Array.from({ length: 128 }, (_, i) => i + 1), - ) - }) - - it(`keeps ordered-key movement bounded during deferred bulk deletes and reorders`, async () => { - // A bulk commit can remove or reorder many existing keys, not only add - // new ones. Moving every later key once per removed or reordered key is - // quadratic; the bound allows a constant number of passes. - const size = 2048 - const map = new SortedMap((a, b) => a - b) - for (let key = 0; key < size; key++) map.set(key, key, true) - map.restoreOrder() - const deleted = await countDisplacedSlots(() => { - for (let key = 0; key < size; key += 2) map.delete(key, true) - map.restoreOrder() - }) - expect(deleted).toBeLessThan(size * 64) - const reordered = await countDisplacedSlots(() => { - for (let key = 1; key < size; key += 4) map.set(key, -key, true) - map.restoreOrder() - }) - expect(reordered).toBeLessThan(size * 64) - const expected = Array.from( - { length: size / 2 }, - (_, index) => 2 * index + 1, - ) - .map((key) => [key, key % 4 === 1 ? -key : key] as const) - .sort(([, a], [, b]) => a - b) - .map(([key]) => key) - expect([...map.keys()]).toEqual(expected) - }) - - it(`keeps Map ownership when a placed comparator value was mutated in place`, () => { - // A sync source may reuse a row object it already wrote. Where the mutated - // row sorts is not promised, but deleting or rewriting it must affect only - // its own key: a binary search guided by the stale order would otherwise - // remove a neighbor. - for (const deferOrder of [false, true]) { - const map = new SortedMap( - (left, right) => left.rank - right.rank, - ) - const rows = [`a`, `b`, `c`, `d`, `e`].map((key, rank) => { - const row = { rank } - map.set(key, row) - return row - }) - rows[1]!.rank = 10 - expect(map.delete(`b`, deferOrder)).toBe(true) - map.restoreOrder() - expect([...map.keys()]).toEqual([`a`, `c`, `d`, `e`]) - rows[3]!.rank = -10 - map.set(`d`, { rank: 1.5 }, deferOrder) - map.restoreOrder() - expect([...map.keys()]).toEqual([`a`, `d`, `c`, `e`]) - expect(map.size).toBe(4) - } - }) - - it(`orders mutable comparator values at the batch checkpoint`, () => { - const map = new SortedMap( - (left, right) => left.rank - right.rank, - ) - const first = { rank: 1 } - map.set(`a`, first, true) - map.set(`b`, { rank: 2 }, true) - first.rank = 3 - map.restoreOrder() - expect([...map.keys()]).toEqual([`b`, `a`]) - expect(map.delete(`a`)).toBe(true) - expect([...map.keys()]).toEqual([`b`]) - }) - - it(`bounds ordered-key movement in a bulk Collection sync commit`, async () => { - type Row = { id: number } - let sync!: Parameters[`sync`]>[0] - const collection = createCollection({ - getKey: (row) => row.id, - startSync: true, - sync: { - sync: (actions) => { - sync = actions - actions.markReady() - }, - }, - }) - try { - sync.begin() - for (let id = 4095; id >= 0; id--) - sync.write({ type: `insert`, value: { id } }) - const displaced = await countDisplacedSlots(() => { - expect(sync.commit()).toBe(true) - }) - expect(displaced).toBeLessThan(4096 * 64) - expect([...collection.keys()]).toEqual( - Array.from({ length: 4096 }, (_, id) => id), - ) - - sync.begin() - for (let id = -1; id >= -1024; id--) - sync.write({ type: `insert`, value: { id } }) - const retainedDisplaced = await countDisplacedSlots(() => { - expect(sync.commit()).toBe(true) - }) - expect(retainedDisplaced).toBeLessThan(4096 * 32) - expect([...collection.keys()]).toEqual( - Array.from({ length: 5120 }, (_, index) => index - 1024), - ) - } finally { - await collection.cleanup() - } - }) - - it(`bounds ordered-key movement across queued sync transactions`, async () => { - type Row = { id: number; value: number } - let sync!: Parameters[`sync`]>[0] - let release!: () => void - const persistence = new Promise((resolve) => { - release = resolve - }) - const collection = createCollection({ - getKey: (row) => row.id, - startSync: true, - sync: { - sync: (actions) => { - sync = actions - actions.begin() - actions.write({ type: `insert`, value: { id: 0, value: 0 } }) - actions.commit() - actions.markReady() - }, - }, - }) - const blocker = createTransaction({ - autoCommit: false, - mutationFn: () => persistence, - }) - let blockerCommit: Promise | undefined - const receipts: Array> = [] - try { - await collection.stateWhenReady() - blocker.mutate(() => - collection.update(0, (draft) => { - draft.value = 1 - }), - ) - blockerCommit = blocker.commit() - await Promise.resolve() - expect(blocker.state).toBe(`persisting`) - - for (let batch = 0; batch < 16; batch++) { - sync.begin() - for (let offset = 0; offset < 64; offset++) { - const id = 1024 - batch * 64 - offset - sync.write({ type: `insert`, value: { id, value: id } }) - } - const receipt = sync.commit() - expect(receipt).not.toBe(true) - if (receipt !== true) { - void receipt.catch(() => undefined) - receipts.push(receipt) - } - } - - const displaced = await countDisplacedSlots(async () => { - release() - await blockerCommit - await Promise.all(receipts) - }) - expect(displaced).toBeLessThan(1024 * 64) - expect([...collection.keys()]).toEqual( - Array.from({ length: 1025 }, (_, id) => id), - ) - } finally { - release() - if (blocker.state === `pending` || blocker.state === `persisting`) - blocker.rollback() - await blockerCommit?.catch(() => undefined) - await Promise.allSettled(receipts) - await collection.cleanup() - } - }) - - it(`publishes a large sync transaction in comparator order`, async () => { - type Row = { id: number; rank: number } - let sync!: Parameters[`sync`]>[0] - const compare = vi.fn((left: Row, right: Row) => left.rank - right.rank) - const collection = createCollection({ - getKey: (row) => row.id, - compare, - startSync: true, - sync: { - rowUpdateMode: `full`, - sync: (actions) => { - sync = actions - actions.markReady() - }, - }, - }) - const model = new Map() - const expectedKeys = () => - [...model.values()] - .sort((left, right) => left.rank - right.rank || left.id - right.id) - .map((row) => row.id) - const published: Array> = [] - const subscription = collection.subscribeChanges( - () => published.push([...collection.keys()]), - { includeInitialState: false }, - ) - try { - const pending = collection.keys() - sync.begin() - for (let id = 127; id >= 0; id--) { - const row = { id, rank: id % 7 } - model.set(id, row) - sync.write({ type: `insert`, value: row }) - } - expect(sync.commit()).toBe(true) - expect([...pending]).toEqual(expectedKeys()) - expect([...collection.keys()]).toEqual(expectedKeys()) - expect(published.at(-1)).toEqual(expectedKeys()) - - sync.begin() - for (let id = 0; id < 64; id++) { - const row = { id, rank: -id } - model.set(id, row) - sync.write({ type: `update`, value: row }) - } - for (let id = 64; id < 70; id++) { - model.delete(id) - sync.write({ type: `delete`, key: id }) - } - expect(sync.commit()).toBe(true) - expect([...collection.keys()]).toEqual(expectedKeys()) - expect(published.at(-1)).toEqual(expectedKeys()) - - sync.begin() - for (let id = 128; id < 10000; id++) { - const row = { id, rank: id % 7 } - model.set(id, row) - sync.write({ type: `insert`, value: row }) - } - expect(sync.commit()).toBe(true) - expect([...collection.keys()]).toEqual(expectedKeys()) - - // A small update batch in a large Collection should keep using - // incremental ordering instead of sorting every retained row. - compare.mockClear() - sync.begin() - for (let id = 0; id < 64; id++) { - const row = { id, rank: -id - 100 } - model.set(id, row) - sync.write({ type: `update`, value: row }) - } - expect(sync.commit()).toBe(true) - expect(compare.mock.calls.length).toBeLessThan(5000) - expect([...collection.keys()]).toEqual(expectedKeys()) - expect(published.at(-1)).toEqual(expectedKeys()) - } finally { - subscription.unsubscribe() - await collection.cleanup() - } - }) - it(`sorts by key by default, even when values have the opposite order`, () => { - const map = new SortedMap() - map.set(`c`, 1) - map.set(`a`, 3) - map.set(`b`, 2) - - const values = Array.from(map.values()) - expect(values).toEqual([3, 2, 1]) - }) - - it(`works with custom comparator`, () => { - // Create a map that sorts numbers in descending order - const map = new SortedMap((a, b) => b - a) - map.set(`a`, 1) - map.set(`c`, 3) - map.set(`b`, 2) - - const values = Array.from(map.values()) - expect(values).toEqual([3, 2, 1]) - }) - - it(`correctly handles updates`, () => { - const map = new SortedMap() - map.set(`a`, 1) - map.set(`b`, 3) - map.set(`a`, 2) // update existing key - - expect(map.size).toBe(2) - expect(map.get(`a`)).toBe(2) - const values = Array.from(map.values()) - expect(values).toEqual([2, 3]) - }) - - it(`correctly handles deletions`, () => { - const map = new SortedMap() - map.set(`a`, 1) - map.set(`b`, 2) - map.set(`c`, 3) - - map.delete(`b`) - expect(map.size).toBe(2) - const values = Array.from(map.values()) - expect(values).toEqual([1, 3]) - }) - - it(`implements iteration methods correctly`, () => { - const map = new SortedMap() - map.set(`b`, 2) - map.set(`a`, 1) - map.set(`c`, 3) - - // Test entries() - const entries = Array.from(map.entries()) - expect(entries).toEqual([ - [`a`, 1], - [`b`, 2], - [`c`, 3], - ]) - - // Test values() - const values = Array.from(map.values()) - expect(values).toEqual([1, 2, 3]) - }) - - it(`orders string keys when no custom comparator is provided`, () => { - const map = new SortedMap() - map.set(`c`, `charlie`) - map.set(`a`, `alpha`) - map.set(`b`, `bravo`) - - const values = Array.from(map.values()) - expect(values).toEqual([`alpha`, `bravo`, `charlie`]) - - // Test with values that would be sorted differently by a custom comparator - const numericMap = new SortedMap() - numericMap.set(`a`, `10`) - numericMap.set(`b`, `2`) - - // Default string comparison will put '10' before '2' - const numericValues = Array.from(numericMap.values()) - expect(numericValues).toEqual([`10`, `2`]) - }) - - // Test for keys() method - it(`provides keys in sorted order`, () => { - const map = new SortedMap() - map.set(`c`, 3) - map.set(`a`, 1) - map.set(`b`, 2) - - const keys = Array.from(map.keys()) - expect(keys).toEqual([`a`, `b`, `c`]) - }) - - // Test for Symbol.iterator implementation - it(`supports direct iteration with for...of`, () => { - const map = new SortedMap() - map.set(`c`, 3) - map.set(`a`, 1) - map.set(`b`, 2) - - const entries: Array<[string, number]> = [] - for (const entry of map) { - entries.push(entry) - } - - expect(entries).toEqual([ - [`a`, 1], - [`b`, 2], - [`c`, 3], - ]) - }) - - // Test for clear method - it(`clears all entries`, () => { - const map = new SortedMap() - map.set(`a`, 1) - map.set(`b`, 2) - - map.clear() - expect(map.size).toBe(0) - expect(Array.from(map.entries())).toEqual([]) - }) + // Without the order promise, a view may reorder keys but must hold each + // key once with its current value. + const expected: Array<[ModelKey, number]> = [ + [1, 3], + [2, 3], + ] + expect(() => + expectView( + [ + [2, 3], + [1, 3], + ], + expected, + false, + ), + ).not.toThrow() + expect(() => + expectView( + [ + [1, 3], + [1, 3], + ], + expected, + false, + ), + ).toThrow() + expect(() => expectView([[1, 3]], expected, false)).toThrow() }) diff --git a/packages/db/tests/SortedMap.test.ts b/packages/db/tests/SortedMap.test.ts new file mode 100644 index 0000000000..cfaae78782 --- /dev/null +++ b/packages/db/tests/SortedMap.test.ts @@ -0,0 +1,516 @@ +import { describe, expect, it, vi } from 'vitest' +import { SortedMap } from '../src/SortedMap' +import { createCollection } from '../src/collection/index.js' +import { createTransaction } from '../src/transactions.js' +import type { SyncConfig } from '../src/types.js' + +// Supplemental examples for SortedMap. The generated law lives in +// SortedMap-oracle.test.ts; these pin scale, scan reuse, and public +// Collection sync paths that its small histories do not reach. + +async function countDisplacedSlots( + action: () => void | Promise, +): Promise { + const splice = Array.prototype.splice + let displaced = 0 + const spy = vi.spyOn(Array.prototype, `splice`).mockImplementation(function ( + this: Array, + start: number, + deleteCount?: number, + ...items: Array + ) { + const removed = deleteCount ?? this.length - start + displaced += Math.max(0, this.length - start - removed) + return splice.call(this, start, removed, ...items) + }) + try { + await action() + } finally { + spy.mockRestore() + } + return displaced +} + +type Order = `key` | `ascending` | `descending` + +function expectMap( + actual: SortedMap, + model: Map, + order: Order, +) { + const expected = [...model].sort( + ([ka, va], [kb, vb]) => + (order === `key` ? 0 : order === `ascending` ? va - vb : vb - va) || + ka - kb, + ) + expect([...actual]).toEqual(expected) + expect(actual.size).toBe(model.size) +} + +describe(`SortedMap`, () => { + it(`keeps ordered-key movement bounded during descending bulk inserts`, async () => { + // This work counter records the array elements displaced by splice. The + // old ordered-key array moves every earlier key on each descending insert. + // It is a diagnostic for this work class, not a wall-clock threshold. + async function displacedSlots(size: number) { + const map = new SortedMap() + const displaced = await countDisplacedSlots(() => { + for (let key = size - 1; key >= 0; key--) map.set(key, key, true) + }) + return { map, displaced } + } + const small = await displacedSlots(512) + const large = await displacedSlots(2048) + expect(large.displaced).toBeLessThan(2048 * 64) + if (small.displaced > 0) + expect(large.displaced).toBeLessThan(small.displaced * 8) + large.map.restoreOrder() + expect([...large.map.keys()]).toEqual( + Array.from({ length: 2048 }, (_, key) => key), + ) + }) + + it.each([`key`, `ascending`, `descending`] as const)( + `keeps exact order after deferred %s inserts, reorders, and deletes`, + (order) => { + const map = new SortedMap( + order === `key` + ? undefined + : order === `ascending` + ? (a, b) => a - b + : (a, b) => b - a, + ) + const model = new Map() + for (let key = 255; key >= 0; key--) { + map.set(key, key % 17, true) + model.set(key, key % 17) + } + expectMap(map, model, order) + for (let key = 0; key < 256; key += 5) { + map.set(key, -key, true) + model.set(key, -key) + } + expectMap(map, model, order) + for (let key = 0; key < 256; key += 7) { + expect(map.delete(key, true)).toBe(model.delete(key)) + } + expectMap(map, model, order) + }, + ) + + it(`keeps a default-order key iterator live across set and delete`, () => { + const map = new SortedMap() + map.set(1, 1).set(3, 3) + const pending = map.keys() + map.set(2, 2, true) + map.restoreOrder() + expect([...pending]).toEqual([1, 2, 3]) + + const running = map.keys() + expect(running.next().value).toBe(1) + map.delete(2, true) + map.restoreOrder() + expect([...running]).toEqual([3]) + + const resumed = map.keys() + expect(resumed.next().value).toBe(1) + map.set(2, 2, true) + map.restoreOrder() + expect([...resumed]).toEqual([2, 3]) + }) + + it(`keeps mixed string, number, and NaN keys distinct in default order`, () => { + const map = new SortedMap() + for (const key of [2, `2`, Number.NaN, -1, `a`, `0`, 0]) { + map.set(key, 1, true) + } + expect([...map.keys()]).toEqual([`0`, `2`, `a`, -1, 0, 2, Number.NaN]) + map.delete(`2`, true) + map.set(Number.NaN, 2, true) + expect([...map.keys()]).toEqual([`0`, `a`, -1, 0, 2, Number.NaN]) + expect(map.size).toBe(6) + expect(map.get(Number.NaN)).toBe(2) + }) + + it(`keeps ordered-key movement bounded during deferred bulk deletes and reorders`, async () => { + // A bulk commit can remove or reorder many existing keys, not only add + // new ones. Moving every later key once per removed or reordered key is + // quadratic; the bound allows a constant number of passes. + const size = 2048 + const map = new SortedMap((a, b) => a - b) + for (let key = 0; key < size; key++) map.set(key, key, true) + map.restoreOrder() + const deleted = await countDisplacedSlots(() => { + for (let key = 0; key < size; key += 2) map.delete(key, true) + map.restoreOrder() + }) + expect(deleted).toBeLessThan(size * 64) + const reordered = await countDisplacedSlots(() => { + for (let key = 1; key < size; key += 4) map.set(key, -key, true) + map.restoreOrder() + }) + expect(reordered).toBeLessThan(size * 64) + const expected = Array.from( + { length: size / 2 }, + (_, index) => 2 * index + 1, + ) + .map((key) => [key, key % 4 === 1 ? -key : key] as const) + .sort(([, a], [, b]) => a - b) + .map(([key]) => key) + expect([...map.keys()]).toEqual(expected) + }) + + it(`keeps Map ownership when a placed comparator value was mutated in place`, () => { + // A sync source may reuse a row object it already wrote. Where the mutated + // row sorts is not promised, but deleting or rewriting it must affect only + // its own key: a binary search guided by the stale order would otherwise + // remove a neighbor. + for (const deferOrder of [false, true]) { + const map = new SortedMap( + (left, right) => left.rank - right.rank, + ) + const rows = [`a`, `b`, `c`, `d`, `e`].map((key, rank) => { + const row = { rank } + map.set(key, row) + return row + }) + rows[1]!.rank = 10 + expect(map.delete(`b`, deferOrder)).toBe(true) + map.restoreOrder() + expect([...map.keys()]).toEqual([`a`, `c`, `d`, `e`]) + rows[3]!.rank = -10 + map.set(`d`, { rank: 1.5 }, deferOrder) + map.restoreOrder() + expect([...map.keys()]).toEqual([`a`, `d`, `c`, `e`]) + expect(map.size).toBe(4) + } + }) + + it(`orders mutable comparator values at the batch checkpoint`, () => { + const map = new SortedMap( + (left, right) => left.rank - right.rank, + ) + const first = { rank: 1 } + map.set(`a`, first, true) + map.set(`b`, { rank: 2 }, true) + first.rank = 3 + map.restoreOrder() + expect([...map.keys()]).toEqual([`b`, `a`]) + expect(map.delete(`a`)).toBe(true) + expect([...map.keys()]).toEqual([`b`]) + }) + + it(`bounds ordered-key movement in a bulk Collection sync commit`, async () => { + type Row = { id: number } + let sync!: Parameters[`sync`]>[0] + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + sync: (actions) => { + sync = actions + actions.markReady() + }, + }, + }) + try { + sync.begin() + for (let id = 4095; id >= 0; id--) + sync.write({ type: `insert`, value: { id } }) + const displaced = await countDisplacedSlots(() => { + expect(sync.commit()).toBe(true) + }) + expect(displaced).toBeLessThan(4096 * 64) + expect([...collection.keys()]).toEqual( + Array.from({ length: 4096 }, (_, id) => id), + ) + + sync.begin() + for (let id = -1; id >= -1024; id--) + sync.write({ type: `insert`, value: { id } }) + const retainedDisplaced = await countDisplacedSlots(() => { + expect(sync.commit()).toBe(true) + }) + expect(retainedDisplaced).toBeLessThan(4096 * 32) + expect([...collection.keys()]).toEqual( + Array.from({ length: 5120 }, (_, index) => index - 1024), + ) + } finally { + await collection.cleanup() + } + }) + + it(`bounds ordered-key movement across queued sync transactions`, async () => { + type Row = { id: number; value: number } + let sync!: Parameters[`sync`]>[0] + let release!: () => void + const persistence = new Promise((resolve) => { + release = resolve + }) + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: { id: 0, value: 0 } }) + actions.commit() + actions.markReady() + }, + }, + }) + const blocker = createTransaction({ + autoCommit: false, + mutationFn: () => persistence, + }) + let blockerCommit: Promise | undefined + const receipts: Array> = [] + try { + await collection.stateWhenReady() + blocker.mutate(() => + collection.update(0, (draft) => { + draft.value = 1 + }), + ) + blockerCommit = blocker.commit() + await Promise.resolve() + expect(blocker.state).toBe(`persisting`) + + for (let batch = 0; batch < 16; batch++) { + sync.begin() + for (let offset = 0; offset < 64; offset++) { + const id = 1024 - batch * 64 - offset + sync.write({ type: `insert`, value: { id, value: id } }) + } + const receipt = sync.commit() + expect(receipt).not.toBe(true) + if (receipt !== true) { + void receipt.catch(() => undefined) + receipts.push(receipt) + } + } + + const displaced = await countDisplacedSlots(async () => { + release() + await blockerCommit + await Promise.all(receipts) + }) + expect(displaced).toBeLessThan(1024 * 64) + expect([...collection.keys()]).toEqual( + Array.from({ length: 1025 }, (_, id) => id), + ) + } finally { + release() + if (blocker.state === `pending` || blocker.state === `persisting`) + blocker.rollback() + await blockerCommit?.catch(() => undefined) + await Promise.allSettled(receipts) + await collection.cleanup() + } + }) + + it(`publishes a large sync transaction in comparator order`, async () => { + type Row = { id: number; rank: number } + let sync!: Parameters[`sync`]>[0] + const compare = vi.fn((left: Row, right: Row) => left.rank - right.rank) + const collection = createCollection({ + getKey: (row) => row.id, + compare, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.markReady() + }, + }, + }) + const model = new Map() + const expectedKeys = () => + [...model.values()] + .sort((left, right) => left.rank - right.rank || left.id - right.id) + .map((row) => row.id) + const published: Array> = [] + const subscription = collection.subscribeChanges( + () => published.push([...collection.keys()]), + { includeInitialState: false }, + ) + try { + const pending = collection.keys() + sync.begin() + for (let id = 127; id >= 0; id--) { + const row = { id, rank: id % 7 } + model.set(id, row) + sync.write({ type: `insert`, value: row }) + } + expect(sync.commit()).toBe(true) + expect([...pending]).toEqual(expectedKeys()) + expect([...collection.keys()]).toEqual(expectedKeys()) + expect(published.at(-1)).toEqual(expectedKeys()) + + sync.begin() + for (let id = 0; id < 64; id++) { + const row = { id, rank: -id } + model.set(id, row) + sync.write({ type: `update`, value: row }) + } + for (let id = 64; id < 70; id++) { + model.delete(id) + sync.write({ type: `delete`, key: id }) + } + expect(sync.commit()).toBe(true) + expect([...collection.keys()]).toEqual(expectedKeys()) + expect(published.at(-1)).toEqual(expectedKeys()) + + sync.begin() + for (let id = 128; id < 10000; id++) { + const row = { id, rank: id % 7 } + model.set(id, row) + sync.write({ type: `insert`, value: row }) + } + expect(sync.commit()).toBe(true) + expect([...collection.keys()]).toEqual(expectedKeys()) + + // A small update batch in a large Collection should keep using + // incremental ordering instead of sorting every retained row. + compare.mockClear() + sync.begin() + for (let id = 0; id < 64; id++) { + const row = { id, rank: -id - 100 } + model.set(id, row) + sync.write({ type: `update`, value: row }) + } + expect(sync.commit()).toBe(true) + expect(compare.mock.calls.length).toBeLessThan(5000) + expect([...collection.keys()]).toEqual(expectedKeys()) + expect(published.at(-1)).toEqual(expectedKeys()) + } finally { + subscription.unsubscribe() + await collection.cleanup() + } + }) + it(`sorts by key by default, even when values have the opposite order`, () => { + const map = new SortedMap() + map.set(`c`, 1) + map.set(`a`, 3) + map.set(`b`, 2) + + const values = Array.from(map.values()) + expect(values).toEqual([3, 2, 1]) + }) + + it(`works with custom comparator`, () => { + // Create a map that sorts numbers in descending order + const map = new SortedMap((a, b) => b - a) + map.set(`a`, 1) + map.set(`c`, 3) + map.set(`b`, 2) + + const values = Array.from(map.values()) + expect(values).toEqual([3, 2, 1]) + }) + + it(`correctly handles updates`, () => { + const map = new SortedMap() + map.set(`a`, 1) + map.set(`b`, 3) + map.set(`a`, 2) // update existing key + + expect(map.size).toBe(2) + expect(map.get(`a`)).toBe(2) + const values = Array.from(map.values()) + expect(values).toEqual([2, 3]) + }) + + it(`correctly handles deletions`, () => { + const map = new SortedMap() + map.set(`a`, 1) + map.set(`b`, 2) + map.set(`c`, 3) + + map.delete(`b`) + expect(map.size).toBe(2) + const values = Array.from(map.values()) + expect(values).toEqual([1, 3]) + }) + + it(`implements iteration methods correctly`, () => { + const map = new SortedMap() + map.set(`b`, 2) + map.set(`a`, 1) + map.set(`c`, 3) + + // Test entries() + const entries = Array.from(map.entries()) + expect(entries).toEqual([ + [`a`, 1], + [`b`, 2], + [`c`, 3], + ]) + + // Test values() + const values = Array.from(map.values()) + expect(values).toEqual([1, 2, 3]) + }) + + it(`orders string keys when no custom comparator is provided`, () => { + const map = new SortedMap() + map.set(`c`, `charlie`) + map.set(`a`, `alpha`) + map.set(`b`, `bravo`) + + const values = Array.from(map.values()) + expect(values).toEqual([`alpha`, `bravo`, `charlie`]) + + // Test with values that would be sorted differently by a custom comparator + const numericMap = new SortedMap() + numericMap.set(`a`, `10`) + numericMap.set(`b`, `2`) + + // Default string comparison will put '10' before '2' + const numericValues = Array.from(numericMap.values()) + expect(numericValues).toEqual([`10`, `2`]) + }) + + // Test for keys() method + it(`provides keys in sorted order`, () => { + const map = new SortedMap() + map.set(`c`, 3) + map.set(`a`, 1) + map.set(`b`, 2) + + const keys = Array.from(map.keys()) + expect(keys).toEqual([`a`, `b`, `c`]) + }) + + // Test for Symbol.iterator implementation + it(`supports direct iteration with for...of`, () => { + const map = new SortedMap() + map.set(`c`, 3) + map.set(`a`, 1) + map.set(`b`, 2) + + const entries: Array<[string, number]> = [] + for (const entry of map) { + entries.push(entry) + } + + expect(entries).toEqual([ + [`a`, 1], + [`b`, 2], + [`c`, 3], + ]) + }) + + // Test for clear method + it(`clears all entries`, () => { + const map = new SortedMap() + map.set(`a`, 1) + map.set(`b`, 2) + + map.clear() + expect(map.size).toBe(0) + expect(Array.from(map.entries())).toEqual([]) + }) +}) diff --git a/packages/db/tests/collection.test.ts b/packages/db/tests/collection.test.ts index 64b856dbc0..6cff571a75 100644 --- a/packages/db/tests/collection.test.ts +++ b/packages/db/tests/collection.test.ts @@ -14,6 +14,7 @@ import { MissingDeleteHandlerError, MissingInsertHandlerError, MissingUpdateHandlerError, + UndefinedKeyError, } from '../src/errors' import { createTransaction } from '../src/transactions' import { createLiveQueryCollection, eq } from '../src/query/index.js' @@ -1273,6 +1274,45 @@ describe(`Collection`, () => { }).toThrow(InvalidKeyError) }) + it.each([ + [`undefined`, undefined, UndefinedKeyError], + [`null`, null, InvalidKeyError], + [`a boolean`, true, InvalidKeyError], + [`a bigint`, 10n, InvalidKeyError], + ])( + `rejects a sync write whose key is %s before staging it`, + async (_name, badKey, error) => { + // Collection keys are strings or numbers. A sync adapter that writes + // another key breaks that contract, so the write throws at the sync + // boundary instead of reaching ordered Collection state. + type Row = { id: unknown; value: string } + let sync!: Parameters[`sync`]>[0] + const collection = createCollection({ + id: `sync-invalid-key-${String(badKey)}`, + getKey: (item) => item.id as string | number, + startSync: true, + sync: { + sync: (actions) => { + sync = actions + actions.markReady() + }, + }, + }) + await collection.stateWhenReady() + sync.begin() + expect(() => + sync.write({ type: `insert`, value: { id: badKey, value: `bad` } }), + ).toThrow(error) + expect(() => + sync.write({ type: `delete`, key: badKey as string | number }), + ).toThrow(error) + sync.write({ type: `insert`, value: { id: 1, value: `good` } }) + sync.commit() + expect([...collection.keys()]).toEqual([1]) + await collection.cleanup() + }, + ) + it(`should accept valid string and number keys`, async () => { const stringKeyCollection = createCollection<{ id: string; value: string }>( { diff --git a/packages/db/tests/oracle-config.ts b/packages/db/tests/oracle-config.ts index bcf9de7f68..e25cf6bb1f 100644 --- a/packages/db/tests/oracle-config.ts +++ b/packages/db/tests/oracle-config.ts @@ -110,6 +110,7 @@ const staticOracleProperties = [ `query-identity.equality-partition`, `where-predicate.publication`, `join-result-key.pairs`, + `group-result-key.groups`, `pooled-live-query.publication`, `flat-change-tracking.equivalence`, `local-storage.value-fidelity`, diff --git a/packages/db/tests/query/group-by.test.ts b/packages/db/tests/query/group-by.test.ts index 0021d933a0..e8b5629367 100644 --- a/packages/db/tests/query/group-by.test.ts +++ b/packages/db/tests/query/group-by.test.ts @@ -1565,8 +1565,13 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { expect(salesRep2?.total_amount).toBe(850) // 150+300+400 expect(salesRep2?.order_count).toBe(3) - // No sales rep (null): order 5 - null becomes the direct value as key - const noSalesRep = salesRepSummary.get(null as any) + // No sales rep (null): order 5. Collection keys are strings or + // numbers, so the null group gets a string key. + const nullGroup = salesRepSummary.toArray.find( + (row) => row.sales_rep_id === null, + ) + expect(typeof nullGroup?.$key).toBe(`string`) + const noSalesRep = salesRepSummary.get(nullGroup!.$key) expect(noSalesRep?.sales_rep_id).toBeNull() expect(noSalesRep?.total_amount).toBe(250) expect(noSalesRep?.order_count).toBe(1) diff --git a/packages/db/tests/query/group-result-key-oracle.property.test.ts b/packages/db/tests/query/group-result-key-oracle.property.test.ts new file mode 100644 index 0000000000..5c64104fb0 --- /dev/null +++ b/packages/db/tests/query/group-result-key-oracle.property.test.ts @@ -0,0 +1,291 @@ +/** + * # Does every group keep a key of the Collection's key type? + * + * Law and source: a Collection's keys are strings or numbers (the `TKey` + * constraint of every Collection type, and the `InvalidKeyError` that + * optimistic mutations throw for any other key). A grouped live-query + * Collection publishes one row per group, so each group needs its own key of + * that type, whatever its group value is. Grouping by a nullable field is a + * valid query, so a `null` or `undefined` group value must still produce a + * string or number key. Maintainer decision (2026-10-27): the grouped key + * matches the declared key type, rather than the type widening to nullish + * keys. The guide does not fix the key's format, so the law fixes only its + * type, uniqueness, and stability: + * + * - every published key is a string or a number; + * - two groups never share a key, including a group value and a string that + * prints like it (`null` and `"null"`, `true` and `"true"`) or a string in + * the namespace TanStack DB reserves for its own encodings; + * - a group that still exists after a change keeps its key, so the change + * publishes as an update of the same row. + * + * Model: `expectedGroups` partitions plain rows by an independent rule. Group + * values are equal when they have the same type and the same value, with + * `NaN` equal to `NaN`. It does not import the compiler, its value identity, + * or its key encoding. + * + * History grammar: rows draw group values from strings, numbers, `NaN`, + * `null`, `undefined`, booleans, a bigint, strings that print like those + * values, and strings in the reserved `\u0000tanstack-db:` namespace. Each + * history then applies up to four synced changes: move a row to another group + * value, delete a row, or insert a row. + * + * Production driver: a `mockSyncCollectionOptions` Collection and a public + * `createLiveQueryCollection` that groups by the field and counts each group. + * + * Refinement check: after preload and after each synced change, the published + * (group value, count) pairs equal the model's, every key is a string or a + * number, the distinct key count equals the row count, and each group present + * at the previous checkpoint has the same key. + * + * Known omissions: Dates, -0, objects, and multi-field groups are outside + * this owner; the aggregate select oracle owns grouped values. The mock sync + * source is a controlled provider; this oracle claims only the compiler's + * keying of the rows it supplies. + */ +import { describe, expect, it } from 'vitest' +import fc from 'fast-check' +import { createLiveQueryCollection } from '../../src/query/index.js' +import { count } from '../../src/query/builder/functions.js' +import { createCollection } from '../../src/collection/index.js' +import { oraclePropertyOptions, oracleRuns } from '../oracle-config.js' +import { createOracleReach } from '../oracle-reach.js' +import { mockSyncCollectionOptions, withOracleCleanup } from '../utils.js' + +const property = `group-result-key.groups` + +type Row = { id: number; g: unknown } +type Change = + | { kind: `move`; index: number; g: unknown } + | { kind: `delete`; index: number } + | { kind: `insert`; g: unknown } +type History = { rows: Array; changes: Array } + +// Values that a key encoding by `String(value)` cannot tell apart, values +// outside the key type, and strings in the reserved namespace that a naive +// encoding of those values could produce. +const groupDomain: ReadonlyArray = [ + `a`, + 1, + 2, + `1`, + Number.NaN, + null, + undefined, + true, + false, + 10n, + `null`, + `undefined`, + `true`, + `10`, + `\u0000tanstack-db:object:null`, + `\u0000tanstack-db:boolean:true`, +] + +// --------------------------------------------------------------------------- +// Model +// --------------------------------------------------------------------------- + +// Two group values are equal when their type and value are equal. +function describeGroup(value: unknown): string { + return `${typeof value}:${typeof value === `number` && Number.isNaN(value) ? `NaN` : String(value)}` +} + +// The (group, count) pairs the query must publish. +function expectedGroups(rows: ReadonlyArray): Map { + const groups = new Map() + for (const row of rows) { + const group = describeGroup(row.g) + groups.set(group, (groups.get(group) ?? 0) + 1) + } + return groups +} + +// --------------------------------------------------------------------------- +// History grammar +// --------------------------------------------------------------------------- + +const groupValue = fc.constantFrom(...groupDomain) + +const historyArbitrary: fc.Arbitrary = fc.record({ + rows: fc.array(groupValue, { minLength: 1, maxLength: 6 }), + changes: fc.array( + fc.oneof( + fc.record({ + kind: fc.constant(`move` as const), + index: fc.nat({ max: 7 }), + g: groupValue, + }), + fc.record({ + kind: fc.constant(`delete` as const), + index: fc.nat({ max: 7 }), + }), + fc.record({ kind: fc.constant(`insert` as const), g: groupValue }), + ), + { maxLength: 4 }, + ), +}) + +// Each lookalike pair publishes together, then one member's group changes. +const pinnedHistories: ReadonlyArray = [ + { + rows: [null, `null`, undefined, `undefined`], + changes: [{ kind: `move`, index: 0, g: `null` }], + }, + { + rows: [true, `true`, false, 10n, `10`], + changes: [{ kind: `insert`, g: true }], + }, + { + rows: [null, `\u0000tanstack-db:object:null`, true], + changes: [ + { kind: `insert`, g: `\u0000tanstack-db:boolean:true` }, + { kind: `delete`, index: 0 }, + ], + }, +] + +// --------------------------------------------------------------------------- +// Production driver and refinement check +// --------------------------------------------------------------------------- + +type Reach = ReturnType< + typeof createOracleReach< + | `nullish-group-published` + | `lookalike-groups-published` + | `group-key-kept-across-change` + > +> + +let collectionSerial = 0 + +async function runHistory(history: History, reach?: Reach): Promise { + reach?.beginHistory() + const serial = collectionSerial++ + let nextId = 0 + const rows: Array = history.rows.map((g) => ({ id: nextId++, g })) + const source = createCollection( + mockSyncCollectionOptions({ + id: `group-key-source-${serial}`, + getKey: (row) => row.id, + initialData: rows.map((row) => ({ ...row })), + }), + ) + const live = createLiveQueryCollection((q) => + q + .from({ row: source }) + .groupBy(({ row }) => row.g) + .select(({ row }) => ({ g: row.g, n: count(row.id) })), + ) + let previousKeys = new Map() + const check = (checkpoint: string) => { + const published = new Map() + const keys = new Map() + for (const [key, row] of live.entries()) { + expect( + typeof key === `string` || typeof key === `number`, + `${checkpoint}: key ${String(key)} of type ${typeof key}`, + ).toBe(true) + const group = describeGroup((row as { g: unknown }).g) + published.set(group, (row as { n: number }).n) + keys.set(group, key) + } + expect([...published].sort(), checkpoint).toEqual( + [...expectedGroups(rows)].sort(), + ) + expect(new Set(keys.values()).size, `${checkpoint}: distinct keys`).toBe( + published.size, + ) + expect(live.size, `${checkpoint}: key count`).toBe(published.size) + for (const [group, key] of keys) { + if (!previousKeys.has(group)) continue + reach?.reach(`group-key-kept-across-change`) + expect( + Object.is(previousKeys.get(group), key), + `${checkpoint}: ${group} keeps its key`, + ).toBe(true) + } + previousKeys = keys + const groups = [...published.keys()] + if ( + groups.some( + (group) => + group.startsWith(`object:null`) || group.startsWith(`undefined:`), + ) + ) + reach?.reach(`nullish-group-published`) + const printed = new Set( + groups + .filter((group) => !group.startsWith(`string:`)) + .map((group) => group.slice(group.indexOf(`:`) + 1)), + ) + if ( + groups.some( + (group) => group.startsWith(`string:`) && printed.has(group.slice(7)), + ) + ) + reach?.reach(`lookalike-groups-published`) + } + await withOracleCleanup(async () => { + await live.preload() + check(`after preload`) + for (const [step, change] of history.changes.entries()) { + source.utils.begin() + if (change.kind === `insert`) { + const row = { id: nextId++, g: change.g } + rows.push(row) + source.utils.write({ type: `insert`, value: { ...row } }) + } else { + if (rows.length === 0) { + source.utils.commit() + continue + } + const index = change.index % rows.length + const target = rows[index]! + if (change.kind === `delete`) { + rows.splice(index, 1) + source.utils.write({ type: `delete`, value: { ...target } }) + } else { + target.g = change.g + source.utils.write({ type: `update`, value: { ...target } }) + } + } + source.utils.commit() + check(`after change ${step} (${change.kind})`) + } + }, [() => live.cleanup(), () => source.cleanup()]) +} + +describe(`grouped result key oracle`, () => { + for (const [index, history] of pinnedHistories.entries()) { + it(`keeps lookalike group values apart (pinned ${index})`, () => + runHistory(history)) + } + + it.each([92_604_118, undefined])( + `publishes one key of the key type per group (seed %s)`, + async (seed) => { + const reach = createOracleReach( + property, + [ + `nullish-group-published`, + `lookalike-groups-published`, + `group-key-kept-across-change`, + ], + { minimum: 5 }, + ) + await fc.assert( + fc.asyncProperty(historyArbitrary, (history) => + runHistory(history, reach), + ), + seed === undefined + ? oraclePropertyOptions(80, property) + : { seed, numRuns: oracleRuns(80) }, + ) + if (process.env.TANSTACK_DB_ORACLE_REACH_REPORT) + console.log(property, seed ?? `random`, [...reach.counts()]) + reach.assertReached() + }, + ) +}) From bad8ac24bb2bc715897f0961000db4dc87809bd4 Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 09:22:02 -0600 Subject: [PATCH 5/9] test(db): cast invalid sync keys through unknown Co-authored-by: Isaac --- packages/db/tests/collection.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/db/tests/collection.test.ts b/packages/db/tests/collection.test.ts index 6cff571a75..f6dba1745b 100644 --- a/packages/db/tests/collection.test.ts +++ b/packages/db/tests/collection.test.ts @@ -1304,7 +1304,10 @@ describe(`Collection`, () => { sync.write({ type: `insert`, value: { id: badKey, value: `bad` } }), ).toThrow(error) expect(() => - sync.write({ type: `delete`, key: badKey as string | number }), + sync.write({ + type: `delete`, + key: badKey as unknown as string | number, + }), ).toThrow(error) sync.write({ type: `insert`, value: { id: 1, value: `good` } }) sync.commit() From 16d1081092e16dea6996b94b2c8a298511895406 Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 09:24:43 -0600 Subject: [PATCH 6/9] docs: record why invalid sync keys have an invariant witness Co-authored-by: Isaac --- docs/contributing/oracle-coverage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/oracle-coverage.md b/docs/contributing/oracle-coverage.md index 6f30babd5b..ec5bbdbb05 100644 --- a/docs/contributing/oracle-coverage.md +++ b/docs/contributing/oracle-coverage.md @@ -68,7 +68,7 @@ that test identifiers must copy production's private data structures. | Indexed predicate filtering | Complete for bounded dotted-path collision | An independent row filter checks direct predicates and public callback predicates for a dotted scalar field beside a nested path, with and without an index. Selected-field ordering checks the same distinction through `$selected`. | | WHERE predicate publication | Complete for bounded predicate and sync-transaction grammar | The contract, Kleene reference evaluator, snapshot and change-history grammar, three subscriber consumers plus direct snapshot, and per-commit key-set refinement check are literate. A fixed same-key update checks live-row and direct-subscriber payloads; a focused descriptor boundary checks stored-row prefilter safety. Generated cleanup and restart histories for filtered subscribers remain open. | | Joined result keys | Complete for bounded two-source key grammar | The contract, nested-loop pair model, delimiter-, number-like, infinite, and `NaN` key grammar, public join driver, and per-checkpoint pair and key-count check are literate. Joins over subqueries, more than two sources, custom `getKey`, and optimistic mutations remain outside this owner. | -| Grouped result keys | [Grouped result key oracle](https://github.com/TanStack/db/blob/main/packages/db/tests/query/group-result-key-oracle.property.test.ts) | A grouped live-query Collection gives each group one key of the Collection key type, a string or a number, whatever the group value. Maintainer decision (2026-10-27): grouped keys match the declared key type rather than widening it to nullish keys. Group values come from strings, numbers, `NaN`, `null`, `undefined`, booleans, a bigint, strings that print like those values, and strings in the reserved `\u0000tanstack-db:` namespace; histories move, delete, and insert rows. After preload and each synced change, the (group value, count) pairs equal an independent partition by type and value, every key is a string or number, no two groups share a key, and a group that survives a change keeps its key. Reach (minimum 5, per campaign of 80): a nullish group published (32 to 38), a group value published beside a string that prints like it (18 to 21), and a key kept across a change (49 to 53). RED on the parent commit: keys `undefined`, `null`, `false`, and `10n`. Mutants that encode with `String(value)` (lookalike keys collide) or that encode only nullish values (boolean keys) fail. Encoding only the internal D2 key is equivalent in this domain, because the Collection keys rows by the public group key. A focused witness in `collection.test.ts` checks that `sync.write` rejects `undefined`, `null`, boolean, and bigint keys before staging them. Not covered: Dates, `-0`, objects, and multi-field groups, whose values the aggregate select oracle owns; real sync adapters, since the source is a controlled mock. | +| Grouped result keys | [Grouped result key oracle](https://github.com/TanStack/db/blob/main/packages/db/tests/query/group-result-key-oracle.property.test.ts) | A grouped live-query Collection gives each group one key of the Collection key type, a string or a number, whatever the group value. Maintainer decision (2026-10-27): grouped keys match the declared key type rather than widening it to nullish keys. Group values come from strings, numbers, `NaN`, `null`, `undefined`, booleans, a bigint, strings that print like those values, and strings in the reserved `\u0000tanstack-db:` namespace; histories move, delete, and insert rows. After preload and each synced change, the (group value, count) pairs equal an independent partition by type and value, every key is a string or number, no two groups share a key, and a group that survives a change keeps its key. Reach (minimum 5, per campaign of 80): a nullish group published (32 to 38), a group value published beside a string that prints like it (18 to 21), and a key kept across a change (49 to 53). RED on the parent commit: keys `undefined`, `null`, `false`, and `10n`. Mutants that encode with `String(value)` (lookalike keys collide) or that encode only nullish values (boolean keys) fail. Encoding only the internal D2 key is equivalent in this domain, because the Collection keys rows by the public group key. A focused witness in `collection.test.ts` checks that `sync.write` rejects `undefined`, `null`, boolean, and bigint keys before staging them, and that a later valid write commits alone. A sync adapter that writes such a key breaks its contract, so this is a fail-fast invariant witness under AGENTS.md ("Separate Valid Edge Cases from Contract Contradictions"), not a generated owner; SortedMap never receives these keys. Not covered: Dates, `-0`, objects, and multi-field groups, whose values the aggregate select oracle owns; real sync adapters, since the source is a controlled mock. | | D2 Index storage | Complete for bounded prefix grammar | The contract, plain-`Map` multiset model, prefixed and unprefixed value grammar, `Index` driver, and per-addition `get`/`has` check are literate. Compaction, presence tracking, and structural payloads remain outside this owner. | | Pooled live queries | Complete for bounded eq-filter grammar | The contract, independent `eq` model, live-query Collection second formulation, sync/optimistic/mount/cleanup-restart grammar, observer driver, and per-step refinement check are literate. On-demand and persisted sources, `DbClient`, Suspense, and every clause beyond `eq` conjuncts keep the live-query Collection. | | Live-query deferred acquisition | Complete for bounded synchronous grammar | The law, independent model, history grammar over source states, shapes, and depth, real-Collection driver, and per-step refinement check are literate. Pooled views and DbClient stream preloads have their own blocks. The grammar also adds a peer consumer starting the source and a source truncate before the first subscriber; pinned blocks cover a subscriber that outlives cleanup, reads that wait for readiness, and release of two independent descriptor sources when one start fails. Asynchronous sources, several live-query consumers, and general failure histories stay open. | From c385f6eed162fca2f9a997f02ecababfff077294 Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 10:59:19 -0600 Subject: [PATCH 7/9] fix(db): match NaN slots in SortedMap and document grouped keys - SortedMap's slot check compares keys as Map does, so deleting or rewriting a placed NaN key no longer removes the last key. The SortedMap oracle now generates NaN keys and declares reach for a placed NaN write. - Move assertCollectionKey to internal utils so it is not public API. - The live-queries guide states that null, undefined, boolean, and bigint group values get string keys, read from $key. Co-authored-by: Isaac --- docs/contributing/oracle-coverage.md | 2 +- docs/guides/live-queries.md | 2 +- packages/db/src/SortedMap.ts | 9 +++- packages/db/src/collection/mutations.ts | 2 +- packages/db/src/collection/sync.ts | 2 +- packages/db/src/errors.ts | 9 ---- packages/db/src/utils.ts | 10 ++++ packages/db/tests/SortedMap-oracle.test.ts | 54 +++++++++++++++++++--- 8 files changed, 68 insertions(+), 22 deletions(-) diff --git a/docs/contributing/oracle-coverage.md b/docs/contributing/oracle-coverage.md index ec5bbdbb05..30733806cf 100644 --- a/docs/contributing/oracle-coverage.md +++ b/docs/contributing/oracle-coverage.md @@ -1065,7 +1065,7 @@ generic visitor behavior outside the named collectors. | Index suggestions | [index suggestion oracle](https://github.com/TanStack/db/blob/main/packages/db/tests/index-suggestion-oracle.test.ts) | A public filtered live-query Collection checks manual/default, eager, threshold, matching-index, and unrelated-index cases against the documented collection-size suggestion policy. A bounded ordered-query decision table separately classifies no field index, a null-placement mismatch, matching but order-ineligible indexes, and usable candidates in either registration order. It compares the development warning after preload; the matching-index selection owner below checks the chosen reader and ordered request. Two cyclic locale-options cells require the diagnostic to preserve valid query construction with or without a field index, and a custom-collation cell requires remediation to preserve comparator reference identity. The original manual-silence/eager-noise implementation failed two expected observations. The issue #2190 baseline failed four diagnostic cells, including a first-unusable/second-usable candidate order; the first formatter draft failed all three non-JSON-safe remediation cells. The owner does not judge repeated-query warning volume, slow-query timing, production-build suppression, every comparison-option mismatch, or every query shape. It runs in `@tanstack/db`'s `test:oracles` campaign. | | Minified DB public API | [consumer bundle check](https://github.com/TanStack/db/blob/main/scripts/test-minified-db.mjs) | CI builds `@tanstack/db`, then bundles its built `dist` entry with db-ivm inlined through esbuild `minify: true`. The build renames TypeScript-private members from `packages/db/mangle-cache.json`, so this lane runs against the renamed output that consumers install. It rejects a `dist` that is older than `src`, the cache, or the package's build configuration. `pnpm --filter @tanstack/db test:dist` also runs five db test files against the built `dist`, chosen for coverage of the modules with the most renamed members. `pnpm check:mangle` fails when a cached name is used other than as a private member in db src, is read by another package, appears as a string, or has a short name that collides with a source identifier ([private-member mangling review](oracle-reviews/code-weight-private-member-mangling.md)). It checks every exported error class's current public `name`, the built-in `BasicIndex` resolver name in index metadata and its event, complete query rows (after removing the four documented virtual fields) against an independent array filter/sort/projection, and one live update. The `new.target.name`, public-member mangle, and extra-output-field calibration modes fail at the intended observations. This fixed slice does not replace the unminified generated oracles, exercise framework adapters, or establish stable names for custom index resolvers. | | Boundary refinements | [cleanup/restart](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-cleanup-restart-oracle.test.ts), [idle cleanup](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-idle-cleanup.test.ts), [issue #1891 async-cleanup review](oracle-reviews/issue-1891-async-cleanup.md), [issue #1891 cleanup-start follow-up](oracle-reviews/issue-1891-cleanup-start-followup.md), [metadata publication](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-metadata-publication-oracle.property.test.ts), [row metadata composition](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-row-metadata-composition-oracle.test.ts), [nested-begin metadata rebuild review](oracle-reviews/2026-10-06-nested-begin-metadata-rebuild.md), [state mutation round 4](oracle-reviews/2026-10-06-state-mutation-round-4.md), [state retention](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-state-retention-oracle.property.test.ts), [acquisition cells](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-subscription-lifecycle-oracle.test.ts), [D2 source reconciliation](https://github.com/TanStack/db/blob/main/packages/db/tests/d2-source-reconciliation-oracle.property.test.ts), [top-K support windows](https://github.com/TanStack/db/blob/main/packages/db-ivm/tests/operators/topk-support-window-oracle.test.ts), [nested Query work](https://github.com/TanStack/db/blob/main/packages/query-db-collection/tests/includes-work-counter-oracle.test.ts) | Explicit lifecycle products, independent source maps and weighted relations, exact publication cuts, support/multiplicity, and value-plus-work observations. These refine the larger subsystem models; they do not replace them. | -| Small structures and test mechanics | [SortedMap](https://github.com/TanStack/db/blob/main/packages/db/tests/SortedMap-oracle.test.ts), [cleanup queue](https://github.com/TanStack/db/blob/main/packages/db/tests/cleanup-queue-oracle.property.test.ts), [live-query GC clock](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-gc-clock.test.ts), [guarded replay](https://github.com/TanStack/db/blob/main/packages/db/tests/oracle-replay.test.ts) | Map/full-sort and elapsed appointment-list models with executed target/seed/path checks. SortedMap promises what Collection state uses, not full Map parity (maintainer decision). Keys are strings or numbers: `sync.write` and mutations throw `InvalidKeyError` or `UndefinedKeyError` for any other key, and grouped live queries key nullish, boolean, and bigint group values with strings (maintainer decision, 2026-10-27). The model is a Map of numbers plus a full sort; production stores row objects so the driver can change one in place. Generated histories mix deferred and immediate sets and deletes, clear, deferred bursts of 17 to 41 keys, prunes that delete entries with deferOrder while iterating, and in-place changes of a deferred or a placed row. A prune must yield the order it began with. A row changed before restore sorts by its current value; once a placed row changes in place, comparator order downgrades to membership (each key once, current value) for the rest of the history, and a later write or delete affects only its own key. At each ordered read with a comparator, restoring k deferred keys among n others makes at most k * (ceil(log2 k) + ceil(log2 (n + 1)) + 2) comparator calls, and none with nothing deferred. Declared reach (minimum 5, per campaign of 100): burst restorations, an entry yielded after a prune deleted one, a restore whose bound is below n (21 to 29), a changed placed row later written (19 to 23), and a deferred row changed before restore (9 to 18). Mutants that re-sort every key (108 calls against a bound of 7), re-sort the kept keys with the dirty keys appended (34 against 7), re-sort clean scans, drop the scan fallback for a changed row, keep dirty slots, apply deferred deletes immediately, or place a deferred row by its value at the write each fail. Supplemental examples in `SortedMap.test.ts` pin splice movement at scale, a key iterator opened before a deferred restore, scan reuse, and public single-transaction and queued-transaction Collection sync paths. Not promised: other mutation during iteration (immediate writes, clear), the order of rows once a placed row changed in place, keys outside the key type, NaN and -0 keys, a comparator that throws, and the work of key order without a comparator, which the public API cannot observe. `forEach` was removed; SortedMap is not exported. Wall-clock steps do not move due times, and a focused public witness checks source subscription release. Callback-reentrant scheduling, OS suspend/resume, environments without `performance.now()`, and fake/real Performance clock replacement while GC is pending remain outside this coverage. Tests must drain or reset pending GC before switching timer providers. | +| Small structures and test mechanics | [SortedMap](https://github.com/TanStack/db/blob/main/packages/db/tests/SortedMap-oracle.test.ts), [cleanup queue](https://github.com/TanStack/db/blob/main/packages/db/tests/cleanup-queue-oracle.property.test.ts), [live-query GC clock](https://github.com/TanStack/db/blob/main/packages/db/tests/collection-gc-clock.test.ts), [guarded replay](https://github.com/TanStack/db/blob/main/packages/db/tests/oracle-replay.test.ts) | Map/full-sort and elapsed appointment-list models with executed target/seed/path checks. SortedMap promises what Collection state uses, not full Map parity (maintainer decision). Keys are strings or numbers: `sync.write` and mutations throw `InvalidKeyError` or `UndefinedKeyError` for any other key, and grouped live queries key nullish, boolean, and bigint group values with strings (maintainer decision, 2026-10-27). The model is a Map of numbers plus a full sort; production stores row objects so the driver can change one in place. Keys include `NaN`, which a grouped live query uses for a `NaN` group; it is one key that key order places after every other number. Generated histories mix deferred and immediate sets and deletes, clear, deferred bursts of 17 to 41 keys, prunes that delete entries with deferOrder while iterating, and in-place changes of a deferred or a placed row. A prune must yield the order it began with. A row changed before restore sorts by its current value; once a placed row changes in place, comparator order downgrades to membership (each key once, current value) for the rest of the history, and a later write or delete affects only its own key. At each ordered read with a comparator, restoring k deferred keys among n others makes at most k * (ceil(log2 k) + ceil(log2 (n + 1)) + 2) comparator calls, and none with nothing deferred. Declared reach (minimum 5, per campaign of 100): burst restorations, an entry yielded after a prune deleted one, a placed `NaN` key written without deferral (7 to 16), a restore whose bound is below n (19 to 30), a changed placed row later written (11 to 20), and a deferred row changed before restore (11 to 19). Mutants that re-sort every key (108 calls against a bound of 7), re-sort the kept keys with the dirty keys appended (34 against 7), re-sort clean scans, drop the scan fallback for a changed row, match slots by `===` (a placed `NaN` key then removes the last key), keep dirty slots, apply deferred deletes immediately, or place a deferred row by its value at the write each fail. Supplemental examples in `SortedMap.test.ts` pin splice movement at scale, a key iterator opened before a deferred restore, scan reuse, and public single-transaction and queued-transaction Collection sync paths. Not promised: other mutation during iteration (immediate writes, clear), the order of rows once a placed row changed in place, keys outside the key type, `-0` keys, a comparator that throws, and the work of key order without a comparator, which the public API cannot observe. `forEach` was removed; SortedMap is not exported. Wall-clock steps do not move due times, and a focused public witness checks source subscription release. Callback-reentrant scheduling, OS suspend/resume, environments without `performance.now()`, and fake/real Performance clock replacement while GC is pending remain outside this coverage. Tests must drain or reset pending GC before switching timer providers. | The index-suggestion oracle's PR #2196 extension checks a matching local custom-collation index against the full-source request and retained warning, diff --git a/docs/guides/live-queries.md b/docs/guides/live-queries.md index 40e938e305..eb4aedf28d 100644 --- a/docs/guides/live-queries.md +++ b/docs/guides/live-queries.md @@ -1696,7 +1696,7 @@ const engineeringStats = deptStats.get(1) ``` > **Note**: Grouped results are keyed differently based on the grouping: -> - **Single column grouping**: Keyed by the actual value (e.g., `deptStats.get(1)`) +> - **Single column grouping**: A string or number group value is the key (e.g., `deptStats.get(1)`). A `null`, `undefined`, boolean, or bigint group value gets a string key instead, because Collection keys are strings or numbers. Read that key from the row's `$key`. > - **Multiple column grouping**: Keyed by a JSON string of the grouped values (e.g., `userStats.get('[1,"admin"]')`) ## findOne diff --git a/packages/db/src/SortedMap.ts b/packages/db/src/SortedMap.ts index 3be4c1aaeb..502216831e 100644 --- a/packages/db/src/SortedMap.ts +++ b/packages/db/src/SortedMap.ts @@ -123,9 +123,14 @@ export class SortedMap { /** Removes a key's slot while its placed value is still mapped. */ private unplace(key: TKey): void { // Binary search misses a value that a sync source mutated in place after - // it was placed, so fall back to a scan. + // it was placed, so fall back to a scan. Slots compare as Map keys do, so + // a NaN key matches its own slot. + const sameKey = (candidate: TKey) => + candidate === key || (candidate !== candidate && key !== key) let index = this.indexOf(key, this.map.get(key)!) - if (this.sortedKeys[index] !== key) index = this.sortedKeys.indexOf(key) + if (!sameKey(this.sortedKeys[index]!)) { + index = this.sortedKeys.findIndex(sameKey) + } this.sortedKeys.splice(index, 1) } diff --git a/packages/db/src/collection/mutations.ts b/packages/db/src/collection/mutations.ts index 1d5a3151da..0c29728a58 100644 --- a/packages/db/src/collection/mutations.ts +++ b/packages/db/src/collection/mutations.ts @@ -3,6 +3,7 @@ import { withChangeTracking, withFlatChangeTracking, } from '../proxy' +import { assertCollectionKey } from '../utils' import { normalizeError } from '../utils/error.js' import { safeRandomUUID } from '../utils/uuid' import { createTransaction, getActiveTransaction } from '../transactions' @@ -20,7 +21,6 @@ import { SchemaMustBeSynchronousError, SchemaValidationError, UpdateKeyNotFoundError, - assertCollectionKey, } from '../errors' import type { Collection, CollectionImpl } from './index.js' import type { StandardSchemaV1 } from '@standard-schema/spec' diff --git a/packages/db/src/collection/sync.ts b/packages/db/src/collection/sync.ts index 168de61095..b18dde6c9d 100644 --- a/packages/db/src/collection/sync.ts +++ b/packages/db/src/collection/sync.ts @@ -10,11 +10,11 @@ import { SyncRowReusedWithoutPreviousValueError, SyncTransactionAlreadyCommittedError, SyncTransactionAlreadyCommittedWriteError, - assertCollectionKey, } from '../errors' import { createDeferred } from '../deferred' import { codedMessage, devBuild } from '../error-message' import { withAcceptedReceipt } from '../sync-receipt' +import { assertCollectionKey } from '../utils' import { isPromiseLike } from '../utils/type-guards' import { LIVE_QUERY_INTERNAL } from '../query/live/internal.js' import { automaticRowMetadataWrite } from './state' diff --git a/packages/db/src/errors.ts b/packages/db/src/errors.ts index 8321c5e929..f2eb93ae59 100644 --- a/packages/db/src/errors.ts +++ b/packages/db/src/errors.ts @@ -232,15 +232,6 @@ export class InvalidKeyError extends CollectionOperationError { } } -/** Throws unless a key has the Collection key type: a string or a number. */ -export function assertCollectionKey(key: unknown, item: unknown): void { - if (typeof key !== `string` && typeof key !== `number`) { - throw key === undefined - ? new UndefinedKeyError(item) - : new InvalidKeyError(key, item) - } -} - export class DuplicateKeyError extends CollectionOperationError { constructor(key: string | number) { super( diff --git a/packages/db/src/utils.ts b/packages/db/src/utils.ts index ec99c9311b..67541bddea 100644 --- a/packages/db/src/utils.ts +++ b/packages/db/src/utils.ts @@ -2,6 +2,7 @@ * Generic utility functions */ +import { InvalidKeyError, UndefinedKeyError } from './errors' import type { CompareOptions } from './query/builder/types' interface TypedArray { @@ -447,3 +448,12 @@ export function warnOnce(key: string, message: string): void { export function resetWarnings(): void { warnedKeys.clear() } + +/** Throws unless a key has the Collection key type: a string or a number. */ +export function assertCollectionKey(key: unknown, item: unknown): void { + if (typeof key !== `string` && typeof key !== `number`) { + throw key === undefined + ? new UndefinedKeyError(item) + : new InvalidKeyError(key, item) + } +} diff --git a/packages/db/tests/SortedMap-oracle.test.ts b/packages/db/tests/SortedMap-oracle.test.ts index bf3cd24f73..c9914b0f7a 100644 --- a/packages/db/tests/SortedMap-oracle.test.ts +++ b/packages/db/tests/SortedMap-oracle.test.ts @@ -14,7 +14,9 @@ import { createOracleReach } from './oracle-reach' * the surviving entries in either key order or the caller's value order. Equal * values use key order as a stable tie-breaker, so no entry disappears inside * a tie. Keys are strings or numbers, the Collection key type; a sync write - * or mutation with another key throws before it reaches SortedMap. Deferred + * or mutation with another key throws before it reaches SortedMap. `NaN` is a + * number, and a grouped live query keys a `NaN` group by it, so a `NaN` key + * is one key that key order places after every other number. Deferred * writes may postpone ordering within one synchronous batch, but point reads * remain current and ordered reads restore the same view. * @@ -36,7 +38,7 @@ import { createOracleReach } from './oracle-reach' * * Not promised: other mutation during iteration (immediate writes, clear), * the order of rows once a placed row changed in place, keys outside the key - * type, NaN and -0 keys, a comparator that throws, and the work of key order + * type, -0 keys, a comparator that throws, and the work of key order * without a comparator, which the public API cannot observe. The coverage map * records these limits. * @@ -66,10 +68,26 @@ function burstEntries(seed: number): Array<[number, number]> { return keys.map((key) => [key, ((key * 7) % 5) - 2]) } -const probeKeys: Array = [-3, -2, -1, 0, 1, 2, 3, 10, 26, 50] +const probeKeys: Array = [ + -3, + -2, + -1, + 0, + 1, + 2, + 3, + Number.NaN, + 10, + 26, + 50, +] -// The generated keys are numbers, which key order sorts ascending. -const compareModelKeys = (a: ModelKey, b: ModelKey) => a - b +// The generated keys are numbers, which key order sorts ascending with `NaN` +// after every other number. +const compareModelKeys = (a: ModelKey, b: ModelKey) => + Number.isNaN(a) || Number.isNaN(b) + ? Number(Number.isNaN(a)) - Number(Number.isNaN(b)) + : a - b function modelOrder(model: Map, order: Order) { return [...model].sort( @@ -155,6 +173,9 @@ const fixedStep = ( * - A placed row changed in place, then neighbours deleted and rewritten. A * binary search guided by the stale order would remove a neighbour. * - A deferred key deleted before a large deferred batch. + * - A placed `NaN` key deleted and rewritten without deferral. A miss check by + * `===` never matches `NaN`, and a fallback `indexOf` returns -1, so a + * splice there removes the last key instead. */ const fixedHistories: Array> = [ [ @@ -184,6 +205,13 @@ const fixedHistories: Array> = [ fixedStep(`burst`, 0, 3), fixedStep(`set`, 2, 0, false, true), ], + [ + fixedStep(`set`, Number.NaN, 0), + fixedStep(`set`, 1, 1), + fixedStep(`set`, 2, 2), + fixedStep(`set`, Number.NaN, 3, false, true), + fixedStep(`delete`, Number.NaN, 0, false, true), + ], ] describe.each([`key`, `ascending`, `descending`] as const)( @@ -210,6 +238,7 @@ describe.each([`key`, `ascending`, `descending`] as const)( [ `deferred-burst-observed`, `entry-yielded-after-pruning`, + `placed-nan-key-written`, ...comparatorStates, ], { minimum: 5 }, @@ -231,7 +260,10 @@ describe.each([`key`, `ascending`, `descending`] as const)( `mutate`, `mutate`, ), - key: fc.integer({ min: -3, max: 3 }), + key: fc.oneof( + { arbitrary: fc.integer({ min: -3, max: 3 }), weight: 6 }, + { arbitrary: fc.constant(Number.NaN), weight: 2 }, + ), value: fc.integer({ min: -3, max: 3 }), deferOrder: fc.boolean(), observe: fc.boolean(), @@ -285,6 +317,14 @@ describe.each([`key`, `ascending`, `descending`] as const)( observe() for (const action of actions) { const key = action.key + if ( + (action.kind === `set` || action.kind === `delete`) && + !action.deferOrder && + Number.isNaN(key) && + model.has(key) && + !deferred.has(key) + ) + reach.reach(`placed-nan-key-written`) if (action.kind === `set`) { written(key) expect( @@ -310,7 +350,7 @@ describe.each([`key`, `ascending`, `descending`] as const)( deferred.has(candidate) === action.deferOrder, ) if (targets.length > 0) { - const target = targets[(action.key + 3) % targets.length]! + const target = targets[(action.value + 3) % targets.length]! actual.get(target)!.rank = action.value model.set(target, action.value) if (order !== `key`) { From ea4cf8792c64dae207f5db018d52ab59b87a4005 Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 11:00:38 -0600 Subject: [PATCH 8/9] docs: add oracle review record for PR 2266 Co-authored-by: Isaac --- .../pr-2266-collection-key-type.md | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 docs/contributing/oracle-reviews/pr-2266-collection-key-type.md diff --git a/docs/contributing/oracle-reviews/pr-2266-collection-key-type.md b/docs/contributing/oracle-reviews/pr-2266-collection-key-type.md new file mode 100644 index 0000000000..3341b02eeb --- /dev/null +++ b/docs/contributing/oracle-reviews/pr-2266-collection-key-type.md @@ -0,0 +1,167 @@ +# Collection key type and SortedMap restore review + +Reviewed product head: `c385f6eed162fca2f9a997f02ecababfff077294` on +`sortedmap-merge-restore`, based on `origin/main` at +`b8d8ee07c95f51d84c880f6bb7371a84d2858394`. Sources: PR #2266, an internal +code review (10 findings), a code-simplifier pass (7 proposals), and two +CodeRabbit reviews (one finding and one nitpick). A scratch ledger was kept +during evaluation; this record keeps the verdict-critical oracle evidence in +the repository. + +## Contract and scope + +Two owners change, and one owner is new. + +**Grouped result keys (new owner).** A Collection key is a string or a +number: the `TKey` constraint of every Collection type, and the +`InvalidKeyError` that optimistic mutations throw for any other key. A grouped +live query that groups by one field used the raw group value as the key, so a +`null`, `undefined`, boolean, or bigint group value produced a key outside +that type. Grouping by a nullable field is a valid query. Maintainer decision +(2026-10-27): the grouped key matches the declared key type; the type does not +widen. Such a group value now keys its group in the namespace that +`normalizeValue` reserves, which no string group value occupies. The law fixes +the key's type, uniqueness, and stability, not its format. `sync.write` now +throws `UndefinedKeyError` or `InvalidKeyError` for a key outside the type, +through the check that mutations already used. + +**SortedMap (repaired owner).** Maintainer decision: SortedMap promises what +Collection state uses, not full `Map` behavior, and is no longer exported. +Promised: Map ownership for Collection keys, including `NaN` as one key after +every other number; key or comparator order with key tie-breaking; a row +changed in place before restore sorts by its current value; after a placed row +changes in place, each key stays in the views once with its current value, and +a later write or delete affects only its own key; a deferred delete during +iteration yields the starting order once; a restore of k deferred keys among n +others makes at most k * (ceil(log2 k) + ceil(log2 (n + 1)) + 2) comparator +calls, and none with nothing deferred. Not promised: immediate writes or +`clear()` during iteration, the order after a placed row changes in place +without a write, `-0` keys, keys outside the key type, a throwing comparator, +and the work of key order without a comparator. + +**Load-subset replay refinement (fixed regression added).** A named replay of +issue #2224's trace (a failed include demand during a marker replay) joins the +existing campaign. PR #2229 had fixed that defect, so the replay is green; it +needs no model change. + +The keyed `MultiSet.consolidate` rewrite and the Collection commit-loop cleanup +keep their observable behavior. The existing MultiSet consolidation oracle and +the Collection oracles own them; this PR does not change those owners. + +## RED, GREEN, and mutant evidence + +Grouped result key oracle +(`packages/db/tests/query/group-result-key-oracle.property.test.ts`): + +- RED on the parent commit: all 5 tests fail at the first checkpoint with + keys `undefined`, `null`, `false`, and `10n`. +- GREEN: 5 of 5. +- Mutants, all assertion failures: an encoding by `String(value)` fails + because `null` and `"null"` (and `true` and `"true"`) share a key; an + encoding of nullish values only fails because boolean keys remain. An + encoding of only the internal D2 key, with the public key unchanged, passes + every test: the Collection keys rows by the public group key, so the change + is equivalent within this domain. That part of the repair was removed. +- Reach in each campaign of 80 histories (fixed seed / random): a nullish + group published 38 / 32; a group beside a string that prints like it 21 / + 18; a key kept across a change 49 / 53. + +Sync write key witness (`packages/db/tests/collection.test.ts`): RED 4 of 4 +(`undefined`, `null`, boolean, bigint keys were staged); GREEN 4 of 4. The +witness also found that `InvalidKeyError` threw a `TypeError` for a bigint +key, because its development message used plain `JSON.stringify`; the message +now shows the bigint. + +SortedMap oracle (`packages/db/tests/SortedMap-oracle.test.ts`): + +- Before this PR's oracle repair, a mutant that sorted every key at each + restore passed all 30 tests. The work law closes that gap. +- A `NaN` key RED: with comparator order, rewriting a placed `NaN` key + duplicated it and deleting it removed the last key, because the slot check + used `===`. This was a regression inside this PR; `main` spliced at the + binary-search index. GREEN after slots compare by SameValueZero. +- GREEN: 7 of 7, and 21 of 21 supplemental examples in `SortedMap.test.ts`. +- Mutants, all assertion failures at the named checkpoint: + +| Mutant | Rejection | +| --- | --- | +| Sort every key at each restore | 108 comparator calls against a bound of 7 (with delete-only restores made free) | +| Sort kept keys with the dirty keys appended | 34 calls against a bound of 7 (same control) | +| Sort again with nothing deferred | calls at a clean ordered read | +| No scan fallback for a changed row | a neighbour is deleted | +| Match slots by `===` | a placed `NaN` key removes the last key | +| Keep dirty keys' old slots | order and membership mismatch | +| Apply deferred deletes immediately | a prune skips an entry | +| Place a deferred row by its value at the write | order mismatch | +| Replace the key array instead of writing in place | the supplemental live-iterator example fails | + +- Reach in each campaign of 100 histories (lowest across two local runs): + burst restores 54; an entry yielded after a prune deleted one 30; a placed + `NaN` key written without deferral 7; a restore whose bound is below n 19; a + changed placed row later written 11; a deferred row changed before restore + 11. The last three apply to comparator orders only. + +## Oracle guide audit + +| Requirement | Grouped result key oracle | SortedMap oracle | +| --- | --- | --- | +| ORC-001 | Law, authority (`TKey`, `InvalidKeyError`, maintainer decision), and limits are in the opening prose. | Law, authority (existing contract plus the maintainer's scope decision), and the not-promised list are in the opening prose. | +| ORC-002 | `expectedGroups` partitions plain rows by type and value with `NaN` equal to `NaN`. It does not import the compiler, value identity, or key encoding. | A native Map of numbers and a fresh full sort derive every observation. The work bound is a formula in n and k, not production's algorithm. | +| ORC-003 | Literate file: contract, model, grammar, driver, and refinement check each have prose beside the code. | Same structure; the three Collection-use laws sit in the opening prose and beside the driver branches that check them. | +| ORC-004 | Reconstruction: the probe that found the bug (nullish groups beside strings) and the bigint and `undefined` counterexamples are pinned or regenerate in the first runs. Ablation: without nullish values the nullish reach state and RED vanish; without lookalike strings the `String(value)` mutant survives; without booleans and bigints the nullish-only mutant survives; without changes the key-stability state is unreachable. Range: 1 to 6 rows over 16 group values, up to 4 changes; marginal cases include an empty source after deletes and a `NaN` group. Exclusion: a move or delete with no rows writes nothing; Dates, `-0`, and objects are outside the domain. | Reconstruction: four fixed histories (main's original, a mutated neighbour, a deferred delete before a burst, a placed `NaN` rewrite) replay through the shared driver. Ablation: without `mutate` two reach states and two mutant kills vanish; without the work counter the full-sort mutants survive (shown on the earlier oracle); without `prune` the iteration law has no generated history; without `NaN` keys the `===` mutant survives. Range: keys -3..3 and `NaN`, values -3..3, up to 40 actions, bursts of 17 to 41 keys. Exclusion: `mutate` targets only present keys; deletes during iteration are always deferred; keys outside the key type are not generated, because `sync.write` rejects them. | +| ORC-005 | Public `createLiveQueryCollection` over a mock sync source; `entries()` and `size` after preload and after each committed change. | SortedMap's own methods are the entry point (it is internal); every view, `size`, `has`, and `get` after each action or chosen checkpoint, comparator calls at each ordered read. Collection sync paths are pinned in `SortedMap.test.ts`. | +| ORC-006 | Mutants above, classified as assertion failures; one equivalent within the domain. | Mutants above, all assertion failures; a calibration test feeds wrong orders, a lost tie, a stale value, duplicates, and a missing key to the comparison. | +| ORC-007 | Fixed seed 92604118 and an unseeded campaign share the property, driver, comparison, and budget; `group-result-key.groups` is registered for seed and path replay. | The existing `sorted-map.*` fixed and unseeded campaigns and replay interface carry the new actions. | +| ORC-008 | Stateless partition; no model state. | `deferred`, `changedPlaced`, and `orderPromised` are kept: a changed deferred row and a changed placed row look alike until the next ordered read, where one must sort exactly and the other only keeps membership. The write-only `placed` set was removed. | +| ORC-009 | Group value, key, and publication use glossary terms; "lookalike" is a model-only label for a string that prints like a non-string value. | "Deferred", "placed", and "changed in place" are local SortedMap terms with no glossary counterpart; sync transaction and publication keep glossary meanings. | +| ORC-010 | `withOracleCleanup` cleans both Collections and preserves the primary failure. | No resources; fast-check reports the shrunk history. | +| ORC-011 | No shared semantic fault between the type-and-value partition and production's value identity was named inside the domain. Dates, whose identity is their timestamp, are excluded rather than modelled. | No shared fault between a full sort and incremental restore was named. | +| ORC-012 | This record. | This record. | +| ORC-013 | The lookalike strings distinguish a type-preserving encoding from `String(value)`; the boolean and bigint values distinguish a complete encoding from a nullish-only one. | The `restore-bound-below-order-size` premise is reached in 19 or more histories per campaign, and the full and presorted re-sorts exceed the bound there. | +| ORC-014 | The mock sync source is a controlled provider; the claim covers only the compiler's keying. Real adapters are not claimed. | No controlled premise is carried to a real provider. | +| ORC-015 | Three declared states, counts above. | Six declared states (three for comparator orders only), counts above. | +| ORC-016 | Pinned histories run the shared driver and comparison without their own expectations. | Example tests moved to `SortedMap.test.ts`; the oracle file keeps the property, its fixed histories as inputs, and checker calibration. | + +## Review items + +| Item | Disposition at the reviewed head | +| --- | --- | +| CR1 NaN unplace | Fixed: slots compare by SameValueZero; generated `NaN` keys and the `===` mutant above. An interim scope note had wrongly called `NaN` keys unpromised. | +| CR2 O(k n) deferred deletes | Fixed: dirty keys leave by membership in one pass; the work law and supplemental splice bounds reject per-key work. | +| CR3 throwing comparator | Not promised (maintainer scope decision); a comparator that throws is a programming error. | +| CR4 rows mutated without a write | Accepted design (maintainer decision A): their order is not promised; membership is, and is generated. | +| CR5 test gap for NaN and work | Fixed by the `NaN` dimension and the comparator-call work law. | +| CR6 reach on pending runtime keys | Superseded: runtime nullish keys no longer reach SortedMap. | +| CR7 boolean and bigint keys | Fixed at the producer: grouped keys and `sync.write` keep keys in the key type. | +| CR8 per-operation allocation | Fixed: immediate writes keep the binary-insert path. | +| CR9 absent delete skips restore | Fixed: delete restores order first. | +| CR10 removed 512 threshold | Fixed: the threshold remains. | +| S1 vacuous work bounds | Fixed: the comparator-call law bounds the merge pass that the splice counter cannot see. | +| S2 drop the `indexOf` value parameter | Refuted: immediate inserts and the restore pass values not yet in the searched array. | +| S3 model mirror of pending keys | Stale after the redesign. | +| S4 stale comment | Fixed. | +| S5 per-commit allocation | Duplicate of CR8 and CR10. | +| S6 history narrative in the coverage map | Fixed. | +| S7 `clear()` and live iterators | Superseded by the scope decision; main's live-iterator example remains. | +| CodeRabbit: `unplace` miss check for an `undefined` key | Valid when raised; its premise is removed, because an `undefined` key no longer reaches SortedMap. | +| CodeRabbit: extend an oracle for invalid sync keys | Partly valid: SortedMap is not the owner and the no-effect assertion already existed; the coverage map now states why this is a fail-fast invariant witness. | + +Siblings found during evaluation: a `-0` key reported as `-0` (now not +promised), an immediate write that lost its slot when the comparator threw +(not promised), and live iterators that skipped or repeated entries across +immediate writes (superseded by the scope decision). The removed `forEach` and +public export were approved by the maintainer. + +Loss audit: 17 raw review items = 10 fixed, 1 accepted design, 1 not promised +by decision, 2 superseded, 1 refuted, 1 stale, and 1 duplicate; plus 2 +CodeRabbit items (1 premise removed, 1 partly valid and documented) and 3 +siblings (2 not promised, 1 superseded). No source claim is omitted. + +## Limits + +The grouped key owner does not cover Dates, `-0`, objects, or multi-field +groups, which the aggregate select oracle owns, nor real sync adapters. +SortedMap does not promise the items listed under scope. Shipped-byte effects +were measured with a local esbuild approximation of `pnpm size:compare`, +because a registry 403 prevented the install that `size:compare` needs; they +are not part of this oracle verdict. From 662338c8cd1332131ad10b2409e5f2f6d158be78 Mon Sep 17 00:00:00 2001 From: Test Date: Tue, 27 Oct 2026 11:20:33 -0600 Subject: [PATCH 9/9] test(db): name the grouped result key oracle's replay owner - Register group-result-key.groups in the replay manifest that main now requires for every property. - The guide note on grouped keys covers reserved-prefix strings and getKey. - The review record appends the merge, the CodeRabbit item, and the size:compare numbers. Co-authored-by: Isaac --- .../pr-2266-collection-key-type.md | 19 +++++++++++++++++++ docs/guides/live-queries.md | 3 ++- packages/db/tests/oracle-replay-manifest.ts | 9 +++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/docs/contributing/oracle-reviews/pr-2266-collection-key-type.md b/docs/contributing/oracle-reviews/pr-2266-collection-key-type.md index 3341b02eeb..5823e1301c 100644 --- a/docs/contributing/oracle-reviews/pr-2266-collection-key-type.md +++ b/docs/contributing/oracle-reviews/pr-2266-collection-key-type.md @@ -165,3 +165,22 @@ SortedMap does not promise the items listed under scope. Shipped-byte effects were measured with a local esbuild approximation of `pnpm size:compare`, because a registry 403 prevented the install that `size:compare` needs; they are not part of this oracle verdict. + +## Update: merge with `origin/main` at `7e46f81ad` + +The branch merged `origin/main` at `7e46f81ad` in `26896c42a`. That change +(#2237) requires each registered property to name its owner, so +`group-result-key.groups` now has an owner entry and a replay test name in +`oracle-replay-manifest.ts`. No law, model, grammar, or comparison changed. +The replay manifest tests (40) and both oracles pass after the merge. + +A third CodeRabbit finding, on `ea4cf8792`, was valid: the guide note did not +say that a string with the reserved `\u0000tanstack-db:` prefix gets an +escaped key, or that a live query's `getKey` replaces grouped keys. The note +now says both. + +Shipped bytes: `pnpm size:compare` against the merge-base `b8d8ee07c` now +runs, with a local-only override for one dependency that the registry proxy +blocks. The gzip difference is -56 bytes for `app`, -38 to -61 bytes for the +other scenarios, and -2 bytes for the `@tanstack/db` package bundle. These +numbers replace the esbuild approximation in the limits above. diff --git a/docs/guides/live-queries.md b/docs/guides/live-queries.md index eb4aedf28d..33158268c3 100644 --- a/docs/guides/live-queries.md +++ b/docs/guides/live-queries.md @@ -1696,8 +1696,9 @@ const engineeringStats = deptStats.get(1) ``` > **Note**: Grouped results are keyed differently based on the grouping: -> - **Single column grouping**: A string or number group value is the key (e.g., `deptStats.get(1)`). A `null`, `undefined`, boolean, or bigint group value gets a string key instead, because Collection keys are strings or numbers. Read that key from the row's `$key`. +> - **Single column grouping**: A string or number group value is the key (e.g., `deptStats.get(1)`). A `null`, `undefined`, boolean, or bigint group value gets a string key instead, because Collection keys are strings or numbers. A string that starts with TanStack DB's reserved `\u0000tanstack-db:` prefix also gets a different string key. Read these keys from the row's `$key`. > - **Multiple column grouping**: Keyed by a JSON string of the grouped values (e.g., `userStats.get('[1,"admin"]')`) +> - **Custom keys**: A `getKey` option on the live query replaces these keys. ## findOne diff --git a/packages/db/tests/oracle-replay-manifest.ts b/packages/db/tests/oracle-replay-manifest.ts index 746272d0ab..1304ba6a95 100644 --- a/packages/db/tests/oracle-replay-manifest.ts +++ b/packages/db/tests/oracle-replay-manifest.ts @@ -394,6 +394,11 @@ const ownerGroups: ReadonlyArray< `join-result-key`, `pairs`, ], + [ + `db/tests/query/group-result-key-oracle.property.test.ts`, + `group-result-key`, + `groups`, + ], [ `db/tests/query/pooled-live-query-oracle.property.test.ts`, `pooled-live-query`, @@ -761,6 +766,10 @@ const replayTestNames = new Map([ `index-update.custom-comparator`, `matches an independent custom-comparator model \\(seed undefined\\)`, ], + [ + `group-result-key.groups`, + `publishes one key of the key type per group \\(seed undefined\\)`, + ], ]) // Properties whose own seeded generator replaces fast-check, so they have no