You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(table-core): prevent median aggregation overflow for large finite values - #6655
When calculating the median of an even-length array, aggregationFn_median sums the two middle values before dividing by two ((values[mid - 1]! + values[mid]!) / 2).
When both middle values are large finite numbers (e.g. [Number.MAX_VALUE, Number.MAX_VALUE] or [1e308, 1.5e308]), the intermediate addition overflows to Infinity even though the true average is finite and representable.
This fix:
Evaluates const sum = a + b.
If sum is finite, or if either operand is non-finite (Infinity, -Infinity, or NaN), it returns sum / 2. This ensures that tiny/subnormal values like [Number.MIN_VALUE, Number.MIN_VALUE] are preserved without prematurely underflowing to 0.
If the intermediate sum overflows to ±Infinity while both operands are finite, it calculates a / 2 + b / 2, returning the accurate finite median.
Added unit test coverage for large finite overflow, subnormal preservation, and non-finite controls.
No actionable comments were generated in the recent review.
Walkthrough
The even-length median calculation now avoids intermediate overflow for large finite middle values. Tests cover large, subnormal, and infinite values. A patch changeset records the correction.
Changes: Median aggregation correction
Layer / File(s)
Summary
Even-length median calculation and validation packages/table-core/src/features/row-aggregation/aggregationFns.ts, packages/table-core/tests/unit/fns/aggregationFns.test.ts, .changeset/fix-median-overflow.md
The calculation avoids overflow when adding large finite middle values and retains the sum-then-divide path when that sum is finite or an operand is non-finite. Tests cover large values, subnormal values, and infinities. The changeset describes the fix.
The median overflow fix appears mergeable after normal checks; no actionable issue is identified.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title names the core change: a fix that prevents median aggregation overflow for large finite values in table-core. It matches the aggregationFn_median change and the added tests. It is specific a…
Description check
The description follows the repository template. It has the Changes section with motivation and a clear explanation of the overflow bug and fix. It links issue #6654. The checklist items are marked, i…
Linked Issues check
[#6654] The issue requires that even-length medians of large finite middle values not overflow, that tiny/subnormal results be preserved, and that zero, NaN, and infinity behavior stay unchanged. At h…
Out of Scope Changes check
The whole-PR diff (merge base c14e022 to head dba0a85) touches three paths. (1) packages/table-core/src/features/row-aggregation/aggregationFns.ts changes only the even-length median branch, which i…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check
Explanation
The title names the core change: a fix that prevents median aggregation overflow for large finite values in table-core. It matches the aggregationFn_median change and the added tests. It is specific and concise, and it follows the conventional commit style.
Full details: Description check
Explanation
The description follows the repository template. It has the Changes section with motivation and a clear explanation of the overflow bug and fix. It links issue #6654. The checklist items are marked, including the changeset item under Release Impact. The description covers the required information, so the check passes. Minor note: the Changes section does not describe the test additions in detail, but it does mention them.
Full details: Linked Issues check
Explanation
[#6654] The issue requires that even-length medians of large finite middle values not overflow, that tiny/subnormal results be preserved, and that zero, NaN, and infinity behavior stay unchanged. At head, aggregationFn_median (packages/table-core/src/features/row-aggregation/aggregationFns.ts) computes sum = a + b. It returns sum / 2 when sum is finite or when either operand is non-finite. Otherwise, when both operands are finite and sum overflows, it returns a / 2 + b / 2. Checked cases: [MAX, MAX] gives MAX; [-MAX, -MAX] gives -MAX; [1e308, 1.5e308] gives 1.25e308; [MIN, MIN] gives MIN (no underflow to 0); [Infinity, Infinity] gives Infinity; [-Infinity, Infinity] gives NaN. Every case that did not overflow takes the same sum / 2 path as before, so existing non-overflow behavior is unchanged. The test added in packages/table-core/tests/unit/fns/aggregationFns.test.ts ('does not overflow median for large finite values and preserves subnormals') covers these overflow, subnormal, and non-finite cases. Limitation: I did not run the test suite. The exact-equality assertion for 1e308/1.5e308 is not verified by execution.
Full details: Out of Scope Changes check
Explanation
The whole-PR diff (merge base c14e022 to head dba0a85) touches three paths. (1) packages/table-core/src/features/row-aggregation/aggregationFns.ts changes only the even-length median branch, which is the overflow fix in #6654. (2) packages/table-core/tests/unit/fns/aggregationFns.test.ts adds median overflow, subnormal, and non-finite cases for that fix. Other existing assertions are unchanged. (3) .changeset/fix-median-overflow.md adds a patch changeset for the published package, as the PR description states. No change falls outside the scope of #6654.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. 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 0 functions across 2 files. (1 skipped: 1 unsupported.)
Finishing Touches
Generate unit tests (beta)
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.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Fixes #6654.
When calculating the median of an even-length array,
aggregationFn_mediansums the two middle values before dividing by two ((values[mid - 1]! + values[mid]!) / 2).When both middle values are large finite numbers (e.g.
[Number.MAX_VALUE, Number.MAX_VALUE]or[1e308, 1.5e308]), the intermediate addition overflows toInfinityeven though the true average is finite and representable.This fix:
const sum = a + b.sumis finite, or if either operand is non-finite (Infinity,-Infinity, orNaN), it returnssum / 2. This ensures that tiny/subnormal values like[Number.MIN_VALUE, Number.MIN_VALUE]are preserved without prematurely underflowing to0.±Infinitywhile both operands are finite, it calculatesa / 2 + b / 2, returning the accurate finite median.Added unit test coverage for large finite overflow, subnormal preservation, and non-finite controls.
✅ Checklist
pnpm testandpnpm test:e2e, or these tests do not apply to this pull request.🚀 Release Impact
Summary by CodeRabbit
Infinityresults.