Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-charts): a sankey with no positive flow says so instead of rendering an empty div - #7146
Conversation
… rendering an empty div The sankey arm keeps only strictly positive measures, so a chart handed real rows whose measure is all 0, all null, all negative, or unparseable built no links and returned a bare `<div>`. Measured in Chromium against a populated control (1 svg / 7 path / 26 descendants): each blank tile rendered `descendantCount: 1`, `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one another — no marks, no text, no role, and so indistinguishable from a widget that had crashed. It now renders through the `ChartRefusal` shell the file's two other refusals use, with `role="status"` and `data-chart-error="no-positive-flow"`. Two boundaries are pinned: no rows at all still returns the bare div (that is the empty-result question, answered upstream in ObjectChart), and one positive row among zeros still draws. 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 and armedAll 30 check runs read in one call at ⚖️ Ruling on the open question — Option A, as shipped. No change needed.You asked whether an all-zero flow and an all-negative flow deserve separate codes and copy. Keeping A: one code The reason is not that B is wrong in principle — you are right that the two situations are distinguishable at runtime, unlike the byte-identical upstream shapes that forced And the situation B would gesture at — a dataset where negatives are present and meaningful — is not reachable from this branch at all. It is objectui#7148, where some rows survive the filter and the chart draws a confident partial flow. Spending this card's copy budget hinting at a defect that lives in the branch next door is worse than naming the predicate plainly. Filing it as its own card was the right instinct. C is correctly rejected for the reason you gave: the file's doctrine is that the code names which refusal fired, so copy varying under one code makes the code stop meaning that.
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7140
What was measured, before anything was written
The card asked for the blank to be rendered and looked at first, and said
closing it measured-and-declined would be a full success. So it was rendered
first, in a real Chromium, at
origin/maine8e4c4d, in a 520x240 tileagainst a populated control — twelve tiles, one page.
0nullstatusThe four blank sankey tiles hashed byte-identical screenshots to one
another (
b53cc8c4…), and the control hashed differently — so the controlreturns non-zero on the same instrument and the zeros are real.
The decision this produced: fix, not decline. Not because a blank is the
wrong picture of an empty flow, but because of what the DOM says: one element,
no marks, no text, no
role— the only path in this file that puts nothingat all on the page. A reader cannot tell a genuinely all-zero flow from a
widget that crashed, and a screen-reader user is handed literally nothing. That
is the exact distinction the file's two existing refusals exist to make, three
lines away.
The change
AdvancedChartImpl.tsx, the sankey arm only. When the filter keeps no rows,it now renders through the
ChartRefusalshell already in this file — samebox, same
role="status", newdata-chart-error="no-positive-flow":Two boundaries are deliberate, and each is pinned by a test:
question, answered upstream in
ObjectChartwhere the query outcome isknown. "No row's measure is above zero" would be a false sentence about a
dataset with no rows in it — measured: that tile hits the same branch today.
link set, never on a thin one.
One code, one sentence, three causes. A genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures
Number(...) || 0folds to zero all arrive at the same branch. The causes differ — an all-zero
pipeline is a young dataset, an all-negative one is a data-modelling mistake —
but naming any single one is a sentence that is false for the other two, so the
copy names the predicate the filter actually applies, which is true for all
three. This is the same reasoning
hasNoPlottableSeries' own docstring givesfor its single code and two causes. No recovery is promised.
No console warning, unlike the two refusals below it: those carry a
diagnostic pair (
xAxisKeyplus the keys the rows carry) that does not fit onscreen. This message already names the key and the exact test it failed, and
the guard sits inside the arm — beside the filter it reads — where no hook can
be added and where it cannot drift from that filter.
Blast radius, measured the same way
Re-rendered the same twelve tiles after the change and hashed every screenshot
against its before image:
unparseable sankey tiles.
one-positive-among-zeros sankey, the no-rows sankey, bar / pie / funnel /
treemap all-zero, and the existing
missing-category-keyrefusal.Ablation
Direction predicted before the run: reverting the guard turns the four
no-positive-flow cases red and leaves the four boundary cases green.
Mutation proven on disk by blob hash and marker count, not by an editor's
exit code:
3105af33…->baf47ce1…, guard marker1->0.4 failed | 4 passed (8)— matching the prediction exactly. The 4 survivorsare the boundary tests, which assert behaviour the change does not touch;
a run that collapsed to "4 failed, 0 passed" would have meant the file stopped
loading rather than that the guard was doing the work.
Restore proven by state:
git diff HEAD,git diff --cachedandgit status --shortall empty afterwards, worktree blob back to3105af33…, marker back to1. No dist leg applies here — the test imports./AdvancedChartImplrelatively inside its own package, so vitest reads themutated source directly, which the red/green flip itself demonstrates.
Verification — union run at
c14d945b6pnpm exec vitest run packages/plugin-charts/Test Files 36 passed (36)/Tests 248 passed (248)Test Files 1 passed (1)/Tests 8 passed (8)pnpm --filter '@object-ui/plugin-charts' run type-checktsc --listFilesputs both changed files in the program, so this reading covers the new test filenpx eslint .inpackages/plugin-chartsno-explicit-any/react-refresh; 49 files linted, both changed files among them, 6 warnings on the new test (siblings carry 2-17 of the same)pnpm changeset:checkAll workspace packages are in the changeset fixed group./privatePackages declared: version=true, tag=false./No changeset declares a 'major' bump.check-control-bytesOK (scanned 5922 tracked text file(s); skipped 85 binary)check-vi-mock-specifiersOK (4089 tracked source file(s), 2362 test-named; 521 carry a mock ...)check-vi-mock-inheritOK (... 117 inherit, 0 auto-mocked ...)check-lint-coveragelint coverage: 46/46 packages linted, 0 with outstanding errorscheck-i18n-call-site-keyscheck-i18n-en-driftNo en value changed in this range.check-i18n-dead-keyscheck-phantom-dependenciesEvery in-scope import is declared by the package that publishes it.check-package-self-importNo package names itself inside its own src/.check-sdui-registration-pinsNo console build to weigh at apps/console/dist/assets; the gate itself says "This is exit 2, not a pass ... a run with nothing to read has measured nothing". CI builds the console and owns this reading. Declared narrowing: this diff adds no registration and touches no registration array.The whole-repo
pnpm lintand the rest of the gate farm are CI's run, notduplicated here.
Scope
AdvancedChartImpl.tsxhas no i18n call sites at all— both existing refusals are hardcoded English — and
check-i18n-driftconfirms no
envalue moved. Adding a translation hook here would be a newsubsystem in a component whose hook count is deliberately fixed, not this
card.
ChartRefusalshell in this same file, which took this case withoutmodification — it already accepts a
code, aclassNameand children.series: []#4695 is not addressed here and its path is not touched: it is acartesian chart handed no series binding, which never reaches the sankey arm.
Files:
packages/plugin-charts/src/AdvancedChartImpl.tsx,packages/plugin-charts/src/AdvancedChartImpl.sankeyNoPositiveFlow.test.tsx(new),
.changeset/7140-sankey-no-positive-flow.md(new).Generated by Claude Code