fix(charts): the pivot branch buckets null first-dimension values instead of dropping the bar (#4497) - #4506
Merged
Merged
Conversation
…tead of dropping the bar (#4497) `buildChartSeries`' multi-dimension pivot branch bucketed rows by `String(xRaw ?? '')` but wrote the RAW value into the emitted row, so a null first-dimension value produced `{status: null, Low: 3}` and reached recharts with a null category — which draws no mark. Measured at the DOM: a two-group pivot drew ONE bar; an all-null pivot drew zero bar rectangles under a full axis. Same mechanism as #4466 (PR #4498), one branch over. The pivot now maps a null/undefined first-dimension VALUE to the same bucket label the single-dimension branch uses, through the same `ChartSeriesOptions.nullCategoryLabel` / `NULL_CATEGORY_LABEL` default. One doctrine, one predicate (`isNullCategory`, extracted from the existing `bucketNullCategories` so the two branches cannot drift), two call sites. No new export: the emitted `.d.ts` declarations are byte-identical. The bucket KEY is untouched, so which rows share a bar is byte-identical and only the display value changes. Rows lacking the category key entirely are still not bucketed (framework#4033's division). Drill-through needed no change, and that was measured rather than assumed: the pivot AGGREGATES, so its emitted rows are not index-aligned with `drillRawRows`, and `DatasetWidget.handleChartDrill` already drills by searching the raw rows through `findChartSeriesRow` — whose `xOf` (from #4498) already covers the multi-dimension arm. Pinned at both levels. #4498's deliberate pin of the pivot's null-drop ("leaves the multi-dimension pivot branch exactly as it was") is updated in this same commit, citing #4497, which is what that pin existed for. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
This was referenced Aug 12, 2026
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
Independent re-verification at the merged head (5dcafaa)The session that opened this PR was killed by a host restart after pushing c2e03fc but before it reported. This is the inheriting session re-deriving every claim above from scratch rather than carrying them forward on trust, plus Red-first, re-derived by commit-then-revertMethod: Core unit, verbatim: DOM, verbatim: The drill measurement, confirmed independentlyRe-ran the reverted source under The one that fails asserts Grading re-measured
Full pass at the merged head
The consumer sweep is 93/1027 rather than the 92/1017 quoted in the description above: the merge added Generated by Claude Code |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
The card's STOP condition — "two raw groups mapping to one label" — was measured in only one of its two row orders. The suite pinned [null first, '' second], where the bucket renders `(None)` and the `''` row's segment drills to -1 (a dead click, filed as #4508). The reverse order was left unmeasured, and it does not behave alike: with the `''` row creating the bucket the label is the raw empty string, `isNullCategory` never fires, and BOTH segments resolve correctly. Only one of the two orders costs a drill. Green on both sides of the fix, deliberately: this order is byte-identical before and after (verified against origin/main's chart-series.ts), so it is a must-not-change control rather than a red-first case. It also pins one asymmetry the writer cannot show: `findChartSeriesRow` accepts BOTH spellings of "no value" unconditionally, so it resolves the bucket label even in the order where no bar carries it. That is #4466's stated design and is harmless — a renderer only ever hands back a category recharts painted — but it is invisible from `buildChartSeries` alone, and it is the slack that keeps the two callers' labels from having to agree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
buildChartSeries' multi-dimension pivot branch (2+ dimensions, single measure) bucketed rows byString(xRaw ?? '')but wrote the RAW value into the emitted row:So a null first-dimension value produced
{status: null, Low: 3}, which reaches recharts with a null category and draws no mark. The map key and the emitted display value were two different things — which is exactly why this branch needed its own measurement rather than the single-dimension answer applied on the way past.Measured at the DOM, with the fix reverted:
Backlog/High+null/Low) → 1.recharts-bar-rectangle, the null group's bar missing while its series still occupied the legend;The doctrine it extends
Same mechanism and same answer as #4466 / PR #4498, one branch below: a null/undefined first-dimension VALUE maps to an explicit bucket label via the existing
ChartSeriesOptions.nullCategoryLabel, defaulting toNULL_CATEGORY_LABEL. No new export — the emitted.d.tsdeclarations are byte-identical (43 lines before and after, comments stripped; the only delta is JSDoc), so this is apatch, not aminor.The two branches now share one predicate,
isNullCategory, extracted from #4498'sbucketNullCategoriesso they cannot drift apart. Two properties are kept deliberately:String(xRaw ?? '')still decides which rows share a bar, so every existing grouping is byte-identical; only the display value the bucket carries changes.Drill measurement (the filing's binding condition)
The card required
findChartSeriesRow's multi-dimension arm to move with the emitted-row change so the newly-visible bar keeps its drill. Measured first, as instructed — and the measurement says the arm had already moved, in PR #4498, so this PR changesbuildChartSeriesalone:DatasetWidget.handleChartDrill— the one production caller — therefore searches the raw rows withfindChartSeriesRow(chartRows, …)and indexesdrillRawRowswith the result.nulleither way, and fix(charts): null-keyed groups render as an explicit bucket instead of silently vanishing (#4466) #4498'sxOfhelper is applied in both arms offindChartSeriesRow. So the pivot's key-vs-value split never reached the drill.Verified independently of the fix: with
chart-series.tsreverted toorigin/main, the four new drill cases pass while the eight forward cases fail. Post-fix both halves agree, and clicking the bucket bar resolves to the right raw row per series ((None)/Lowto index 1,(None)/Highto index 2).The STOP condition, measured in both row orders
The card said to STOP and report if the label match turned out ambiguous — "two raw groups mapping to one label". It does happen, and it is order-dependent, which the suite now pins on both sides:
(None), and the segment sourced from the''row resolves to-1: a no-op click, never a drill into the wrong records. Pre-fix the whole bar was invisible, so this is strictly more affordance than before.''as its label,isNullCategorynever fires, and both segments resolve correctly (0and1). Nothing is lost in this order at all.So only one of the two orders costs a drill. Widening
findChartSeriesRowto close it would make a bar labelled(None)drill to a raw row whose stored value is''— trading a dead click for a wrong one — so the limit is filed as #4508 rather than fixed here, which is the STOP the card asked for.The both-orders pin also records one asymmetry that
buildChartSeriesalone cannot show:findChartSeriesRowaccepts both spellings of "no value" unconditionally, so it resolves the bucket label even in the order where no bar carries it. That is #4466's stated design and is harmless — a renderer only ever hands back a category recharts actually painted — but it is the slack that keeps the two callers' labels from having to agree, and it is now pinned rather than assumed.A second ambiguity is pinned the same way: a stored value spelling the label literally keeps its own bucket, so two bars carry the same axis text and the click resolves to the first. Inherited unchanged from the single-dimension branch, where #4466 shipped exactly this trade. Also #4508.
Interaction with #4509 (landed after this branch's first CI run)
#4509 (
DatasetWidget localizes the null-bucket label, #4500) landed onmainat 00:12, six minutes after this PR's first CI run, so that run never saw the combination.mainhas since been merged in and the pair verified locally:DatasetWidgetpass{ nullCategoryLabel }to bothbuildChartSeriesandfindChartSeriesRow, so a dashboard pivot widget now inherits this fix directly: the bucket bar renders under the localized label and the drill matches it.xOfreasoning used here.Red-first
Unit (
packages/core), new expectations againstorigin/main's source:DOM (
packages/plugin-charts), same revert:The three that stayed green pre-fix are the must-not-change controls (non-null groups, non-null drill, empty result) — green on both sides, as they should be. #4498's own single-dimension pins are green on both sides too (19/19 under the revert), which is what shows the fix isolates to the pivot branch.
The deliberate-pin update
PR #4498 pinned the pivot's null-drop as-is, not as correct — its case read "leaves the multi-dimension pivot branch exactly as it was" and asserted
{status: null, Low: 3}— precisely so that changing it would have to be deliberate. That pin is updated in the same commit as the fix, citing #4497, which is what it existed for.Verification
pnpm exec vitest run packages/core/ packages/plugin-charts/— 108 files, 1942 passedpnpm exec vitest run packages/plugin-dashboard/ packages/plugin-report/ packages/react/— 94 files, 1034 passed (consumer sweep, fix(plugin-dashboard): DatasetWidget localizes the null-bucket label (#4500) #4509 included; no plugin-dashboard test pinned the chart pivot's null-drop — thenull-dimension cases there belong tobuildPivot, the pivot table)tsc --noEmitandtsc -p tsconfig.test.jsonfor@object-ui/core,tsc --noEmitfor@object-ui/plugin-chartsand@object-ui/plugin-dashboard— all exit 0eslinton both packages — 0 errorsnode scripts/check-control-bytes.mjs— OK (4219 files), plus a direct control-byte scan of every touched filecheck-changeset-fixed/check-changeset-no-major/check-changeset-presence— all OKFindings filed while measuring this card
hasNoCategoryKeyguard can never fire for a pivoted chart (observation-class; unchanged by this PR, and the reason a key-absent row is deliberately not bucketed here)Closes #4497
Generated by Claude Code