From 20c2b0ce3a7bc25d88d7bcf303ce2eb886da661f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 19:04:08 +0000 Subject: [PATCH 1/4] wip(lint): widget filter keys + sortBy limbs on the #14105 seam --- packages/lint/src/index.ts | 19 +- packages/lint/src/object-graph.ts | 83 ++++++++ .../lint/src/validate-dataset-references.ts | 75 +------ packages/lint/src/validate-widget-bindings.ts | 187 ++++++++++++++++++ 4 files changed, 292 insertions(+), 72 deletions(-) diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 7239244f7c..7d29f86752 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -24,6 +24,13 @@ export { WIDGET_LEGACY_ANALYTICS_UNRENDERABLE, DASHBOARD_FILTER_FIELD_UNKNOWN, DASHBOARD_FILTER_FIELD_UNPROVISIONED, + // [#14148] The widget's OWN two references, at the same site: the keys of its + // presentation-scope `filter` (resolved on the #14105 object-graph seam, with + // the ADR-0021 `include` clause its `runtimeFilter` really is subject to) and + // `options.sortBy` against what the widget selects. + WIDGET_FILTER_FIELD_UNKNOWN, + WIDGET_FILTER_FIELD_NOT_INCLUDED, + WIDGET_SORTBY_UNSELECTED, } from './validate-widget-bindings.js'; export type { WidgetBindingFinding, WidgetBindingSeverity } from './validate-widget-bindings.js'; @@ -485,16 +492,26 @@ export type { DatasetRefFinding, DatasetRefSeverity } from './validate-dataset-r // positions) must reuse ONE mechanism rather than growing a second hop-walker // and a second filter-key reader. Both hold mechanism only — no rule ids, no // severities, no findings; the judgement stays with the rule that asks. +// [#14148] `joinablePrefixes` and `describeFieldPathVerdict` joined them when +// the widget limb landed: both were local to the dataset rule, and both are +// answers to questions the widget position asks identically — how ADR-0021 +// expands an `include` into joinable prefixes, and how one verdict reads in +// prose. Copying either would have been the second implementation this seam +// exists to prevent, one release after it was written to prevent it. export { indexObjectGraph, resolveFieldPath, isUnjudgeable, + joinablePrefixes, + describeFieldPathVerdict, nearestName, suggestName, listNames, RELATIONSHIP_FIELD_TYPES, } from './object-graph.js'; -export type { ObjectGraph, GraphObject, GraphField, FieldPathVerdict } from './object-graph.js'; +export type { + ObjectGraph, GraphObject, GraphField, FieldPathVerdict, FieldPathAccount, +} from './object-graph.js'; export { walkFilterFieldKeys } from './filter-walk.js'; export type { FilterFieldKey } from './filter-walk.js'; diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts index ce7d38c8b9..f9ca4791d0 100644 --- a/packages/lint/src/object-graph.ts +++ b/packages/lint/src/object-graph.ts @@ -248,6 +248,89 @@ export function resolveFieldPath( return { kind: 'field-unknown', object: current, field: leaf, candidates: obj.names }; } +/** + * The relationship prefixes a document declared as joinable. + * + * ADR-0021: *"Declaring `a.b` implicitly includes the intermediate `a`."* So + * every PREFIX of every declared path is joinable, not only the paths as + * written — which is why this expands rather than reading `include` verbatim. + * + * Here rather than in a rule because the SAME `include` governs positions two + * different rules judge: a dataset's own `dimensions[].field` / `measures[].field` + * / filter keys (#14105), and a dashboard widget's `filter` keys (#14148), whose + * condition is ANDed into that same dataset's compiled query as `runtimeFilter`. + * Two copies of the prefix expansion would let the two positions drift apart on + * a clause that is one sentence of one ADR. + */ +export function joinablePrefixes(include: unknown): ReadonlySet { + const prefixes = new Set(); + if (!Array.isArray(include)) return prefixes; + for (const entry of include) { + if (typeof entry !== 'string' || !entry) continue; + const segments = entry.split('.'); + for (let i = 1; i <= segments.length; i++) { + prefixes.add(segments.slice(0, i).join('.')); + } + } + return prefixes; +} + +/** The two halves of a rendered verdict: the finding's message, and its detail. */ +export interface FieldPathAccount { + /** What is wrong, in prose, carrying the "did you mean" when there is one. */ + message: string; + /** The supporting field list, for the finding's hint. */ + detail: string; +} + +/** + * Turn a resolution verdict into the message half of an existence finding, or + * `undefined` when the verdict is one no rule may report. + * + * Shared by every position that resolves a field PATH — a dataset dimension, a + * measure, a dataset filter key (#14105), a widget filter key (#14148) — so + * they cannot drift into N different accounts of the same miss. The caller + * supplies `subject` (how the position is named in prose) and owns the rule id, + * the severity, the path and the hint's prescription; this function holds none + * of them, matching the rest of this module. + */ +export function describeFieldPathVerdict( + verdict: FieldPathVerdict, + path: string, + subject: string, +): FieldPathAccount | undefined { + switch (verdict.kind) { + case 'ok': + case 'unknowable': + case 'hop-untargeted': + return undefined; + case 'hop-unknown': + return { + message: + `${subject} "${path}" traverses "${verdict.segment}", which is not a field on object ` + + `"${verdict.object}".${suggestName(verdict.segment, verdict.candidates)}`, + detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`, + }; + case 'hop-not-relationship': + return { + message: + `${subject} "${path}" traverses "${verdict.segment}", which is a` + + `${verdict.type ? ` \`${verdict.type}\`` : 'n ordinary'} field on object ` + + `"${verdict.object}" and not a relationship — there is nothing to join through.`, + detail: + `Only ${[...RELATIONSHIP_FIELD_TYPES].sort().join(' / ')} fields are traversable ` + + `(ADR-0021 derives every join from the object graph; you never write an ON clause).`, + }; + case 'field-unknown': + return { + message: + `${subject} "${path}" is not a field on object "${verdict.object}".` + + `${suggestName(verdict.field, verdict.candidates)}`, + detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`, + }; + } +} + /** * True when the verdict is one no rule may report — the graph could not answer. * Callers spell the skip through this predicate rather than re-listing the diff --git a/packages/lint/src/validate-dataset-references.ts b/packages/lint/src/validate-dataset-references.ts index 02024959d5..9209e559e8 100644 --- a/packages/lint/src/validate-dataset-references.ts +++ b/packages/lint/src/validate-dataset-references.ts @@ -118,12 +118,11 @@ import { walkFilterFieldKeys } from './filter-walk.js'; import { RELATIONSHIP_FIELD_TYPES, + describeFieldPathVerdict, indexObjectGraph, isUnjudgeable, - listNames, + joinablePrefixes, resolveFieldPath, - suggestName, - type FieldPathVerdict, type ObjectGraph, } from './object-graph.js'; @@ -172,72 +171,6 @@ function asArray(v: unknown): AnyRec[] { return []; } -/** - * The relationship prefixes a dataset declared as joinable. - * - * ADR-0021: *"Declaring `a.b` implicitly includes the intermediate `a`."* So - * every PREFIX of every declared path is joinable, not only the paths as - * written — which is why this expands rather than reading `include` verbatim. - */ -function joinablePrefixes(include: unknown): ReadonlySet { - const prefixes = new Set(); - if (!Array.isArray(include)) return prefixes; - for (const entry of include) { - if (typeof entry !== 'string' || !entry) continue; - const segments = entry.split('.'); - for (let i = 1; i <= segments.length; i++) { - prefixes.add(segments.slice(0, i).join('.')); - } - } - return prefixes; -} - -/** - * Turn a resolution verdict into the message half of an existence finding, or - * `undefined` when the verdict is one no rule may report. - * - * Shared by the three positions that resolve a field PATH (dimension, measure, - * filter key) so they cannot drift into three different accounts of the same - * miss. The caller supplies `subject` — how the position is named in prose — - * and owns the rule id, the path and the hint's prescription. - */ -function existenceMessage( - verdict: FieldPathVerdict, - path: string, - subject: string, -): { message: string; detail: string } | undefined { - switch (verdict.kind) { - case 'ok': - case 'unknowable': - case 'hop-untargeted': - return undefined; - case 'hop-unknown': - return { - message: - `${subject} "${path}" traverses "${verdict.segment}", which is not a field on object ` + - `"${verdict.object}".${suggestName(verdict.segment, verdict.candidates)}`, - detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`, - }; - case 'hop-not-relationship': - return { - message: - `${subject} "${path}" traverses "${verdict.segment}", which is a` + - `${verdict.type ? ` \`${verdict.type}\`` : 'n ordinary'} field on object ` + - `"${verdict.object}" and not a relationship — there is nothing to join through.`, - detail: - `Only ${[...RELATIONSHIP_FIELD_TYPES].sort().join(' / ')} fields are traversable ` + - `(ADR-0021 derives every join from the object graph; you never write an ON clause).`, - }; - case 'field-unknown': - return { - message: - `${subject} "${path}" is not a field on object "${verdict.object}".` + - `${suggestName(verdict.field, verdict.candidates)}`, - detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`, - }; - } -} - /** The shared consequence sentence — why an unresolved path is not merely inert. */ const SILENT_EMPTY = 'The path is compiled into the analytics query as written, so it addresses a column ' + @@ -309,7 +242,7 @@ export function validateDatasetReferences(stack: AnyRec): DatasetRefFinding[] { return; } - const account = existenceMessage(verdict, entry, `include[${ii}]`); + const account = describeFieldPathVerdict(verdict, entry, `include[${ii}]`); if (!account) return; findings.push({ severity: 'error', @@ -342,7 +275,7 @@ export function validateDatasetReferences(stack: AnyRec): DatasetRefFinding[] { const verdict = resolveFieldPath(graph, object, written); if (isUnjudgeable(verdict) || !verdict) return; - const account = existenceMessage(verdict, written, subject); + const account = describeFieldPathVerdict(verdict, written, subject); if (account) { findings.push({ severity: 'error', diff --git a/packages/lint/src/validate-widget-bindings.ts b/packages/lint/src/validate-widget-bindings.ts index e73ea7d48c..08625db682 100644 --- a/packages/lint/src/validate-widget-bindings.ts +++ b/packages/lint/src/validate-widget-bindings.ts @@ -3,6 +3,15 @@ import { isIncoherentAggregate } from '@objectstack/spec/data'; import { ChartTypeSchema } from '@objectstack/spec/ui'; +import { walkFilterFieldKeys } from './filter-walk.js'; +import { + describeFieldPathVerdict, + indexObjectGraph, + isUnjudgeable, + joinablePrefixes, + resolveFieldPath, + type ObjectGraph, +} from './object-graph.js'; import { SYSTEM_FIELDS, indexUnprovisionedAnchors, @@ -84,6 +93,65 @@ import { * of the same invariant, warned rather than errored because this pass cannot * see the remote schema (#8116's severity reasoning). * + * ── The widget's OWN two references (#14148) ───────────────────────────────── + * + * - `widget-filter-field-unknown` — a KEY of the widget's own `filter` resolves + * to no column on the bound dataset's object graph. + * - `widget-filter-field-not-included` — that key RESOLVES, but its relationship + * prefix is not declared in the dataset's `include`, so ADR-0021 compiles no + * join for it and the column is out of the query's reach. + * - `widget-sortby-unselected` — `options.sortBy` names neither a `dimensions[]` + * nor a `values[]` entry of this widget, so the ordering the author wrote + * cannot be applied to a result that will not contain that column. + * + * All three are `error`, and the reason is the reporting card's, quoted because + * it is the sharpest statement of it in the family: the dashboard it was + * measured on leads with a "not moving" tile — open work untouched >14 days — + * and *"an empty tile is indistinguishable from a healthy team: a missing + * number reads as zero, and zero is the answer the manager is hoping for."* + * The failure is not merely silent, it is silent in the direction the reader + * WANTS to believe, which is why it gates rather than advises. + * + * ### Why these two were the surviving holes + * + * On the very same node, the TOKEN was checked and the COLUMN was not: + * `filter-token-unknown` (#3574) fires path-precise at + * `…widgets[4].filter.due_date.$lte`, so the traversal already walked the + * filter tree and already knew the widget's dataset. And the identical + * resolution already existed one key over — `dashboard-filter-field-unknown` + * resolves a DASHBOARD-level filter's field against each widget's dataset base + * object. `widgets[].filter` is the same field-existence invariant on the + * filter an author is MORE likely to write by hand, and it was simply never fed + * through it. `options.sortBy` is the declared-≠-enforced half: + * `DashboardWidgetOptionsSchema.sortBy` states its own contract in prose — + * *"must be one this widget actually selects"* — and nothing enforced it. + * + * ### The `include` clause, and why a dotted path is RESOLVED here + * + * A widget's `filter` is ANDed into the dataset query as `runtimeFilter` + * (`DashboardWidgetSchema.filter`; `dataset-executor.ts` `combineFilters( + * compiled.filter, selection.runtimeFilter)`), and that compiled query carries + * ONLY the joins the dataset's `include` declared (`dataset-compiler.ts`: joins + * are derived from `include`, and `assertDeclared` refuses an undeclared + * relationship path). So a dotted key here is judged on the same two clauses + * `validate-dataset-references.ts` applies one level down — existence, then + * joinability — rather than skipped. That decision is deliberate and recorded + * rather than implicit: `dashboard-filter-field-unknown` above SKIPS a dotted + * field (`field.includes('.')`), which was correct when nothing in this package + * could walk hops, and a third silent pass-through would have reproduced the + * very card this rule closes. The runtime is NOT a backstop for it either — + * `assertDeclared` runs over `dimensions` and `measures` only, never over + * `runtimeFilter` — so nothing between the author and the empty tile asks this + * question. + * + * Resolution is {@link resolveFieldPath}'s and its `unknowable` verdicts are + * never reported (ADR-0072 D1), so the three skips every field-existence rule + * in this package takes apply unchanged: an object this stack does not define, + * an object with no readable field map (ADR-0015 `external`), and a + * registry-injected system column — the last resolved PER OBJECT rather than + * through the flat `SYSTEM_FIELDS` union, which is what lets a reference to + * `owner_id` on an `ownership: 'none'` object stay a real finding. + * * Warnings can be deliberately suppressed per widget via * `suppressWarnings: ['']`; errors cannot — they describe a * binding the analytics service cannot satisfy. @@ -100,6 +168,12 @@ export const WIDGET_LEGACY_ANALYTICS_SHAPE = 'widget-legacy-analytics-shape'; export const WIDGET_LEGACY_ANALYTICS_UNRENDERABLE = 'widget-legacy-analytics-unrenderable'; export const DASHBOARD_FILTER_FIELD_UNKNOWN = 'dashboard-filter-field-unknown'; export const DASHBOARD_FILTER_FIELD_UNPROVISIONED = 'dashboard-filter-field-unprovisioned'; +/** [#14148] A key of the widget's OWN `filter` that resolves to no column. */ +export const WIDGET_FILTER_FIELD_UNKNOWN = 'widget-filter-field-unknown'; +/** [#14148] A widget filter key whose relationship prefix is not in `include`. */ +export const WIDGET_FILTER_FIELD_NOT_INCLUDED = 'widget-filter-field-not-included'; +/** [#14148] `options.sortBy` names nothing this widget selects. */ +export const WIDGET_SORTBY_UNSELECTED = 'widget-sortby-unselected'; /** * Pre-ADR-0021 inline-analytics keys. The single-form cutover replaced them @@ -337,6 +411,11 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { // behind them. Built once for the whole stack; empty for every object that is // not ADR-0015 `external`, so the filter check below pays one map lookup. const unprovisionedAnchors = indexUnprovisionedAnchors(stack); + // [#14148] The resolution universe for a widget's OWN filter keys. Indexed + // once for the whole stack and shared by every widget, exactly as + // `validate-dataset-references.ts` does one level down — the same seam, so + // the two positions cannot drift into two accounts of one object graph. + const graph: ObjectGraph = indexObjectGraph(stack); const datasetList = asArray(stack.datasets); for (let i = 0; i < datasetList.length; i++) { const ds = datasetList[i]; @@ -547,6 +626,67 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { } } + // ── (a2) the widget's OWN filter keys resolve (#14148) ── + // `filter-token-unknown` already stands inside this exact subtree and + // judges the VALUES; this asks the question that was missing about the + // KEYS. The condition is ANDed into the dataset query as `runtimeFilter`, + // so a key naming no column either widens the scope (the condition is + // dropped) or empties it — and the widget renders successfully either way. + if (w.filter !== undefined && w.filter !== null) { + const filterObject = typeof dataset.object === 'string' ? dataset.object : undefined; + // Skips 1 and 2, taken once for the whole widget: an object this stack + // does not define, or one with no readable field map. Resolving against + // it would turn an unknowable base binding into a finding per filter key. + const base = filterObject ? graph.get(filterObject) : undefined; + if (filterObject && base) { + const included = joinablePrefixes(dataset.include); + walkFilterFieldKeys(w.filter, `${path}.filter`, ({ field, path: at }) => { + const verdict = resolveFieldPath(graph, filterObject, field); + if (isUnjudgeable(verdict) || !verdict) return; + + const account = describeFieldPathVerdict(verdict, field, 'filter key'); + if (account) { + push({ + severity: 'error', + rule: WIDGET_FILTER_FIELD_UNKNOWN, + message: + `${account.message} The widget's own \`filter\` is ANDed into the ` + + `dataset query as \`runtimeFilter\`, so the condition addresses a column ` + + `that does not exist: the widget renders successfully and empty, and ` + + `nothing reports the miss.`, + hint: + `Filter on a field that exists on "${filterObject}" (dataset "${dsName}"), ` + + `or on a \`relationship[.relationship].field\` path whose prefix is declared ` + + `in that dataset's \`include\`. ${account.detail}`, + }); + return; + } + + // The key RESOLVES. Second clause: ADR-0021 joins ONLY declared + // paths, and the compiler's `assertDeclared` never sees a + // `runtimeFilter` — so an undeclared prefix is a real defect with + // no runtime door in front of it. + const cut = field.lastIndexOf('.'); + if (cut < 0) return; // a base column needs no join + const prefix = field.slice(0, cut); + if (included.has(prefix)) return; + push({ + severity: 'error', + rule: WIDGET_FILTER_FIELD_NOT_INCLUDED, + message: + `filter key "${field}" resolves on the object graph, but its relationship ` + + `prefix "${prefix}" is not declared in dataset "${dsName}"'s \`include\` — ` + + `and ADR-0021 joins ONLY declared paths, so no join is compiled and the ` + + `column is out of this query's reach. The widget renders empty.`, + hint: + `Add "${prefix}" to dataset "${dsName}"'s include (declaring "a.b" implicitly ` + + `includes "a"), or filter on a field of "${filterObject}" itself. Declared ` + + `include paths: ${included.size > 0 ? [...included].sort().join(', ') : '(none)'}.`, + }); + }); + } + } + const dimensionNames = new Set(); for (const d of asArray(dataset.dimensions)) { if (typeof d.name === 'string') dimensionNames.add(d.name); @@ -589,6 +729,53 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { }); } + // ── (c1) `options.sortBy` names something this widget selects (#14148) ── + // `DashboardWidgetOptionsSchema.sortBy` states its own contract in prose + // — "must be one this widget actually selects (a `dimensions` entry or a + // `values` entry)" — and nothing enforced it. It is lowered into a + // `DatasetSelection.order`, whose key must name a selected dimension or + // measure; a key that does not is either dropped in favour of the + // implicit ordering or refused by the executor (`resolveOrdering` + // throws `DATASET_INVALID`). Both outcomes lose the order the author + // wrote, and the first loses it in silence — which is the whole defect + // where the authored order IS the product rule (ordering business units + // by a COUNT turns a workload chart into a league table). + // + // Resolved against the AUTHORED `dimensions`/`values` arrays, not the + // validated subset: an entry that does not resolve is rules (b)/(c)'s + // finding, and re-reporting it here would double-report one typo. Same + // call `measureField` below makes for `chartConfig`. + const widgetOptions = (w.options && typeof w.options === 'object' && !Array.isArray(w.options)) + ? (w.options as AnyRec) + : undefined; + const sortBy = typeof widgetOptions?.sortBy === 'string' ? widgetOptions.sortBy : undefined; + if (sortBy && !dims.includes(sortBy) && !values.includes(sortBy)) { + // A name the DATASET declares but this widget did not select is the + // more helpful diagnosis — the fix is a `values`/`dimensions` entry, + // not a spelling correction — so it is named apart from a name the + // dataset does not declare at all. + const declaredButUnselected = dimensionNames.has(sortBy) || measures.has(sortBy); + const selected = [...dims, ...values]; + push({ + severity: 'error', + rule: WIDGET_SORTBY_UNSELECTED, + message: declaredButUnselected + ? `options.sortBy "${sortBy}" is declared by dataset "${dsName}" but is not ` + + `selected by this widget (selects: ${list(selected)}), so the query result ` + + `will not contain that column and the authored order cannot be applied.` + : `options.sortBy "${sortBy}" is neither a \`dimensions\` nor a \`values\` entry ` + + `of this widget (selects: ${list(selected)}) — \`sortBy\` must name one this ` + + `widget actually selects, so the authored order cannot be applied.`, + hint: declaredButUnselected + ? `Add "${sortBy}" to this widget's ${dimensionNames.has(sortBy) ? 'dimensions' : 'values'}, ` + + `or order by something it already selects.` + + `${suggest(sortBy, selected)}` + : `Point options.sortBy at one of this widget's selected names (${list(selected)}).` + + `${suggest(sortBy, selected)} Ordering is applied to the query RESULT, so it ` + + `can only name a column that result carries.`, + }); + } + // ── (d) chartConfig bindings resolve against the widget's selection ── const chartConfig = (w.chartConfig && typeof w.chartConfig === 'object') ? (w.chartConfig as AnyRec) From a33c81abab0193ad9827de4f4373822624738a98 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 19:09:26 +0000 Subject: [PATCH 2/4] test(lint): pin both #14148 limbs; path-precise findings --- .../lint/src/validate-widget-bindings.test.ts | 261 ++++++++++++++++++ packages/lint/src/validate-widget-bindings.ts | 14 +- 2 files changed, 273 insertions(+), 2 deletions(-) diff --git a/packages/lint/src/validate-widget-bindings.test.ts b/packages/lint/src/validate-widget-bindings.test.ts index 2539ff5f2c..612d6f89e4 100644 --- a/packages/lint/src/validate-widget-bindings.test.ts +++ b/packages/lint/src/validate-widget-bindings.test.ts @@ -12,6 +12,9 @@ import { WIDGET_LEGACY_ANALYTICS_UNRENDERABLE, DASHBOARD_FILTER_FIELD_UNKNOWN, DASHBOARD_FILTER_FIELD_UNPROVISIONED, + WIDGET_FILTER_FIELD_UNKNOWN, + WIDGET_FILTER_FIELD_NOT_INCLUDED, + WIDGET_SORTBY_UNSELECTED, } from './validate-widget-bindings.js'; /** The downstream repro from issue #1719 — dataset with a count AND a sum @@ -762,3 +765,261 @@ describe('validateWidgetBindings (dashboard-filter-field-unprovisioned, issue #8 expect(only(validateWidgetBindings(s))).toHaveLength(0); }); }); + +// ── [#14148] The widget's OWN filter keys and options.sortBy ───────────────── + +/** + * The card's measured shape, reduced: a widget bound to a dataset over + * `duly_task`, carrying its own presentation-scope `filter` and an + * `options.sortBy`. The dataset joins `owner` so the include clause has both a + * declared and an undeclared prefix to exercise. + */ +function widgetOwnStack( + widgetOverrides: Record = {}, + datasetOverrides: Record = {}, +) { + return { + objects: [ + { + name: 'duly_task', + fields: [ + { name: 'subject', type: 'text' }, + { name: 'due_date', type: 'date' }, + { name: 'business_unit', type: 'select' }, + { name: 'owner', type: 'lookup', reference: 'duly_user' }, + { name: 'estimate', type: 'number' }, + ], + }, + { + name: 'duly_user', + fields: [ + { name: 'name', type: 'text' }, + { name: 'region', type: 'select' }, + ], + }, + ], + datasets: [{ + name: 'duly_workload', + object: 'duly_task', + include: ['owner'], + dimensions: [{ name: 'business_unit', field: 'business_unit' }], + measures: [ + { name: 'untouched_over_14d', aggregate: 'count' }, + { name: 'total_estimate', aggregate: 'sum', field: 'estimate' }, + ], + ...datasetOverrides, + }], + dashboards: [{ + name: 'duly_duty_health', + widgets: [{ + id: 'not_moving_14d', + type: 'table', + dataset: 'duly_workload', + dimensions: ['business_unit'], + values: ['untouched_over_14d'], + ...widgetOverrides, + }], + }], + }; +} + +const idsOf = (fs: { rule: string }[], rule: string) => fs.filter((f) => f.rule === rule); + +describe('widget-filter-field-unknown (#14148 limb A)', () => { + it('errors on the card\'s repro — a widget filter key that is not a column', () => { + const findings = idsOf( + validateWidgetBindings(widgetOwnStack({ + filter: { due_daet: { $gte: '{today}', $lte: '{14_days_from_now}' } }, + })), + WIDGET_FILTER_FIELD_UNKNOWN, + ); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.severity).toBe('error'); + // Names dashboard, widget, key and object — the card's acceptance criterion. + expect(f.where).toBe('dashboard "duly_duty_health" › widget "not_moving_14d"'); + expect(f.message).toContain('due_daet'); + expect(f.message).toContain('duly_task'); + expect(f.path).toBe('dashboards[0].widgets[0].filter.due_daet'); + // The "did you mean", and the object's field list, both present. + expect(f.message).toContain('Did you mean "due_date"?'); + expect(f.hint).toContain('business_unit'); + }); + + it('is silent on the clean shape — a real column', () => { + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ + filter: { due_date: { $gte: '{today}' } }, + })), + WIDGET_FILTER_FIELD_UNKNOWN, + )).toHaveLength(0); + }); + + it('descends $and / $or / $not rather than reading only top-level keys', () => { + const findings = idsOf( + validateWidgetBindings(widgetOwnStack({ + filter: { $and: [{ due_date: { $lte: '{today}' } }, { $not: { bogus_column: 1 } }] }, + })), + WIDGET_FILTER_FIELD_UNKNOWN, + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('bogus_column'); + }); + + it('judges the `{ field, operator }` rule shape and the `[field, op, value]` triple too', () => { + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ + filter: { field: 'no_such_col', operator: 'equals', value: 1 }, + })), + WIDGET_FILTER_FIELD_UNKNOWN, + )).toHaveLength(1); + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ filter: ['no_such_col', '=', 1] })), + WIDGET_FILTER_FIELD_UNKNOWN, + )).toHaveLength(1); + }); + + it('RESOLVES a dotted path through a declared include — the sub-question, answered', () => { + // `owner` is declared in include and `region` is a real column on duly_user. + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ filter: { 'owner.region': 'emea' } })), + WIDGET_FILTER_FIELD_UNKNOWN, + )).toHaveLength(0); + // ...and a dangling leaf on the joined object is caught, not skipped. + const findings = idsOf( + validateWidgetBindings(widgetOwnStack({ filter: { 'owner.regionn': 'emea' } })), + WIDGET_FILTER_FIELD_UNKNOWN, + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('duly_user'); + }); + + it('reads a nested condition object as one dotted position, not a bare leaf', () => { + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ filter: { owner: { region: { $eq: 'emea' } } } })), + WIDGET_FILTER_FIELD_UNKNOWN, + )).toHaveLength(0); + }); + + it('errors when a hop is not a relationship at all', () => { + const findings = idsOf( + validateWidgetBindings(widgetOwnStack({ filter: { 'estimate.total': 1 } })), + WIDGET_FILTER_FIELD_UNKNOWN, + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('not a relationship'); + }); + + it('takes skip 1 — an object this stack does not define is never reported', () => { + const s = widgetOwnStack({ filter: { due_daet: 1 } }); + s.objects = s.objects.filter((o) => o.name !== 'duly_task'); + expect(idsOf(validateWidgetBindings(s), WIDGET_FILTER_FIELD_UNKNOWN)).toHaveLength(0); + }); + + it('takes skip 2 — an object with no readable field map is never reported', () => { + const s = widgetOwnStack({ filter: { due_daet: 1 } }); + delete (s.objects[0] as { fields?: unknown }).fields; + expect(idsOf(validateWidgetBindings(s), WIDGET_FILTER_FIELD_UNKNOWN)).toHaveLength(0); + }); + + it('takes skip 3 — a registry-injected system column resolves', () => { + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ filter: { created_at: { $gte: '{today}' } } })), + WIDGET_FILTER_FIELD_UNKNOWN, + )).toHaveLength(0); + }); +}); + +describe('widget-filter-field-not-included (#14148 limb A, the include clause)', () => { + it('errors when a resolvable dotted key traverses an UNDECLARED relationship', () => { + const findings = idsOf( + validateWidgetBindings(widgetOwnStack( + { filter: { 'owner.region': 'emea' } }, + { include: [] }, + )), + WIDGET_FILTER_FIELD_NOT_INCLUDED, + ); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.severity).toBe('error'); + expect(f.message).toContain('owner'); + expect(f.message).toContain('duly_workload'); + expect(f.hint).toContain('(none)'); + }); + + it('is silent for a bare base column — it needs no join', () => { + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ filter: { due_date: 1 } }, { include: [] })), + WIDGET_FILTER_FIELD_NOT_INCLUDED, + )).toHaveLength(0); + }); + + it('does not double-report: an unresolvable key yields the existence finding only', () => { + const all = validateWidgetBindings(widgetOwnStack( + { filter: { 'owner.regionn': 'emea' } }, + { include: [] }, + )); + expect(idsOf(all, WIDGET_FILTER_FIELD_UNKNOWN)).toHaveLength(1); + expect(idsOf(all, WIDGET_FILTER_FIELD_NOT_INCLUDED)).toHaveLength(0); + }); +}); + +describe('widget-sortby-unselected (#14148 limb B)', () => { + it('errors on the card\'s repro — sortBy names nothing the widget selects', () => { + const findings = idsOf( + validateWidgetBindings(widgetOwnStack({ + options: { sortBy: 'not_selected', sortOrder: 'asc' }, + })), + WIDGET_SORTBY_UNSELECTED, + ); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.severity).toBe('error'); + expect(f.where).toBe('dashboard "duly_duty_health" › widget "not_moving_14d"'); + expect(f.path).toBe('dashboards[0].widgets[0].options.sortBy'); + expect(f.message).toContain('not_selected'); + // Lists what the widget DOES select — the card's acceptance criterion. + expect(f.message).toContain('business_unit'); + expect(f.message).toContain('untouched_over_14d'); + }); + + it('is silent when sortBy names a selected dimension', () => { + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ options: { sortBy: 'business_unit' } })), + WIDGET_SORTBY_UNSELECTED, + )).toHaveLength(0); + }); + + it('is silent when sortBy names a selected measure', () => { + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ options: { sortBy: 'untouched_over_14d' } })), + WIDGET_SORTBY_UNSELECTED, + )).toHaveLength(0); + }); + + it('is silent when there is no sortBy at all', () => { + expect(idsOf( + validateWidgetBindings(widgetOwnStack({ options: { limit: 10 } })), + WIDGET_SORTBY_UNSELECTED, + )).toHaveLength(0); + }); + + it('names the declared-but-unselected case apart — the fix is a selection, not a spelling', () => { + const findings = idsOf( + validateWidgetBindings(widgetOwnStack({ options: { sortBy: 'total_estimate' } })), + WIDGET_SORTBY_UNSELECTED, + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('is declared by dataset "duly_workload" but is not'); + expect(findings[0].hint).toContain("this widget's values"); + }); + + it('does not double-report a dimension entry that rule (b) already errored on', () => { + const all = validateWidgetBindings(widgetOwnStack({ + dimensions: ['business_unitt'], + options: { sortBy: 'business_unitt' }, + })); + expect(idsOf(all, WIDGET_DIMENSION_UNKNOWN)).toHaveLength(1); + expect(idsOf(all, WIDGET_SORTBY_UNSELECTED)).toHaveLength(0); + }); +}); diff --git a/packages/lint/src/validate-widget-bindings.ts b/packages/lint/src/validate-widget-bindings.ts index 08625db682..4c3379d74c 100644 --- a/packages/lint/src/validate-widget-bindings.ts +++ b/packages/lint/src/validate-widget-bindings.ts @@ -462,9 +462,16 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { const path = `dashboards[${i}].widgets[${j}]`; const suppressed = (rule: string): boolean => Array.isArray(w.suppressWarnings) && w.suppressWarnings.includes(rule); - const push = (f: Omit): void => { + // [#14148] `path` defaults to the WIDGET, and a caller may override it + // with a position inside the widget. The two #14148 limbs do: a filter + // key is reported at `…widgets[j].filter.` and `sortBy` at + // `…widgets[j].options.sortBy`, matching the precision `filter-token-unknown` + // already offers in this exact subtree (`…widgets[4].filter.due_date.$lte`). + // Reporting a five-key filter's one bad key at the widget is a location the + // author still has to search. + const push = (f: Omit & { path?: string }): void => { if (f.severity === 'warning' && suppressed(f.rule)) return; - findings.push({ ...f, where, path }); + findings.push({ ...f, where, path: f.path ?? path }); }; // ── (a0) legacy pre-ADR-0021 analytics shape ── @@ -649,6 +656,7 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { push({ severity: 'error', rule: WIDGET_FILTER_FIELD_UNKNOWN, + path: at, message: `${account.message} The widget's own \`filter\` is ANDed into the ` + `dataset query as \`runtimeFilter\`, so the condition addresses a column ` + @@ -673,6 +681,7 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { push({ severity: 'error', rule: WIDGET_FILTER_FIELD_NOT_INCLUDED, + path: at, message: `filter key "${field}" resolves on the object graph, but its relationship ` + `prefix "${prefix}" is not declared in dataset "${dsName}"'s \`include\` — ` + @@ -759,6 +768,7 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { push({ severity: 'error', rule: WIDGET_SORTBY_UNSELECTED, + path: `${path}.options.sortBy`, message: declaredButUnselected ? `options.sortBy "${sortBy}" is declared by dataset "${dsName}" but is not ` + `selected by this widget (selects: ${list(selected)}), so the query result ` + From 59cf5f55e11918c01fdfc8eb4036813fabab3eb3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 19:12:54 +0000 Subject: [PATCH 3/4] test(lint): pin the validate+build acceptance criterion for both limbs --- .../lint/src/validate-widget-bindings.test.ts | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/lint/src/validate-widget-bindings.test.ts b/packages/lint/src/validate-widget-bindings.test.ts index 612d6f89e4..13411a57fb 100644 --- a/packages/lint/src/validate-widget-bindings.test.ts +++ b/packages/lint/src/validate-widget-bindings.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect } from 'vitest'; +import { runAuthoringRules, splitBySeverity } from './authoring-rules.js'; import { validateWidgetBindings, TABLE_COUNT_ONLY, @@ -823,7 +824,8 @@ function widgetOwnStack( }; } -const idsOf = (fs: { rule: string }[], rule: string) => fs.filter((f) => f.rule === rule); +const idsOf = (fs: T[], rule: string): T[] => + fs.filter((f) => f.rule === rule); describe('widget-filter-field-unknown (#14148 limb A)', () => { it('errors on the card\'s repro — a widget filter key that is not a column', () => { @@ -1023,3 +1025,39 @@ describe('widget-sortby-unselected (#14148 limb B)', () => { expect(idsOf(all, WIDGET_SORTBY_UNSELECTED)).toHaveLength(0); }); }); + +/** + * [#14148] The card's binding acceptance criterion, pinned end-to-end rather + * than inferred from the registry entry: BOTH limbs must fail `validate` AND + * `build`. `build` is the publish gate and is where these currently ship, so a + * validate-only fix was explicitly not acceptable — and nothing else in this + * file would notice if the entry's `commands` were narrowed later. + */ +describe('#14148 acceptance — both limbs gate `validate` AND `build`', () => { + const limbA = widgetOwnStack({ filter: { due_daet: { $gte: '{today}' } } }); + const limbB = widgetOwnStack({ options: { sortBy: 'not_selected', sortOrder: 'asc' } }); + + for (const command of ['validate', 'build'] as const) { + it(`limb A (widget filter key) fails \`${command}\``, () => { + const { errors } = splitBySeverity(runAuthoringRules(command, { normalized: limbA })); + expect(errors.map((f) => f.rule)).toContain(WIDGET_FILTER_FIELD_UNKNOWN); + }); + + it(`limb B (options.sortBy) fails \`${command}\``, () => { + const { errors } = splitBySeverity(runAuthoringRules(command, { normalized: limbB })); + expect(errors.map((f) => f.rule)).toContain(WIDGET_SORTBY_UNSELECTED); + }); + + it(`the clean shape passes \`${command}\` on both limbs`, () => { + const clean = widgetOwnStack({ + filter: { due_date: { $gte: '{today}' }, 'owner.region': 'emea' }, + options: { sortBy: 'business_unit', sortOrder: 'asc' }, + }); + const { errors } = splitBySeverity(runAuthoringRules(command, { normalized: clean })); + const mine = errors.filter((f) => [ + WIDGET_FILTER_FIELD_UNKNOWN, WIDGET_FILTER_FIELD_NOT_INCLUDED, WIDGET_SORTBY_UNSELECTED, + ].includes(f.rule)); + expect(mine).toEqual([]); + }); + } +}); From 5216886e2b62f59830dd01a8b98ea486a5de9c6d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 19:14:10 +0000 Subject: [PATCH 4/4] chore: changeset for #14148 --- .changeset/widget-filter-sortby-resolution.md | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 .changeset/widget-filter-sortby-resolution.md diff --git a/.changeset/widget-filter-sortby-resolution.md b/.changeset/widget-filter-sortby-resolution.md new file mode 100644 index 0000000000..bc79e2cecb --- /dev/null +++ b/.changeset/widget-filter-sortby-resolution.md @@ -0,0 +1,62 @@ +--- +'@objectstack/lint': minor +--- + +Resolve a dashboard widget's OWN `filter` keys and `options.sortBy` at validate/build + +A dashboard widget could filter by a column that does not exist, and order by a name +it never selected, and `objectstack validate` exited 0 with "Validation passed"; +`build` — the publish gate — wrote the dashboard into `dist/objectstack.json`. The +widget then rendered **empty**. + +The surrounding surface was already covered, which is what made the two misses so +narrow: `widget-dataset-unknown`, `widget-dimension-unknown`, `widget-measure-unknown`, +`filter-token-unknown` and `dashboard-filter-field-unknown` all failed both gates on the +same dashboard. On the very same node, the filter TOKEN was checked and the filter +COLUMN was not — `filter-token-unknown` fires path-precise at +`…widgets[4].filter.due_date.$lte`, so the traversal already walked the filter tree and +already knew the widget's dataset. Only the key resolution was missing. And +`options.sortBy` was declared-≠-enforced in the plainest way available: the spec states +the contract in its own prose — *"must be one this widget actually selects"* — and +nothing enforced it. + +Why this class of miss is expensive rather than untidy, in the reporter's words: the +dashboard it was measured on leads with a "not moving" tile — open work untouched more +than 14 days — and *"an empty tile is indistinguishable from a healthy team: a missing +number reads as zero, and zero is the answer the manager is hoping for."* The failure is +silent in the direction the reader wants to believe. + +Three gating rule ids, all at the site that already emits `widget-dataset-unknown` / +`dashboard-filter-field-unknown`, and all failing **`validate` and `build`** (pinned +end-to-end, not inferred from the registry entry): + +- `widget-filter-field-unknown` — a key of the widget's own `filter` resolves to no + column on the bound dataset's object graph. Reported path-precise at + `dashboards[i].widgets[j].filter.`, matching `filter-token-unknown`'s precision in + that same subtree. +- `widget-filter-field-not-included` — the key resolves, but its relationship prefix is + not declared in the dataset's `include`, so ADR-0021 compiles no join and the column is + out of the query's reach. +- `widget-sortby-unselected` — `options.sortBy` names neither a `dimensions[]` nor a + `values[]` entry of the widget. A name the dataset declares but the widget did not + select gets its own message, because the fix is a selection rather than a spelling. + +**A dotted path through a declared `include` is RESOLVED, not skipped.** A widget's +`filter` is ANDed into the dataset query as `runtimeFilter`, and that compiled query +carries only the joins `include` declared — so the same two clauses the dataset rule +applies one level down (existence, then joinability) apply here. The runtime is not a +backstop for the second: the dataset compiler's `assertDeclared` runs over `dimensions` +and `measures` only, never over `runtimeFilter`. + +Built on the seams that shipped with the dataset-level sibling rather than a second +implementation: `walkFilterFieldKeys` (all three authored filter shapes) and +`indexObjectGraph` / `resolveFieldPath`. Two helpers that were local to that rule — +`joinablePrefixes` and `describeFieldPathVerdict` — moved into the shared seam and are +now exported, because both are answers this position asks identically and copying either +would have been the second implementation the seam exists to prevent. + +Minor rather than patch: this narrows the accept set. Metadata that built yesterday and +names a column or an order that does not exist now fails the build — which is the point. +The three skips every field-existence rule in this package takes are unchanged, so an +object the stack does not define, an ADR-0015 `external` object with no readable field +map, and a registry-injected system column are never reported.