Skip to content

fix: tolerate treemap rounding and collapsed tiles - #244

Merged
tannerlinsley merged 1 commit into
mainfrom
taren/treemap-layout-bounds
Oct 23, 2026
Merged

tannerlinsley merged 1 commit into
mainfrom
taren/treemap-layout-bounds

Conversation

@tannerlinsley

@tannerlinsley tannerlinsley commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

Thanks to @quinnj for the clear report and the four reproduction cases. This fixes the bounds check without accepting materially invalid layouts.

Fixes #243.

AI review notes:
The fix allows machine-scale rounding at plot boundaries, clamps that rounding to the plot, and leaves empty padded tiles to the existing filter. Non-finite, reversed, and materially out-of-bounds coordinates remain rejected.
Added regression coverage for all four reported cases and invalid custom layouts. All 15 focused tests pass. The earlier 14,424-case source layout sweep passed; it was not a packed-package or painted-pixel sweep.
Final local pnpm validate passes on Node 24.18.0, including package consumers, seven framework adapters, Native/Expo, bundle policy, comparison policy, and preview checks. The final run reused matching Nx results from this verification pass. An earlier preview check read the old manifest during regeneration and failed; the completed manifest check and final validation pass.
Tanner explicitly approved only the treemap incremental gzip ceiling of 4.022 decimal kB and the scoped preview/fingerprint refresh. Equivalent source bundles measured 16.985/6.755 kB minified/gzip before and 17.150/6.826 kB after. Other limits, numeric baselines, and tolerances are unchanged. All 188 preview SVGs are unchanged; only the manifest source hash changed.
Runtime work remains linear in leaf count, with constant-time numeric checks and clamping per leaf. Prior noisy local timing samples do not establish zero overhead. No CI waiver or release claim.
Includes a patch Changeset for @tanstack/charts, joining the existing fixed-set release queue.

Summary by CodeRabbit

  • Bug Fixes
    • Treemap tiles now remain within chart boundaries when small floating-point rounding differences occur, reducing the chance of layout errors.
    • Tiles with no visible area after inner padding are omitted, preventing empty tiles from appearing in the chart.
    • Invalid layouts that place tiles beyond the chart’s boundaries continue to be rejected.

@changeset-bot

changeset-bot Bot commented Oct 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2a65bee

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@tanstack/charts Patch
@tanstack/alpine-charts Patch
@tanstack/angular-charts Patch
@tanstack/lit-charts Patch
@tanstack/octane-charts Patch
@tanstack/preact-charts Patch
@tanstack/react-charts Patch
@tanstack/react-native-charts Patch
@tanstack/solid-charts Patch
@tanstack/svelte-charts Patch
@tanstack/vue-charts Patch
@tanstack/charts-scales Patch

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

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 289a670b-2737-4fa9-9ac7-1bb333568972
📥 Commits

Reviewing files that changed from the base of the PR and between 478de43 and 2a65bee.

📒 Files selected for processing: 6
  • .changeset/treemap-layout-bounds.md
  • benchmarks/comparison/bundle-baseline.json
  • benchmarks/conformance/previews/manifest.json
  • packages/charts-core/src/hierarchy-treemap.test.ts
  • packages/charts-core/src/hierarchy-treemap.ts
  • scripts/measure-bundles.mjs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Treemap layout validation now tolerates small floating-point boundary overshoots, clamps accepted coordinates, and omits zero-area tiles. Tests and benchmark records were also updated.

Changes: Treemap layout bounds

Layer / File(s) Summary
Validate and bound treemap tiles
packages/charts-core/src/hierarchy-treemap.ts, packages/charts-core/src/hierarchy-treemap.test.ts, .changeset/treemap-layout-bounds.md, benchmarks/comparison/bundle-baseline.json, benchmarks/conformance/previews/manifest.json, scripts/measure-bundles.mjs
The validator skips zero-area tiles, allows boundary drift within a dimension-scaled tolerance, and clamps accepted coordinates. Tests cover invalid coordinates and rendered bounds. The changeset, benchmark hashes, and treemap gzip budget are updated.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: greglum

Merge Risk: ⚪ Minimal · up to 2a65b

The treemap bounds change is ready to merge after normal checks; no actionable risk remains.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main changes: tolerating treemap rounding and handling collapsed tiles.
Linked Issues check Passed Issue #243 requires treemap layouts to tolerate machine-scale boundary overshoot and to omit zero-area tiles collapsed outside the plot, while still rejecting invalid layouts. `packages/charts-core/sr…
Out of Scope Changes check Passed The changes stay within issue #243 and its release support. The source change implements the requested treemap behavior. The added tests provide regression coverage. The changeset records the fix. The…
Full details: Linked Issues check

Explanation

Issue #243 requires treemap layouts to tolerate machine-scale boundary overshoot and to omit zero-area tiles collapsed outside the plot, while still rejecting invalid layouts. packages/charts-core/src/hierarchy-treemap.ts applies a finite tolerance, skips zero-width or zero-height tiles before bounds rejection, and clamps accepted coordinates to the plot. It still rejects non-finite, reversed, and materially out-of-bounds coordinates. hierarchy-treemap.test.ts covers the reported widths and padding case, checks final bounds and tile counts, and verifies rejection of non-finite and out-of-bounds custom layouts.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #243 and its release support. The source change implements the requested treemap behavior. The added tests provide regression coverage. The changeset records the fix. The bundle budget, comparison baseline hash, and preview manifest hash support the measured bundle and generated preview updates described for this fix. No unrelated feature or behavior change is shown.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 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.

❤️ Share

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

@nx-cloud

nx-cloud Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 2a65bee

Command Status Duration Result
nx run charts-workspace:ci-distributed ✅ Succeeded 5m 37s View ↗
nx run charts-workspace:package-check ✅ Succeeded <1s View ↗
nx run charts-workspace:benchmark-check ✅ Succeeded 56s View ↗

☁️ Nx Cloud last updated this comment at 2026-10-23 05:02:20 UTC

@tannerlinsley
tannerlinsley merged commit 778f2cc into main Oct 23, 2026
19 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 23, 2026
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.

treemap: bounds check throws on floating-point overshoot from d3 squarify (and on paddingInner collapse)

1 participant