Uh oh!
There was an error while loading. Please reload this page.
fix(core): draw a null second-dimension group instead of orphaning its measure - #4681
Merged
Merged
Conversation
…s measure `buildChartSeries`' pivot branch kept the pre-objectui#4466 answer on the SECOND dimension: `String(row[groupKey] ?? '')` behind a `gId !== ''` gate, so a null (or empty-string) group never joined `seriesKeys` — while the next line still wrote its measure into the bucket under the `''` key. The number was in the emitted row and bound to no mark, which is #4466's harm verbatim one dimension over: the chart understated its own data without saying so. Two such groups were worse than unbound — null and `''` both keyed `''`, so the later one silently overwrote the earlier one's measure. A known-empty group now DRAWS, exactly as on the first dimension, while an unprojected key still refuses (framework#4033): a row that does not carry the group key gets no bucket and contributes no column, where it used to write its measure under `''`. `pivotSeriesBuckets` is the one place that decides what a group IS, what it SAYS and which column carries it, and both helpers go through it. A series key is a row column and a renderer `dataKey`, not a private map key, so it must be unique within the emitted row: a bucket keys by its own label, unless that label is shared with another bucket (objectui#4508's collision 2, on this axis), reserved by the row itself (`xKey`, `CHART_BUCKET_ID_KEY`), or equal to some bucket's identity — then it keys by its identity, which no other bucket has. That third exclusion is what makes the assignment injective rather than merely unlikely to collide. An ordinary pivot's rows and series are unchanged, so no encoded key reaches an authoring surface. `findChartSeriesRow` resolves a clicked series key back through the same assignment and matches on IDENTITY. `String(r[gDim] ?? '')` was the display matching objectui#4508 removed on the x-axis for spelling a null group and an empty-string group alike; premise C of the card is to extend that answer, not re-widen the tolerance. The empty-string group keys by `''`. That recharts binds a falsy `dataKey` is a fact about recharts, so it is measured at the DOM rather than assumed. Fixes#4673 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ
The latest updates on your projects. Learn more about Vercel for GitHub. |
This was referenced Aug 15, 2026
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 15, 2026 08:43
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Fixes#4673
buildChartSeries' pivot branch kept the pre-#4466 answer on the seconddimension. It bucketed groups by
String(row[groupKey] ?? '')behind agId !== ''gate, so a group whose second dimension isnull,undefinedor''never joinedseriesKeys— while the line below the gate still wrote itsmeasure into the emitted row under the
''key. The number was in the data andbound to no mark, which is #4466's harm verbatim one dimension over: the chart
understated its own data without saying so.
Measured, before the fix
The card's repro, run through the transform and then through the real renderer:
The 40 hours are in the row, accommodated by the y-axis scale, and painted on
nothing. Two such groups were worse than unbound:
nulland''both key'',so the later group silently overwrote the earlier one's measure and one of the
two numbers did not survive the transform at all (measured:
series: [], zerobars, for a fixture with both).
What changed
A known-empty group draws; an unprojected key still refuses. That is the
split the first dimension already used (#4466 versus
hasNoCategoryKey,framework#4033), now answering the same way on both dimensions. A row that does
not carry the group key at all gets no bucket and contributes no column, where
it previously wrote its measure under
''.nulland''are two differentgroups with two series, following #4508's collision-1 ruling.
One place decides what a group is, what it says, and which column carries
it.
pivotSeriesBucketsreturns{ id, label, dataKey }per group and bothhelpers go through it, so the writer and the reader cannot drift.
The series key is collision-safe, not merely unlikely to collide. Unlike an
axis bucket's private map key, a series key is a column of the emitted row and
the
dataKeya renderer binds to, so it must be unique within that row. A groupkeys its column by its own display label — leaving an ordinary pivot's rows,
series, legend, tooltip and drill title exactly as they were — unless that label
cannot name it:
The chart null-category bucket has two identity collisions: it merges a null group with an empty-string group, and collides with a stored value spelling the label #4508's collision 2, on this axis);
CHART_BUCKET_ID_KEYcarrier);
Those key by identity instead, which no other group has. The third exclusion is
what makes the assignment injective rather than merely unlikely to collide: no
surviving label is any group's identity, so the two key spaces cannot meet. A
record whose stored group value literally spells
[null]is a real value with areal bar, and it keeps it.
Drill-through follows the same assignment. A clicked series key resolves back
to the group identity it names, and rows are matched on that.
String(r[gDim] ?? '')was the display-string matching #4508 removed on thex-axis — it spelled a null group and an empty-string group alike, so the
empty-string group's segment resolved to the null group's records. This is the
card's premise C: extend the identity answer, do not re-widen the tolerance.
No renderer change was needed. The null group's series carries the same
nullCategoryLabelthe renderers already pass for the first dimension, so theexisting plumbing (#4466 / #4497) covers it. The
handleCartesianClickregionheld by #4672 is untouched — see "Not addressed here" below.
The bounding sweep, named
Two keys beyond the card's own list are treated as reserved, because the
assignment's whole job is to be injective into the emitted row's key space and
that space already contains them:
dimension's name (
priority: 'status'underdimensions: ['status', 'priority']) would, keyed by its display string, overwrite the axis value onits own bucket row — the bar drawn against a category that reads as a number.
CHART_BUCKET_ID_KEY.The chart null-category bucket has two identity collisions: it merges a null group with an empty-string group, and collides with a stored value spelling the label #4508 writes the identity onto an ambiguous bucketrow after the series columns, so a series holding that key would be silently
overwritten by the identity: a lost measure and a phantom series.
Both are pinned by their own cases. Leaving a known hole in a function whose
entire purpose is collision-safety was the alternative.
Verification
Gate union run at
a6a2282bb, which is this branch's head:pnpm exec vitest run packages/core/ packages/plugin-charts/ packages/plugin-dashboard/— 171 files, 2497 tests, all passing.
pnpm --filter @object-ui/core --filter @object-ui/plugin-charts --filter @object-ui/plugin-dashboard --workspace-concurrency=2 run type-check— clean (scope confirmed non-empty: "3 of 47 workspace projects", each running
a real
tsc, with the dependency closure built first).eslint --quieton the three changed files — clean.check-changeset-presence/check-changeset-no-major/check-changeset-fixed/check-control-bytes— all green.Reverse verification, direction predicted before running
Writer ablated (the
gId !== ''gate restored). Predicted red in thebuildChartSeriesblocks and the DOM file, green in the reader-only drillcases. Measured exactly that: 14 red / 50 green in core, and 3 red / 1 green at
the DOM. The drill cases that do not derive their key from the writer —
resolves the null group's segment to the null row,resolves the empty-string group to ITS row,never matches a row that lacks the second dimension key—stayed green, which is the honest statement that the reader half is
independently correct. Every first-dimension suite (#4466 / #4497 / #4508)
stayed green, confirming the change is confined to the second dimension.
Reader ablated (
String(r[gDim] ?? '') === srestored). Predicted red in thedrill block only. Measured 7 red / 57 green — including
expected 1 to be 2,which is the wrong-row failure rather than a dead one: under display-string
matching the empty-string group's click resolves to the null group's row. The
ordinary-series drills and the #4497 / #4508 pivot drill suites stayed green.
The DOM leg ran against a rebuilt
dist/in both directions, and the mutationwas proved to have reached the artifact before each run (4 ablation markers
present when ablated, 0 after restore, so no mutated code stays live for later
runs).
Not addressed here
activePayload, which recharts 3 does not send — every cartesian drill loses its series and value, killing the pivoted drill outright #4672 remains open and its region is untouched. ItshandleCartesianClickcurrently forwardsap?.dataKey ? String(ap.dataKey) : undefined,and
''is falsy — so clicking the empty-string group's bar sendsseries: undefined, whichString(seriesKey ?? '')lands back on''hereand resolves correctly. The two halves agree by arithmetic rather than by
design; that constraint is pinned in a core test and noted on AdvancedChartImpl's cartesian click reads
activePayload, which recharts 3 does not send — every cartesian drill loses its series and value, killing the pivoted drill outright #4672 forwhoever rewrites that handler.
identity,
DatasetWidget.handleChartDrillputs the raw series key into thedrawer title, so it would read
[null]rather than(None). The drillresolves to the right rows; only the title text is opaque. Carrying a series
label on
ChartSegmentClickEventwould mean editing the heldhandleCartesianClickregion, so it is filed rather than done.phantom
''column, but nothing announces it the wayhasNoCategoryKeydoesfor the first dimension. Filed; related to buildChartSeries' pivot branch writes the category key onto every bucket, so framework#4033's hasNoCategoryKey guard can never fire for a pivoted chart #4507.
Generated by Claude Code