Repository navigation
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
Description
Activity
- added a commit that references this issue
on Aug 13, 2026 Triage:
needs-user-decision— the filer explicitly asks for a ruling, and it's warranted: a real fix means giving chart buckets an identity distinct from their display label, which is a doctrine-level change to #4466's shape (shared by every chart consumer, currently pinned as "measured limits, not correct" inchart-series.nullCategory.test.ts).Four-axis read:
- Measured business pull: item 2 (stored value spelling the locale label, e.g. a literal
"(None)"or"(未指定)"row) can produce a genuinely wrong drill, not just a dead click — narrow trigger condition (byte-equal collision with the active locale's bucket label) but a real data-integrity-adjacent bug when it hits. Item 1 (null/empty-string merge) is currently a no-op click, strictly better than the pre-A NULL first-dimension value is still dropped by the multi-dimension pivot branch of buildChartSeries (the sibling of #4466) #4497 fully-invisible bar. - Platform long-term coherence: a sentinel bucket identity carried alongside the label is the coherent fix and answers both collisions with one change; patching around either symptom individually would add more special-casing to a utility that's already accreting it (A NULL-keyed group is dropped from a bar chart, leaving an axis with no marks and no empty state — the default first-boot state of System Overview's "Events by User" #4466 → A NULL first-dimension value is still dropped by the multi-dimension pivot branch of buildChartSeries (the sibling of #4466) #4497 pivot extension → this).
- AI-agent error-resistance: identity-vs-label conflation is exactly the shape of bug that's easy for an agent to re-introduce piecemeal; a real fix wants a single ruling and one PR touching
buildChartSeries/findChartSeriesRowtogether, not incremental patches per report. - Startup scope discipline: this is a bug-fix-shaped decision (restoring correct identity semantics), not a feature — low risk to rule on now rather than deferring, since deferral just means the next chart consumer inherits the same two collisions.
Recommendation: approve the sentinel-identity direction (distinct bucket key from display label, matching what
buildPivot's COLUMN-dimension handling already does per the filer's cross-reference) and dispatch as one card touching both functions plus their pinned tests.
Generated by Claude Code
- Measured business pull: item 2 (stored value spelling the locale label, e.g. a literal
- addedtarget:v17v17 发布窗口工作集(GA 前排查 2026-08-04)v17 发布窗口工作集(GA 前排查 2026-08-04)and removed
on Aug 14, 2026 Maintainer ruling (2026-08-14, verbatim: 「同意」, approving item 5 of the six-item ruling list: 「objectui#4508 批 sentinel-identity?」): approved — sentinel bucket identity carried alongside the display label, aligning
buildChartSeries(writes identity) andfindChartSeriesRow(reads it) with the table branch's existing distinct-bucket-id form. One combined dispatch covering both collisions (null+'' merge; stored literal "(None)" vs the null bucket).needs-user-decision→pm:queue,target:v17; enters the v17 bug-focus Seat B (objectstack-ai/objectstack#8668).
Generated by Claude Code
Claim: PM loop round 6
Session:session_01RnQd8iMMUwXQEV1crFmQiQ
Branch:claude/issue-4508-chart-bucket-identity
Worktree:objectui-issue-4508
Domain:repo:objectui
File surface:packages/core/src/utils/chart-series*(buildChartSeries+findChartSeriesRow+ the pinned-limits test) + the drill consumers that read the row back (per re-derivation; likely plugin-dashboard/plugin-charts drill path) (stop on breach; explain in the report)
Container & model: M,mode:subagent,model: opus
Serial constraints cleared: in-flight #4640 also touchespackages/corebut in the evaluator region (evalRowPredicateentry — disjoint files, mutually named); #4648 holds parity/registry surfaces, #4644 holds metadata-admin field designer, #8830 is objectstack-side — all disjoint; PR #4670 (merged) touchedcore/predicate-fields— work from post-merge origin/main; no open dev PR touches chart-series (checked this round).
Generated by Claude Code
{ "issue": 4508, "status": "done", "branch": "claude/issue-4508-chart-bucket-identity", "pr": "https://github.com/objectstack-ai/objectui/pull/4677", "premise_still_valid": true, "summary": "Both collisions fixed as one change, per the 2026-08-14 「同意」 ruling. `chartBucketId` (core) is the bucket identity — literally the encoder `buildPivot` already keys the pivot TABLE with (`pivotBucketId` over `pivotDimensionValue`), so the chart and table stop answering one question two ways; the pivot branch now buckets by it instead of by `String(xRaw ?? '')`, which is what makes null and '' two groups again. Where two DISTINCT buckets still paint the same axis text (the null bucket beside a record storing the literal label), the emitted row carries that identity under `CHART_BUCKET_ID_KEY` and `findChartSeriesRow` takes it back as an authoritative `options.bucketId`; it is written exactly and only there, so an ordinary chart's rows are still returned by identity and no renderer-internal key reaches the `chart` schema's `data`. Plumbing: the drill event gains `categoryId` via `ChartSegmentClickEvent`, now declared once in core instead of as three inline literals (AdvancedChartImpl / ChartRenderer / ObjectChart), read off the clicked row on the cartesian, pie and funnel paths and consumed by `DatasetWidget.handleChartDrill`. Two measurements shaped the design and are worth the PM's attention: recharts builds a pie sector's `payload` as a SPREAD COPY of the row, so the identity had to be an ordinary enumerable property (a symbol or non-enumerable one would read as 'no identity' with every pure test still green); and recharts 3's chart-level onClick sends a `MouseHandlerDataParam` with no row at all, so the cartesian arm reads its row out of our own `data` by `activeTooltipIndex`. Also flipped, as a consequence of ruling item 1: a null category no longer reads as `''` too — no producer of this lookup's `category` writes that spelling, while `''` IS a genuine empty-string group's axis text, so the tolerance was handing that group's bar another group's records.", "tests": "All at f3e14d59a (final commit; gate union re-run after it). `pnpm exec vitest run packages/core/ packages/plugin-charts/ packages/plugin-dashboard/` -> 'Test Files 170 passed (170) / Tests 2463 passed (2463)'. `turbo run type-check lint` for the three packages with the dependency closure built -> 'Tasks: 19 successful, 19 total', 0 lint errors; type-check confirmed executed ('> tsc --noEmit && tsc -p tsconfig.test.json'). Gate scripts all OK: changeset-fixed, changeset-no-major, changeset-presence, control-bytes, spec-symbol-derivation, phantom-dependencies, i18n-call-site-keys, i18n-en-drift, lint-coverage, type-check-coverage, skills-paths. New coverage at three layers: the transform (chart-series.nullCategory.test.ts, rewritten limits describe + a new identity-contract describe), the chart library seam (AdvancedChartImpl.bucketIdentity.test.tsx — clicks a REAL pie sector, which is the only way the spread-copy hazard is observable), and the consumer (DatasetWidget.chartBucketIdentity.test.tsx — asserts which records the drawer opens on). REVERSE VERIFICATION, directions predicted before running, split into two legs for attribution: ablating the WRITER (pivot bucket key + identity write) predicted both collisions' bucketing plus every identity-carrier assertion red and the reader-only cases green -> observed 9 failed / 39 passed, exactly the predicted set; ablating the READER (exact display matching) predicted only the empty-string cases red -> observed 4 failed / 44 passed, the predicted set. Restored via path-scoped `git checkout` of the commit sha between legs (never a stash); final numbers are from the restored tree. CROSS-PACKAGE TYPE reverse check: renaming ev.categoryId to a non-existent field in DatasetWidget yields 'error TS2339: Property categoryIdNoSuchField does not exist on type ChartSegmentClickEvent' — the error names the type from the freshly built core .d.ts, proving plugin-dashboard checked against the rebuilt declaration, not a cached one. No ablation was run against dist/ (no dogfood leg applies to this card). Consumer sweep direction: downstream — plugin-charts and plugin-dashboard are the consumers of core's changed contract, and both are in the suites above.", "open_questions": [], "out_of_scope_findings": [ "filed as #4672: AdvancedChartImpl's cartesian click reads `activePayload`, a recharts 2 field that recharts 3 does not send (its MouseHandlerDataParam has activeIndex/activeTooltipIndex/activeLabel/activeDataKey only), so `series` and `value` are ALWAYS undefined — which kills the pivoted dataset drill outright, since findChartSeriesRow's pivot arm requires the series key. Independent of this PR and pre-existing; not fixed here because the correct value for activeDataKey under a shared cartesian tooltip is not pinned by existing evidence (in-place exemption condition 2 unmet). Worth PM attention as it may outrank this card in user impact.", "filed as #4673: buildChartSeries' pivot branch drops a null SECOND-dimension group's series entirely (the `gId !== ''` gate), while still writing its measure into the bucket — the value is in the data and on no mark. The #4466 defect one dimension over; needs its own ruling because a series key is also a row key (collision risk with measure names), so it is not mechanical." ] }Notes for the PM, outside the JSON:
- File surface as claimed, plus two the claim anticipated. Claimed:
packages/core/src/utils/chart-series*and the drill consumers. Actual:core/src/utils/chart-series.ts+ itsnullCategorytest,plugin-charts/{AdvancedChartImpl,ChartRenderer,ObjectChart}.tsx+ a new test,plugin-dashboard/DatasetWidget.tsx+ a new test, one changeset.ChartRenderer.tsxandObjectChart.tsxare in scope only as the two forwarding hops between the renderer that builds the click event and the widget that consumes it — the claim's "drill consumers that read the row back (per re-derivation)". No sibling in-flight region was touched: nothing undercoreevaluator ([app-shell] ActionParamDialogfilterVisibleParamsresolves a broken paramvisibleSILENTLY, and in a direction decided by the predicate's dialect #4640), metadata-admin field designer (console: field designer still offers an "Indexed" toggle for a key FieldSchema rejects by name — ticking it hard-blocks the save #4644), or parity/registry ([Decision] spec 17.0.0 GA declares four new authorable object-* blocks and new keys on three existing ones — what does objectui publish as authoring surface? #4648);core/predicate-fields(merged fix(core,app-shell): project every declared recordIdField, and refuse an action that names no record #4670) untouched. - Base drift: branched from
b1119ece4;mainhas since advanced to541c72425(test(parity): adapt the spec parity gates to 17.0.0 GA (ruled items 1-5); fork the four-block exemption #4671, parity gates). It touches none of this PR's files. - Labels read back after bots settled:
package: core,plugin,plugin: charts,tests.skip-changesetdeliberately NOT applied — this is a behaviour change in three published packages and carries aminorchangeset. - Gate list re-derived against the actual changed paths rather than taken from the dispatch prompt: the prompt named vitest +
type-check; I added the changeset trio, control-bytes, spec-symbol-derivation, phantom-dependencies, both i18n gates, lint + lint-coverage, type-check-coverage and skills-paths, all green. - Reporting at draft-PR time per the standing contract; CI convergence is the PM's read.
Generated by Claude Code
Generated by Claude Code
- File surface as claimed, plus two the claim anticipated. Claimed:
ACCEPT — PR #4677 (reviewer of record; readings taken on GitHub at head
f3e14d59a, which matches the gate-union HEAD in the report and PR body).- Shape: draft →
main, first lineFixes #4508— correct: both ruled collisions land in this one PR. Body re-scanned for closure keywords:Fixes #4508is the only one; AdvancedChartImpl's cartesian click readsactivePayload, which recharts 3 does not send — every cartesian drill loses its series and value, killing the pivoted drill outright #4672 / buildChartSeries' pivot branch silently drops a null SECOND-dimension group: its measure is written to the bucket but never becomes a series, so the value vanishes from the chart #4673 are referenced bare (and explicitly stated as NOT addressed / still open — the safe spelling). - Scope (from the 9 changed files):
chart-series.ts+ its test (the claimed core region), the drill-event plumbing (AdvancedChartImpl/ChartRenderer/ObjectChart/DatasetWidget— the consumer half the card itself names), two new tests at the seam and consumer layers, and a real changeset. No file outside the card's surface; no overlap with any in-flight sibling region. - The boundary question this card lives or dies on — answered with a measurement, not an assumption: recharts builds a pie sector's payload as a spread copy, so the identity is an ordinary enumerable property, and a real-sector DOM click test exists precisely because every pure test would stay green if that hazard regressed. That is the "does the benefit survive the mandatory boundary" check done right. The cartesian path reads the row out of our own
databyactiveTooltipIndex, so no stale-copy risk there. - Authoring-surface containment:
CHART_BUCKET_ID_KEYis written exactly and only where two distinct buckets paint the same axis text; ordinary charts' rows are returned by identity — no renderer-internal key reacheschart.data. The identity encoder is the same onebuildPivotalready uses, so chart and table stop answering one question two ways — the ruled direction, delivered. - Behavior change reviewed and accepted:
''no longer resolves to a null-valued row — the re-measurement stands (no producer of this lookup'scategorywrites that spelling;''IS a genuine empty-string group's axis text, so the old tolerance handed one group's bar another group's records). The A NULL first-dimension value is still dropped by the multi-dimension pivot branch of buildChartSeries (the sibling of #4466) #4497 pins were explicitly "measured limits, not correct"; flipping them with the re-measurement recorded is the intended lifecycle of such pins. - Evidence: 170 files / 2463 tests green across the three packages; 19 turbo tasks green with the closure built and
tscexecution confirmed; two-leg reverse verification with per-leg predicted failure sets matching observed exactly (9/39 writer, 4/44 reader — attribution clean); cross-packageTS2339probe proves the rebuilt.d.tswas consumed.ChartSegmentClickEventdeclared once in core kills the three-inline-literals drift trap. - Out-of-scope findings: 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 and buildChartSeries' pivot branch silently drops a null SECOND-dimension group: its measure is written to the bucket but never becomes a series, so the value vanishes from the chart #4673 — both graded this round (pm:queue+target:v17; buildChartSeries' pivot branch silently drops a null SECOND-dimension group: its measure is written to the bucket but never becomes a series, so the value vanishes from the chart #4673 carries a delegated direction, AdvancedChartImpl's cartesian click readsactivePayload, which recharts 3 does not send — every cartesian drill loses its series and value, killing the pivoted drill outright #4672 a premise-first dispatch note). The refusal to widen into AdvancedChartImpl's cartesian click readsactivePayload, which recharts 3 does not send — every cartesian drill loses its series and value, killing the pivoted drill outright #4672 was correct: in-place exemption condition 2 (correct form pinned by existing evidence) genuinely unmet.
CI on this head is converging — ready + auto-merge flips when Lint / Type Check / Test shards conclude green (idempotent timer armed). This card leaves the
target:v17board at close. Note for #4673's future dev: that card is queued behind THIS merge (same pivot branch ofchart-series.ts).
Generated by Claude Code
- Shape: draft →
- added 3 commits that reference this issue
on Aug 17, 2026
Measured while implementing #4497 (PR #4506) and pinned there as limits; filed unassigned rather than widened into that card. Both are properties of the #4466 doctrine itself, not of the pivot extension — the pivot inherits them.
1. A null group and an empty-string group merge into one bar (pivot branch)
buildChartSeries' pivot branch keys buckets byString(xRaw ?? ''), so a storednulland a stored''land in the same bucket and draw one bar. The bar takes its label from whichever row created the bucket, so with a null row first it renders(None)— and the drill for the segment sourced from the''row then finds nothing:handleChartDrillreturns on-1, so this is a no-op click, never a drill into the wrong records — and pre-#4497 the whole bar was invisible, so it is strictly more affordance than before, not a regression. It is still an inconsistency: a bar labelled "no value" carries a segment belonging to a group whose value is the empty string.Widening
findChartSeriesRowto match''rows against the null label was considered and rejected in #4497: it would make a bar labelled(None)drill to a raw row whose stored value is'', i.e. filter records by a value the label does not name — trading a dead click for a wrong one.The pivot table answers the same question differently, which is worth deciding together:
buildPivot(plugin-dashboard) gives a null dimension value its own bucket id, distinct from the literal placeholder character, and labels it with an em-dash. SeeDatasetWidget.test.tsx, "buildPivot keeps a null COLUMN-dimension value apart from the literal placeholder".2. A stored value spelling the label collides with the bucket (both branches)
A row whose stored category is the label string keeps its own bucket (its key is
'(None)', not''), so two bars carry the same axis text and the click resolves to the first:This one can produce a wrong drill: clicking the null bucket's bar resolves to the literal-valued row and filters on it. It is inherited unchanged from the single-dimension branch, where #4466 (PR #4498) shipped exactly this trade, and it applies to every localized label too (
(未指定)and the other nine packs).Dormant in practice: it needs a stored first-dimension value byte-equal to the active locale's bucket label.
Why they are one issue
Both are the same missing property — the bucket has no identity distinct from the display string — and any fix touches the same two functions as a pair (
buildChartSerieswrites the identity,findChartSeriesRowreads it back). A sentinel bucket identity carried alongside the label, rather than the label serving as its own key, would answer both; that is a doctrine-level change to #4466's shape and wants its own ruling.Both are pinned as measured limits (not as correct) in
packages/core/src/utils/chart-series.nullCategory.test.ts, describe "findChartSeriesRow — the measured limits of the bucket label (objectui#4497)", so a future change to either has to face them.Generated by Claude Code