From 9fd7194451f77bb344d20af7cf35bc6158da5853 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 05:03:39 +0000 Subject: [PATCH 1/4] feat(spec)!: converge dashboard widget compareTo on the executor contract (#5011) WIP: schema + contract + executor resolution rule + ADR-0087 D2/D3. --- .../service-analytics/src/dataset-executor.ts | 72 +++++- .../spec/src/contracts/analytics-service.ts | 21 +- packages/spec/src/conversions/registry.ts | 112 ++++++++++ packages/spec/src/migrations/registry.ts | 48 +++- packages/spec/src/shared/strict-object.ts | 33 ++- packages/spec/src/ui/dashboard.zod.ts | 210 ++++++++++++------ 6 files changed, 416 insertions(+), 80 deletions(-) diff --git a/packages/services/service-analytics/src/dataset-executor.ts b/packages/services/service-analytics/src/dataset-executor.ts index aff378a66b..d86da68349 100644 --- a/packages/services/service-analytics/src/dataset-executor.ts +++ b/packages/services/service-analytics/src/dataset-executor.ts @@ -474,6 +474,63 @@ function shiftYear(date: string, years: number): string { return toISODate(d.getTime()); } +/** + * Resolve which time dimension a `compareTo` shifts (#5011). + * + * `DatasetCompareTo.dimension` is optional, and this is the rule that makes the + * omission deterministic. It is an EXECUTOR resolution rule, not consumer-side + * tolerance (PD #12): the choice is made once, here, at the producer of the + * comparison — so a dashboard widget, a report, and a raw `queryDataset` caller + * that all omit it get the same dimension or the same error, and no renderer is + * ever tempted to guess one on their behalf. + * + * Candidates use the executor's own long-standing criterion, unchanged: a + * `timeDimensions` entry that carries a `dateRange`. That is exactly the set + * {@link shiftRange} can act on — a time dimension with no window has nothing to + * shift — so the resolution can never select a dimension the shift then fails on. + * + * Ambiguity is loud, never silently first-wins: picking `created_at` when the + * author meant `close_date` produces a comparison column that is wrong rather + * than missing, which is the failure mode nobody audits. The message names every + * candidate so the fix is a copy-paste. + */ +function resolveCompareDimension(selection: DatasetSelection): string { + const cmp = selection.compareTo!; + const shiftable = (selection.timeDimensions ?? []).filter( + (t) => (t as { dateRange?: unknown }).dateRange != null, + ); + const names = shiftable.map((t) => t.dimension); + + if (cmp.dimension != null) { + if (!names.includes(cmp.dimension)) { + throw new Error( + `[dataset-executor] compareTo requires a timeDimension "${cmp.dimension}" with a dateRange. ` + + (names.length > 0 + ? `This selection dates ${names.map((n) => `"${n}"`).join(', ')} — name one of those, or omit compareTo.dimension to let the executor choose when there is only one.` + : 'This selection declares no timeDimension with a dateRange, so there is no window to shift; give the dimension a dateRange (a dashboard date-range filter is the usual source).'), + ); + } + return cmp.dimension; + } + + if (names.length === 1) return names[0]; + + if (names.length === 0) { + throw new Error( + '[dataset-executor] compareTo needs a dated window to shift, but this selection declares no ' + + 'timeDimension with a dateRange. Give the time dimension a dateRange (a dashboard date-range ' + + 'filter is the usual source), or drop compareTo — a period-over-period comparison is only ' + + 'defined against a bounded window.', + ); + } + + throw new Error( + `[dataset-executor] compareTo.dimension is ambiguous: ${names.length} time dimensions carry a ` + + `dateRange (${names.map((n) => `"${n}"`).join(', ')}). Name the one to shift — ` + + `compareTo: { kind: '${cmp.kind}', dimension: '${names[0]}' }.`, + ); +} + /** Compute the comparison window for a [start,end] range. */ export function shiftRange(range: [string, string], kind: CompareTo['kind']): [string, string] { const [start, end] = range; @@ -868,18 +925,17 @@ export class DatasetExecutor { context?: ExecutionContext, ): Promise[]> { const cmp = selection.compareTo!; - const td = (selection.timeDimensions ?? []).find((t) => t.dimension === cmp.dimension); - if (!td || !td.dateRange) { - throw new Error( - `[dataset-executor] compareTo requires a timeDimension "${cmp.dimension}" with a dateRange.`, - ); - } + // `dimension` is optional since #5011; resolve it (or fail loudly) before + // touching a window. Both the "which one?" and the "with a dateRange" + // questions are answered in one place — see `resolveCompareDimension`. + const dimension = resolveCompareDimension(selection); + const td = (selection.timeDimensions ?? []).find((t) => t.dimension === dimension)!; const range: [string, string] = Array.isArray(td.dateRange) ? [td.dateRange[0], td.dateRange[1] ?? td.dateRange[0]] - : [td.dateRange, td.dateRange]; + : [td.dateRange as string, td.dateRange as string]; const shifted = shiftRange(range, cmp.kind); const shiftedTd = (selection.timeDimensions ?? []).map((t) => - t.dimension === cmp.dimension ? { ...t, dateRange: shifted } : t, + t.dimension === dimension ? { ...t, dateRange: shifted } : t, ); // Run the SAME pass the current period ran, over the shifted window: same // measures, same dimensions, same base filter, and — since #4820 — the same diff --git a/packages/spec/src/contracts/analytics-service.ts b/packages/spec/src/contracts/analytics-service.ts index d9d3d275e9..672bb615b7 100644 --- a/packages/spec/src/contracts/analytics-service.ts +++ b/packages/spec/src/contracts/analytics-service.ts @@ -104,8 +104,25 @@ export interface CubeMeta { export interface DatasetCompareTo { /** previousPeriod = equal-length window immediately before; previousYear = same window −1y. */ kind: 'previousPeriod' | 'previousYear'; - /** The time dimension (by name) whose dateRange is shifted. */ - dimension: string; + /** + * The time dimension (by name) whose `dateRange` is shifted. + * + * **Optional since #5011, resolved by the EXECUTOR — not by any consumer.** + * When omitted the executor takes the selection's shiftable time dimensions + * (its own long-standing criterion: a `timeDimensions` entry carrying a + * `dateRange`) and: + * + * - exactly one candidate → that one is shifted; + * - zero candidates → throws, saying a comparison needs a dated window; + * - two or more → throws, listing the candidates by name so the author can + * pick one. + * + * The ambiguous and empty cases are LOUD by design. A consumer must never + * paper over them by guessing a dimension (PD #12): the resolution rule + * lives at the producer of the comparison — the executor — precisely so + * every caller gets the same answer or the same error. + */ + dimension?: string; } /** diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 0b4f8231e1..59c81f4518 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -2284,6 +2284,117 @@ const dashboardWidgetResponsiveRemoved: MetadataConversion = { }, }; +/** + * dashboard.widgets[].compareTo (#5011) — a VOCABULARY convergence, not a + * removal: the widget's three declared arms are replaced by the one shape the + * analytics executor implements, `{ kind, dimension? }` + * (`DatasetSelection.compareTo`). + * + * Why a conversion rather than a plain tombstone: two of the three arms have an + * exact target, so leaving them for the author to retype would be make-work on + * a rewrite a machine can prove. + * + * 'previousPeriod' → { kind: 'previousPeriod' } value verbatim, container changed + * 'previousYear' → { kind: 'previousYear' } value verbatim, container changed + * { offset: '1y' } → { kind: 'previousYear' } '1y' IS previousYear, by definition + * + * `dimension` is deliberately NOT synthesised. The conversion sees a stack, not + * a dataset — it cannot know which time dimension carries the window — and it + * does not need to: `dimension` is optional precisely so the executor can + * resolve it (one dated candidate) or refuse loudly (zero, or several). Writing + * a guess here would convert a loud runtime error into a wrong comparison. + * + * Every OTHER `{ offset }` (`'7d'`, `'1M'`, `'2w'`, …) is left UNTOUCHED, on + * purpose. There is no faithful target: `previousPeriod` shifts by the resolved + * window's own length, which equals `7d` only when the window happens to be + * seven days. Rewriting it would silently change which rows the comparison + * column counts — the failure class this issue exists to end. So the source + * keeps the key, the strict schema rejects it with `COMPARE_TO_OFFSET_RETIRED` + * (which prescribes kind + filter-window), and the residue is declared as the + * `dashboard-widget-compareto-offset` semantic migration. + * + * Measured before writing (#5011's adjudication asked for stored/authored + * instances first): repo-wide, four authored instances, all of them the string + * form, all in `examples/app-crm`; ZERO `{ offset }` instances in either repo. + * Expected — a canonical-path author who tried `{ offset }` got a thrown widget, + * so it could never accumulate there. + * + * `retiredFromLoadPath: true`: the old spellings get NO acceptance window. An + * auto-converting loader would let `compareTo: 'previousPeriod'` keep parsing + * clean, which is the lenient-consumer shape PD #12 forbids and the exact + * dynamic that let the widget and executor vocabularies drift apart unnoticed + * for a whole major. Stored `sys_metadata` rows are still covered — every + * rehydration seam replays retired entries via `applyConversionsToStoredItem` + * (#3903). + */ +const dashboardWidgetCompareToConverged: MetadataConversion = { + id: 'dashboard-widget-compareto-converged', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'dashboard.widgets[].compareTo', + summary: + "dashboard widget 'compareTo' converged on the executor's { kind, dimension? } contract " + + "(#5011 — the bare strings and { offset: '1y' } rewrite mechanically; other { offset } " + + 'durations have no faithful target and are reported, not guessed)', + apply(stack, emit) { + return mapCollection(stack, 'dashboards', (d, path) => { + const widgets = d.widgets; + if (!Array.isArray(widgets)) return d; + let touched = false; + const rebuilt = widgets.map((w, i) => { + if (!w || typeof w !== 'object' || Array.isArray(w)) return w; + const widget = w as Record; + if (!('compareTo' in widget)) return w; + const cmp = widget.compareTo; + const at = `${path}.widgets[${i}].compareTo`; + + // Arm 1/2 — the bare string form. The value survives verbatim. + if (cmp === 'previousPeriod' || cmp === 'previousYear') { + emit({ from: `'${cmp}'`, to: `{ kind: '${cmp}' }`, path: at }); + touched = true; + return { ...widget, compareTo: { kind: cmp } }; + } + + // Arm 3 — `{ offset }`, and only the one duration with an exact target. + if (cmp && typeof cmp === 'object' && !Array.isArray(cmp)) { + const offset = (cmp as Record).offset; + if (offset === '1y') { + emit({ from: "{ offset: '1y' }", to: "{ kind: 'previousYear' }", path: at }); + touched = true; + return { ...widget, compareTo: { kind: 'previousYear' } }; + } + } + return w; + }); + if (!touched) return d; + return { ...d, widgets: rebuilt }; + }); + }, + fixture: { + before: { + dashboards: [{ + name: 'revenue_review', + widgets: [ + { id: 'w1', type: 'kpi', dataset: 'orders', values: ['total'], compareTo: 'previousPeriod' }, + { id: 'w2', type: 'kpi', dataset: 'orders', values: ['total'], compareTo: 'previousYear' }, + { id: 'w3', type: 'kpi', dataset: 'orders', values: ['total'], compareTo: { offset: '1y' } }, + ], + }], + }, + after: { + dashboards: [{ + name: 'revenue_review', + widgets: [ + { id: 'w1', type: 'kpi', dataset: 'orders', values: ['total'], compareTo: { kind: 'previousPeriod' } }, + { id: 'w2', type: 'kpi', dataset: 'orders', values: ['total'], compareTo: { kind: 'previousYear' } }, + { id: 'w3', type: 'kpi', dataset: 'orders', values: ['total'], compareTo: { kind: 'previousYear' } }, + ], + }], + }, + expectedNotices: 3, + }, +}; + /** * agent.knowledge — a grounding claim nothing enforced (the RAG path reads * `sourceIds` from the LLM's tool-call arguments, never the agent record). @@ -3727,6 +3838,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly__compare` column over the window its author intended.', + }, { id: 'job-retry-policy-constraints-tightened', surface: 'job.retryPolicy.maxRetries (> 10) / job.retryPolicy.backoffMultiplier (< 1)', diff --git a/packages/spec/src/shared/strict-object.ts b/packages/spec/src/shared/strict-object.ts index 4bfd7e2ffb..400eab94df 100644 --- a/packages/spec/src/shared/strict-object.ts +++ b/packages/spec/src/shared/strict-object.ts @@ -129,6 +129,23 @@ export interface StrictObjectOptions { * keys here keeps the suggestion useful on the extended surface. */ extraKeys?: readonly string[]; + /** + * Prescriptions for a retired **value form** of this slot — a scalar that + * used to be legal where an object is now required. Keyed by the exact + * authored value and dispatched on `issue.input`, so only the spelling that + * really was legal gets the retirement text and every other wrong type keeps + * zod's own message (the `HookBodyCapability` / `object.managedBy: 'system'` + * precedent — telling the author of `previosPeriod` that their value "was + * removed" would misinform). + * + * `guidance` cannot reach this case: it is consulted for + * `unrecognized_keys`, which never fires when the input is not an object at + * all. Without this hook a slot that converged from `'previousPeriod'` to + * `{ kind: 'previousPeriod' }` rejects the old spelling with the bare + * `Invalid input: expected object, received string` — loud, but carrying + * none of the upgrade the author needs. Added for #5011. + */ + retiredForms?: Readonly>; } /** @@ -137,7 +154,7 @@ export interface StrictObjectOptions { * list read from `shape` rather than transcribed alongside it. */ export function strictObject(options: StrictObjectOptions, shape: T) { - const { surface, history, aliases, guidance, extraKeys = [] } = options; + const { surface, history, aliases, guidance, extraKeys = [], retiredForms } = options; // The error map is built on FIRST USE, not at construction. // @@ -159,8 +176,17 @@ export function strictObject(options: StrictObjectOptio // than making each of them prove it is not in a loop. Same shape as the // deferred map `data/object.zod.ts` already carries for its TDZ problem. let build: z.core.$ZodErrorMap | undefined; - const error: z.core.$ZodErrorMap = (issue) => - (build ??= strictUnknownKeyError({ + const error: z.core.$ZodErrorMap = (issue) => { + // A retired VALUE FORM is rejected before the unknown-key map is even + // consulted: `issue.code` here is `invalid_type` (the input is not an + // object), so `strictUnknownKeyError` would return undefined and zod's + // bare "expected object, received string" would be all the author sees. + if (retiredForms && issue.code === 'invalid_type') { + const prescription = + typeof issue.input === 'string' ? retiredForms[issue.input] : undefined; + if (prescription) return prescription; + } + return (build ??= strictUnknownKeyError({ surface, // Declared-but-unwritable keys (tombstones) are excluded — see // `acceptsNothing`. They stay in the SHAPE, so writing one still raises @@ -174,6 +200,7 @@ export function strictObject(options: StrictObjectOptio aliases, guidance, }))(issue); + }; return z.object(shape, { error }).strict(); } diff --git a/packages/spec/src/ui/dashboard.zod.ts b/packages/spec/src/ui/dashboard.zod.ts index a56c7730b0..d0ade7658e 100644 --- a/packages/spec/src/ui/dashboard.zod.ts +++ b/packages/spec/src/ui/dashboard.zod.ts @@ -215,6 +215,49 @@ export const DashboardWidgetOptionsSchema = lazySchema(() => z.object({ .describe('Explicit category order for funnel/pyramid stages (stored values)'), }).passthrough().describe('Widget configuration — declared query keys + open renderer extras')); +// ── `compareTo` convergence prescriptions (#5011) ──────────────────────────── +// +// Declared with `//` rather than `/** */` on purpose (the `CRYPTO_HASH_RETIRED` +// house style in `data/hook-body.zod.ts`): build-docs lifts JSDoc onto the +// reference page, and a retirement prescription is an upgrade note, not a doc +// for a shape that still exists. +// +// `{ offset }` promised a shift by an explicit duration. Nothing on the ADR-0021 +// dataset path could run it: the executor's comparison contract is +// `{ kind, dimension? }` and there is no `offset` concept anywhere in it, so a +// forwarded `{ offset }` reached `dataset-executor.ts` with `dimension: +// undefined` and threw. It ran only on the legacy inline chart path. Rather than +// implement calendar-offset arithmetic (whose month-length and leap-year corner +// cases are a silent-wrong-window bug farm) the arm retires: `'1y'` already has +// an exact equivalent, and the rest are expressible as a kind plus the window +// the widget's own `filter` resolves to. +const COMPARE_TO_OFFSET_RETIRED = + '`dashboard.widgets[].compareTo.offset` was removed in @objectstack/spec 17.0.0 (#5011, ' + + 'ADR-0049 enforce-or-remove) — the analytics executor never had an `offset` concept, so on the ' + + 'ADR-0021 dataset path this arm did not shift a window, it threw ' + + '(`compareTo requires a timeDimension "undefined"`) and took the whole widget down with it. ' + + "Write the kind instead: `compareTo: { kind: 'previousPeriod' }` for the equal-length window " + + "immediately before, `compareTo: { kind: 'previousYear' }` for the same window a calendar year " + + "back — `{ offset: '1y' }` is exactly `previousYear`. For any other duration " + + "(`'7d'`, `'1M'`, …) there is no faithful one-key rewrite: state the window you want on the " + + "widget's own `filter` and compare it with `previousPeriod`, which shifts by whatever length " + + 'that window resolves to. ' + + 'Run `os migrate meta --from 16` to rewrite the `1y` case automatically; the other durations ' + + 'are reported for you to re-state.'; + +// The two string arms. They parsed, and on a dataset widget they then did +// NOTHING — DatasetWidget dropped them deliberately, because forwarding one made +// the executor throw. The value survives the rewrite verbatim; only its +// container changes, which is what makes this a mechanical conversion. +const COMPARE_TO_STRING_RETIRED = (kind: 'previousPeriod' | 'previousYear') => + `\`dashboard.widgets[].compareTo: '${kind}'\` (the bare string form) was removed in ` + + '@objectstack/spec 17.0.0 (#5011) — the ADR-0021 dataset renderer silently DROPPED it, so the ' + + 'widget rendered its base numbers with the comparison the author asked for quietly absent. ' + + `Write \`compareTo: { kind: '${kind}' }\` instead — same comparison, spelled the way the ` + + 'analytics executor actually reads it (`DatasetSelection.compareTo`). Add `dimension` only ' + + 'when the selection has more than one dated time dimension; with one, the executor resolves ' + + 'it. Run `os migrate meta --from 16` to rewrite it automatically.'; + /** * Dashboard Widget Schema * A single component on the dashboard grid. @@ -268,75 +311,110 @@ export const DashboardWidgetSchema = lazySchema(() => z.object({ filter: FilterConditionSchema.optional().describe('Presentation-scope filter (runtimeFilter)'), /** - * Period-over-period comparison primitive. + * Period-over-period comparison window. + * + * When set, the runtime runs a second query against a shifted time window + * and attaches a `__compare` column per selected measure; metric + * widgets show a secondary value + arrow, chart widgets render a + * muted/dashed overlay series. + * + * - `kind: 'previousPeriod'` — the equal-length window immediately before + * the resolved one. + * - `kind: 'previousYear'` — the same window shifted back one calendar year. + * - `dimension` — OPTIONAL. Which time dimension's window to shift; omit it + * and the executor resolves it (see below). + * + * ## Why this shape (#5011) + * + * This is a **thin projection of `DatasetSelection.compareTo`** + * (`contracts/analytics-service.ts`) — the one comparison contract the + * analytics executor actually implements. Until #5011 the widget declared a + * different vocabulary from the executor and the two never met: the two + * string arms were dropped on the floor by the dataset renderer (a + * comparison silently absent from a widget whose author had asked for one) + * and `{ offset }` was forwarded verbatim into a contract with no `offset` + * in it, so the executor threw `compareTo requires a timeDimension + * "undefined"` and the whole widget errored. Every arm was broken on the + * ADR-0021 dataset path — the path this spec calls the single author-facing + * analytics shape — while all three worked on the legacy inline chart path. + * Same key, two fates, the failing one blessed. + * + * Converging on the executor's own words makes `declared = enforced` true by + * construction rather than by vigilance: there is no widget-side vocabulary + * left to drift. It also leaves the slot union-free, which matters more than + * it looks — a union collapses into one bare `Invalid input` on the wire + * (#5014), so every curated message this campaign puts inside a union arm is + * written for a reader who never receives it. A plain strict object's errors + * reach the author. + * + * ## Resolving `dimension` * - * When set, the renderer runs a second query against a shifted time - * window and surfaces the delta (metric widgets show a secondary - * value + arrow; chart widgets render a muted/dashed overlay series). + * Omit it and the EXECUTOR resolves it, by its own criterion: the selection's + * time dimensions that carry a `dateRange`. Exactly one candidate is used; + * zero or several is a loud error naming what it found. That rule lives in + * `dataset-executor.ts` — deliberately at the producer of the comparison, not + * as a renderer-side guess, so every caller gets the same answer or the same + * error (PD #12). * - * - `'previousPeriod'` — auto-detect the comparison window from the - * widget's `filter` date macros (e.g. `{current_month_start}` → - * `{last_month_start}`). Falls back to no comparison when the - * filter contains no resolvable date range. - * - `'previousYear'` — shift the resolved filter window back by one - * calendar year. - * - `{ offset: '7d' | '1M' | '1y' }` — shift by an explicit - * ISO-8601-like duration. Units: `d` (days), `w` (weeks), - * `M` (months), `y` (years). + * `{ offset: '7d' | '1M' | '1y' }` was REMOVED in the same change; see + * `COMPARE_TO_OFFSET_RETIRED` below. */ - compareTo: z.union([ - z.literal('previousPeriod'), - z.literal('previousYear'), - // #4001 批 14: the object arm is closed. `DashboardWidgetSchema` has been - // strict since the ADR-0021 cutover, but STRICTNESS DOES NOT RECURSE — so - // `compareTo: { offset: '7d', granularity: 'month' }` parsed clean on `main` - // and came back `{ offset: '7d' }`, the widget rendering a comparison the - // author did not describe. A strict container around strip children is the - // silhouette of a closed surface, not a closed one. - // - // ⚠️ KNOWN REACH LIMIT, measured rather than assumed — this closure REJECTS - // reliably but its PROSE does not currently reach the author. `compareTo` is - // a union, and zod collapses a failed union into one top-level - // `invalid_union` issue whose message is the bare `Invalid input`; the arm - // errors (including the guidance below) live in `issue.errors`, and - // `zodIssuesToFields` in `rest/src/rest-server.ts` maps only top-level - // issues, so nothing carries them onto the wire. The rejection is still the - // #4001 win — a silent half-discard became a hard failure at `compareTo`. - // The transport gap is #5014, and it affects every curated - // unknown-key message this campaign has put inside a union arm, not just - // this one. `strictness-batch14.test.ts` pins BOTH halves: the bare - // top-level text an author sees today, and the guidance waiting in the arm - // errors — split deliberately, so a green test cannot stand in for a message - // no consumer prints. - strictObject({ - surface: 'this comparison window', - history: DASHBOARD_HISTORY, - // The neighbouring vocabularies for "shift a time window": the widget's own - // string arms (`previousPeriod` / `previousYear`) spelled as an object, and - // the date-macro / granularity words used elsewhere on this same widget. - aliases: { - period: 'offset', - duration: 'offset', - interval: 'offset', - shift: 'offset', - delta: 'offset', - by: 'offset', - amount: 'offset', - value: 'offset', - }, - guidance: { - // Naming an arm of this very union from inside its object arm. - type: 'the comparison KIND is the union itself, not a key: write `compareTo: \'previousPeriod\'` or `compareTo: \'previousYear\'` as a bare string. The object arm exists only for an explicit shift — `compareTo: { offset: \'7d\' }`.', - kind: 'the comparison KIND is the union itself, not a key: write `compareTo: \'previousPeriod\'` or `compareTo: \'previousYear\'` as a bare string. The object arm exists only for an explicit shift — `compareTo: { offset: \'7d\' }`.', - mode: 'the comparison KIND is the union itself, not a key: write `compareTo: \'previousPeriod\'` or `compareTo: \'previousYear\'` as a bare string. The object arm exists only for an explicit shift — `compareTo: { offset: \'7d\' }`.', - // Two real slots one level up, both easy to reach for here. - granularity: 'a comparison window carries no granularity — the shift is a whole duration (`7d` / `1M` / `1y`). Date bucketing is declared on the DATASET dimension (`dateGranularity`), which every widget bound to that dataset then shares.', - filter: '`filter` is the widget\'s own presentation-scope key, one level up — `compareTo` shifts whatever window that filter already resolves to. Move it out of `compareTo`.', - }, - }, { - offset: z.string().regex(/^\d+[dwMy]$/, 'Offset must match (d|w|M|y), e.g. "7d", "1M", "1y"'), - }), - ]).optional().describe('Period-over-period comparison window'), + compareTo: strictObject({ + surface: 'this comparison window', + history: DASHBOARD_HISTORY, + // Near-misses for the two live keys. The `kind` entries are not invented: + // #5042 curated `type`/`mode` here as guidance ("the comparison KIND is the + // union itself, not a key"), i.e. it had already measured that authors + // reach for those words on this slot. Now that `kind` IS the key, the same + // claim becomes a faithful rename. The `dimension` entries name the + // neighbouring vocabulary for "which date column": `dateRange.field` on the + // dashboard filter bar and `timeDimensions[].dimension` on the wire. + aliases: { + type: 'kind', + mode: 'kind', + field: 'dimension', + dateField: 'dimension', + timeDimension: 'dimension', + }, + guidance: { + // The retired `{ offset }` arm and every word #5042 measured authors + // spelling it with. All resolve to one prescription: an explicit-duration + // shift has no executor behind it and never had one. + offset: COMPARE_TO_OFFSET_RETIRED, + period: COMPARE_TO_OFFSET_RETIRED, + duration: COMPARE_TO_OFFSET_RETIRED, + interval: COMPARE_TO_OFFSET_RETIRED, + shift: COMPARE_TO_OFFSET_RETIRED, + delta: COMPARE_TO_OFFSET_RETIRED, + by: COMPARE_TO_OFFSET_RETIRED, + amount: COMPARE_TO_OFFSET_RETIRED, + value: COMPARE_TO_OFFSET_RETIRED, + // Two real slots one level up, both easy to reach for from in here. + granularity: 'a comparison window carries no granularity — it shifts a window, it does not bucket one. Date bucketing is `options.dateGranularity` on this widget (or the DATASET dimension\'s own `dateGranularity` default), and the comparison pass reuses whatever the primary pass resolved.', + filter: '`filter` is the widget\'s own presentation-scope key, one level up — `compareTo` shifts whatever window that filter already resolves to. Move it out of `compareTo`.', + }, + // The two spellings that really were legal until #5011 — and, being the + // documented ones, overwhelmingly the shape an upgrading source carries. + retiredForms: { + previousPeriod: COMPARE_TO_STRING_RETIRED('previousPeriod'), + previousYear: COMPARE_TO_STRING_RETIRED('previousYear'), + }, + }, { + /** + * Which comparison window to run — the executor's own two kinds + * (`DatasetCompareTo.kind`). + */ + kind: z.enum(['previousPeriod', 'previousYear']) + .describe('Comparison window: previousPeriod (equal-length, immediately before) or previousYear (−1 calendar year)'), + /** + * The time dimension (by name) whose window is shifted. Omit it when the + * selection has exactly one dated time dimension — the executor resolves it + * and errors loudly, listing the candidates, when the choice is ambiguous + * or there is nothing dated to shift. + */ + dimension: z.string().optional() + .describe('Time dimension to shift; omit when the selection has exactly one dated time dimension'), + }).optional().describe('Period-over-period comparison window ({ kind, dimension? })'), /** * ADR-0021 — the semantic-layer `dataset` this widget binds to. The widget From 38ecce6ef3c688894919f69a6ec2a1de92d0381f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 05:26:10 +0000 Subject: [PATCH 2/4] test(spec): pin the converged compareTo shape (#5011) --- content/docs/ui/dashboards.mdx | 2 +- .../src/dashboards/pipeline.dashboard.ts | 32 ++- examples/app-crm/test/smoke.test.ts | 18 +- packages/spec/liveness/dashboard.json | 5 +- .../spec/src/ui/dashboard-compareto.test.ts | 266 ++++++++++++++++++ skills/objectstack-query/rules/aggregation.md | 10 +- skills/objectstack-ui/SKILL.md | 47 +++- 7 files changed, 337 insertions(+), 43 deletions(-) create mode 100644 packages/spec/src/ui/dashboard-compareto.test.ts diff --git a/content/docs/ui/dashboards.mdx b/content/docs/ui/dashboards.mdx index 35fdc69139..083a8eced1 100644 --- a/content/docs/ui/dashboards.mdx +++ b/content/docs/ui/dashboards.mdx @@ -104,7 +104,7 @@ selects `dimensions` (X / group / split) and `values` (the measures to plot): | `layout` | `object` | optional | Grid position and size (auto-flowed into the grid when omitted) | | `chartConfig` | `object` | optional | Advanced chart configuration | | `colorVariant` | `enum` | optional | KPI/card accent color | -| `compareTo` | `enum \| object` | optional | Period-over-period comparison window | +| `compareTo` | `object` | optional | Period-over-period comparison: `{ kind: 'previousPeriod' \| 'previousYear', dimension? }`. Omit `dimension` when the selection dates exactly one time dimension — the runtime resolves it, and errors naming the candidates rather than guessing when it cannot. | | `options` | `object` | optional | Renderer extras **plus** the query keys below | ### Widget `options` diff --git a/examples/app-crm/src/dashboards/pipeline.dashboard.ts b/examples/app-crm/src/dashboards/pipeline.dashboard.ts index 4d3c5c7ee5..251de4c60a 100644 --- a/examples/app-crm/src/dashboards/pipeline.dashboard.ts +++ b/examples/app-crm/src/dashboards/pipeline.dashboard.ts @@ -5,20 +5,26 @@ import type { Dashboard } from '@objectstack/spec/ui'; /** * Pipeline Dashboard — aggregate view of the sales pipeline. * - * Demonstrates period-over-period comparison via `compareTo`: + * Demonstrates period-over-period comparison via `compareTo`, whose shape is + * `{ kind, dimension? }` — the same contract the analytics executor reads + * (`DatasetSelection.compareTo`). Every widget below omits `dimension`: + * `opportunity_metrics` dates exactly one dimension (`close_date`), so the + * executor resolves it. Name it explicitly (`{ kind: 'previousYear', dimension: + * 'close_date' }`) only on a dataset that dates more than one — the executor + * refuses to guess, and says which candidates it found. * - * - **Won This Quarter** — metric with `compareTo: 'previousPeriod'`. The - * filter uses `{current_quarter_start}` / `{current_quarter_end}`, so + * - **Won This Quarter** — metric with `compareTo: { kind: 'previousPeriod' }`. + * The filter uses `{current_quarter_start}` / `{current_quarter_end}`, so * the renderer issues a parallel aggregate for Q-1 and shows a delta * labelled "vs last quarter". - * - **Avg Deal Size YoY** — metric with `compareTo: 'previousYear'` to - * compare against the same window one year prior. + * - **Avg Deal Size YoY** — metric with `compareTo: { kind: 'previousYear' }` + * to compare against the same window one year prior. * - **Pipeline Trend (12 months)** — line chart with - * `categoryGranularity: 'month'` bucketing and a `compareTo: 'previousYear'` - * overlay, rendered as a dashed muted series on top of the current - * 12-month trend. + * `categoryGranularity: 'month'` bucketing and a + * `compareTo: { kind: 'previousYear' }` overlay, rendered as a dashed muted + * series on top of the current 12-month trend. * - **Opportunities by Stage** — bar chart with - * `compareTo: 'previousPeriod'` to overlay the prior quarter. + * `compareTo: { kind: 'previousPeriod' }` to overlay the prior quarter. * - **Pipeline by Industry** — pie chart without `compareTo` * (pie / donut / funnel ignore overlays even if set). */ @@ -52,7 +58,7 @@ export const PipelineDashboard: Dashboard = { $lte: '{current_quarter_end}', }, }, - compareTo: 'previousPeriod', + compareTo: { kind: 'previousPeriod' }, dataset: 'opportunity_metrics', values: ['total_amount'], options: { format: 'currency', currency: 'USD' }, @@ -70,7 +76,7 @@ export const PipelineDashboard: Dashboard = { $lte: '{current_year_end}', }, }, - compareTo: 'previousYear', + compareTo: { kind: 'previousYear' }, dataset: 'opportunity_metrics', values: ['avg_amount'], options: { format: 'currency', currency: 'USD' }, @@ -86,7 +92,7 @@ export const PipelineDashboard: Dashboard = { filter: { close_date: { $gte: '{1_years_ago}', $lte: '{today}' }, }, - compareTo: 'previousYear', + compareTo: { kind: 'previousYear' }, dataset: 'opportunity_metrics', dimensions: ['close_date'], values: ['opp_count'], @@ -112,7 +118,7 @@ export const PipelineDashboard: Dashboard = { $lte: '{current_quarter_end}', }, }, - compareTo: 'previousPeriod', + compareTo: { kind: 'previousPeriod' }, dataset: 'opportunity_metrics', dimensions: ['stage'], values: ['opp_count'], diff --git a/examples/app-crm/test/smoke.test.ts b/examples/app-crm/test/smoke.test.ts index 1b2d1bed7a..fbd1fa66bb 100644 --- a/examples/app-crm/test/smoke.test.ts +++ b/examples/app-crm/test/smoke.test.ts @@ -132,23 +132,27 @@ describe('Pipeline dashboard', () => { ); }); - it('uses `compareTo: previousPeriod` for the current-quarter KPI', () => { + it('uses `compareTo: { kind: previousPeriod }` for the current-quarter KPI', () => { const w: any = byId.get('won_this_quarter'); - expect(w.compareTo).toBe('previousPeriod'); + // #5011: `compareTo` is the executor's own `{ kind, dimension? }` shape. + // `dimension` is omitted deliberately — `opportunity_metrics` dates exactly + // one dimension, so the executor resolves it (and would error, naming the + // candidates, if it could not). + expect(w.compareTo).toEqual({ kind: 'previousPeriod' }); expect(w.filter.close_date.$gte).toBe('{current_quarter_start}'); expect(w.filter.close_date.$lte).toBe('{current_quarter_end}'); }); - it('uses `compareTo: previousYear` for the YoY KPI', () => { + it('uses `compareTo: { kind: previousYear }` for the YoY KPI', () => { const w: any = byId.get('avg_deal_size_yoy'); - expect(w.compareTo).toBe('previousYear'); + expect(w.compareTo).toEqual({ kind: 'previousYear' }); expect(w.filter.close_date.$gte).toBe('{current_year_start}'); expect(w.filter.close_date.$lte).toBe('{current_year_end}'); }); it('uses a YoY `previousYear` compareTo on the trend chart', () => { const w: any = byId.get('pipeline_trend_90d'); - expect(w.compareTo).toBe('previousYear'); + expect(w.compareTo).toEqual({ kind: 'previousYear' }); expect(w.type).toBe('line'); // ADR-0021 single-form: the date axis is a dataset dimension (its monthly // bucketing lives on the dataset's close_date dimension, not the widget). @@ -160,9 +164,9 @@ describe('Pipeline dashboard', () => { expect((byId.get('pipeline_by_industry') as any).compareTo).toBeUndefined(); }); - it('uses `compareTo: previousPeriod` on the Opportunities by Stage bar chart', () => { + it('uses `compareTo: { kind: previousPeriod }` on the Opportunities by Stage bar chart', () => { const w: any = byId.get('opportunities_by_stage'); - expect(w.compareTo).toBe('previousPeriod'); + expect(w.compareTo).toEqual({ kind: 'previousPeriod' }); expect(w.type).toBe('bar'); }); diff --git a/packages/spec/liveness/dashboard.json b/packages/spec/liveness/dashboard.json index 237e8f76e1..71f0165cf4 100644 --- a/packages/spec/liveness/dashboard.json +++ b/packages/spec/liveness/dashboard.json @@ -100,8 +100,9 @@ "compareTo": { "status": "live", "verifiedAt": "2026-08-03", - "evidence": "objectui @91757a7: packages/plugin-dashboard/src/DashboardRenderer.tsx:495 (passed into the object-chart schema); objectui: packages/core/src/utils/compare-to.ts:23-25 — shiftFilterByCompareTo honours all three declared arms; objectui: packages/plugin-charts/src/ObjectChart.tsx:468-477", - "note": "LIVE ON ONE PATH ONLY — recorded rather than smoothed over, the `action.disabled` precedent. The inline object-provider chart path honours all three arms ('previousPeriod' / 'previousYear' / { offset }) via shiftFilterByCompareTo. The ADR-0021 dataset-bound path — which the spec calls the single author-facing analytics shape — does NOT: DatasetWidget.tsx:163-168 deliberately DROPS the two string arms, and forwards the `{ offset }` object into DatasetSelection.compareTo, whose contract (packages/spec/src/contracts/analytics-service.ts:104-109) is `{ kind, dimension }` — so packages/services/service-analytics/src/dataset-executor.ts:870-876 throws 'compareTo requires a timeDimension \"undefined\"'. Filed as #5011; do not read this `live` as 'works on a dataset widget'." + "verifiedAt": "2026-08-04", + "evidence": "packages/services/service-analytics/src/dataset-executor.ts — runCompare() reads `compareTo.kind` (shiftRange) and resolves `compareTo.dimension` via resolveCompareDimension(); packages/spec/src/contracts/analytics-service.ts DatasetCompareTo is the same `{ kind, dimension? }` shape the widget now declares; objectui @91757a7: packages/plugin-dashboard/src/DatasetWidget.tsx:163-168 (forwards the structured object into DatasetSelection.compareTo)", + "note": "CONVERGED 2026-08-04 (#5011) — this entry SUPERSEDES the path-split record it carried, which is now history rather than the shape. What it recorded was real: the widget declared three arms ('previousPeriod' / 'previousYear' / { offset }) that only the LEGACY inline object-provider chart path could run (objectui packages/core/src/utils/compare-to.ts shiftFilterByCompareTo), while on the ADR-0021 dataset path — the one the spec calls the single author-facing analytics shape — DatasetWidget deliberately DROPPED the two string arms and forwarded `{ offset }` into a contract with no `offset` in it, so dataset-executor.ts threw 'compareTo requires a timeDimension \"undefined\"'. Same key, two fates, the failing one blessed. The fix converged the widget's vocabulary onto the executor's: `compareTo` is now `{ kind, dimension? }`, a thin projection of DatasetSelection.compareTo, so `declared = enforced` holds by construction with no second vocabulary left to drift. `dimension` is optional and resolved BY THE EXECUTOR (exactly one dated time dimension → that one; zero or several → a loud error listing candidates) — a producer-side resolution rule, not the consumer-side tolerance PD #12 forbids. `{ offset }` retired via the ADR-0087 `dashboard-widget-compareto-converged` conversion (+ the `dashboard-widget-compareto-offset` semantic migration for durations with no faithful target). Still LIVE, and now on the canonical path: the reason the verdict is unchanged is that the CONSUMER was never missing, only the agreement about what it consumes. ⚠️ One half is out of this repo: objectui's legacy inline chart path (DashboardRenderer.tsx:495 → ObjectChart, CompareToConfig in packages/core/src/utils/compare-to.ts) still expects the retired three-arm shape and adapts in objectui#3337, which also deletes the now-unnecessary DatasetWidget.tsx:163-168 string-drop workaround. Until that lands, read this `live` as 'the dataset path honours it'; the inline path is mid-handoff, not unread." }, "dataset": { "status": "live", diff --git a/packages/spec/src/ui/dashboard-compareto.test.ts b/packages/spec/src/ui/dashboard-compareto.test.ts new file mode 100644 index 0000000000..7db4b1b21c --- /dev/null +++ b/packages/spec/src/ui/dashboard-compareto.test.ts @@ -0,0 +1,266 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5011 — `dashboard.widgets[].compareTo` converged on the executor's contract. + * + * ## What was broken (the shape these pins protect against returning) + * + * The widget declared three arms with confident TSDoc — `'previousPeriod'`, + * `'previousYear'`, `{ offset: '7d' | '1M' | '1y' }` — and the analytics + * executor implemented a fourth thing: `DatasetSelection.compareTo`, which is + * `{ kind, dimension? }` and has no `offset` in it anywhere. On the ADR-0021 + * dataset path (the spec's own "single author-facing analytics shape") all three + * arms failed, in two different ways: the string arms were DROPPED by the + * renderer, so a widget whose author asked for a comparison rendered its base + * numbers with the comparison quietly absent; `{ offset }` was forwarded into + * that contract with no dimension, so the executor threw + * `compareTo requires a timeDimension "undefined"` and errored the whole widget. + * All three worked on the legacy inline chart path. Same key, two fates, and the + * failing one was the blessed path. + * + * ## What is pinned here, and why each pin exists + * + * The convergence is only worth anything if BOTH halves hold: the widget must + * declare exactly the executor's shape (nothing to drift), and the retired + * spellings must be rejected with the upgrade in hand. So the classes are: + * + * 1. the converged shape parses THROUGH THE METADATA ROOT, at the real carrier + * slot path — a strict schema nobody parses gates nothing (#5000); + * 2. every retired spelling is rejected AND its prescription reaches the + * author at the top level; + * 3. the slot is NOT a union — the design benefit, asserted rather than + * claimed (see the `#5014` block below); + * 4. what the widget declares and what `DatasetCompareTo` declares are the + * same two keys, checked structurally rather than by eye. + * + * Every assertion here was run RED first against the pre-#5011 schema. + */ + +import { describe, it, expect } from 'vitest'; + +import { DashboardWidgetSchema } from './dashboard.zod'; +import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; +import type { DatasetCompareTo } from '../contracts/analytics-service'; + +/** A minimal ADR-0021 widget, with `compareTo` swapped in. */ +const widget = (compareTo: unknown) => ({ + id: 'won_this_quarter', + type: 'metric', + dataset: 'opportunity_metrics', + values: ['total_amount'], + ...(compareTo === undefined ? {} : { compareTo }), +}); + +/** Reject and hand back the serialized issues, so a pin reads what an author would. */ +function reject(value: unknown): string { + const r = DashboardWidgetSchema.safeParse(widget(value)); + expect(r.success, `expected compareTo ${JSON.stringify(value)} to be REJECTED`).toBe(false); + return JSON.stringify(r.success ? [] : r.error.issues); +} + +describe('#5011 — compareTo is the executor contract, projected', () => { + it('accepts `{ kind }` with `dimension` omitted — the executor resolves it', () => { + const r = DashboardWidgetSchema.safeParse(widget({ kind: 'previousPeriod' })); + expect(r.success).toBe(true); + expect(r.success && (r.data as { compareTo?: unknown }).compareTo).toEqual({ kind: 'previousPeriod' }); + }); + + it('accepts `{ kind, dimension }` — named explicitly when the selection is ambiguous', () => { + const r = DashboardWidgetSchema.safeParse(widget({ kind: 'previousYear', dimension: 'close_date' })); + expect(r.success).toBe(true); + expect(r.success && (r.data as { compareTo?: unknown }).compareTo) + .toEqual({ kind: 'previousYear', dimension: 'close_date' }); + }); + + it('rejects a kind the executor cannot run', () => { + // The executor's `shiftRange` implements exactly two windows. A third + // spelling here would be the whole #5011 defect, reintroduced one value at + // a time. zod's enum issue does not echo the offending value, so the pin is + // the ALLOWED set — which is the half that must not grow ahead of the + // executor. + for (const bogus of ['previousQuarter', 'previousWeek', 'previousMonth']) { + const msg = reject({ kind: bogus }); + expect(msg).toContain('"compareTo","kind"'); + expect(msg).toContain('"previousPeriod","previousYear"'); + } + }); + + /** + * The door. `DashboardWidgetSchema` is only a gate on documents that actually + * reach it through a metadata root — #5000's lesson, and the reason a strict + * schema plus a green unit test can still gate nothing. The control proves the + * probe is not vacuous: the same document with a legal `compareTo` parses. + */ + it('binds through the `dashboard` metadata root, at the real carrier slot path', () => { + const dashboard = getMetadataTypeSchema('dashboard'); + expect(dashboard, 'the dashboard root must resolve — this is the parse door').toBeTruthy(); + const doc = (compareTo: unknown) => ({ + name: 'pipeline_dashboard', + label: 'Pipeline Dashboard', + widgets: [widget(compareTo)], + }); + + // NEGATIVE CONTROL, run first and required to be GREEN: if the root + // rejected everything (or accepted everything) the assertion below would be + // satisfied for the wrong reason. + const control = dashboard!.safeParse(doc({ kind: 'previousPeriod' })); + expect(control.success, 'control: the converged shape parses through the root').toBe(true); + + for (const retired of ['previousPeriod', 'previousYear', { offset: '7d' }, { offset: '1y' }]) { + const r = dashboard!.safeParse(doc(retired)); + expect(r.success, `retired form ${JSON.stringify(retired)} must not survive the root`).toBe(false); + expect(JSON.stringify(r.error?.issues)).toContain('widgets'); + } + }); +}); + +describe('#5011 — every retired spelling is rejected WITH its upgrade', () => { + it('the bare string arms carry a prescription naming the exact replacement', () => { + const period = reject('previousPeriod'); + expect(period).toContain('was removed in'); + expect(period).toContain('#5011'); + expect(period).toContain('DROPPED'); + expect(period).toContain("kind: "); + expect(period).toContain('previousPeriod'); + expect(period).toContain('os migrate meta --from 16'); + + const year = reject('previousYear'); + expect(year).toContain('previousYear'); + expect(year).toContain('os migrate meta --from 16'); + }); + + /** + * The `HookBodyCapability` discipline: only the value that really WAS legal + * gets the retirement text. Telling the author of `previosPeriod` that their + * value "was removed" would misinform — that is a typo, not an upgrade. + */ + it('a misspelt string is NOT told it was removed — only the two real spellings are', () => { + const typo = reject('previosPeriod'); + expect(typo).not.toContain('was removed in'); + const wrongType = reject(7); + expect(wrongType).not.toContain('was removed in'); + }); + + it('`offset` is prescribed, never renamed onto a key that means something else', () => { + const msg = reject({ offset: '7d' }); + expect(msg).toContain('was removed in'); + expect(msg).toContain('never had an `offset` concept'); + // The prescription must say what to do INSTEAD, both halves: the kind, and + // (for a duration with no faithful target) the filter window. + expect(msg).toContain('previousPeriod'); + expect(msg).toContain('filter'); + expect(msg, 'must not suggest a rename for a key with no correct target') + .not.toContain('`offset` →'); + }); + + it("`{ offset: '1y' }` is told its exact equivalent, since it has one", () => { + expect(reject({ offset: '1y' })).toContain('previousYear'); + }); + + it('the words #5042 measured authors spelling `offset` with all reach the same prescription', () => { + // These were `aliases: { period: 'offset', … }` before the convergence — + // i.e. an empirical claim about what authors write on this slot. The claim + // survives; its target does not, so each now resolves to the retirement + // rather than to a key that no longer exists. + for (const word of ['period', 'duration', 'interval', 'shift', 'delta', 'by', 'amount', 'value']) { + expect(reject({ kind: 'previousPeriod', [word]: '7d' }), `${word} must carry the offset prescription`) + .toContain('was removed in'); + } + }); + + it('renames the near-misses for the two LIVE keys instead of prescribing at them', () => { + // `type`/`mode` were curated as guidance in #5042 ("the comparison KIND is + // the union itself, not a key") — that measurement said authors reach for + // those words here. Now that `kind` IS the key, the same claim is a faithful + // rename rather than a lecture. + expect(reject({ type: 'previousPeriod' })).toContain('`type` → `kind`'); + expect(reject({ mode: 'previousPeriod' })).toContain('`mode` → `kind`'); + // The neighbouring vocabulary for "which date column". + expect(reject({ kind: 'previousYear', field: 'close_date' })).toContain('`field` → `dimension`'); + expect(reject({ kind: 'previousYear', dateField: 'close_date' })).toContain('`dateField` → `dimension`'); + expect(reject({ kind: 'previousYear', timeDimension: 'close_date' })).toContain('`timeDimension` → `dimension`'); + }); + + it('the two wrong-layer keys still point one level up', () => { + expect(reject({ kind: 'previousYear', granularity: 'month' })).toContain('dateGranularity'); + expect(reject({ kind: 'previousYear', filter: { a: 1 } })).toContain('one level up'); + }); + + it('an unrecognised key names the surface and echoes itself', () => { + const msg = reject({ kind: 'previousYear', notACompareKey: 1 }); + expect(msg).toContain('this comparison window'); + expect(msg).toContain('notACompareKey'); + }); +}); + +/** + * The design benefit, asserted rather than claimed. + * + * `compareTo` used to be a union, and #5042 measured what that cost: zod + * collapses a failed union into ONE top-level `invalid_union` issue whose + * message is the bare `'Invalid input'`, and `zodIssuesToFields` + * (`rest/src/rest-server.ts`) maps only top-level issues — so every curated + * prescription this campaign wrote inside a union arm was produced and never + * delivered (#5014). The strictness ledger's `dashboard.zod.ts` row carried that + * caveat. + * + * The converged slot is a plain strict object, so its message is top-level and + * reaches the wire. This pin is what makes that a property of the schema rather + * than a claim in a comment: reintroduce a union here and it goes red. + */ +describe('#5011 — the converged slot is union-free, so its prescriptions reach the author', () => { + const issuesFor = (compareTo: unknown) => { + const r = DashboardWidgetSchema.safeParse(widget(compareTo)); + expect(r.success).toBe(false); + return r.success ? [] : r.error.issues; + }; + + it('produces NO `invalid_union` issue for any retired spelling', () => { + for (const retired of ['previousPeriod', 'previousYear', { offset: '7d' }, { kind: 'x', bogus: 1 }]) { + expect(issuesFor(retired).some((i) => i.code === 'invalid_union'), + `${JSON.stringify(retired)} must not collapse into a union issue`).toBe(false); + } + }); + + it('the prescription is the TOP-LEVEL message, not buried in arm errors', () => { + // The exact thing #5042 could not assert. `'Invalid input'` was the whole + // message an author used to get. + const top = issuesFor('previousPeriod').map((i) => i.message).join('\n'); + expect(top).not.toBe('Invalid input'); + expect(top).toContain('was removed in'); + expect(top).toContain('#5011'); + + const offsetTop = issuesFor({ offset: '7d' }).map((i) => i.message).join('\n'); + expect(offsetTop).toContain('was removed in'); + }); + + it('every issue is rooted at `compareTo`, so a field-level error map can place it', () => { + for (const i of issuesFor('previousPeriod')) expect(i.path[0]).toBe('compareTo'); + for (const i of issuesFor({ offset: '7d' })) expect(i.path[0]).toBe('compareTo'); + }); +}); + +/** + * The convergence claim itself: the widget declares the executor's contract, not + * a lookalike. Checked structurally, because "these two look the same" is + * exactly the review that failed for a whole major. + */ +describe('#5011 — widget and executor declare ONE vocabulary', () => { + it('a parsed widget `compareTo` is assignable to `DatasetCompareTo` with no mapping', () => { + const r = DashboardWidgetSchema.parse(widget({ kind: 'previousYear', dimension: 'close_date' })); + // No `as`, no re-spelling, no `??` — if the shapes ever diverge this line + // stops compiling, which is the point. + const forExecutor: DatasetCompareTo = (r as { compareTo: DatasetCompareTo }).compareTo; + expect(forExecutor).toEqual({ kind: 'previousYear', dimension: 'close_date' }); + + const resolved: DatasetCompareTo = + (DashboardWidgetSchema.parse(widget({ kind: 'previousPeriod' })) as { compareTo: DatasetCompareTo }) + .compareTo; + expect(resolved.dimension).toBeUndefined(); + }); + + it('declares exactly the two keys the contract does — no widget-side extras', () => { + const r = DashboardWidgetSchema.safeParse(widget({ kind: 'previousPeriod', dimension: 'd', extra: 1 })); + expect(r.success, 'a third key would be a second vocabulary starting over').toBe(false); + }); +}); diff --git a/skills/objectstack-query/rules/aggregation.md b/skills/objectstack-query/rules/aggregation.md index d3fd174df7..531e60f8b4 100644 --- a/skills/objectstack-query/rules/aggregation.md +++ b/skills/objectstack-query/rules/aggregation.md @@ -200,11 +200,11 @@ const withTotals = txns.map((t) => ({ ...t, running_total: (runningTotal += t.am ### Period-over-Period -For dashboard widgets, use the higher-level `compareTo: -'previousPeriod' | 'previousYear' | { offset }` field on the widget -schema (see *objectstack-ui* → *Period-over-period — `compareTo`*). -The renderer issues the shifted query for you and aligns the result -bucket-for-bucket with `categoryGranularity`. For ad-hoc comparisons, +For dashboard widgets, use the higher-level +`compareTo: { kind: 'previousPeriod' | 'previousYear', dimension? }` field on +the widget schema (see *objectstack-ui* → *Period-over-period — `compareTo`*). +The runtime issues the shifted query for you and aligns the result +bucket-for-bucket with the dataset dimension's `dateGranularity`. For ad-hoc comparisons, run two date-bucketed aggregations (see *Date-Bucketed Grouping* above) over the two periods and join the buckets in app code. diff --git a/skills/objectstack-ui/SKILL.md b/skills/objectstack-ui/SKILL.md index 24f510aeed..347c3935e9 100644 --- a/skills/objectstack-ui/SKILL.md +++ b/skills/objectstack-ui/SKILL.md @@ -1342,7 +1342,7 @@ export const SalesDashboard: Dashboard = { options: { icon: 'DollarSign' }, // the measure's own `format` drives the number // Period-over-period: renderer fetches the prior quarter and // surfaces a secondary value + delta arrow automatically. - compareTo: 'previousPeriod', + compareTo: { kind: 'previousPeriod' }, actionType: 'url', actionUrl: '/objects/opportunity?filter=open', }, @@ -1355,7 +1355,7 @@ export const SalesDashboard: Dashboard = { title: 'Revenue — This Year vs Last', dataset: 'order_metrics', dimensions: ['closed_at'], values: ['total_sum'], filter: { closed_at: { $gte: '{current_year_start}', $lte: '{current_year_end}' } }, - compareTo: 'previousYear', + compareTo: { kind: 'previousYear' }, layout: { x: 3, y: 0, w: 9, h: 4 }, }, ], @@ -1373,16 +1373,32 @@ Set `compareTo` on any data-bound widget to add a second query against a shifted time window. The renderer derives the comparison automatically; no second `filter` is required. -| Value | Behaviour | -|:--|:--| -| `'previousPeriod'` | Inspect the widget `filter` for date-macro tokens (`{current_month_start}`, `{last_7_days}`, …) and shift the window back by one period of the same kind. | -| `'previousYear'` | Shift the resolved filter window back by one calendar year. | -| `{ offset: '7d' }` | Shift by an explicit duration. Units: `d` (days), `w` (weeks), `M` (months), `y` (years). | +`compareTo` is `{ kind, dimension? }` — the same shape the analytics executor +reads (`DatasetSelection.compareTo`), so what a widget declares is exactly what +runs. There is no second widget-side vocabulary. + +| Key | Value | Behaviour | +|:--|:--|:--| +| `kind` | `'previousPeriod'` | The equal-length window immediately before the resolved one. | +| `kind` | `'previousYear'` | The same window shifted back one calendar year. | +| `dimension` | dimension name, **optional** | Which time dimension's window to shift. Omit it when the selection dates exactly one — the executor resolves it. With zero or several it errors, naming the candidates; it never guesses. | + +```typescript +compareTo: { kind: 'previousPeriod' } // one dated dimension +compareTo: { kind: 'previousYear', dimension: 'close_date' } // several — say which +``` + +> **Removed in v17 (#5011):** the bare strings `compareTo: 'previousPeriod'` / +> `'previousYear'` and the `{ offset: '7d' | '1M' | '1y' }` arm. The strings and +> `{ offset: '1y' }` are rewritten for you by `os migrate meta --from 16`; any +> other `offset` duration has no faithful target — state the window on the +> widget's `filter` and compare it with `{ kind: 'previousPeriod' }`, which +> shifts by that window's own length. * **Metric widgets** — the prior-period value renders as a small caption beneath the headline number, alongside a green/red delta arrow and an i18n trend label resolved from the comparison kind (e.g. `vs previous - period`, `vs previous year`, `vs previous 7d`). Authors should *not* + period`, `vs previous year`). Authors should *not* hand-author `options.trend` when `compareTo` is set; the renderer wins and overwrites it. * **Cartesian charts** (`line` / `area` / `bar` / `horizontal-bar` / @@ -1392,23 +1408,24 @@ no second `filter` is required. bars). Override per-series with `series.dashArray` / `series.opacity`. * **Pie / donut / funnel** — `compareTo` is silently ignored; there is no meaningful "two-period" composition for part-of-whole charts. -* **Requirements** — `compareTo` is a no-op when the filter contains no - resolvable date macros and no global `dateRange` is configured. The - shifted query reuses the original `filter` shape and replaces only the - date-bound clauses. +* **Requirements** — a comparison needs a **dated window** to shift. When the + selection carries no time dimension with a date range (no resolvable date + macro in the widget `filter`, no dashboard `dateRange`), the executor says so + rather than rendering a silently empty comparison column. The shifted query + reuses the original `filter` shape and replaces only the date-bound clauses. ```typescript // Metric — WoW delta (binds the task_metrics dataset; filter = runtimeFilter) { id: 'done_this_week', type: 'metric', dataset: 'task_metrics', values: ['task_count'], filter: { assignee: '{current_user_id}', status: 'done', completed_at: { $gte: '{week_start}' } }, - compareTo: 'previousPeriod' } + compareTo: { kind: 'previousPeriod' } } // Bar — YoY overlay on a stable category set { id: 'headcount_by_dept', type: 'bar', dataset: 'employee_metrics', dimensions: ['department'], values: ['headcount'], filter: { status: { $ne: 'terminated' } }, - compareTo: 'previousYear' } + compareTo: { kind: 'previousYear' } } ``` ### Server-side date bucketing — `dateGranularity` (ADR-0021) @@ -1436,7 +1453,7 @@ defineDataset({ { id: 'signed_by_month', type: 'line', dataset: 'contract_metrics', dimensions: ['signed_date'], values: ['signed_count'], filter: { signed_date: { $gte: '{12_months_ago}' } }, - compareTo: 'previousYear' } + compareTo: { kind: 'previousYear' } } ``` ### Drilldown From a11fd3caf5d5efa357f8fd0ed4c0bc35ba663e23 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 05:29:33 +0000 Subject: [PATCH 3/4] test: pin the conversion + executor dimension-resolution rule (#5011) --- ...taset-compare-dimension-resolution.test.ts | 234 ++++++++++++++++++ .../spec/src/conversions/conversions.test.ts | 96 +++++++ 2 files changed, 330 insertions(+) create mode 100644 packages/services/service-analytics/src/__tests__/dataset-compare-dimension-resolution.test.ts diff --git a/packages/services/service-analytics/src/__tests__/dataset-compare-dimension-resolution.test.ts b/packages/services/service-analytics/src/__tests__/dataset-compare-dimension-resolution.test.ts new file mode 100644 index 0000000000..1bd4b0fc3f --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/dataset-compare-dimension-resolution.test.ts @@ -0,0 +1,234 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5011 — `DatasetCompareTo.dimension` is optional, resolved BY THE EXECUTOR. + * + * ## Why the rule lives here and not in a renderer + * + * The widget's `compareTo` used to be a different vocabulary from the one this + * executor implements, and the ADR-0021 renderer papered over the gap by + * dropping what it could not forward. The convergence removed the widget's + * second vocabulary; making `dimension` optional is what stops the convergence + * from being a downgrade — an author with one obvious date column should not + * have to name it. But "resolve the obvious one" is a rule, and a rule + * implemented in each consumer is N rules. So it lives at the producer of the + * comparison: every caller — dashboard widget, report, raw `queryDataset` — + * gets the same dimension or the same error, and no renderer is ever in a + * position to guess (PD #12). + * + * ## What is pinned + * + * The three branches, and the two properties that make the rule safe: + * + * - **exactly one dated candidate** → it is used, and the SHIFT lands on it; + * - **zero** → a loud error saying a comparison needs a bounded window; + * - **several** → a loud error LISTING them, never a silent first-wins. This + * is the branch that matters most: picking `created_at` when the author + * meant `close_date` yields a comparison column that is WRONG rather than + * missing, which is the failure nobody audits. + * - the candidate set is the executor's own criterion (`dateRange` present), + * so a resolution can never pick something `shiftRange` then fails on; + * - an explicitly named dimension is still validated, and its error now names + * what IS dated instead of printing `"undefined"`. + * + * Every assertion here was run RED first against the pre-#5011 executor. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { IAnalyticsService, AnalyticsQuery, AnalyticsResult } from '@objectstack/spec/contracts'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import { compileDataset } from '../dataset-compiler.js'; +import { DatasetExecutor } from '../dataset-executor.js'; + +const dataset = DatasetSchema.parse({ + name: 'sales', + label: 'Sales', + object: 'opportunity', + filter: { is_deleted: { $ne: true } }, + dimensions: [ + { name: 'region', field: 'region', type: 'string' }, + { name: 'close_date', field: 'close_date', type: 'date' }, + { name: 'created_at', field: 'created_at', type: 'date' }, + ], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], +}); + +function fakeService(handler: (q: AnalyticsQuery) => AnalyticsResult): IAnalyticsService { + return { query: vi.fn(async (q: AnalyticsQuery) => handler(q)), getMeta: async () => [] }; +} + +/** Runs a selection and hands back every query the executor issued. */ +async function run(selection: Record) { + const seen: AnalyticsQuery[] = []; + const svc = fakeService((q) => { + seen.push(q); + return { rows: [{ region: 'NA', revenue: 100 }], fields: [] }; + }); + const res = await new DatasetExecutor(svc).execute(compileDataset(dataset), selection as never); + return { seen, res }; +} + +const WINDOW = ['2026-01-01', '2026-01-31'] as [string, string]; +const SHIFTED = ['2025-12-01', '2025-12-31']; + +describe('#5011 — compareTo.dimension resolution (exactly one dated candidate)', () => { + it('resolves the single dated time dimension when `dimension` is omitted', async () => { + const { seen } = await run({ + dimensions: ['region'], + measures: ['revenue'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + compareTo: { kind: 'previousPeriod' }, + }); + // The comparison pass must exist AND have shifted the right dimension. + const shifted = seen.find((q) => JSON.stringify(q.timeDimensions).includes('2025-12')); + expect(shifted, 'the comparison pass must have run').toBeDefined(); + expect(shifted!.timeDimensions![0]!.dimension).toBe('close_date'); + expect(shifted!.timeDimensions![0]!.dateRange).toEqual(SHIFTED); + }); + + it('attaches the comparison column, so the resolution reaches the RESULT', async () => { + const svc = fakeService((q) => { + const isShifted = JSON.stringify(q.timeDimensions).includes('2025-12'); + return { rows: [{ region: 'NA', revenue: isShifted ? 80 : 100 }], fields: [] }; + }); + const res = await new DatasetExecutor(svc).execute(compileDataset(dataset), { + dimensions: ['region'], + measures: ['revenue'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + compareTo: { kind: 'previousPeriod' }, + } as never); + expect(res.rows[0]).toMatchObject({ region: 'NA', revenue: 100, revenue__compare: 80 }); + }); + + /** + * The candidate set is `dateRange`-bearing entries, not "every time + * dimension". A second time dimension with no window is not a candidate, + * because there is nothing to shift on it — so this selection is + * unambiguous even though it declares two `timeDimensions`. + */ + it('ignores a time dimension that carries no dateRange — nothing to shift is not a candidate', async () => { + const { seen } = await run({ + dimensions: ['region'], + measures: ['revenue'], + timeDimensions: [ + { dimension: 'created_at', granularity: 'month' }, + { dimension: 'close_date', dateRange: WINDOW }, + ], + compareTo: { kind: 'previousYear' }, + }); + const shifted = seen.find((q) => JSON.stringify(q.timeDimensions).includes('2025-01')); + expect(shifted, 'the comparison pass must have run on the DATED dimension').toBeDefined(); + const entry = shifted!.timeDimensions!.find((t) => t.dimension === 'close_date')!; + expect(entry.dateRange).toEqual(['2025-01-01', '2025-01-31']); + }); +}); + +describe('#5011 — an unresolvable dimension is LOUD, never guessed', () => { + const expectRejection = async (selection: Record) => { + const svc = fakeService(() => ({ rows: [], fields: [] })); + return new DatasetExecutor(svc) + .execute(compileDataset(dataset), selection as never) + .then( + () => { throw new Error('expected the executor to REJECT this selection'); }, + (e: Error) => e.message, + ); + }; + + it('several dated candidates → names every one of them and refuses to pick', async () => { + const msg = await expectRejection({ + dimensions: ['region'], + measures: ['revenue'], + timeDimensions: [ + { dimension: 'close_date', dateRange: WINDOW }, + { dimension: 'created_at', dateRange: WINDOW }, + ], + compareTo: { kind: 'previousPeriod' }, + }); + expect(msg).toContain('ambiguous'); + // Both candidates by name — the fix must be copy-pasteable, not a hunt. + expect(msg).toContain('"close_date"'); + expect(msg).toContain('"created_at"'); + // And it must show the shape to write, with the kind the caller asked for. + expect(msg).toContain("kind: 'previousPeriod'"); + expect(msg).toContain('dimension:'); + }); + + it('zero dated candidates → says a comparison needs a bounded window', async () => { + const msg = await expectRejection({ + dimensions: ['region'], + measures: ['revenue'], + compareTo: { kind: 'previousPeriod' }, + }); + expect(msg).toContain('dateRange'); + expect(msg).toContain('or drop compareTo'); + expect(msg).toContain('only defined against a bounded window'); + }); + + it('a NAMED dimension that is not dated still fails — and now says what IS', async () => { + // The pre-#5011 message printed `"undefined"` here, because the widget path + // never supplied a dimension at all. Naming the real candidates is the + // difference between a bug report and a fix. + const msg = await expectRejection({ + dimensions: ['region'], + measures: ['revenue'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + compareTo: { kind: 'previousYear', dimension: 'created_at' }, + }); + expect(msg).toContain('"created_at"'); + expect(msg).toContain('"close_date"'); + expect(msg, 'the "undefined" message is the #5011 symptom itself').not.toContain('"undefined"'); + }); + + it('a NAMED dimension with nothing dated at all gets the other half of the advice', async () => { + const msg = await expectRejection({ + dimensions: ['region'], + measures: ['revenue'], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }); + expect(msg).toContain('"close_date"'); + expect(msg).toContain('no timeDimension with a dateRange'); + }); + + /** + * Control: none of the above is satisfied by an executor that simply throws. + * The same selections MINUS `compareTo` must all succeed. + */ + it('control — every one of those selections runs fine without compareTo', async () => { + for (const timeDimensions of [ + undefined, + [{ dimension: 'close_date', dateRange: WINDOW }], + [{ dimension: 'close_date', dateRange: WINDOW }, { dimension: 'created_at', dateRange: WINDOW }], + ]) { + const { res } = await run({ + dimensions: ['region'], + measures: ['revenue'], + ...(timeDimensions ? { timeDimensions } : {}), + }); + expect(res.rows).toHaveLength(1); + } + }); +}); + +/** + * The widget projection, end to end: what `DashboardWidgetSchema` now accepts is + * exactly what this executor consumes, with no mapping step in between. If the + * two shapes ever drift again, this is where it shows up as a runtime failure + * rather than as a renderer quietly dropping something. + */ +describe('#5011 — a widget-authored compareTo runs verbatim', () => { + it('forwards `{ kind }` and `{ kind, dimension }` with no translation', async () => { + for (const compareTo of [ + { kind: 'previousPeriod' as const }, + { kind: 'previousYear' as const, dimension: 'close_date' }, + ]) { + const { seen } = await run({ + dimensions: ['region'], + measures: ['revenue'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + compareTo, + }); + // Two passes: the current window and the shifted one. + expect(seen.length).toBeGreaterThanOrEqual(2); + } + }); +}); diff --git a/packages/spec/src/conversions/conversions.test.ts b/packages/spec/src/conversions/conversions.test.ts index ee9788d859..9237056172 100644 --- a/packages/spec/src/conversions/conversions.test.ts +++ b/packages/spec/src/conversions/conversions.test.ts @@ -548,4 +548,100 @@ describe('conversion layer (ADR-0087 D2)', () => { expect(notices).toHaveLength(0); }); }); + /** + * #5011 — `dashboard.widgets[].compareTo` converged on the executor contract. + * + * The fixture pair above already proves the three mechanical rewrites. What it + * cannot express is the entry's real judgement: the durations it deliberately + * does NOT touch, and the fact that it takes no acceptance window. Both are + * asserted here, because a conversion that quietly rewrote `{ offset: '7d' }` + * into a kind would turn a loud rejection into a wrong comparison — the exact + * failure class #5011 exists to end. + */ + describe('dashboard-widget-compareto-converged (#5011)', () => { + const dash = (compareTo: unknown) => ({ + dashboards: [{ + name: 'revenue_review', + widgets: [{ id: 'w1', type: 'kpi', dataset: 'orders', values: ['total'], compareTo }], + }], + }); + const convert = (compareTo: unknown) => + collectConversionNotices(structuredClone(dash(compareTo)), { includeRetired: true }); + const widgetOf = (stack: Record) => + (stack.dashboards as Array<{ widgets: Array> }>)[0]!.widgets[0]!; + + it('rewrites both bare strings, value verbatim, container changed', () => { + for (const kind of ['previousPeriod', 'previousYear'] as const) { + const { stack, notices } = convert(kind); + expect(widgetOf(stack).compareTo).toEqual({ kind }); + expect(notices).toHaveLength(1); + expect(notices[0]!.from).toBe(`'${kind}'`); + } + }); + + it("rewrites `{ offset: '1y' }` — the one duration with an exact equivalent", () => { + const { stack, notices } = convert({ offset: '1y' }); + expect(widgetOf(stack).compareTo).toEqual({ kind: 'previousYear' }); + expect(notices).toHaveLength(1); + }); + + /** + * The deliberate non-rewrite, and the assertion this whole describe exists + * for. `previousPeriod` shifts by the resolved window's own length, which + * equals `7d` only when the window happens to be seven days — so a + * mechanical rewrite would silently change which rows the comparison column + * counts. Left untouched, the strict schema meets the author with the + * prescription instead. + */ + it('leaves every other duration UNTOUCHED and silent, rather than guessing', () => { + for (const offset of ['7d', '1M', '2w', '30d', '3y']) { + const before = dash({ offset }); + const { stack, notices } = convert({ offset }); + expect(stack, `${offset} must not be rewritten`).toEqual(before); + expect(notices, `${offset} must emit no notice`).toHaveLength(0); + } + }); + + it('never synthesises a `dimension` — the executor resolves it, the chain cannot', () => { + // The conversion sees a stack, not a dataset: it cannot know which time + // dimension carries the window. Writing a guess here would convert a loud + // runtime error into a wrong comparison. + for (const input of ['previousPeriod', 'previousYear', { offset: '1y' }]) { + expect(widgetOf(convert(input).stack)).not.toHaveProperty('compareTo.dimension'); + } + }); + + it('is retired from the load path — the old spellings get NO acceptance window', () => { + // Without `includeRetired` (the normalizeStackInput posture) nothing is + // rewritten: the strict schema must be what an AUTHORED legacy spelling + // meets, so the prescription is delivered instead of the shape being + // silently absorbed (PD #12). + for (const input of ['previousPeriod', 'previousYear', { offset: '1y' }]) { + const before = dash(input); + const { stack, notices } = collectConversionNotices(structuredClone(before)); + expect(stack).toEqual(before); + expect(notices).toHaveLength(0); + } + }); + + it('touches only `compareTo` — a widget without one passes through by reference', () => { + const before = { + dashboards: [{ + name: 'other', widgets: [{ id: 'w1', type: 'kpi', dataset: 'orders', values: ['total'] }], + }], + }; + const { stack, notices } = collectConversionNotices(structuredClone(before), { includeRetired: true }); + expect(stack).toEqual(before); + expect(notices).toHaveLength(0); + }); + + it('is idempotent by construction — the converged shape is not a match', () => { + // No test replays a conversion twice for us (the skill's note), so this + // asserts it directly: `{ kind }` is neither a string arm nor an `offset`. + const before = dash({ kind: 'previousYear' }); + const { stack, notices } = collectConversionNotices(structuredClone(before), { includeRetired: true }); + expect(stack).toEqual(before); + expect(notices).toHaveLength(0); + }); + }); }); From d54a52208f65f0411dd177b9b23d5e5e50165383 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 05:35:44 +0000 Subject: [PATCH 4/4] docs+gen(spec): regenerate artifacts, correct ledgers, add changeset (#5011) --- .../dashboard-widget-compareto-converged.md | 75 ++++++++++++ content/docs/references/ui/dashboard.mdx | 2 +- .../2026-07-unknown-key-strictness-ledger.md | 2 +- docs/protocol-upgrade-guide.md | 6 + packages/spec/liveness/dashboard.json | 2 +- packages/spec/spec-changes.json | 26 ++++ packages/spec/src/ui/dashboard.test.ts | 7 +- .../spec/src/ui/strictness-batch14.test.ts | 114 +++++------------- 8 files changed, 147 insertions(+), 87 deletions(-) create mode 100644 .changeset/dashboard-widget-compareto-converged.md diff --git a/.changeset/dashboard-widget-compareto-converged.md b/.changeset/dashboard-widget-compareto-converged.md new file mode 100644 index 0000000000..dbd3078004 --- /dev/null +++ b/.changeset/dashboard-widget-compareto-converged.md @@ -0,0 +1,75 @@ +--- +"@objectstack/spec": major +"@objectstack/service-analytics": major +--- + +**BREAKING — `dashboard.widgets[].compareTo` converges on the analytics executor's contract (#5011).** + +The widget declared three period-over-period arms with confident TSDoc. The analytics +executor implements one shape, and it was never the same one — so on the ADR-0021 dataset +path (the spec's own "single author-facing analytics shape") **all three arms were +broken**, in two different ways: + +- `compareTo: 'previousPeriod'` / `'previousYear'` were **silently DROPPED** by the dataset + renderer. The widget rendered its base numbers and the comparison the author asked for + simply was not there. +- `compareTo: { offset: '7d' }` was forwarded into `DatasetSelection.compareTo`, whose + contract is `{ kind, dimension }` and has no `offset` in it — so the executor threw + `compareTo requires a timeDimension "undefined"` and the whole widget errored out. + +All three worked on the legacy inline chart path. Same key, two fates, and the failing one +was the path the spec calls canonical. + +`compareTo` is now a thin projection of the contract that is actually implemented: + +```ts +compareTo?: { kind: 'previousPeriod' | 'previousYear'; dimension?: string } +``` + +There is no widget-side vocabulary left to drift from the executor's, so `declared = +enforced` holds by construction rather than by review. + +## FROM → TO + +| v16 | v17 | Fix | +|:--|:--|:--| +| `compareTo: 'previousPeriod'` | `compareTo: { kind: 'previousPeriod' }` | `os migrate meta --from 16` rewrites it | +| `compareTo: 'previousYear'` | `compareTo: { kind: 'previousYear' }` | `os migrate meta --from 16` rewrites it | +| `compareTo: { offset: '1y' }` | `compareTo: { kind: 'previousYear' }` | `os migrate meta --from 16` rewrites it — `1y` **is** `previousYear` | +| `compareTo: { offset: '7d' \| '1M' \| … }` | **no faithful target** | State the window on the widget's own `filter` and compare with `{ kind: 'previousPeriod' }`, which shifts by that window's own length | + +The last row is deliberately *not* rewritten. `previousPeriod` shifts by the length of +whatever window the filter resolves to, which equals `7d` only when that window happens to +be seven days — a mechanical rewrite would silently change which rows the comparison +column counts, turning a loud failure into a wrong number. It is registered as the +`dashboard-widget-compareto-offset` semantic migration; the schema rejects the key with the +prescription in hand. + +Retired at the schema, so every old spelling is a parse error carrying its own upgrade — +including the bare strings, which are dispatched by value so a *typo* is still told it is a +typo rather than told it "was removed". + +## `dimension` is optional — resolved by the executor, not by a renderer + +Omit it and `dataset-executor.ts` resolves it, by its own long-standing criterion (a +`timeDimensions` entry carrying a `dateRange`): + +- exactly one candidate → that one is shifted; +- **zero** → a loud error: a comparison is only defined against a bounded window; +- **two or more** → a loud error **listing the candidates by name**, never a silent + first-wins. Picking `created_at` when the author meant `close_date` produces a comparison + that is *wrong* rather than *missing*, which is the failure nobody audits. + +This is a producer-side resolution rule, not consumer-side tolerance (Prime Directive +#12): every caller — dashboard widget, report, raw `queryDataset` — gets the same dimension +or the same error, and no renderer is ever in a position to guess one. + +## Notes + +- `DatasetCompareTo.dimension` is now optional. Callers that always passed it are + unaffected; callers that relied on the old "must be present" typing get a wider type. +- The converged slot is **union-free**. That is not cosmetic: zod collapses a failed union + into one bare `Invalid input`, so curated guidance written inside a union arm never + reaches the author (#5014). This slot's prescriptions are top-level and do. +- objectui's legacy inline chart path adapts separately (objectui#3337), which also deletes + the `DatasetWidget` string-drop workaround this change makes unnecessary. diff --git a/content/docs/references/ui/dashboard.mdx b/content/docs/references/ui/dashboard.mdx index 19c4f7a5d4..8ef07f9274 100644 --- a/content/docs/references/ui/dashboard.mdx +++ b/content/docs/references/ui/dashboard.mdx @@ -102,7 +102,7 @@ Dashboard header action | **actionType** | `Enum<'script' \| 'url' \| 'modal' \| 'flow' \| 'api' \| 'form'>` | optional | Type of action for the widget action button | | **actionIcon** | `string` | optional | Icon identifier for the widget action button | | **filter** | `any` | optional | Presentation-scope filter (runtimeFilter) | -| **compareTo** | `'previousPeriod' \| 'previousYear' \| { offset: string }` | optional | Period-over-period comparison window | +| **compareTo** | `{ kind: Enum<'previousPeriod' \| 'previousYear'>; dimension?: string }` | optional | Period-over-period comparison window (`{ kind, dimension? }`) | | **dataset** | `string` | ✅ | Dataset name to bind (ADR-0021) | | **dimensions** | `string[]` | optional | Dimension names — X/group/split | | **values** | `string[]` | ✅ | Measure names — Y (at least one) | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 61a4d367bb..6c6ea23ed5 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -580,7 +580,7 @@ not verdicts). | `component.zod.ts` | 29 | authorable | **next candidate** — SDUI component defs; check React-prop open slots first (p) | | `theme.zod.ts` | 14 | authorable | **strict as of #4001 批 15** — all 14 sites. The `(p)` resolved to authorable on two doors, both measured: `stack.zod.ts` declares `themes: z.array(ThemeSchema)` (so `defineStack()` parses every theme on boot and on `objectstack build`), and `defineTheme()` parses one directly. A BFS from all 24 metadata-type roots plus `ObjectStackSchema` reaches every schema in the file, with `PageSchema`/`DashboardSchema`/`ReportSchema`/`WebhookSchema`/`StateMachineSchema` passing as positive controls and 批 13's no-door shapes failing as negative controls **in the same run**. Note what is NOT claimed: `theme` is deliberately absent from `BUILTIN_METADATA_TYPE_SCHEMAS`, so a stored theme row is not validated by the metadata REST door — the gate is the authoring one, and the file says so rather than implying reach it lacks. **The `passthrough` question was asked per BLOCK, not per file**, and the answer split: objectui's `ThemeEngine` reads `colors`/`borderRadius`/`shadows`/`typography.fontFamily` through FIXED maps (an extra key is read by nothing, ever), but spreads `fontSize`/`fontWeight`/`lineHeight`/`letterSpacing`/`duration`/`timing`/`zIndex` with `Object.entries` into `--font-size-` … — the #4909 open shape at the runtime. Closed anyway, on two measurements: `.strip` already discarded those extras before the engine saw them (so no author depends on the openness and nothing the renderer receives changes), and `customVars` is a DECLARED escape hatch that emits an arbitrary CSS custom property by name, so closing the token scales removes no capability and only removes a second, undocumented way to spell one — the way whose typos are indistinguishable from intent. Curation is measured throughout: the shadcn vocabulary (`card`→`surface`, `foreground`→`text`, `destructive`→`error`) comes from objectui's own `COLOR_TO_CSS_MAP`, which RENAMES every palette key on the way out; `md`→`base` on `fontSize` and `base`→`normal` on `fontWeight` are a same-file scale disagreement (`borderRadius`/`shadows` declare `md`, `fontSize` does not); `radius`→`base` because `base` is emitted as the bare `--radius`, the one radius variable objectui's CSS actually reads; and `easeIn`→`ease_in` because `animation.timing` is the file's single snake_case vocabulary, so the camelCase spelling is an author obeying AGENTS.md #3 rather than making a typo. The eight #3494 removals get one distinct tombstone each. ⚠️ **Two of those tombstones deliberately prescribe NO replacement slot**: `touchTarget`/`keyboardNavigation` read like they should point at `ui/touch.zod.ts`/`ui/keyboard.zod.ts`, which 批 13 measured as having no carrier at all (#4988) — prescribing them would walk an author out of a loud rejection into a silent one, the ledger's finding 7. ⚠️ **Separately filed, not answered here**: `--font-size-*`, `--font-weight-*`, `--line-height-*`, `--letter-spacing-*`, `--z-*`, `--duration-*`, `--timing-*`, `--font-heading` and `--font-mono` have ZERO first-party consumers (only the colour vars, `--radius*`, `--shadow*` and `--font-sans` are read). That is ADR-0049 liveness, not unknown keys, and the two must not be run together — strictness makes a dropped key loud, it cannot make a slot live | | `app.zod.ts` | 18 | authorable | **strict as of #4001 PR B** — `AppSchema` + branding / area / context-selector / contribution, and the nav-item union converted to `z.discriminatedUnion('type', …)` (the union-error question, settled empirically: matched-branch-only errors, exact recursive paths, `toJSONSchema` clean). Per-target `params` stay open. PR A (#4142) tombstoned the seven audit-dead keys first | -| `dashboard.zod.ts` | 11 | authorable | **strict as of #4001 批 14 — 0 strip sites remain.** `DashboardWidgetSchema` has been strict since the ADR-0021 cutover; 批 14 closed the two NESTED holes inside it (`compareTo`'s object arm, `layout`), the same strict-shell-over-strip-children silhouette 批 13 found on `page.components[]`. `DashboardWidgetOptionsSchema` stays `passthrough` **deliberately** (renderer escape hatch) and the `responsive` tombstone (#4876) is untouched. ⚠️ `compareTo` is a UNION, so its curated prescription is produced but not delivered — `zodIssuesToFields` maps only top-level issues and a failed union collapses to a bare `Invalid input` (#5014). The REJECTION is unaffected | +| `dashboard.zod.ts` | 11 | authorable | **strict as of #4001 批 14 — 0 strip sites remain.** `DashboardWidgetSchema` has been strict since the ADR-0021 cutover; 批 14 closed the two NESTED holes inside it (`compareTo`'s object arm, `layout`), the same strict-shell-over-strip-children silhouette 批 13 found on `page.components[]`. `DashboardWidgetOptionsSchema` stays `passthrough` **deliberately** (renderer escape hatch) and the `responsive` tombstone (#4876) is untouched. ⚠️ **The `compareTo` union caveat this row carried is RESOLVED, and it is the one entry in this table whose limit was dissolved rather than worked around.** 批 14 recorded that `compareTo` was a UNION, so its curated prescription was produced but never delivered — `zodIssuesToFields` maps only top-level issues and a failed union collapses to a bare `Invalid input` (#5014) — with the rejection itself unaffected. **#5011 removed the union**: the slot converged onto the analytics executor's own contract, `{ kind, dimension? }`, a plain strict object whose message IS top-level. The reason was not the message, it was worse — all three declared arms were broken on the ADR-0021 dataset path (the two strings silently dropped by the renderer, `{ offset }` throwing `compareTo requires a timeDimension "undefined"`), while all three worked on the legacy inline path: same key, two fates, the failing one blessed. The union-free shape is the design benefit, pinned in `dashboard-compareto.test.ts` so it cannot silently return. **#5014 still binds every OTHER curated message this campaign has put inside a union arm** — this row is one slot's correction, not the finding's retraction | | `widget.zod.ts` | 9 | ~~authorable (p)~~ **no door** | **no authoring door (measured, #4001 批 16)** — the `(p)` resolved NEGATIVE for the whole file, the second such run after 批 13's five. Three independent measurements on 2026-08-04: (1) nothing under `packages/spec/src` imports this module except the `ui/index.ts` barrel, so no schema anywhere declares a carrier key for a widget shape — `field.widget` is a `z.string()` naming a registered *component* and has never referenced `WidgetManifest`; (2) a BFS over the in-memory Zod graph from all 24 metadata-type roots plus `defineStack` (4 766 nodes) reaches none of the six shapes, while `PageSchema` / `ObjectListViewSchema` resolve in the same run, a fresh `z.object` and a deliberate look-alike both resolve unreachable, and a synthetic carrier flips all six to reachable; (3) zero `.parse()` / `.safeParse()` in `objectstack`, `objectui` or `cloud` outside this file's own tests — objectui re-exports the inferred TYPES only and under different names (`RuntimeWidgetManifest` / `FieldWidgetComponentProps`, #4115 / #3161), and a `cloud` code search returns 0 for every symbol against a working index (`"@objectstack/spec"` → 345). ADR-0049 enforce-or-remove is **#5055**. ⚠️ **The campaign's own BFS said REACHABLE on the first run** — a false positive in the derived-clone bridge, filed as **#5056**: zod's `.describe()` returns a clone that SHARES the original `_zod.def`, so `WidgetManifestSchema.name` / `.label` (a described `SnakeCaseIdentifierSchema` / `I18nLabelSchema`) are def-identical to the same leaves on live schemas, and a bridge firing on ANY one shared property links two unrelated shapes. 2 shared keys of 20. The error is one-directional — it can only manufacture a door, i.e. it can only make a batch tighten something dead. Corrected to whole-shape overlap in `ui/door-reachability.testkit.ts` and pinned in `widget.test.ts` | | `page.zod.ts` | 7 | authorable | partially strict (ADR-0089) | | `chart.zod.ts` | 7 | **mixed — 5 authorable, 2 no gate** | **5 strict as of #4001 批 15**; 2 deliberately left open. `ChartConfigSchema` / `ChartAxis` / `ChartSeries` / `ChartAnnotation` / `ChartInteraction` are `root-graph`-reachable from the `dashboard` and `report` metadata roots (`DashboardWidget.chartConfig`, `ReportChartSchema`), so they are judged on the stored-metadata path and are now closed. **`ChartAggregateSchema` and `ChartGroupBySchema`'s object arm are NOT**, and this is the batch's real finding. They are not 批 13's no-door case — their carrier is LIVE: `aggregate` is a real authorable prop on the react tier's `` (ADR-0081), published in the generated react-blocks contract, and objectui's `ObjectChart` reads `schema.aggregate` to run the query. What is missing is the PARSE: neither schema is reachable from any metadata-type root or from `ObjectStackSchema` (both `UNREACHABLE` in the run where the five above come back `root-graph`), nothing in the three repos calls `.parse()` on them outside this file's unit tests, and the gate that DOES judge an authored `aggregate` — the react-page publish lint — re-derives the rules by hand (`CHART_FUNCTIONS`, the count/field requirement, the result-column naming) and never checks unknown keys. `react-blocks.ts` publishes the prop as a hand-written TYPE STRING; the Zod schema beside it is not what the contract is generated from. So `groupby` / `dateGranularty` are silently dropped today and would go on being silently dropped after a `strictObject` here — `.strict()` is a property of a parse. A fourth class, **`no gate`**: carrier live, parse absent. Distinct from `no door` (批 13), where the carrier itself does not exist. The contract-first fix is to make the publish gate PARSE the schema instead of re-deriving it — a `packages/lint` change, filed rather than smuggled into a spec strictness batch. Recorded in three places (schema-adjacent comment, test pin incl. a standing BFS assertion that goes red the day a carrier key appears, this row). ⚠️ One correction shipped with the tightening: the `clickAction` migration text #3752 wrote into this file prescribed **`drillDown`, which is not a key this protocol declares anywhere** — it is an untyped `(schema as any).drillDown` read inside objectui's `ObjectChart`. Promoting that sentence into a strict rejection would have handed an author the platform's authority for a key the same gate then rejects: finding 7, third occurrence, this time caught before shipping. The prose and the tombstone now name `onSegmentClick` / `ReportSchema.drilldown` / the widget's `options` bag, all of which exist. Filed separately. **`chart` 6 → 7 at the re-measurement** — no schema changed: `ChartAggregateSchema` is written `z\n .object({`, and the old counter's `z\.object\(` could not match across the line break | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 518dcc4033..729ee9fd9f 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -192,6 +192,8 @@ It also removes `connector.rateLimitConfig` and its whole shape (#4911). This on Last, it removes `dashboard.widgets[].responsive` (#4876) — the straggler of the #3896 sweep above, which retired the literally same-named `view.responsive` on the same evidence four days earlier. Re-measured before removal: no objectui code reads `widget.responsive` (DashboardRenderer, DashboardEditor and plugin-designer name it only in comments), and there are zero authored instances repo-wide, so the conversion is expected to be a no-op on every real source — it exists so that a stored dashboard carrying the key is cleaned deterministically rather than meeting the tombstone at load. What kept it alive was not evidence but a hole in the instrument: the liveness ledger declares no `children` on `dashboard.widgets`, and the walk only drills one level through an explicit `children`, so no widget-level key has ever been classified at all (filed as #4956, fixed separately). The removal is deliberately narrow — it takes the widget EMBED, not the shape. `ResponsiveConfig` stays exported and stays live on `page.components[].responsive`, which objectui `useResponsiveConfig` genuinely reads, so no import breaks and authors who need breakpoint behaviour today have somewhere real to put it. Per-widget responsive layout returns if and when a renderer implements it. +Finally it CONVERGES `dashboard.widgets[].compareTo` (#5011) — the one entry in this step that is not a removal but a vocabulary merge, and the one whose defect was worst-shaped. The widget declared three arms with confident TSDoc; the analytics executor implements one contract, `DatasetSelection.compareTo` = `{ kind, dimension? }`, which has no `offset` in it. On the ADR-0021 dataset path the two string arms were DROPPED by the renderer (a comparison silently absent from a widget whose author asked for one) and `{ offset }` was forwarded into that contract with no dimension, so the executor threw `compareTo requires a timeDimension "undefined"` and errored the whole widget. All three arms worked on the legacy inline chart path. Same key, two fates — and the failing one was the path the spec itself calls canonical, which is why this ranks above an ordinary declared-but-unread key: the documentation was actively teaching a shape that crashes. The widget now declares the executor's own words, so `declared = enforced` holds by construction with no second vocabulary left to drift. `dimension` is optional and resolved by the EXECUTOR (one dated time dimension → that one; zero or several → a loud error naming the candidates), which is a producer-side resolution rule, not the consumer-side tolerance PD #12 forbids. The bare strings and `{ offset: '1y' }` replay mechanically; every other `{ offset }` duration is a semantic TODO below, because `previousPeriod` shifts by the resolved window's own length and rewriting `7d` into it would change which rows the comparison counts. The converged slot is also union-free, which is not cosmetic: zod collapses a failed union into one bare `Invalid input` and #5014 showed that curated guidance inside a union arm never reaches the author at all. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -217,6 +219,7 @@ Last, it removes `dashboard.widgets[].responsive` (#4876) — the straggler of t | `view-inert-keys-removed` | `view.list.responsive / view.list.performance / view.form.defaultSort / view.form.aria` | view keys removed (#3896 close-out): list 'responsive'/'performance', form 'defaultSort'/'aria' — no renderer read them (list aria/data and form data stay live) | retired — `migrate meta` only | | `dashboard-inert-keys-removed` | `dashboard.aria / dashboard.performance / dashboard.widgets[].performance` | dashboard keys 'aria'/'performance' and widget 'performance' removed (#3896 close-out — no renderer applied any of them) | retired — `migrate meta` only | | `dashboard-widget-responsive-removed` | `dashboard.widgets[].responsive` | dashboard widget key 'responsive' removed (#4876 — no renderer ever applied per-widget breakpoint overrides; page.components[].responsive is unaffected) | retired — `migrate meta` only | +| `dashboard-widget-compareto-converged` | `dashboard.widgets[].compareTo` | dashboard widget 'compareTo' converged on the executor's { kind, dimension? } contract (#5011 — the bare strings and { offset: '1y' } rewrite mechanically; other { offset } durations have no faithful target and are reported, not guessed) | retired — `migrate meta` only | | `agent-knowledge-removed` | `agent.knowledge` | agent key 'knowledge' removed (#3896 close-out — declaring sources/indexes never scoped retrieval; restrict at the knowledge-service level) | retired — `migrate meta` only | | `skill-trigger-phrases-removed` | `skill.triggerPhrases` | skill key 'triggerPhrases' removed (#3896 close-out — activation is triggerConditions + the agent's skills[] allowlist; phrases were a dead-end projection) | retired — `migrate meta` only | | `stack-api-require-auth-removed` | `stack.api.requireAuth` | stack key 'api.requireAuth' removed — anonymous access is always denied; publish public surfaces by declaration (#3963) | retired — `migrate meta` only | @@ -238,6 +241,9 @@ Last, it removes `dashboard.widgets[].responsive` (#4876) — the straggler of t ### Semantic (delegated to you, with acceptance criteria) +- **`dashboard-widget-compareto-offset`** — `dashboard.widgets[].compareTo: { offset: '7d' | '1M' | … } (every duration except '1y')` → compareTo: { kind: 'previousPeriod' } plus an explicit window on the widget's own `filter` + - Why not automatic: The widget declared three comparison arms; the analytics executor implements one shape, `{ kind, dimension? }`, with no `offset` concept in it at all. On the ADR-0021 dataset path — the spec's single author-facing analytics shape — `{ offset }` was forwarded verbatim into that contract and threw `compareTo requires a timeDimension "undefined"`, taking the widget down; the arm ever only ran on the legacy inline chart path (#5011). The conversion rewrites `{ offset: '1y' }`, which IS `previousYear` by definition. Every other duration has NO faithful target: `previousPeriod` shifts by the length of whatever window the widget's filter resolves to, which equals `7d` only when that window happens to be seven days long. Rewriting mechanically would silently change which rows the comparison column counts — a wrong number rather than a missing one, which is strictly worse and exactly the class this convergence exists to end. Re-stating the intended window is a judgment about the presentation, not a transform. + - Done when: No dashboard widget declares `compareTo.offset`. Each former offset comparison states its window on the widget's `filter` and compares with `compareTo: { kind: 'previousPeriod' }` (or `'previousYear'`), and `dimension` is named wherever the selection dates more than one time dimension. `objectstack validate` passes, and each affected widget renders a `__compare` column over the window its author intended. - **`job-retry-policy-constraints-tightened`** — `job.retryPolicy.maxRetries (> 10) / job.retryPolicy.backoffMultiplier (< 1)` → maxRetries <= 10, and backoffMultiplier >= 1 - Why not automatic: The converged RetryPolicy (#4661) keeps the automation side's bounds, which the job side never had: `maxRetries` is capped at 10 and `backoffMultiplier` floored at 1. Neither has a lossless rewrite. Clamping `maxRetries: 20` to 10 would halve a retry budget its author chose, and a `backoffMultiplier` below 1 describes a delay that SHRINKS on each attempt — retrying a failing dependency ever faster, which is the opposite of backoff and was never a shape the engine meant to offer. Both now fail at parse time with the bound named, rather than being silently reinterpreted. Choosing the replacement count (or accepting the cap) is the author's call. - Done when: Every job declaring `retryPolicy` parses: no `maxRetries` above 10 and no `backoffMultiplier` below 1 remain, and each adjusted value was re-chosen knowing a retry re-runs the handler with its writes and callouts. No job fails to register with the retry-policy bound prescription. diff --git a/packages/spec/liveness/dashboard.json b/packages/spec/liveness/dashboard.json index 71f0165cf4..493d8c1949 100644 --- a/packages/spec/liveness/dashboard.json +++ b/packages/spec/liveness/dashboard.json @@ -1,6 +1,6 @@ { "type": "dashboard", - "_note": "DashboardSchema (UI, ADR-0021 dataset-bound). Live path: objectui DashboardView → DashboardRenderer → DatasetWidget. Seeded from docs/audits/2026-06-dashboardschema-property-liveness.md and re-verified against objectui HEAD — several audit-era findings are superseded: the ADR-0021 widget migration shipped (Studio WidgetConfigPanel + DashboardRenderer on dataset/dimensions/values, framework#3251; DashboardWidgetSchema is now `.strict()`); `globalFilters`/`dateRange` are LIVE (dashboard-level filters, framework#2501); the `title`↔`label` drift is fixed (renderer falls back to `label`, objectui#2806); the undeclared widget props were reconciled (#1894). objectui paths cited as prose in `note` (not `evidence`) on the dashboard-level entries; the widget children added in #4956 use the realm-marked `evidence` form (`objectui @91757a7: …`) that the gate can attribute. Framework provenance/lock fields auto-classify live (ADR-0010). 2026-07-30 (#3896 close-out sweep): the dead authoring keys were REMOVED — tombstoned at the schema with prescriptions (retiredKey) and stripped by the protocol-17 close-out conversions; entries deleted per the #3715 precedent. 2026-08-03 (#4876): `widgets[].responsive` REMOVED — tombstoned (retiredKey) and stripped by the protocol-17 `dashboard-widget-responsive-removed` conversion. 2026-08-03 (#4956, landed after #4876): the widget subtree is DRILLED — `widgets.children` classifies all 22 authorable DashboardWidgetSchema keys. This SUPERSEDES two sentences that stood here. The first, for a release: 'Widget-level props are classified in the DashboardWidgetSchema subtree, not drilled here' — FALSE in the only way that mattered, because no such subtree existed in any ledger file, the walk drills one level and only through an explicit `children`, and `widgets` declared none, so all 22 keys sat outside the map while the gate printed green; `widgets[].responsive` survived the #3896 sweep on that gap alone, not on evidence. The second, from #4876 itself: that `responsive` deliberately carries NO row here because one would be an ORPHAN. That was correct only while `widgets` was undrilled — the retiredKey tombstone KEEPS the key in the walked shape, so now that the drill has landed the row is REQUIRED (omitting it reports UNCLASSIFIED), and it is present below with the dead verdict the sweep never got to record. The gate now refuses an undeclared container inheritance outright (scripts/liveness/drill.mts), so this class of claim cannot be re-asserted in prose.", + "_note": "DashboardSchema (UI, ADR-0021 dataset-bound). Live path: objectui DashboardView → DashboardRenderer → DatasetWidget. Seeded from docs/audits/2026-06-dashboardschema-property-liveness.md and re-verified against objectui HEAD — several audit-era findings are superseded: the ADR-0021 widget migration shipped (Studio WidgetConfigPanel + DashboardRenderer on dataset/dimensions/values, framework#3251; DashboardWidgetSchema is now `.strict()`); `globalFilters`/`dateRange` are LIVE (dashboard-level filters, framework#2501); the `title`↔`label` drift is fixed (renderer falls back to `label`, objectui#2806); the undeclared widget props were reconciled (#1894). objectui paths cited as prose in `note` (not `evidence`) on the dashboard-level entries; the widget children added in #4956 use the realm-marked `evidence` form (`objectui @91757a7: …`) that the gate can attribute. Framework provenance/lock fields auto-classify live (ADR-0010). 2026-07-30 (#3896 close-out sweep): the dead authoring keys were REMOVED — tombstoned at the schema with prescriptions (retiredKey) and stripped by the protocol-17 close-out conversions; entries deleted per the #3715 precedent. 2026-08-03 (#4876): `widgets[].responsive` REMOVED — tombstoned (retiredKey) and stripped by the protocol-17 `dashboard-widget-responsive-removed` conversion. 2026-08-03 (#4956, landed after #4876): the widget subtree is DRILLED — `widgets.children` classifies all 22 authorable DashboardWidgetSchema keys. This SUPERSEDES two sentences that stood here. The first, for a release: 'Widget-level props are classified in the DashboardWidgetSchema subtree, not drilled here' — FALSE in the only way that mattered, because no such subtree existed in any ledger file, the walk drills one level and only through an explicit `children`, and `widgets` declared none, so all 22 keys sat outside the map while the gate printed green; `widgets[].responsive` survived the #3896 sweep on that gap alone, not on evidence. The second, from #4876 itself: that `responsive` deliberately carries NO row here because one would be an ORPHAN. That was correct only while `widgets` was undrilled — the retiredKey tombstone KEEPS the key in the walked shape, so now that the drill has landed the row is REQUIRED (omitting it reports UNCLASSIFIED), and it is present below with the dead verdict the sweep never got to record. The gate now refuses an undeclared container inheritance outright (scripts/liveness/drill.mts), so this class of claim cannot be re-asserted in prose. 2026-08-04 (#5011): `widgets[].compareTo` CONVERGED — the widget's three-arm vocabulary is replaced by a thin projection of the executor's own `DatasetSelection.compareTo` (`{ kind, dimension? }`), and the `{ offset }` arm retires via the `dashboard-widget-compareto-converged` conversion. Note what did NOT change: the verdict stays `live`. This was never a declared-but-unread key — the consumer existed the whole time; what was missing was agreement about what it consumes, which is a failure class this ledger had no vocabulary for until now and which its own `compareTo` row had to describe in a paragraph of prose.", "props": { "name": { "status": "live", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 09745be9a4..f33b19f252 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -194,6 +194,12 @@ "conversionId": "dashboard-widget-responsive-removed", "toMajor": 17 }, + { + "surface": "dashboard.widgets[].compareTo", + "to": "dashboard widget 'compareTo' converged on the executor's { kind, dimension? } contract (#5011 — the bare strings and { offset: '1y' } rewrite mechanically; other { offset } durations have no faithful target and are reported, not guessed)", + "conversionId": "dashboard-widget-compareto-converged", + "toMajor": 17 + }, { "surface": "agent.knowledge", "to": "agent key 'knowledge' removed (#3896 close-out — declaring sources/indexes never scoped retrieval; restrict at the knowledge-service level)", @@ -374,6 +380,13 @@ "toMajor": 16, "rationale": "The `.strict()` flip turns a previously silently-stripped unknown key into a parse error. There is no mapping target for an arbitrary unknown key — auto-deleting it would be exactly the silent data loss ADR-0078 bans — so each occurrence needs the author to decide: bind a `dataset` and select `dimensions`/`values`, move a renderer setting under `options`, or delete the dead key." }, + { + "surface": "dashboard.widgets[].compareTo: { offset: '7d' | '1M' | … } (every duration except '1y')", + "replacement": "compareTo: { kind: 'previousPeriod' } plus an explicit window on the widget's own `filter`", + "migrationId": "dashboard-widget-compareto-offset", + "toMajor": 17, + "rationale": "The widget declared three comparison arms; the analytics executor implements one shape, `{ kind, dimension? }`, with no `offset` concept in it at all. On the ADR-0021 dataset path — the spec's single author-facing analytics shape — `{ offset }` was forwarded verbatim into that contract and threw `compareTo requires a timeDimension \"undefined\"`, taking the widget down; the arm ever only ran on the legacy inline chart path (#5011). The conversion rewrites `{ offset: '1y' }`, which IS `previousYear` by definition. Every other duration has NO faithful target: `previousPeriod` shifts by the length of whatever window the widget's filter resolves to, which equals `7d` only when that window happens to be seven days long. Rewriting mechanically would silently change which rows the comparison column counts — a wrong number rather than a missing one, which is strictly worse and exactly the class this convergence exists to end. Re-stating the intended window is a judgment about the presentation, not a transform." + }, { "surface": "job.retryPolicy.maxRetries (> 10) / job.retryPolicy.backoffMultiplier (< 1)", "replacement": "maxRetries <= 10, and backoffMultiplier >= 1", @@ -836,6 +849,12 @@ "conversionId": "dashboard-widget-responsive-removed", "toMajor": 17 }, + { + "surface": "dashboard.widgets[].compareTo", + "to": "dashboard widget 'compareTo' converged on the executor's { kind, dimension? } contract (#5011 — the bare strings and { offset: '1y' } rewrite mechanically; other { offset } durations have no faithful target and are reported, not guessed)", + "conversionId": "dashboard-widget-compareto-converged", + "toMajor": 17 + }, { "surface": "agent.knowledge", "to": "agent key 'knowledge' removed (#3896 close-out — declaring sources/indexes never scoped retrieval; restrict at the knowledge-service level)", @@ -946,6 +965,13 @@ } ], "migrated": [ + { + "surface": "dashboard.widgets[].compareTo: { offset: '7d' | '1M' | … } (every duration except '1y')", + "replacement": "compareTo: { kind: 'previousPeriod' } plus an explicit window on the widget's own `filter`", + "migrationId": "dashboard-widget-compareto-offset", + "toMajor": 17, + "rationale": "The widget declared three comparison arms; the analytics executor implements one shape, `{ kind, dimension? }`, with no `offset` concept in it at all. On the ADR-0021 dataset path — the spec's single author-facing analytics shape — `{ offset }` was forwarded verbatim into that contract and threw `compareTo requires a timeDimension \"undefined\"`, taking the widget down; the arm ever only ran on the legacy inline chart path (#5011). The conversion rewrites `{ offset: '1y' }`, which IS `previousYear` by definition. Every other duration has NO faithful target: `previousPeriod` shifts by the length of whatever window the widget's filter resolves to, which equals `7d` only when that window happens to be seven days long. Rewriting mechanically would silently change which rows the comparison column counts — a wrong number rather than a missing one, which is strictly worse and exactly the class this convergence exists to end. Re-stating the intended window is a judgment about the presentation, not a transform." + }, { "surface": "job.retryPolicy.maxRetries (> 10) / job.retryPolicy.backoffMultiplier (< 1)", "replacement": "maxRetries <= 10, and backoffMultiplier >= 1", diff --git a/packages/spec/src/ui/dashboard.test.ts b/packages/spec/src/ui/dashboard.test.ts index 519459ef57..bd94ec71ea 100644 --- a/packages/spec/src/ui/dashboard.test.ts +++ b/packages/spec/src/ui/dashboard.test.ts @@ -52,11 +52,14 @@ describe('DashboardWidgetSchema (dataset-bound)', () => { it('keeps the presentation-scope filter (runtimeFilter) and compareTo', () => { const w = DashboardWidgetSchema.parse({ id: 'won', type: 'metric', dataset: 'sales', values: ['revenue'], - filter: { stage: 'closed_won' }, compareTo: 'previousPeriod', + // #5011: `compareTo` is the executor's `{ kind, dimension? }` contract. + // The bare string this used to assert is retired — see + // `dashboard-compareto.test.ts` for the prescription it now raises. + filter: { stage: 'closed_won' }, compareTo: { kind: 'previousPeriod' }, layout: { x: 0, y: 0, w: 3, h: 2 }, }); expect(w.filter).toEqual({ stage: 'closed_won' }); - expect(w.compareTo).toBe('previousPeriod'); + expect(w.compareTo).toEqual({ kind: 'previousPeriod' }); }); it('rejects a widget with no dataset', () => { diff --git a/packages/spec/src/ui/strictness-batch14.test.ts b/packages/spec/src/ui/strictness-batch14.test.ts index ecfb3272b7..7da3d8c00f 100644 --- a/packages/spec/src/ui/strictness-batch14.test.ts +++ b/packages/spec/src/ui/strictness-batch14.test.ts @@ -179,59 +179,43 @@ describe('批 14 — curated prescriptions', () => { }); /** - * `compareTo` is a UNION, and that changes what a rejection is worth — a fact - * this batch measured rather than assumed, after writing the assertion the - * obvious way and watching it go red on `'Invalid input'`. + * `compareTo` was a UNION when this batch closed its object arm, and that + * changed what a rejection was worth — a fact 批 14 measured rather than + * assumed, after writing the assertion the obvious way and watching it go red + * on `'Invalid input'`. Zod collapses a failed union into ONE top-level + * `invalid_union` issue whose message is the bare `'Invalid input'`; the arm + * errors — including the curated prescription — live in `issue.errors`, and + * `zodIssuesToFields` (`rest/src/rest-server.ts`) maps only top-level issues, + * so nothing carried them onto the wire (#5014). * - * Zod collapses a failed union into ONE top-level `invalid_union` issue whose - * message is the bare `'Invalid input'`; the arm errors — including the - * curated unknown-key prescription — live in `issue.errors`, one array per - * arm. And `zodIssuesToFields` (`rest/src/rest-server.ts`) maps only - * top-level issues, so nothing carries them onto the wire. + * ⚠️ #5011 DISSOLVED that limit for this slot, and the correction is recorded + * here rather than by deleting the finding: `compareTo` is no longer a union + * at all. It converged onto the analytics executor's own contract, + * `{ kind, dimension? }` — a plain strict object — because the three arms it + * declared were all broken on the ADR-0021 dataset path (two silently dropped, + * `{ offset }` throwing). The batch's measurement stands as the reason the + * union-free shape is worth something; #5014 still binds every OTHER curated + * message this campaign has put inside a union arm. * - * The closure is still worth having and still does #4001's job: the widget - * now FAILS at `compareTo` instead of silently discarding half the object. - * But the prescription is currently reachable only by walking sub-errors, so - * these two assertions are deliberately split — one for what an author sees - * today, one for the text that is there to be surfaced once the transport is - * fixed (filed separately). Asserting only the second would have been a green - * test over a message no consumer prints. + * What survives here is the pin that keeps the correction honest: this slot + * must not become a union again. The converged behaviour itself + * (prescriptions, aliases, the executor projection) is pinned in + * `dashboard-compareto.test.ts`. */ - function unionSubMessages(schema: z.ZodTypeAny, payload: unknown): string { - const r = schema.safeParse(payload); - if (r.success) return ''; - const out: string[] = []; - for (const issue of r.error.issues) { - const arms = (issue as { errors?: Array> }).errors; - for (const arm of arms ?? []) for (const sub of arm) out.push(sub.message); - } - return out.join('\n'); - } - - it('dashboard compareTo: an undeclared key now FAILS the widget (what an author sees today)', () => { + it('dashboard compareTo: no longer a union — the #4001 arm-error limit does not apply to it (#5011)', () => { const r = DashboardWidgetSchema.safeParse({ id: 'w1', dataset: 'sales', values: ['revenue'], compareTo: { offset: '7d', granularity: 'month' }, }); expect(r.success).toBe(false); - const union = r.success ? undefined : r.error.issues.find((i) => i.code === 'invalid_union'); - expect(union).toBeDefined(); - expect(union!.path).toEqual(['compareTo']); - // Pinning the CURRENT top-level text, so the day the transport starts - // surfacing arm errors this test says so instead of quietly improving. - expect(union!.message).toBe('Invalid input'); - }); - - it('dashboard compareTo: the curated prescription exists in the arm errors', () => { - expect(unionSubMessages(DashboardWidgetSchema, { - id: 'w1', dataset: 'sales', values: ['revenue'], - compareTo: { offset: '7d', granularity: 'month' }, - })).toContain('this comparison window'); - - expect(unionSubMessages(DashboardWidgetSchema, { - id: 'w1', dataset: 'sales', values: ['revenue'], - compareTo: { type: 'previousPeriod' }, - })).toContain("compareTo: 'previousPeriod'"); + const issues = r.success ? [] : r.error.issues; + expect(issues.some((i) => i.code === 'invalid_union'), + 'a union here would put the prescription back out of reach').toBe(false); + // …and the prescription really is at the top level now, which is exactly + // what 批 14 could assert only about the arm errors. + const top = issues.map((i) => i.message).join('\n'); + expect(top).not.toBe('Invalid input'); + expect(top).toContain('this comparison window'); }); it('action option: guidance separates keys that exist one layer down from keys that exist nowhere', () => { @@ -338,42 +322,6 @@ function shapeOf(schema: unknown, depth = 0): Record | null { return null; } -/** - * The object arm of a union — the only arm an unknown-key error can come from. - * - * Unwraps the wrappers first: on the widget, `compareTo` is - * `ZodOptional(ZodUnion([...]))`, so reading `def.options` off the schema handed - * in finds nothing. The first draft did exactly that and the suite went red on - * *"could not resolve the shape behind this comparison window"* rather than - * quietly skipping the surface — the walker's hard-failure guard doing its job - * (ledger finding 9: a walker going quiet is precisely when it stops covering - * something). - */ -function unionObjectArm(schema: unknown, depth = 0): Record | null { - if (depth > 12 || schema == null) return null; - const def = (schema as { _zod?: { def?: Record } })._zod?.def; - if (!def) return null; - const options = (def as { options?: unknown[] }).options; - if (Array.isArray(options)) { - for (const arm of options) { - const shape = shapeOf(arm); - if (shape) return shape; - } - return null; - } - for (const key of ['innerType', 'in', 'out', 'schema'] as const) { - const inner = def[key]; - if (inner && typeof inner === 'object') { - const found = unionObjectArm(inner, depth + 1); - if (found) return found; - } - } - if (typeof (def as { getter?: unknown }).getter === 'function') { - return unionObjectArm((def as { getter: () => unknown }).getter(), depth + 1); - } - return null; -} - describe('批 14 — no prescription points at a key the schema rejects', () => { /** * `surface` string → the DECLARED keys of the shape that surface names, @@ -395,7 +343,9 @@ describe('批 14 — no prescription points at a key the schema rejects', () => ['this dataset dimension', shapeOf(DatasetDimensionSchema)], ['this dataset measure', measure], ['this derived-measure spec', shapeOf(measure.derived)], - ['this comparison window', unionObjectArm(widget.compareTo)], + // #5011: `compareTo` converged from a union to a plain strict object, + // so the arm-unwrapper this entry used is gone with it. + ['this comparison window', shapeOf(widget.compareTo)], ['this widget layout box', shapeOf(widget.layout)], ]; return new Map(entries.map(([surface, shape]) => {