Repository navigation
buildChartSeries' pivot branch writes the category key onto every bucket, so framework#4033's hasNoCategoryKey guard can never fire for a pivoted chart #4507
Description
Activity
Concentrated finding round (unified triage seat, 2026-08-17): promoted to
pm:queue, typeTask. Restores a guard invariant:hasNoCategoryKeyexists to catch the framework#4033 shape, and the pivot branch structurally defeats it, so the defense is dead exactly where the defect it guards against would land. PM-suggested route is the card's first option (pivot preserves key-absence so one guard keeps one meaning) — but the card's own precondition is binding: first measure how a dataset query actually reports an unprojected dimension in a 2-dim grouping; if the measurement contradicts the route, report the fork rather than forcing it. The pinned limit inchart-series.nullCategory.test.tsmust keep its meaning ((None)-relabel stays refused for key-absent rows). S–M / opus.
Generated by Claude Code
Claim: PM loop round 12 (refill)
Session:session_01RV6yuVCxymHYE16PL9vQkE
Branch:claude/issue-4507-pivot-preserves-key-absence
Worktree:objectui-issue-4507
Domain:repo:objectui(core / plugin-charts)
File surface:packages/core/src/utils/chart-series.ts(the pivot branch) + its tests;packages/plugin-chartsonly if the measurement forces route 2 — and that is a fork to report, not a widening to take (stop on breach; explain in the report)
Container & model: M,mode:subagent,model: opus(triage suggested S–M / opus)
Clause-②: no — restores a guard invariant; accept set unchanged, no public surface widened.
Serial constraints cleared:packages/core/srcandpackages/plugin-chartsare both free. No open PR or in-flight claim touches either — the four other in-flight cards are inapp-shell/src/providers(#5243),packages/components(#5253),packages/plugin-report(#5225), andcontent/docs/core(#5136). Note #5136 iscontent/docs/core/**, notpackages/core— different tree, no overlap.⚠️ Card age is itself a premise trigger: this was filed 2026-08-12, six days ago, and measured during #4497 (PR4506, since merged). The dev is instructed to re-derive the whole chain on currentorigin/mainbefore editing —buildChartSeries' pivot branch,hasNoCategoryKey's predicate, and the pinned limit — rather than trusting any line in the card.Triage's binding precondition is carried into the dispatch order verbatim: measure first how a dataset query actually reports an unprojected dimension in a 2-dimension grouping; if that measurement contradicts route 1, report the fork rather than forcing it.
Generated by Claude Code
- added a commit that references this issue
on Aug 19, 2026 { "issue": 4507, "status": "done", "branch": "claude/issue-4507-pivot-preserves-key-absence", "pr": "https://github.com/objectstack-ai/objectui/pull/5281", "premise_still_valid": true, "summary": "Re-derived on current origin/main (d871f8e05): the chain still holds unchanged — the pivot branch wrote `[xKey]` onto every bucket via `byX.set(xId, { [xKey]: isNullCategory(row, xKey) ? nullLabel : xRaw })`, `hasNoCategoryKey`'s predicate (AdvancedChartImpl.tsx:1404) still reads `key in row` over `props.data`, and the pinned limit still stood in chart-series.nullCategory.test.ts. The card's binding precondition was measured FIRST and CONFIRMS route 1 (no fork): an unprojected dimension is reported by OMITTING the key. Route 1 implemented — an emitted bucket carries the axis key exactly when some source row did, which is `hasNoCategoryKey`'s own `rows.some(key in row)` lifted through the pivot's aggregation. A bucket that also collects a key-carrying row (an absent value and a stored null share the `[null]` identity) keeps its `(None)` label and draws, so #4466/#4497's answer is untouched. `packages/plugin-charts` not touched. The pinned limit did NOT fail: its `(None)`-refusal meaning is intact and now tightened from `toEqual` (blind to undefined-valued keys) to `toStrictEqual` + an explicit `'status' in` assertion; only its stale limit note was rewritten.", "tests": "MEASUREMENT (precondition, run before any edit, through the real ObjectStackAdapter with a stubbed server answering the framework#4033 payload for dimensions ['status','priority']): `'status' in rows[0] : false` / `Object.keys(rows[0]) : ['priority','est_hours']` — key OMITTED; after buildChartSeries `Object.keys(data[0]) : ['status','High','Low']`, `'status' in data[0] : true`, `data[0].status : undefined`; guard fires on RAW rows `true`, on PIVOTED `false`. Producer half read at source: objectql-strategy writes a dim key only under `if (shortName in row)`, native-sql returns driver rows verbatim, JSON cannot carry `undefined`, adapter passes rows by reference; framework pins it as `expect(row).not.toHaveProperty('due_date')` and #4033 measured `[{count:2},{count:8}]` live. GATES at 02779e625 (union re-derived after the final commit, tree == HEAD): `pnpm --filter '@object-ui/core^...' build` green; `pnpm exec vitest run packages/core/ packages/plugin-charts/` → Test Files 121 passed, Tests 2132 passed; `pnpm exec vitest run packages/plugin-dashboard/ packages/plugin-report/ packages/data-objectstack/` → 117 files, 1280 tests passed (all buildChartSeries consumers); `pnpm --filter @object-ui/core type-check` green (script name echoed, not a zero-match); eslint on the 3 changed files green; check-control-bytes / check-changeset-presence / check-changeset-no-major / check-changeset-fixed / check-type-check-coverage / check:esm-specifiers / check:self-import / check:phantom-deps all green. REVERSE VERIFICATION: ablation = restore pre-fix chart-series.ts from origin/main, keep the new tests. NO REBUILD is in the resolution path — the tests import `./chart-series.js`, which vitest resolves to packages/core/src/utils/chart-series.ts (source), and vitest.config.mts aliases @object-ui/core to packages/core/src; there is no dist and no package-exports hop, so a stale build cannot hold the result, and the ablation turning red is itself the proof the run reads the edited source. Predicted 5 red BY NAME before running; observed exactly those 5 (`Tests 5 failed | 73 passed (78)`, `AssertionError: expected false to be true` on the guard assertion), with the 7 predicted-green controls green — including 'does NOT relabel the key-absent bucket (None)' (already true pre-fix) and 'order does not decide it' (old code was correct in that direction only). Restore leg: `git status` and `git diff HEAD` both empty (tree byte-identical to the commit), suites back to Test Files 3 passed / Tests 88 passed.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
Generated by Claude Code
PR #5281 is up. CI running; I'll accept when it converges. Recording the two things here that outlive this card.
The binding precondition was measured, and it confirmed the route
Triage made route 1 conditional on measuring how a dataset query actually reports an unprojected dimension in a 2-dim grouping, because omitting
[xKey]only helps if key-absence is genuinely the shape that arrives. It was measured four ways before a line was written:- the ObjectQL strategy writes a dimension key only under
if (shortName in row)— pinned framework-side asexpect(row).not.toHaveProperty('due_date'); - the native-SQL strategy returns driver rows carrying only the selected columns;
- JSON cannot transport
undefined, so nothing between server and chart can turn "no column" into a present-but-undefined key; ObjectStackAdapter.queryDatasetpasses rows through by reference.
Then end-to-end through the real adapter with a stubbed server answering the framework#4033 payload:
'status' in rows[0]→ false; afterbuildChartSeries→ true, valueundefined. The guard fires on the raw rows and not on the pivoted ones. The measurement confirmed route 1, so the fork clause did not fire andpackages/plugin-chartswas never touched.⭐ Why this defect survived four cards' worth of pins over the same rows
This is the part worth carrying elsewhere.
toEqualdrops an undefined-valued key, and so doesJSON.stringify. So{ status: undefined, Low: 3 }and{ Low: 3 }read as identical under the assertion this file had been using — whilekey in row, which is the guard's entire signal, tells them apart.Four cards (#4466 / #4497 / #4508 / this one) pinned these exact rows. None could see the bug, because the matcher they all reached for is blind on precisely the axis that mattered. The pinned "measured limit" in
chart-series.nullCategory.test.tswas itself written withtoEqual— it recorded the limit while being structurally unable to detect it changing.A pin is only as sharp as its matcher. When the property under test is key presence rather than key value,
toEqualis not a weaker assertion — it is the wrong one. The new tests sayin/Object.keys/toStrictEqualout loud for that reason.The pin was updated, not deleted — and its meaning held
Its
(None)-refusal assertion is unchanged: a key-absent row is still never relabelled(None), because "this dimension was never projected" and "these records have no value" are different sentences. Only the row-shape expectation moved, because the behaviour it described moved.The nuance the fix had to get right:
chartBucketIdencodes an absent value and a storednullidentically ([null]), so one bucket can collect both. Rule adopted — a bucket carries the key exactly when some row of it did. If any row carried it, the bucket is "known to be empty" and draws under the label; refusing there would tell an author their query never projected a dimension that it demonstrably did. That is the mirror image of #4497's refusal, so both halves of the doctrine now live in the same branch.
Generated by Claude Code
- the ObjectQL strategy writes a dimension key only under
ACCEPT — PR #5281 flipped ready, enqueued, merged. All 21 checks resolved: 19 success, 2 skipped by design. Path surface:
packages/core/src/utils/chart-series.ts+ two test files + the changeset.packages/plugin-chartsuntouched, which is the mechanical proof that route 1 held and the fork never arose.The binding precondition did its job. Triage made the route conditional on measuring how a dataset query actually reports an unprojected dimension — because omitting
[xKey]only helps if key-absence is genuinely the arriving shape. Measured four ways, and end-to-end through the realObjectStackAdapterwith a server answering the framework#4033 payload:- ObjectQL strategy writes a dimension key only under
if (shortName in row)— framework-side pinned asexpect(row).not.toHaveProperty('due_date') - native-SQL returns driver rows carrying only selected columns
- JSON cannot transport
undefinedat all - the adapter passes rows through by reference
'status' in rows[0]→ false; afterbuildChartSeries→ true, valueundefined. The route was confirmed, not assumed.⭐ The finding worth carrying past this card
This defect survived four cards' worth of pins over the same rows — because the pins used the wrong matcher.
toEqual(andJSON.stringify) drop an undefined-valued key. So{ status: undefined, Low: 3 }and{ Low: 3 }read as identical under every assertion this file had — whilekey in row, the guard's entire signal, distinguishes them. The "measured limit" pin that recorded this very limitation was itself written withtoEqual: it documented the constraint while being structurally incapable of noticing the constraint change.The new assertions say
in/Object.keys/toStrictEqualout loud, and the old pin was updated rather than deleted — its(None)-refusal meaning is intact, tightened, and its stale limit note rewritten.A pin is only as sharp as its matcher. When the property under test is whether a key exists rather than what its value is,
toEqualis not a weaker assertion — it is the wrong one.The nuance the fix got right
chartBucketIdencodes an absent value and a storednullidentically ([null]), so one bucket can collect both. The rule landed is: a bucket carries the key exactly when some row of it did — which ishasNoCategoryKey's ownrows.some(key in row)lifted through the pivot's aggregation. If any row carried it, the bucket is "known to be empty" and draws under its label; refusing there would tell an author their query never projected a dimension it demonstrably did. That is the mirror of #4497's refusal to relabel key-absent rows(None)— both halves of the doctrine are now live in the same branch.
Generated by Claude Code
- ObjectQL strategy writes a dimension key only under
- added a commit that references this issue
on Aug 23, 2026
Measured while implementing #4497 (PR #4506); filed unassigned rather than widened into that card's ruled surface.
What
hasNoCategoryKey(plugin-charts'AdvancedChartImpl) exists to catch the framework#4033 shape — a dimension grouped by but never projected, so no row carries the category key at all. Instead of drawing an axis with no marks, it renders an explanatory placeholder naming the missing key:That predicate reads
props.data, which for a dataset-bound chart isbuildChartSeries' output. The pivot branch always writes the key onto every bucket it creates:so
key in rowis unconditionally true downstream, and the guard cannot fire for a 2-dimension/1-measure chart regardless of what the query returned.Effect
A pivoted chart whose first dimension was never projected collapses every row into one unnamed bucket and draws an axis with an empty category label — the exact silent shape framework#4033 introduced the placeholder to eliminate. The single-dimension branch is unaffected: it passes rows through, so key-absent rows stay key-absent and the guard sees them.
Status
Pre-existing and unchanged by #4497 — that card deliberately kept the key-absent shape out of the pivot's bucket (
isNullCategoryrequireskey in row), so a never-projected dimension is not relabelled(None), which would state something false about the data. Pinned as a measured limit inpackages/core/src/utils/chart-series.nullCategory.test.ts, case "does NOT bucket a row that lacks the category key ENTIRELY".Shape of a fix (not prescriptive)
Either the pivot preserves key-absence (omit
[xKey]from a bucket whose source rows never carried it, so the existing guard sees it), orhasNoCategoryKeylearns the pivot's shape. The first keeps one guard with one meaning; the second spreads the pivot's internals into the renderer. Needs its own measurement of how a dataset query actually reports an unprojected dimension in a 2-dimension grouping.Observation-class: nothing a user hits unless a dataset groups by a dimension it does not project, which is itself the framework#4033 defect.
Generated by Claude Code