From 4b7a8ce25f1e94d9713a2d45dee6124c8d68bf5c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 03:58:13 +0000 Subject: [PATCH 1/2] feat(grid): compute all eleven spec column summary aggregations (#2890) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ColumnSummarySchema` accepts eleven aggregation names; `useColumnSummary` computed five. The other six — `none`, `count_empty`, `count_filled`, `count_unique`, `percent_empty`, `percent_filled` — passed validation at authoring time and then rendered a blank footer cell, with no error raised on either side. objectstack#3761 widened the reachable surface further by promoting the `{ type, field }` object form, whose `type` reuses this enum. Implement the six, splitting the computation into two families: - count/percent read *raw* cell values, before the numeric parse, so they work on text, select and lookup columns and a value that does not parse as a number still counts as a filled row. A cell is empty when it is null, undefined, "" or an empty array — the convention already used by audit history display and form dirty-checking — so an unset multi-select reads as empty rather than as a filled `[]`. `count_unique` keys objects and arrays by value; a raw Set compares by reference and would call every row distinct. - sum/avg/min/max keep the existing numeric parse and column formatting. Two behavior changes fall out of the enum having both names: - `count` is now every row. `count_filled` is the non-empty variant, and with both in the enum they cannot mean the same thing. Only a column whose values are all empty renders differently than before. - a zero count renders "Empty: 0" instead of collapsing to a blank cell. Zero is the answer to "how many are empty", not the absence of one. Column currency/percent formatting is now gated to the numeric family in one place rather than scattered `type !== 'count'` guards, so `count_unique` on a currency column reads "Unique: 3" and not "$3.00". `none` and unrecognized names skip the entry entirely instead of registering a blank one, so a view whose columns all opt out renders no footer row. Widen the objectui-local `{ type, field }` arm of `ListColumnSchema` to take its vocabulary from `SpecColumnSummarySchema` by reference. It was stuck at the same five names, which left the per-column `field` override unavailable for the six new aggregations, and it collapses to zero delta when that spec release lands and the `.extend()` goes away. Guard both directions with a parity test: a spec name the renderer omits is the bug this commit fixes, and a renderer name the spec omits would be local dialect (Commandment #0). The footer summary was undocumented in both the package README and the plugin docs page; both now carry the full table and the emptiness rule. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V3EDGLKNvbcpDTLzyiAd3Q --- content/docs/plugins/plugin-grid.mdx | 45 ++++ packages/plugin-grid/README.md | 45 ++++ packages/plugin-grid/package.json | 1 + .../src/__tests__/column-features.test.tsx | 123 +++++++++++ .../src/__tests__/summary-spec-parity.test.ts | 44 ++++ .../plugin-grid/src/useColumnSummary.test.tsx | 49 +++++ packages/plugin-grid/src/useColumnSummary.ts | 207 +++++++++++++++--- .../__tests__/spec-subschema-parity.test.ts | 13 ++ packages/types/src/zod/objectql.zod.ts | 8 +- pnpm-lock.yaml | 3 + 10 files changed, 500 insertions(+), 38 deletions(-) create mode 100644 packages/plugin-grid/src/__tests__/summary-spec-parity.test.ts diff --git a/content/docs/plugins/plugin-grid.mdx b/content/docs/plugins/plugin-grid.mdx index 3709cea4d8..079aa2b5a4 100644 --- a/content/docs/plugins/plugin-grid.mdx +++ b/content/docs/plugins/plugin-grid.mdx @@ -62,6 +62,51 @@ interface GridColumn { } ``` +### Column Summaries + +A column can declare a footer aggregation with `summary`, either as a shorthand +string or as an object that aggregates a different field than the one displayed: + +```json +{ + "columns": [ + { "field": "name", "summary": "count_filled" }, + { "field": "amount", "type": "currency", "summary": "sum" }, + { "field": "owner", "summary": { "type": "count_unique", "field": "owner_id" } } + ] +} +``` + +The accepted values are `ColumnSummarySchema` from `@objectstack/spec`: + +| `summary` | Footer shows | Reads | +|---|---|---| +| `none` | nothing — the column opts out | — | +| `count` | number of rows | every row | +| `count_filled` | rows whose cell is non-empty | raw values | +| `count_empty` | rows whose cell is empty | raw values | +| `count_unique` | distinct non-empty values | raw values | +| `percent_filled` | share of rows that are non-empty | raw values | +| `percent_empty` | share of rows that are empty | raw values | +| `sum` | total | numeric values | +| `avg` | mean | numeric values | +| `min` | smallest | numeric values | +| `max` | largest | numeric values | + +A cell counts as empty when it is `null`, `undefined`, `""` or an empty array, +so an unset multi-select or lookup reads as empty rather than as a filled `[]`. + +The count and percent families read raw cell values, so they work on text, +select and lookup columns. `sum`/`avg`/`min`/`max` need numeric values (numeric +strings are parsed) and render nothing when the column has none. + +A `currency` or `percent` column formats its `sum`/`avg`/`min`/`max` in that +unit. Counts stay plain cardinalities and percentages carry their own `%`, so +`count_unique` on a currency column reads `Unique: 3`, not `$3.00`. + +The footer row renders only when at least one column resolves to a summary — a +view whose columns are all `none` (or carry no `summary`) has no footer. + ## Usage ### Auto-registration (Side-effect Import) diff --git a/packages/plugin-grid/README.md b/packages/plugin-grid/README.md index 4668663c5b..fcfa08ed3f 100644 --- a/packages/plugin-grid/README.md +++ b/packages/plugin-grid/README.md @@ -82,6 +82,51 @@ interface GridColumn { } ``` +### Column Summaries + +A column can declare a footer aggregation with `summary`, either as a shorthand +string or as an object that aggregates a different field than the one displayed: + +```json +{ + "columns": [ + { "field": "name", "summary": "count_filled" }, + { "field": "amount", "type": "currency", "summary": "sum" }, + { "field": "owner", "summary": { "type": "count_unique", "field": "owner_id" } } + ] +} +``` + +The accepted values are `ColumnSummarySchema` from `@objectstack/spec`: + +| `summary` | Footer shows | Reads | +|---|---|---| +| `none` | nothing — the column opts out | — | +| `count` | number of rows | every row | +| `count_filled` | rows whose cell is non-empty | raw values | +| `count_empty` | rows whose cell is empty | raw values | +| `count_unique` | distinct non-empty values | raw values | +| `percent_filled` | share of rows that are non-empty | raw values | +| `percent_empty` | share of rows that are empty | raw values | +| `sum` | total | numeric values | +| `avg` | mean | numeric values | +| `min` | smallest | numeric values | +| `max` | largest | numeric values | + +A cell counts as empty when it is `null`, `undefined`, `""` or an empty array, +so an unset multi-select or lookup reads as empty rather than as a filled `[]`. + +The count and percent families read raw cell values, so they work on text, +select and lookup columns. `sum`/`avg`/`min`/`max` need numeric values (numeric +strings are parsed) and render nothing when the column has none. + +A `currency` or `percent` column formats its `sum`/`avg`/`min`/`max` in that +unit. Counts stay plain cardinalities and percentages carry their own `%`, so +`count_unique` on a currency column reads `Unique: 3`, not `$3.00`. + +The footer row renders only when at least one column resolves to a summary — a +view whose columns are all `none` (or carry no `summary`) has no footer. + ## Examples ### Basic Grid diff --git a/packages/plugin-grid/package.json b/packages/plugin-grid/package.json index 2694230091..019133902d 100644 --- a/packages/plugin-grid/package.json +++ b/packages/plugin-grid/package.json @@ -38,6 +38,7 @@ }, "devDependencies": { "@object-ui/data-objectstack": "workspace:*", + "@objectstack/spec": "^16.0.0-rc.0", "@vitejs/plugin-react": "^6.0.4", "msw": "^2.15.0", "typescript": "^6.0.3", diff --git a/packages/plugin-grid/src/__tests__/column-features.test.tsx b/packages/plugin-grid/src/__tests__/column-features.test.tsx index 30aa7523de..9819858706 100644 --- a/packages/plugin-grid/src/__tests__/column-features.test.tsx +++ b/packages/plugin-grid/src/__tests__/column-features.test.tsx @@ -173,6 +173,129 @@ describe('useColumnSummary', () => { }); }); +// ========================================================================= +// Count and percent aggregation families +// +// `ColumnSummarySchema` accepts eleven aggregation names. The renderer used to +// compute five of them, so a view configured with e.g. `count_unique` passed +// validation and then rendered a blank footer cell. These cover the six that +// were missing, on raw (non-numeric) cell values. +// ========================================================================= +describe('useColumnSummary count/percent families', () => { + // 4 rows with deliberate holes: an empty string, a null, an empty array and + // a missing key — every shape the emptiness test has to recognize. + const sparseData = [ + { id: '1', name: 'Alice', dept: 'Eng', tags: ['a'] }, + { id: '2', name: 'Bob', dept: 'Eng', tags: [] }, + { id: '3', name: '', dept: 'Sales', tags: ['a'] }, + { id: '4', name: 'Dave', dept: null }, + ]; + const summaryFor = (field: string, type: string, data: any[] = sparseData) => { + const columns = [{ field, label: field, summary: { type } }] as any[]; + const { result } = renderHook(() => useColumnSummary(columns, data)); + return result.current.summaries.get(field); + }; + + it('counts every row for `count`, filled or not', () => { + // Distinct from count_filled: `dept` has one null, but count is all rows. + const summary = summaryFor('dept', 'count'); + expect(summary!.value).toBe(4); + expect(summary!.label).toBe('Count: 4'); + }); + + it('counts only non-empty cells for `count_filled`', () => { + expect(summaryFor('dept', 'count_filled')!.value).toBe(3); + expect(summaryFor('name', 'count_filled')!.value).toBe(3); + }); + + it('counts empty cells for `count_empty`, including missing keys', () => { + expect(summaryFor('dept', 'count_empty')!.value).toBe(1); + // `name` holds an empty string; `tags` has an empty array plus a row that + // omits the key entirely. + expect(summaryFor('name', 'count_empty')!.value).toBe(1); + expect(summaryFor('tags', 'count_empty')!.value).toBe(2); + }); + + it('reports a zero count rather than a blank cell', () => { + // 0 is a meaningful answer — "no empty cells" — and must not be swallowed + // into an empty label the way a null result is. + const summary = summaryFor('name', 'count_empty', [{ name: 'Alice' }, { name: 'Bob' }]); + expect(summary!.value).toBe(0); + expect(summary!.label).toBe('Empty: 0'); + }); + + it('counts distinct non-empty values for `count_unique`', () => { + // 'Eng' twice, 'Sales' once, one null → 2. + expect(summaryFor('dept', 'count_unique')!.value).toBe(2); + }); + + it('compares array cells by value for `count_unique`', () => { + // Two rows hold a *different* ['a'] array instance. A raw Set would key on + // reference and report 2 distinct values for what an author sees as one. + expect(summaryFor('tags', 'count_unique')!.value).toBe(1); + }); + + it('computes `percent_filled` and `percent_empty` over all rows', () => { + expect(summaryFor('dept', 'percent_filled')!.value).toBe(75); + expect(summaryFor('dept', 'percent_empty')!.value).toBe(25); + }); + + it('renders percent aggregations with a percent sign', () => { + expect(summaryFor('dept', 'percent_filled')!.label).toBe('Filled: 75%'); + expect(summaryFor('dept', 'percent_empty')!.label).toBe('Empty: 25%'); + }); + + it('rounds a repeating percentage to one decimal', () => { + const data = [{ v: 1 }, { v: null }, { v: null }]; + expect(summaryFor('v', 'percent_filled', data)!.label).toBe('Filled: 33.3%'); + }); + + it('counts non-numeric text values, which the numeric family cannot', () => { + // The whole point of the count family: `sum` over a text column has nothing + // to aggregate, but "how many are filled" is still a valid question. + expect(summaryFor('name', 'count_filled')!.value).toBe(3); + expect(summaryFor('name', 'sum')!.value).toBeNull(); + }); + + it('skips a column whose summary is `none`', () => { + const columns = [{ field: 'dept', label: 'Dept', summary: 'none' }] as any[]; + const { result } = renderHook(() => useColumnSummary(columns, sparseData)); + // No entry at all, so a view with only `none` columns renders no footer row + // rather than an empty one. + expect(result.current.summaries.size).toBe(0); + expect(result.current.hasSummary).toBe(false); + }); + + it('keeps other columns when one is `none`', () => { + const columns = [ + { field: 'dept', label: 'Dept', summary: 'none' }, + { field: 'name', label: 'Name', summary: 'count_filled' }, + ] as any[]; + const { result } = renderHook(() => useColumnSummary(columns, sparseData)); + expect(result.current.summaries.size).toBe(1); + expect(result.current.summaries.get('name')!.value).toBe(3); + }); + + it('skips an unrecognized aggregation name instead of rendering a blank cell', () => { + const columns = [{ field: 'dept', label: 'Dept', summary: 'median' }] as any[]; + const { result } = renderHook(() => useColumnSummary(columns, sparseData)); + expect(result.current.hasSummary).toBe(false); + }); + + it('does not mistake an inherited Object key for an aggregation name', () => { + // The supported-name check must not be an `in` test against the label map. + const columns = [{ field: 'dept', label: 'Dept', summary: 'toString' }] as any[]; + const { result } = renderHook(() => useColumnSummary(columns, sparseData)); + expect(result.current.hasSummary).toBe(false); + }); + + it('honours the per-column field override for the count family', () => { + const columns = [{ field: 'name', label: 'Name', summary: { type: 'count_unique', field: 'dept' } }] as any[]; + const { result } = renderHook(() => useColumnSummary(columns, sparseData)); + expect(result.current.summaries.get('name')!.value).toBe(2); + }); +}); + // ========================================================================= // Summary footer rendering in ObjectGrid // ========================================================================= diff --git a/packages/plugin-grid/src/__tests__/summary-spec-parity.test.ts b/packages/plugin-grid/src/__tests__/summary-spec-parity.test.ts new file mode 100644 index 0000000000..0e1e23ad78 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/summary-spec-parity.test.ts @@ -0,0 +1,44 @@ +/** + * Column summary ↔ spec vocabulary parity. + * + * `ColumnSummarySchema` is the contract: a name it accepts is a name an author + * can publish, and every published name has to render something. When the + * renderer implemented five of the eleven aggregations, a view configured with + * `count_unique` validated at authoring time and then rendered a blank footer + * cell — the failure was invisible on both sides. + * + * This test closes that gap in the direction that matters: the spec widening + * without the renderer following. It fails the moment a twelfth aggregation + * lands in the spec, which is exactly when someone needs to decide how the + * footer should show it. + */ +import { describe, it, expect } from 'vitest'; +import { ColumnSummarySchema } from '@objectstack/spec/ui'; +import { SUPPORTED_SUMMARY_TYPES } from '../useColumnSummary'; + +describe('useColumnSummary covers the spec summary vocabulary', () => { + const rawOptions = (ColumnSummarySchema as unknown as { options?: readonly string[] }).options; + const specNames: string[] = Array.isArray(rawOptions) ? [...rawOptions] : []; + + it('reads a non-empty enum from the spec', () => { + // Guards the assertions below against silently passing on an empty list if + // the spec's lazy-schema wrapper ever stops exposing `.options`. + expect(specNames, 'could not read ColumnSummarySchema.options from the spec').not.toEqual([]); + }); + + it('implements every aggregation the spec accepts', () => { + const unimplemented = specNames.filter((name) => !SUPPORTED_SUMMARY_TYPES.has(name)); + expect( + unimplemented, + 'these pass schema validation but render a blank footer cell — implement them in useColumnSummary', + ).toEqual([]); + }); + + it('does not accept aggregations the spec rejects', () => { + const extra = [...SUPPORTED_SUMMARY_TYPES].filter((name) => !specNames.includes(name)); + expect( + extra, + 'these are renderer-local dialect — promote them into @objectstack/spec instead', + ).toEqual([]); + }); +}); diff --git a/packages/plugin-grid/src/useColumnSummary.test.tsx b/packages/plugin-grid/src/useColumnSummary.test.tsx index 9dc1b58b0f..4ca1c84999 100644 --- a/packages/plugin-grid/src/useColumnSummary.test.tsx +++ b/packages/plugin-grid/src/useColumnSummary.test.tsx @@ -72,3 +72,52 @@ describe('useColumnSummary tenant-default currency', () => { expect(label).toMatch(/1,234\.00/); }); }); + +/** + * A column's currency/percent formatting describes values *in that column's + * unit*. The count family produces cardinalities and the percent family + * produces percentages, so neither may inherit it — "3 distinct customers" is + * not "$3.00", and "75% filled" is not "75% of a percent column". + */ +describe('useColumnSummary formatting stays with the numeric family', () => { + const CURRENCY_DATA = [{ amount: 1000 }, { amount: 234 }, { amount: 1000 }]; + + it('does not apply currency formatting to a count on a currency column', () => { + const cols: any[] = [{ field: 'amount', summary: 'count_unique', type: 'currency', currency: 'USD' }]; + const { result } = renderHook(() => useColumnSummary(cols, CURRENCY_DATA), { + wrapper: wrapper('USD'), + }); + const label = result.current.summaries.get('amount')?.label ?? ''; + expect(label).toBe('Unique: 2'); + expect(label).not.toMatch(/[¥$€£]/); + }); + + it('does not apply currency formatting to a percent aggregation', () => { + const cols: any[] = [{ field: 'amount', summary: 'percent_filled', type: 'currency', currency: 'USD' }]; + const { result } = renderHook(() => useColumnSummary(cols, CURRENCY_DATA), { + wrapper: wrapper('USD'), + }); + const label = result.current.summaries.get('amount')?.label ?? ''; + expect(label).toBe('Filled: 100%'); + expect(label).not.toMatch(/[¥$€£]/); + }); + + it('does not route a count through the percent column formatter', () => { + // The percent-column branch scales a fraction to 0-100; a count of 2 must + // not be reinterpreted as a ratio. + const cols: any[] = [{ field: 'rate', summary: 'count_unique', type: 'percent' }]; + const { result } = renderHook(() => useColumnSummary(cols, [{ rate: 0.5 }, { rate: 0.25 }]), { + wrapper: wrapper('USD'), + }); + expect(result.current.summaries.get('rate')?.label).toBe('Unique: 2'); + }); + + it('still applies currency formatting to the numeric family', () => { + // Guard against over-correcting: sum/avg/min/max keep column formatting. + const cols: any[] = [{ field: 'amount', summary: 'sum', type: 'currency', currency: 'USD' }]; + const { result } = renderHook(() => useColumnSummary(cols, CURRENCY_DATA), { + wrapper: wrapper('USD'), + }); + expect(result.current.summaries.get('amount')?.label).toMatch(/\$|US\$/); + }); +}); diff --git a/packages/plugin-grid/src/useColumnSummary.ts b/packages/plugin-grid/src/useColumnSummary.ts index 333873ea65..f535370361 100644 --- a/packages/plugin-grid/src/useColumnSummary.ts +++ b/packages/plugin-grid/src/useColumnSummary.ts @@ -10,11 +10,32 @@ import { useMemo } from 'react'; import type { ListColumn } from '@object-ui/types'; import { useLocalization, resolveFieldCurrency } from '@object-ui/i18n'; +/** + * Aggregation functions for the column footer — the full `ColumnSummarySchema` + * vocabulary from `@objectstack/spec`. Every value here is computed; a name the + * schema accepts but the renderer ignores would validate and then render a + * blank footer cell, so this union is kept in lockstep with the spec enum + * rather than being a renderer-local subset of it. + */ +export type ColumnSummaryType = + | 'none' + | 'count' + | 'count_empty' + | 'count_filled' + | 'count_unique' + | 'percent_empty' + | 'percent_filled' + | 'sum' + | 'avg' + | 'min' + | 'max'; + /** * Summary configuration for a column. - * Can be a string shorthand (e.g. 'sum') or a full config object. + * Can be a string shorthand (e.g. 'sum') or a full config object carrying a + * per-column `field` override. */ -export type ColumnSummaryConfig = string | { type: 'count' | 'sum' | 'avg' | 'min' | 'max'; field?: string }; +export type ColumnSummaryConfig = ColumnSummaryType | { type: ColumnSummaryType; field?: string }; export interface ColumnSummaryResult { field: string; @@ -22,6 +43,76 @@ export interface ColumnSummaryResult { label: string; } +/** A data row as the aggregations read it — cells are untyped until inspected. */ +type SummaryRow = Record; + +/** + * Aggregations that read raw cell values instead of parsed numbers, so they + * work on text, select and lookup columns rather than numeric ones only. + */ +const NON_NUMERIC_TYPES = new Set([ + 'count', + 'count_empty', + 'count_filled', + 'count_unique', + 'percent_empty', + 'percent_filled', +]); + +/** Aggregations whose result is a percentage (0-100), not a value in the column's unit. */ +const PERCENT_TYPES = new Set(['percent_empty', 'percent_filled']); + +const TYPE_LABELS: Record = { + none: '', + count: 'Count', + count_empty: 'Empty', + count_filled: 'Filled', + count_unique: 'Unique', + // The trailing `%` is what distinguishes these from the count-family pair. + percent_empty: 'Empty', + percent_filled: 'Filled', + sum: 'Sum', + avg: 'Avg', + min: 'Min', + max: 'Max', +}; + +/** + * Every aggregation name the renderer computes. A Set rather than an `in` check + * against TYPE_LABELS, because `in` also matches inherited keys — a column + * configured with `summary: 'toString'` would otherwise read as supported. + * + * Exported so a test can assert it covers the spec's `ColumnSummarySchema` + * exactly: a name the schema accepts but this set omits validates at authoring + * time and then renders a blank footer cell. + */ +export const SUPPORTED_SUMMARY_TYPES: ReadonlySet = new Set(Object.keys(TYPE_LABELS)); + +/** + * Emptiness test for the count/percent aggregations. Matches the convention + * used elsewhere in the codebase (audit history display, form dirty-checking): + * null/undefined/empty-string, plus empty arrays so an unset multi-select or + * lookup column counts as empty instead of as a filled `[]`. + */ +const isEmptyValue = (v: unknown): boolean => + v === null || v === undefined || v === '' || (Array.isArray(v) && v.length === 0); + +/** + * Stable identity for `count_unique`. Object and array cells (lookup and + * multi-select columns) compare by value — a raw `Set` compares by reference + * and would report every row as unique. + */ +function uniqueKey(v: unknown): unknown { + if (typeof v === 'object') { + try { + return JSON.stringify(v) ?? String(v); + } catch { + return String(v); + } + } + return v; +} + /** * Normalize summary config from string or object to a standard shape. */ @@ -33,14 +124,66 @@ function normalizeSummary(summary: ColumnSummaryConfig): { type: string; field?: } /** - * Compute a single aggregation over data values. + * Collect the numeric values of a field, parsing numeric strings so a column + * backed by string-encoded decimals still aggregates. + */ +function numericValues(rows: SummaryRow[], field: string): number[] { + const values: number[] = []; + for (const row of rows) { + const v = row[field]; + if (typeof v === 'number' && !isNaN(v)) { + values.push(v); + } else if (typeof v === 'string') { + const parsed = parseFloat(v); + if (!isNaN(parsed)) values.push(parsed); + } + } + return values; +} + +/** + * Compute a single aggregation over the rows. + * + * The count and percent families are cardinalities over *raw* cell values, so + * they are computed before the numeric parse — `count_unique` on a text column + * has to work, and a row whose value does not parse as a number is still a + * filled row. */ -function computeAggregation(type: string, values: number[]): number | null { +function computeAggregation(type: string, rows: SummaryRow[], field: string): number | null { + if (NON_NUMERIC_TYPES.has(type)) { + const total = rows.length; + if (total === 0) return null; + + switch (type) { + case 'count': + // Every row, filled or not — `count_filled` is the non-empty variant. + return total; + case 'count_filled': + return rows.filter((r) => !isEmptyValue(r[field])).length; + case 'count_empty': + return rows.filter((r) => isEmptyValue(r[field])).length; + case 'count_unique': { + // An empty cell is not a distinct value, it is the absence of one. + const seen = new Set(); + for (const row of rows) { + const v = row[field]; + if (!isEmptyValue(v)) seen.add(uniqueKey(v)); + } + return seen.size; + } + case 'percent_filled': + return (rows.filter((r) => !isEmptyValue(r[field])).length / total) * 100; + case 'percent_empty': + return (rows.filter((r) => isEmptyValue(r[field])).length / total) * 100; + default: + return null; + } + } + + const values = numericValues(rows, field); if (values.length === 0) return null; switch (type) { - case 'count': - return values.length; case 'sum': return values.reduce((a, b) => a + b, 0); case 'avg': @@ -64,6 +207,11 @@ function computeAggregation(type: string, values: number[]): number | null { * Currency code defaults to USD when neither `currency` nor * `defaultCurrency` is supplied — mirrors the CurrencyCellRenderer * behavior so cells and footer agree. + * + * That column formatting applies to the numeric family only. Count + * aggregations are plain cardinalities and percent aggregations carry their own + * unit, so neither may inherit the column's currency or percent formatting — + * `count_unique` on a currency column reads "3", not "$3.00". */ function formatSummaryLabel( type: string, @@ -72,18 +220,18 @@ function formatSummaryLabel( tenantDefault?: string, ): string { if (value === null) return ''; - const typeLabels: Record = { - count: 'Count', - sum: 'Sum', - avg: 'Avg', - min: 'Min', - max: 'Max', - }; - const label = typeLabels[type] || type; + const label = TYPE_LABELS[type as ColumnSummaryType] || type; + + if (PERCENT_TYPES.has(type)) { + return `${label}: ${value.toLocaleString(undefined, { maximumFractionDigits: 1 })}%`; + } + if (NON_NUMERIC_TYPES.has(type)) { + return `${label}: ${value.toLocaleString()}`; + } const colType = column?.type; let formatted: string; - if (type !== 'count' && colType === 'currency') { + if (colType === 'currency') { const currency = resolveFieldCurrency(column, tenantDefault); // Decimal places come from `scale`, not `precision` (the total digit count // of a decimal(p, s) column) — see #2131. Reading `precision` padded a @@ -104,7 +252,7 @@ function formatSummaryLabel( } catch { formatted = value.toLocaleString(); } - } else if (type !== 'count' && colType === 'percent') { + } else if (colType === 'percent') { const decimals = column?.precision ?? 0; const pct = (value > -1 && value < 1) ? value * 100 : value; formatted = `${pct.toFixed(decimals)}%`; @@ -145,28 +293,15 @@ export function useColumnSummary( if (!col.summary) continue; const config = normalizeSummary(col.summary as ColumnSummaryConfig); - const targetField = config.field || col.field; - // Extract numeric values from data - const values: number[] = []; - for (const row of data) { - const v = row[targetField]; - if (v != null && typeof v === 'number' && !isNaN(v)) { - values.push(v); - } else if (v != null && typeof v === 'string') { - const parsed = parseFloat(v); - if (!isNaN(parsed)) values.push(parsed); - } - } + // `none` is the spec's explicit opt-out, and an unrecognized name has no + // value to show. Both skip the entry entirely rather than registering a + // blank one — otherwise a view whose columns all say `none` would render + // an empty footer row. + if (config.type === 'none' || !SUPPORTED_SUMMARY_TYPES.has(config.type)) continue; - // For 'count', count all non-null values (not just numeric) - let result: number | null; - if (config.type === 'count') { - const count = data.filter(row => row[targetField] != null && row[targetField] !== '').length; - result = count > 0 ? count : null; - } else { - result = computeAggregation(config.type, values); - } + const targetField = config.field || col.field; + const result = computeAggregation(config.type, data, targetField); // Merge column-level hints (`col.currency`, `col.precision`, etc.) with // any matching fieldMetadata entry so authors get correct currency/ diff --git a/packages/types/src/__tests__/spec-subschema-parity.test.ts b/packages/types/src/__tests__/spec-subschema-parity.test.ts index 4a2cb1b33d..9aafe25f8c 100644 --- a/packages/types/src/__tests__/spec-subschema-parity.test.ts +++ b/packages/types/src/__tests__/spec-subschema-parity.test.ts @@ -147,6 +147,19 @@ describe('ListColumnSchema derives from the spec (extend, not fork)', () => { ); }); + it('the object arm draws its aggregation names from the same spec enum', () => { + // Both arms must accept the same vocabulary: a name valid as the string + // shorthand but rejected in the `{ type, field }` form (or vice versa) + // means the per-column `field` override is unavailable for that + // aggregation for no reason the author can see. + const summary = ouiShape.summary as { unwrap(): { def: { options: unknown[] } } }; + const objectArm = summary.unwrap().def.options[1] as { shape: Record }; + expect( + objectArm.shape.type, + 'summary object arm `type` must be the spec ColumnSummarySchema by reference', + ).toBe(SpecColumnSummarySchema); + }); + it('summary accepts every spec aggregation and the renderer object form', () => { for (const agg of ['none', 'count', 'count_unique', 'percent_filled', 'sum', 'avg', 'min', 'max']) { expect(ListColumnSchema.shape.summary.safeParse(agg).success, `summary "${agg}"`).toBe(true); diff --git a/packages/types/src/zod/objectql.zod.ts b/packages/types/src/zod/objectql.zod.ts index f46a386806..5b78d07eff 100644 --- a/packages/types/src/zod/objectql.zod.ts +++ b/packages/types/src/zod/objectql.zod.ts @@ -62,7 +62,11 @@ export const ViewDataSchema = SpecViewDataSchema; * `{ type, field }` object form the grid renderer supports (per-column field * override — see `useColumnSummary` in `@object-ui/plugin-grid`). The old * mirror's free-string arm is gone: unknown aggregation names now fail - * validation instead of silently rendering nothing. + * validation instead of silently rendering nothing. Both arms take their + * vocabulary from `SpecColumnSummarySchema` by reference, so the shorthand + * and the object form can never accept different aggregation names — + * matching the object form promoted into the spec by objectstack#3761, + * which this extension collapses into once that release lands. * - `prefix` is objectui-only compound-cell rendering (read by `ObjectGrid`); * promote it into the spec rather than growing this extension. */ @@ -70,7 +74,7 @@ export const ListColumnSchema = SpecListColumnSchema.extend({ summary: z.union([ SpecColumnSummarySchema, z.object({ - type: z.enum(['count', 'sum', 'avg', 'min', 'max']).describe('Aggregation type'), + type: SpecColumnSummarySchema, field: z.string().optional().describe('Field to aggregate (defaults to column field)'), }), ]).optional().describe('Column footer summary/aggregation'), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0d4237c69f..57c034fb8a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1975,6 +1975,9 @@ importers: '@object-ui/data-objectstack': specifier: workspace:* version: link:../data-objectstack + '@objectstack/spec': + specifier: ^16.0.0-rc.0 + version: 16.1.0(ai@7.0.37(zod@4.4.3)) '@vitejs/plugin-react': specifier: ^6.0.4 version: 6.0.4(vite@8.1.5(@types/node@26.1.1)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.23.1)(yaml@2.9.0)) From 1767124a8b17bdbcda9fcc2b72e36c32398cef79 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 04:10:37 +0000 Subject: [PATCH 2/2] refactor(react): drop the unused second useColumnSummary hook (#2890) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@object-ui/react` exported a hook of the same name as the grid's, with no callers anywhere in the repo. It was not a duplicate implementation so much as a competing one: - a different API — `(data, config, locale)` against the grid's `(columns, data, fieldMetadata)` — and a colliding `ColumnSummaryConfig` export of a different shape; - a doc comment claiming it implements "ColumnSummarySchema from @objectstack/spec v2.0.7", six majors behind; - a `distinct` aggregation that is not in the spec vocabulary at all. The spec calls that `count_unique`. That last point is what makes it worth removing rather than leaving inert: a publicly exported name the spec does not define is the second de-facto contract Commandment #0.1 warns about, and the parity guard added alongside the grid hook does not reach it. Callers wanting footer aggregations use `useColumnSummary` from `@object-ui/plugin-grid`, which now implements the spec enum in full. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V3EDGLKNvbcpDTLzyiAd3Q --- .changeset/column-summary-spec-vocabulary.md | 43 ++++++++++ packages/react/src/hooks/index.ts | 1 - packages/react/src/hooks/useColumnSummary.ts | 87 -------------------- 3 files changed, 43 insertions(+), 88 deletions(-) create mode 100644 .changeset/column-summary-spec-vocabulary.md delete mode 100644 packages/react/src/hooks/useColumnSummary.ts diff --git a/.changeset/column-summary-spec-vocabulary.md b/.changeset/column-summary-spec-vocabulary.md new file mode 100644 index 0000000000..9f3e35e73b --- /dev/null +++ b/.changeset/column-summary-spec-vocabulary.md @@ -0,0 +1,43 @@ +--- +"@object-ui/plugin-grid": minor +"@object-ui/types": minor +"@object-ui/react": minor +--- + +feat(grid): compute all eleven spec column summary aggregations (#2890) + +`ColumnSummarySchema` accepts eleven aggregation names; `useColumnSummary` computed +five. The other six — `none`, `count_empty`, `count_filled`, `count_unique`, +`percent_empty`, `percent_filled` — passed validation at authoring time and then +rendered a blank footer cell, with no error raised on either side. + +The computation now splits into two families. Count and percent read *raw* cell +values, before the numeric parse, so they work on text, select and lookup columns and +a value that does not parse as a number still counts as a filled row; a cell is empty +when it is `null`, `undefined`, `""` or an empty array. `sum`/`avg`/`min`/`max` keep +the existing numeric parse and column formatting. + +Two behavior changes follow from the enum carrying both `count` and `count_filled`, +which cannot mean the same thing: + +- `count` is now every row; `count_filled` is the non-empty variant. Only a column + whose values are all empty renders differently than before. +- a zero count renders `Empty: 0` instead of collapsing to a blank cell. + +Column currency/percent formatting is gated to the numeric family, so `count_unique` +on a currency column reads `Unique: 3` and not `$3.00`. `none` and unrecognized names +skip the entry entirely, so a view whose columns all opt out renders no footer row. + +`ListColumnSchema`'s objectui-local `{ type, field }` arm now takes its vocabulary +from `SpecColumnSummarySchema` by reference — it was stuck at the same five names, +which left the per-column `field` override unavailable for the six new aggregations. + +A parity test asserts the renderer's supported set equals the spec enum in both +directions: a spec name the renderer omits is the bug above, and a renderer name the +spec omits would be local dialect (Commandment #0). + +**Removed:** `useColumnSummary` from `@object-ui/react`. It was a second, unrelated +hook of the same name with no callers — a different API, a comment claiming it +implemented spec v2.0.7, and a `distinct` aggregation that is not in the spec +vocabulary at all (the spec calls it `count_unique`). Use `useColumnSummary` from +`@object-ui/plugin-grid`, which implements the spec enum. diff --git a/packages/react/src/hooks/index.ts b/packages/react/src/hooks/index.ts index 6b0dc65664..99665065b3 100644 --- a/packages/react/src/hooks/index.ts +++ b/packages/react/src/hooks/index.ts @@ -22,7 +22,6 @@ export * from './useKeyboardShortcuts'; export * from './useCrudShortcuts'; export * from './useReducedMotion'; export * from './useAnimation'; -export * from './useColumnSummary'; export * from './useDensityMode'; export * from './useViewSharing'; export * from './useClientNotifications'; diff --git a/packages/react/src/hooks/useColumnSummary.ts b/packages/react/src/hooks/useColumnSummary.ts deleted file mode 100644 index 8102771207..0000000000 --- a/packages/react/src/hooks/useColumnSummary.ts +++ /dev/null @@ -1,87 +0,0 @@ -/** - * ObjectUI - * Copyright (c) 2024-present ObjectStack Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { useMemo } from 'react'; - -/** Summary function types aligned with ColumnSummarySchema */ -export type SummaryFunction = 'sum' | 'avg' | 'count' | 'min' | 'max' | 'distinct'; - -export interface ColumnSummaryConfig { - /** The field/column to aggregate */ - field: string; - /** The aggregation function */ - function: SummaryFunction; - /** Optional label for the summary */ - label?: string; - /** Number format options */ - format?: Intl.NumberFormatOptions; -} - -export interface ColumnSummaryResult { - /** The computed value */ - value: number; - /** Formatted display string */ - formatted: string; - /** The label */ - label: string; -} - -/** - * Hook for computing column-level summaries (SUM, AVG, COUNT, MIN, MAX, DISTINCT). - * Implements ColumnSummarySchema from @objectstack/spec v2.0.7. - * - * @example - * ```tsx - * const summary = useColumnSummary(data, { field: 'amount', function: 'sum', label: 'Total' }); - * // summary.formatted => "$12,345.67" - * ``` - */ -export function useColumnSummary( - data: Record[], - config: ColumnSummaryConfig, - locale?: string -): ColumnSummaryResult { - return useMemo(() => { - const values = data - .map((row) => { - const val = row[config.field]; - return typeof val === 'number' ? val : Number(val); - }) - .filter((v) => !isNaN(v)); - - let value: number; - - switch (config.function) { - case 'sum': - value = values.reduce((acc, v) => acc + v, 0); - break; - case 'avg': - value = values.length > 0 ? values.reduce((acc, v) => acc + v, 0) / values.length : 0; - break; - case 'count': - value = data.length; - break; - case 'min': - value = values.length > 0 ? Math.min(...values) : 0; - break; - case 'max': - value = values.length > 0 ? Math.max(...values) : 0; - break; - case 'distinct': - value = new Set(data.map((row) => row[config.field])).size; - break; - default: - value = 0; - } - - const formatted = new Intl.NumberFormat(locale, config.format).format(value); - const label = config.label ?? config.function.toUpperCase(); - - return { value, formatted, label }; - }, [data, config.field, config.function, config.label, config.format, locale]); -}