diff --git a/.changeset/22161-lint-slice-6-one-line.md b/.changeset/22161-lint-slice-6-one-line.md new file mode 100644 index 00000000000..ab18932ef8d --- /dev/null +++ b/.changeset/22161-lint-slice-6-one-line.md @@ -0,0 +1,20 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): the list-view sort and search, form-predicate path, bulk-dispatch, component-type and preset-comparand findings print one verdict line, and `os explain ` carries their reasoning + +Clause-②: no + +- **Shorter verdicts.** Each finding of these 12 rule ids now prints a `message` of one verdict sentence. Every finding the rules' own test suites fire is at most 193 characters, and the runtime publish gate's suites at most 137; before, the longest of each ran from 287 to 927 characters. The ids: + - list-view `sort` (`objects[].listViews`, `views[]` lists, list overlays and ViewItem records): `sort-field-unknown`, `sort-field-unsortable`, `sort-field-unprovisioned`; + - `searchableFields` (the object's own set, list views, and a react page's ``): `searchable-field-unknown`, `searchable-field-unsearchable`, `searchable-field-unprovisioned`; + - metadata-form `visibleWhen` predicates (`views[]` forms bound to a schema): `predicate-path-unresolved`, `predicate-path-unrooted`, `predicate-rhs-path-shaped`; + - list-view bulk wiring: `action-dispatch-contract-mismatch`; + - page component types (`pages[]`): `component-type-unknown`; + - filter comparands: `filter-preset-comparand`. + + A verdict no longer repeats what the finding's `where` already names: the view that wires an action (`action-dispatch-contract-mismatch`). The unprovisioned-anchor ids print the same one-clause cause the other converted anchor rules print (`'owner_id' is an injected column with no storage on external object 'x'`), and the two virtual-entry ids (`sort-field-unsortable`, `searchable-field-unsearchable`) state the storage fact in one shared wording. `searchable-field-unsearchable` quotes at most three names of an object's declared set, then `(and N more)`. A retired component type (`user:profile`, `element:filter`, `element:form`, `ai:chat_window`) is quoted to the head of its prescription in `RETIRED_PAGE_COMPONENT_TYPES` (`` `element:filter` was removed in @objectstack/spec 17 (ADR-0049) ``), and the verdict says the parse refuses the node by name; the whole prescription is the parse door's refusal of the same name, which `os validate` and `os build` print. `filter-preset-comparand` opens with the first sentence of the refusal the schema door shares (`"last_30_days" is a dashboard date-range PRESET name, not a filter value`), then names the operator and the preset's `{date-macro}` window. The `fix` (the CLI's `fix:` line, the runtime issue's `hint`), every rule id, severity and `path`, and what each rule accepts or refuses are unchanged. A tool that matched the old message text should match on `rule` and `path` instead. +- **`os explain ` takes these 12 ids**, for example `os explain sort-field-unknown`. It prints the reasoning the verdicts no longer carry: why an unknown sort field breaks a view's first fetch and every load after it, and which list-view surfaces the sort and search rules walk and skip; what a `formula` field's lack of storage does to an ORDER BY and to a search; what an unprovisioned anchor is and what sorting or searching one measured; how a stale `searchableFields` entry narrows a search or falls through to the auto-default set, and how a list view's narrowing reaches the runtime as the `$searchFields` override; how a metadata-form predicate path is resolved against the edited schema, why a dead predicate fails open, and why the right side of `==` / `!=` is a literal; how the two bulk wirings call an action and why nothing refuses a mismatch at run time; which component types the vocabulary closes and where a retired type's prescription lives; and where a date-range preset name is understood and what each layer does with a bare one. Paragraphs shared across ids are one text, printed under every id they explain. The `rule:` line under each of these findings now ends with `` — `os explain ` for … ``. The no-argument listing and its `--json` `rules` array list the 12 ids, and so does the unknown-id error's `Rules with an explanation:` line. `RULE_EXPLANATIONS` in `@objectstack/lint` gains the 12 entries. +- **Where the new text prints.** On the CLI, all 12 ids: `os validate`, `os build` (and `os compile`, which `os dev` runs on every compile), `os lint`, `os verify`, and the scaffold check `os init` and `os generate` run print the new `message` on the text face, and `os validate --json` and `os build --json` carry it in their `errors` and author-time `issues`. At the runtime publish gate (Studio, REST `/meta`, MCP), the 422 issue's `message` and the refusal log line under `OS_ALLOW_UNLINTED_METADATA_WRITES` change for: the three `searchable-field-*` ids on a `view`, `object` or `flow` write; the three `sort-field-*` ids on a `view` or `flow` write; the three `predicate-*` ids on a `view` write; and `filter-preset-comparand` on a `dashboard`, `view`, `object`, `page`, `flow` or `report` write. Each issue's `hint` is unchanged. +- **Never at the runtime gate:** `component-type-unknown`, which runs on the CLI doors only, and `action-dispatch-contract-mismatch`, whose rule runs at that door only for a `flow` write, whose snapshot carries no actions to judge, so both speak only on the CLI doors above. diff --git a/content/docs/data-modeling/schema-design.mdx b/content/docs/data-modeling/schema-design.mdx index 5a729b8e6a7..7a78034b429 100644 --- a/content/docs/data-modeling/schema-design.mdx +++ b/content/docs/data-modeling/schema-design.mdx @@ -167,10 +167,8 @@ send you to the same fix, so either message is greppable back to this section. `os validate` reports `searchable-field-unknown`: ```text -searchableFields entry "project_id.name" is not a field on object "task". The -declaration is stale: searching it can never match, and the engine silently -drops it — leaving a narrower search than declared, or the auto-default set once -every entry is dropped. +searchableFields entry "project_id.name" is not a field on object "task", so the +engine drops it from the search. hint: 'search' scans this object's own columns, so a related record's column cannot be a search target — expand the relation and search the related object, diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 1d7df6c965a..c52e66aa6f8 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -367,7 +367,8 @@ const DASHBOARD_FILTER_FIELD_NOT_INCLUDED_EXPLANATION: RuleExplanation = { /** * What an unprovisioned anchor is — the cause half of every read-side anchor - * id (`dashboard-filter-field-unprovisioned`, `react-chart-field-unprovisioned`). + * id (`dashboard-filter-field-unprovisioned`, `react-chart-field-unprovisioned`, + * `sort-field-unprovisioned`, `searchable-field-unprovisioned`). * The rule files share its verdict clause, `unprovisionedAnchorVerdict()`. */ const UNPROVISIONED_ANCHOR_CAUSE = @@ -1719,6 +1720,320 @@ const REACT_PAGE_SOURCE_UNPARSEABLE_EXPLANATION: RuleExplanation = { ], }; +// ── List-view sort and search fields (the SORT and SEARCH axes) ───────────── +// `validate-sortable-fields.ts` and `validate-searchable-fields.ts` resolve a +// list view's `sort` and `searchableFields` against one object index, so the +// two axes share their surfaces, their skips and their storage fact. + +/** Which declarations the two axes judge, and where they stop (both write it out). */ +const LIST_VIEW_FIELD_SKIPS = + 'Not judged, so a miss is never a false finding: an object this stack does not define (it may ' + + 'come from another package), an object with no authored field map (an ADR-0015 external object ' + + 'or an introspected datasource), and a registry-injected system column such as `created_at`, ' + + 'which exists at runtime but is never in authored `fields` — whose one question, on an external ' + + 'object, is the matching `*-unprovisioned` warning\'s.'; + +/** + * The storage fact behind the two virtual-entry ids (`sort-field-unsortable`, + * `searchable-field-unsearchable`). The rule files share its verdict clause, + * `virtualFieldClause()`. + */ +const VIRTUAL_FIELD_STORAGE = + 'A `formula` field\'s value is computed on read and never stored, so no driver materializes a ' + + 'column for it. Virtuality is judged by the spec\'s storage predicate (`isVirtualSearchField`, ' + + '`SEARCH_VIRTUAL_TYPES`: `formula` alone), the one the REST ingress and the engine read, so the ' + + 'linter and the runtime cannot disagree. `summary` and `autonumber` are not judged: they are ' + + 'stored (the engine maintains the one and assigns the other), and the spec\'s ' + + '`COMPUTED_VALUE_TYPES` that groups them with `formula` is the write contract, not a storage ' + + 'fact.'; + +/** What a list view's `searchableFields` becomes at request time (the three search ids). */ +const SEARCH_FIELDS_ECHO = + 'A list view\'s `searchableFields` narrows the object\'s searchable set (ADR-0061): clients echo ' + + 'it verbatim as the `$searchFields` override on every toolbar search, and the REST ingress ' + + '(`assertSearchFieldsAreSearchable`) refuses an entry outside the object\'s allowed set with ' + + '`400 INVALID_FIELD` — the whole toolbar search, for every role. A `` ' + + 'prop on a react page is judged by the same core. The object\'s own `searchableFields` is the ' + + 'canonical set.'; + +const SORT_FIELD_UNKNOWN_EXPLANATION: RuleExplanation = { + rule: 'sort-field-unknown', + covers: 'why an unknown sort field breaks the whole view', + paragraphs: [ + 'A list view\'s `sort` is the ORDER BY its FIRST fetch carries. A name that is not a field of ' + + 'the bound object is refused at the REST ingress (`assertSortFieldsExist`), not dropped: every ' + + 'load of the view answers `400 INVALID_SORT`, and the cause is an authoring typo made long ' + + 'before. `ListViewSchema.sort` types the name as a bare string, so the parse cannot see it.', + 'The name is judged on its HEAD segment, as the ingress gate judges it, so the two agree about ' + + 'which names are unknown; a dotted name with a known head passes here and is refused at request ' + + 'time, because `sort` reaches only the object\'s own columns.', + 'Walked: an object\'s built-in `listViews`, a `defineView` aggregate\'s `list` and `listViews`, a ' + + 'flattened list overlay (a `views[]` entry with `viewKind: \'list\'`, the shape ' + + '`PUT /api/v1/meta/view` carries) and a ViewItem record\'s `config`. A report\'s ordering, a ' + + 'dashboard widget\'s `sortBy` and flow nodes have no field-name sort to judge here.', + LIST_VIEW_FIELD_SKIPS, + ], +}; + +const SORT_FIELD_UNSORTABLE_EXPLANATION: RuleExplanation = { + rule: 'sort-field-unsortable', + covers: 'why a formula field cannot be sorted by', + paragraphs: [ + VIRTUAL_FIELD_STORAGE, + 'Measured before the runtime refused it: an ORDER BY over a formula returned \'asc\' and \'desc\' ' + + 'in byte-identical row order, carrying the very values it was asked to order by, unordered, ' + + 'under a success. Both runtime doors now refuse it with `400 INVALID_SORT` — the REST ingress ' + + '(`assertSortFieldsExist`) and the engine (`assertOrderByIsMaterializable`) — so the declaration ' + + 'breaks the view\'s first fetch, and every fetch after it.', + LIST_VIEW_FIELD_SKIPS, + ], +}; + +const SORT_FIELD_UNPROVISIONED_EXPLANATION: RuleExplanation = { + rule: 'sort-field-unprovisioned', + covers: 'why sorting by an unprovisioned anchor loses the order', + paragraphs: [ + UNPROVISIONED_ANCHOR_CAUSE, + 'Measured on a federated object over a real remote table: the ORDER BY reaches the driver, finds ' + + 'no column and is dropped, so \'asc\' and \'desc\' return byte-identical row order under a 200 ' + + 'while the same query on a real column reverses. Unlike the two other sort ids nothing refuses ' + + 'it — the REST ingress and the engine judge only `formula` — so the view\'s first fetch and every ' + + 'fetch after it answer in an arbitrary order, which `limit` / `offset` then slice into an ' + + 'arbitrary page.', + 'Only an undotted name that resolved BECAUSE it is injected is judged: an author-declared column ' + + 'of the same name is one the author vouches for, and a dotted sort name is refused by the ' + + 'ingress gate on its own. The rule is a warning because it cannot see the remote table, only ' + + 'that the platform provisions no storage.', + ], +}; + +const SEARCHABLE_FIELD_UNKNOWN_EXPLANATION: RuleExplanation = { + rule: 'searchable-field-unknown', + covers: 'what a stale searchableFields entry does', + paragraphs: [ + '`searchableFields` is an array of bare strings on the object and on a list view, so the parse ' + + 'never checks that an entry resolves. The engine tolerates a stale one — `resolveSearchFields` ' + + 'filters the declaration down to fields that exist — and that tolerance hides it: with some ' + + 'entries stale, search scans a narrower set than declared and a record that should match does ' + + 'not; with every entry stale, resolution falls through to the AUTO-DEFAULT set, which the ' + + 'author never wrote.', + SEARCH_FIELDS_ECHO, + 'A dotted entry is judged too, unlike in other field rules: search matches the field map by exact ' + + 'string, so `owner_id.name` is dropped exactly like a typo.', + LIST_VIEW_FIELD_SKIPS, + ], +}; + +const SEARCHABLE_FIELD_UNSEARCHABLE_EXPLANATION: RuleExplanation = { + rule: 'searchable-field-unsearchable', + covers: 'which searchableFields entries the runtime refuses', + paragraphs: [ + SEARCH_FIELDS_ECHO, + // Held equal to `SEARCHABLE_TEXTUAL_TYPES` + `SEARCHABLE_ENUM_TYPES` by the rule's tests. + 'The allowed set is computed by the function the ingress and the engine consult ' + + '(`resolveSearchFieldResolution`): the object\'s declared `searchableFields` when it declares ' + + 'them, otherwise the AUTO-DEFAULT set — its text-like columns (text / email / phone / url / ' + + 'autonumber / textarea / markdown / select), never a system/audit column or a hidden field. A ' + + '`lookup` or `master_detail` column stores only the referenced record\'s id, so it is never a ' + + 'keyword target. The verdict quotes at most three names of a declared set.', + VIRTUAL_FIELD_STORAGE, + 'A formula entry is refused on EVERY surface, the object\'s own set included: measured, it ' + + 'matched 0 rows on driver-memory, and 0 rows with no error on driver-sql. Otherwise the ' + + 'object\'s own `searchableFields` is existence-only: its declared branch scans any stored ' + + 'column, so a json or lookup column declared there is a narrow choice the engine executes, not ' + + 'a finding. A system column outside the allowed set is not judged either: its runtime ' + + 'metadata is registry-owned and invisible here.', + ], +}; + +const SEARCHABLE_FIELD_UNPROVISIONED_EXPLANATION: RuleExplanation = { + rule: 'searchable-field-unprovisioned', + covers: 'why searching an unprovisioned anchor matches nothing', + paragraphs: [ + UNPROVISIONED_ANCHOR_CAUSE, + 'Existence says yes — the anchor is a registry-injected column — so the entry survives into the ' + + 'resolved allow-list. On a list view it is echoed as the `$searchFields` override, so every ' + + 'toolbar search on the list scans a column that is empty on every record: it reads as search ' + + 'coverage and matches nothing. On the object\'s own set, search scans it on every record and ' + + 'never matches, so the searchable set is narrower than it declares — and if it is the only ' + + 'entry that resolves, the set scans nothing at all.', + 'Only a name that resolved BECAUSE it is injected is judged: an author-declared column of the ' + + 'same name is one the author vouches for. The rule is a warning because it cannot see the ' + + 'remote table, only that the platform provisions no storage.', + ], +}; + +// ── Metadata-form predicate paths (`validate-predicate-path-refs.ts`) ─────── +// A metadata-editing form's `visibleWhen` predicate, judged against the schema +// of the metadata type the form edits; the three ids share the scope. + +/** Which predicates the three ids judge, and the oracle they ask (all three). */ +const PREDICATE_FORM_SCOPE = + 'Judged: the `visibleWhen` (or `visibleOn`) predicates on a metadata-editing form — a `views[]` ' + + 'form, its default `form` or a `formViews` entry whose data source is `{ provider: \'schema\', ' + + 'schemaId }` — on its sections and fields, nested repeater rows included, where `data` is rebound ' + + 'to the ROW. A path is resolved against the Zod schema of the metadata type the form edits ' + + '(`getMetadataTypeSchema(schemaId)`, the entry `saveMetaItem` validates the saved row against), ' + + 'never asked of CEL. A form bound to an ObjectQL object (`record.*`) is not judged: lookup ' + + 'traversal, injected columns and formula outputs leave that set open, and an error-level gate ' + + 'over an open set refuses good metadata.'; + +/** What a predicate that resolves to nothing does in the console (the two resolution ids). */ +const PREDICATE_FAIL_OPEN = + 'A reference that resolves to nothing makes the predicate unevaluable, and the console\'s ' + + 'evaluator then fails OPEN — its settled behaviour: the element renders unconditionally, ' + + 'pixel-identical to one carrying no predicate at all, so nothing tells the author the condition ' + + 'is dead.'; + +const PREDICATE_PATH_UNRESOLVED_EXPLANATION: RuleExplanation = { + rule: 'predicate-path-unresolved', + covers: 'how a predicate path is resolved, and where it stops', + paragraphs: [ + PREDICATE_FAIL_OPEN, + 'A `data.`-rooted path is walked segment by segment through the target schema; the first segment ' + + 'the schema does not declare at that point is the verdict. The syntax rules cannot see this: ' + + '`data.tpye == \'formula\'` parses, is rooted, and is rooted on the right root.', + PREDICATE_FORM_SCOPE, + 'Where the walk stops — a missed catch, never a false error: a scope that is not key-bearing (a ' + + 'record map\'s values, `unknown`, `any`, an array reached by `.`), below which nothing is ' + + 'judged; a record map\'s KEY segment (`data.fields.acme` accepts any `acme`); index access ' + + '(`data.x[\'y\']`); a predicate that does not parse (`visibility-predicate-syntax` reports it); ' + + 'and a `schemaId` that resolves to no schema.', + ], +}; + +const PREDICATE_PATH_UNROOTED_EXPLANATION: RuleExplanation = { + rule: 'predicate-path-unrooted', + covers: 'why a bare schema key in a form predicate never matches', + paragraphs: [ + 'A metadata-editing form binds the row under edit as `data`, at every depth — inside a repeater ' + + '`data` is the ROW, still spelled `data` — and never flattens its values to top level. A bare ' + + 'identifier that IS a key of the edited schema is that key with its root dropped: it resolves ' + + 'to nothing.', + PREDICATE_FAIL_OPEN, + 'Narrower than `visibility-bare-identifier` on purpose: the schema-key membership makes the ' + + 'verdict unambiguous, and it reaches an identifier CEL itself declares, such as `type`, which ' + + 'the strict CEL checker reports as an overload rather than an unknown variable. Not reported: a ' + + 'comprehension-macro variable (`data.tags.all(t, …)`), and a word used only as the bare right ' + + 'operand of `==` / `!=`, which is `predicate-rhs-path-shaped`\'s literal slot.', + PREDICATE_FORM_SCOPE, + ], +}; + +const PREDICATE_RHS_PATH_SHAPED_EXPLANATION: RuleExplanation = { + rule: 'predicate-rhs-path-shaped', + covers: 'why the right side of == or != is a literal', + paragraphs: [ + 'The metadata-admin evaluator resolves the LEFT side of `==` / `!=` as a path and hands the RIGHT ' + + 'side to its literal parser, which returns anything it does not recognise as a literal ' + + 'verbatim. So `data.a == data.b` compares `data.a` against the string "data.b": it is FALSE ' + + 'however equal the two values are — an `==` written this way hides the element on every row — ' + + 'and `data.a != data.b` is correspondingly TRUE. The supported subset is `path == \'literal\'` ' + + 'and `path != \'literal\'`, nothing wider, and the evaluator says so only in a development build.', + 'Two severities under one id. A dotted chain is an error: nobody writes one meaning its text, so ' + + 'there is no reading under which it worked. A bare word (`status == active`) is a warning: it ' + + 'compares as the text today, which is usually what was meant, but it is outside the subset and ' + + 'breaks when this surface moves to the real CEL evaluator, where a bare `active` is an ' + + 'undeclared reference.', + 'The bare word also reads as a `data.` root someone dropped, so the finding carries both ' + + 'readings and the `fix:` line names both spellings. Adding the root in place (`== data.active`) ' + + 'is not one of them: it is a path on the right, this id\'s error arm. For that one position ' + + '`visibility-bare-identifier` and `predicate-path-unrooted` stand down, so the author gets one ' + + 'prescription rather than three contradicting ones.', + 'The right-hand grammar is the console\'s own path-shaped-literal pattern, so the producer and the ' + + 'renderer refuse and warn about the same set; `true`, `false`, `null`, numbers and quoted strings ' + + 'are literals and never reported. The check asks about a POSITION, not a resolution, so it runs ' + + 'even where the form\'s schema is unknown.', + ], +}; + +// ── List-view bulk dispatch (`validate-action-dispatch-contract.ts`) ──────── + +const ACTION_DISPATCH_CONTRACT_MISMATCH_EXPLANATION: RuleExplanation = { + rule: 'action-dispatch-contract-mismatch', + covers: 'how the two bulk wirings call an action', + paragraphs: [ + 'A list view can wire one declared action two ways, and they hand the same body OPPOSITE input. ' + + '`bulkActions: [\'NAME\']`, the bare-string form, is `execution: \'perRecord\'`: the renderer ' + + 'promotes the action to a def and dispatches it ONCE PER selected row, each call carrying that ' + + 'row\'s `recordId` and no `_selectedIds`. A `bulkActionDefs` entry with ' + + '`execution: \'aggregate\'` dispatches ONCE for the whole selection, with every selected id in ' + + '`params._selectedIds` and no `recordId`.', + 'Wired against its declaration, a body misreads its input in silence: one written for the ' + + 'aggregate call reads `_selectedIds` as `undefined` on every per-row call, falls through to its ' + + 'single-record branch and reports success for one row of the selection; one written per record ' + + 'finds no `recordId` on the single aggregate call and throws its own "nothing selected", which ' + + 'reads like a selection bug rather than a wiring one.', + 'Nothing refuses it at runtime: `recordId` and `_selectedIds` are both builtin action params ' + + '(ADR-0104), so the strict params gate admits either bag, and no single parse sees both ends — ' + + 'the wiring lives on the view, the declaration on the action. The verdict names both contracts ' + + 'because the fix is a choice between them.', + 'Not reported: an action that declares no `execution` (undeclared is not defaulted to either ' + + 'contract; the ADR-0087 migration `action-bulk-dispatch-contract-undeclared` derives ' + + 'declarations where a wiring is unambiguous); a name declared with different contracts, or ' + + 'declared in one place and undeclared in another, anywhere in the stack\'s one action namespace; ' + + 'an `update` / `delete` def or a hand-inlined `actionDef`; and a name that resolves to no action ' + + '(`action-name-undefined`).', + ], +}; + +// ── Page component types (`validate-component-types.ts`) ──────────────────── + +const COMPONENT_TYPE_UNKNOWN_EXPLANATION: RuleExplanation = { + rule: 'component-type-unknown', + covers: 'which component types the platform vocabulary closes', + paragraphs: [ + '`PageComponentSchema.type` accepts a standard component type or any string, so an arbitrary ' + + 'string parses — on purpose: plugin widgets (`mcp:connect-agent`), kebab SDUI blocks (`flex`, ' + + '`page-header`) and dot shapes (`custom.widget`) depend on that open arm. What it also ' + + 'swallowed was a typo inside the spec\'s OWN namespaces: `global:serch` validated clean on every ' + + 'authoring command, and the console drew the component-placeholder scaffold in front of the end ' + + 'user.', + 'So a `type` inside a namespace the standard vocabulary populates (derived from the enum, never ' + + 'restated) must be a type the spec answers for — an enum member, a `ComponentPropsMap` row or a ' + + 'registered string-arm type — and the verdict names the closest declared spellings. A type ' + + 'outside those namespaces is untouched. It was an error from birth because the in-repo page ' + + 'corpus was measured clean first. A source-authored (`kind: \'react\'`) page is not walked.', + 'A RETIRED type (`RETIRED_PAGE_COMPONENT_TYPES`) is reported by exact name, before the namespace ' + + 'check, because a retirement can take its namespace with it: `user:profile` was the `user:` ' + + 'namespace\'s only member. The verdict quotes the head of the retirement\'s prescription. The ' + + 'whole prescription — what replaces the type, or that nothing does — is the parse door\'s ' + + 'refusal of the same name, which `os validate` and `os build` print, and the kept ' + + '`ComponentPropsMap` row carries it too: one copy, never restated here.', + ], +}; + +// ── Filter comparands (`validate-preset-comparands.ts`) ──────────────────── + +const FILTER_PRESET_COMPARAND_EXPLANATION: RuleExplanation = { + rule: 'filter-preset-comparand', + covers: 'where a date-range preset name is understood', + paragraphs: [ + 'The 13 dashboard date-range presets (`last_30_days`, `this_quarter`, …) are real names only in ' + + 'the dashboard date-filter positions — `dateRange.defaultRange` and a date global filter\'s ' + + '`defaultValue` — where the console lowers them to `{date-macro}` bounds before any query. As a ' + + 'bare filter comparand nothing resolves them: a declared `date` / `datetime` field refuses the ' + + 'query at the engine (`INVALID_FILTER`, 400), and any other column compares the literal string. ' + + 'Measured before the refusal: `$gte "last_30_days"` answered 200 with 0 rows where ' + + '`{30_days_ago}` found 38.', + 'Two arms. An ordering position — `$gt` / `$gte` / `$lt` / `$lte`, a `$between` endpoint, and ' + + 'their infix and view-rule spellings — is refused on any column: an ordered comparison against a ' + + 'preset name has no legitimate reading. An equality or membership position — bare, `$eq` / ' + + '`$ne`, `$in` / `$nin` and their spellings — is refused only on a declared `date` / `datetime` ' + + 'field, because a picklist column may store a value named like a preset. A bare value is ' + + 'reported under the operator it lowers to, `$eq`.', + 'The fix is the preset\'s `{date-macro}` window, which the verdict names — a rolling preset is ' + + '`{ $gte: START }`, a calendar one `{ $between: [START, END] }` — or an ISO date. The schema ' + + 'door refuses the Mongo-shape ordering positions too, with the whole explanation in its ' + + 'refusal; this rule also reaches view filter rules and filter-array triples, which no condition ' + + 'parse touches, and is the one door for the equality arm.', + 'Not judged: a filter no ancestor binds to an object, an object or dataset the stack does not ' + + 'declare, a `time` field, and a field the object does not declare (the `*-filter-field-unknown` ' + + 'rules\' finding).', + ], +}; + + /** * Every rule explanation this package ships, keyed by rule id. `os explain * ` reads this table and nothing else. @@ -1799,6 +2114,18 @@ export const RULE_EXPLANATIONS: Readonly> = Obje [REACT_CHART_FIELD_UNPROVISIONED_EXPLANATION.rule]: REACT_CHART_FIELD_UNPROVISIONED_EXPLANATION, [REACT_BLOCK_NEEDS_RECORD_CONTEXT_EXPLANATION.rule]: REACT_BLOCK_NEEDS_RECORD_CONTEXT_EXPLANATION, [REACT_PAGE_SOURCE_UNPARSEABLE_EXPLANATION.rule]: REACT_PAGE_SOURCE_UNPARSEABLE_EXPLANATION, + [SORT_FIELD_UNKNOWN_EXPLANATION.rule]: SORT_FIELD_UNKNOWN_EXPLANATION, + [SORT_FIELD_UNSORTABLE_EXPLANATION.rule]: SORT_FIELD_UNSORTABLE_EXPLANATION, + [SORT_FIELD_UNPROVISIONED_EXPLANATION.rule]: SORT_FIELD_UNPROVISIONED_EXPLANATION, + [SEARCHABLE_FIELD_UNKNOWN_EXPLANATION.rule]: SEARCHABLE_FIELD_UNKNOWN_EXPLANATION, + [SEARCHABLE_FIELD_UNSEARCHABLE_EXPLANATION.rule]: SEARCHABLE_FIELD_UNSEARCHABLE_EXPLANATION, + [SEARCHABLE_FIELD_UNPROVISIONED_EXPLANATION.rule]: SEARCHABLE_FIELD_UNPROVISIONED_EXPLANATION, + [PREDICATE_PATH_UNRESOLVED_EXPLANATION.rule]: PREDICATE_PATH_UNRESOLVED_EXPLANATION, + [PREDICATE_PATH_UNROOTED_EXPLANATION.rule]: PREDICATE_PATH_UNROOTED_EXPLANATION, + [PREDICATE_RHS_PATH_SHAPED_EXPLANATION.rule]: PREDICATE_RHS_PATH_SHAPED_EXPLANATION, + [ACTION_DISPATCH_CONTRACT_MISMATCH_EXPLANATION.rule]: ACTION_DISPATCH_CONTRACT_MISMATCH_EXPLANATION, + [COMPONENT_TYPE_UNKNOWN_EXPLANATION.rule]: COMPONENT_TYPE_UNKNOWN_EXPLANATION, + [FILTER_PRESET_COMPARAND_EXPLANATION.rule]: FILTER_PRESET_COMPARAND_EXPLANATION, }); /** The explanation for `rule`, or `undefined` when the rule has none. Exact id match. */ diff --git a/packages/lint/src/validate-action-dispatch-contract.test.ts b/packages/lint/src/validate-action-dispatch-contract.test.ts index ab895323bc8..b654e1ab287 100644 --- a/packages/lint/src/validate-action-dispatch-contract.test.ts +++ b/packages/lint/src/validate-action-dispatch-contract.test.ts @@ -9,9 +9,23 @@ import { describe, it, expect } from 'vitest'; import { - validateActionDispatchContract, + validateActionDispatchContract as validateActionDispatchContractUnrecorded, ACTION_DISPATCH_CONTRACT_MISMATCH, } from './validate-action-dispatch-contract.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding is one verdict sentence; the reasoning it used to carry +// is the id's `os explain` entry. Every call below records what it fired, and +// the last cases in this file hold each recorded verdict to one line of at most +// 200 characters — so the pin covers every firing variant this suite exercises, +// not a chosen few. Run the whole file: those cases read what the cases above +// fired. +const fired: Array<{ rule: string; message: string }> = []; +const validateActionDispatchContract: typeof validateActionDispatchContractUnrecorded = (...args) => { + const findings = validateActionDispatchContractUnrecorded(...args); + fired.push(...findings); + return findings; +}; /** * The showcase's own pair, reduced: two actions against ONE endpoint, written @@ -284,3 +298,51 @@ describe('validateActionDispatchContract — absence counts as disagreement (the ).toEqual([]); }); }); + +describe('[#22161] one-line verdicts — action-dispatch-contract-mismatch', () => { + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: the id fired, so the shape assertion below + // cannot pass over an empty record. + expect([...new Set(fired.map((f) => f.rule))]).toEqual([ACTION_DISPATCH_CONTRACT_MISMATCH]); + for (const f of fired) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + it('reads as one sentence in both directions', () => { + const [toPerRow] = validateActionDispatchContract( + stack([AGGREGATE], { bulkActions: ['recalc_selection'] }), + ); + expect(toPerRow!.message).toBe( + 'action "recalc_selection" declares execution: \'aggregate\', wired as execution: \'perRecord\' ' + + '(one call per row: `recordId`, no `_selectedIds`), so its body takes its single-record branch', + ); + const [toAggregate] = validateActionDispatchContract( + stack([PER_RECORD], { + bulkActionDefs: [{ name: 'recalc_estimate', operation: 'custom', execution: 'aggregate' }], + }), + ); + expect(toAggregate!.message).toBe( + 'action "recalc_estimate" declares execution: \'perRecord\', wired as execution: \'aggregate\' ' + + '(one call: `_selectedIds`, no `recordId`), so its body finds no `recordId` and throws', + ); + }); + + it('`os explain` carries what the verdict no longer says', () => { + const entry = explainRule(ACTION_DISPATCH_CONTRACT_MISMATCH); + expect(entry, 'no `os explain action-dispatch-contract-mismatch` entry').toBeDefined(); + const text = entry!.paragraphs.join('\n'); + for (const fact of [ + 'ONCE PER selected row', + '`params._selectedIds`', + '"nothing selected"', + 'ADR-0104', + 'strict params gate', + 'action-bulk-dispatch-contract-undeclared', + '`action-name-undefined`', + ]) { + expect(text, `explanation names ${fact}`).toContain(fact); + } + }); +}); diff --git a/packages/lint/src/validate-action-dispatch-contract.ts b/packages/lint/src/validate-action-dispatch-contract.ts index 3be3e4678b4..536107956de 100644 --- a/packages/lint/src/validate-action-dispatch-contract.ts +++ b/packages/lint/src/validate-action-dispatch-contract.ts @@ -40,9 +40,9 @@ * ## What this rule refuses — and what it deliberately does not * * It refuses a **mismatch**: an action that DECLARES `execution` wired by a - * list view under the other contract. It names the action, the view and BOTH - * contracts, because the fix is a choice between them and a message naming one - * is a message that has already chosen. + * list view under the other contract. The finding names the action, the view + * (its `where`) and BOTH contracts, because the fix is a choice between them and + * a message naming one is a message that has already chosen. * * It says nothing about an action that declares NO `execution`. That is not a * gap left open, it is the ruling's 「创业阶段不渐进」 in the one place it @@ -98,32 +98,22 @@ function contractOf(v: unknown): ActionDispatchContract | undefined { } /** - * One sentence per contract, written so the pair reads as a choice: each says - * how many dispatches happen and which of the two builtin keys arrives. Both - * sentences appear in every finding — the declared one and the wired one — - * because naming only the one that is "wrong" presumes which end the author - * meant to change. + * What each wiring delivers, in the verdict's words: how many calls, and which + * of the two builtin keys arrives. The verdict names BOTH contracts — the + * declared one and the wired one — because naming only the one that is "wrong" + * presumes which end the author meant to change. [#22161] The long account of + * each (the renderer's per-row promotion of a bare-string entry, the aggregate + * def's single dispatch) is the id's `os explain` entry. */ -const CONTRACT_PROSE: Readonly> = { - perRecord: - "`execution: 'perRecord'` (the view's `bulkActions: ['']` bare-string form) — the " - + "renderer promotes the action to a def and dispatches it ONCE PER selected row, each call " - + "carrying that row's `recordId` and NO `_selectedIds`", - aggregate: - "`execution: 'aggregate'` (a `bulkActionDefs` entry naming the action) — ONE dispatch for the " - + 'whole selection, carrying every selected id in `params._selectedIds` and NO `recordId`', +const DELIVERS: Readonly> = { + perRecord: 'one call per row: `recordId`, no `_selectedIds`', + aggregate: 'one call: `_selectedIds`, no `recordId`', }; -/** What the body actually sees when it is written for one contract and wired the other. */ +/** What a body written for the DECLARED contract does with the other wiring's input. */ const MISFIRE: Readonly> = { - perRecord: - 'a body written per-record finds no `recordId` on the single aggregate call and typically ' - + 'throws its own "nothing selected" — which reads in the console like a selection bug rather ' - + 'than a wiring one', - aggregate: - 'a body written for the aggregate call reads `_selectedIds` as `undefined` on every per-row ' - + 'dispatch, falls through to its single-record branch, and reports success for one row out of ' - + 'however many were selected', + perRecord: 'its body finds no `recordId` and throws', + aggregate: 'its body takes its single-record branch', }; /** @@ -205,7 +195,6 @@ export function validateActionDispatchContract(stack: AnyRec): ActionDispatchCon wired: ActionDispatchContract, where: string, path: string, - viewLabel: string, ) => { const declared = declaredBy.get(name); if (declared === undefined || declared === wired) return; @@ -219,12 +208,12 @@ export function validateActionDispatchContract(stack: AnyRec): ActionDispatchCon rule: ACTION_DISPATCH_CONTRACT_MISMATCH, where, path, + // [#22161] One verdict sentence; the view is the finding's `where`, and + // why no runtime door refuses the mismatch (ADR-0104's builtin params) is + // the id's `os explain` entry. message: - `Action "${name}" declares ${CONTRACT_PROSE[declared]}, but ${viewLabel} wires it as ` - + `${CONTRACT_PROSE[wired]}. The two contracts deliver opposite input to the same body: ` - + `${MISFIRE[declared]}. Nothing refuses this at runtime — \`recordId\` and ` - + '`_selectedIds` are both builtin action params (ADR-0104), so the strict params gate ' - + 'admits either bag without a word.', + `action "${name}" declares execution: '${declared}', wired as execution: '${wired}' ` + + `(${DELIVERS[wired]}), so ${MISFIRE[declared]}`, hint: `Pick the contract the body is actually written for and make both ends say it: either ` + `change the action's declaration to \`execution: '${wired}'\` (if the body was written ` @@ -254,7 +243,7 @@ export function validateActionDispatchContract(stack: AnyRec): ActionDispatchCon for (let ai = 0; ai < bare.length; ai++) { const name = strName(bare[ai]); if (!name) continue; - check(name, 'perRecord', `${viewLabel} · bulkActions`, `${path}.bulkActions[${ai}]`, viewLabel); + check(name, 'perRecord', `${viewLabel} · bulkActions`, `${path}.bulkActions[${ai}]`); } // Only an `execution: 'aggregate'` def NAMES an action (#4457): an @@ -275,7 +264,6 @@ export function validateActionDispatchContract(stack: AnyRec): ActionDispatchCon 'aggregate', `${viewLabel} · bulkActionDefs[${di}]`, `${path}.bulkActionDefs[${di}]`, - viewLabel, ); } }; diff --git a/packages/lint/src/validate-component-types.test.ts b/packages/lint/src/validate-component-types.test.ts index 8b2242b6e7f..aa5ae4163a8 100644 --- a/packages/lint/src/validate-component-types.test.ts +++ b/packages/lint/src/validate-component-types.test.ts @@ -13,9 +13,29 @@ import { RETIRED_PAGE_COMPONENT_TYPES, } from '@objectstack/spec/ui'; import { - validateComponentTypes, + validateComponentTypes as validateComponentTypesUnrecorded, + retiredTypeHead, COMPONENT_TYPE_UNKNOWN, } from './validate-component-types.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding is one verdict sentence; the reasoning it used to carry +// is the id's `os explain` entry. Every call below records what it fired, and +// the last cases in this file hold each recorded verdict to one line of at most +// 200 characters — so the pin covers every firing variant this suite exercises +// (every retired type included), not a chosen few. Run the whole file: those +// cases read what the cases above fired. +const fired: Array<{ rule: string; message: string }> = []; +const validateComponentTypes: typeof validateComponentTypesUnrecorded = (...args) => { + const findings = validateComponentTypesUnrecorded(...args); + fired.push(...findings); + return findings; +}; + +/** The verdict the retired arm prints for `type`: the prescription's head, then the parse door. */ +const retiredVerdict = (type: string): string => + `${retiredTypeHead(RETIRED_PAGE_COMPONENT_TYPES.get(type)!)}, so the parse refuses this node by name ` + + '(`os validate` prints the prescription)'; const page = (components: unknown[], name = 'p1', extra: Record = {}) => ({ pages: [{ name, regions: [{ name: 'main', components }], ...extra }], @@ -145,10 +165,10 @@ describe('leaves the declared vocabulary and the open arm alone', () => { * exactly what made this rule walk past it in silence while * `PageComponentSchema.type` refuses the same name at the parse. Measured on * both sides of the #17592 review: `element:filter` → no finding, before and - * after. This suite pins the report, and pins it BYTE-EQUAL to - * `RETIRED_PAGE_COMPONENT_TYPES` — the rule relays the spec's prescription, it - * does not author a second copy, so drift between the three doors is not - * expressible. + * after. This suite pins the report, and pins it to + * `RETIRED_PAGE_COMPONENT_TYPES` — the rule relays the spec's prescription + * ([#22161] quoted to its head, a prefix of the map's text), it does not author + * a second copy, so drift between the three doors is not expressible. * * Driven off the map rather than a restated list: a type retired tomorrow * arrives here covered on the day it lands. @@ -168,8 +188,12 @@ describe('reports an EXACT retired component type, relaying the spec prescriptio expect(f.severity).toBe('error'); expect(f.path).toBe('pages[0].regions[0].components[0].type'); expect(f.where).toBe(`page "p1" · ${type}`); - // Verbatim, not "contains": the relay is the contract. - expect(f.message).toBe(RETIRED_PAGE_COMPONENT_TYPES.get(type)); + // The relay is the contract: [#22161] the map's own text, cut to its head — + // a PREFIX of the prescription, never a paraphrase of it. + const prescription = RETIRED_PAGE_COMPONENT_TYPES.get(type)!; + expect(f.message).toBe(retiredVerdict(type)); + expect(prescription.startsWith(retiredTypeHead(prescription))).toBe(true); + expect(retiredTypeHead(prescription)).toContain(`\`${type}\``); expect(f.hint).toContain(type); }); @@ -184,7 +208,7 @@ describe('reports an EXACT retired component type, relaying the spec prescriptio expect(hasReservedComponentNamespace('user:profile')).toBe(false); const findings = validateComponentTypes(page([{ type: 'user:profile' }])); expect(findings).toHaveLength(1); - expect(findings[0].message).toBe(RETIRED_PAGE_COMPONENT_TYPES.get('user:profile')); + expect(findings[0].message).toBe(retiredVerdict('user:profile')); }); it('reaches a retired type nested under a tab item, at its own path', () => { @@ -282,3 +306,55 @@ describe('retired types are never proposed as typo suggestions (#15110)', () => .toContain('record:details'); }); }); + +describe('[#22161] one-line verdicts — component-type-unknown', () => { + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: the id fired, and the retired arm fired for + // every retired type, so the shape assertion below cannot pass over a + // record that skipped an arm. + expect([...new Set(fired.map((f) => f.rule))]).toEqual([COMPONENT_TYPE_UNKNOWN]); + for (const type of RETIRED_PAGE_COMPONENT_TYPES.keys()) { + expect(fired.some((f) => f.message === retiredVerdict(type)), type).toBe(true); + } + for (const f of fired) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + it('the namespace verdict reads as one sentence', () => { + const [f] = validateComponentTypes(page([{ type: 'global:serch' }])); + expect(f.message).toBe( + "`global:serch` is not a declared type of the reserved `global:` namespace, so the page draws a " + + "placeholder in its place. Did you mean 'global:search'?", + ); + }); + + it('retiredTypeHead cuts at the first dash, else the first sentence, and never rewrites', () => { + expect(retiredTypeHead('`a:b` is gone — use `a:c` instead.')).toBe('`a:b` is gone'); + expect(retiredTypeHead('`a:b` is gone. Use `a:c` instead.')).toBe('`a:b` is gone'); + expect(retiredTypeHead('`a:b` is gone.')).toBe('`a:b` is gone'); + }); + + it('`os explain` carries what the verdict no longer says', () => { + const entry = explainRule(COMPONENT_TYPE_UNKNOWN); + expect(entry, 'no `os explain component-type-unknown` entry').toBeDefined(); + const text = entry!.paragraphs.join('\n'); + for (const fact of [ + 'an arbitrary string parses', + 'component-placeholder scaffold', + '`ComponentPropsMap`', + 'measured clean', + '`RETIRED_PAGE_COMPONENT_TYPES`', + '`user:profile`', + '`os validate`', + ]) { + expect(text, `explanation names ${fact}`).toContain(fact); + } + // One copy of the guidance: the explanation names where the prescription + // lives and never restates one. + for (const prescription of RETIRED_PAGE_COMPONENT_TYPES.values()) { + expect(text).not.toContain(prescription.slice(0, 80)); + } + }); +}); diff --git a/packages/lint/src/validate-component-types.ts b/packages/lint/src/validate-component-types.ts index 93bc0e71fc8..36c2e25147c 100644 --- a/packages/lint/src/validate-component-types.ts +++ b/packages/lint/src/validate-component-types.ts @@ -54,11 +54,13 @@ * at the parse. Lint saying "the stack is fine" about a name the parser then * refuses is the declared-not-enforced shape inverted: the author's EARLIEST * feedback channel was the one that stayed quiet. So the arm below reports it, - * and the reported text is `RETIRED_PAGE_COMPONENT_TYPES`'s own entry — - * relayed verbatim, never re-authored, so this file cannot drift from the enum - * error map and the kept props row that carry the same string. The pin is byte - * equality against the map (`validate-component-types.test.ts`), which is also - * what makes a member retired tomorrow arrive here already covered. + * and the reported text is `RETIRED_PAGE_COMPONENT_TYPES`'s own entry, quoted + * to its head ({@link retiredTypeHead}, [#22161]: one verdict line) — cut, + * never re-authored, so this file cannot drift from the enum error map and the + * kept props row that carry the whole string. The pin is that the verdict + * opens with a prefix of the map's entry (`validate-component-types.test.ts`), + * which is also what makes a member retired tomorrow arrive here already + * covered. * * ⛔ `isKnownComponentType` is NOT the seam for this. Flipping it would MOVE * the refusal out of the props door instead of ADDING a report here, and would @@ -122,6 +124,22 @@ function strName(v: unknown): string | undefined { return typeof v === 'string' && v.length > 0 ? v : undefined; } +/** + * [#22161] A retired type's prescription, cut to its HEAD — the clause before + * its first ` — ` (`\`element:filter\` was removed in @objectstack/spec 17 + * (ADR-0049)`, `\`user:profile\` is not a page-placeable element`), else its + * first sentence, else the whole entry. It cuts and never rewrites, so the + * verdict is still a prefix of `RETIRED_PAGE_COMPONENT_TYPES`' own text and no + * second copy of the guidance exists; the whole prescription is what the parse + * door (`PageComponentSchema.type`) answers the same name with. + */ +export function retiredTypeHead(prescription: string): string { + const dash = prescription.indexOf(' — '); + if (dash > 0) return prescription.slice(0, dash); + const stop = prescription.indexOf('. '); + return stop > 0 ? prescription.slice(0, stop) : prescription.replace(/\.$/, ''); +} + export function validateComponentTypes(stack: AnyRec): ComponentTypeFinding[] { const findings: ComponentTypeFinding[] = []; if (!isRec(stack)) return findings; @@ -138,7 +156,7 @@ export function validateComponentTypes(stack: AnyRec): ComponentTypeFinding[] { // EXACT-name arm, judged before the namespace guard (header: `user:` // stopped being a reserved namespace when its only member was retired). - // The message IS the map's entry — relayed, never re-authored. + // The message QUOTES the map's entry — cut to its head, never re-authored. const prescription = RETIRED_PAGE_COMPONENT_TYPES.get(type); if (prescription !== undefined) { findings.push({ @@ -146,7 +164,7 @@ export function validateComponentTypes(stack: AnyRec): ComponentTypeFinding[] { rule: COMPONENT_TYPE_UNKNOWN, where: `page "${pageName}" · ${type}`, path: `${path}.type`, - message: prescription, + message: `${retiredTypeHead(prescription)}, so the parse refuses this node by name (\`os validate\` prints the prescription)`, hint: `Apply the prescription above: \`${type}\` is a retired component type, refused by ` + 'name at the parse door (`PageComponentSchema.type`), so this page cannot validate or ' + @@ -165,11 +183,11 @@ export function validateComponentTypes(stack: AnyRec): ComponentTypeFinding[] { rule: COMPONENT_TYPE_UNKNOWN, where: `page "${pageName}" · ${type}`, path: `${path}.type`, + // [#22161] One verdict sentence; why a reserved namespace is closed + // while the open arm stays open is the id's `os explain` entry. message: - `\`${type}\` is not a component type the platform vocabulary declares. Its namespace ` + - `(\`${type.slice(0, type.indexOf(':'))}:\`) belongs to the standard component vocabulary, so nothing ` + - 'will ever render this node — the page would validate, publish, and then draw a placeholder ' + - 'scaffold in front of the end user.' + + `\`${type}\` is not a declared type of the reserved \`${type.slice(0, type.indexOf(':'))}:\` ` + + 'namespace, so the page draws a placeholder in its place.' + (suggestion ? ` ${suggestion}` : ''), hint: suggestions.length ? `Rename \`${type}\` → \`${suggestions[0]}\`.` diff --git a/packages/lint/src/validate-predicate-path-refs.test.ts b/packages/lint/src/validate-predicate-path-refs.test.ts index 2723368fc0d..5945b81148e 100644 --- a/packages/lint/src/validate-predicate-path-refs.test.ts +++ b/packages/lint/src/validate-predicate-path-refs.test.ts @@ -20,12 +20,13 @@ import { METADATA_FORM_REGISTRY } from '@objectstack/spec/system'; import { getMetadataTypeSchema } from '@objectstack/spec/kernel'; import { - validatePredicatePathRefs, + validatePredicatePathRefs as validatePredicatePathRefsUnrecorded, PREDICATE_PATH_UNRESOLVED, PREDICATE_PATH_UNROOTED, PREDICATE_RHS_PATH_SHAPED, } from './validate-predicate-path-refs.js'; import { AUTHORING_RULES } from './authoring-rules.js'; +import { explainRule } from './rule-explanations.js'; // The published barrel is loaded HERE, at module top, and not by a dynamic // import inside the case that reads the id off it. That import loads the whole // `@objectstack/lint` index graph, and inside a case the load is charged to @@ -57,6 +58,23 @@ const DemoSchema = z.object({ tags: z.array(z.string()).optional(), }); +// [#22161] Each finding of the three ids is one verdict sentence; the reasoning +// it used to carry is the id's `os explain` entry. Every call below records +// what it fired, and the last cases in this file hold each recorded verdict to +// one line of at most 200 characters — so the pin covers every firing variant +// this suite exercises (the shipped-corpus block included), not a chosen few. +// Run the whole file: those cases read what the cases above fired. +const PREDICATE_IDS: readonly string[] = [PREDICATE_PATH_UNRESOLVED, PREDICATE_PATH_UNROOTED, PREDICATE_RHS_PATH_SHAPED]; +const fired: Array<{ rule: string; message: string }> = []; +const validatePredicatePathRefs: typeof validatePredicatePathRefsUnrecorded = (...args) => { + const findings = validatePredicatePathRefsUnrecorded(...args); + fired.push(...findings); + return findings; +}; + +/** The `os explain` text of `rule`, one string. */ +const explanationOf = (rule: string): string => explainRule(rule)?.paragraphs.join('\n') ?? ''; + const resolveSchema = (schemaId: string) => (schemaId === 'demo' ? DemoSchema : undefined); const run = (stack: Record) => validatePredicatePathRefs(stack, { resolveSchema }); @@ -820,3 +838,45 @@ describe('#7010 corpus — shipped METADATA_FORM_REGISTRY', () => { expect(findings[0].message).toContain('`type`'); }); }); + +describe('[#22161] one-line verdicts — the three predicate ids', () => { + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: each id fired at least once, so the shape + // assertion below cannot pass over an empty record. + expect([...new Set(fired.map((f) => f.rule))].sort()).toEqual([...PREDICATE_IDS].sort()); + for (const f of fired) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + it('the two resolution verdicts end with one consequence clause', () => { + const [unresolved] = run(form([{ fields: [{ field: 'x', visibleWhen: "data.tpye == 'formula'" }] }])); + const [unrooted] = run(form([{ fields: [{ field: 'x', visibleWhen: "type == 'formula'" }] }])); + expect(unresolved.message).toBe( + 'predicate references `data.tpye`, but `tpye` is not a key of `data`, so the predicate never ' + + 'evaluates and the console fails open: the element always renders', + ); + expect(unrooted.message).toBe( + 'predicate names `type` bare, but it is a key of the edited schema with its `data.` root dropped, ' + + 'so the predicate never evaluates and the console fails open: the element always renders', + ); + }); + + // What each verdict stopped saying, which `os explain RULE_ID` now prints. + const MOVED: Record = { + [PREDICATE_PATH_UNRESOLVED]: ['fails OPEN', 'pixel-identical', '`getMetadataTypeSchema(schemaId)`', 'record map', '`visibility-predicate-syntax`'], + [PREDICATE_PATH_UNROOTED]: ['never flattens', 'fails OPEN', '`visibility-bare-identifier`', 'overload', 'comprehension-macro'], + [PREDICATE_RHS_PATH_SHAPED]: ['literal parser', 'hides the element on every row', '`path == \'literal\'`', 'real CEL evaluator', 'stand down'], + }; + + it('covers exactly the three ids', () => { + expect(Object.keys(MOVED).sort()).toEqual([...PREDICATE_IDS].sort()); + }); + + it.each([...PREDICATE_IDS])('`os explain %s` carries what its verdict no longer says', (rule) => { + expect(explainRule(rule), `no \`os explain ${rule}\` entry`).toBeDefined(); + const text = explanationOf(rule); + for (const fact of MOVED[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + }); +}); diff --git a/packages/lint/src/validate-predicate-path-refs.ts b/packages/lint/src/validate-predicate-path-refs.ts index b5c4e314b0f..7fae4b721ab 100644 --- a/packages/lint/src/validate-predicate-path-refs.ts +++ b/packages/lint/src/validate-predicate-path-refs.ts @@ -312,6 +312,14 @@ const PREDICATE_KEYS = ['visibleWhen', 'visibleOn'] as const; /** The binding root this rule resolves. Metadata-editing forms only — see the module note. */ const ROOT = 'data'; +/** + * [#22161] The consequence clause both path-RESOLUTION verdicts end with, one + * wording for the two: a reference that resolves to nothing never evaluates, + * and the console fails open. Why it fails open, and what "open" looks like, + * is the ids' `os explain` entries. + */ +const FAILS_OPEN = 'so the predicate never evaluates and the console fails open: the element always renders'; + // ── Zod introspection ─────────────────────────────────────────────── // // Reads `.def` directly rather than importing zod's internals, and tolerates @@ -646,11 +654,8 @@ function checkPredicate( where, path, message: - `predicate references \`${full}\`, which the target schema does not declare — ` - + `\`${segment}\` is not a key of \`${container}\`. The reference resolves to nothing, ` - + `so the predicate can never evaluate and the console falls OPEN: the element renders ` - + `unconditionally and looks exactly like one carrying no predicate at all (failing open is the ` - + `console's settled behaviour).`, + `predicate references \`${full}\`, but \`${segment}\` is not a key of \`${container}\`, ` + + FAILS_OPEN, hint: `${formatSuggestion(findClosestMatches(segment, step.declared)) || `\`${container}\` declares: ${step.declared.slice(0, 12).sort().join(', ')}`}` @@ -688,11 +693,8 @@ function checkPredicate( where, path, message: - `predicate references \`${id}\` as a bare identifier, but \`${id}\` is a key of the schema ` - + `this form edits — the binding root was dropped. Values are bound under \`${ROOT}\` and are ` - + `never flattened to top level, so \`${id}\` resolves to nothing, the predicate can never ` - + `evaluate and the console falls OPEN: the element renders unconditionally and looks exactly ` - + `like one carrying no predicate at all (failing open is the console's settled behaviour).`, + `predicate names \`${id}\` bare, but it is a key of the edited schema with its \`${ROOT}.\` ` + + `root dropped, ${FAILS_OPEN}`, hint: `Write \`${ROOT}.${id}\` instead of \`${id}\`. A metadata-editing form binds the row under ` + `edit as \`${ROOT}\` at every depth — inside a repeater \`${ROOT}\` is the ROW, but it is ` @@ -722,23 +724,14 @@ function checkPredicate( rule: PREDICATE_RHS_PATH_SHAPED, where, path, + // [#22161] One verdict sentence per arm; the evaluator's left/right + // split, the subset and the two readings are the id's `os explain` entry. message: dotted - ? `predicate compares against \`${text}\` on the RIGHT of \`${op}\`, which is a path but is ` - + `not evaluated as one. A metadata-editing form resolves paths on the LEFT of \`${op}\` ` - + `only; the right-hand side goes to the literal parser, so \`${text}\` is compared as the ` - + `literal string "${text}". The verdict therefore does not depend on the right-hand path ` - + `at all: \`a == ${text}\` is FALSE even when both sides hold the same value, and ` - + `\`a != ${text}\` is correspondingly TRUE. An \`==\` written this way hides the element ` - + `on every row: the form evaluator keeps its right-hand side a literal by design, and says ` - + `why only in a development build.` - : `predicate compares against the unquoted word \`${text}\` on the RIGHT of \`${op}\`. The ` - + `right-hand side of \`${op}\` is a literal, never a reference, so this is read as the ` - + `literal string "${text}" — which is probably what you meant, and is why it appears to ` - + `work. It is outside the declared subset all the same (\`path == 'literal'\`), and it ` - + `stops working when this surface moves to the real CEL evaluator, where a bare ` - + `\`${text}\` resolves to nothing. The token also reads as a \`${ROOT}.\` ` - + `root someone dropped, so this one finding carries BOTH readings: which one you meant ` - + `is the thing no linter can know, and it changes the fix.`, + ? `predicate compares against \`${text}\` on the RIGHT of \`${op}\`, which this form reads ` + + `as the literal string "${text}", so the result never depends on that path's value` + : `predicate compares against the unquoted word \`${text}\` on the RIGHT of \`${op}\`, ` + + `which reads as a literal today but is outside the declared subset and breaks under ` + + `the real CEL evaluator`, hint: dotted ? `Two sanctioned spellings. (1) If you meant the TEXT, quote it: \`${op} '${text}'\`. ` + `(2) If you meant the PATH, restructure so the path is on the LEFT and a literal is on ` diff --git a/packages/lint/src/validate-preset-comparands.test.ts b/packages/lint/src/validate-preset-comparands.test.ts index f3cf9555f5a..b9e4b6458f1 100644 --- a/packages/lint/src/validate-preset-comparands.test.ts +++ b/packages/lint/src/validate-preset-comparands.test.ts @@ -1,7 +1,30 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { validatePresetComparands, FILTER_PRESET_COMPARAND } from './validate-preset-comparands.js'; +import { + DATE_RANGE_PRESETS, + DATE_RANGE_PRESET_MACRO_WINDOWS, + bareDateRangePresetComparandMessage, +} from '@objectstack/spec/data'; +import { + validatePresetComparands as validatePresetComparandsUnrecorded, + presetComparandVerdict, + FILTER_PRESET_COMPARAND, +} from './validate-preset-comparands.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding is one verdict sentence; the reasoning it used to carry +// is the id's `os explain` entry. Every call below records what it fired, and +// the last cases in this file hold each recorded verdict to one line of at most +// 200 characters — so the pin covers every firing variant this suite exercises, +// not a chosen few. Run the whole file: those cases read what the cases above +// fired. +const fired: Array<{ rule: string; message: string }> = []; +const validatePresetComparands: typeof validatePresetComparandsUnrecorded = (...args) => { + const findings = validatePresetComparandsUnrecorded(...args); + fired.push(...findings); + return findings; +}; describe('validatePresetComparands (#8793 — the ruled C half of #8690)', () => { it('returns nothing for an empty / absent stack', () => { @@ -246,8 +269,8 @@ describe('validatePresetComparands — arm 2, the FIELD-TYPED equality / members expect(f.hint).toContain('{date-macro}'); } // The implicit-equality position is reported under the operator it lowers to. - expect(findings[0].message).toContain('As a bare "$eq" comparand'); - expect(findings[2].message).toContain('As a bare "$in" comparand'); + expect(findings[0].message).toContain('as a bare "$eq" comparand'); + expect(findings[2].message).toContain('as a bare "$in" comparand'); }); it('judges $ne / $nin, every member of a list, and a declared datetime field the same way', () => { @@ -456,17 +479,11 @@ describe('validatePresetComparands — arm 2, the FIELD-TYPED equality / members expect(f.where).toBe('dashboard "sales" · widget "w"'); expect(f.path).toBe('dashboards[0].widgets[0].filter.created_at'); // Verbatim. The author acts on this text; a re-wording is a decision, not a - // refactor, and it should turn this red. + // refactor, and it should turn this red. [#22161] One verdict sentence: the + // rest of the shared refusal is the id's `os explain` entry. expect(f.message).toBe( - '"last_30_days" is a dashboard date-range PRESET name, not a filter value. ' - + 'It is only understood by the dashboard date-filter positions ' - + "(dateRange.defaultRange, a date global filter's defaultValue), where the " - + 'console lowers it to {date-macro} bounds before querying. As a bare "$eq" ' - + 'comparand nothing resolves it: a declared datetime/date field refuses the ' - + 'query at the engine (INVALID_FILTER / 400), and any other column compares ' - + "the literal string. Write the date-macro window instead — e.g. { $gte: " - + "'{30_days_ago}' } — or an ISO date such as \"2026-01-15\". Refused at " - + 'authoring time so the error surfaces where the filter is written.', + '"last_30_days" is a dashboard date-range PRESET name, not a filter value as a bare "$eq" ' + + "comparand; its window is { $gte: '{30_days_ago}' }", ); expect(f.hint).toBe( 'Presets belong to the dashboard date-filter bar (dateRange.defaultRange, a ' @@ -690,3 +707,59 @@ describe('validatePresetComparands — page filterBy and lookup-field lookupFilt })).toEqual([]); }); }); + +describe('[#22161] one-line verdicts — filter-preset-comparand', () => { + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: the id fired, so the shape assertion below + // cannot pass over an empty record. + expect([...new Set(fired.map((f) => f.rule))]).toEqual([FILTER_PRESET_COMPARAND]); + for (const f of fired) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + it('opens with the shared refusal\'s own first sentence, and names the window that refusal names', () => { + // One wording across the two doors (the schema door prints the whole + // refusal): the verdict's head is a PREFIX of it, cut and never rewritten, + // and its window is the very text the refusal's prescription quotes. + for (const preset of DATE_RANGE_PRESETS) { + const refusal = bareDateRangePresetComparandMessage(preset, '$gte'); + const verdict = presetComparandVerdict(preset, '$gte'); + const head = verdict.slice(0, verdict.indexOf(' as a bare')); + expect(refusal.startsWith(`${head}. `), preset).toBe(true); + const window = verdict.slice(verdict.indexOf('its window is ') + 'its window is '.length); + expect(refusal, preset).toContain(`e.g. ${window} — or an ISO date`); + const [start] = DATE_RANGE_PRESET_MACRO_WINDOWS[preset]; + expect(window, preset).toContain(start); + } + }); + + it('stays within the bound for every preset under the longest operator spelling it reports', () => { + // `greater_than_or_equal` is the longest view-rule spelling `normalizeFilterOperator` + // folds onto an ordering operator; `$between` carries the longest window. + for (const preset of DATE_RANGE_PRESETS) { + for (const op of ['greater_than_or_equal', 'less_than_or_equal', '$between', 'not_equals']) { + const verdict = presetComparandVerdict(preset, op); + expect(verdict.length, verdict).toBeLessThanOrEqual(200); + } + } + }); + + it('`os explain` carries what the verdict no longer says', () => { + const entry = explainRule(FILTER_PRESET_COMPARAND); + expect(entry, 'no `os explain filter-preset-comparand` entry').toBeDefined(); + const text = entry!.paragraphs.join('\n'); + for (const fact of [ + 'dateRange.defaultRange', + '`{date-macro}` bounds', + '`INVALID_FILTER`, 400', + 'compares the literal string', + 'picklist', + 'lowers to, `$eq`', + 'ISO date', + ]) { + expect(text, `explanation names ${fact}`).toContain(fact); + } + }); +}); diff --git a/packages/lint/src/validate-preset-comparands.ts b/packages/lint/src/validate-preset-comparands.ts index c4a57bd83ca..4d06dce7f77 100644 --- a/packages/lint/src/validate-preset-comparands.ts +++ b/packages/lint/src/validate-preset-comparands.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { + DATE_RANGE_PRESET_MACRO_WINDOWS, VALID_AST_OPERATORS, bareDateRangePresetComparandMessage, canonicalAstOperator, @@ -71,9 +72,11 @@ import { indexObjectGraph, recordsOf, resolveFieldPath, type ObjectGraph } from * `date` / `datetime` field refuses one of the 13 declared preset names in * EVERY comparand position — bare (implicit equality), `$eq` / `$ne`, `$in` / * `$nin` and their view-rule and triple spellings, alongside the ordering - * positions already judged.* Same message, same prescription (the #5240 - * convention): the window the preset already means is exactly what an author - * writing `close_date == 'last_30_days'` intended. + * positions already judged.* Same verdict, same prescription (the #5240 + * convention; [#22161] both arms print {@link presetComparandVerdict}, which + * opens with the schema door's own first sentence): the window the preset + * already means is exactly what an author writing + * `close_date == 'last_30_days'` intended. * * Measured first (ruling item 3), as real queries on a declared `date` field * (`close_date`) and a declared `datetime` sibling, on two real drivers — @@ -279,6 +282,26 @@ function literalObjectName(v: unknown): string | undefined { return s && !s.includes('{') ? s : undefined; } +/** + * [#22161] The one-line verdict. It opens with the shared refusal's own first + * sentence (`bareDateRangePresetComparandMessage`, cut and never rewritten — the + * sentence both doors open with), then names the operator the comparand sits + * under and the window the preset means, which is what the `fix:` line points + * at. The window is read from the table the shared refusal quotes + * (`DATE_RANGE_PRESET_MACRO_WINDOWS`), in the refusal's spelling — the tests + * hold the two equal. The rest of the shared refusal (where a preset IS + * understood, what each layer does with a bare one) is the id's `os explain` + * entry. + */ +export function presetComparandVerdict(preset: DateRangePreset, operator: string): string { + const refusal = bareDateRangePresetComparandMessage(preset, operator); + const stop = refusal.indexOf('. '); + const head = stop > 0 ? refusal.slice(0, stop) : refusal; + const [start, end] = DATE_RANGE_PRESET_MACRO_WINDOWS[preset]; + const window = end === null ? `{ $gte: '${start}' }` : `{ $between: ['${start}', '${end}'] }`; + return `${head} as a bare "${operator}" comparand; its window is ${window}`; +} + function finding( where: string, path: string, @@ -290,7 +313,7 @@ function finding( rule: FILTER_PRESET_COMPARAND, where, path, - message: bareDateRangePresetComparandMessage(preset, operator), + message: presetComparandVerdict(preset, operator), hint: 'Presets belong to the dashboard date-filter bar (dateRange.defaultRange, a date ' + "global filter's defaultValue). In a filter comparand, write the {date-macro} " diff --git a/packages/lint/src/validate-react-page-props.test.ts b/packages/lint/src/validate-react-page-props.test.ts index 73ddfc69706..c9d61c26eb2 100644 --- a/packages/lint/src/validate-react-page-props.test.ts +++ b/packages/lint/src/validate-react-page-props.test.ts @@ -532,8 +532,11 @@ describe('validateReactPageProps — searchableFields (#4329)', () => expect(f[0].rule).toBe(SEARCHABLE_FIELD_UNPROVISIONED); expect(f[0].severity).toBe('warning'); expect(f[0].path).toBe('pages[0].source › searchableFields[1]'); - expect(f[0].message).toContain('external object (ADR-0015)'); - expect(f[0].message).toContain('$searchFields'); + // [#22161] The shared one-clause anchor verdict and the narrowing's + // consequence; ADR-0015 and the `$searchFields` echo are the id's + // `os explain` entry. + expect(f[0].message).toContain("injected column with no storage on external object 'ext_account'"); + expect(f[0].message).toContain("the list's toolbar search scans an empty column"); }); it('flags a dotted path — search cannot resolve the traversal', () => { diff --git a/packages/lint/src/validate-searchable-fields.test.ts b/packages/lint/src/validate-searchable-fields.test.ts index f4c92259e14..9fdde3429d8 100644 --- a/packages/lint/src/validate-searchable-fields.test.ts +++ b/packages/lint/src/validate-searchable-fields.test.ts @@ -1,16 +1,48 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { resolveSearchFields } from '@objectstack/spec/data'; import { - validateSearchableFields, - checkSearchableFieldList, + resolveSearchFields, + SEARCHABLE_ENUM_TYPES, + SEARCHABLE_TEXTUAL_TYPES, + SEARCH_VIRTUAL_TYPES, +} from '@objectstack/spec/data'; +import { + validateSearchableFields as validateSearchableFieldsUnrecorded, + checkSearchableFieldList as checkSearchableFieldListUnrecorded, indexObjectSearchTargets, SEARCHABLE_FIELD_UNKNOWN, SEARCHABLE_FIELD_UNSEARCHABLE, SEARCHABLE_FIELD_UNPROVISIONED, } from './validate-searchable-fields.js'; import { indexUnprovisionedAnchors } from './system-fields.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding of the three ids is one verdict sentence; the reasoning +// it used to carry is the id's `os explain` entry. Every call below records +// what it fired, and the last cases in this file hold each recorded verdict to +// one line of at most 200 characters — so the pin covers every firing variant +// this suite exercises, not a chosen few. Run the whole file: those cases read +// what the cases above fired. +const SEARCH_IDS: readonly string[] = [ + SEARCHABLE_FIELD_UNKNOWN, + SEARCHABLE_FIELD_UNSEARCHABLE, + SEARCHABLE_FIELD_UNPROVISIONED, +]; +const fired: Array<{ rule: string; message: string }> = []; +const validateSearchableFields: typeof validateSearchableFieldsUnrecorded = (...args) => { + const findings = validateSearchableFieldsUnrecorded(...args); + fired.push(...findings); + return findings; +}; +const checkSearchableFieldList: typeof checkSearchableFieldListUnrecorded = (...args) => { + const findings = checkSearchableFieldListUnrecorded(...args); + fired.push(...findings); + return findings; +}; + +/** The `os explain` text of `rule`, one string. */ +const explanationOf = (rule: string): string => explainRule(rule)?.paragraphs.join('\n') ?? ''; /** * The drift this rule exists for: `email` was renamed to `billing_email` and @@ -633,8 +665,9 @@ describe('validateSearchableFields — list views that narrow the set', () => { /** * [#6675] Skill-parity — `skills/objectstack-ui/SKILL.md` › "Toolbar Search - * (`searchableFields`, ADR-0061)" quotes this rule's two diagnostics verbatim - * and states three boundaries as fact. The skill ships to third parties via + * (`searchableFields`, ADR-0061)" names this rule's two diagnostics by id + * (`rules/list-views.md`; [#22161] it quotes neither verdict's text) and states + * three boundaries as fact. The skill ships to third parties via * `npx skills add`, so a reader who follows it is following THIS code; if the * wording or a verdict moves and nobody re-reads the skill, the published text * teaches a rule the platform no longer has. @@ -740,9 +773,7 @@ describe('validateSearchableFields — objectstack-ui SKILL.md parity (#6675)', expect(findings[0].severity).toBe('error'); expect(findings[0].message).toBe( 'list-view searchableFields entry "account_id.name" is not a field on object ' - + '"support_case". The declaration is stale: searching it can never match, and the ' - + 'engine silently drops it — leaving a narrower search than declared, or the ' - + 'auto-default set once every entry is dropped.', + + '"support_case", so the engine drops it from the search.', ); }); @@ -753,11 +784,8 @@ describe('validateSearchableFields — objectstack-ui SKILL.md parity (#6675)', expect(findings[0].rule).toBe(SEARCHABLE_FIELD_UNSEARCHABLE); expect(findings[0].severity).toBe('error'); expect(findings[0].message).toBe( - 'list-view searchableFields entry "status" is outside object "support_case"\'s ' - + 'declared searchableFields (subject, case_number, description) — the set \'search\' ' - + 'scans. Clients echo this declaration verbatim as the \'$searchFields\' override, ' - + 'and the runtime refuses an entry outside the allowed set: every toolbar search on ' - + 'this list returns 400 INVALID_FIELD.', + 'list-view searchableFields entry "status" is outside the declared searchableFields ' + + '(subject, case_number, description), so list search answers 400 INVALID_FIELD', ); }); @@ -941,7 +969,9 @@ describe('[#8404] validateSearchableFields — a declared unprovisioned anchor', expect(warned[0].severity).toBe('warning'); expect(warned[0].path).toBe('objects[0].searchableFields[1]'); expect(warned[0].message).toContain('owner_id'); - expect(warned[0].message).toContain('external object (ADR-0015)'); + // [#22161] The shared one-clause anchor verdict; its long form (ADR-0015 + // federation) is the id's `os explain` entry. + expect(warned[0].message).toContain("injected column with no storage on external object 'ext_customer'"); // The canonical consequence, not the narrowing one. expect(warned[0].message).toContain('narrower than it declares'); expect(warned[0].hint).toContain('columnMap'); @@ -986,8 +1016,10 @@ describe('[#8404] validateSearchableFields — a declared unprovisioned anchor', 'objects[0].searchableFields[1]', 'objects[0].listViews.all.searchableFields[0]', ]); - expect(warned[1].message).toContain('$searchFields'); - expect(warned[1].message).toContain('empty on every'); + // [#22161] Why a view's narrowing reaches the runtime (the `$searchFields` + // echo) is the id's `os explain` entry; the verdict keeps the consequence. + expect(warned[1].message).toContain("the list's toolbar search scans an empty column"); + expect(warned[0].message).not.toContain('toolbar'); // The stub keeps the anchor inside the resolved allow-list, so the #4830 // admissibility rule stays silent and this is the ONLY finding on it. expect(findings.filter((f) => f.rule === SEARCHABLE_FIELD_UNSEARCHABLE)).toHaveLength(0); @@ -1026,3 +1058,61 @@ describe('[#8404] validateSearchableFields — a declared unprovisioned anchor', expect(withIndex[0].path).toBe('p[1]'); }); }); + +describe('[#22161] one-line verdicts — the three searchable ids', () => { + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: each id fired at least once, so the shape + // assertion below cannot pass over an empty record. + expect([...new Set(fired.map((f) => f.rule))].sort()).toEqual([...SEARCH_IDS].sort()); + for (const f of fired) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + it('a declared set longer than three names is quoted to three, with the rest counted', () => { + const findings = validateSearchableFields({ + objects: [ + { + name: 'crm_account', + fields: { + name: { type: 'text' }, code: { type: 'text' }, city: { type: 'text' }, + email: { type: 'email' }, phone: { type: 'phone' }, notes: { type: 'textarea' }, + }, + searchableFields: ['name', 'code', 'city', 'email', 'phone'], + listViews: { all: { type: 'grid', searchableFields: ['notes'] } }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('(name, code, city (and 2 more))'); + // The one fact the verdict drops, the hint still carries: where the set lives. + expect(findings[0].hint).toContain('crm_account.searchableFields'); + }); + + // What each verdict stopped saying, which `os explain RULE_ID` now prints. + const MOVED: Record = { + [SEARCHABLE_FIELD_UNKNOWN]: ['`resolveSearchFields`', 'narrower set', 'AUTO-DEFAULT', '`$searchFields`', 'exact string'], + [SEARCHABLE_FIELD_UNSEARCHABLE]: ['`$searchFields`', '`resolveSearchFieldResolution`', 'AUTO-DEFAULT', 'driver-sql', 'existence-only'], + [SEARCHABLE_FIELD_UNPROVISIONED]: ['ADR-0015', '`$searchFields`', 'reads as search coverage', 'scans nothing at all', 'warning'], + }; + + it('covers exactly the three ids', () => { + expect(Object.keys(MOVED).sort()).toEqual([...SEARCH_IDS].sort()); + }); + + it.each([...SEARCH_IDS])('`os explain %s` carries what its verdict no longer says', (rule) => { + expect(explainRule(rule), `no \`os explain ${rule}\` entry`).toBeDefined(); + const text = explanationOf(rule); + for (const fact of MOVED[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + }); + + it('the explanation\'s lists match the spec constants it writes out', () => { + // The explanation module imports nothing, so the auto-default type list and + // the virtual-type claim it states are held to their spec sources here. + const types = [...SEARCHABLE_TEXTUAL_TYPES, ...SEARCHABLE_ENUM_TYPES].join(' / '); + expect(explanationOf(SEARCHABLE_FIELD_UNSEARCHABLE)).toContain(`(${types})`); + expect([...SEARCH_VIRTUAL_TYPES]).toEqual(['formula']); + expect(explanationOf(SEARCHABLE_FIELD_UNSEARCHABLE)).toContain('`formula` alone'); + }); +}); diff --git a/packages/lint/src/validate-searchable-fields.ts b/packages/lint/src/validate-searchable-fields.ts index 771c57d6d82..3dde4e0d7e2 100644 --- a/packages/lint/src/validate-searchable-fields.ts +++ b/packages/lint/src/validate-searchable-fields.ts @@ -122,8 +122,6 @@ import { resolveSearchFieldResolution, isVirtualSearchField, - SEARCHABLE_TEXTUAL_TYPES, - SEARCHABLE_ENUM_TYPES, SEARCH_AUTO_EXCLUDED_FIELDS, type SearchFieldMeta, } from '@objectstack/spec/data'; @@ -131,8 +129,8 @@ import { recordsOf, suggestName } from './object-graph.js'; import { SYSTEM_FIELDS, indexUnprovisionedAnchors, - unprovisionedAnchorCause, unprovisionedAnchorHint, + unprovisionedAnchorVerdict, } from './system-fields.js'; export const SEARCHABLE_FIELD_UNKNOWN = 'searchable-field-unknown'; @@ -172,6 +170,26 @@ export type SearchableFieldRole = 'canonical' | 'narrowing'; type AnyRec = Record; +/** + * [#22161] The verdict clause for a virtual (`formula`) entry, shared by the + * SEARCH and SORT axes (`sort-field-unsortable` reads it from here) so the two + * rules state the one storage fact in one wording. Each rule appends its own + * consequence; the measured reading behind it is each id's `os explain` entry. + */ +export function virtualFieldClause(vtype: string | undefined): string { + return `a virtual '${vtype}' field: its value is computed on read and never stored`; +} + +/** + * [#22161] The object's declared searchable set as a verdict quotes it: the + * first three names, then a count, so a long declaration cannot stretch the + * one-line verdict (the `fix:` line names where the set is declared). + */ +function declaredRoster(names: readonly string[]): string { + const shown = names.slice(0, 3).join(', '); + return names.length > 3 ? `${shown} (and ${names.length - 3} more)` : shown; +} + function isRec(v: unknown): v is AnyRec { return !!v && typeof v === 'object' && !Array.isArray(v); } @@ -340,10 +358,8 @@ export function checkSearchableFieldList( where, path: `${path}[${i}]`, message: - `${subject} entry "${name}" is not a field on object "${objectName}". ` + - `The declaration is stale: searching it can never match, and the engine ` + - `silently drops it — leaving a narrower search than declared, or the ` + - `auto-default set once every entry is dropped.` + + `${subject} entry "${name}" is not a field on object "${objectName}", so the ` + + `engine drops it from the search.` + (dotted ? '' : suggestName(name, known)), hint: (dotted @@ -378,16 +394,14 @@ export function checkSearchableFieldList( rule: SEARCHABLE_FIELD_UNPROVISIONED, where, path: `${path}[${i}]`, + // [#22161] One verdict sentence: the shared short anchor clause and + // the role's consequence. Why a narrowing reaches the runtime at all + // (the `$searchFields` echo) is the id's `os explain` entry. message: - `${subject} entry "${name}" resolves on object "${objectName}", but ` + - `${unprovisionedAnchorCause(objectName, name)}` + + `${subject}: ${unprovisionedAnchorVerdict(objectName, name)}, so ` + (role === 'narrowing' - ? ` — clients echo this declaration verbatim as the '$searchFields' override, so ` + - `every toolbar search on this list scans a column that is empty on every ` + - `record: it reads as search coverage and matches nothing.` - : ` — 'search' scans it on every record and it can never match, so the object's ` + - `searchable set is narrower than it declares. Should it be the ONLY entry that ` + - `resolves, the set scans nothing at all.`), + ? `the list's toolbar search scans an empty column and matches nothing` + : `'search' never matches it and the searchable set is narrower than it declares`), hint: unprovisionedAnchorHint(objectName, name), }); } @@ -407,11 +421,8 @@ export function checkSearchableFieldList( where, path: `${path}[${i}]`, message: - `${subject} entry "${name}" on object "${objectName}" is a virtual ` + - `'${vtype}' field: its value is computed on read and never stored, so no ` + - `driver materializes a column for 'search' to scan and the entry can never ` + - `match. It reads as search coverage and delivers none — the runtime used to ` + - `admit it verbatim because the declaration named it.`, + `${subject} entry "${name}" on object "${objectName}" is ` + + `${virtualFieldClause(vtype)}, so 'search' never matches it`, hint: `Mirror the computed value onto a stored text field on "${objectName}" and ` + `declare that instead, or drop "${name}". At runtime the ingress gate now ` + @@ -436,11 +447,8 @@ export function checkSearchableFieldList( where, path: `${path}[${i}]`, message: - `${subject} entry "${name}" is outside object "${objectName}"'s declared ` + - `searchableFields (${resolution.declaredList.join(', ')}) — the set 'search' ` + - `scans. Clients echo this declaration verbatim as the '$searchFields' ` + - `override, and the runtime refuses an entry outside the allowed set: every ` + - `toolbar search on this list returns 400 INVALID_FIELD.`, + `${subject} entry "${name}" is outside the declared searchableFields ` + + `(${declaredRoster(resolution.declaredList)}), so list search answers 400 INVALID_FIELD`, hint: `Add "${name}" to ${objectName}.searchableFields, or drop it from this ` + `view — a view narrows the object's searchable set, never widens it ` + @@ -454,7 +462,7 @@ export function checkSearchableFieldList( const isReference = meta?.type === 'lookup' || meta?.type === 'master_detail'; let why: string; if (SEARCH_AUTO_EXCLUDED_FIELDS.has(name)) { - why = 'a system/audit column, which the auto-default set never includes'; + why = 'a system/audit column'; } else if (meta?.hidden) { why = 'hidden'; } else if (typeof meta?.type === 'string') { @@ -467,13 +475,11 @@ export function checkSearchableFieldList( rule: SEARCHABLE_FIELD_UNSEARCHABLE, where, path: `${path}[${i}]`, + // [#22161] The auto-default set's type list and the `$searchFields` + // echo are the id's `os explain` entry. message: - `${subject} entry "${name}" on object "${objectName}" is ${why}. With no ` + - `'searchableFields' declared on the object, 'search' scans its text-like ` + - `columns (${[...SEARCHABLE_TEXTUAL_TYPES, ...SEARCHABLE_ENUM_TYPES].join(' / ')}). ` + - `Clients echo this declaration verbatim as the '$searchFields' override, and ` + - `the runtime refuses it: every toolbar search on this list returns ` + - `400 INVALID_FIELD.`, + `${subject} entry "${name}" is ${why}, so the auto-default search set excludes it ` + + `and list search answers 400 INVALID_FIELD`, hint: (isReference ? `A ${meta?.type} column stores only the referenced record's id, so it ` + diff --git a/packages/lint/src/validate-sortable-fields.test.ts b/packages/lint/src/validate-sortable-fields.test.ts index 53dd4c2d2f2..f97a720a2ff 100644 --- a/packages/lint/src/validate-sortable-fields.test.ts +++ b/packages/lint/src/validate-sortable-fields.test.ts @@ -3,14 +3,37 @@ import { describe, it, expect } from 'vitest'; import { SEARCH_VIRTUAL_TYPES, COMPUTED_VALUE_TYPES } from '@objectstack/spec/data'; import { - validateSortableFields, - checkSortDeclaration, + validateSortableFields as validateSortableFieldsUnrecorded, + checkSortDeclaration as checkSortDeclarationUnrecorded, SORT_FIELD_UNKNOWN, SORT_FIELD_UNSORTABLE, SORT_FIELD_UNPROVISIONED, } from './validate-sortable-fields.js'; import { indexObjectSearchTargets } from './validate-searchable-fields.js'; import { indexUnprovisionedAnchors } from './system-fields.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding of the three ids is one verdict sentence; the reasoning +// it used to carry is the id's `os explain` entry. Every call below records +// what it fired, and the last cases in this file hold each recorded verdict to +// one line of at most 200 characters — so the pin covers every firing variant +// this suite exercises, not a chosen few. Run the whole file: those cases read +// what the cases above fired. +const SORT_IDS: readonly string[] = [SORT_FIELD_UNKNOWN, SORT_FIELD_UNSORTABLE, SORT_FIELD_UNPROVISIONED]; +const fired: Array<{ rule: string; message: string }> = []; +const validateSortableFields: typeof validateSortableFieldsUnrecorded = (...args) => { + const findings = validateSortableFieldsUnrecorded(...args); + fired.push(...findings); + return findings; +}; +const checkSortDeclaration: typeof checkSortDeclarationUnrecorded = (...args) => { + const findings = checkSortDeclarationUnrecorded(...args); + fired.push(...findings); + return findings; +}; + +/** The `os explain` text of `rule`, one string. */ +const explanationOf = (rule: string): string => explainRule(rule)?.paragraphs.join('\n') ?? ''; /** * The object the whole file judges against. It carries one field of each of the @@ -554,9 +577,13 @@ describe('validateSortableFields — the provenance verdict (#10474)', () => { expect(f.message).toContain('created_at'); // The CAUSE clause is the package-shared sentence, not a re-typed one: // a rule that re-words it drifts from the runtime guards whose verdict it - // reports (`unprovisionedAnchorCause`). - expect(f.message).toContain('injected system column with NO storage behind it'); - expect(f.message).toContain('ADR-0015'); + // reports — [#22161] the one-clause `unprovisionedAnchorVerdict`, whose long + // form (ADR-0015 federation) is the id's `os explain` entry. + expect(f.message).toBe( + "list-view sort: 'created_at' is an injected column with no storage on external object " + + "'showcase_ext_customer', so its ORDER BY is dropped and every fetch returns rows in an " + + 'arbitrary order', + ); // The SORT-axis consequence, which is this rule's own half of the sentence. expect(f.message).toContain('ORDER BY'); expect(f.hint).toContain('columnMap'); @@ -687,3 +714,47 @@ describe('checkSortDeclaration — the provenance parameter is OPTIONAL (#10474) expect(findings[0].message).toContain('page sort'); }); }); + +describe('[#22161] one-line verdicts — the three sort ids', () => { + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: each id fired at least once, so the shape + // assertion below cannot pass over an empty record. + expect([...new Set(fired.map((f) => f.rule))].sort()).toEqual([...SORT_IDS].sort()); + for (const f of fired) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + it('the virtual verdict states the storage fact in the SEARCH axis\' wording', () => { + const [f] = validateSortableFields(withListView([{ field: 'expected_revenue', order: 'desc' }])); + expect(f.message).toBe( + "list-view sort orders by \"expected_revenue\" on object \"crm_opportunity\", a virtual 'formula' " + + 'field: its value is computed on read and never stored, so there is no column to ORDER BY', + ); + }); + + // What each verdict stopped saying, which `os explain RULE_ID` now prints. + const MOVED: Record = { + [SORT_FIELD_UNKNOWN]: ['FIRST fetch', '`assertSortFieldsExist`', 'HEAD segment', 'flattened list overlay', 'introspected datasource'], + [SORT_FIELD_UNSORTABLE]: ['byte-identical', '`assertOrderByIsMaterializable`', '`SEARCH_VIRTUAL_TYPES`', '`COMPUTED_VALUE_TYPES`'], + [SORT_FIELD_UNPROVISIONED]: ['ADR-0015', 'byte-identical', 'judge only `formula`', '`limit` / `offset`', 'warning'], + }; + + it('covers exactly the three ids', () => { + expect(Object.keys(MOVED).sort()).toEqual([...SORT_IDS].sort()); + }); + + it.each([...SORT_IDS])('`os explain %s` carries what its verdict no longer says', (rule) => { + expect(explainRule(rule), `no \`os explain ${rule}\` entry`).toBeDefined(); + const text = explanationOf(rule); + for (const fact of MOVED[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + }); + + it('the explanation\'s virtual-type claim matches the spec predicate it names', () => { + // The explanation module imports nothing, so the one fact it states about + // the predicate is held to the predicate here. + expect([...SEARCH_VIRTUAL_TYPES]).toEqual(['formula']); + expect(explanationOf(SORT_FIELD_UNSORTABLE)).toContain('`formula` alone'); + }); +}); diff --git a/packages/lint/src/validate-sortable-fields.ts b/packages/lint/src/validate-sortable-fields.ts index 7ab9de708bd..ace03ff2b4d 100644 --- a/packages/lint/src/validate-sortable-fields.ts +++ b/packages/lint/src/validate-sortable-fields.ts @@ -235,11 +235,12 @@ import { suggestName } from './object-graph.js'; import { SYSTEM_FIELDS, indexUnprovisionedAnchors, - unprovisionedAnchorCause, unprovisionedAnchorHint, + unprovisionedAnchorVerdict, } from './system-fields.js'; import { indexObjectSearchTargets, + virtualFieldClause, type ObjectSearchTarget, } from './validate-searchable-fields.js'; @@ -405,17 +406,12 @@ export function checkSortDeclaration( rule: SORT_FIELD_UNPROVISIONED, where, path: `${path}${key.at}`, + // [#22161] One verdict sentence: the shared short anchor clause and + // this axis' consequence. The measured byte-identical reading and why + // no door refuses it are the id's `os explain` entry. message: - `${subject} orders by "${name}", which resolves on object ` + - `"${objectName}", but ${unprovisionedAnchorCause(objectName, name)} — ` + - `so the ORDER BY reaches the driver, finds no column, and is dropped. ` + - `Measured on a federated object over a real remote table, 'asc' and ` + - `'desc' return BYTE-IDENTICAL row order under a 200 while the same ` + - `query on a real column reverses. Unlike this rule's other two ` + - `verdicts nothing refuses it: the REST ingress and the engine ` + - `both judge only 'formula', so the view's FIRST fetch — and ` + - `every fetch after it — silently answers in an arbitrary order, which ` + - `'limit'/'offset' then slice into an arbitrary page.`, + `${subject}: ${unprovisionedAnchorVerdict(objectName, name)}, so its ORDER BY is ` + + `dropped and every fetch returns rows in an arbitrary order`, hint: unprovisionedAnchorHint(objectName, name), }); } @@ -431,9 +427,7 @@ export function checkSortDeclaration( path: `${path}${key.at}`, message: `${subject} orders by "${name}", which is not a field on object ` + - `"${objectName}". The runtime refuses the sort rather than dropping it: ` + - `every load of this view answers 400 INVALID_SORT, because a sort ` + - `is the view's FIRST fetch and not an optional interaction.` + + `"${objectName}", so every load of this view answers 400 INVALID_SORT.` + (dotted ? '' : suggestName(head, known)), hint: (dotted @@ -459,11 +453,8 @@ export function checkSortDeclaration( where, path: `${path}${key.at}`, message: - `${subject} orders by "${name}" on object "${objectName}", a virtual ` + - `'${vtype}' field: its value is computed on read and never stored, so no ` + - `driver materialises a column to ORDER BY. Measured, an unrefused sort on ` + - `one returns 'asc' and 'desc' in byte-identical order — the rows carry the ` + - `values they were asked to be ordered by, unordered, under a success.`, + `${subject} orders by "${name}" on object "${objectName}", ` + + `${virtualFieldClause(vtype)}, so there is no column to ORDER BY`, hint: `Denormalise the value onto "${objectName}" (a stored field, written when ` + `the source changes) and sort by that, or drop "${name}" from this sort. ` +