Skip to content

harden(charts): production-hardening pass (follow-up to #3672)#3694

Open
josephfarina wants to merge 17 commits into
mainfrom
feat/charts-hardening
Open

harden(charts): production-hardening pass (follow-up to #3672)#3694
josephfarina wants to merge 17 commits into
mainfrom
feat/charts-hardening

Conversation

@josephfarina

@josephfarina josephfarina commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #3672 (merged). A complete, line-by-line hardening pass over every file in @astryxdesign/charts — defensive robustness and correctness only. No new features, no API/type-contract changes, no scope creep. Package stays private / canary-only. All 31 source files were audited one slice at a time, each change typecheck-gated; full verification (typecheck + lint + build + Storybook consumer typecheck) passes.

Genuine bugs fixed

  • Stacked y-domain under-coverage (layout.ts) — mixed-sign and yBaseline: 'data' stacks rendered outside the scale and clipped; the domain now walks every layer boundary.
  • GPU-pick hover Y-flip off-by-one (dotGLInteractive) — readPixels sampled one row too high (top-edge miss); fixed to texH - 1 - py.
  • Unbounded tickCount crash (ChartAxis/ChartGrid) — an out-of-range count reaching d3 .ticks() throws RangeError; now clamped.
  • Non-finite palette count (getChartColors) — Infinity slipped past the guard into a ~2^53 Array.from allocation (hang/OOM); now rejected.
  • Custom-tooltip-render-null card reappeared + stale hovered-index "undefined" card (ChartTooltip); plus NaN guards on the hover dots and crosshair.
  • createProgram shader leak (webgl.ts) — shaders orphaned on the compile-failure path and never flagged for deletion after link.
  • Non-finite guards added for xPixel input, Chart size (a non-finite height/margin no longer collapses every scale), errorBar cap width, and date-only strings in formatters.

Robustness / hygiene

  • 'use client' on the barrel-exported ChartContext.
  • Annotation marks converged onto the shared xPixel/isBandScale helpers (removed unsafe casts).
  • Scatter dodge excludes non-finite y; color accessor wrapped in try/catch; whitespace-only paints treated as unset.

Files audited and found already solid (left unchanged to avoid churn): bar/line/area, heatmapGL/streamGL, dotGL, legend.ts, ChartSwatch, useChartColors, index.ts, and most of types.ts.

Test plan

  • charts typecheck, lint, build all clean
  • Storybook (consumer) typechecks clean
  • Code-owner review

Made with Cursor


Update — categorical-y heatmap stacked on this branch

feat/charts-hardening now also carries the categorical-y heatmap fast-follow on top of the hardening pass:

  • Cells tile to the band step — no gaps between columns/rows.
  • The y-axis is now categorical — row labels aligned to each cell, instead of a meaningless linear value scale.
  • Both the x and y axis lines frame the grid edge-to-edge.

Traffic heatmap

josephfarina and others added 4 commits July 8, 2026 09:45
Co-authored-by: Cursor <cursoragent@cursor.com>
…t net totals

Co-authored-by: Cursor <cursoragent@cursor.com>
…ext barrel path

Co-authored-by: Cursor <cursoragent@cursor.com>
…ows and whitespace paints

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 9, 2026 8:04pm

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.6KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

github-actions Bot added a commit that referenced this pull request Jul 8, 2026
josephfarina and others added 6 commits July 8, 2026 10:09
…d errorBar cap width

Co-authored-by: Cursor <cursoragent@cursor.com>
…urce leak

Co-authored-by: Cursor <cursoragent@cursor.com>
…eractive

Co-authored-by: Cursor <cursoragent@cursor.com>
…ror crash

Co-authored-by: Cursor <cursoragent@cursor.com>
…d, and stale hovered index

Co-authored-by: Cursor <cursoragent@cursor.com>
…trings in formatters

Co-authored-by: Cursor <cursoragent@cursor.com>
@josephfarina josephfarina marked this pull request as ready for review July 8, 2026 17:10
github-actions Bot added a commit that referenced this pull request Jul 8, 2026
@cixzhang cixzhang added area:charts Charts (packages/charts, ChartV2) type:fix Bug fix labels Jul 8, 2026
WebGL heatmap cells were sized to the shared x-scale's bar-padded bandwidth,
leaving large gaps between columns while the local y-scale stayed tight — a
sparse, asymmetric grid. Size cells to the band step (centered on the band)
so they fill their slot: a proper contiguous heatmap, still axis-aligned.

Co-authored-by: Cursor <cursoragent@cursor.com>
josephfarina and others added 5 commits July 9, 2026 13:01
Cells were sized to xScale.bandwidth(), but the shared x band scale
carries ~0.2 padding, so step > bandwidth left permanent gaps between
columns that cellGap could not close (a sparse, striped grid instead of
a contiguous heatmap). Size cells to the full band step and center them
on the band so cellGap alone controls spacing. Applied to both the
charts heatmapGL mark and the lab ChartHeatmapGL component.

Co-authored-by: Cursor <cursoragent@cursor.com>
A heatmap's rows are categories (e.g. days), but the chart's y-axis was a
linear value scale — so a left <ChartAxis> showed meaningless 0-N value
ticks and the rows didn't correspond to the axis.

Add an opt-in categorical y-axis: a series can declare `layout.yBandKey`,
and the layout pass builds a shared `yBandScale` over that key's unique
values, exposed on the series/chart context. ChartAxis prefers
`yBandScale` for vertical axes, so the left axis renders the row labels
aligned to each cell. heatmapGL now declares `dataKeys: []` +
`layout.yBandKey` and draws rows from the shared band scale, keeping cells
aligned with the axis and out of the linear y-domain / value tooltip.

Co-authored-by: Cursor <cursoragent@cursor.com>
A heatmap's categorical y-axis leaves the linear y-domain degenerate and
spanning zero, so ChartAxis' "draw the bottom edge line at zero" rule
placed the x-axis line mid-plot, cutting across the cells. Anchor the
bottom edge line to the plot edge whenever the y-axis is categorical
(yBandScale present), and fill heatmap rows edge-to-edge (band padding 0)
so the grid meets both axis lines. The WebGL heatmap story now enables the
x and y axis lines, which frame the grid at its edges.

Co-authored-by: Cursor <cursoragent@cursor.com>
For a categorical grid (heatmap), the x band scale still carried the
bar-style outer padding (0.2), so columns were inset ~4-5px from the
left/right axis lines while the rows (padding 0) sat flush top/bottom — an
asymmetric, gappy look. When the y-axis is categorical, drop the x padding
too so the grid fills the plot edge-to-edge and meets both axis lines
flush on every side. Also hoists the yBandKey detection so x and y share it.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:charts Charts (packages/charts, ChartV2) CLA Signed This label is managed by the Meta Open Source bot. type:fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants