Repository navigation
fix(db): reject negative limits before changing queries or windows - #2333
Conversation
Preserve query construction and window admission while removing duplicated merges, condition validation, and unreachable source/window bookkeeping. Strengthen the admission oracle to require the documented RangeError. Law: existing immutable builder and window admission contracts. Reach: all six admission receivers retain 21 histories per campaign. RED: ordinary Error and throw-undefined mutants fail all 12 negative campaigns at the shared error-class comparison; current production was already correct. GREEN: 754 tests, test typecheck, changed lint, mangle check and consumer error erasure pass. Mutant: both wrong error-class implementations were restored. Limits: existing admission domain and observation cuts are unchanged; no raw-IR accessor timing or numeric-policy expansion.
🦋 Changeset detectedLatest commit: e55c530 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Query builder clause updates packages/db/src/query/builder/index.ts, packages/db/mangle-cache.json |
Query builder methods merge clause patches through _clone. Source-reference lookup and condition conversion are shared. The obsolete _createRefForSource mangle-cache entry is removed. |
Negative limit validation packages/db/src/query/validate-limit.ts, packages/db/src/query/builder/index.ts, packages/db/src/query/live/collection-config-builder.ts, packages/db/tests/fixtures/error-site-messages.json, packages/db/src/query/live/ARCHITECTURE.md, docs/errors.md, .changeset/reject-negative-limits.md |
A shared validator rejects negative values at .limit() and setWindow() entry points. Documentation and the changeset record the behavior, including support for zero limits and negative offsets. |
Admission oracle and replay packages/db/tests/query/limit-admission-oracle.ts, packages/db/tests/query/pagination-oracle.property.test.ts, packages/db/tests/oracle-config.ts, packages/db/tests/oracle-replay-manifest.ts, packages/db/tests/oracle-replay.test.ts, packages/db/tests/generate-error-docs.test.ts, docs/contributing/oracle-coverage.md, docs/contributing/oracle-reviews/* |
Oracle campaigns cover negative and valid limits across builder and live-query states. Replay configuration and documentation record campaign results, tested scope, and exclusions. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Suggested reviewers: samwillis
Merge Risk: 
Minimal · up to e55c5
-
No identified issue blocks merging after normal checks.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title "fix(db): reject negative limits before changing queries or windows" matches the main change. The changeset shows that the query builder's .limit() and setWindow() now reject negative limits before changing state. The title is specific, concise, and follows a conventional commit style. It does not cover the refactors or the oracle test additions, which is expected.
Full details: Description check
-
Explanation
The description follows the template. It has the Changes section, which explains the new RangeError for negative limits and the setWindow behavior. It has an Oracle Coverage section that names the pagination owner and the limit-admission companion. It states the law, the reach across six receivers, the RED/GREEN results, mutants, and remaining limits, as the bug-fix guidance requires. It has the Checklist and Release Impact sections. The checklist item for local tests is unchecked, and the author states the complete root test command did not run. This gap is disclosed, so it does not make the description incomplete. The Oracle Coverage section is dense, but it contains the required evidence.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 8 functions across 9 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Incremental update benchmarkComparing Overall median write time vs base: 1.00× · cold hydrate time: 1.04× (geometric mean of per-case ratios; lower is faster). Writes: 3 regression(s), 4 improvement(s) (threshold: ±20% and >0.05ms). Cold hydrate: 3 regression(s), 0 improvement(s) (threshold: ±50% and >5ms). Per-case flags are noisy on shared runners. Read the geometric means first. Writes
Cold hydrate
Each row aggregates the 14.4 scale/index/write-mode configurations of that query; per-configuration tables below. 100 rows/collection | source indexes: none | synced writes — geomean 0.94×, cold 0.98×, 1 change(s)
100 rows/collection | source indexes: none | optimistic writes — geomean 1.01×, cold 0.81×
100 rows/collection | source indexes: manual | synced writes — geomean 0.99×, cold 1.06×
100 rows/collection | source indexes: manual | optimistic writes — geomean 1.01×, cold 1.04×
1,000 rows/collection | source indexes: none | synced writes — geomean 0.84×, cold 1.00×, 3 change(s)
1,000 rows/collection | source indexes: none | optimistic writes — geomean 0.91×, cold 1.14×, 1 change(s)
1,000 rows/collection | source indexes: manual | synced writes — geomean 0.96×, cold 1.38×, 1 change(s)
1,000 rows/collection | source indexes: manual | optimistic writes — geomean 0.98×, cold 1.01×
10,000 rows/collection | source indexes: none | synced writes — geomean 1.10×, cold 1.00×, 1 change(s)
10,000 rows/collection | source indexes: none | optimistic writes — geomean 1.03×, cold 0.99×
10,000 rows/collection | source indexes: manual | synced writes — geomean 1.03×, cold 1.08×
10,000 rows/collection | source indexes: manual | optimistic writes — geomean 1.19×, cold 1.13×, 2 change(s)
Runner: node v24.8.0, linux 7.0.0-1017-azure, AMD EPYC 7763 64-Core Processor. Timings on shared CI runners are noisy; treat small deltas as indicative only. |
More templates
@tanstack/angular-db
@tanstack/browser-db-sqlite-persistence
@tanstack/capacitor-db-sqlite-persistence
@tanstack/cloudflare-durable-objects-db-sqlite-persistence
@tanstack/convex-db-collection
@tanstack/db
@tanstack/db-ivm
@tanstack/db-sqlite-persistence-core
@tanstack/electric-db-collection
@tanstack/electron-db-sqlite-persistence
@tanstack/expo-db-sqlite-persistence
@tanstack/node-db-sqlite-persistence
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/preact-db
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/react-native-db-sqlite-persistence
@tanstack/react-router-with-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: +224 B (+0.09%) Total Size: 239 kB 📦 View Changed
ℹ️ View Unchanged
|
🎯 Changes
The query builder now rejects
.limit(-2)with a synchronousRangeError. Ordinary ordered list queries also reject negativesetWindowlimits before changing their window or source demand. Both paths previously admitted negative widths, which could produce inconsistent window deltas.Zero, including
-0, positive integer limits,Infinity, and negative offsets retain their existing behavior. ExistingfindOne, ordering, and reentrancy guards keep their error precedence. One internal validator checks both entry points. This adds no public API.Shared clause merging and condition validation remove repeated builder code. The refactor also removes redundant source validation and unreachable window bookkeeping. These reductions offset the new admission checks.
This follows the negative-limit question found during merged #2316. That PR already resolved the page-window optimization in #2291.
🔬 Oracle Coverage
The pagination owner gains a limit-admission companion. Its independent sign law checks admission through six public receivers. Rejected operations must preserve captured public observations.
RangeErrorafter the existing query-shape and reentrancy guards pass.7ecf97dffprecedes the production fix. Unchanged production fails all 12 negative campaigns across the six receivers. All 12 valid campaigns pass.Errororundefinedeach fail all 12 negative campaigns at the error-class comparison. Request-generation corruption fails the settlement comparison with a reduced replayable history. Failed-move rollback corruption fails the next-use comparison. A valid task-scheduled settlement control passes.NaN, positive fractions, and raw IVM negative-width semantics.Run the admission comparison from
packages/db:The versioned evidence record preserves exact commands, review findings, mutants, reach counts, and reviewed source hashes. The coverage map records the boundary. The committed error page now includes error 261. A CLI check protects that artifact from catalog drift. No oracle run budget decreased.
📦 Shipped Size
Comparison against merged baseline
30e85464includes the negative-limit fix:All eight app scenarios shrink in gzip. Brotli varies by scenario. Tests and evidence do not enter the production bundles.
✅ Checklist
pnpm test.The complete root command did not run. Local validation used the selected suites and checks recorded above.
🚀 Release Impact
The existing changeset patches
@tanstack/db.Summary by CodeRabbit
.limit()orsetWindow()now throw aRangeErrorimmediately, without changing the query or active window.