From 8d0af30a88406c8ad771290da5acabce45719052 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:10:05 +0000 Subject: [PATCH 1/2] fix(plugin-charts): a sankey that drew only some of its rows says how many MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sankey arm filters to strictly positive measures, so a mixed dataset draws a normal, healthy, confident chart of a fraction of itself with nothing recording that the other rows existed. Measured in Chromium across 27 tiles at origin/main fd11e1644: the mixed-sign dataset rendered svg 1 / path 3 / 18 descendants, no role, no text and — against a live console control that did fire — zero console output, and its screenshot hashed byte-identical to five other datasets including a genuinely one-row one. Six datasets, one image. The drop itself stands: a flow has no negative width. What is added is a footnote naming the ratio and the predicate the filter applies, which is true of negatives, zeros, nulls, unparseable measures and a missing key alike. A refusal is unavailable — objectui#7146 pins "one positive among zeros still draws" and that fixture is itself a thinned dataset. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM --- ...vancedChartImpl.sankeyOmittedRows.test.tsx | 222 ++++++++++++++++++ .../plugin-charts/src/AdvancedChartImpl.tsx | 122 +++++++++- 2 files changed, 333 insertions(+), 11 deletions(-) create mode 100644 packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx diff --git a/packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx new file mode 100644 index 000000000..0161a9390 --- /dev/null +++ b/packages/plugin-charts/src/AdvancedChartImpl.sankeyOmittedRows.test.tsx @@ -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('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>) => + render( + , + ); + +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>, 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( + , + ); + expect(noteOf(container), `${chartType} omitted nothing and must say nothing`).toBeNull(); + cleanup(); + } + }); +}); diff --git a/packages/plugin-charts/src/AdvancedChartImpl.tsx b/packages/plugin-charts/src/AdvancedChartImpl.tsx index 3105af33a..5e2486b8f 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -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 `

` 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 `

` 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 ( +
+
{children}
+
{note}
+
+ ); +} + /** * AdvancedChartImpl - The heavy implementation that imports Recharts with full features * This component is lazy-loaded to avoid including Recharts in the initial bundle @@ -1088,18 +1122,84 @@ function AdvancedChartImplInner({ ); } + // 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 ( - - - - - + 0 ? ( +

+ Showing {rows.length} of {data.length} rows — {omittedRowCount}{' '} + {omittedRowCount === 1 ? 'row has' : 'rows have'} no{' '} + {dataKey} above zero, which a flow cannot + draw. +

+ ) : null + } + > + + + + + + ); } From f2df891447e2bc029dd131ecc39b01d76d43b04a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:20:31 +0000 Subject: [PATCH 2/2] chore(changeset): sankey partial-flow footnote Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM --- .changeset/7148-sankey-omitted-rows-note.md | 34 +++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 .changeset/7148-sankey-omitted-rows-note.md diff --git a/.changeset/7148-sankey-omitted-rows-note.md b/.changeset/7148-sankey-omitted-rows-note.md new file mode 100644 index 000000000..48b8bf99c --- /dev/null +++ b/.changeset/7148-sankey-omitted-rows-note.md @@ -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.