Skip to content

test(db): extract reusable trace runner - #1718

Merged
tannerlinsley merged 3 commits into
mainfrom
codex/includes-trace-runner
Aug 12, 2026
Merged

test(db): extract reusable trace runner#1718
tannerlinsley merged 3 commits into
mainfrom
codex/includes-trace-runner

Conversation

@KyleAMathews

@KyleAMathews KyleAMathews commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Extract the includes recompute-oracle loop into a reusable trace runner, then validate the driver/projection boundary with structural histories, scalar materialization, and full-row sync batches. This is test-only, and the expanded scalar driver has already exposed a new deterministic incremental-materialization bug.

Reviewer guidance

Root cause

The includes oracle had two hand-written execution loops. That made each new mutation or lifecycle domain repeat setup, checkpoint, and cleanup behavior instead of sharing one contract.

The first extracted runner also awaited every hook unconditionally. Awaiting a synchronous apply yields to the microtask queue before the checkpoint, which could hide stale state repaired by a queued microtask. Its cleanup path could replace the useful trace failure, and assertEqual: () => void allowed an async assertion whose promise the runner ignored.

Approach

  • A driver owns setup, startup, step application, and cleanup.
  • A projection observes live state, recomputes expected state independently, and compares the two.
  • The runner checks after startup and every step. Drivers can request extra checkpoints within a step.
  • The runner awaits only promise-like hook results, so synchronous mutations and checkpoints stay in the same turn.
  • If both the trace and cleanup fail, the trace error remains primary and the cleanup error is attached as suppressed diagnostic data.
  • Projection assertions must return undefined; a type test rejects async assertions.
  • A deterministic full-row driver sends multi-change sync batches through the structural projection. Its trace covers inserts, updates, reparenting, deletion, and insertion.
  • The scalar-materialization driver can update a nested reference. A reduced five-step expected-failure seed records the new bug it found: changing middle.sharedId updates the visible key but leaves the old nested shared row materialized.

Key invariants

  • Synchronous mutations and their checkpoints occur in the same turn.
  • Async lifecycle hooks finish before the runner advances.
  • Every trace checks its initial post-start state and the state after each step.
  • Explicit checkpoints can verify intermediate optimistic states within a step.
  • Cleanup runs after startup, step, or assertion failures.
  • A trace failure stays primary if cleanup also fails; cleanup alone still fails the run.
  • Observed state and recomputed state remain independent.
  • Known and discovered runtime failures must reject with the oracle assertion, not an unrelated harness error.

Non-goals

  • No production query fix for the newly discovered nested-reference bug; that belongs in a separate small PR.
  • No broad generators yet for full-row batches, timing, lifecycle, publication, rekeying, or deeper guaranteed paths.
  • No changes to existing known-failure seeds.
  • No replay system or multiple-projection orchestration.

Trade-offs

Assertions remain synchronous to preserve exact checkpoint timing. The batch driver uses a short deterministic trace: it is enough to test the abstraction with a distinct mutation model while leaving the larger fuzz-domain expansion for later RFC slices. The new bug stays as an assertion-specific expected failure so this test-infrastructure PR remains green without hiding unrelated failures.

Verification

From packages/db:

pnpm exec vitest run tests/query/includes-oracle.property.test.ts tests/trace-runner.test.ts tests/trace-runner.test-d.ts
pnpm test
  • Oracle file: 12 tests passed; no type errors.
  • Full DB suite: 110 files, 2,541 tests passed, 5 skipped; no type errors.
  • ESLint, Prettier, and git diff --check pass.

Files changed

  • packages/db/tests/trace-runner.ts: adds the shared runner and its timing and error-handling guarantees.
  • packages/db/tests/trace-runner.test.ts: tests checkpoints, same-turn timing, and cleanup failure precedence.
  • packages/db/tests/trace-runner.test-d.ts: proves async projection assertions are rejected.
  • packages/db/tests/query/includes-oracle.property.test.ts: migrates the existing oracle drivers, adds the full-row batch driver, and records the nested-reference failure.

Release impact

  • This change is docs/CI/dev-only (no release).

Related work: RFC #1658, #1716, #1717.

Summary by CodeRabbit

  • Tests
    • Added comprehensive trace-based coverage for structural, full-row batch, and materialization scenarios.
    • Added validation for checkpoints during startup, after each step, and at explicit checkpoints.
    • Ensured cleanup runs reliably after assertion failures and preserves the original error.
    • Added coverage for batched partial and full-row updates.
    • Added materialization checks for leaf updates following inserts.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a typed asynchronous trace runner with checkpoint and cleanup handling. Structural and materialization property-test scenarios now use trace drivers and projections. Tests cover lifecycle ordering, synchronous checkpoints, type constraints, and cleanup failures.

Changes

Trace runner property tests

Layer / File(s) Summary
Trace runner contracts and execution
packages/db/tests/trace-runner.ts, packages/db/tests/trace-runner.test.ts, packages/db/tests/trace-runner.test-d.ts
Adds runTrace, typed driver and projection contracts, lifecycle handling, checkpoint assertions, cleanup error handling, runtime tests, and a compile-time assertion for synchronous assertEqual.
Structural scenario trace migration
packages/db/tests/query/includes-oracle.property.test.ts
Adds configurable row updates and batch writes. Replaces manual structural execution and assertions with typed trace setup, drivers, projections, and cleanup.
Materialization scenario trace migration
packages/db/tests/query/includes-oracle.property.test.ts
Adds named materialization types, leaf-increment and middle-redirection steps, projection-based oracle comparison, and deterministic batch scenarios.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant runTrace
  participant TraceDriver
  participant TraceProjection
  runTrace->>TraceDriver: setup()
  runTrace->>TraceProjection: observe() and recompute() state
  runTrace->>TraceProjection: assertEqual()
  runTrace->>TraceDriver: apply(step, checkpoint)
  TraceDriver-->>runTrace: step completed
  runTrace->>TraceProjection: assertEqual() after checkpoint
  runTrace->>TraceDriver: cleanup()
Loading

Possibly related PRs

  • TanStack/db#1669: Modifies related property-test scenarios in includes-oracle.property.test.ts.
  • TanStack/db#1716: Introduces the property-test harness that this PR refactors.
  • TanStack/db#1717: Modifies the same oracle and action-testing logic.

Suggested reviewers: kevin-dp

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: extracting a reusable trace runner for database tests.
Description check ✅ Passed The description explains the changes, approach, verification, and release impact, although it does not use every template heading exactly.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/includes-trace-runner

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

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (4)
packages/db/tests/query/includes-oracle.property.test.ts (2)

945-960: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: build the increment steps with filter and spread.

The second loop repeats the leaf-1/leaf-2 test and the id mapping. An array-method form states the intent in one pass.

♻️ Proposed refactor
 function createMaterializeTraceSteps(
   insertOrder: Array<MaterializeInsert>,
 ): Array<MaterializeTraceStep> {
-  const steps: Array<MaterializeTraceStep> = insertOrder.map((insert) => ({
-    type: `insert`,
-    insert,
-  }))
-
-  for (const insert of insertOrder) {
-    if (insert === `leaf-1` || insert === `leaf-2`) {
-      steps.push({ type: `incrementLeaf`, id: insert === `leaf-1` ? 1 : 2 })
-    }
-  }
-
-  return steps
+  const leafId = (insert: MaterializeInsert): number =>
+    insert === `leaf-1` ? 1 : 2
+
+  return [
+    ...insertOrder.map(
+      (insert): MaterializeTraceStep => ({ type: `insert`, insert }),
+    ),
+    ...insertOrder
+      .filter((insert) => insert === `leaf-1` || insert === `leaf-2`)
+      .map((insert): MaterializeTraceStep => ({
+        type: `incrementLeaf`,
+        id: leafId(insert),
+      })),
+  ]
 }

As per coding guidelines: "Use array methods like filter() instead of manual loops for array transformations" and "Use the spread operator for combining arrays instead of manual loops with push".

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

In `@packages/db/tests/query/includes-oracle.property.test.ts` around lines 945 -
960, Refactor createMaterializeTraceSteps to build incrementLeaf steps with
filter/map and combine them with the initial insert steps using spread, removing
the manual loop and push while preserving the existing leaf-to-ID mapping.

Source: Coding guidelines


777-787: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Both projections type the observed value as unknown. assertEqual then compares an untyped value against a precisely typed expectation, so a shape drift between the live query result and the oracle result no longer fails type checking. If stripVirtualProperties returns a typed value, pass that type instead of unknown. If it returns a loose type, add a small typed wrapper for the observed shape.

  • packages/db/tests/query/includes-oracle.property.test.ts#L777-L787: replace the unknown type argument of structuralProjection with the observed structural shape, for example Array<OracleNode>.
  • packages/db/tests/query/includes-oracle.property.test.ts#L1000-L1016: replace the unknown type argument of materializeProjection with the observed materialization shape, for example Array<MaterializeTree>.

As per coding guidelines: "Always provide the most precise return type annotation; avoid unknown or any return types unless truly necessary".

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

In `@packages/db/tests/query/includes-oracle.property.test.ts` around lines 777 -
787, Replace the unknown observed-type arguments in both projections with their
precise observed shapes: use Array<OracleNode> for structuralProjection at
packages/db/tests/query/includes-oracle.property.test.ts:777-787 and
Array<MaterializeTree> for materializeProjection at
packages/db/tests/query/includes-oracle.property.test.ts:1000-1016, preserving
the existing observation and equality logic.

Source: Coding guidelines

packages/db/tests/trace-runner.ts (1)

44-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider preserving the original failure if cleanup also throws.

If a checkpoint assertion fails and driver.cleanup then rejects, the cleanup error replaces the assertion error. The oracle drivers call incremental.cleanup() and cleanupSources(sources), which can reject after a failed step. The property test then reports a teardown error instead of the mismatch that caused it.

♻️ Proposed change to keep the primary error
   try {
     await driver.start?.(context)
     checkpoint()
 
     for (const step of steps) {
       await driver.apply(step, context, checkpoint)
       checkpoint()
     }
+  } catch (error) {
+    try {
+      await driver.cleanup(context)
+    } catch {
+      // Keep the original trace failure as the reported error.
+    }
+    throw error
   } finally {
-    await driver.cleanup(context)
+    // cleanup already ran on the error path
   }

A simpler variant is a let failed = false flag, or Promise.allSettled-style handling, if you prefer to keep a single finally.

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

In `@packages/db/tests/trace-runner.ts` around lines 44 - 54, Update the trace
runner’s try/finally flow around driver.start, step execution, and checkpoint so
cleanup failures do not replace the original execution or assertion error.
Preserve the primary failure, while still awaiting driver.cleanup(context) and
surfacing its error when no earlier failure occurred.
packages/db/tests/trace-runner.test.ts (1)

45-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add corner-case tests for the runner lifecycle.

The current suite covers the happy path and a failing checkpoint. Three cases remain untested, and all three are reachable through the oracle drivers:

  • steps: []. The runner must still check after startup and call cleanup once.
  • An async driver. setup, start, apply, and cleanup each may return a promise. Assert that the runner awaits them in order.
  • driver.apply throwing. Assert that cleanup still runs.

Note also that apply on Line 53 never executes in this test, because the startup checkpoint throws first.

💚 Proposed additional tests
   it(`cleans up when a checkpoint fails`, async () => {
it(`checks after startup and cleans up for an empty trace`, async () => {
  const checkpoints: Array<number> = []
  const cleanup = vi.fn()

  await runTrace({
    steps: [],
    driver: {
      setup: () => ({ observed: 0, expected: 0 }),
      apply: () => undefined,
      cleanup,
    },
    projection: {
      observe: (context) => context.observed,
      recompute: (context) => context.expected,
      assertEqual: (observed, expected) => {
        expect(observed).toBe(expected)
        checkpoints.push(observed)
      },
    },
  })

  expect(checkpoints).toEqual([0])
  expect(cleanup).toHaveBeenCalledOnce()
})

it(`awaits an asynchronous driver in order`, async () => {
  const calls: Array<string> = []
  const tick = async () => {
    await Promise.resolve()
  }

  await runTrace({
    steps: [1],
    driver: {
      setup: async () => {
        await tick()
        calls.push(`setup`)
        return { observed: 0, expected: 0 }
      },
      start: async () => {
        await tick()
        calls.push(`start`)
      },
      apply: async (step, context) => {
        await tick()
        context.observed += step
        context.expected += step
        calls.push(`apply`)
      },
      cleanup: async () => {
        await tick()
        calls.push(`cleanup`)
      },
    },
    projection: {
      observe: (context) => context.observed,
      recompute: (context) => context.expected,
      assertEqual: (observed, expected) => {
        expect(observed).toBe(expected)
        calls.push(`check`)
      },
    },
  })

  expect(calls).toEqual([`setup`, `start`, `check`, `apply`, `check`, `cleanup`])
})

it(`cleans up when apply throws`, async () => {
  const cleanup = vi.fn()

  await expect(
    runTrace({
      steps: [1],
      driver: {
        setup: () => ({ observed: 0, expected: 0 }),
        apply: () => {
          throw new Error(`apply failed`)
        },
        cleanup,
      },
      projection: {
        observe: (context) => context.observed,
        recompute: (context) => context.expected,
        assertEqual: (observed, expected) => {
          expect(observed).toBe(expected)
        },
      },
    }),
  ).rejects.toThrow(`apply failed`)

  expect(cleanup).toHaveBeenCalledOnce()
})

As per coding guidelines: "Test corner cases including: empty arrays/sets, single-element collections, undefined vs null values, resolved promises, async race conditions, and limit/offset edge cases".

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

In `@packages/db/tests/trace-runner.test.ts` around lines 45 - 67, Add the three
missing lifecycle tests alongside the existing trace-runner tests: verify empty
steps perform the startup checkpoint and call cleanup once, verify async
setup/start/apply/cleanup methods are awaited in the order setup, start, check,
apply, check, cleanup, and verify cleanup runs when driver.apply throws while
preserving the original error. Use the existing runTrace, driver, and projection
symbols and assert the specified checkpoint, call-order, and cleanup behavior.

Sources: Coding guidelines, Learnings

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

Nitpick comments:
In `@packages/db/tests/query/includes-oracle.property.test.ts`:
- Around line 945-960: Refactor createMaterializeTraceSteps to build
incrementLeaf steps with filter/map and combine them with the initial insert
steps using spread, removing the manual loop and push while preserving the
existing leaf-to-ID mapping.
- Around line 777-787: Replace the unknown observed-type arguments in both
projections with their precise observed shapes: use Array<OracleNode> for
structuralProjection at
packages/db/tests/query/includes-oracle.property.test.ts:777-787 and
Array<MaterializeTree> for materializeProjection at
packages/db/tests/query/includes-oracle.property.test.ts:1000-1016, preserving
the existing observation and equality logic.

In `@packages/db/tests/trace-runner.test.ts`:
- Around line 45-67: Add the three missing lifecycle tests alongside the
existing trace-runner tests: verify empty steps perform the startup checkpoint
and call cleanup once, verify async setup/start/apply/cleanup methods are
awaited in the order setup, start, check, apply, check, cleanup, and verify
cleanup runs when driver.apply throws while preserving the original error. Use
the existing runTrace, driver, and projection symbols and assert the specified
checkpoint, call-order, and cleanup behavior.

In `@packages/db/tests/trace-runner.ts`:
- Around line 44-54: Update the trace runner’s try/finally flow around
driver.start, step execution, and checkpoint so cleanup failures do not replace
the original execution or assertion error. Preserve the primary failure, while
still awaiting driver.cleanup(context) and surfacing its error when no earlier
failure occurred.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2723ea87-aec6-4372-8a73-8a03f2c8f117

📥 Commits

Reviewing files that changed from the base of the PR and between 2985e0b and de67bd4.

📒 Files selected for processing (3)
  • packages/db/tests/query/includes-oracle.property.test.ts
  • packages/db/tests/trace-runner.test.ts
  • packages/db/tests/trace-runner.ts

@pkg-pr-new

pkg-pr-new Bot commented Aug 12, 2026

Copy link
Copy Markdown
More templates

@tanstack/angular-db

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

@tanstack/browser-db-sqlite-persistence

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

@tanstack/capacitor-db-sqlite-persistence

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

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

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

@tanstack/db

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

@tanstack/db-ivm

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

@tanstack/db-sqlite-persistence-core

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

@tanstack/electric-db-collection

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

@tanstack/electron-db-sqlite-persistence

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

@tanstack/expo-db-sqlite-persistence

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

@tanstack/node-db-sqlite-persistence

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

@tanstack/offline-transactions

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

@tanstack/powersync-db-collection

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

@tanstack/query-db-collection

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

@tanstack/react-db

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

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

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

@tanstack/rxdb-db-collection

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

@tanstack/solid-db

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

@tanstack/svelte-db

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

@tanstack/tauri-db-sqlite-persistence

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

@tanstack/trailbase-db-collection

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

@tanstack/vue-db

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

commit: 210ad3c

@github-actions

Copy link
Copy Markdown
Contributor

Size Change: 0 B

Total Size: 128 kB

ℹ️ View Unchanged
Filename Size
packages/db/dist/esm/collection/change-events.js 1.44 kB
packages/db/dist/esm/collection/changes.js 1.43 kB
packages/db/dist/esm/collection/cleanup-queue.js 810 B
packages/db/dist/esm/collection/events.js 434 B
packages/db/dist/esm/collection/index.js 3.83 kB
packages/db/dist/esm/collection/indexes.js 1.99 kB
packages/db/dist/esm/collection/lifecycle.js 1.7 kB
packages/db/dist/esm/collection/mutations.js 2.47 kB
packages/db/dist/esm/collection/state.js 5.51 kB
packages/db/dist/esm/collection/subscription.js 3.82 kB
packages/db/dist/esm/collection/sync.js 2.94 kB
packages/db/dist/esm/collection/transaction-metadata.js 144 B
packages/db/dist/esm/deferred.js 207 B
packages/db/dist/esm/errors.js 5.13 kB
packages/db/dist/esm/event-emitter.js 748 B
packages/db/dist/esm/index.js 3.21 kB
packages/db/dist/esm/indexes/auto-index.js 829 B
packages/db/dist/esm/indexes/base-index.js 784 B
packages/db/dist/esm/indexes/basic-index.js 2.17 kB
packages/db/dist/esm/indexes/btree-index.js 2.29 kB
packages/db/dist/esm/indexes/index-registry.js 820 B
packages/db/dist/esm/indexes/reverse-index.js 557 B
packages/db/dist/esm/live-query-adapter.js 318 B
packages/db/dist/esm/live-query-observer.js 2.29 kB
packages/db/dist/esm/local-only.js 916 B
packages/db/dist/esm/local-storage.js 2.12 kB
packages/db/dist/esm/optimistic-action.js 359 B
packages/db/dist/esm/paced-mutations.js 496 B
packages/db/dist/esm/proxy.js 3.75 kB
packages/db/dist/esm/query/builder/functions.js 1.47 kB
packages/db/dist/esm/query/builder/index.js 5.84 kB
packages/db/dist/esm/query/builder/ref-proxy.js 1.24 kB
packages/db/dist/esm/query/compiler/evaluators.js 1.89 kB
packages/db/dist/esm/query/compiler/expressions.js 430 B
packages/db/dist/esm/query/compiler/group-by.js 3.56 kB
packages/db/dist/esm/query/compiler/index.js 6.67 kB
packages/db/dist/esm/query/compiler/joins.js 2.5 kB
packages/db/dist/esm/query/compiler/lazy-targets.js 923 B
packages/db/dist/esm/query/compiler/order-by.js 1.74 kB
packages/db/dist/esm/query/compiler/select.js 1.53 kB
packages/db/dist/esm/query/effect.js 4.77 kB
packages/db/dist/esm/query/expression-helpers.js 1.43 kB
packages/db/dist/esm/query/ir.js 1.25 kB
packages/db/dist/esm/query/live-query-collection.js 360 B
packages/db/dist/esm/query/live/collection-config-builder.js 9.24 kB
packages/db/dist/esm/query/live/collection-registry.js 264 B
packages/db/dist/esm/query/live/collection-subscriber.js 1.93 kB
packages/db/dist/esm/query/live/internal.js 145 B
packages/db/dist/esm/query/live/utils.js 1.81 kB
packages/db/dist/esm/query/optimizer.js 2.92 kB
packages/db/dist/esm/query/predicate-utils.js 2.97 kB
packages/db/dist/esm/query/query-once.js 359 B
packages/db/dist/esm/query/subset-dedupe.js 960 B
packages/db/dist/esm/scheduler.js 1.3 kB
packages/db/dist/esm/SortedMap.js 1.3 kB
packages/db/dist/esm/strategies/debounceStrategy.js 247 B
packages/db/dist/esm/strategies/queueStrategy.js 428 B
packages/db/dist/esm/strategies/throttleStrategy.js 246 B
packages/db/dist/esm/transactions.js 3.04 kB
packages/db/dist/esm/utils.js 927 B
packages/db/dist/esm/utils/array-utils.js 273 B
packages/db/dist/esm/utils/browser-polyfills.js 304 B
packages/db/dist/esm/utils/btree.js 5.61 kB
packages/db/dist/esm/utils/comparison.js 1.15 kB
packages/db/dist/esm/utils/cursor.js 457 B
packages/db/dist/esm/utils/index-optimization.js 2.39 kB
packages/db/dist/esm/utils/type-guards.js 157 B
packages/db/dist/esm/utils/uuid.js 449 B
packages/db/dist/esm/virtual-props.js 360 B

compressed-size-action::db-package-size

@github-actions

Copy link
Copy Markdown
Contributor

Size Change: 0 B

Total Size: 3.81 kB

ℹ️ View Unchanged
Filename Size
packages/react-db/dist/esm/index.js 249 B
packages/react-db/dist/esm/useLiveInfiniteQuery.js 1.32 kB
packages/react-db/dist/esm/useLiveQuery.js 920 B
packages/react-db/dist/esm/useLiveQueryEffect.js 355 B
packages/react-db/dist/esm/useLiveSuspenseQuery.js 567 B
packages/react-db/dist/esm/usePacedMutations.js 401 B

compressed-size-action::react-db-package-size

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
packages/db/tests/query/includes-oracle.property.test.ts (2)

1143-1149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider it instead of fcTest for this fixed trace.

fullRowBatchTrace is a fixed literal array. This case takes no fast-check arbitraries, so it is an example-based test, not a property test. Using it states the intent more precisely and avoids the fast-check runner overhead.

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

In `@packages/db/tests/query/includes-oracle.property.test.ts` around lines 1143 -
1149, Change the test declaration for the fixed fullRowBatchTrace case from
fcTest to it. Keep the existing runTrace call, driver, projection, and
assertions unchanged.

776-782: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Guarantee source cleanup when the live query cleanup fails.

cleanupStructuralTrace awaits incremental.cleanup() first. If that call rejects, cleanupSources(sources) never runs and the five source collections stay alive. A property test executes this path many times, so the leak accumulates.

♻️ Proposed fix
 async function cleanupStructuralTrace({
   incremental,
   sources,
 }: StructuralTraceContext): Promise<void> {
-  await incremental.cleanup()
-  await cleanupSources(sources)
+  try {
+    await incremental.cleanup()
+  } finally {
+    await cleanupSources(sources)
+  }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/db/tests/query/includes-oracle.property.test.ts` around lines 776 -
782, Update cleanupStructuralTrace so cleanupSources(sources) always runs even
when incremental.cleanup() rejects, using guaranteed cleanup control flow such
as a finally block. Preserve the existing cleanup order by attempting
incremental.cleanup() first, then source cleanup, while allowing the original
failure to propagate.
packages/db/tests/trace-runner.test.ts (1)

122-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for asynchronous driver results.

All drivers in this file return synchronously. The isPromiseLike branches in runTrace for setup, start, apply, and cleanup are never exercised. Add one test with a rejected async cleanup and one test with a non-Promise thenable setup. This pins the thenable detection that the runner advertises.

Based on learnings, tests should cover corner cases including "resolved promises" and "async race conditions".

🧪 Proposed additional tests
it(`rejects with an asynchronous cleanup failure`, async () => {
  const cleanupError = new Error(`cleanup failed`)

  await expect(
    runTrace({
      steps: [],
      driver: {
        setup: () => ({ observed: 0, expected: 0 }),
        apply: () => undefined,
        cleanup: () => Promise.reject(cleanupError),
      },
      projection: {
        observe: (context) => context.observed,
        recompute: (context) => context.expected,
        assertEqual: (observed, expected) => {
          expect(observed).toBe(expected)
        },
      },
    }),
  ).rejects.toBe(cleanupError)
})

it(`awaits a thenable setup result`, async () => {
  const context = { observed: 1, expected: 1 }

  await expect(
    runTrace({
      steps: [],
      driver: {
        setup: () => ({
          then: (resolve: (value: typeof context) => void) => resolve(context),
        }),
        apply: () => undefined,
        cleanup: () => undefined,
      },
      projection: {
        observe: (value) => value.observed,
        recompute: (value) => value.expected,
        assertEqual: (observed, expected) => {
          expect(observed).toBe(expected)
        },
      },
    }),
  ).resolves.toBeUndefined()
})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/db/tests/trace-runner.test.ts` around lines 122 - 144, Add coverage
in the trace-runner tests for asynchronous driver results: add a test where
cleanup returns a rejected Promise and assert runTrace rejects with that error,
then add a test where setup returns a non-Promise thenable resolving to a valid
context and assert runTrace resolves successfully. Keep the existing synchronous
cleanup-failure test and exercise the isPromiseLike handling in runTrace.

Source: Learnings

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

Nitpick comments:
In `@packages/db/tests/query/includes-oracle.property.test.ts`:
- Around line 1143-1149: Change the test declaration for the fixed
fullRowBatchTrace case from fcTest to it. Keep the existing runTrace call,
driver, projection, and assertions unchanged.
- Around line 776-782: Update cleanupStructuralTrace so cleanupSources(sources)
always runs even when incremental.cleanup() rejects, using guaranteed cleanup
control flow such as a finally block. Preserve the existing cleanup order by
attempting incremental.cleanup() first, then source cleanup, while allowing the
original failure to propagate.

In `@packages/db/tests/trace-runner.test.ts`:
- Around line 122-144: Add coverage in the trace-runner tests for asynchronous
driver results: add a test where cleanup returns a rejected Promise and assert
runTrace rejects with that error, then add a test where setup returns a
non-Promise thenable resolving to a valid context and assert runTrace resolves
successfully. Keep the existing synchronous cleanup-failure test and exercise
the isPromiseLike handling in runTrace.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b4d57f3-e19d-4a85-8cb3-caf5b7c19ece

📥 Commits

Reviewing files that changed from the base of the PR and between de67bd4 and 5ece238.

📒 Files selected for processing (4)
  • packages/db/tests/query/includes-oracle.property.test.ts
  • packages/db/tests/trace-runner.test-d.ts
  • packages/db/tests/trace-runner.test.ts
  • packages/db/tests/trace-runner.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/db/tests/query/includes-oracle.property.test.ts (2)

892-901: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use precise observed types for both projections.

structuralProjection and materializeProjection use unknown for TObserved. This removes compile-time checking between the live query shape and the oracle shape. Use Array<OracleNode> and Array<MaterializeTree>, and type or narrow stripVirtualProperties at the observation boundary. (raw.githubusercontent.com)

Proposed type refinement
 const structuralProjection: TraceProjection<
   StructuralTraceContext,
-  unknown,
+  Array<OracleNode>,
   Array<OracleNode>
 > = {

 const materializeProjection: TraceProjection<
   MaterializeTraceContext,
-  unknown,
+  Array<MaterializeTree>,
   Array<MaterializeTree>
 > = {

As per coding guidelines: “Always provide the most precise return type annotation; avoid unknown or any return types unless truly necessary.”

Also applies to: 1122-1139

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

In `@packages/db/tests/query/includes-oracle.property.test.ts` around lines 892 -
901, Replace the unknown observed types in both structuralProjection and
materializeProjection with Array<OracleNode> and Array<MaterializeTree>,
respectively. Update or narrow stripVirtualProperties at the observation
boundary so its result conforms to the precise projection type while preserving
the existing observation behavior and type checking.

Source: Coding guidelines


1059-1075: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace the manual loop and push with array methods.

createMaterializeTraceSteps maps insert steps, then scans insertOrder again and mutates steps with push. Build the leaf steps with filter().map() and combine both arrays with spread. (raw.githubusercontent.com)

Proposed refactor
 function createMaterializeTraceSteps(
   insertOrder: Array<MaterializeInsert>,
 ): Array<MaterializeTraceStep> {
-  const steps: Array<MaterializeTraceStep> = insertOrder.map((insert) => ({
-    type: `insert`,
-    insert,
-  }))
+  const leafId = (insert: MaterializeInsert): number =>
+    insert === `leaf-1` ? 1 : 2

-  for (const insert of insertOrder) {
-    if (insert === `leaf-1` || insert === `leaf-2`) {
-      steps.push({ type: `incrementLeaf`, id: insert === `leaf-1` ? 1 : 2 })
-    }
-  }
-
-  return steps
+  return [
+    ...insertOrder.map(
+      (insert): MaterializeTraceStep => ({ type: `insert`, insert }),
+    ),
+    ...insertOrder
+      .filter((insert) => insert === `leaf-1` || insert === `leaf-2`)
+      .map((insert): MaterializeTraceStep => ({
+        type: `incrementLeaf`,
+        id: leafId(insert),
+      })),
+  ]
 }

As per coding guidelines: “Use array methods like filter() instead of manual loops for array transformations” and “Use the spread operator for combining arrays instead of manual loops with push.”

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

In `@packages/db/tests/query/includes-oracle.property.test.ts` around lines 1059 -
1075, Refactor createMaterializeTraceSteps to avoid the manual loop and
mutation: keep the existing insert-step map, derive incrementLeaf steps from
insertOrder with filter().map(), and return the two arrays combined via spread
while preserving the current leaf-1/leaf-2 IDs and ordering.

Source: Coding guidelines

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

Nitpick comments:
In `@packages/db/tests/query/includes-oracle.property.test.ts`:
- Around line 892-901: Replace the unknown observed types in both
structuralProjection and materializeProjection with Array<OracleNode> and
Array<MaterializeTree>, respectively. Update or narrow stripVirtualProperties at
the observation boundary so its result conforms to the precise projection type
while preserving the existing observation behavior and type checking.
- Around line 1059-1075: Refactor createMaterializeTraceSteps to avoid the
manual loop and mutation: keep the existing insert-step map, derive
incrementLeaf steps from insertOrder with filter().map(), and return the two
arrays combined via spread while preserving the current leaf-1/leaf-2 IDs and
ordering.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ea388cbb-542e-400b-80e0-9b5bf0a54a94

📥 Commits

Reviewing files that changed from the base of the PR and between 5ece238 and 210ad3c.

📒 Files selected for processing (1)
  • packages/db/tests/query/includes-oracle.property.test.ts

@tannerlinsley
tannerlinsley merged commit e4940a4 into main Aug 12, 2026
11 checks passed
@tannerlinsley
tannerlinsley deleted the codex/includes-trace-runner branch August 12, 2026 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants