fix(plugin-charts): a sankey that drew only some of its rows says how many#7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-warren merged 2 commits into from
Sep 1, 2026
Merged

fix(plugin-charts): a sankey that drew only some of its rows says how many #7161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d0af30
fix(plugin-charts): a sankey that drew only some of its rows says how…
claude Sep 1, 2026
f2df891
chore(changeset): sankey partial-flow footnote
claude Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .changeset/7148-sankey-omitted-rows-note.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
'@object-ui/plugin-charts': patch
---

A sankey that drew only SOME of its rows now says how many (objectui#7148).

The sankey arm keeps strictly positive measures
(`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a mixed dataset
drew a normal, healthy, confident chart of a fraction of itself and nothing
anywhere recorded that the other rows existed. Measured in Chromium across 27
tiles: `[{New business: 40}, {Refunds: -25}, {Chargebacks: -12}]` rendered
`svg: 1`, `path: 3`, 18 descendants, no `role`, no text, and — against a live
console control that did fire on the same instrument — zero console output.
Its screenshot hashed byte-identical to five other datasets, one of which
genuinely had a single row. Six datasets, one image: a reader had no bit of
information separating a complete flow from a third of one.

The discard itself stands — a flow has no negative width, so it is the only
thing that arm can do with those rows. What is added is a footnote under the
plot naming the ratio and the predicate the filter applies:

> Showing 1 of 3 rows — 2 rows have no `amount` above zero, which a flow
> cannot draw.

It names the predicate rather than a cause because `Number(…) || 0` folds
negatives, zeros, `null`, unparseable strings and a missing key into one
discard, and all five were measured reaching this branch beside a survivor;
naming any one of them is a sentence that is false for the other four.

A complete flow is byte-for-byte unchanged and gains no wrapper element, and a
drawable sankey is never replaced by prose: the `no-positive-flow` refusal
still owns the case where NOTHING survives the filter, and the "one positive
among zeros still draws" boundary still draws — that fixture is itself a
thinned dataset, so it now draws *and* says so.
222 changes: 222 additions & 0 deletions packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — SOME rows survive the positive filter and the rest are dropped
* silently (objectui#7148). The branch next door to objectui#7140's refusal,
* and the more dangerous of the two.
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`). When that filter
* keeps NOTHING, objectui#7146 now says so. When it keeps SOME, the chart drew
* a normal, healthy, confident sankey of a fraction of its dataset and nothing
* anywhere recorded that the other rows existed.
*
* ## The measurement that decided fix-over-decline
*
* 27 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main`
* fd11e1644, each tile screenshotted and SHA-256'd. The card's own dataset
* (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
* `path: 3`, 18 descendants, no `role`, no text — and, against a live console
* control that DID fire on the same instrument (`missing-category-key`), zero
* console output. Its screenshot hashed `13237e6e19a7072a`, byte-identical to
* FIVE other tiles including a genuinely one-row dataset
* `[{ stage: 'New business', amount: 40 }]`. Six datasets, one image: the
* reader had no bit of information separating a complete flow from a third of
* one. That is why "discarding is all a flow CAN do" does not settle the card —
* the drop is fine, the silence is not.
*
* ## Why a note and not a refusal
*
* objectui#7146 pins "one positive row among zeros still draws". That fixture
* (`0 / 7 / 0`) is itself a THINNED dataset — it lands in this branch and
* hashed identical to the mixed-sign tile — so a refusal here would blank a
* chart that pin requires drawn. The two cards meet exactly at
* `rows.length === 0`: none survive → refusal (objectui#7146); some survive and
* some do not → this note; all survive → untouched.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const noteOf = (container: HTMLElement) =>
container.querySelector('[data-chart-note="omitted-rows"]');
const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* Every shape `Number(…) || 0` folds to zero, each one BESIDE a survivor.
*
* They are not the same situation and the copy deliberately names none of them:
* a message that said "negative" would be false of the null row, and the card
* itself notes that `null` and unparseable measures vanish through the very
* same filter. All five were measured reaching this branch in Chromium.
*/
const THINNED: Array<[string, Array<Record<string, unknown>>, number, number]> = [
['negative rows (the card\'s own dataset)', [
{ stage: 'New business', amount: 40 },
{ stage: 'Refunds', amount: -25 },
{ stage: 'Chargebacks', amount: -12 },
], 1, 3],
['zero rows (objectui#7146\'s pinned boundary)', [
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
], 1, 3],
['null measures', [
{ stage: 'New business', amount: 40 },
{ stage: 'Refunds', amount: null },
{ stage: 'Credits', amount: null },
], 1, 3],
['unparseable measures', [
{ stage: 'New business', amount: 40 },
{ stage: 'Refunds', amount: 'n/a' },
], 1, 2],
['a row missing the measure key entirely', [
{ stage: 'New business', amount: 40 },
{ stage: 'Refunds' },
], 1, 2],
['a mix that keeps more than one row', [
{ stage: 'New business', amount: 40 },
{ stage: 'Expansion', amount: 25 },
{ stage: 'Chargebacks', amount: -12 },
], 2, 3],
];

describe('AdvancedChartImpl — sankey that drew only SOME of its rows (objectui#7148)', () => {
it.each(THINNED)('says how many rows it drew when handed %s', (_label, rows, kept, total) => {
const { container } = renderSankey(rows);

const note = noteOf(container);
expect(note, 'a partial flow states that it is partial').not.toBeNull();

// BOTH halves of the count. "Some rows were dropped" would still leave a
// thinned flow indistinguishable from a complete one; the ratio is the bit
// the picture cannot carry.
expect(note?.textContent).toContain(`Showing ${kept} of ${total} rows`);
// Names the measure it tested and the exact test it failed, so the author
// knows WHICH column decided it — the same diagnosis the refusal gives.
expect(note?.textContent).toContain('amount');
expect(note?.textContent).toContain('above zero');
// A note annotates; it is not a state change and not an alert.
expect(note?.getAttribute('role')).toBe('note');

// ⛔ The chart is still DRAWN. The drop is not the defect — a flow has no
// negative width — so nothing here may replace a drawable sankey with
// prose. This is objectui#7146's boundary restated from the other side.
expect(container.querySelector('svg'), 'a drawable sankey still draws').not.toBeNull();
expect(refusalOf(container), 'a drawn chart is never also a refusal').toBeNull();
});

it('agrees with itself in the singular', () => {
const { container } = renderSankey([
{ stage: 'New business', amount: 40 },
{ stage: 'Refunds', amount: -25 },
]);
expect(noteOf(container)?.textContent).toContain('1 row has no');
});

it('agrees with itself in the plural', () => {
const { container } = renderSankey([
{ stage: 'New business', amount: 40 },
{ stage: 'Refunds', amount: -25 },
{ stage: 'Chargebacks', amount: -12 },
]);
expect(noteOf(container)?.textContent).toContain('2 rows have no');
});

it('CONTROL — an all-positive sankey carries no note, and gains no wrapper', () => {
const { container } = renderSankey([
{ stage: 'New business', amount: 40 },
{ stage: 'Expansion', amount: 25 },
{ stage: 'Renewal', amount: 12 },
]);

expect(noteOf(container), 'nothing was omitted, so nothing is claimed').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
// The complete-flow path returns the container it always returned. The
// footnote's flex wrapper would take over the height chain (see
// `ChartFootnote`), so a chart with nothing to say must not get one.
expect(
container.firstElementChild?.getAttribute('data-slot'),
'the chart container is still the root element',
).toBe('chart');
});

// ---- the seam with objectui#7146 -------------------------------------
//
// `rows.length` is the whole boundary: 0 survivors is that card, 1-or-more
// survivors alongside a casualty is this one, and no casualties at all is
// neither. Pinned from both sides so neither answer can drift into the
// other's branch.

it('SEAM — no row survives: objectui#7146 refuses, and this note stays out of it', () => {
const { container } = renderSankey([
{ stage: 'A', amount: 0 },
{ stage: 'B', amount: 0 },
{ stage: 'C', amount: -5 },
]);

expect(refusalOf(container), 'all-non-positive is still the refusal branch').not.toBeNull();
expect(noteOf(container), 'a chart that drew nothing has no partial draw to annotate').toBeNull();
expect(container.querySelector('svg')).toBeNull();
});

it('SEAM — no rows at all: still the bare div, untouched by both cards', () => {
const { container } = renderSankey([]);

expect(refusalOf(container)).toBeNull();
expect(noteOf(container)).toBeNull();
// The empty-RESULT question (objectui#7130), answered upstream in
// ObjectChart where the query outcome is known.
expect(container.textContent).toBe('');
});

it('does not reach other chart families handed the same mixed-sign rows', () => {
// The note reads the sankey filter's own result and no other family has
// such a filter — measured in Chromium, bar/pie/donut/funnel/treemap all
// keep every row of this dataset in their data array. Pinned because
// hoisting the count out of this arm is the obvious refactor and would
// annotate four charts that omitted nothing.
for (const chartType of ['bar', 'pie', 'donut', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[
{ stage: 'New business', amount: 40 },
{ stage: 'Refunds', amount: -25 },
] as any}
/>,
);
expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull();
cleanup();
}
});
});
122 changes: 111 additions & 11 deletions packages/plugin-charts/src/AdvancedChartImpl.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,6 +373,40 @@ function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?:
);
}

/**
* `ChartFrame`'s mirror image: chrome BELOW the plot, for a chart that drew
* something but did not draw all of it (objectui#7148).
*
* Same construction, and deliberately so, because the construction is the whole
* difficulty. A footnote cannot simply be rendered as a SIBLING of
* `ChartContainer`: measured in Chromium in the dashboard shape — a fixed,
* clipping card whose chart carries `h-full` — the container takes the card's
* full height and a following `<p>` lands at y=327 in a box that ends at y=316,
* i.e. entirely outside the clip. A note invisible in dashboards is no note at
* all, and dashboards are where these charts live.
*
* Nor can it be a plain wrapper `<div className={className}>` around the
* container: the consumer's className is the chart's height contract and it has
* to keep reaching the element Recharts measures (see `CHART_MIN_HEIGHT` in
* `ChartContainerImpl` for the measurement of what happens when it does not —
* a permanently zero box and an invisible chart, with no refusal and no empty
* state).
*
* So the plot keeps its own `className` and gains a definite height through the
* flex chain instead: `h-full` outer, `min-h-0 flex-1` around the plot,
* `shrink-0` under it. With no footnote this returns the chart untouched, so no
* existing caller gains a wrapper element — the same gate `ChartFrame` uses.
*/
function ChartFootnote({ note, children }: { note?: React.ReactNode; children: React.ReactNode }) {
if (!note) return <>{children}</>;
return (
<div className="flex h-full w-full flex-col">
<div className="min-h-0 flex-1">{children}</div>
<div className="mt-1 shrink-0">{note}</div>
</div>
);
}

/**
* AdvancedChartImpl - The heavy implementation that imports Recharts with full features
* This component is lazy-loaded to avoid including Recharts in the initial bundle
Expand DownExpand Up@@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({
</ChartRefusal>
);
}
// A PARTIAL flow SAYS it is partial — objectui#7148, the branch next door
// to the refusal above.
//
// The filter is unconditional, so a dataset where only SOME rows survive it
// draws a normal, healthy, confident sankey of a fraction of itself, and
// nothing in the output carried that fact. Measured in Chromium at
// origin/main fd11e1644 across 27 tiles: the card's own dataset
// (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
// `path: 3`, 18 descendants, no `role`, no text, and — against a live
// console control that did fire on the same instrument — ZERO console
// output. Its screenshot hashed `13237e6e19a7072a`, BYTE-IDENTICAL to a
// genuinely one-row dataset `[{ New business: 40 }]` and to three other
// thinned shapes (one positive among zeros, positive + nulls, positive +
// unparseable). Six datasets, one image. A reader had no bit of information
// distinguishing a complete flow from a third of one, and nobody re-reads a
// dataset that renders fine.
//
// ## Why a note beside the chart, and not a refusal
//
// Not a style preference — a refusal is unavailable here. objectui#7146
// pins "one positive row among zeros still draws", and that fixture
// (`0 / 7 / 0`) is ITSELF a thinned dataset: it lands in this branch, and
// hashes identical to the mixed-sign tile above. Refusing on a thinned flow
// would blank the chart that pin requires drawn.
//
// The drop is also not the defect. A flow has no negative width, so
// discarding those rows is the only thing this arm CAN do with them. What
// was missing was saying so — which is the whole change: the plot is the
// element this arm already returned, unchanged, with one line of prose
// under it.
//
// ## Why the copy names the PREDICATE and a COUNT, not a cause
//
// The reason the refusal above gives, and this branch is where that family
// actually lives: `Number(…) || 0` folds negatives, zeros, `null`,
// unparseable strings and a missing key into ONE discard, and all five were
// measured reaching here beside a survivor. Naming any one of them is a
// sentence that is false for the other four, so the copy names the
// predicate the filter actually applies, which is true of all of them.
//
// The COUNT is the half a reader cannot recover from the picture. "Some
// rows were dropped" still leaves a thinned flow indistinguishable from a
// complete one; `1 of 3` is the bit that was missing.
//
// No console warning, matching the refusal above and unlike the two guards
// at the bottom of this file: those carry a diagnostic PAIR that does not
// fit on screen, whereas this sentence already names the key, the test it
// failed, and how many rows failed it.
const omittedRowCount = data.length - rows.length;
return (
<ChartContainer config={config} className={className} {...containerProps}>
<Sankey
data={{ nodes, links }}
nodePadding={24}
link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
node={{ fill: 'hsl(var(--chart-1))' } as any}
{...sankeyClickProps}
>
<Tooltip />
</Sankey>
</ChartContainer>
<ChartFootnote
note={
omittedRowCount > 0 ? (
<p
role="note"
data-chart-note="omitted-rows"
className="px-1 text-xs text-muted-foreground"
>
Showing {rows.length} of {data.length} rows &mdash; {omittedRowCount}{' '}
{omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '}
<code className="font-mono">{dataKey}</code> above zero, which a flow cannot
draw.
</p>
) : null
}
>
<ChartContainer config={config} className={className} {...containerProps}>
<Sankey
data={{ nodes, links }}
nodePadding={24}
link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
node={{ fill: 'hsl(var(--chart-1))' } as any}
{...sankeyClickProps}
>
<Tooltip />
</Sankey>
</ChartContainer>
</ChartFootnote>
);
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix(plugin-charts): a sankey that drew only some of its rows says how many#7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-warren
merged 2 commits into

from

Sep 1, 2026
 Merged


fix(plugin-charts): a sankey that drew only some of its rows says how many
#7161
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
8d0af30

fix(plugin-charts): a sankey that drew only some of its rows says how…


claude Sep 1, 2026
f2df891

chore(changeset): sankey partial-flow footnote


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


34 changes: 34 additions & 0 deletions
.changeset/7148-sankey-omitted-rows-note.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
'@object-ui/plugin-charts': patch
---

A sankey that drew only SOME of its rows now says how many (objectui#7148).

The sankey arm keeps strictly positive measures
(`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a mixed dataset
drew a normal, healthy, confident chart of a fraction of itself and nothing
anywhere recorded that the other rows existed. Measured in Chromium across 27
tiles: `[{New business: 40}, {Refunds: -25}, {Chargebacks: -12}]` rendered
`svg: 1`, `path: 3`, 18 descendants, no `role`, no text, and — against a live
console control that did fire on the same instrument — zero console output.
Its screenshot hashed byte-identical to five other datasets, one of which
genuinely had a single row. Six datasets, one image: a reader had no bit of
information separating a complete flow from a third of one.

The discard itself stands — a flow has no negative width, so it is the only
thing that arm can do with those rows. What is added is a footnote under the
plot naming the ratio and the predicate the filter applies:

> Showing 1 of 3 rows — 2 rows have no `amount` above zero, which a flow
> cannot draw.

It names the predicate rather than a cause because `Number(…) || 0` folds
negatives, zeros, `null`, unparseable strings and a missing key into one
discard, and all five were measured reaching this branch beside a survivor;
naming any one of them is a sentence that is false for the other four.

A complete flow is byte-for-byte unchanged and gains no wrapper element, and a
drawable sankey is never replaced by prose: the `no-positive-flow` refusal
still owns the case where NOTHING survives the filter, and the "one positive
among zeros still draws" boundary still draws — that fixture is itself a
thinned dataset, so it now draws *and* says so.

222 changes: 222 additions & 0 deletions
packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
/**
 * ObjectUI
 * Copyright (c) 2024-present ObjectStack Inc.
 *
 * Sankey — SOME rows survive the positive filter and the rest are dropped
 * silently (objectui#7148). The branch next door to objectui#7140's refusal,
 * and the more dangerous of the two.
 *
 * The sankey arm keeps only strictly positive measures
 * (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`). When that filter
 * keeps NOTHING, objectui#7146 now says so. When it keeps SOME, the chart drew
 * a normal, healthy, confident sankey of a fraction of its dataset and nothing
 * anywhere recorded that the other rows existed.
 *
 * ## The measurement that decided fix-over-decline
 *
 * 27 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main`
 * fd11e1644, each tile screenshotted and SHA-256'd. The card's own dataset
 * (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 * `path: 3`, 18 descendants, no `role`, no text — and, against a live console
 * control that DID fire on the same instrument (`missing-category-key`), zero
 * console output. Its screenshot hashed `13237e6e19a7072a`, byte-identical to
 * FIVE other tiles including a genuinely one-row dataset
 * `[{ stage: 'New business', amount: 40 }]`. Six datasets, one image: the
 * reader had no bit of information separating a complete flow from a third of
 * one. That is why "discarding is all a flow CAN do" does not settle the card —
 * the drop is fine, the silence is not.
 *
 * ## Why a note and not a refusal
 *
 * objectui#7146 pins "one positive row among zeros still draws". That fixture
 * (`0 / 7 / 0`) is itself a THINNED dataset — it lands in this branch and
 * hashed identical to the mixed-sign tile — so a refusal here would blank a
 * chart that pin requires drawn. The two cards meet exactly at
 * `rows.length === 0`: none survive → refusal (objectui#7146); some survive and
 * some do not → this note; all survive → untouched.
 */
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
 const actual = await vi.importActual<any>('recharts');
 return {
 ...actual,
 ResponsiveContainer: ({ children }: any) =>
 React.cloneElement(children, { width: 480, height: 320 }),
 };
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
 render(
 <AdvancedChartImpl
 chartType="sankey"
 xAxisKey="stage"
 series={SERIES as any}
 data={data as any}
 />,
 );

const noteOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-note="omitted-rows"]');
const refusalOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-error="no-positive-flow"]');

/**
 * Every shape `Number(…) || 0` folds to zero, each one BESIDE a survivor.
 *
 * They are not the same situation and the copy deliberately names none of them:
 * a message that said "negative" would be false of the null row, and the card
 * itself notes that `null` and unparseable measures vanish through the very
 * same filter. All five were measured reaching this branch in Chromium.
 */
const THINNED: Array<[string, Array<Record<string, unknown>>, number, number]> = [
 ['negative rows (the card\'s own dataset)', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 1, 3],
 ['zero rows (objectui#7146\'s pinned boundary)', [
 { stage: 'Prospecting', amount: 0 },
 { stage: 'Proposal', amount: 7 },
 { stage: 'Won', amount: 0 },
 ], 1, 3],
 ['null measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: null },
 { stage: 'Credits', amount: null },
 ], 1, 3],
 ['unparseable measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: 'n/a' },
 ], 1, 2],
 ['a row missing the measure key entirely', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds' },
 ], 1, 2],
 ['a mix that keeps more than one row', [
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 2, 3],
];

describe('AdvancedChartImpl — sankey that drew only SOME of its rows (objectui#7148)', () => {
 it.each(THINNED)('says how many rows it drew when handed %s', (_label, rows, kept, total) => {
 const { container } = renderSankey(rows);

 const note = noteOf(container);
 expect(note, 'a partial flow states that it is partial').not.toBeNull();

 // BOTH halves of the count. "Some rows were dropped" would still leave a
 // thinned flow indistinguishable from a complete one; the ratio is the bit
 // the picture cannot carry.
 expect(note?.textContent).toContain(`Showing ${kept} of ${total} rows`);
 // Names the measure it tested and the exact test it failed, so the author
 // knows WHICH column decided it — the same diagnosis the refusal gives.
 expect(note?.textContent).toContain('amount');
 expect(note?.textContent).toContain('above zero');
 // A note annotates; it is not a state change and not an alert.
 expect(note?.getAttribute('role')).toBe('note');

 // ⛔ The chart is still DRAWN. The drop is not the defect — a flow has no
 // negative width — so nothing here may replace a drawable sankey with
 // prose. This is objectui#7146's boundary restated from the other side.
 expect(container.querySelector('svg'), 'a drawable sankey still draws').not.toBeNull();
 expect(refusalOf(container), 'a drawn chart is never also a refusal').toBeNull();
 });

 it('agrees with itself in the singular', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ]);
 expect(noteOf(container)?.textContent).toContain('1 row has no');
 });

 it('agrees with itself in the plural', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ]);
 expect(noteOf(container)?.textContent).toContain('2 rows have no');
 });

 it('CONTROL — an all-positive sankey carries no note, and gains no wrapper', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Renewal', amount: 12 },
 ]);

 expect(noteOf(container), 'nothing was omitted, so nothing is claimed').toBeNull();
 expect(container.querySelector('svg')).not.toBeNull();
 // The complete-flow path returns the container it always returned. The
 // footnote's flex wrapper would take over the height chain (see
 // `ChartFootnote`), so a chart with nothing to say must not get one.
 expect(
 container.firstElementChild?.getAttribute('data-slot'),
 'the chart container is still the root element',
 ).toBe('chart');
 });

 // ---- the seam with objectui#7146 -------------------------------------
 //
 // `rows.length` is the whole boundary: 0 survivors is that card, 1-or-more
 // survivors alongside a casualty is this one, and no casualties at all is
 // neither. Pinned from both sides so neither answer can drift into the
 // other's branch.

 it('SEAM — no row survives: objectui#7146 refuses, and this note stays out of it', () => {
 const { container } = renderSankey([
 { stage: 'A', amount: 0 },
 { stage: 'B', amount: 0 },
 { stage: 'C', amount: -5 },
 ]);

 expect(refusalOf(container), 'all-non-positive is still the refusal branch').not.toBeNull();
 expect(noteOf(container), 'a chart that drew nothing has no partial draw to annotate').toBeNull();
 expect(container.querySelector('svg')).toBeNull();
 });

 it('SEAM — no rows at all: still the bare div, untouched by both cards', () => {
 const { container } = renderSankey([]);

 expect(refusalOf(container)).toBeNull();
 expect(noteOf(container)).toBeNull();
 // The empty-RESULT question (objectui#7130), answered upstream in
 // ObjectChart where the query outcome is known.
 expect(container.textContent).toBe('');
 });

 it('does not reach other chart families handed the same mixed-sign rows', () => {
 // The note reads the sankey filter's own result and no other family has
 // such a filter — measured in Chromium, bar/pie/donut/funnel/treemap all
 // keep every row of this dataset in their data array. Pinned because
 // hoisting the count out of this arm is the obvious refactor and would
 // annotate four charts that omitted nothing.
 for (const chartType of ['bar', 'pie', 'donut', 'funnel', 'treemap'] as const) {
 const { container } = render(
 <AdvancedChartImpl
 chartType={chartType}
 xAxisKey="stage"
 series={SERIES as any}
 data={[
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ] as any}
 />,
 );
 expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull();
 cleanup();
 }
 });
});

122 changes: 111 additions & 11 deletions
packages/plugin-charts/src/AdvancedChartImpl.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -373,6 +373,40 @@ function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?:
 );
}

/**
 * `ChartFrame`'s mirror image: chrome BELOW the plot, for a chart that drew
 * something but did not draw all of it (objectui#7148).
 *
 * Same construction, and deliberately so, because the construction is the whole
 * difficulty. A footnote cannot simply be rendered as a SIBLING of
 * `ChartContainer`: measured in Chromium in the dashboard shape — a fixed,
 * clipping card whose chart carries `h-full` — the container takes the card's
 * full height and a following `<p>` lands at y=327 in a box that ends at y=316,
 * i.e. entirely outside the clip. A note invisible in dashboards is no note at
 * all, and dashboards are where these charts live.
 *
 * Nor can it be a plain wrapper `<div className={className}>` around the
 * container: the consumer's className is the chart's height contract and it has
 * to keep reaching the element Recharts measures (see `CHART_MIN_HEIGHT` in
 * `ChartContainerImpl` for the measurement of what happens when it does not —
 * a permanently zero box and an invisible chart, with no refusal and no empty
 * state).
 *
 * So the plot keeps its own `className` and gains a definite height through the
 * flex chain instead: `h-full` outer, `min-h-0 flex-1` around the plot,
 * `shrink-0` under it. With no footnote this returns the chart untouched, so no
 * existing caller gains a wrapper element — the same gate `ChartFrame` uses.
 */
function ChartFootnote({ note, children }: { note?: React.ReactNode; children: React.ReactNode }) {
 if (!note) return <>{children}</>;
 return (
 <div className="flex h-full w-full flex-col">
 <div className="min-h-0 flex-1">{children}</div>
 <div className="mt-1 shrink-0">{note}</div>
 </div>
 );
}

/**
 * AdvancedChartImpl - The heavy implementation that imports Recharts with full features
 * This component is lazy-loaded to avoid including Recharts in the initial bundle
Expand DownExpand Up@@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({
 </ChartRefusal>
 );
 }
 // A PARTIAL flow SAYS it is partial — objectui#7148, the branch next door
 // to the refusal above.
 //
 // The filter is unconditional, so a dataset where only SOME rows survive it
 // draws a normal, healthy, confident sankey of a fraction of itself, and
 // nothing in the output carried that fact. Measured in Chromium at
 // origin/main fd11e1644 across 27 tiles: the card's own dataset
 // (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 // `path: 3`, 18 descendants, no `role`, no text, and — against a live
 // console control that did fire on the same instrument — ZERO console
 // output. Its screenshot hashed `13237e6e19a7072a`, BYTE-IDENTICAL to a
 // genuinely one-row dataset `[{ New business: 40 }]` and to three other
 // thinned shapes (one positive among zeros, positive + nulls, positive +
 // unparseable). Six datasets, one image. A reader had no bit of information
 // distinguishing a complete flow from a third of one, and nobody re-reads a
 // dataset that renders fine.
 //
 // ## Why a note beside the chart, and not a refusal
 //
 // Not a style preference — a refusal is unavailable here. objectui#7146
 // pins "one positive row among zeros still draws", and that fixture
 // (`0 / 7 / 0`) is ITSELF a thinned dataset: it lands in this branch, and
 // hashes identical to the mixed-sign tile above. Refusing on a thinned flow
 // would blank the chart that pin requires drawn.
 //
 // The drop is also not the defect. A flow has no negative width, so
 // discarding those rows is the only thing this arm CAN do with them. What
 // was missing was saying so — which is the whole change: the plot is the
 // element this arm already returned, unchanged, with one line of prose
 // under it.
 //
 // ## Why the copy names the PREDICATE and a COUNT, not a cause
 //
 // The reason the refusal above gives, and this branch is where that family
 // actually lives: `Number(…) || 0` folds negatives, zeros, `null`,
 // unparseable strings and a missing key into ONE discard, and all five were
 // measured reaching here beside a survivor. Naming any one of them is a
 // sentence that is false for the other four, so the copy names the
 // predicate the filter actually applies, which is true of all of them.
 //
 // The COUNT is the half a reader cannot recover from the picture. "Some
 // rows were dropped" still leaves a thinned flow indistinguishable from a
 // complete one; `1 of 3` is the bit that was missing.
 //
 // No console warning, matching the refusal above and unlike the two guards
 // at the bottom of this file: those carry a diagnostic PAIR that does not
 // fit on screen, whereas this sentence already names the key, the test it
 // failed, and how many rows failed it.
 const omittedRowCount = data.length - rows.length;
 return (
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 <ChartFootnote
 note={
 omittedRowCount > 0 ? (
 <p
 role="note"
 data-chart-note="omitted-rows"
 className="px-1 text-xs text-muted-foreground"
 >
 Showing {rows.length} of {data.length} rows &mdash; {omittedRowCount}{' '}
 {omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '}
 <code className="font-mono">{dataKey}</code> above zero, which a flow cannot
 draw.
 </p>
 ) : null
 }
 >
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 </ChartFootnote>
 );
 }

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(plugin-charts): a sankey that drew only some of its rows says how many#7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-warren
merged 2 commits into

from

Sep 1, 2026
 Merged


fix(plugin-charts): a sankey that drew only some of its rows says how many
#7161
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
8d0af30

fix(plugin-charts): a sankey that drew only some of its rows says how…


claude Sep 1, 2026
f2df891

chore(changeset): sankey partial-flow footnote


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


34 changes: 34 additions & 0 deletions
.changeset/7148-sankey-omitted-rows-note.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
'@object-ui/plugin-charts': patch
---

A sankey that drew only SOME of its rows now says how many (objectui#7148).

The sankey arm keeps strictly positive measures
(`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a mixed dataset
drew a normal, healthy, confident chart of a fraction of itself and nothing
anywhere recorded that the other rows existed. Measured in Chromium across 27
tiles: `[{New business: 40}, {Refunds: -25}, {Chargebacks: -12}]` rendered
`svg: 1`, `path: 3`, 18 descendants, no `role`, no text, and — against a live
console control that did fire on the same instrument — zero console output.
Its screenshot hashed byte-identical to five other datasets, one of which
genuinely had a single row. Six datasets, one image: a reader had no bit of
information separating a complete flow from a third of one.

The discard itself stands — a flow has no negative width, so it is the only
thing that arm can do with those rows. What is added is a footnote under the
plot naming the ratio and the predicate the filter applies:

> Showing 1 of 3 rows — 2 rows have no `amount` above zero, which a flow
> cannot draw.

It names the predicate rather than a cause because `Number(…) || 0` folds
negatives, zeros, `null`, unparseable strings and a missing key into one
discard, and all five were measured reaching this branch beside a survivor;
naming any one of them is a sentence that is false for the other four.

A complete flow is byte-for-byte unchanged and gains no wrapper element, and a
drawable sankey is never replaced by prose: the `no-positive-flow` refusal
still owns the case where NOTHING survives the filter, and the "one positive
among zeros still draws" boundary still draws — that fixture is itself a
thinned dataset, so it now draws *and* says so.

222 changes: 222 additions & 0 deletions
packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
/**
 * ObjectUI
 * Copyright (c) 2024-present ObjectStack Inc.
 *
 * Sankey — SOME rows survive the positive filter and the rest are dropped
 * silently (objectui#7148). The branch next door to objectui#7140's refusal,
 * and the more dangerous of the two.
 *
 * The sankey arm keeps only strictly positive measures
 * (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`). When that filter
 * keeps NOTHING, objectui#7146 now says so. When it keeps SOME, the chart drew
 * a normal, healthy, confident sankey of a fraction of its dataset and nothing
 * anywhere recorded that the other rows existed.
 *
 * ## The measurement that decided fix-over-decline
 *
 * 27 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main`
 * fd11e1644, each tile screenshotted and SHA-256'd. The card's own dataset
 * (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 * `path: 3`, 18 descendants, no `role`, no text — and, against a live console
 * control that DID fire on the same instrument (`missing-category-key`), zero
 * console output. Its screenshot hashed `13237e6e19a7072a`, byte-identical to
 * FIVE other tiles including a genuinely one-row dataset
 * `[{ stage: 'New business', amount: 40 }]`. Six datasets, one image: the
 * reader had no bit of information separating a complete flow from a third of
 * one. That is why "discarding is all a flow CAN do" does not settle the card —
 * the drop is fine, the silence is not.
 *
 * ## Why a note and not a refusal
 *
 * objectui#7146 pins "one positive row among zeros still draws". That fixture
 * (`0 / 7 / 0`) is itself a THINNED dataset — it lands in this branch and
 * hashed identical to the mixed-sign tile — so a refusal here would blank a
 * chart that pin requires drawn. The two cards meet exactly at
 * `rows.length === 0`: none survive → refusal (objectui#7146); some survive and
 * some do not → this note; all survive → untouched.
 */
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
 const actual = await vi.importActual<any>('recharts');
 return {
 ...actual,
 ResponsiveContainer: ({ children }: any) =>
 React.cloneElement(children, { width: 480, height: 320 }),
 };
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
 render(
 <AdvancedChartImpl
 chartType="sankey"
 xAxisKey="stage"
 series={SERIES as any}
 data={data as any}
 />,
 );

const noteOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-note="omitted-rows"]');
const refusalOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-error="no-positive-flow"]');

/**
 * Every shape `Number(…) || 0` folds to zero, each one BESIDE a survivor.
 *
 * They are not the same situation and the copy deliberately names none of them:
 * a message that said "negative" would be false of the null row, and the card
 * itself notes that `null` and unparseable measures vanish through the very
 * same filter. All five were measured reaching this branch in Chromium.
 */
const THINNED: Array<[string, Array<Record<string, unknown>>, number, number]> = [
 ['negative rows (the card\'s own dataset)', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 1, 3],
 ['zero rows (objectui#7146\'s pinned boundary)', [
 { stage: 'Prospecting', amount: 0 },
 { stage: 'Proposal', amount: 7 },
 { stage: 'Won', amount: 0 },
 ], 1, 3],
 ['null measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: null },
 { stage: 'Credits', amount: null },
 ], 1, 3],
 ['unparseable measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: 'n/a' },
 ], 1, 2],
 ['a row missing the measure key entirely', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds' },
 ], 1, 2],
 ['a mix that keeps more than one row', [
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 2, 3],
];

describe('AdvancedChartImpl — sankey that drew only SOME of its rows (objectui#7148)', () => {
 it.each(THINNED)('says how many rows it drew when handed %s', (_label, rows, kept, total) => {
 const { container } = renderSankey(rows);

 const note = noteOf(container);
 expect(note, 'a partial flow states that it is partial').not.toBeNull();

 // BOTH halves of the count. "Some rows were dropped" would still leave a
 // thinned flow indistinguishable from a complete one; the ratio is the bit
 // the picture cannot carry.
 expect(note?.textContent).toContain(`Showing ${kept} of ${total} rows`);
 // Names the measure it tested and the exact test it failed, so the author
 // knows WHICH column decided it — the same diagnosis the refusal gives.
 expect(note?.textContent).toContain('amount');
 expect(note?.textContent).toContain('above zero');
 // A note annotates; it is not a state change and not an alert.
 expect(note?.getAttribute('role')).toBe('note');

 // ⛔ The chart is still DRAWN. The drop is not the defect — a flow has no
 // negative width — so nothing here may replace a drawable sankey with
 // prose. This is objectui#7146's boundary restated from the other side.
 expect(container.querySelector('svg'), 'a drawable sankey still draws').not.toBeNull();
 expect(refusalOf(container), 'a drawn chart is never also a refusal').toBeNull();
 });

 it('agrees with itself in the singular', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ]);
 expect(noteOf(container)?.textContent).toContain('1 row has no');
 });

 it('agrees with itself in the plural', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ]);
 expect(noteOf(container)?.textContent).toContain('2 rows have no');
 });

 it('CONTROL — an all-positive sankey carries no note, and gains no wrapper', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Renewal', amount: 12 },
 ]);

 expect(noteOf(container), 'nothing was omitted, so nothing is claimed').toBeNull();
 expect(container.querySelector('svg')).not.toBeNull();
 // The complete-flow path returns the container it always returned. The
 // footnote's flex wrapper would take over the height chain (see
 // `ChartFootnote`), so a chart with nothing to say must not get one.
 expect(
 container.firstElementChild?.getAttribute('data-slot'),
 'the chart container is still the root element',
 ).toBe('chart');
 });

 // ---- the seam with objectui#7146 -------------------------------------
 //
 // `rows.length` is the whole boundary: 0 survivors is that card, 1-or-more
 // survivors alongside a casualty is this one, and no casualties at all is
 // neither. Pinned from both sides so neither answer can drift into the
 // other's branch.

 it('SEAM — no row survives: objectui#7146 refuses, and this note stays out of it', () => {
 const { container } = renderSankey([
 { stage: 'A', amount: 0 },
 { stage: 'B', amount: 0 },
 { stage: 'C', amount: -5 },
 ]);

 expect(refusalOf(container), 'all-non-positive is still the refusal branch').not.toBeNull();
 expect(noteOf(container), 'a chart that drew nothing has no partial draw to annotate').toBeNull();
 expect(container.querySelector('svg')).toBeNull();
 });

 it('SEAM — no rows at all: still the bare div, untouched by both cards', () => {
 const { container } = renderSankey([]);

 expect(refusalOf(container)).toBeNull();
 expect(noteOf(container)).toBeNull();
 // The empty-RESULT question (objectui#7130), answered upstream in
 // ObjectChart where the query outcome is known.
 expect(container.textContent).toBe('');
 });

 it('does not reach other chart families handed the same mixed-sign rows', () => {
 // The note reads the sankey filter's own result and no other family has
 // such a filter — measured in Chromium, bar/pie/donut/funnel/treemap all
 // keep every row of this dataset in their data array. Pinned because
 // hoisting the count out of this arm is the obvious refactor and would
 // annotate four charts that omitted nothing.
 for (const chartType of ['bar', 'pie', 'donut', 'funnel', 'treemap'] as const) {
 const { container } = render(
 <AdvancedChartImpl
 chartType={chartType}
 xAxisKey="stage"
 series={SERIES as any}
 data={[
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ] as any}
 />,
 );
 expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull();
 cleanup();
 }
 });
});

122 changes: 111 additions & 11 deletions
packages/plugin-charts/src/AdvancedChartImpl.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -373,6 +373,40 @@ function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?:
 );
}

/**
 * `ChartFrame`'s mirror image: chrome BELOW the plot, for a chart that drew
 * something but did not draw all of it (objectui#7148).
 *
 * Same construction, and deliberately so, because the construction is the whole
 * difficulty. A footnote cannot simply be rendered as a SIBLING of
 * `ChartContainer`: measured in Chromium in the dashboard shape — a fixed,
 * clipping card whose chart carries `h-full` — the container takes the card's
 * full height and a following `<p>` lands at y=327 in a box that ends at y=316,
 * i.e. entirely outside the clip. A note invisible in dashboards is no note at
 * all, and dashboards are where these charts live.
 *
 * Nor can it be a plain wrapper `<div className={className}>` around the
 * container: the consumer's className is the chart's height contract and it has
 * to keep reaching the element Recharts measures (see `CHART_MIN_HEIGHT` in
 * `ChartContainerImpl` for the measurement of what happens when it does not —
 * a permanently zero box and an invisible chart, with no refusal and no empty
 * state).
 *
 * So the plot keeps its own `className` and gains a definite height through the
 * flex chain instead: `h-full` outer, `min-h-0 flex-1` around the plot,
 * `shrink-0` under it. With no footnote this returns the chart untouched, so no
 * existing caller gains a wrapper element — the same gate `ChartFrame` uses.
 */
function ChartFootnote({ note, children }: { note?: React.ReactNode; children: React.ReactNode }) {
 if (!note) return <>{children}</>;
 return (
 <div className="flex h-full w-full flex-col">
 <div className="min-h-0 flex-1">{children}</div>
 <div className="mt-1 shrink-0">{note}</div>
 </div>
 );
}

/**
 * AdvancedChartImpl - The heavy implementation that imports Recharts with full features
 * This component is lazy-loaded to avoid including Recharts in the initial bundle
Expand DownExpand Up@@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({
 </ChartRefusal>
 );
 }
 // A PARTIAL flow SAYS it is partial — objectui#7148, the branch next door
 // to the refusal above.
 //
 // The filter is unconditional, so a dataset where only SOME rows survive it
 // draws a normal, healthy, confident sankey of a fraction of itself, and
 // nothing in the output carried that fact. Measured in Chromium at
 // origin/main fd11e1644 across 27 tiles: the card's own dataset
 // (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 // `path: 3`, 18 descendants, no `role`, no text, and — against a live
 // console control that did fire on the same instrument — ZERO console
 // output. Its screenshot hashed `13237e6e19a7072a`, BYTE-IDENTICAL to a
 // genuinely one-row dataset `[{ New business: 40 }]` and to three other
 // thinned shapes (one positive among zeros, positive + nulls, positive +
 // unparseable). Six datasets, one image. A reader had no bit of information
 // distinguishing a complete flow from a third of one, and nobody re-reads a
 // dataset that renders fine.
 //
 // ## Why a note beside the chart, and not a refusal
 //
 // Not a style preference — a refusal is unavailable here. objectui#7146
 // pins "one positive row among zeros still draws", and that fixture
 // (`0 / 7 / 0`) is ITSELF a thinned dataset: it lands in this branch, and
 // hashes identical to the mixed-sign tile above. Refusing on a thinned flow
 // would blank the chart that pin requires drawn.
 //
 // The drop is also not the defect. A flow has no negative width, so
 // discarding those rows is the only thing this arm CAN do with them. What
 // was missing was saying so — which is the whole change: the plot is the
 // element this arm already returned, unchanged, with one line of prose
 // under it.
 //
 // ## Why the copy names the PREDICATE and a COUNT, not a cause
 //
 // The reason the refusal above gives, and this branch is where that family
 // actually lives: `Number(…) || 0` folds negatives, zeros, `null`,
 // unparseable strings and a missing key into ONE discard, and all five were
 // measured reaching here beside a survivor. Naming any one of them is a
 // sentence that is false for the other four, so the copy names the
 // predicate the filter actually applies, which is true of all of them.
 //
 // The COUNT is the half a reader cannot recover from the picture. "Some
 // rows were dropped" still leaves a thinned flow indistinguishable from a
 // complete one; `1 of 3` is the bit that was missing.
 //
 // No console warning, matching the refusal above and unlike the two guards
 // at the bottom of this file: those carry a diagnostic PAIR that does not
 // fit on screen, whereas this sentence already names the key, the test it
 // failed, and how many rows failed it.
 const omittedRowCount = data.length - rows.length;
 return (
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 <ChartFootnote
 note={
 omittedRowCount > 0 ? (
 <p
 role="note"
 data-chart-note="omitted-rows"
 className="px-1 text-xs text-muted-foreground"
 >
 Showing {rows.length} of {data.length} rows &mdash; {omittedRowCount}{' '}
 {omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '}
 <code className="font-mono">{dataKey}</code> above zero, which a flow cannot
 draw.
 </p>
 ) : null
 }
 >
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 </ChartFootnote>
 );
 }

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(plugin-charts): a sankey that drew only some of its rows says how many#7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-warren
merged 2 commits into

from

Sep 1, 2026
 Merged


fix(plugin-charts): a sankey that drew only some of its rows says how many
#7161
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
8d0af30

fix(plugin-charts): a sankey that drew only some of its rows says how…


claude Sep 1, 2026
f2df891

chore(changeset): sankey partial-flow footnote


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


34 changes: 34 additions & 0 deletions
.changeset/7148-sankey-omitted-rows-note.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
'@object-ui/plugin-charts': patch
---

A sankey that drew only SOME of its rows now says how many (objectui#7148).

The sankey arm keeps strictly positive measures
(`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a mixed dataset
drew a normal, healthy, confident chart of a fraction of itself and nothing
anywhere recorded that the other rows existed. Measured in Chromium across 27
tiles: `[{New business: 40}, {Refunds: -25}, {Chargebacks: -12}]` rendered
`svg: 1`, `path: 3`, 18 descendants, no `role`, no text, and — against a live
console control that did fire on the same instrument — zero console output.
Its screenshot hashed byte-identical to five other datasets, one of which
genuinely had a single row. Six datasets, one image: a reader had no bit of
information separating a complete flow from a third of one.

The discard itself stands — a flow has no negative width, so it is the only
thing that arm can do with those rows. What is added is a footnote under the
plot naming the ratio and the predicate the filter applies:

> Showing 1 of 3 rows — 2 rows have no `amount` above zero, which a flow
> cannot draw.

It names the predicate rather than a cause because `Number(…) || 0` folds
negatives, zeros, `null`, unparseable strings and a missing key into one
discard, and all five were measured reaching this branch beside a survivor;
naming any one of them is a sentence that is false for the other four.

A complete flow is byte-for-byte unchanged and gains no wrapper element, and a
drawable sankey is never replaced by prose: the `no-positive-flow` refusal
still owns the case where NOTHING survives the filter, and the "one positive
among zeros still draws" boundary still draws — that fixture is itself a
thinned dataset, so it now draws *and* says so.

222 changes: 222 additions & 0 deletions
packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
/**
 * ObjectUI
 * Copyright (c) 2024-present ObjectStack Inc.
 *
 * Sankey — SOME rows survive the positive filter and the rest are dropped
 * silently (objectui#7148). The branch next door to objectui#7140's refusal,
 * and the more dangerous of the two.
 *
 * The sankey arm keeps only strictly positive measures
 * (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`). When that filter
 * keeps NOTHING, objectui#7146 now says so. When it keeps SOME, the chart drew
 * a normal, healthy, confident sankey of a fraction of its dataset and nothing
 * anywhere recorded that the other rows existed.
 *
 * ## The measurement that decided fix-over-decline
 *
 * 27 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main`
 * fd11e1644, each tile screenshotted and SHA-256'd. The card's own dataset
 * (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 * `path: 3`, 18 descendants, no `role`, no text — and, against a live console
 * control that DID fire on the same instrument (`missing-category-key`), zero
 * console output. Its screenshot hashed `13237e6e19a7072a`, byte-identical to
 * FIVE other tiles including a genuinely one-row dataset
 * `[{ stage: 'New business', amount: 40 }]`. Six datasets, one image: the
 * reader had no bit of information separating a complete flow from a third of
 * one. That is why "discarding is all a flow CAN do" does not settle the card —
 * the drop is fine, the silence is not.
 *
 * ## Why a note and not a refusal
 *
 * objectui#7146 pins "one positive row among zeros still draws". That fixture
 * (`0 / 7 / 0`) is itself a THINNED dataset — it lands in this branch and
 * hashed identical to the mixed-sign tile — so a refusal here would blank a
 * chart that pin requires drawn. The two cards meet exactly at
 * `rows.length === 0`: none survive → refusal (objectui#7146); some survive and
 * some do not → this note; all survive → untouched.
 */
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
 const actual = await vi.importActual<any>('recharts');
 return {
 ...actual,
 ResponsiveContainer: ({ children }: any) =>
 React.cloneElement(children, { width: 480, height: 320 }),
 };
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
 render(
 <AdvancedChartImpl
 chartType="sankey"
 xAxisKey="stage"
 series={SERIES as any}
 data={data as any}
 />,
 );

const noteOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-note="omitted-rows"]');
const refusalOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-error="no-positive-flow"]');

/**
 * Every shape `Number(…) || 0` folds to zero, each one BESIDE a survivor.
 *
 * They are not the same situation and the copy deliberately names none of them:
 * a message that said "negative" would be false of the null row, and the card
 * itself notes that `null` and unparseable measures vanish through the very
 * same filter. All five were measured reaching this branch in Chromium.
 */
const THINNED: Array<[string, Array<Record<string, unknown>>, number, number]> = [
 ['negative rows (the card\'s own dataset)', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 1, 3],
 ['zero rows (objectui#7146\'s pinned boundary)', [
 { stage: 'Prospecting', amount: 0 },
 { stage: 'Proposal', amount: 7 },
 { stage: 'Won', amount: 0 },
 ], 1, 3],
 ['null measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: null },
 { stage: 'Credits', amount: null },
 ], 1, 3],
 ['unparseable measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: 'n/a' },
 ], 1, 2],
 ['a row missing the measure key entirely', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds' },
 ], 1, 2],
 ['a mix that keeps more than one row', [
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 2, 3],
];

describe('AdvancedChartImpl — sankey that drew only SOME of its rows (objectui#7148)', () => {
 it.each(THINNED)('says how many rows it drew when handed %s', (_label, rows, kept, total) => {
 const { container } = renderSankey(rows);

 const note = noteOf(container);
 expect(note, 'a partial flow states that it is partial').not.toBeNull();

 // BOTH halves of the count. "Some rows were dropped" would still leave a
 // thinned flow indistinguishable from a complete one; the ratio is the bit
 // the picture cannot carry.
 expect(note?.textContent).toContain(`Showing ${kept} of ${total} rows`);
 // Names the measure it tested and the exact test it failed, so the author
 // knows WHICH column decided it — the same diagnosis the refusal gives.
 expect(note?.textContent).toContain('amount');
 expect(note?.textContent).toContain('above zero');
 // A note annotates; it is not a state change and not an alert.
 expect(note?.getAttribute('role')).toBe('note');

 // ⛔ The chart is still DRAWN. The drop is not the defect — a flow has no
 // negative width — so nothing here may replace a drawable sankey with
 // prose. This is objectui#7146's boundary restated from the other side.
 expect(container.querySelector('svg'), 'a drawable sankey still draws').not.toBeNull();
 expect(refusalOf(container), 'a drawn chart is never also a refusal').toBeNull();
 });

 it('agrees with itself in the singular', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ]);
 expect(noteOf(container)?.textContent).toContain('1 row has no');
 });

 it('agrees with itself in the plural', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ]);
 expect(noteOf(container)?.textContent).toContain('2 rows have no');
 });

 it('CONTROL — an all-positive sankey carries no note, and gains no wrapper', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Renewal', amount: 12 },
 ]);

 expect(noteOf(container), 'nothing was omitted, so nothing is claimed').toBeNull();
 expect(container.querySelector('svg')).not.toBeNull();
 // The complete-flow path returns the container it always returned. The
 // footnote's flex wrapper would take over the height chain (see
 // `ChartFootnote`), so a chart with nothing to say must not get one.
 expect(
 container.firstElementChild?.getAttribute('data-slot'),
 'the chart container is still the root element',
 ).toBe('chart');
 });

 // ---- the seam with objectui#7146 -------------------------------------
 //
 // `rows.length` is the whole boundary: 0 survivors is that card, 1-or-more
 // survivors alongside a casualty is this one, and no casualties at all is
 // neither. Pinned from both sides so neither answer can drift into the
 // other's branch.

 it('SEAM — no row survives: objectui#7146 refuses, and this note stays out of it', () => {
 const { container } = renderSankey([
 { stage: 'A', amount: 0 },
 { stage: 'B', amount: 0 },
 { stage: 'C', amount: -5 },
 ]);

 expect(refusalOf(container), 'all-non-positive is still the refusal branch').not.toBeNull();
 expect(noteOf(container), 'a chart that drew nothing has no partial draw to annotate').toBeNull();
 expect(container.querySelector('svg')).toBeNull();
 });

 it('SEAM — no rows at all: still the bare div, untouched by both cards', () => {
 const { container } = renderSankey([]);

 expect(refusalOf(container)).toBeNull();
 expect(noteOf(container)).toBeNull();
 // The empty-RESULT question (objectui#7130), answered upstream in
 // ObjectChart where the query outcome is known.
 expect(container.textContent).toBe('');
 });

 it('does not reach other chart families handed the same mixed-sign rows', () => {
 // The note reads the sankey filter's own result and no other family has
 // such a filter — measured in Chromium, bar/pie/donut/funnel/treemap all
 // keep every row of this dataset in their data array. Pinned because
 // hoisting the count out of this arm is the obvious refactor and would
 // annotate four charts that omitted nothing.
 for (const chartType of ['bar', 'pie', 'donut', 'funnel', 'treemap'] as const) {
 const { container } = render(
 <AdvancedChartImpl
 chartType={chartType}
 xAxisKey="stage"
 series={SERIES as any}
 data={[
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ] as any}
 />,
 );
 expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull();
 cleanup();
 }
 });
});

122 changes: 111 additions & 11 deletions
packages/plugin-charts/src/AdvancedChartImpl.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -373,6 +373,40 @@ function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?:
 );
}

/**
 * `ChartFrame`'s mirror image: chrome BELOW the plot, for a chart that drew
 * something but did not draw all of it (objectui#7148).
 *
 * Same construction, and deliberately so, because the construction is the whole
 * difficulty. A footnote cannot simply be rendered as a SIBLING of
 * `ChartContainer`: measured in Chromium in the dashboard shape — a fixed,
 * clipping card whose chart carries `h-full` — the container takes the card's
 * full height and a following `<p>` lands at y=327 in a box that ends at y=316,
 * i.e. entirely outside the clip. A note invisible in dashboards is no note at
 * all, and dashboards are where these charts live.
 *
 * Nor can it be a plain wrapper `<div className={className}>` around the
 * container: the consumer's className is the chart's height contract and it has
 * to keep reaching the element Recharts measures (see `CHART_MIN_HEIGHT` in
 * `ChartContainerImpl` for the measurement of what happens when it does not —
 * a permanently zero box and an invisible chart, with no refusal and no empty
 * state).
 *
 * So the plot keeps its own `className` and gains a definite height through the
 * flex chain instead: `h-full` outer, `min-h-0 flex-1` around the plot,
 * `shrink-0` under it. With no footnote this returns the chart untouched, so no
 * existing caller gains a wrapper element — the same gate `ChartFrame` uses.
 */
function ChartFootnote({ note, children }: { note?: React.ReactNode; children: React.ReactNode }) {
 if (!note) return <>{children}</>;
 return (
 <div className="flex h-full w-full flex-col">
 <div className="min-h-0 flex-1">{children}</div>
 <div className="mt-1 shrink-0">{note}</div>
 </div>
 );
}

/**
 * AdvancedChartImpl - The heavy implementation that imports Recharts with full features
 * This component is lazy-loaded to avoid including Recharts in the initial bundle
Expand DownExpand Up@@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({
 </ChartRefusal>
 );
 }
 // A PARTIAL flow SAYS it is partial — objectui#7148, the branch next door
 // to the refusal above.
 //
 // The filter is unconditional, so a dataset where only SOME rows survive it
 // draws a normal, healthy, confident sankey of a fraction of itself, and
 // nothing in the output carried that fact. Measured in Chromium at
 // origin/main fd11e1644 across 27 tiles: the card's own dataset
 // (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 // `path: 3`, 18 descendants, no `role`, no text, and — against a live
 // console control that did fire on the same instrument — ZERO console
 // output. Its screenshot hashed `13237e6e19a7072a`, BYTE-IDENTICAL to a
 // genuinely one-row dataset `[{ New business: 40 }]` and to three other
 // thinned shapes (one positive among zeros, positive + nulls, positive +
 // unparseable). Six datasets, one image. A reader had no bit of information
 // distinguishing a complete flow from a third of one, and nobody re-reads a
 // dataset that renders fine.
 //
 // ## Why a note beside the chart, and not a refusal
 //
 // Not a style preference — a refusal is unavailable here. objectui#7146
 // pins "one positive row among zeros still draws", and that fixture
 // (`0 / 7 / 0`) is ITSELF a thinned dataset: it lands in this branch, and
 // hashes identical to the mixed-sign tile above. Refusing on a thinned flow
 // would blank the chart that pin requires drawn.
 //
 // The drop is also not the defect. A flow has no negative width, so
 // discarding those rows is the only thing this arm CAN do with them. What
 // was missing was saying so — which is the whole change: the plot is the
 // element this arm already returned, unchanged, with one line of prose
 // under it.
 //
 // ## Why the copy names the PREDICATE and a COUNT, not a cause
 //
 // The reason the refusal above gives, and this branch is where that family
 // actually lives: `Number(…) || 0` folds negatives, zeros, `null`,
 // unparseable strings and a missing key into ONE discard, and all five were
 // measured reaching here beside a survivor. Naming any one of them is a
 // sentence that is false for the other four, so the copy names the
 // predicate the filter actually applies, which is true of all of them.
 //
 // The COUNT is the half a reader cannot recover from the picture. "Some
 // rows were dropped" still leaves a thinned flow indistinguishable from a
 // complete one; `1 of 3` is the bit that was missing.
 //
 // No console warning, matching the refusal above and unlike the two guards
 // at the bottom of this file: those carry a diagnostic PAIR that does not
 // fit on screen, whereas this sentence already names the key, the test it
 // failed, and how many rows failed it.
 const omittedRowCount = data.length - rows.length;
 return (
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 <ChartFootnote
 note={
 omittedRowCount > 0 ? (
 <p
 role="note"
 data-chart-note="omitted-rows"
 className="px-1 text-xs text-muted-foreground"
 >
 Showing {rows.length} of {data.length} rows &mdash; {omittedRowCount}{' '}
 {omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '}
 <code className="font-mono">{dataKey}</code> above zero, which a flow cannot
 draw.
 </p>
 ) : null
 }
 >
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 </ChartFootnote>
 );
 }

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix(plugin-charts): a sankey that drew only some of its rows says how many#7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-warren
merged 2 commits into

from

Sep 1, 2026
 Merged


fix(plugin-charts): a sankey that drew only some of its rows says how many
#7161
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
8d0af30

fix(plugin-charts): a sankey that drew only some of its rows says how…


claude Sep 1, 2026
f2df891

chore(changeset): sankey partial-flow footnote


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


34 changes: 34 additions & 0 deletions
.changeset/7148-sankey-omitted-rows-note.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
'@object-ui/plugin-charts': patch
---

A sankey that drew only SOME of its rows now says how many (objectui#7148).

The sankey arm keeps strictly positive measures
(`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a mixed dataset
drew a normal, healthy, confident chart of a fraction of itself and nothing
anywhere recorded that the other rows existed. Measured in Chromium across 27
tiles: `[{New business: 40}, {Refunds: -25}, {Chargebacks: -12}]` rendered
`svg: 1`, `path: 3`, 18 descendants, no `role`, no text, and — against a live
console control that did fire on the same instrument — zero console output.
Its screenshot hashed byte-identical to five other datasets, one of which
genuinely had a single row. Six datasets, one image: a reader had no bit of
information separating a complete flow from a third of one.

The discard itself stands — a flow has no negative width, so it is the only
thing that arm can do with those rows. What is added is a footnote under the
plot naming the ratio and the predicate the filter applies:

> Showing 1 of 3 rows — 2 rows have no `amount` above zero, which a flow
> cannot draw.

It names the predicate rather than a cause because `Number(…) || 0` folds
negatives, zeros, `null`, unparseable strings and a missing key into one
discard, and all five were measured reaching this branch beside a survivor;
naming any one of them is a sentence that is false for the other four.

A complete flow is byte-for-byte unchanged and gains no wrapper element, and a
drawable sankey is never replaced by prose: the `no-positive-flow` refusal
still owns the case where NOTHING survives the filter, and the "one positive
among zeros still draws" boundary still draws — that fixture is itself a
thinned dataset, so it now draws *and* says so.

222 changes: 222 additions & 0 deletions
packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
/**
 * ObjectUI
 * Copyright (c) 2024-present ObjectStack Inc.
 *
 * Sankey — SOME rows survive the positive filter and the rest are dropped
 * silently (objectui#7148). The branch next door to objectui#7140's refusal,
 * and the more dangerous of the two.
 *
 * The sankey arm keeps only strictly positive measures
 * (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`). When that filter
 * keeps NOTHING, objectui#7146 now says so. When it keeps SOME, the chart drew
 * a normal, healthy, confident sankey of a fraction of its dataset and nothing
 * anywhere recorded that the other rows existed.
 *
 * ## The measurement that decided fix-over-decline
 *
 * 27 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main`
 * fd11e1644, each tile screenshotted and SHA-256'd. The card's own dataset
 * (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 * `path: 3`, 18 descendants, no `role`, no text — and, against a live console
 * control that DID fire on the same instrument (`missing-category-key`), zero
 * console output. Its screenshot hashed `13237e6e19a7072a`, byte-identical to
 * FIVE other tiles including a genuinely one-row dataset
 * `[{ stage: 'New business', amount: 40 }]`. Six datasets, one image: the
 * reader had no bit of information separating a complete flow from a third of
 * one. That is why "discarding is all a flow CAN do" does not settle the card —
 * the drop is fine, the silence is not.
 *
 * ## Why a note and not a refusal
 *
 * objectui#7146 pins "one positive row among zeros still draws". That fixture
 * (`0 / 7 / 0`) is itself a THINNED dataset — it lands in this branch and
 * hashed identical to the mixed-sign tile — so a refusal here would blank a
 * chart that pin requires drawn. The two cards meet exactly at
 * `rows.length === 0`: none survive → refusal (objectui#7146); some survive and
 * some do not → this note; all survive → untouched.
 */
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
 const actual = await vi.importActual<any>('recharts');
 return {
 ...actual,
 ResponsiveContainer: ({ children }: any) =>
 React.cloneElement(children, { width: 480, height: 320 }),
 };
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
 render(
 <AdvancedChartImpl
 chartType="sankey"
 xAxisKey="stage"
 series={SERIES as any}
 data={data as any}
 />,
 );

const noteOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-note="omitted-rows"]');
const refusalOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-error="no-positive-flow"]');

/**
 * Every shape `Number(…) || 0` folds to zero, each one BESIDE a survivor.
 *
 * They are not the same situation and the copy deliberately names none of them:
 * a message that said "negative" would be false of the null row, and the card
 * itself notes that `null` and unparseable measures vanish through the very
 * same filter. All five were measured reaching this branch in Chromium.
 */
const THINNED: Array<[string, Array<Record<string, unknown>>, number, number]> = [
 ['negative rows (the card\'s own dataset)', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 1, 3],
 ['zero rows (objectui#7146\'s pinned boundary)', [
 { stage: 'Prospecting', amount: 0 },
 { stage: 'Proposal', amount: 7 },
 { stage: 'Won', amount: 0 },
 ], 1, 3],
 ['null measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: null },
 { stage: 'Credits', amount: null },
 ], 1, 3],
 ['unparseable measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: 'n/a' },
 ], 1, 2],
 ['a row missing the measure key entirely', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds' },
 ], 1, 2],
 ['a mix that keeps more than one row', [
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 2, 3],
];

describe('AdvancedChartImpl — sankey that drew only SOME of its rows (objectui#7148)', () => {
 it.each(THINNED)('says how many rows it drew when handed %s', (_label, rows, kept, total) => {
 const { container } = renderSankey(rows);

 const note = noteOf(container);
 expect(note, 'a partial flow states that it is partial').not.toBeNull();

 // BOTH halves of the count. "Some rows were dropped" would still leave a
 // thinned flow indistinguishable from a complete one; the ratio is the bit
 // the picture cannot carry.
 expect(note?.textContent).toContain(`Showing ${kept} of ${total} rows`);
 // Names the measure it tested and the exact test it failed, so the author
 // knows WHICH column decided it — the same diagnosis the refusal gives.
 expect(note?.textContent).toContain('amount');
 expect(note?.textContent).toContain('above zero');
 // A note annotates; it is not a state change and not an alert.
 expect(note?.getAttribute('role')).toBe('note');

 // ⛔ The chart is still DRAWN. The drop is not the defect — a flow has no
 // negative width — so nothing here may replace a drawable sankey with
 // prose. This is objectui#7146's boundary restated from the other side.
 expect(container.querySelector('svg'), 'a drawable sankey still draws').not.toBeNull();
 expect(refusalOf(container), 'a drawn chart is never also a refusal').toBeNull();
 });

 it('agrees with itself in the singular', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ]);
 expect(noteOf(container)?.textContent).toContain('1 row has no');
 });

 it('agrees with itself in the plural', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ]);
 expect(noteOf(container)?.textContent).toContain('2 rows have no');
 });

 it('CONTROL — an all-positive sankey carries no note, and gains no wrapper', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Renewal', amount: 12 },
 ]);

 expect(noteOf(container), 'nothing was omitted, so nothing is claimed').toBeNull();
 expect(container.querySelector('svg')).not.toBeNull();
 // The complete-flow path returns the container it always returned. The
 // footnote's flex wrapper would take over the height chain (see
 // `ChartFootnote`), so a chart with nothing to say must not get one.
 expect(
 container.firstElementChild?.getAttribute('data-slot'),
 'the chart container is still the root element',
 ).toBe('chart');
 });

 // ---- the seam with objectui#7146 -------------------------------------
 //
 // `rows.length` is the whole boundary: 0 survivors is that card, 1-or-more
 // survivors alongside a casualty is this one, and no casualties at all is
 // neither. Pinned from both sides so neither answer can drift into the
 // other's branch.

 it('SEAM — no row survives: objectui#7146 refuses, and this note stays out of it', () => {
 const { container } = renderSankey([
 { stage: 'A', amount: 0 },
 { stage: 'B', amount: 0 },
 { stage: 'C', amount: -5 },
 ]);

 expect(refusalOf(container), 'all-non-positive is still the refusal branch').not.toBeNull();
 expect(noteOf(container), 'a chart that drew nothing has no partial draw to annotate').toBeNull();
 expect(container.querySelector('svg')).toBeNull();
 });

 it('SEAM — no rows at all: still the bare div, untouched by both cards', () => {
 const { container } = renderSankey([]);

 expect(refusalOf(container)).toBeNull();
 expect(noteOf(container)).toBeNull();
 // The empty-RESULT question (objectui#7130), answered upstream in
 // ObjectChart where the query outcome is known.
 expect(container.textContent).toBe('');
 });

 it('does not reach other chart families handed the same mixed-sign rows', () => {
 // The note reads the sankey filter's own result and no other family has
 // such a filter — measured in Chromium, bar/pie/donut/funnel/treemap all
 // keep every row of this dataset in their data array. Pinned because
 // hoisting the count out of this arm is the obvious refactor and would
 // annotate four charts that omitted nothing.
 for (const chartType of ['bar', 'pie', 'donut', 'funnel', 'treemap'] as const) {
 const { container } = render(
 <AdvancedChartImpl
 chartType={chartType}
 xAxisKey="stage"
 series={SERIES as any}
 data={[
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ] as any}
 />,
 );
 expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull();
 cleanup();
 }
 });
});

122 changes: 111 additions & 11 deletions
packages/plugin-charts/src/AdvancedChartImpl.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -373,6 +373,40 @@ function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?:
 );
}

/**
 * `ChartFrame`'s mirror image: chrome BELOW the plot, for a chart that drew
 * something but did not draw all of it (objectui#7148).
 *
 * Same construction, and deliberately so, because the construction is the whole
 * difficulty. A footnote cannot simply be rendered as a SIBLING of
 * `ChartContainer`: measured in Chromium in the dashboard shape — a fixed,
 * clipping card whose chart carries `h-full` — the container takes the card's
 * full height and a following `<p>` lands at y=327 in a box that ends at y=316,
 * i.e. entirely outside the clip. A note invisible in dashboards is no note at
 * all, and dashboards are where these charts live.
 *
 * Nor can it be a plain wrapper `<div className={className}>` around the
 * container: the consumer's className is the chart's height contract and it has
 * to keep reaching the element Recharts measures (see `CHART_MIN_HEIGHT` in
 * `ChartContainerImpl` for the measurement of what happens when it does not —
 * a permanently zero box and an invisible chart, with no refusal and no empty
 * state).
 *
 * So the plot keeps its own `className` and gains a definite height through the
 * flex chain instead: `h-full` outer, `min-h-0 flex-1` around the plot,
 * `shrink-0` under it. With no footnote this returns the chart untouched, so no
 * existing caller gains a wrapper element — the same gate `ChartFrame` uses.
 */
function ChartFootnote({ note, children }: { note?: React.ReactNode; children: React.ReactNode }) {
 if (!note) return <>{children}</>;
 return (
 <div className="flex h-full w-full flex-col">
 <div className="min-h-0 flex-1">{children}</div>
 <div className="mt-1 shrink-0">{note}</div>
 </div>
 );
}

/**
 * AdvancedChartImpl - The heavy implementation that imports Recharts with full features
 * This component is lazy-loaded to avoid including Recharts in the initial bundle
Expand DownExpand Up@@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({
 </ChartRefusal>
 );
 }
 // A PARTIAL flow SAYS it is partial — objectui#7148, the branch next door
 // to the refusal above.
 //
 // The filter is unconditional, so a dataset where only SOME rows survive it
 // draws a normal, healthy, confident sankey of a fraction of itself, and
 // nothing in the output carried that fact. Measured in Chromium at
 // origin/main fd11e1644 across 27 tiles: the card's own dataset
 // (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 // `path: 3`, 18 descendants, no `role`, no text, and — against a live
 // console control that did fire on the same instrument — ZERO console
 // output. Its screenshot hashed `13237e6e19a7072a`, BYTE-IDENTICAL to a
 // genuinely one-row dataset `[{ New business: 40 }]` and to three other
 // thinned shapes (one positive among zeros, positive + nulls, positive +
 // unparseable). Six datasets, one image. A reader had no bit of information
 // distinguishing a complete flow from a third of one, and nobody re-reads a
 // dataset that renders fine.
 //
 // ## Why a note beside the chart, and not a refusal
 //
 // Not a style preference — a refusal is unavailable here. objectui#7146
 // pins "one positive row among zeros still draws", and that fixture
 // (`0 / 7 / 0`) is ITSELF a thinned dataset: it lands in this branch, and
 // hashes identical to the mixed-sign tile above. Refusing on a thinned flow
 // would blank the chart that pin requires drawn.
 //
 // The drop is also not the defect. A flow has no negative width, so
 // discarding those rows is the only thing this arm CAN do with them. What
 // was missing was saying so — which is the whole change: the plot is the
 // element this arm already returned, unchanged, with one line of prose
 // under it.
 //
 // ## Why the copy names the PREDICATE and a COUNT, not a cause
 //
 // The reason the refusal above gives, and this branch is where that family
 // actually lives: `Number(…) || 0` folds negatives, zeros, `null`,
 // unparseable strings and a missing key into ONE discard, and all five were
 // measured reaching here beside a survivor. Naming any one of them is a
 // sentence that is false for the other four, so the copy names the
 // predicate the filter actually applies, which is true of all of them.
 //
 // The COUNT is the half a reader cannot recover from the picture. "Some
 // rows were dropped" still leaves a thinned flow indistinguishable from a
 // complete one; `1 of 3` is the bit that was missing.
 //
 // No console warning, matching the refusal above and unlike the two guards
 // at the bottom of this file: those carry a diagnostic PAIR that does not
 // fit on screen, whereas this sentence already names the key, the test it
 // failed, and how many rows failed it.
 const omittedRowCount = data.length - rows.length;
 return (
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 <ChartFootnote
 note={
 omittedRowCount > 0 ? (
 <p
 role="note"
 data-chart-note="omitted-rows"
 className="px-1 text-xs text-muted-foreground"
 >
 Showing {rows.length} of {data.length} rows &mdash; {omittedRowCount}{' '}
 {omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '}
 <code className="font-mono">{dataKey}</code> above zero, which a flow cannot
 draw.
 </p>
 ) : null
 }
 >
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 </ChartFootnote>
 );
 }

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(plugin-charts): a sankey that drew only some of its rows says how many#7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-warren
merged 2 commits into

from

Sep 1, 2026
 Merged


fix(plugin-charts): a sankey that drew only some of its rows says how many
#7161
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
8d0af30

fix(plugin-charts): a sankey that drew only some of its rows says how…


claude Sep 1, 2026
f2df891

chore(changeset): sankey partial-flow footnote


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


34 changes: 34 additions & 0 deletions
.changeset/7148-sankey-omitted-rows-note.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
'@object-ui/plugin-charts': patch
---

A sankey that drew only SOME of its rows now says how many (objectui#7148).

The sankey arm keeps strictly positive measures
(`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a mixed dataset
drew a normal, healthy, confident chart of a fraction of itself and nothing
anywhere recorded that the other rows existed. Measured in Chromium across 27
tiles: `[{New business: 40}, {Refunds: -25}, {Chargebacks: -12}]` rendered
`svg: 1`, `path: 3`, 18 descendants, no `role`, no text, and — against a live
console control that did fire on the same instrument — zero console output.
Its screenshot hashed byte-identical to five other datasets, one of which
genuinely had a single row. Six datasets, one image: a reader had no bit of
information separating a complete flow from a third of one.

The discard itself stands — a flow has no negative width, so it is the only
thing that arm can do with those rows. What is added is a footnote under the
plot naming the ratio and the predicate the filter applies:

> Showing 1 of 3 rows — 2 rows have no `amount` above zero, which a flow
> cannot draw.

It names the predicate rather than a cause because `Number(…) || 0` folds
negatives, zeros, `null`, unparseable strings and a missing key into one
discard, and all five were measured reaching this branch beside a survivor;
naming any one of them is a sentence that is false for the other four.

A complete flow is byte-for-byte unchanged and gains no wrapper element, and a
drawable sankey is never replaced by prose: the `no-positive-flow` refusal
still owns the case where NOTHING survives the filter, and the "one positive
among zeros still draws" boundary still draws — that fixture is itself a
thinned dataset, so it now draws *and* says so.

222 changes: 222 additions & 0 deletions
packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
/**
 * ObjectUI
 * Copyright (c) 2024-present ObjectStack Inc.
 *
 * Sankey — SOME rows survive the positive filter and the rest are dropped
 * silently (objectui#7148). The branch next door to objectui#7140's refusal,
 * and the more dangerous of the two.
 *
 * The sankey arm keeps only strictly positive measures
 * (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`). When that filter
 * keeps NOTHING, objectui#7146 now says so. When it keeps SOME, the chart drew
 * a normal, healthy, confident sankey of a fraction of its dataset and nothing
 * anywhere recorded that the other rows existed.
 *
 * ## The measurement that decided fix-over-decline
 *
 * 27 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main`
 * fd11e1644, each tile screenshotted and SHA-256'd. The card's own dataset
 * (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 * `path: 3`, 18 descendants, no `role`, no text — and, against a live console
 * control that DID fire on the same instrument (`missing-category-key`), zero
 * console output. Its screenshot hashed `13237e6e19a7072a`, byte-identical to
 * FIVE other tiles including a genuinely one-row dataset
 * `[{ stage: 'New business', amount: 40 }]`. Six datasets, one image: the
 * reader had no bit of information separating a complete flow from a third of
 * one. That is why "discarding is all a flow CAN do" does not settle the card —
 * the drop is fine, the silence is not.
 *
 * ## Why a note and not a refusal
 *
 * objectui#7146 pins "one positive row among zeros still draws". That fixture
 * (`0 / 7 / 0`) is itself a THINNED dataset — it lands in this branch and
 * hashed identical to the mixed-sign tile — so a refusal here would blank a
 * chart that pin requires drawn. The two cards meet exactly at
 * `rows.length === 0`: none survive → refusal (objectui#7146); some survive and
 * some do not → this note; all survive → untouched.
 */
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
 const actual = await vi.importActual<any>('recharts');
 return {
 ...actual,
 ResponsiveContainer: ({ children }: any) =>
 React.cloneElement(children, { width: 480, height: 320 }),
 };
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
 render(
 <AdvancedChartImpl
 chartType="sankey"
 xAxisKey="stage"
 series={SERIES as any}
 data={data as any}
 />,
 );

const noteOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-note="omitted-rows"]');
const refusalOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-error="no-positive-flow"]');

/**
 * Every shape `Number(…) || 0` folds to zero, each one BESIDE a survivor.
 *
 * They are not the same situation and the copy deliberately names none of them:
 * a message that said "negative" would be false of the null row, and the card
 * itself notes that `null` and unparseable measures vanish through the very
 * same filter. All five were measured reaching this branch in Chromium.
 */
const THINNED: Array<[string, Array<Record<string, unknown>>, number, number]> = [
 ['negative rows (the card\'s own dataset)', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 1, 3],
 ['zero rows (objectui#7146\'s pinned boundary)', [
 { stage: 'Prospecting', amount: 0 },
 { stage: 'Proposal', amount: 7 },
 { stage: 'Won', amount: 0 },
 ], 1, 3],
 ['null measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: null },
 { stage: 'Credits', amount: null },
 ], 1, 3],
 ['unparseable measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: 'n/a' },
 ], 1, 2],
 ['a row missing the measure key entirely', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds' },
 ], 1, 2],
 ['a mix that keeps more than one row', [
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 2, 3],
];

describe('AdvancedChartImpl — sankey that drew only SOME of its rows (objectui#7148)', () => {
 it.each(THINNED)('says how many rows it drew when handed %s', (_label, rows, kept, total) => {
 const { container } = renderSankey(rows);

 const note = noteOf(container);
 expect(note, 'a partial flow states that it is partial').not.toBeNull();

 // BOTH halves of the count. "Some rows were dropped" would still leave a
 // thinned flow indistinguishable from a complete one; the ratio is the bit
 // the picture cannot carry.
 expect(note?.textContent).toContain(`Showing ${kept} of ${total} rows`);
 // Names the measure it tested and the exact test it failed, so the author
 // knows WHICH column decided it — the same diagnosis the refusal gives.
 expect(note?.textContent).toContain('amount');
 expect(note?.textContent).toContain('above zero');
 // A note annotates; it is not a state change and not an alert.
 expect(note?.getAttribute('role')).toBe('note');

 // ⛔ The chart is still DRAWN. The drop is not the defect — a flow has no
 // negative width — so nothing here may replace a drawable sankey with
 // prose. This is objectui#7146's boundary restated from the other side.
 expect(container.querySelector('svg'), 'a drawable sankey still draws').not.toBeNull();
 expect(refusalOf(container), 'a drawn chart is never also a refusal').toBeNull();
 });

 it('agrees with itself in the singular', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ]);
 expect(noteOf(container)?.textContent).toContain('1 row has no');
 });

 it('agrees with itself in the plural', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ]);
 expect(noteOf(container)?.textContent).toContain('2 rows have no');
 });

 it('CONTROL — an all-positive sankey carries no note, and gains no wrapper', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Renewal', amount: 12 },
 ]);

 expect(noteOf(container), 'nothing was omitted, so nothing is claimed').toBeNull();
 expect(container.querySelector('svg')).not.toBeNull();
 // The complete-flow path returns the container it always returned. The
 // footnote's flex wrapper would take over the height chain (see
 // `ChartFootnote`), so a chart with nothing to say must not get one.
 expect(
 container.firstElementChild?.getAttribute('data-slot'),
 'the chart container is still the root element',
 ).toBe('chart');
 });

 // ---- the seam with objectui#7146 -------------------------------------
 //
 // `rows.length` is the whole boundary: 0 survivors is that card, 1-or-more
 // survivors alongside a casualty is this one, and no casualties at all is
 // neither. Pinned from both sides so neither answer can drift into the
 // other's branch.

 it('SEAM — no row survives: objectui#7146 refuses, and this note stays out of it', () => {
 const { container } = renderSankey([
 { stage: 'A', amount: 0 },
 { stage: 'B', amount: 0 },
 { stage: 'C', amount: -5 },
 ]);

 expect(refusalOf(container), 'all-non-positive is still the refusal branch').not.toBeNull();
 expect(noteOf(container), 'a chart that drew nothing has no partial draw to annotate').toBeNull();
 expect(container.querySelector('svg')).toBeNull();
 });

 it('SEAM — no rows at all: still the bare div, untouched by both cards', () => {
 const { container } = renderSankey([]);

 expect(refusalOf(container)).toBeNull();
 expect(noteOf(container)).toBeNull();
 // The empty-RESULT question (objectui#7130), answered upstream in
 // ObjectChart where the query outcome is known.
 expect(container.textContent).toBe('');
 });

 it('does not reach other chart families handed the same mixed-sign rows', () => {
 // The note reads the sankey filter's own result and no other family has
 // such a filter — measured in Chromium, bar/pie/donut/funnel/treemap all
 // keep every row of this dataset in their data array. Pinned because
 // hoisting the count out of this arm is the obvious refactor and would
 // annotate four charts that omitted nothing.
 for (const chartType of ['bar', 'pie', 'donut', 'funnel', 'treemap'] as const) {
 const { container } = render(
 <AdvancedChartImpl
 chartType={chartType}
 xAxisKey="stage"
 series={SERIES as any}
 data={[
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ] as any}
 />,
 );
 expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull();
 cleanup();
 }
 });
});

122 changes: 111 additions & 11 deletions
packages/plugin-charts/src/AdvancedChartImpl.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -373,6 +373,40 @@ function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?:
 );
}

/**
 * `ChartFrame`'s mirror image: chrome BELOW the plot, for a chart that drew
 * something but did not draw all of it (objectui#7148).
 *
 * Same construction, and deliberately so, because the construction is the whole
 * difficulty. A footnote cannot simply be rendered as a SIBLING of
 * `ChartContainer`: measured in Chromium in the dashboard shape — a fixed,
 * clipping card whose chart carries `h-full` — the container takes the card's
 * full height and a following `<p>` lands at y=327 in a box that ends at y=316,
 * i.e. entirely outside the clip. A note invisible in dashboards is no note at
 * all, and dashboards are where these charts live.
 *
 * Nor can it be a plain wrapper `<div className={className}>` around the
 * container: the consumer's className is the chart's height contract and it has
 * to keep reaching the element Recharts measures (see `CHART_MIN_HEIGHT` in
 * `ChartContainerImpl` for the measurement of what happens when it does not —
 * a permanently zero box and an invisible chart, with no refusal and no empty
 * state).
 *
 * So the plot keeps its own `className` and gains a definite height through the
 * flex chain instead: `h-full` outer, `min-h-0 flex-1` around the plot,
 * `shrink-0` under it. With no footnote this returns the chart untouched, so no
 * existing caller gains a wrapper element — the same gate `ChartFrame` uses.
 */
function ChartFootnote({ note, children }: { note?: React.ReactNode; children: React.ReactNode }) {
 if (!note) return <>{children}</>;
 return (
 <div className="flex h-full w-full flex-col">
 <div className="min-h-0 flex-1">{children}</div>
 <div className="mt-1 shrink-0">{note}</div>
 </div>
 );
}

/**
 * AdvancedChartImpl - The heavy implementation that imports Recharts with full features
 * This component is lazy-loaded to avoid including Recharts in the initial bundle
Expand DownExpand Up@@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({
 </ChartRefusal>
 );
 }
 // A PARTIAL flow SAYS it is partial — objectui#7148, the branch next door
 // to the refusal above.
 //
 // The filter is unconditional, so a dataset where only SOME rows survive it
 // draws a normal, healthy, confident sankey of a fraction of itself, and
 // nothing in the output carried that fact. Measured in Chromium at
 // origin/main fd11e1644 across 27 tiles: the card's own dataset
 // (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 // `path: 3`, 18 descendants, no `role`, no text, and — against a live
 // console control that did fire on the same instrument — ZERO console
 // output. Its screenshot hashed `13237e6e19a7072a`, BYTE-IDENTICAL to a
 // genuinely one-row dataset `[{ New business: 40 }]` and to three other
 // thinned shapes (one positive among zeros, positive + nulls, positive +
 // unparseable). Six datasets, one image. A reader had no bit of information
 // distinguishing a complete flow from a third of one, and nobody re-reads a
 // dataset that renders fine.
 //
 // ## Why a note beside the chart, and not a refusal
 //
 // Not a style preference — a refusal is unavailable here. objectui#7146
 // pins "one positive row among zeros still draws", and that fixture
 // (`0 / 7 / 0`) is ITSELF a thinned dataset: it lands in this branch, and
 // hashes identical to the mixed-sign tile above. Refusing on a thinned flow
 // would blank the chart that pin requires drawn.
 //
 // The drop is also not the defect. A flow has no negative width, so
 // discarding those rows is the only thing this arm CAN do with them. What
 // was missing was saying so — which is the whole change: the plot is the
 // element this arm already returned, unchanged, with one line of prose
 // under it.
 //
 // ## Why the copy names the PREDICATE and a COUNT, not a cause
 //
 // The reason the refusal above gives, and this branch is where that family
 // actually lives: `Number(…) || 0` folds negatives, zeros, `null`,
 // unparseable strings and a missing key into ONE discard, and all five were
 // measured reaching here beside a survivor. Naming any one of them is a
 // sentence that is false for the other four, so the copy names the
 // predicate the filter actually applies, which is true of all of them.
 //
 // The COUNT is the half a reader cannot recover from the picture. "Some
 // rows were dropped" still leaves a thinned flow indistinguishable from a
 // complete one; `1 of 3` is the bit that was missing.
 //
 // No console warning, matching the refusal above and unlike the two guards
 // at the bottom of this file: those carry a diagnostic PAIR that does not
 // fit on screen, whereas this sentence already names the key, the test it
 // failed, and how many rows failed it.
 const omittedRowCount = data.length - rows.length;
 return (
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 <ChartFootnote
 note={
 omittedRowCount > 0 ? (
 <p
 role="note"
 data-chart-note="omitted-rows"
 className="px-1 text-xs text-muted-foreground"
 >
 Showing {rows.length} of {data.length} rows &mdash; {omittedRowCount}{' '}
 {omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '}
 <code className="font-mono">{dataKey}</code> above zero, which a flow cannot
 draw.
 </p>
 ) : null
 }
 >
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 </ChartFootnote>
 );
 }

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(plugin-charts): a sankey that drew only some of its rows says how many#7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-warren
merged 2 commits into

from

Sep 1, 2026
 Merged


fix(plugin-charts): a sankey that drew only some of its rows says how many
#7161
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
8d0af30

fix(plugin-charts): a sankey that drew only some of its rows says how…


claude Sep 1, 2026
f2df891

chore(changeset): sankey partial-flow footnote


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


34 changes: 34 additions & 0 deletions
.changeset/7148-sankey-omitted-rows-note.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
'@object-ui/plugin-charts': patch
---

A sankey that drew only SOME of its rows now says how many (objectui#7148).

The sankey arm keeps strictly positive measures
(`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a mixed dataset
drew a normal, healthy, confident chart of a fraction of itself and nothing
anywhere recorded that the other rows existed. Measured in Chromium across 27
tiles: `[{New business: 40}, {Refunds: -25}, {Chargebacks: -12}]` rendered
`svg: 1`, `path: 3`, 18 descendants, no `role`, no text, and — against a live
console control that did fire on the same instrument — zero console output.
Its screenshot hashed byte-identical to five other datasets, one of which
genuinely had a single row. Six datasets, one image: a reader had no bit of
information separating a complete flow from a third of one.

The discard itself stands — a flow has no negative width, so it is the only
thing that arm can do with those rows. What is added is a footnote under the
plot naming the ratio and the predicate the filter applies:

> Showing 1 of 3 rows — 2 rows have no `amount` above zero, which a flow
> cannot draw.

It names the predicate rather than a cause because `Number(…) || 0` folds
negatives, zeros, `null`, unparseable strings and a missing key into one
discard, and all five were measured reaching this branch beside a survivor;
naming any one of them is a sentence that is false for the other four.

A complete flow is byte-for-byte unchanged and gains no wrapper element, and a
drawable sankey is never replaced by prose: the `no-positive-flow` refusal
still owns the case where NOTHING survives the filter, and the "one positive
among zeros still draws" boundary still draws — that fixture is itself a
thinned dataset, so it now draws *and* says so.

222 changes: 222 additions & 0 deletions
packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
/**
 * ObjectUI
 * Copyright (c) 2024-present ObjectStack Inc.
 *
 * Sankey — SOME rows survive the positive filter and the rest are dropped
 * silently (objectui#7148). The branch next door to objectui#7140's refusal,
 * and the more dangerous of the two.
 *
 * The sankey arm keeps only strictly positive measures
 * (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`). When that filter
 * keeps NOTHING, objectui#7146 now says so. When it keeps SOME, the chart drew
 * a normal, healthy, confident sankey of a fraction of its dataset and nothing
 * anywhere recorded that the other rows existed.
 *
 * ## The measurement that decided fix-over-decline
 *
 * 27 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main`
 * fd11e1644, each tile screenshotted and SHA-256'd. The card's own dataset
 * (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 * `path: 3`, 18 descendants, no `role`, no text — and, against a live console
 * control that DID fire on the same instrument (`missing-category-key`), zero
 * console output. Its screenshot hashed `13237e6e19a7072a`, byte-identical to
 * FIVE other tiles including a genuinely one-row dataset
 * `[{ stage: 'New business', amount: 40 }]`. Six datasets, one image: the
 * reader had no bit of information separating a complete flow from a third of
 * one. That is why "discarding is all a flow CAN do" does not settle the card —
 * the drop is fine, the silence is not.
 *
 * ## Why a note and not a refusal
 *
 * objectui#7146 pins "one positive row among zeros still draws". That fixture
 * (`0 / 7 / 0`) is itself a THINNED dataset — it lands in this branch and
 * hashed identical to the mixed-sign tile — so a refusal here would blank a
 * chart that pin requires drawn. The two cards meet exactly at
 * `rows.length === 0`: none survive → refusal (objectui#7146); some survive and
 * some do not → this note; all survive → untouched.
 */
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
 const actual = await vi.importActual<any>('recharts');
 return {
 ...actual,
 ResponsiveContainer: ({ children }: any) =>
 React.cloneElement(children, { width: 480, height: 320 }),
 };
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
 render(
 <AdvancedChartImpl
 chartType="sankey"
 xAxisKey="stage"
 series={SERIES as any}
 data={data as any}
 />,
 );

const noteOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-note="omitted-rows"]');
const refusalOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-error="no-positive-flow"]');

/**
 * Every shape `Number(…) || 0` folds to zero, each one BESIDE a survivor.
 *
 * They are not the same situation and the copy deliberately names none of them:
 * a message that said "negative" would be false of the null row, and the card
 * itself notes that `null` and unparseable measures vanish through the very
 * same filter. All five were measured reaching this branch in Chromium.
 */
const THINNED: Array<[string, Array<Record<string, unknown>>, number, number]> = [
 ['negative rows (the card\'s own dataset)', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 1, 3],
 ['zero rows (objectui#7146\'s pinned boundary)', [
 { stage: 'Prospecting', amount: 0 },
 { stage: 'Proposal', amount: 7 },
 { stage: 'Won', amount: 0 },
 ], 1, 3],
 ['null measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: null },
 { stage: 'Credits', amount: null },
 ], 1, 3],
 ['unparseable measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: 'n/a' },
 ], 1, 2],
 ['a row missing the measure key entirely', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds' },
 ], 1, 2],
 ['a mix that keeps more than one row', [
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 2, 3],
];

describe('AdvancedChartImpl — sankey that drew only SOME of its rows (objectui#7148)', () => {
 it.each(THINNED)('says how many rows it drew when handed %s', (_label, rows, kept, total) => {
 const { container } = renderSankey(rows);

 const note = noteOf(container);
 expect(note, 'a partial flow states that it is partial').not.toBeNull();

 // BOTH halves of the count. "Some rows were dropped" would still leave a
 // thinned flow indistinguishable from a complete one; the ratio is the bit
 // the picture cannot carry.
 expect(note?.textContent).toContain(`Showing ${kept} of ${total} rows`);
 // Names the measure it tested and the exact test it failed, so the author
 // knows WHICH column decided it — the same diagnosis the refusal gives.
 expect(note?.textContent).toContain('amount');
 expect(note?.textContent).toContain('above zero');
 // A note annotates; it is not a state change and not an alert.
 expect(note?.getAttribute('role')).toBe('note');

 // ⛔ The chart is still DRAWN. The drop is not the defect — a flow has no
 // negative width — so nothing here may replace a drawable sankey with
 // prose. This is objectui#7146's boundary restated from the other side.
 expect(container.querySelector('svg'), 'a drawable sankey still draws').not.toBeNull();
 expect(refusalOf(container), 'a drawn chart is never also a refusal').toBeNull();
 });

 it('agrees with itself in the singular', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ]);
 expect(noteOf(container)?.textContent).toContain('1 row has no');
 });

 it('agrees with itself in the plural', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ]);
 expect(noteOf(container)?.textContent).toContain('2 rows have no');
 });

 it('CONTROL — an all-positive sankey carries no note, and gains no wrapper', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Renewal', amount: 12 },
 ]);

 expect(noteOf(container), 'nothing was omitted, so nothing is claimed').toBeNull();
 expect(container.querySelector('svg')).not.toBeNull();
 // The complete-flow path returns the container it always returned. The
 // footnote's flex wrapper would take over the height chain (see
 // `ChartFootnote`), so a chart with nothing to say must not get one.
 expect(
 container.firstElementChild?.getAttribute('data-slot'),
 'the chart container is still the root element',
 ).toBe('chart');
 });

 // ---- the seam with objectui#7146 -------------------------------------
 //
 // `rows.length` is the whole boundary: 0 survivors is that card, 1-or-more
 // survivors alongside a casualty is this one, and no casualties at all is
 // neither. Pinned from both sides so neither answer can drift into the
 // other's branch.

 it('SEAM — no row survives: objectui#7146 refuses, and this note stays out of it', () => {
 const { container } = renderSankey([
 { stage: 'A', amount: 0 },
 { stage: 'B', amount: 0 },
 { stage: 'C', amount: -5 },
 ]);

 expect(refusalOf(container), 'all-non-positive is still the refusal branch').not.toBeNull();
 expect(noteOf(container), 'a chart that drew nothing has no partial draw to annotate').toBeNull();
 expect(container.querySelector('svg')).toBeNull();
 });

 it('SEAM — no rows at all: still the bare div, untouched by both cards', () => {
 const { container } = renderSankey([]);

 expect(refusalOf(container)).toBeNull();
 expect(noteOf(container)).toBeNull();
 // The empty-RESULT question (objectui#7130), answered upstream in
 // ObjectChart where the query outcome is known.
 expect(container.textContent).toBe('');
 });

 it('does not reach other chart families handed the same mixed-sign rows', () => {
 // The note reads the sankey filter's own result and no other family has
 // such a filter — measured in Chromium, bar/pie/donut/funnel/treemap all
 // keep every row of this dataset in their data array. Pinned because
 // hoisting the count out of this arm is the obvious refactor and would
 // annotate four charts that omitted nothing.
 for (const chartType of ['bar', 'pie', 'donut', 'funnel', 'treemap'] as const) {
 const { container } = render(
 <AdvancedChartImpl
 chartType={chartType}
 xAxisKey="stage"
 series={SERIES as any}
 data={[
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ] as any}
 />,
 );
 expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull();
 cleanup();
 }
 });
});

122 changes: 111 additions & 11 deletions
packages/plugin-charts/src/AdvancedChartImpl.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -373,6 +373,40 @@ function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?:
 );
}

/**
 * `ChartFrame`'s mirror image: chrome BELOW the plot, for a chart that drew
 * something but did not draw all of it (objectui#7148).
 *
 * Same construction, and deliberately so, because the construction is the whole
 * difficulty. A footnote cannot simply be rendered as a SIBLING of
 * `ChartContainer`: measured in Chromium in the dashboard shape — a fixed,
 * clipping card whose chart carries `h-full` — the container takes the card's
 * full height and a following `<p>` lands at y=327 in a box that ends at y=316,
 * i.e. entirely outside the clip. A note invisible in dashboards is no note at
 * all, and dashboards are where these charts live.
 *
 * Nor can it be a plain wrapper `<div className={className}>` around the
 * container: the consumer's className is the chart's height contract and it has
 * to keep reaching the element Recharts measures (see `CHART_MIN_HEIGHT` in
 * `ChartContainerImpl` for the measurement of what happens when it does not —
 * a permanently zero box and an invisible chart, with no refusal and no empty
 * state).
 *
 * So the plot keeps its own `className` and gains a definite height through the
 * flex chain instead: `h-full` outer, `min-h-0 flex-1` around the plot,
 * `shrink-0` under it. With no footnote this returns the chart untouched, so no
 * existing caller gains a wrapper element — the same gate `ChartFrame` uses.
 */
function ChartFootnote({ note, children }: { note?: React.ReactNode; children: React.ReactNode }) {
 if (!note) return <>{children}</>;
 return (
 <div className="flex h-full w-full flex-col">
 <div className="min-h-0 flex-1">{children}</div>
 <div className="mt-1 shrink-0">{note}</div>
 </div>
 );
}

/**
 * AdvancedChartImpl - The heavy implementation that imports Recharts with full features
 * This component is lazy-loaded to avoid including Recharts in the initial bundle
Expand DownExpand Up@@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({
 </ChartRefusal>
 );
 }
 // A PARTIAL flow SAYS it is partial — objectui#7148, the branch next door
 // to the refusal above.
 //
 // The filter is unconditional, so a dataset where only SOME rows survive it
 // draws a normal, healthy, confident sankey of a fraction of itself, and
 // nothing in the output carried that fact. Measured in Chromium at
 // origin/main fd11e1644 across 27 tiles: the card's own dataset
 // (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 // `path: 3`, 18 descendants, no `role`, no text, and — against a live
 // console control that did fire on the same instrument — ZERO console
 // output. Its screenshot hashed `13237e6e19a7072a`, BYTE-IDENTICAL to a
 // genuinely one-row dataset `[{ New business: 40 }]` and to three other
 // thinned shapes (one positive among zeros, positive + nulls, positive +
 // unparseable). Six datasets, one image. A reader had no bit of information
 // distinguishing a complete flow from a third of one, and nobody re-reads a
 // dataset that renders fine.
 //
 // ## Why a note beside the chart, and not a refusal
 //
 // Not a style preference — a refusal is unavailable here. objectui#7146
 // pins "one positive row among zeros still draws", and that fixture
 // (`0 / 7 / 0`) is ITSELF a thinned dataset: it lands in this branch, and
 // hashes identical to the mixed-sign tile above. Refusing on a thinned flow
 // would blank the chart that pin requires drawn.
 //
 // The drop is also not the defect. A flow has no negative width, so
 // discarding those rows is the only thing this arm CAN do with them. What
 // was missing was saying so — which is the whole change: the plot is the
 // element this arm already returned, unchanged, with one line of prose
 // under it.
 //
 // ## Why the copy names the PREDICATE and a COUNT, not a cause
 //
 // The reason the refusal above gives, and this branch is where that family
 // actually lives: `Number(…) || 0` folds negatives, zeros, `null`,
 // unparseable strings and a missing key into ONE discard, and all five were
 // measured reaching here beside a survivor. Naming any one of them is a
 // sentence that is false for the other four, so the copy names the
 // predicate the filter actually applies, which is true of all of them.
 //
 // The COUNT is the half a reader cannot recover from the picture. "Some
 // rows were dropped" still leaves a thinned flow indistinguishable from a
 // complete one; `1 of 3` is the bit that was missing.
 //
 // No console warning, matching the refusal above and unlike the two guards
 // at the bottom of this file: those carry a diagnostic PAIR that does not
 // fit on screen, whereas this sentence already names the key, the test it
 // failed, and how many rows failed it.
 const omittedRowCount = data.length - rows.length;
 return (
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 <ChartFootnote
 note={
 omittedRowCount > 0 ? (
 <p
 role="note"
 data-chart-note="omitted-rows"
 className="px-1 text-xs text-muted-foreground"
 >
 Showing {rows.length} of {data.length} rows &mdash; {omittedRowCount}{' '}
 {omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '}
 <code className="font-mono">{dataKey}</code> above zero, which a flow cannot
 draw.
 </p>
 ) : null
 }
 >
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 </ChartFootnote>
 );
 }

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix(plugin-charts): a sankey that drew only some of its rows says how many#7161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-warren
merged 2 commits into

from

Sep 1, 2026
 Merged


fix(plugin-charts): a sankey that drew only some of its rows says how many
#7161
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
8d0af30

fix(plugin-charts): a sankey that drew only some of its rows says how…


claude Sep 1, 2026
f2df891

chore(changeset): sankey partial-flow footnote


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


34 changes: 34 additions & 0 deletions
.changeset/7148-sankey-omitted-rows-note.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
'@object-ui/plugin-charts': patch
---

A sankey that drew only SOME of its rows now says how many (objectui#7148).

The sankey arm keeps strictly positive measures
(`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a mixed dataset
drew a normal, healthy, confident chart of a fraction of itself and nothing
anywhere recorded that the other rows existed. Measured in Chromium across 27
tiles: `[{New business: 40}, {Refunds: -25}, {Chargebacks: -12}]` rendered
`svg: 1`, `path: 3`, 18 descendants, no `role`, no text, and — against a live
console control that did fire on the same instrument — zero console output.
Its screenshot hashed byte-identical to five other datasets, one of which
genuinely had a single row. Six datasets, one image: a reader had no bit of
information separating a complete flow from a third of one.

The discard itself stands — a flow has no negative width, so it is the only
thing that arm can do with those rows. What is added is a footnote under the
plot naming the ratio and the predicate the filter applies:

> Showing 1 of 3 rows — 2 rows have no `amount` above zero, which a flow
> cannot draw.

It names the predicate rather than a cause because `Number(…) || 0` folds
negatives, zeros, `null`, unparseable strings and a missing key into one
discard, and all five were measured reaching this branch beside a survivor;
naming any one of them is a sentence that is false for the other four.

A complete flow is byte-for-byte unchanged and gains no wrapper element, and a
drawable sankey is never replaced by prose: the `no-positive-flow` refusal
still owns the case where NOTHING survives the filter, and the "one positive
among zeros still draws" boundary still draws — that fixture is itself a
thinned dataset, so it now draws *and* says so.

222 changes: 222 additions & 0 deletions
packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
/**
 * ObjectUI
 * Copyright (c) 2024-present ObjectStack Inc.
 *
 * Sankey — SOME rows survive the positive filter and the rest are dropped
 * silently (objectui#7148). The branch next door to objectui#7140's refusal,
 * and the more dangerous of the two.
 *
 * The sankey arm keeps only strictly positive measures
 * (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`). When that filter
 * keeps NOTHING, objectui#7146 now says so. When it keeps SOME, the chart drew
 * a normal, healthy, confident sankey of a fraction of its dataset and nothing
 * anywhere recorded that the other rows existed.
 *
 * ## The measurement that decided fix-over-decline
 *
 * 27 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main`
 * fd11e1644, each tile screenshotted and SHA-256'd. The card's own dataset
 * (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 * `path: 3`, 18 descendants, no `role`, no text — and, against a live console
 * control that DID fire on the same instrument (`missing-category-key`), zero
 * console output. Its screenshot hashed `13237e6e19a7072a`, byte-identical to
 * FIVE other tiles including a genuinely one-row dataset
 * `[{ stage: 'New business', amount: 40 }]`. Six datasets, one image: the
 * reader had no bit of information separating a complete flow from a third of
 * one. That is why "discarding is all a flow CAN do" does not settle the card —
 * the drop is fine, the silence is not.
 *
 * ## Why a note and not a refusal
 *
 * objectui#7146 pins "one positive row among zeros still draws". That fixture
 * (`0 / 7 / 0`) is itself a THINNED dataset — it lands in this branch and
 * hashed identical to the mixed-sign tile — so a refusal here would blank a
 * chart that pin requires drawn. The two cards meet exactly at
 * `rows.length === 0`: none survive → refusal (objectui#7146); some survive and
 * some do not → this note; all survive → untouched.
 */
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
 const actual = await vi.importActual<any>('recharts');
 return {
 ...actual,
 ResponsiveContainer: ({ children }: any) =>
 React.cloneElement(children, { width: 480, height: 320 }),
 };
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
 render(
 <AdvancedChartImpl
 chartType="sankey"
 xAxisKey="stage"
 series={SERIES as any}
 data={data as any}
 />,
 );

const noteOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-note="omitted-rows"]');
const refusalOf = (container: HTMLElement) =>
 container.querySelector('[data-chart-error="no-positive-flow"]');

/**
 * Every shape `Number(…) || 0` folds to zero, each one BESIDE a survivor.
 *
 * They are not the same situation and the copy deliberately names none of them:
 * a message that said "negative" would be false of the null row, and the card
 * itself notes that `null` and unparseable measures vanish through the very
 * same filter. All five were measured reaching this branch in Chromium.
 */
const THINNED: Array<[string, Array<Record<string, unknown>>, number, number]> = [
 ['negative rows (the card\'s own dataset)', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 1, 3],
 ['zero rows (objectui#7146\'s pinned boundary)', [
 { stage: 'Prospecting', amount: 0 },
 { stage: 'Proposal', amount: 7 },
 { stage: 'Won', amount: 0 },
 ], 1, 3],
 ['null measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: null },
 { stage: 'Credits', amount: null },
 ], 1, 3],
 ['unparseable measures', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: 'n/a' },
 ], 1, 2],
 ['a row missing the measure key entirely', [
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds' },
 ], 1, 2],
 ['a mix that keeps more than one row', [
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Chargebacks', amount: -12 },
 ], 2, 3],
];

describe('AdvancedChartImpl — sankey that drew only SOME of its rows (objectui#7148)', () => {
 it.each(THINNED)('says how many rows it drew when handed %s', (_label, rows, kept, total) => {
 const { container } = renderSankey(rows);

 const note = noteOf(container);
 expect(note, 'a partial flow states that it is partial').not.toBeNull();

 // BOTH halves of the count. "Some rows were dropped" would still leave a
 // thinned flow indistinguishable from a complete one; the ratio is the bit
 // the picture cannot carry.
 expect(note?.textContent).toContain(`Showing ${kept} of ${total} rows`);
 // Names the measure it tested and the exact test it failed, so the author
 // knows WHICH column decided it — the same diagnosis the refusal gives.
 expect(note?.textContent).toContain('amount');
 expect(note?.textContent).toContain('above zero');
 // A note annotates; it is not a state change and not an alert.
 expect(note?.getAttribute('role')).toBe('note');

 // ⛔ The chart is still DRAWN. The drop is not the defect — a flow has no
 // negative width — so nothing here may replace a drawable sankey with
 // prose. This is objectui#7146's boundary restated from the other side.
 expect(container.querySelector('svg'), 'a drawable sankey still draws').not.toBeNull();
 expect(refusalOf(container), 'a drawn chart is never also a refusal').toBeNull();
 });

 it('agrees with itself in the singular', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ]);
 expect(noteOf(container)?.textContent).toContain('1 row has no');
 });

 it('agrees with itself in the plural', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 { stage: 'Chargebacks', amount: -12 },
 ]);
 expect(noteOf(container)?.textContent).toContain('2 rows have no');
 });

 it('CONTROL — an all-positive sankey carries no note, and gains no wrapper', () => {
 const { container } = renderSankey([
 { stage: 'New business', amount: 40 },
 { stage: 'Expansion', amount: 25 },
 { stage: 'Renewal', amount: 12 },
 ]);

 expect(noteOf(container), 'nothing was omitted, so nothing is claimed').toBeNull();
 expect(container.querySelector('svg')).not.toBeNull();
 // The complete-flow path returns the container it always returned. The
 // footnote's flex wrapper would take over the height chain (see
 // `ChartFootnote`), so a chart with nothing to say must not get one.
 expect(
 container.firstElementChild?.getAttribute('data-slot'),
 'the chart container is still the root element',
 ).toBe('chart');
 });

 // ---- the seam with objectui#7146 -------------------------------------
 //
 // `rows.length` is the whole boundary: 0 survivors is that card, 1-or-more
 // survivors alongside a casualty is this one, and no casualties at all is
 // neither. Pinned from both sides so neither answer can drift into the
 // other's branch.

 it('SEAM — no row survives: objectui#7146 refuses, and this note stays out of it', () => {
 const { container } = renderSankey([
 { stage: 'A', amount: 0 },
 { stage: 'B', amount: 0 },
 { stage: 'C', amount: -5 },
 ]);

 expect(refusalOf(container), 'all-non-positive is still the refusal branch').not.toBeNull();
 expect(noteOf(container), 'a chart that drew nothing has no partial draw to annotate').toBeNull();
 expect(container.querySelector('svg')).toBeNull();
 });

 it('SEAM — no rows at all: still the bare div, untouched by both cards', () => {
 const { container } = renderSankey([]);

 expect(refusalOf(container)).toBeNull();
 expect(noteOf(container)).toBeNull();
 // The empty-RESULT question (objectui#7130), answered upstream in
 // ObjectChart where the query outcome is known.
 expect(container.textContent).toBe('');
 });

 it('does not reach other chart families handed the same mixed-sign rows', () => {
 // The note reads the sankey filter's own result and no other family has
 // such a filter — measured in Chromium, bar/pie/donut/funnel/treemap all
 // keep every row of this dataset in their data array. Pinned because
 // hoisting the count out of this arm is the obvious refactor and would
 // annotate four charts that omitted nothing.
 for (const chartType of ['bar', 'pie', 'donut', 'funnel', 'treemap'] as const) {
 const { container } = render(
 <AdvancedChartImpl
 chartType={chartType}
 xAxisKey="stage"
 series={SERIES as any}
 data={[
 { stage: 'New business', amount: 40 },
 { stage: 'Refunds', amount: -25 },
 ] as any}
 />,
 );
 expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull();
 cleanup();
 }
 });
});

122 changes: 111 additions & 11 deletions
packages/plugin-charts/src/AdvancedChartImpl.tsx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -373,6 +373,40 @@ function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?:
 );
}

/**
 * `ChartFrame`'s mirror image: chrome BELOW the plot, for a chart that drew
 * something but did not draw all of it (objectui#7148).
 *
 * Same construction, and deliberately so, because the construction is the whole
 * difficulty. A footnote cannot simply be rendered as a SIBLING of
 * `ChartContainer`: measured in Chromium in the dashboard shape — a fixed,
 * clipping card whose chart carries `h-full` — the container takes the card's
 * full height and a following `<p>` lands at y=327 in a box that ends at y=316,
 * i.e. entirely outside the clip. A note invisible in dashboards is no note at
 * all, and dashboards are where these charts live.
 *
 * Nor can it be a plain wrapper `<div className={className}>` around the
 * container: the consumer's className is the chart's height contract and it has
 * to keep reaching the element Recharts measures (see `CHART_MIN_HEIGHT` in
 * `ChartContainerImpl` for the measurement of what happens when it does not —
 * a permanently zero box and an invisible chart, with no refusal and no empty
 * state).
 *
 * So the plot keeps its own `className` and gains a definite height through the
 * flex chain instead: `h-full` outer, `min-h-0 flex-1` around the plot,
 * `shrink-0` under it. With no footnote this returns the chart untouched, so no
 * existing caller gains a wrapper element — the same gate `ChartFrame` uses.
 */
function ChartFootnote({ note, children }: { note?: React.ReactNode; children: React.ReactNode }) {
 if (!note) return <>{children}</>;
 return (
 <div className="flex h-full w-full flex-col">
 <div className="min-h-0 flex-1">{children}</div>
 <div className="mt-1 shrink-0">{note}</div>
 </div>
 );
}

/**
 * AdvancedChartImpl - The heavy implementation that imports Recharts with full features
 * This component is lazy-loaded to avoid including Recharts in the initial bundle
Expand DownExpand Up@@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({
 </ChartRefusal>
 );
 }
 // A PARTIAL flow SAYS it is partial — objectui#7148, the branch next door
 // to the refusal above.
 //
 // The filter is unconditional, so a dataset where only SOME rows survive it
 // draws a normal, healthy, confident sankey of a fraction of itself, and
 // nothing in the output carried that fact. Measured in Chromium at
 // origin/main fd11e1644 across 27 tiles: the card's own dataset
 // (`New business 40 / Refunds -25 / Chargebacks -12`) rendered `svg: 1`,
 // `path: 3`, 18 descendants, no `role`, no text, and — against a live
 // console control that did fire on the same instrument — ZERO console
 // output. Its screenshot hashed `13237e6e19a7072a`, BYTE-IDENTICAL to a
 // genuinely one-row dataset `[{ New business: 40 }]` and to three other
 // thinned shapes (one positive among zeros, positive + nulls, positive +
 // unparseable). Six datasets, one image. A reader had no bit of information
 // distinguishing a complete flow from a third of one, and nobody re-reads a
 // dataset that renders fine.
 //
 // ## Why a note beside the chart, and not a refusal
 //
 // Not a style preference — a refusal is unavailable here. objectui#7146
 // pins "one positive row among zeros still draws", and that fixture
 // (`0 / 7 / 0`) is ITSELF a thinned dataset: it lands in this branch, and
 // hashes identical to the mixed-sign tile above. Refusing on a thinned flow
 // would blank the chart that pin requires drawn.
 //
 // The drop is also not the defect. A flow has no negative width, so
 // discarding those rows is the only thing this arm CAN do with them. What
 // was missing was saying so — which is the whole change: the plot is the
 // element this arm already returned, unchanged, with one line of prose
 // under it.
 //
 // ## Why the copy names the PREDICATE and a COUNT, not a cause
 //
 // The reason the refusal above gives, and this branch is where that family
 // actually lives: `Number(…) || 0` folds negatives, zeros, `null`,
 // unparseable strings and a missing key into ONE discard, and all five were
 // measured reaching here beside a survivor. Naming any one of them is a
 // sentence that is false for the other four, so the copy names the
 // predicate the filter actually applies, which is true of all of them.
 //
 // The COUNT is the half a reader cannot recover from the picture. "Some
 // rows were dropped" still leaves a thinned flow indistinguishable from a
 // complete one; `1 of 3` is the bit that was missing.
 //
 // No console warning, matching the refusal above and unlike the two guards
 // at the bottom of this file: those carry a diagnostic PAIR that does not
 // fit on screen, whereas this sentence already names the key, the test it
 // failed, and how many rows failed it.
 const omittedRowCount = data.length - rows.length;
 return (
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 <ChartFootnote
 note={
 omittedRowCount > 0 ? (
 <p
 role="note"
 data-chart-note="omitted-rows"
 className="px-1 text-xs text-muted-foreground"
 >
 Showing {rows.length} of {data.length} rows &mdash; {omittedRowCount}{' '}
 {omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '}
 <code className="font-mono">{dataKey}</code> above zero, which a flow cannot
 draw.
 </p>
 ) : null
 }
 >
 <ChartContainer config={config} className={className} {...containerProps}>
 <Sankey
 data={{ nodes, links }}
 nodePadding={24}
 link={{ stroke: 'hsl(var(--muted-foreground))', strokeOpacity: 0.25 }}
 node={{ fill: 'hsl(var(--chart-1))' } as any}
 {...sankeyClickProps}
 >
 <Tooltip />
 </Sankey>
 </ChartContainer>
 </ChartFootnote>
 );
 }

Expand Down
Loading