Skip to content

Commit cf6e117

Browse files
authored
Merge pull request #1720 from objectstack-ai/feat-1719-table-count-only-warning
feat(cli): build-time warning for count-only table/pivot dashboard widgets (table-count-only)
2 parents c029f0c + 0dd6aba commit cf6e117

5 files changed

Lines changed: 289 additions & 0 deletions

File tree

packages/cli/src/commands/compile.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ import { ObjectStackDefinitionSchema, normalizeStackInput } from '@objectstack/s
99
import { loadConfig } from '../utils/config.js';
1010
import { lowerCallables } from '../utils/lower-callables.js';
1111
import { validateStackExpressions } from '../utils/validate-expressions.js';
12+
import { validateWidgetBindings } from '../utils/validate-widget-bindings.js';
1213
import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js';
1314
import {
1415
printHeader,
1516
printKV,
1617
printSuccess,
1718
printError,
1819
printStep,
20+
printWarning,
1921
createTimer,
2022
formatZodErrors,
2123
collectMetadataStats,
@@ -145,6 +147,22 @@ export default class Compile extends Command {
145147
this.exit(1);
146148
}
147149

150+
// 3c. Widget-binding diagnostics (issue #1719) — semantic checks that
151+
// need the widget's `dataset` reference resolved to its dataset and
152+
// `values` resolved to measures with known aggregates. Advisory
153+
// only: warnings are printed (and included in --json output) but
154+
// never fail the build. Suppress per widget via
155+
// `suppressWarnings: ['<rule-id>']`.
156+
const widgetWarnings = validateWidgetBindings(result.data as Record<string, unknown>);
157+
if (widgetWarnings.length > 0 && !flags.json) {
158+
console.log('');
159+
for (const w of widgetWarnings) {
160+
printWarning(`${w.where}: ${w.message}`);
161+
console.log(chalk.dim(` ${w.hint}`));
162+
console.log(chalk.dim(` rule: ${w.rule} at ${w.path}`));
163+
}
164+
}
165+
148166
// 4. Generate Artifact
149167
if (!flags.json) printStep('Writing artifact...');
150168
const output = flags.output!;
@@ -227,6 +245,7 @@ export default class Compile extends Command {
227245
handlersBundled: lowering.count,
228246
runtimeModule: runtimeBundle?.outputFileName ?? null,
229247
runtimeModuleSize: runtimeBundle?.size ?? 0,
248+
warnings: widgetWarnings,
230249
stats,
231250
duration: timer.elapsed(),
232251
}));
@@ -236,6 +255,9 @@ export default class Compile extends Command {
236255
// 5. Summary
237256
console.log('');
238257
printSuccess(`Build complete ${chalk.dim(`(${timer.display()})`)}`);
258+
if (widgetWarnings.length > 0) {
259+
printWarning(`${widgetWarnings.length} widget-binding warning(s) — see above`);
260+
}
239261
console.log('');
240262
printMetadataStats(stats);
241263
console.log('');

packages/cli/src/commands/lint.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { normalizeStackInput } from '@objectstack/spec';
77
import { loadConfig, BUNDLE_REQUIRE_EXTERNALS } from '../utils/config.js';
88
import { computeI18nCoverage } from '../utils/i18n-coverage.js';
99
import { lintDataModel } from '../lint/data-model-rules.js';
10+
import { validateWidgetBindings } from '../utils/validate-widget-bindings.js';
1011
import { scoreMetadata } from '../lint/score.js';
1112
import { runMetadataEval } from '../lint/metadata-eval.js';
1213
import { DEFAULT_METADATA_EVAL_CORPUS } from '../lint/corpus.js';
@@ -219,6 +220,20 @@ export function lintConfig(config: any): LintIssue[] {
219220
// objectstack-data/-ui skills. These double as the eval rubric (see score.ts).
220221
issues.push(...lintDataModel(objects));
221222

223+
// ── Dashboard widget bindings (ADR-0021, issue #1719) ──
224+
// e.g. a table/pivot widget whose binding resolves to count-only measures
225+
// with no dimensions — almost always a record listing that belongs in an
226+
// object-bound ListView (ADR-0017), not an analytics dataset.
227+
for (const w of validateWidgetBindings(config)) {
228+
issues.push({
229+
severity: 'warning',
230+
rule: w.rule,
231+
message: `${w.where}: ${w.message}`,
232+
path: w.path,
233+
fix: w.hint,
234+
});
235+
}
236+
222237
return issues;
223238
}
224239

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { validateWidgetBindings, TABLE_COUNT_ONLY } from './validate-widget-bindings.js';
3+
4+
/** The downstream repro from issue #1719 — dataset with a count AND a sum
5+
* measure plus a dimension; the widget selects only the count, no dims. */
6+
function reproStack(widgetOverrides: Record<string, unknown> = {}) {
7+
return {
8+
datasets: [{
9+
name: 'expense_report_metrics',
10+
label: 'Expense report metrics',
11+
object: 'expense_report',
12+
measures: [
13+
{ name: 'report_count', label: 'report_count', aggregate: 'count' },
14+
{ name: 'total_amount', label: 'total_amount', aggregate: 'sum', field: 'total_amount' },
15+
],
16+
dimensions: [{ name: 'cost_center', field: 'cost_center' }],
17+
}],
18+
dashboards: [{
19+
name: 'expenses_overview_dashboard',
20+
label: 'Expenses Overview',
21+
widgets: [{
22+
id: 'pending_reports_table',
23+
type: 'table',
24+
dataset: 'expense_report_metrics',
25+
values: ['report_count'],
26+
filter: { status: 'submitted' },
27+
layout: { x: 0, y: 0, w: 6, h: 4 },
28+
...widgetOverrides,
29+
}],
30+
}],
31+
};
32+
}
33+
34+
describe('validateWidgetBindings (table-count-only, issue #1719)', () => {
35+
it('warns on the issue repro: count-only table widget without dimensions', () => {
36+
const warnings = validateWidgetBindings(reproStack());
37+
expect(warnings).toHaveLength(1);
38+
expect(warnings[0].rule).toBe(TABLE_COUNT_ONLY);
39+
expect(warnings[0].where).toContain('expenses_overview_dashboard');
40+
expect(warnings[0].where).toContain('pending_reports_table');
41+
expect(warnings[0].path).toBe('dashboards[0].widgets[0]');
42+
expect(warnings[0].message).toContain('report_count');
43+
expect(warnings[0].message).toContain('single summary row');
44+
expect(warnings[0].hint).toContain('ListView (ADR-0017)');
45+
expect(warnings[0].hint).toContain(`suppressWarnings: ['${TABLE_COUNT_ONLY}']`);
46+
});
47+
48+
it('warns for pivot widgets too', () => {
49+
const warnings = validateWidgetBindings(reproStack({ type: 'pivot' }));
50+
expect(warnings).toHaveLength(1);
51+
expect(warnings[0].message).toContain("'pivot' widget");
52+
});
53+
54+
it('is keyed on the WIDGET binding — selecting the sum measure is clean', () => {
55+
expect(validateWidgetBindings(reproStack({ values: ['total_amount'] }))).toHaveLength(0);
56+
});
57+
58+
it('mixed count + non-count selection is clean', () => {
59+
expect(validateWidgetBindings(reproStack({ values: ['report_count', 'total_amount'] }))).toHaveLength(0);
60+
});
61+
62+
it('declaring a dimension on the widget is clean', () => {
63+
expect(validateWidgetBindings(reproStack({ dimensions: ['cost_center'] }))).toHaveLength(0);
64+
});
65+
66+
it('metric widgets are exactly what a count-only binding wants — clean', () => {
67+
expect(validateWidgetBindings(reproStack({ type: 'metric' }))).toHaveLength(0);
68+
});
69+
70+
it('suppressWarnings opts a deliberate single-row table out', () => {
71+
expect(validateWidgetBindings(reproStack({ suppressWarnings: [TABLE_COUNT_ONLY] }))).toHaveLength(0);
72+
});
73+
74+
it('unrelated suppressWarnings entries do not suppress', () => {
75+
expect(validateWidgetBindings(reproStack({ suppressWarnings: ['some-other-rule'] }))).toHaveLength(1);
76+
});
77+
78+
it('skips dangling dataset references (cross-reference finding, not this rule)', () => {
79+
expect(validateWidgetBindings(reproStack({ dataset: 'no_such_dataset' }))).toHaveLength(0);
80+
});
81+
82+
it('skips unresolvable measure names (a different diagnostic)', () => {
83+
expect(validateWidgetBindings(reproStack({ values: ['no_such_measure'] }))).toHaveLength(0);
84+
});
85+
86+
it('treats derived measures as non-count even when aggregate says count', () => {
87+
const stack = reproStack({ values: ['count_ratio'] });
88+
(stack.datasets[0].measures as Record<string, unknown>[]).push({
89+
name: 'count_ratio',
90+
aggregate: 'count',
91+
derived: { op: 'ratio', of: ['report_count', 'report_count'] },
92+
});
93+
expect(validateWidgetBindings(stack)).toHaveLength(0);
94+
});
95+
96+
it('count_distinct is a deliberate analytic — clean', () => {
97+
const stack = reproStack({ values: ['unique_requesters'] });
98+
(stack.datasets[0].measures as Record<string, unknown>[]).push({
99+
name: 'unique_requesters',
100+
aggregate: 'count_distinct',
101+
field: 'requester',
102+
});
103+
expect(validateWidgetBindings(stack)).toHaveLength(0);
104+
});
105+
106+
it('handles map-keyed datasets/dashboards collections', () => {
107+
const arrayForm = reproStack();
108+
const { name: _dsName, ...dsRest } = arrayForm.datasets[0];
109+
const { name: _dashName, ...dashRest } = arrayForm.dashboards[0];
110+
const stack = {
111+
datasets: { expense_report_metrics: dsRest },
112+
dashboards: { expenses_overview_dashboard: dashRest },
113+
};
114+
const warnings = validateWidgetBindings(stack);
115+
expect(warnings).toHaveLength(1);
116+
expect(warnings[0].where).toContain('expenses_overview_dashboard');
117+
});
118+
119+
it('is silent on stacks without dashboards or datasets', () => {
120+
expect(validateWidgetBindings({})).toHaveLength(0);
121+
expect(validateWidgetBindings({ dashboards: [], datasets: [] })).toHaveLength(0);
122+
});
123+
});
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Build-time dashboard widget binding diagnostics (issue #1719).
5+
*
6+
* Runs at `objectstack compile`/`build` AFTER the stack has been schema-parsed,
7+
* so every widget's `dataset` reference can be linked to its `defineDataset`
8+
* and each entry in `values` resolved to a concrete measure with a known
9+
* `aggregate`. This is the semantic/cross-reference phase — the rules here
10+
* cannot run during plain Zod parsing of the raw widget literal.
11+
*
12+
* Rule `table-count-only`: a `table` / `pivot` widget whose selected measures
13+
* are ALL `aggregate: 'count'` and which declares no `dimensions` asks the
14+
* analytics service for a single summary row. That is the shape a `metric`
15+
* widget wants — for a table it almost always means the author wanted a
16+
* per-record listing, which is not an analytics dataset at all (model it as an
17+
* object-bound ListView, ADR-0017). The signal is evaluated on the WIDGET's
18+
* binding, not the dataset: a dataset may well carry other measures and
19+
* dimensions the widget simply doesn't select.
20+
*
21+
* Warnings are non-fatal by design — `objectstack build` stays green — and a
22+
* deliberate single-row table can opt out per widget via
23+
* `suppressWarnings: ['table-count-only']`.
24+
*/
25+
26+
export const TABLE_COUNT_ONLY = 'table-count-only';
27+
28+
export interface WidgetBindingWarning {
29+
/** Diagnostic rule id (registry entry), e.g. `table-count-only`. */
30+
rule: string;
31+
/** Human-readable location, e.g. `dashboard "x" › widget "y"`. */
32+
where: string;
33+
/** Config path, e.g. `dashboards[0].widgets[3]`. */
34+
path: string;
35+
/** What is wrong. */
36+
message: string;
37+
/** How to fix (or deliberately suppress) it. */
38+
hint: string;
39+
}
40+
41+
type AnyRec = Record<string, unknown>;
42+
43+
/** Coerce a collection (array or name-keyed map) to an array. */
44+
function asArray(v: unknown): AnyRec[] {
45+
if (Array.isArray(v)) return v as AnyRec[];
46+
if (v && typeof v === 'object') {
47+
return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) }));
48+
}
49+
return [];
50+
}
51+
52+
/**
53+
* Validate every dashboard widget's dataset binding. Returns the list of
54+
* warnings (empty = clean). Caller decides how to surface them — these are
55+
* advisory and must never fail the build on their own.
56+
*/
57+
export function validateWidgetBindings(stack: AnyRec): WidgetBindingWarning[] {
58+
const warnings: WidgetBindingWarning[] = [];
59+
60+
const datasets = new Map<string, AnyRec>();
61+
for (const ds of asArray(stack.datasets)) {
62+
if (typeof ds.name === 'string') datasets.set(ds.name, ds);
63+
}
64+
65+
const dashboards = asArray(stack.dashboards);
66+
for (let i = 0; i < dashboards.length; i++) {
67+
const dash = dashboards[i];
68+
const dashName = typeof dash.name === 'string' ? dash.name : `(dashboard ${i})`;
69+
const widgets = Array.isArray(dash.widgets) ? (dash.widgets as AnyRec[]) : [];
70+
71+
for (let j = 0; j < widgets.length; j++) {
72+
const w = widgets[j];
73+
if (w.type !== 'table' && w.type !== 'pivot') continue;
74+
// Grouped by at least one dimension → genuinely aggregated rows.
75+
if (Array.isArray(w.dimensions) && w.dimensions.length > 0) continue;
76+
// Author opted out — a single-row summary table is intentional here.
77+
if (Array.isArray(w.suppressWarnings) && w.suppressWarnings.includes(TABLE_COUNT_ONLY)) continue;
78+
79+
const dsName = typeof w.dataset === 'string' ? w.dataset : undefined;
80+
const dataset = dsName ? datasets.get(dsName) : undefined;
81+
// A dangling dataset reference is a cross-reference finding, not this rule.
82+
if (!dataset) continue;
83+
84+
const measures = new Map<string, AnyRec>();
85+
for (const m of asArray(dataset.measures)) {
86+
if (typeof m.name === 'string') measures.set(m.name, m);
87+
}
88+
89+
const values = Array.isArray(w.values)
90+
? (w.values as unknown[]).filter((v): v is string => typeof v === 'string')
91+
: [];
92+
if (values.length === 0) continue;
93+
const resolved = values.map((v) => measures.get(v));
94+
// An unresolvable measure name is a different diagnostic — don't guess.
95+
if (resolved.some((m) => !m)) continue;
96+
97+
// Derived measures combine other measures; treat them as non-count even
98+
// when their (ignored) `aggregate` says otherwise.
99+
const countOnly = resolved.every((m) => m!.aggregate === 'count' && !m!.derived);
100+
if (!countOnly) continue;
101+
102+
const widgetId = typeof w.id === 'string' ? w.id : `(widget ${j})`;
103+
warnings.push({
104+
rule: TABLE_COUNT_ONLY,
105+
where: `dashboard "${dashName}" › widget "${widgetId}"`,
106+
path: `dashboards[${i}].widgets[${j}]`,
107+
message:
108+
`a '${w.type}' widget bound to dataset "${dsName}" selects only count ` +
109+
`measure(s) (${values.join(', ')}) and no dimensions, so it renders a ` +
110+
`single summary row — not a per-record list.`,
111+
hint:
112+
`A flat record listing is not an analytics dataset. Model it as an ` +
113+
`object-bound ListView (ADR-0017) surfaced through app navigation, and ` +
114+
`use a 'metric' widget here if you only need the count. If a single-row ` +
115+
`table is intentional, add an explicit dimension or suppress with: ` +
116+
`suppressWarnings: ['${TABLE_COUNT_ONLY}']`,
117+
});
118+
}
119+
}
120+
121+
return warnings;
122+
}

packages/spec/src/ui/dashboard.zod.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,13 @@ export const DashboardWidgetSchema = lazySchema(() => z.object({
176176
/** Widget specific options (colors, legend, etc.) */
177177
options: z.unknown().optional().describe('Widget specific configuration'),
178178

179+
/**
180+
* Rule ids of build diagnostics intentionally suppressed on this widget
181+
* (e.g. `'table-count-only'` when a single-row summary table is deliberate).
182+
* Consumed by `objectstack build` / `objectstack lint`; no runtime effect.
183+
*/
184+
suppressWarnings: z.array(z.string()).optional().describe('Build diagnostic rule ids suppressed on this widget'),
185+
179186
/** Responsive layout overrides per breakpoint */
180187
responsive: ResponsiveConfigSchema.optional().describe('Responsive layout configuration'),
181188

0 commit comments

Comments
 (0)