fix(charts): a fieldless count aggregate keyed its value column undefined (framework#3701)

.changeset/objectchart-fieldless-count-column.md

@@ -0,0 +1,30 @@
+---
+"@object-ui/plugin-charts": patch
+"@object-ui/data-objectstack": patch
+---
+
+fix(charts): a fieldless `count` aggregate keyed its value column `undefined`, so the chart plotted nothing (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; 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 the paths that build these rows have 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` / `ObjectDataSource.aggregateClientSide` emitted
+  `{ [groupBy]: key, [undefined]: value }`, i.e. a column literally named
+  `undefined` that no axis binding could ever name;
+- the legacy analytics path was worse: it remapped the server's `count` measure
+  onto `params.field` and **deleted** the original key, so the value the server
+  did return was thrown away before the chart saw it.
+
+All of them now resolve the column through one helper (`aggregateValueKey`) so a
+fieldless count lands under `count`, matching the framework contract. The
+comparison-overlay column is derived from the same key (`count__comparison`
+instead of `undefined__comparison`), and `aggregate.field` is typed optional to
+match the spec's `ChartAggregateSchema`. Charts that name a field are unchanged.

packages/data-objectstack/src/index.ts

@@ -2603,6 +2603,12 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
       const measureName = params.function === 'count'
         ? 'count'
         : `${params.field}_${params.function}`;
+      // The column the caller expects the value under — the raw `field`, or the
+      // literal `count` when a count names no field (framework#3701). Reading
+      // `params.field` directly here keyed the row `undefined` for a fieldless
+      // count and deleted the `count` the server sent, so the chart plotted
+      // nothing.
+      const valueKey = this.aggregateValueKey(params);
 
       const payload: Record<string, unknown> = {
         cube: resource,
@@ -2634,7 +2640,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
       const measureMissing = rawRows.length > 0 && rawRows.every((row: any) => {
         if (row == null) return true;
         if (measureName in row && row[measureName] != null) return false;
-        if (params.field in row && row[params.field] != null) return false;
+        if (valueKey in row && row[valueKey] != null) return false;
         return true;
       });
       if (measureMissing) {
@@ -2644,14 +2650,15 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
         return this.aggregateClientSide(records, params);
       }
 
-      // Map measure keys back to the original field name so that consumers
-      // (ObjectChart, DashboardRenderer, etc.) can access values by field name.
-      // This includes count → field (e.g. 'count' → 'amount') to match the
-      // output format of aggregateClientSide() which always uses params.field.
+      // Map measure keys back to the object-bound result column so consumers
+      // (ObjectChart, DashboardRenderer, …) read values by the name the
+      // convention promises: `field`, or `count` for a fieldless count
+      // (framework#3701). This includes count → field (e.g. 'count' →
+      // 'amount'), matching aggregateClientSide()'s output.
       return rawRows.map((row: any) => {
         const mapped = { ...row };
-        if (measureName !== params.field && measureName in mapped) {
-          mapped[params.field] = mapped[measureName];
+        if (measureName !== valueKey && measureName in mapped) {
+          mapped[valueKey] = mapped[measureName];
           delete mapped[measureName];
         }
         return mapped;
@@ -2776,8 +2783,19 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
   }
 
   /** Client-side aggregation fallback */
-  private aggregateClientSide(records: any[], params: { field: string; function: string; groupBy: string }): any[] {
+  /**
+   * The result column an object-bound `aggregate` projects its value under
+   * (framework#3701, `chartAggregateValueKey` in `@objectstack/spec/ui`): the
+   * raw `field` name — no `sum_`-style decoration, unlike a dataset measure —
+   * or the literal `count` when a count names no field.
+   */
+  private aggregateValueKey(params: { field?: string; function?: string }): string {
+    return params.field || params.function || 'count';
+  }
+
+  private aggregateClientSide(records: any[], params: { field?: string; function: string; groupBy: string }): any[] {
     const { field, function: aggFn, groupBy } = params;
+    const valueKey = this.aggregateValueKey(params);
     const groups: Record<string, any[]> = {};
 
     for (const record of records) {
@@ -2787,7 +2805,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
     }
 
     return Object.entries(groups).map(([key, group]) => {
-      const values = group.map(r => Number(r[field]) || 0);
+      const values = field ? group.map(r => Number(r[field]) || 0) : [];
       let result: number;
 
       switch (aggFn) {
@@ -2798,7 +2816,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
         case 'sum': default: result = values.reduce((a, b) => a + b, 0); break;
       }
 
-      return { [groupBy]: key, [field]: result };
+      return { [groupBy]: key, [valueKey]: result };
     });
   }
 

packages/plugin-charts/src/ObjectChart.aggregateResultColumns.test.ts

@@ -0,0 +1,67 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * The object-bound aggregate RESULT-COLUMN convention (framework#3701).
+ *
+ * An object-bound chart (`objectName` + an inline `aggregate`) returns rows
+ * keyed by the RAW FIELD NAMES it was given — `groupBy` for the category,
+ * `field` for the value — with NO `sum_`-style decoration. That is the opposite
+ * of a dataset, whose rows are keyed by the declared measure `name`, and it is
+ * what `xAxisKey` / `series[].dataKey` resolve against. The framework now pins
+ * the rule (`chartAggregateResultKeys` in `@objectstack/spec/ui`) and lints
+ * page sources against it, so the row builders here have to honour it exactly.
+ *
+ * Regression: `count` is the one function that may omit `field`, and every
+ * builder read `params.field` directly — so a fieldless count keyed its value
+ * `undefined` and the chart plotted nothing.
+ */
+import { describe, it, expect } from 'vitest';
+import { aggregateRecords, aggregateValueKey } from './ObjectChart';
+
+const rows = [
+  { status: 'open', total: 100 },
+  { status: 'open', total: 50 },
+  { status: 'paid', total: 20 },
+];
+
+describe('aggregateValueKey', () => {
+  it('is the raw field — never a decorated measure name', () => {
+    expect(aggregateValueKey({ field: 'total', function: 'sum' })).toBe('total');
+  });
+
+  it('is the literal "count" when a count names no field', () => {
+    expect(aggregateValueKey({ function: 'count' })).toBe('count');
+  });
+
+  it('prefers an explicit field even for count', () => {
+    expect(aggregateValueKey({ field: 'total', function: 'count' })).toBe('total');
+  });
+});
+
+describe('aggregateRecords — result columns', () => {
+  it('keys rows by groupBy and the raw field', () => {
+    expect(aggregateRecords(rows, { field: 'total', function: 'sum', groupBy: 'status' })).toEqual([
+      { status: 'open', total: 150 },
+      { status: 'paid', total: 20 },
+    ]);
+  });
+
+  it('keys a fieldless count under "count", not undefined', () => {
+    const out = aggregateRecords(rows, { function: 'count', groupBy: 'status' });
+    expect(out).toEqual([
+      { status: 'open', count: 2 },
+      { status: 'paid', count: 1 },
+    ]);
+    // The pre-fix shape: a column literally named "undefined", which no axis
+    // binding could ever name.
+    expect(Object.keys(out[0])).not.toContain('undefined');
+  });
+
+  it('still counts rows when a field IS named', () => {
+    expect(aggregateRecords(rows, { field: 'total', function: 'count', groupBy: 'status' })).toEqual([
+      { status: 'open', total: 2 },
+      { status: 'paid', total: 1 },
+    ]);
+  });
+});

packages/plugin-charts/src/ObjectChart.tsx

@@ -15,16 +15,34 @@ export function humanizeLabel(value: string): string {
   return value.replace(/[_-]/g, ' ').replace(/\b\w/g, c => c.toUpperCase());
 }
 
+/**
+ * The result column an object-bound `aggregate` projects its value under
+ * (framework#3701, `chartAggregateValueKey` in `@objectstack/spec/ui`).
+ *
+ * The raw `field` name — an object-bound aggregate does NOT decorate it the way
+ * a dataset measure is named (`sum_amount`). Only `count` may omit `field`, and
+ * it then lands under the literal `'count'`, which is the alias the engine
+ * projects `COUNT(*)` under. Exported so every path that builds these rows
+ * agrees on one key instead of each re-deriving it.
+ */
+export function aggregateValueKey(aggregate: { field?: string; function?: string }): string {
+  return aggregate.field || aggregate.function || 'count';
+}
+
+/** Suffix the previous-window value carries under a compareTo overlay. */
+export const COMPARISON_SUFFIX = '__comparison';
+
 /**
  * Client-side aggregation for fetched records.
  * Groups records by `groupBy` field and applies the aggregation function
  * to the `field` values in each group.
  */
 export function aggregateRecords(
   records: any[],
-  aggregate: { field: string; function: string; groupBy: string }
+  aggregate: { field?: string; function: string; groupBy: string }
 ): any[] {
   const { field, function: aggFn, groupBy } = aggregate;
+  const valueKey = aggregateValueKey(aggregate);
   const groups: Record<string, any[]> = {};
 
   for (const record of records) {
@@ -34,7 +52,7 @@ export function aggregateRecords(
   }
 
   return Object.entries(groups).map(([key, group]) => {
-    const values = group.map(r => Number(r[field]) || 0);
+    const values = field ? group.map(r => Number(r[field]) || 0) : [];
     let result: number;
 
     switch (aggFn) {
@@ -56,7 +74,7 @@ export function aggregateRecords(
         break;
     }
 
-    return { [groupBy]: key, [field]: result };
+    return { [groupBy]: key, [valueKey]: result };
   });
 }
 
@@ -369,8 +387,9 @@ export const ObjectChart = (props: any) => {
         const aggField = schema.aggregate.field;
         const aggFn = schema.aggregate.function;
         // Project the measure under its plain field name so downstream
-        // (xAxisKey + series.dataKey lookups) finds it unchanged.
-        const alias = aggField || aggFn;
+        // (xAxisKey + series.dataKey lookups) finds it unchanged — the
+        // object-bound result-column convention (framework#3701).
+        const alias = aggregateValueKey(schema.aggregate);
         // For `count`, omit `field` so the engine emits `count(*)` /
         // `COUNT(*)`. The upstream dashboard wiring defaults `field: 'value'`
         // for charts without an explicit valueField, which crashes on SQL
@@ -479,17 +498,22 @@ export const ObjectChart = (props: any) => {
           if (wantsComparison && comparisonRows.length > 0 && schema.aggregate) {
             const aggField = schema.aggregate.field;
             const aggFn = schema.aggregate.function;
+            // The column this aggregate projects its value under — `field`,
+            // or `count` for a fieldless count (framework#3701).
+            const valueKey = aggregateValueKey(schema.aggregate);
             const readValue = (row: Record<string, any>): number | null => {
               if (row == null) return null;
-              const suffixed = `${aggField}_${aggFn}`;
-              if (suffixed in row) return Number(row[suffixed]);
-              if (aggFn === 'count' && `${aggField}_count` in row) return Number(row[`${aggField}_count`]);
-              if (aggField in row) return Number(row[aggField]);
+              if (aggField) {
+                const suffixed = `${aggField}_${aggFn}`;
+                if (suffixed in row) return Number(row[suffixed]);
+                if (aggFn === 'count' && `${aggField}_count` in row) return Number(row[`${aggField}_count`]);
+              }
+              if (valueKey in row) return Number(row[valueKey]);
               if ('value' in row) return Number(row.value);
               if ('count' in row) return Number(row.count);
               return null;
             };
-            const comparisonKey = `${aggField}__comparison`;
+            const comparisonKey = `${valueKey}${COMPARISON_SUFFIX}`;
             const gb = groupByField;
             if (gb && data.some((r: any) => r[gb] != null) && comparisonRows.some((r: any) => r[gb] != null)) {
               const cmpByKey = new Map<string, number | null>();
@@ -601,16 +625,20 @@ export const ObjectChart = (props: any) => {
   // for a supported chart type, also synthesize a second series so the
   // chart implementation renders the comparison overlay (dashed / muted).
   const compareToConfig: CompareToConfig | undefined = (schema as any).compareTo;
+  // The result column this aggregate projects its value under, and the column
+  // the comparison overlay arrives in (framework#3701).
+  const valueKey = schema.aggregate ? aggregateValueKey(schema.aggregate) : undefined;
+  const comparisonKey = valueKey ? `${valueKey}${COMPARISON_SUFFIX}` : undefined;
   const enableComparisonSeries =
     !!compareToConfig &&
     supportsCompareTo(schema.chartType) &&
-    !!schema.aggregate &&
-    finalData.some((row: Record<string, any>) => row[`${schema.aggregate!.field}__comparison`] != null);
+    !!comparisonKey &&
+    finalData.some((row: Record<string, any>) => row[comparisonKey] != null);
 
   const augmentedSeries = useMemo(() => {
     const existing = Array.isArray((schema as any).series) ? (schema as any).series : null;
     if (!enableComparisonSeries) return existing;
-    const primary = existing || [{ dataKey: schema.aggregate!.field }];
+    const primary = existing || [{ dataKey: valueKey }];
     const labelMap: Record<string, string> = {
       vsLastWeek: 'Previous week',
       vsLastMonth: 'Previous month',
@@ -624,12 +652,12 @@ export const ObjectChart = (props: any) => {
     return [
       ...primary.map((s: any) => ({ ...s, variant: s.variant || 'current' })),
       {
-        dataKey: `${schema.aggregate!.field}__comparison`,
+        dataKey: comparisonKey,
         label: friendlyLabel,
         variant: 'comparison',
       },
     ];
-  }, [enableComparisonSeries, (schema as any).series, schema.aggregate, schema.filter, compareToConfig]);
+  }, [enableComparisonSeries, (schema as any).series, valueKey, comparisonKey, schema.filter, compareToConfig]);
 
   // ADR-0021 (#1759): when the chart binds to a dataset, derive data/xAxisKey/
   // series from its dimensions/measures via the shared buildChartSeries helper —