fix(charts): a fieldless count aggregate keyed its value column undefined (framework#3701) - #2878
Merged
Merged
Conversation
…ined (framework#3701) framework#3701 pinned down what an OBJECT-bound chart aggregate names its result columns: the raw field names it was given — `groupBy` for the category, `field` for the value, with no `sum_`-style decoration (unlike a dataset measure) — plus the literal `count` when a `count` omits `field`, which is the alias the engine projects COUNT(*) under. `os validate` now lints page sources against that convention, so every path that builds these rows has to honour it exactly. Three of the four did. The odd one out was `count`, the one function that may legitimately omit `field`, because every row builder read `params.field` directly: `aggregateRecords` and `ObjectDataSource.aggregateClientSide` emitted a column literally named `undefined`, and the legacy analytics path remapped the server's `count` measure onto that key and deleted the original — throwing away the value the server did return. All of them now resolve the column through one helper (`aggregateValueKey`), so a fieldless count lands under `count`. The comparison-overlay column derives from the same key (`count__comparison`, not `undefined__comparison`), and `aggregate.field` is typed optional to match the spec's ChartAggregateSchema. Charts that name a field are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UMHZBHTjH4rw8xmDYirFC7
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
July 28, 2026 00:32
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
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.
Companion to objectstack-ai/objectstack#3725 (framework issue #3701).
Context
framework#3701 pinned down what an object-bound chart aggregate names its result columns: the raw field names it was given —
groupByfor the category,fieldfor the value, with nosum_-style decoration (unlike a dataset measure) — plus the literalcountwhen acountomitsfield, which is the alias the engine projectsCOUNT(*)under.os validatenow lints react page sources against that convention, so every path that builds these rows has to honour it exactly.The bug
Three of the four paths did. The odd one out was
count— the one function that may legitimately omitfield— because every row builder readparams.fielddirectly:aggregateRecords(plugin-charts) andObjectDataSource.aggregateClientSide(data-objectstack) emitted{ [groupBy]: key, [undefined]: value }, i.e. a column literally namedundefinedthat no axis binding could ever name;countmeasure ontoparams.fieldand deleted the original key, throwing away the value the server had returned before the chart ever saw it.Net effect:
<ObjectChart aggregate={{ function: 'count', groupBy: 'status' }} />rendered axes and no bars.The fix
All of them now resolve the column through one helper (
aggregateValueKey), so a fieldless count lands undercount:plugin-charts—aggregateValueKey+COMPARISON_SUFFIXexported fromObjectChart.tsx; the structured-groupByalias, the comparison-overlay merge, and the synthesized comparison series all derive from it (count__comparisoninstead ofundefined__comparison).data-objectstack— the analytics remap, the missing-measure guard, andaggregateClientSideall use the same rule.aggregate.fieldis typed optional throughout, matching the spec's newChartAggregateSchema.Charts that name a field are unchanged —
aggregateValueKeyreturnsfieldwhenever it is present.Verification
ObjectChart.aggregateResultColumns.test.tspins the convention, including the regression: a fieldless count must not produce a key namedundefined.plugin-charts+data-objectstack: 232 tests pass (20 files).type-checkclean on both packages.🤖 Generated with Claude Code
https://claude.ai/code/session_01UMHZBHTjH4rw8xmDYirFC7
Generated by Claude Code