Skip to content

Commit fa1eca3

Browse files
baozhoutaoclaude
andauthored
fix(lint): resolve a dashboard widget's own filter keys and options.sortBy at validate/build (#14276)
* wip(lint): widget filter keys + sortBy limbs on the #14105 seam * test(lint): pin both #14148 limbs; path-precise findings * test(lint): pin the validate+build acceptance criterion for both limbs * chore: changeset for #14148 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6eb8e3c commit fa1eca3

6 files changed

Lines changed: 665 additions & 74 deletions

File tree

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
---
2+
'@objectstack/lint': minor
3+
---
4+
5+
Resolve a dashboard widget's OWN `filter` keys and `options.sortBy` at validate/build
6+
7+
A dashboard widget could filter by a column that does not exist, and order by a name
8+
it never selected, and `objectstack validate` exited 0 with "Validation passed";
9+
`build` — the publish gate — wrote the dashboard into `dist/objectstack.json`. The
10+
widget then rendered **empty**.
11+
12+
The surrounding surface was already covered, which is what made the two misses so
13+
narrow: `widget-dataset-unknown`, `widget-dimension-unknown`, `widget-measure-unknown`,
14+
`filter-token-unknown` and `dashboard-filter-field-unknown` all failed both gates on the
15+
same dashboard. On the very same node, the filter TOKEN was checked and the filter
16+
COLUMN was not — `filter-token-unknown` fires path-precise at
17+
`…widgets[4].filter.due_date.$lte`, so the traversal already walked the filter tree and
18+
already knew the widget's dataset. Only the key resolution was missing. And
19+
`options.sortBy` was declared-≠-enforced in the plainest way available: the spec states
20+
the contract in its own prose — *"must be one this widget actually selects"* — and
21+
nothing enforced it.
22+
23+
Why this class of miss is expensive rather than untidy, in the reporter's words: the
24+
dashboard it was measured on leads with a "not moving" tile — open work untouched more
25+
than 14 days — and *"an empty tile is indistinguishable from a healthy team: a missing
26+
number reads as zero, and zero is the answer the manager is hoping for."* The failure is
27+
silent in the direction the reader wants to believe.
28+
29+
Three gating rule ids, all at the site that already emits `widget-dataset-unknown` /
30+
`dashboard-filter-field-unknown`, and all failing **`validate` and `build`** (pinned
31+
end-to-end, not inferred from the registry entry):
32+
33+
- `widget-filter-field-unknown` — a key of the widget's own `filter` resolves to no
34+
column on the bound dataset's object graph. Reported path-precise at
35+
`dashboards[i].widgets[j].filter.<key>`, matching `filter-token-unknown`'s precision in
36+
that same subtree.
37+
- `widget-filter-field-not-included` — the key resolves, but its relationship prefix is
38+
not declared in the dataset's `include`, so ADR-0021 compiles no join and the column is
39+
out of the query's reach.
40+
- `widget-sortby-unselected``options.sortBy` names neither a `dimensions[]` nor a
41+
`values[]` entry of the widget. A name the dataset declares but the widget did not
42+
select gets its own message, because the fix is a selection rather than a spelling.
43+
44+
**A dotted path through a declared `include` is RESOLVED, not skipped.** A widget's
45+
`filter` is ANDed into the dataset query as `runtimeFilter`, and that compiled query
46+
carries only the joins `include` declared — so the same two clauses the dataset rule
47+
applies one level down (existence, then joinability) apply here. The runtime is not a
48+
backstop for the second: the dataset compiler's `assertDeclared` runs over `dimensions`
49+
and `measures` only, never over `runtimeFilter`.
50+
51+
Built on the seams that shipped with the dataset-level sibling rather than a second
52+
implementation: `walkFilterFieldKeys` (all three authored filter shapes) and
53+
`indexObjectGraph` / `resolveFieldPath`. Two helpers that were local to that rule —
54+
`joinablePrefixes` and `describeFieldPathVerdict` — moved into the shared seam and are
55+
now exported, because both are answers this position asks identically and copying either
56+
would have been the second implementation the seam exists to prevent.
57+
58+
Minor rather than patch: this narrows the accept set. Metadata that built yesterday and
59+
names a column or an order that does not exist now fails the build — which is the point.
60+
The three skips every field-existence rule in this package takes are unchanged, so an
61+
object the stack does not define, an ADR-0015 `external` object with no readable field
62+
map, and a registry-injected system column are never reported.

packages/lint/src/index.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ export {
2424
WIDGET_LEGACY_ANALYTICS_UNRENDERABLE,
2525
DASHBOARD_FILTER_FIELD_UNKNOWN,
2626
DASHBOARD_FILTER_FIELD_UNPROVISIONED,
27+
// [#14148] The widget's OWN two references, at the same site: the keys of its
28+
// presentation-scope `filter` (resolved on the #14105 object-graph seam, with
29+
// the ADR-0021 `include` clause its `runtimeFilter` really is subject to) and
30+
// `options.sortBy` against what the widget selects.
31+
WIDGET_FILTER_FIELD_UNKNOWN,
32+
WIDGET_FILTER_FIELD_NOT_INCLUDED,
33+
WIDGET_SORTBY_UNSELECTED,
2734
} from './validate-widget-bindings.js';
2835
export type { WidgetBindingFinding, WidgetBindingSeverity } from './validate-widget-bindings.js';
2936

@@ -485,16 +492,26 @@ export type { DatasetRefFinding, DatasetRefSeverity } from './validate-dataset-r
485492
// positions) must reuse ONE mechanism rather than growing a second hop-walker
486493
// and a second filter-key reader. Both hold mechanism only — no rule ids, no
487494
// severities, no findings; the judgement stays with the rule that asks.
495+
// [#14148] `joinablePrefixes` and `describeFieldPathVerdict` joined them when
496+
// the widget limb landed: both were local to the dataset rule, and both are
497+
// answers to questions the widget position asks identically — how ADR-0021
498+
// expands an `include` into joinable prefixes, and how one verdict reads in
499+
// prose. Copying either would have been the second implementation this seam
500+
// exists to prevent, one release after it was written to prevent it.
488501
export {
489502
indexObjectGraph,
490503
resolveFieldPath,
491504
isUnjudgeable,
505+
joinablePrefixes,
506+
describeFieldPathVerdict,
492507
nearestName,
493508
suggestName,
494509
listNames,
495510
RELATIONSHIP_FIELD_TYPES,
496511
} from './object-graph.js';
497-
export type { ObjectGraph, GraphObject, GraphField, FieldPathVerdict } from './object-graph.js';
512+
export type {
513+
ObjectGraph, GraphObject, GraphField, FieldPathVerdict, FieldPathAccount,
514+
} from './object-graph.js';
498515
export { walkFilterFieldKeys } from './filter-walk.js';
499516
export type { FilterFieldKey } from './filter-walk.js';
500517

packages/lint/src/object-graph.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,89 @@ export function resolveFieldPath(
248248
return { kind: 'field-unknown', object: current, field: leaf, candidates: obj.names };
249249
}
250250

251+
/**
252+
* The relationship prefixes a document declared as joinable.
253+
*
254+
* ADR-0021: *"Declaring `a.b` implicitly includes the intermediate `a`."* So
255+
* every PREFIX of every declared path is joinable, not only the paths as
256+
* written — which is why this expands rather than reading `include` verbatim.
257+
*
258+
* Here rather than in a rule because the SAME `include` governs positions two
259+
* different rules judge: a dataset's own `dimensions[].field` / `measures[].field`
260+
* / filter keys (#14105), and a dashboard widget's `filter` keys (#14148), whose
261+
* condition is ANDed into that same dataset's compiled query as `runtimeFilter`.
262+
* Two copies of the prefix expansion would let the two positions drift apart on
263+
* a clause that is one sentence of one ADR.
264+
*/
265+
export function joinablePrefixes(include: unknown): ReadonlySet<string> {
266+
const prefixes = new Set<string>();
267+
if (!Array.isArray(include)) return prefixes;
268+
for (const entry of include) {
269+
if (typeof entry !== 'string' || !entry) continue;
270+
const segments = entry.split('.');
271+
for (let i = 1; i <= segments.length; i++) {
272+
prefixes.add(segments.slice(0, i).join('.'));
273+
}
274+
}
275+
return prefixes;
276+
}
277+
278+
/** The two halves of a rendered verdict: the finding's message, and its detail. */
279+
export interface FieldPathAccount {
280+
/** What is wrong, in prose, carrying the "did you mean" when there is one. */
281+
message: string;
282+
/** The supporting field list, for the finding's hint. */
283+
detail: string;
284+
}
285+
286+
/**
287+
* Turn a resolution verdict into the message half of an existence finding, or
288+
* `undefined` when the verdict is one no rule may report.
289+
*
290+
* Shared by every position that resolves a field PATH — a dataset dimension, a
291+
* measure, a dataset filter key (#14105), a widget filter key (#14148) — so
292+
* they cannot drift into N different accounts of the same miss. The caller
293+
* supplies `subject` (how the position is named in prose) and owns the rule id,
294+
* the severity, the path and the hint's prescription; this function holds none
295+
* of them, matching the rest of this module.
296+
*/
297+
export function describeFieldPathVerdict(
298+
verdict: FieldPathVerdict,
299+
path: string,
300+
subject: string,
301+
): FieldPathAccount | undefined {
302+
switch (verdict.kind) {
303+
case 'ok':
304+
case 'unknowable':
305+
case 'hop-untargeted':
306+
return undefined;
307+
case 'hop-unknown':
308+
return {
309+
message:
310+
`${subject} "${path}" traverses "${verdict.segment}", which is not a field on object ` +
311+
`"${verdict.object}".${suggestName(verdict.segment, verdict.candidates)}`,
312+
detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`,
313+
};
314+
case 'hop-not-relationship':
315+
return {
316+
message:
317+
`${subject} "${path}" traverses "${verdict.segment}", which is a` +
318+
`${verdict.type ? ` \`${verdict.type}\`` : 'n ordinary'} field on object ` +
319+
`"${verdict.object}" and not a relationship — there is nothing to join through.`,
320+
detail:
321+
`Only ${[...RELATIONSHIP_FIELD_TYPES].sort().join(' / ')} fields are traversable ` +
322+
`(ADR-0021 derives every join from the object graph; you never write an ON clause).`,
323+
};
324+
case 'field-unknown':
325+
return {
326+
message:
327+
`${subject} "${path}" is not a field on object "${verdict.object}".` +
328+
`${suggestName(verdict.field, verdict.candidates)}`,
329+
detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`,
330+
};
331+
}
332+
}
333+
251334
/**
252335
* True when the verdict is one no rule may report — the graph could not answer.
253336
* Callers spell the skip through this predicate rather than re-listing the

packages/lint/src/validate-dataset-references.ts

Lines changed: 4 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,11 @@
118118
import { walkFilterFieldKeys } from './filter-walk.js';
119119
import {
120120
RELATIONSHIP_FIELD_TYPES,
121+
describeFieldPathVerdict,
121122
indexObjectGraph,
122123
isUnjudgeable,
123-
listNames,
124+
joinablePrefixes,
124125
resolveFieldPath,
125-
suggestName,
126-
type FieldPathVerdict,
127126
type ObjectGraph,
128127
} from './object-graph.js';
129128

@@ -172,72 +171,6 @@ function asArray(v: unknown): AnyRec[] {
172171
return [];
173172
}
174173

175-
/**
176-
* The relationship prefixes a dataset declared as joinable.
177-
*
178-
* ADR-0021: *"Declaring `a.b` implicitly includes the intermediate `a`."* So
179-
* every PREFIX of every declared path is joinable, not only the paths as
180-
* written — which is why this expands rather than reading `include` verbatim.
181-
*/
182-
function joinablePrefixes(include: unknown): ReadonlySet<string> {
183-
const prefixes = new Set<string>();
184-
if (!Array.isArray(include)) return prefixes;
185-
for (const entry of include) {
186-
if (typeof entry !== 'string' || !entry) continue;
187-
const segments = entry.split('.');
188-
for (let i = 1; i <= segments.length; i++) {
189-
prefixes.add(segments.slice(0, i).join('.'));
190-
}
191-
}
192-
return prefixes;
193-
}
194-
195-
/**
196-
* Turn a resolution verdict into the message half of an existence finding, or
197-
* `undefined` when the verdict is one no rule may report.
198-
*
199-
* Shared by the three positions that resolve a field PATH (dimension, measure,
200-
* filter key) so they cannot drift into three different accounts of the same
201-
* miss. The caller supplies `subject` — how the position is named in prose —
202-
* and owns the rule id, the path and the hint's prescription.
203-
*/
204-
function existenceMessage(
205-
verdict: FieldPathVerdict,
206-
path: string,
207-
subject: string,
208-
): { message: string; detail: string } | undefined {
209-
switch (verdict.kind) {
210-
case 'ok':
211-
case 'unknowable':
212-
case 'hop-untargeted':
213-
return undefined;
214-
case 'hop-unknown':
215-
return {
216-
message:
217-
`${subject} "${path}" traverses "${verdict.segment}", which is not a field on object ` +
218-
`"${verdict.object}".${suggestName(verdict.segment, verdict.candidates)}`,
219-
detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`,
220-
};
221-
case 'hop-not-relationship':
222-
return {
223-
message:
224-
`${subject} "${path}" traverses "${verdict.segment}", which is a` +
225-
`${verdict.type ? ` \`${verdict.type}\`` : 'n ordinary'} field on object ` +
226-
`"${verdict.object}" and not a relationship — there is nothing to join through.`,
227-
detail:
228-
`Only ${[...RELATIONSHIP_FIELD_TYPES].sort().join(' / ')} fields are traversable ` +
229-
`(ADR-0021 derives every join from the object graph; you never write an ON clause).`,
230-
};
231-
case 'field-unknown':
232-
return {
233-
message:
234-
`${subject} "${path}" is not a field on object "${verdict.object}".` +
235-
`${suggestName(verdict.field, verdict.candidates)}`,
236-
detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`,
237-
};
238-
}
239-
}
240-
241174
/** The shared consequence sentence — why an unresolved path is not merely inert. */
242175
const SILENT_EMPTY =
243176
'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[] {
309242
return;
310243
}
311244

312-
const account = existenceMessage(verdict, entry, `include[${ii}]`);
245+
const account = describeFieldPathVerdict(verdict, entry, `include[${ii}]`);
313246
if (!account) return;
314247
findings.push({
315248
severity: 'error',
@@ -342,7 +275,7 @@ export function validateDatasetReferences(stack: AnyRec): DatasetRefFinding[] {
342275
const verdict = resolveFieldPath(graph, object, written);
343276
if (isUnjudgeable(verdict) || !verdict) return;
344277

345-
const account = existenceMessage(verdict, written, subject);
278+
const account = describeFieldPathVerdict(verdict, written, subject);
346279
if (account) {
347280
findings.push({
348281
severity: 'error',

0 commit comments

Comments
 (0)