PR 2171: review evaluation, October 28#

Checked code head: 751f9a74500a947050ddba0048a93c99d87339b7. Starting review head: 7c53f271acd4be62d40475b509d2751d27479866. Merged main: b305fb2dcfbc296fd1980ceb6e611e0993fec740.

This record evaluates the user's ten numbered findings. Finding 10 has two independent claims, retained as R10a and R10b. The ledger below accounts for all 11 claims. No finding was deferred. The task-local raw ledger and command logs are under /private/tmp/rc2171-review; the conclusions and reproduction commands needed after that scratch directory disappears are recorded here.

Laws, experiments, and inference#

The authority for the three policy choices is the maintainer's explicit answers in this review. Query and Electric discard legacy handler values at runtime. Inline getKey and schema bind until query identity changes. Structured source selections and spreads retain source metadata; a functional selection retains it only when it returns a source row directly. New functional objects receive the output Collection's metadata. Colliding reserved names cannot change that rule. The migration guide and executable owners now state those decisions.

Handler settlement and refresh#

A fulfilled handler must not become a failed transaction merely because it returned a discarded value. A rejected handler must retain its exact rejection. The earlier Electric histories did not cross handler verdict with a legacy return value. The Query owner covered confirmation paths but omitted that return-value dimension. Those gaps let the adapters disagree at settlement. The neighboring law is explicit synchronization: completing a Query handler does not fetch or replace authoritative rows. The handler must await refetch or a direct write if settlement must expose its confirmed result.

The Electric owner is electric-oracle.property.test.ts. Its finite grammar crosses insert/update/delete, fulfillment/rejection, and absent/legacy returns: 12 histories. Each history holds the handler, checks optimistic rows, then compares the exact rejection identity, simulated server-write count, and settled public rows. Successful handlers explicitly deliver ShapeStream confirmation and await its txid. Each history requires one optimistic and one settled reach. The driver uses real Collection transactions and controlled SDK event delivery. It does not claim a live Electric-server result.

The reviewed runtime-rejection wrapper fails three histories at the public Promise-verdict assertion. The other nine pass. Passing those neighbors rejects a repair that merely suppresses every handler error. With the rejection wrapper removed, all 12 pass; the surrounding Electric selection passes 334 runtime tests.

The Query ownership owner crosses CRUD, no confirmation/refetch/direct write, and absent/legacy return: 18 histories. It checks optimistic rows while held and exact fetch counts and public rows at settlement. Each history requires both reach states once. A real QueryClient supplies the fetch boundary. External focus, reconnect, polling, and invalidation are separate triggers, outside this grammar. The earlier automatic-refetch calibration failed all nine original confirmation histories at fetch counts; its evidence remains in pr-2171-explicit-query-refresh.md. The final complete Query runtime suite passes all 978 tests in 29 files, including both ordered-refetch campaigns. Moved main tests now initiate refetch explicitly without changing their public assertions or observation checkpoints.

Query identity#

The earlier identity owner did not vary inline option references across host rerenders. It could stay green while ordinary render inputs rebuilt a Collection. The React source-ID owner now crosses live/Suspense hooks, getKey/schema/both, and derived/explicit-key identity: 12 histories. Each starts a ready source, rerenders three times with new closures, applies a source update, then changes query identity. Collection and configured option references must persist before the change and be replaced afterward. Every history requires rerender, source-update, and identity-change reach once.

The earlier three derived-identity regressions fail before the repair. The final matrix with the previous option-reference identity rule fails six derived cases at Collection identity; the six explicit-key neighbors pass. The repaired source identity file passes all 92 tests. The broader React selection passes 442 tests.

schema currently has a separate public declaration limitation: its hook config type is never. The runtime histories supply schema with Object.assign at the JavaScript boundary. They do not establish a new schema typing contract. Infinite hooks do not accept inline Collection options; their existing identity/window histories run as neighboring guarantees. The shared implementation also serves other adapters, but this new inline-option matrix supplies a real React-host witness, not a witness for every framework scheduler. Preact's existing complete runtime conformance suite passes 98 tests.

Metadata at Collection publication#

The earlier metadata owner did not cross selection form, joined source choice, and individual reserved-name collisions at pending and settled checkpoints. A source-guessing implementation therefore escaped its narrower comparisons. The primary virtual-row-fields owner now enumerates 50 legal histories: single/joined sources, structured projection/spread, functional projection/spread/direct return, and none/each/all reserved-name collisions. Direct returns cannot author a collision, so that combination is excluded. The source's label changes while an optimistic transaction is held. Each history checks all four fields before mutation, during the hold, and after settlement. The model decides ownership from the authored selection form, never by comparing selected values. A clean sibling supplies only the output-generated key.

Declared reach is 25 single-source pending histories, 25 joined-source pending histories, 40 colliding histories, and two direct functional returns. The minimum for each declared state is one. Individual $collectionId collisions matter: a fix for only $key or all-fields collisions would miss them.

The initial expanded tests failed on the reviewed implementation. The final 50-history grammar, run with the previous compiler restored, fails 26 histories at metadata assertions and passes 24. This is assertion failure, not setup failure. The repaired primary and supporting metadata tests pass 57 tests. The broader metadata/retention selection passes 164. Supplemental tests preserve two spread orders, an unmatched RIGHT join, direct functional returns, facade projections, and inline-value shape. The new grammar does not establish every join kind, arbitrary nested histories, custom-key projections, provider behavior, or every framework receiving path. Those limits remain explicit in the coverage map.

Compatibility and integration#

A probe against the actual DB 0.15.2 package confirms that when exists and completeWithoutCommit does not. Calling the latter throws a TypeError. Offline Transactions now peers only on ^1.0.0-rc.0 and receives a stable minor bump. The release inventory test enforces that manifest boundary. No runtime fallback preserves a false compatibility claim. Existing restore, serializer, and settlement owners pass 89 tests. This is package compatibility metadata, so a manifest/version fixture is the appropriate new instrument.

The main merge provides Convex and Preact. Convex's obsolete BaseCollectionConfig return generic is migrated. All 22 Convex runtime files pass, with 108 tests. Both adapters build and typecheck. The release fixture checks every actual public workspace manifest against the first-RC inventory; version and publish fixtures pass 11 tests. Query and Offline Transactions remain stable minors, as requested. The inventory/version checks are configuration evidence rather than a state model.

Reproduction entry points#

Run each command from the named package directory at the checked code head. These commands isolate the three new finite grammars. Omit -t to run each complete owner file.

# packages/db
../../node_modules/.bin/vitest run tests/query/virtual-row-fields-oracle.test.ts -t 'attributes root metadata' --coverage.enabled=false --typecheck.enabled=false

# packages/react-db
../../node_modules/.bin/vitest run tests/source-id-reuse-oracle.test.tsx -t 'retains inline' --coverage.enabled=false --typecheck.enabled=false

# packages/electric-db-collection
../../node_modules/.bin/vitest run tests/electric-oracle.property.test.ts -t 'settles a handler independently' --coverage.enabled=false --typecheck.enabled=false

Calibration used those same commands with one previous production file restored at a time, while keeping the final tests. For metadata and Electric, restore packages/db/src/query/compiler/index.ts or packages/electric-db-collection/src/electric.ts from the starting review head. For identity, restore packages/db/src/live-query-options.ts from merged main b305fb2dcfbc296fd1980ceb6e611e0993fec740. Restore the checked code after each experiment. The expected failures and passing neighbors are recorded above.

Lossless finding dispositions#

Accounting: 11 claims = eight fixed-now + one accepted-design + two stale. No refuted, duplicate, deferred, or unresolved product-decision entries. The original line references belong to the starting review head and moved during the main merge. No source-order ID was merged away.

Adjacent integration repairs#

Main's split ordinary/oracle tests remain split. Mechanical migrations replace isPersisted.promise waits and $synced reads without relaxing law assertions. The virtual-field type oracle retains its declaration and shared type fixture. A malformed generated readonly line is removed from the persistence type instrument; both fixed and random campaigns pass (four tests total). The minified-export inventory drops the removed DuplicateAliasInSubqueryError. Published error-code documentation and its frozen code registry remain available for old errors. The production-error oracle passes 18 tests, and the consumer bundle check passes for 263 distinctive literals. The minified API checks 113 error names plus index metadata, rows, and live updates.

Authored guide/skill examples now pass without adding failing snippets to the baseline. Twenty obsolete baseline entries were removed. Existing 252 failing examples remain acknowledged. Pseudo-code fragments explicitly declare nocheck; current API examples receive concrete row types and required configuration. The 25 relative migration-guide links resolve. The repository-wide link checker still reports 51 existing failures, including code links outside its docs root. Generated API references match main exactly and are absent from the PR diff, as the maintainer requested. Their CI workflow owns regeneration.

Oracle conformance and evidence limits#

• ORC-001/002/003/005/009: authority, independent judgments, legal forms, drivers, and public cuts are stated above and beside their executable owners. No concrete missing-source-information hypothesis remains for these controlled checkpoints. Metadata authorship is now an explicit policy, not inferred data.
• ORC-004/008/013: removing CRUD, confirmation, verdict, return-value, identity, selection-shape, or collision axes loses a distinguishing case named above. The finite bounds are exhaustive for their declared grammar. A direct return with authored overrides and inline infinite-hook config are excluded invalid forms. No new production-like state machine was introduced in the models.
• ORC-006: previous production designs fail at Collection identity, metadata, Promise verdict, or fetch-count comparisons. The passing neighboring cases reject blanket failure suppression, source-always attribution, and collision-only fixes. Calibration is not a universal proof.
• ORC-007: the new matrices enumerate finite grammars rather than claiming random-campaign coverage. Existing ordered-refetch and persistence type owners retain their fixed/random campaigns and replay interface.
• ORC-010: controlled gates release and Collections clean up after histories; the metadata owner preserves primary/secondary cleanup failures. The evidence includes clean green process exits, not merely some passing assertions.
• ORC-011: source metadata and output metadata are compared through distinct source-preserving and clean projection forms. No additional named shared-model semantic fault required another independent model in this review.
• ORC-012/015/016: this versioned record identifies the checked code head. New reach requirements and counts appear above. Oracle declarations remain intact; supplemental handwritten examples stay in ordinary files.
• ORC-014: React uses the real host. Query uses real QueryClient with controlled rows. Electric controls SDK delivery. No claim about an actual provider commit or every receiving framework follows from those premises.

The full repository CI and native provider suites did not run locally. Early Vitest combined runtime/type invocations reported cross-package rootDir setup errors. Clean runtime runs and separate tsc --rootDir . checks replace that invalid combined evidence. Initial all-files ESLint exceeded Node's default heap; package batches and the normal chunked commit hook supply lint evidence. These setup failures are not RED or GREEN product evidence.

Code weight and review judgment#

pnpm size:compare --exact-base --base origin/main measured production app bundles against main at b305fb2dc. Measured packages save 750 gzip bytes and 528 brotli bytes together. The typical app saves 302 gzip and 230 brotli bytes. DB grows 130 gzip bytes for explicit metadata-source rules. Deleting value-matching guesses and consolidating copy-on-write attachment reduces the repair's cost; deleting Electric matching wrappers, Query auto-refetch, and deprecated APIs makes the total negative. Tests and contract documentation grow separately and do not enter those figures.

Reviewer recommendation: hire for defect discovery, with maintainer review of policy proposals. The review found real compatibility, identity, settlement, and metadata hazards. Its strongest observations crossed package and async boundaries. It did not supply oracle or test-gap evidence, treated an explicitly chosen minor as a new decision, and included two claims superseded by main. Those limits reduce prioritization quality without erasing the useful findings.

Readiness: unproven pending current-head CI and broader integration evidence. The reviewed repairs and finite-law evidence are complete; this record does not claim that all release, provider, platform, or repository checks passed.

Follow-up main integration#

Checked merge head: ade3920bf67d804f3d5bbfb30c757be3ec75ba0b. Merged main: 2dc655f55fee13bd8f37252c592022ce3848b9bf.

The maintainer requested another main merge after the review fixes were pushed. The merge applied without conflicts. It imports the lazy-demand replay repair, a persistence guide correction, and Angular/UUID dependency updates. No imported test required a deprecated-API migration. The authored migration guide now notes that a synchronous loadSubset throw retains demand and that releasing this failed demand calls unloadSubset.

The combined tree passes 804 DB runtime tests in 11 files, including replay, ordered loading, virtual-row metadata, and Collection/include publication. DB TypeScript checking and DB/Query builds pass. All 978 Query runtime tests in 29 files pass against the rebuilt DB, including both ordered-refetch campaigns. React's 92-case source identity owner also passes after that build. All 11 release fixtures pass. Offline frozen-lockfile verification passes without an installation workaround. The normal merge commit hook passes.

The first Query run used the previous DB dist and failed the two newly imported bounded-refetch witnesses. Rebuilding DB makes both pass, followed by the complete green Query run. That stale-build run is setup evidence, not a product RED. The broad persisted-session run also started before that rebuild; its 70 passes are not credited as a current-build integration witness. After rebuilding, both new persisted-session and browser multi-tab replay witnesses pass their existing it.fails checks. They still reproduce the known source/persistence gaps in #2281, rather than proving those product behaviors correct. Those sync metadata/durability gaps are separate from the virtual-row attribution contract evaluated above.

This integration changes none of the 11 finding dispositions. The original calibration remains evidence for the unchanged compiler attribution, option identity, and handler-return policies; the affected comparisons pass on the combined tree. Generated references still match main. The earlier byte figures retain their stated b305fb2dc base and were not remeasured for this merge. Angular/TrailBase runtime suites and current-head CI remain unverified locally. Readiness remains unproven, separate from the completed review accounting.

Dependency and CI formatting follow-up#

Integration head: e46d67b2d includes main bc77952e5 and the PR's automated formatting commit b05e4052c. The latter changes quote style in three project example files. The main follow-up updates Vite dependencies. A direct diff confirms that DB, Query, React, Electric, and Offline Transactions production sources are unchanged since the runtime-checked merge ade3920bf. Frozen offline lockfile verification and all 11 release fixtures pass again. The updated Vite toolchain was not installed or runtime-tested locally; CI must validate it. No generated reference diff or installation workaround is included.