Skip to content

fix(db-ivm): treat 0 and empty string as valid min/max extremes - #1809

Merged
KyleAMathews merged 4 commits into
TanStack:mainfrom
aakbarpour:cursor/db-minmax-falsy-a997
Sep 23, 2026
Merged

KyleAMathews merged 4 commits into
TanStack:mainfrom
aakbarpour:cursor/db-minmax-falsy-a997

Conversation

@aakbarpour

@aakbarpour aakbarpour commented Sep 11, 2026

Copy link
Copy Markdown

Fix @tanstack/db-ivm min/max aggregation so numeric zero and bigint zero remain valid minima or maxima, and the empty string remains a valid string minimum. Grouped live-query results no longer silently replace these falsy extrema with later values.

Root Cause

The reducers used truthiness both as the “no accumulator yet” sentinel and as a guard around comparisons. That made 0, 0n, and "" indistinguishable from an unset accumulator: a falsy extreme could be skipped when encountered later or overwritten by the next truthy value.

Approach

  • Treat only undefined as an unset accumulator.
  • Ignore undefined reducer entries before comparing values, preserving the aggregate's declared input type.
  • Narrow direct aggregates from the public basic-or-piped union before testing their reducers.
  • Keep focused replay witnesses for number, bigint, string, and the full grouped-dataflow path.
  • Extend the existing literate DBSP incrementalization oracle so generated weighted histories compare grouped sum, min, and max deltas and retained state with independent full recomputation.
  • Add the old truthiness reducers as a hostile mutant and require the checker to reject them.

Key Invariants

  • Falsy values participate according to their domain ordering, not JavaScript truthiness.
  • undefined never becomes a min/max candidate.
  • Empty groups still produce undefined.
  • Atomic and split delivery agree with independent full recomputation at every logical checkpoint.

Non-goals

  • No changes to supported min/max value domains.
  • No changes to multiplicity handling or aggregate result types.
  • No changes outside @tanstack/db-ivm.

RED/GREEN Verification

  • Restoring the old truthiness reducers makes the fixed generated campaign fail after one case and shrink to rows containing 0 and -1.
  • The named cell reports minimum: 5 instead of 0 and maximum: -2 instead of 0 under the old code.
  • Focused unit and generated-history suites: 22/22 passed after restoring the fix.
  • Full @tanstack/db-ivm suite: 39 files and 559 tests passed.
  • Standalone TypeScript, ESLint, Prettier, and diff checks passed.

Files changed

  • packages/db-ivm/src/operators/groupBy.ts — use explicit undefined guards for min/max reduction.
  • packages/db-ivm/tests/operators/groupBy.test.ts — retain readable reducer and pipeline replay witnesses.
  • packages/db-ivm/tests/incrementalization-law.property.test.ts — add independent min/max recomputation, fixed/random/replay histories, named reach, and the exact truthiness mutant.
  • .changeset/minmax-falsy-extremes.md — publish the correction as a patch.

Fixes #1775

Summary by CodeRabbit

  • Bug Fixes

    • Fixed min and max grouping results so valid falsy values—including 0, 0n, and empty strings—are correctly considered.
    • Grouped calculations now return accurate minimum and maximum values when data includes zero or other falsy values.
  • Tests

    • Added coverage for falsy extremes and grouped results containing zero values.

min() and max() used truthiness to detect an unset accumulator, so 0,
0n, and "" were skipped or overwritten. Compare against undefined.

Fixes TanStack#1775
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d702af5b-5899-4432-9295-818e58e5de38

📥 Commits

Reviewing files that changed from the base of the PR and between 1f39943 and 7d9aefa.

📒 Files selected for processing (2)
  • packages/db-ivm/tests/incrementalization-law.property.test.ts
  • packages/db-ivm/tests/operators/groupBy.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/db-ivm/tests/operators/groupBy.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The min and max reducers now use undefined to detect an unset extreme and compare falsy values. Tests cover reducer results, grouped results containing zero, and incrementalization behavior. A changeset declares a patch release.

Changes

Falsy min/max aggregate handling

Layer / File(s) Summary
Update min/max reducer checks
packages/db-ivm/src/operators/groupBy.ts
The min and max reducers use undefined checks and consider falsy values as candidates.
Test falsy extrema and incrementalization
packages/db-ivm/tests/operators/groupBy.test.ts, packages/db-ivm/tests/incrementalization-law.property.test.ts, .changeset/minmax-falsy-extremes.md
Tests cover falsy reducer extremes, grouped results containing zero, incrementalization witnesses, and rejection of truthiness-filtered extrema. The changeset declares a patch release.

Priority: ⚪ Not assessed

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: kyleamathews

Merge Risk: ⚪ Minimal · up to 7d9ae

The falsy-extrema change appears mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The pull request satisfies the coding requirements in issue #1775. packages/db-ivm/src/operators/groupBy.ts now uses undefined as the only unset sentinel and compares candidates with domain orderi…
Out of Scope Changes check ✅ Passed The changes remain within issue #1775. The source change fixes min() and max(). The direct, grouped, and property tests verify the fix. The changeset documents the patch release for `@tanstack/db-…
Title check ✅ Passed The title clearly identifies the main change: treating falsy values such as 0 and the empty string as valid min/max extremes in db-ivm.
Description check ✅ Passed The description explains the motivation, root cause, implementation, invariants, testing, release impact, and changed files. It does not reproduce the template's Checklist and Release Impact headings …
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@pkg-pr-new

pkg-pr-new Bot commented Sep 23, 2026

Copy link
Copy Markdown
More templates

@tanstack/angular-db

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

@tanstack/browser-db-sqlite-persistence

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

@tanstack/capacitor-db-sqlite-persistence

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

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

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

@tanstack/db

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

@tanstack/db-ivm

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

@tanstack/db-sqlite-persistence-core

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

@tanstack/electric-db-collection

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

@tanstack/electron-db-sqlite-persistence

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

@tanstack/expo-db-sqlite-persistence

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

@tanstack/node-db-sqlite-persistence

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

@tanstack/offline-transactions

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

@tanstack/powersync-db-collection

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

@tanstack/query-db-collection

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

@tanstack/react-db

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

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

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

@tanstack/react-router-with-db

npm i https://pkg.pr.new/@tanstack/react-router-with-db@1809

@tanstack/rxdb-db-collection

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

@tanstack/solid-db

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

@tanstack/svelte-db

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

@tanstack/tauri-db-sqlite-persistence

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

@tanstack/trailbase-db-collection

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

@tanstack/vue-db

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

commit: 7d9aefa

@KyleAMathews
KyleAMathews merged commit 850b241 into TanStack:main Sep 23, 2026
11 checks passed
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.

db-ivm: min()/max() aggregates return wrong results when the extreme value is falsy (0 or empty string)

2 participants