Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-charts): pie, funnel and treemap say when rows carry no magnitude they can draw - #7169
Conversation
…gnitude they can draw These four families (pie, donut, funnel, treemap) size a mark BY its measure, so a row whose value is zero, negative, null or unparseable stays in `data` and is simply given no area. That is a third mechanism, distinct from the early return objectui#7146 answered and the silent row drop objectui#7148 answered: those rows are never filtered, so objectui#7148's dropped-row count is exactly zero here and hoisting its footnote would have rendered nothing while looking like coverage. Measured in real Chromium across 74 tiles at 40c4711, each screenshotted, MD5'd and pixel-diffed against an empty div of the same box: - all-zero, all-null and all-negative pies put ZERO non-white pixels out of 124,800 on the page, byte-identical to the empty div, with 31 descendants and a real svg in the DOM - a pie handed 40 beside a null drew a FULL circle, 99.35% pixel-identical to a legitimately one-row dataset - a funnel handed 40 beside a null drew ZERO segments and one label reading the name of the row that had NO value - a treemap handed 40/null, 40/0 or 40/-25/-12 drew one full-bleed leaf, byte-identical to a genuinely one-row treemap in all three cases - an all-zero treemap drew one full-bleed leaf labelled with the LAST category No row sizable now renders the file's refusal shell under its own code (no-positive-magnitude); some rows sizable keeps the plot and adds a note counting the rest (data-chart-note="unsized-rows"). All-positive charts gain no wrapper element, the no-rows case is untouched (that is the empty-result question, answered upstream in ObjectChart), bar keeps its axes, and both sankey answers keep their own codes -- every sankey and bar tile hashed identically before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-warren
commented
Sep 1, 2026
✅ Reviewed — will arm on green
|
| case | result |
|---|---|
| pie/donut, all-zero / all-null / all-negative | 0 non-white pixels of 124,800, while carrying 31 DOM descendants and a real svg |
pie, 40 beside a null | drew a FULL circle — 99.35% pixel-identical to a legitimately one-row dataset |
funnel, 40 beside a null | 0 segments and ONE label — reading the name of the row that had NO value. The row worth 40 vanished and the empty one got the label |
treemap, 40/null, 40/0, 40/-25/-12 | byte-identical (0.000%) to a genuinely one-row treemap — four datasets, one image |
The funnel case is the sharpest thing here. It is not a blank and not a partial draw; it is a chart labelling the wrong row. A reader sees one named category and no indication that the category carrying all the value was dropped.
measured-and-declined was assessed per family — and survived for two
Bar and radar are clean and stay untouched. Radar's grid and axes survive an all-zero dataset with hashes distinct from both its all-positive and its one-row control. That is the outcome I most wanted to see possible: the fence said declining was legitimate, and it was taken where the evidence supported it rather than applied uniformly because a fix was available.
50 of 74 tiles unchanged, including every all-positive control, every one-row control, every no-rows tile, all 9 bar tiles and all 9 sankey tiles byte-identical.
The ablation design is the best of the session
Two legs with disjoint red sets. Neutering the refusal guard: 26 failed / 38 passed, all 26 in the refusal block. Neutering the note guard: 26 failed / 38 passed, all 26 in the note block. Each leg leaves the other's 38 green — which is what makes them discriminating rather than merely red.
And the third leg is the part I have not seen before: you reproduced the pre-fix renderer and verified the reproduction against the true pre-fix run — 56/56 tiles byte-identical. So the before/after table is measured against the real thing, not against a stand-in that merely resembles it. That closes the gap where a reconstruction quietly differs from what it claims to reconstruct.
Browser instrument rebuilt on each leg with the markers verified present in dist/assets/*.js afterwards — so the measurement ran against mutated code, not a stale bundle. And the three tracked-file gates were re-run after git add with their counts moving (5940→5942, 524→525), which is how you know they saw the new test rather than skipping it as untracked. That is a control on the gate itself.
Two things you reported rather than filed — both correct, both being carried
Scatter is NOT MEASURED, not clean. Every scatter tile including the all-positive control drew zero marks, so its zeros say nothing — a control that returns zero is no control. Scatter takes two measures and this sweep's single-measure fixture does not bind it. I am filing that as its own card; it has no existing home, unlike objectui#7148's finding which had objectui#7147 to attach to.
CHART_MIN_HEIGHT (280), so no footnote can be seen in one — measured with objectui#7148's landed note as the live control, byte-identical before and after at 240. That means objectui#7148's shipped note is equally invisible at that size. You are right that this PR inherits the property without worsening it, and right to quote every before/after number at a box that fits the floor. I am recording it on objectui#7148.
Ledger
Refusal ships under its own code no-positive-magnitude rather than reusing objectui#7146's — correct, since the codes name which refusal fired. One shared predicate Number.isFinite(v) && v > 0 across three arms rather than three spellings. type-check first returned NOT MEASURED (16 unbuilt-closure errors), named as such and re-run to a real verdict after building the closure.
Generated by Claude Code
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7147
Re-derived on
origin/main40c4711 (the head this branch forks from; it already carries PR #7146'sno-positive-flowrefusal and PR #7161'sChartFootnote). The card's snippets and line numbers were stale, as PM assumed.Rendered it and looked first
74 tiles in real Chromium (
/opt/pw-browsers/chromium) — 8 chart families x 9 datasets, plus a blank reference and a live console control. Every tile screenshotted, MD5'd, and pixel-diffed against a literally emptydivof the same box.measured-and-declinedwas genuinely on the table for each family separately. It survived for radar and bar, and for none of pie / donut / funnel / treemap.The instrument's discriminating power, from its own controls: a two-row pie differs from a one-row pie by 9.683% of its pixels, a two-row treemap from a one-row treemap by 38.301%, and an all-zero bar puts 5,128 ink pixels (axes and ticks) against a blank tile's 0.
Per-family verdict
div, while the DOM carried 31 descendants and a realsvg40beside anullpaddingAnglehairline, not information). The picture asserts "Alpha is 100%" of a dataset where Beta was never measured40beside anull40 / null,40 / 0,40 / -25 / -120.000%) to a genuinely one-row treemap — one full-bleed leaf labelled "Alpha". Four datasets, one imageThe mechanism is the same across all three families, and it is neither landed answer
The rows are never filtered.
datareaches the pie, funnel and treemap elements whole — PR #7161 pinned that the sankey arm holds the only row-dropping filter in this file, and that still reads true at this head (AdvancedChartImpl.tsx:1081, the other two.filter(sites filter series). What happens instead is that the layout gives a non-positive row no area.So PR #7161's count (
data.length - rows.length) is exactly zero against these families. A hoisted copy of that footnote would have rendered nothing while looking like coverage. Confirmed rather than assumed: the sankey tiles are the only ones in the sweep carryingomitted-rows.A2.3 was falsified in one respect and it changed nothing. Pie and funnel are not the same failure at the Recharts level — an unsizable pie row produces no sector element at all (
path.recharts-sector= 0), whereas an unsizable funnel row produces a NaN-width trapezoid that also poisons its neighbour (which is why40beside anullloses the row worth 40, not thenullone). Treemap is a third: the leaf collapses and its neighbours expand to fill the box. Three different Recharts behaviours, one authoring-side predicate — a row can only occupy area if its measure is a positive finite number — so one answer serves all three.What changed
Three helpers, stated once so the arms cannot drift, and wired into the pie/donut, funnel and treemap arms:
countSizableRows(rows, dataKey)—Number.isFinite(v) && v > 0.Number.isFiniterather than the sankey arm'sNumber(x) || 0idiom, because this predicate must also rejectInfinity, which|| 0lets straight through.data-chart-error="no-positive-magnitude".ChartFootnotewithdata-chart-note="unsized-rows", carrying the count.Copy names the predicate, not a cause — the reason
no-positive-flow's docstring already gives: five shapes reach here (a genuine zero, a negative,null, an unparseable string, a missing key) and naming any one is a sentence false for the other four.The note deliberately does not say "showing N of M". PR #7161's sankey note can, because there the missing rows are genuinely absent from the plot. Here they are not: a mixed-sign pie paints a sector for every row (measured:
40 / -25 / -12drew 3 sectors) — it just paints them at a scale that means nothing. "Showing 1 of 3" would be a false statement about what is on the screen.No console warning, matching both sankey answers and unlike the two guards at the bottom of the file: those carry a diagnostic pair that does not fit on screen, whereas these sentences already name the key, the test it failed, and how many rows failed it.
i18n: none added. This file has no i18n call sites at all and both landed refusals plus PR #7161's footnote are hardcoded English; this matches the file.
Before / after, and the 50 tiles that did not move
Measured at a tile taller than the chart's own
CHART_MIN_HEIGHTfloor of 280 (see "assumption falsified" below). The pre-fix renderer was reproduced by double ablation and verified against the true pre-fix run: 56/56 tiles byte-identical, so the "before" column is the real thing rather than a stand-in.24 tiles changed. 50 did not. The 50 are what make the 24 discriminating:
be719ec4d968is broken)ObjectCharthas NO empty branch at all — an empty result draws a bare chart frame, the fourth distinct answer on this surface to "is it broken or is it young" #7130), answered upstream inObjectChartwhere the query outcome is known, so the refusal is gated ontotal > 0no-positive-flowon all-zero/null/negative andomitted-rowson all three thinned datasets. Both landed answers keep firing under their own codes.treemap--posNullandtreemap--posZerostill share an image after the fix, and that is correct: to a chart that sizes by value anulland a0are the same fact, and the copy names the predicate rather than the cause.Seam map, pinned from both sides
no-positive-flow(#7146)no-positive-magnitude(this PR)omitted-rows(#7161)unsized-rows(this PR)A test asserts the four codes are mutually exclusive across every family x dataset pair in the sweep, and two more assert a sankey never receives this PR's code or attribute and vice versa.
Console diagnostic, with its control
Across all 72 chart tiles: zero console output. That zero is readable only because the same instrument's positive control did fire on the same run — a rows-carry-no-category-key tile printed
[chart] no row has the category key "name" .... A2.4 confirmed on the widened population.Assumptions falsified
hsl(var(--foreground))resolves and the labels are visible. The finding stands regardless — 0 of 2 segments — but the specific pixel-identity claim is not what I measured, so I am not repeating it.CHART_MIN_HEIGHTof 280, so no footnote can be seen in it. Measured with the landed footnote as the control:sankey--posNullis byte-identical before and after this change at 240 (2b123b92769b), i.e. PR fix(plugin-charts): a sankey that drew only some of its rows says how many #7161's shipped note is equally invisible there. This is a pre-existing property of sub-280 boxes that this PR inherits and does not worsen, not something introduced here. All before/after numbers above are therefore quoted at a box that fits the floor.Tests
AdvancedChartImpl.degenerateMagnitude.test.tsx, 64 tests. Run from the repo root with root-relative paths.Ablation, direction predicted before running
Two legs, each proving mutation on disk by marker count and blob hash and restore by state, both with an absolute-path
trap:sizable === 0tosizable === -1, 3 sites)unsized <= 0tounsized >= 0, 1 site)The two red sets are disjoint, and each leg left the other 38 green — which is what makes them discriminating rather than a blanket break. Blob hashes:
433eb895...mutated to328319dd...(A) and5d9ad41a...(B); both restored to433eb895...withgit diff HEADempty.Gates
All run on the committed head
dd79defc7, after the final commit.vitest packages/plugin-charts/Test Files 39 passed (39)/Tests 329 passed (329)type-checktsc --noEmit, script name echoedlint287 problems (0 errors, 287 warnings)— exit 0, all warnings pre-existingcheck-changeset-presence2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-majorNo changeset declares a major bump.check-control-bytesOK (scanned 5942 tracked text file(s); skipped 85 binary).check-vi-mock-specifiersOK (4099 tracked source file(s), 2371 test-named; 525 carry a mock; ...)check-vi-mock-inheritOK (4099 tracked source file(s), ... 118 inherit, 0 auto-mocked ...)check-lint-coveragelint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).check-type-check-coveragetype-check coverage: 45/46 via type-check, ... 1 not compiled.The three tracked-file gates were re-run after
git add; their own counts moved (5940 to 5942 text files, 524 to 525 files carrying a mock), which is how I know they actually saw the new test rather than skipping it as untracked.type-checkwasNOT MEASUREDon first attempt — 16Cannot find module '@object-ui/*'errors because the dependency closure was unbuilt. Built withpnpm --workspace-concurrency=2 --filter '@object-ui/plugin-charts^...' buildand re-run to a real verdict.tsc --listFilesconfirms both edited files are in the compiled set (1 hit each), so the green covers them.Declared narrowing: repo-wide
pnpm lintwas not run locally; CI owns it. The per-package run is a measurement, not a guess — eslint's own config selected 52 files in this package (--format jsoncount), both edited files are in that set with 0 errors, and this package's eslint config enables no type-aware linting, so nothing in this diff can move a verdict in a file it does not touch.Out of scope, reported not fixed
scatterisNOT MEASURED, not clean. Its all-positive control drew zero marks on this instrument (markN: 0on every scatter tile including the control), so its zeros say nothing — a control that returns zero is no control. Scatter takes two measures and this sweep's single-measure fixture does not bind it. Worth a proper sweep with a correct fixture; I did not file a card, per the report-do-not-duplicate fence.radarmeasured and clean for this defect class: grid and axes survive an all-zero dataset and its hashes are distinct from both its all-positive and its one-row control.Generated by Claude Code