From 5be9a3154e5001b316e676105fe90ae99ff9f051 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 16:07:16 +0000 Subject: [PATCH 1/3] wip(lint): one-line verdicts for slice 5's 15 rule ids, reasoning in RULE_EXPLANATIONS Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude --- packages/lint/src/rule-explanations.ts | 422 +++++++++++++++++- ...alidate-dataset-measure-aggregates.test.ts | 131 +++++- .../validate-dataset-measure-aggregates.ts | 94 ++-- .../validate-flow-trigger-readiness.test.ts | 205 +++++++-- .../src/validate-flow-trigger-readiness.ts | 117 +++-- .../src/validate-react-page-props.test.ts | 142 +++++- .../lint/src/validate-react-page-props.ts | 68 ++- .../src/validate-rule-compilability.test.ts | 76 +++- .../lint/src/validate-rule-compilability.ts | 55 ++- .../src/validate-rule-schema-formats.test.ts | 59 ++- .../lint/src/validate-rule-schema-formats.ts | 20 +- 11 files changed, 1166 insertions(+), 223 deletions(-) diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index fe81aa69631..1d7df6c965a 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -365,14 +365,22 @@ 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`). + * The rule files share its verdict clause, `unprovisionedAnchorVerdict()`. + */ +const UNPROVISIONED_ANCHOR_CAUSE = + 'The registry injects system columns — the ownership and audit anchors such as `owner_id`, ' + + '`created_at` and `created_by` — into objects. On an ADR-0015 external object the remote ' + + 'database owns the schema, so the platform registers these anchors without provisioning a ' + + 'column: the name resolves, but no storage stands behind it.'; + const DASHBOARD_FILTER_FIELD_UNPROVISIONED_EXPLANATION: RuleExplanation = { rule: 'dashboard-filter-field-unprovisioned', covers: 'why a filter on an unprovisioned anchor matches nothing', paragraphs: [ - 'The registry injects system columns — the ownership and audit anchors such as `owner_id`, ' + - '`created_at` and `created_by` — into objects. On an ADR-0015 external object the remote ' + - 'database owns the schema, so the platform registers these anchors without provisioning a ' + - 'column: the name resolves, but no storage stands behind it.', + UNPROVISIONED_ANCHOR_CAUSE, 'A dashboard filter whose effective field lands on such an anchor is ANDed into the widget\'s ' + 'analytics query and can never match a real value: on SQLite it silently degrades to ' + 'constant-false and the widget renders empty (HTTP 200, zero rows, no error).', @@ -1320,6 +1328,397 @@ const SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT_EXPLANATION: RuleExplanation = { ], }; +// ── Flow trigger readiness (the never-fire family) ────────────────────────── +// `validate-flow-trigger-readiness.ts`: five ids whose verdict is settled by a +// contract this repository ships, so no installed package can make the flow +// fire. The engine's routing chain, the record-trigger grammar, the +// time-relative partition and the publish-gate reach are shared, so each is +// written ONCE below and referenced by every entry that needs it. The rule +// file shares its verdict clause the same way (`NEVER_FIRES`). + +/** How the engine picks a flow's trigger — the routing chain every flow id reads. */ +const FLOW_TRIGGER_ROUTING = + 'The automation engine picks a flow\'s trigger from its start node in one fixed chain ' + + '(`resolveTriggerBinding`): a `triggerType` starting with \'record-\' routes to the record-change ' + + 'trigger (an array-form `triggerType` holding such a token is routed there too, to be refused); a ' + + '`config.timeRelative` that is an object routes to the time-relative sweep; a `config.schedule` or ' + + '`type: \'schedule\'` to the schedule trigger; `type: \'api\'` or `triggerType: \'api\'` to the inbound ' + + 'api trigger. Every test in the chain is a literal string or `typeof` check with no registry lookup, so ' + + 'no installed package can teach the engine a new token, and a flow that matches none of them is a ' + + 'manual flow.'; + +/** The record trigger's closed token grammar — the two triggerType ids. */ +const RECORD_TRIGGER_GRAMMAR = + 'The record-change trigger maps a token to ObjectQL hook events only on the grammar ' + + 'record-{before,after}-{create,insert,update,delete,write}: `insert` is a synonym for `create`, and ' + + '`write` fires on create OR update in one flow. Any other token maps to no hook event, and there is no ' + + '"any change" token.'; + +/** The routing predicate that splits the two time-relative ids. */ +const TIME_RELATIVE_PARTITION = + 'The engine routes a flow to the time-relative sweep only when `config.timeRelative` is present and ' + + '`typeof` says \'object\' (arrays and dates included). The two time-relative ids partition the key\'s ' + + 'values along that predicate, so exactly one of them can fire on a descriptor: ' + + '`flow-time-relative-descriptor-invalid` for a routed value the descriptor schema refuses, ' + + '`flow-time-relative-descriptor-unroutable` for a value the engine never routes.'; + +/** Where the never-fire family gates, and what the runtime door judges — every flow error id. */ +const FLOW_GATE_REACH = + 'The finding is an error, so it fails `os validate` and `os build`, and the runtime publish gate ' + + 'refuses an active flow write that carries it. That gate judges only the flow being written, against ' + + 'the findings the stored stack already had, so a dead flow already stored never blocks another flow\'s ' + + 'save; what is refused is the dead flow\'s own publish.'; + +const FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID_EXPLANATION: RuleExplanation = { + rule: 'flow-time-relative-descriptor-invalid', + covers: 'why a refused timeRelative descriptor never binds', + paragraphs: [ + TIME_RELATIVE_PARTITION, + 'The time-relative trigger safeParses the descriptor against `TimeRelativeTriggerSchema` ' + + '(`@objectstack/spec/automation`) when it binds, and a descriptor the schema refuses is not bound: ' + + 'the sweep is never installed, so the flow declares a time-relative trigger and then never runs. ' + + 'Before this rule the only trace was one warn in the server log at bind time — a channel an ' + + 'authoring loop never reads.', + 'The rule asks that same schema; it restates none of the descriptor\'s contract. The verdict quotes ' + + 'the schema\'s own words for ONE issue — an unrecognized key first, since its rename usually ' + + 'explains the others (a misspelled `field` is also the missing `dateField`) — and counts the rest; ' + + 'the bind-time warn prints the whole list. Each issue is quoted to its verdict: the key or value ' + + 'that is wrong, and the schema\'s rename when it has one.', + 'Two keys the schema answers with a prescription rather than a rename, because each is real one ' + + 'layer out: `schedule` is a sibling of `timeRelative` on the START node\'s `config`, not a key ' + + 'inside it (omitting it means daily at 08:00 UTC), and `runAs` is a FLOW-level key: a sweep has no ' + + 'trigger user, so declare `runAs: \'system\'` beside `nodes` and `edges`.', + FLOW_GATE_REACH, + ], +}; + +const FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE_EXPLANATION: RuleExplanation = { + rule: 'flow-time-relative-descriptor-unroutable', + covers: 'why a non-object timeRelative is never routed', + paragraphs: [ + TIME_RELATIVE_PARTITION, + 'A `config.timeRelative` that is a string, a number, a boolean or a function — `\'daily\'` is the ' + + 'usual one, a cadence written into the descriptor slot — is never routed to the sweep, so the ' + + 'descriptor schema never sees it and the sweep is never installed. A node\'s `config` is an open ' + + 'slot, so the scalar parses everywhere, and nothing at any layer says a word: not the schema, not ' + + 'the engine, not even the one bind-time warn a descriptor that IS an object gets when it is refused.', + FLOW_TRIGGER_ROUTING, + 'What follows depends on the rest of the start node, so the verdict names it. When the node also ' + + 'declares a trigger the engine does route — a record-change token, a `config.schedule` cadence, the ' + + 'api trigger — the flow fires on THAT trigger\'s terms (once per firing, with no record on the ' + + 'context) instead of once per matching record, and the descriptor is silently dropped. When nothing ' + + 'else declares a trigger the flow binds to nothing and never fires.', + 'A descriptor is an object: `{ object, dateField, and exactly one of withinDays | offsetDays }`, plus ' + + 'an optional `filter` and `maxRecords`. HOW OFTEN the sweep runs is the sibling key ' + + '`config.schedule` on the same start node.', + FLOW_GATE_REACH, + ], +}; + +const FLOW_TRIGGER_UNROUTABLE_EXPLANATION: RuleExplanation = { + rule: 'flow-trigger-unroutable', + covers: 'why a record_change flow with no record token never fires', + paragraphs: [ + FLOW_TRIGGER_ROUTING, + 'A flow that declares `type: \'record_change\'` has stated its intent, so a start node whose ' + + '`triggerType` is off that chain (`onCreate`, `on_update`, an empty string) or absent altogether is ' + + 'a defect, not a manual flow: the engine routes it to no trigger, the flow is demoted to a manual ' + + 'one, and it never fires. A plugin cannot rescue it — triggers are registered by the RESOLVED type, ' + + 'and this flow never resolves one.', + 'Nothing names it at run time. The unbound-flow audit resolves the same binding and skips the flow ' + + 'as "manual — nothing to bind", so neither the boot warning nor the startup summary lists it; the ' + + 'only trace is the banner\'s flow count being one higher than its bound count.', + RECORD_TRIGGER_GRAMMAR, + 'Not reported: a `record_change` flow that ALSO declares something the engine routes ' + + '(`config.schedule`, `triggerType: \'api\'`) — it binds and fires, on the wrong trigger\'s terms, ' + + 'which is a different defect. A flow that really is launched by hand or from a screen declares ' + + '`type: \'autolaunched\'` or `\'screen\'`, which have no trigger to be missing.', + FLOW_GATE_REACH, + ], +}; + +const FLOW_TRIGGER_UNKNOWN_EVENT_EXPLANATION: RuleExplanation = { + rule: 'flow-trigger-unknown-event', + covers: 'which record trigger tokens fire', + paragraphs: [ + RECORD_TRIGGER_GRAMMAR, + 'A token that starts with \'record-\' but is off that grammar — a typo (`record-after-updated`), a ' + + 'phase-less noun (`record-change`), a bad phase (`record-during-update`) — is still routed to the ' + + 'record-change trigger, which maps it to no hook event: the flow binds and never fires, with one ' + + 'bind-time warn in the server log as the only trace.', + 'An array-form `triggerType` (`[\'record-after-create\', \'record-after-delete\']`) is a multi-event ' + + 'shape the engine does not support: a start node takes one trigger event, so the flow binds to ' + + 'nothing and never fires. For "created or updated" use `record-after-write`; any other combination ' + + 'is one flow per event.', + FLOW_GATE_REACH, + ], +}; + +const FLOW_API_TRIGGER_SECRET_MISSING_EXPLANATION: RuleExplanation = { + rule: 'flow-api-trigger-secret-missing', + covers: 'why an inbound api flow needs a secret', + paragraphs: [ + 'An inbound api hook is armed only with a per-flow secret that every post is HMAC-verified against ' + + '(ADR-0041): the `x-objectstack-signature` header carries \'sha256=\' and the hex HMAC-SHA256 of ' + + 'the raw body. A usable secret is a string that is not blank after trimming, on the start node\'s ' + + '`config.secret`; absent, blank and non-string are one verdict.', + 'The automation engine refuses to register a flow bound to the api trigger without one — a ' + + 'hardcoded check in `registerFlow`, before any trigger is consulted and whatever the flow\'s ' + + '`status` — and the trigger refuses to arm it, so the flow never receives a post. The `/automation` ' + + 'write doors answer 400, and a boot skips the flow with a warning.', + 'Which flows bind the api trigger is the engine\'s answer, not a reading of `type`: the chain ranks a ' + + 'record token, a time-relative descriptor and a schedule ahead of `api`, so a `type: \'api\'` flow ' + + 'whose start node also carries one of those is bound to THAT trigger and needs no secret. The ' + + 'verdict names which declaration binds the flow; the secret\'s value is never printed, only its type.', + 'At the runtime publish gate a signed flow\'s ordinary edit arrives without its secret, because the ' + + 'read path withholds it; the gate passes the positions the stored secret will be restored to, and ' + + 'the rule reads those as present. A secretless api flow is refused there.', + ], +}; + +// ── Validation rules that compile at publish time ─────────────────────────── +// `validate-rule-compilability.ts` and `validate-rule-schema-formats.ts`: the +// write path's fail-open skip and the runtime's ajv environment are shared, +// so each is written ONCE below. The compile verdicts share their closing +// clause in the rule file (`SKIPPED_ON_EVERY_WRITE`). + +/** What the write path does with a rule whose artifact does not compile — the two compile ids. */ +const VALIDATION_RULE_FAIL_OPEN = + 'On the write path (`rule-validator.ts`), `checkFormat` builds a `format` rule\'s `regex` with ' + + '`new RegExp(rule.regex)` and `checkJsonSchema` compiles a `json_schema` rule\'s `schema` with ajv, ' + + 'each inside a `try/catch` whose catch logs "… — skipped" and returns no error. A rule whose artifact ' + + 'does not compile is therefore declared, listed in the metadata and in every listing of what protects ' + + 'the object, and enforces nothing on any record, forever; one WARN line in the server log is the only ' + + 'trace. Both artifacts are static, so the rule refuses them before they ship. Rules nested in a ' + + '`conditional`\'s `then` / `otherwise` are judged too: they reach the same checks.'; + +/** The ajv the runtime compiles with, mirrored by the gate — the two `json_schema` ids. */ +const RUNTIME_AJV_ENVIRONMENT = + 'The runtime compiles every `json_schema` rule with one shared ajv: `new Ajv({ allErrors: true, ' + + 'strict: false })` plus the `ajv-formats` plugin. The gate builds the same environment (its test pins ' + + 'both halves against `rule-validator.ts`), so it never invents a verdict the runtime does not share: ' + + '`strict: false` lets an author\'s vendor keywords through, and `ajv-formats` registers the format ' + + 'names and the `formatMinimum` / `formatMaximum` keywords.'; + +const VALIDATION_RULE_REGEX_UNCOMPILABLE_EXPLANATION: RuleExplanation = { + rule: 'validation-rule-regex-uncompilable', + covers: 'what the write path does with a regex that does not compile', + paragraphs: [ + VALIDATION_RULE_FAIL_OPEN, + 'The pattern is compiled exactly as `checkFormat` compiles it — `new RegExp(source)`, no flags — and ' + + 'the verdict quotes the engine\'s reason; the `fix:` line quotes the source. The source is a STRING, ' + + 'so a backslash in it is written twice in TypeScript.', + ], +}; + +const VALIDATION_RULE_JSON_SCHEMA_UNCOMPILABLE_EXPLANATION: RuleExplanation = { + rule: 'validation-rule-json-schema-uncompilable', + covers: 'what the write path does with a schema ajv refuses', + paragraphs: [ + VALIDATION_RULE_FAIL_OPEN, + RUNTIME_AJV_ENVIRONMENT, + 'Under `allErrors: true` a schema that fails ajv\'s metaschema lists every violation; the verdict ' + + 'quotes the first, which names the offending keyword, and counts the rest.', + 'Each schema is compiled on a fresh instance, so a duplicate `$id` between two deployed rules — an ' + + 'outcome that depends on which schema the runtime happened to compile first — is not judged here. ' + + 'A misspelled `format` name compiles in both environments; that is ' + + '`validation-rule-json-schema-unknown-format`\'s verdict, not this one\'s.', + ], +}; + +const VALIDATION_RULE_JSON_SCHEMA_UNKNOWN_FORMAT_EXPLANATION: RuleExplanation = { + rule: 'validation-rule-json-schema-unknown-format', + covers: 'why an unregistered format name enforces nothing', + paragraphs: [ + RUNTIME_AJV_ENVIRONMENT, + 'Under `strict: false` an unregistered `format` name is not an error: ajv logs ' + + '`unknown format "NAME" ignored in schema at path …` once at compile time and DROPS the keyword — in ' + + 'the write path and at the publish gate alike. The schema compiles, the rule ships and runs on every ' + + 'write, its other keywords (`type`, `required`) are enforced, and this constraint is enforced on no ' + + 'record, ever. The record is ACCEPTED, the silent direction, so nothing downstream reports the gap.', + 'The registered names are read off a live instance of that ajv, never written down, so the rule ' + + 'follows `ajv-formats` across an upgrade. Names are case-sensitive and hyphenated (`date-time`, not ' + + '`datetime`); the `fix:` line names the nearest registered one and lists them all.', + 'Only positions JSON Schema defines as subschemas are walked (`properties`, `items`, `$defs`, ' + + '`allOf` / `anyOf` / `oneOf`, `if` / `then` / `else` and the rest), because those are the places ' + + 'ajv applies the keyword; a `format` inside `default`, `const`, `enum` or `examples` is data. A ' + + 'non-string `format` is the compile gate\'s refusal (`validation-rule-json-schema-uncompilable`), ' + + 'not this rule\'s.', + ], +}; + +// ── Dataset and cube members the analytics doors refuse ───────────────────── +// `validate-dataset-measure-aggregates.ts`: two ids on one walk. What a +// JSON-stored column is, the cube leg and the shared skips are written ONCE. + +/** What "JSON-stored" means — both ids. Held to the spec's three type sets by the rule's test. */ +const JSON_STORED_COLUMN = + 'A JSON-stored column is a structured-JSON type — json, composite, repeater, record, location, ' + + 'address, vector — or a multi-value field: an inherently multi option type (multiselect, checkboxes, ' + + 'tags), or a multi-capable type (select, radio, lookup, user, file, image) flagged `multiple: true`. ' + + 'The same multi-capable type without the flag stores one value.'; + +/** Where the rule reads its columns, and what it never judges — both ids. */ +const ANALYTICS_MEMBER_WALK = + 'Dataset measures and dimensions name their column with `field`; authored cubes (`analyticsCubes`) ' + + 'name it with a member\'s `sql`, resolved the way the analytics door resolves it — a hop the cube ' + + 'declares a join for reaches that join\'s object, any other hop is walked on the object graph ' + + '("joined object" in the verdict), and the row wildcard `\'*\'` names no column. Never judged: an ' + + 'object this stack does not define or that has no readable field map, a path that does not resolve ' + + '(`dataset-field-unknown` reports it), a leaf with no declared type, and, for a measure, an aggregate ' + + 'outside the table\'s vocabulary. At the runtime publish gate a `dataset` write carries no `analyticsCubes`, so ' + + 'the cube leg speaks on the CLI doors.'; + +const MEASURE_AGGREGATE_FIELD_TYPE_REFUSED_EXPLANATION: RuleExplanation = { + rule: 'measure-aggregate-field-type-refused', + covers: 'which aggregates each field type accepts', + paragraphs: [ + 'A measure pairs an aggregate with a column, and the aggregate × field-type compatibility table ' + + '(`AGGREGATE_FIELD_TYPE_COMPATIBILITY`, `@objectstack/spec/data`) decides which pairs are coherent. ' + + 'For a refused pair the number a backend returns is a property of the SQL dialect rather than of the ' + + 'data: SQLite coerces a `datetime` column\'s stored text by its leading digits, so `avg` over it ' + + 'returns the average YEAR with no error and no log, while another backend has no such function and ' + + 'fails at query time.', + // Held equal to `AGGREGATE_FIELD_TYPE_COMPATIBILITY` by `validate-dataset-measure-aggregates.test.ts`. + 'The table\'s rows: `count` accepts every type, because it reads no value; `count_distinct` every ' + + 'type but the JSON-stored ones; `sum`: number, currency, rating, slider, progress, summary, ' + + 'boolean, toggle; `avg`: number, currency, percent, rating, slider, progress, summary, boolean, ' + + 'toggle; `min` and `max`: number, currency, percent, rating, slider, progress, summary, date, ' + + 'datetime, time, boolean, toggle.', + JSON_STORED_COLUMN, + 'The per-type row cannot see `multiple: true`, so `count_distinct` over a multi-capable field ' + + 'flagged that way is refused by the declaration: the value is a list stored as JSON, and ' + + '`count_distinct` compares values for equality, which no two backends do alike for JSON — one ' + + 'counts every row apart, one compares the serialized text, another has no equality for the type ' + + 'and fails at query time.', + 'The same pair is refused later by the doors the member reaches: a dataset\'s by its compile ' + + '(`400 DATASET_INVALID`), a cube\'s by the analytics door when a query names the measure ' + + '(`400 INVALID_FIELD`). This rule makes the same refusal where the author is standing. A dotted ' + + 'path is judged on the leaf\'s own declaration, which the compile leg cannot see.', + ANALYTICS_MEMBER_WALK, + 'Its advisory neighbour `measure-aggregate-incoherent` judges whether a number MEANS anything, so ' + + '`sum` over a `percent` field is reported by both: an advisory about meaning, and this refusal ' + + 'about the contract.', + ], +}; + +const DIMENSION_JSON_STORED_FIELD_REFUSED_EXPLANATION: RuleExplanation = { + rule: 'dimension-json-stored-field-refused', + covers: 'why analytics does not group by a JSON-stored column', + paragraphs: [ + 'A dimension is a GROUP KEY: a dataset dimension compiles to a cube dimension whose `sql` is its ' + + '`field`, and a query that selects it groups by that column. The analytics door refuses a grouped ' + + 'member whose column is JSON-stored with `400 INVALID_FIELD`, before any SQL is built, because a ' + + 'JSON value is no group key the SQL dialects share: one groups each serialized value apart, another ' + + 'refuses the statement. Every report, dashboard or query that selects the dimension gets that ' + + 'refusal instead of an answer, and no selection of it is served as a group.', + JSON_STORED_COLUMN, + ANALYTICS_MEMBER_WALK, + ], +}; + +// ── React page props (`validate-react-page-props.ts`) ─────────────────────── +// Two ids forward `@objectstack/spec`'s own refusal of a chart prop. The +// verdict quotes it to its head — the key or value, and the schema's rename +// (`schemaRefusalHead()`); the shape note below is shared, and each entry +// names the keys its schema answers with a prescription instead. + +/** How a forwarded chart-prop refusal is quoted — the two `` schema ids. */ +const CHART_PROP_REFUSAL = + 'The prop is judged by PARSING its `@objectstack/spec/ui` schema, never by a second copy of its rules, ' + + 'so the vocabulary, the refinements and the unknown-key handling all arrive from the spec. Only a ' + + 'fully static literal is judged: a value built at run time is unknowable here, not wrong. The verdict ' + + 'quotes the schema\'s refusal to its verdict — the key or value that is wrong, and the schema\'s ' + + 'rename when it has one — and leaves out the sentence on why the key used to be dropped silently.'; + +const REACT_CHART_DRILLDOWN_INVALID_EXPLANATION: RuleExplanation = { + rule: 'react-chart-drilldown-invalid', + covers: 'what an ObjectChart drillDown block accepts', + paragraphs: [ + '`` is checked against `ChartDrillDownSchema`, a closed shape. Until it ' + + 'was closed every key inside it, right or wrong, reached the renderer unchecked, and a misspelling ' + + 'was simply ignored at click time.', + CHART_PROP_REFUSAL, + 'Keys the schema answers with a prescription rather than a rename, because each is real on another ' + + 'surface: `drilldown` (all lowercase) is `ReportSchema.drilldown`, a boolean on a summary or matrix ' + + 'report, while the chart\'s is `drillDown` and takes an object; `mode` is objectui\'s ' + + '`object-data-table` drill key, and a chart drill is always the filtered-list kind, so delete it; ' + + '`report` is a metric or pivot widget capability `` does not read; `view` and `sort` ' + + 'are declared by objectui\'s renderer-side type and read by no renderer, so delete them.', + ], +}; + +const REACT_CHART_AGGREGATE_INVALID_EXPLANATION: RuleExplanation = { + rule: 'react-chart-aggregate-invalid', + covers: 'what an ObjectChart aggregate accepts', + paragraphs: [ + '`` is checked against `ChartAggregateSchema`, a closed shape whose ' + + '`groupBy` is a bare field name or a closed `{ field, dateGranularity?, alias? }`. Until both were ' + + 'closed an undeclared key was dropped at parse: `groupby` for `groupBy` degraded the chart to a ' + + 'single ungrouped point, `fn` for `function` fell back to the default, and `dateGranularty` for ' + + '`dateGranularity` cost the date bucketing, all with `build` and `validate` green.', + CHART_PROP_REFUSAL, + 'When no form of a union matches, the verdict keeps the forms whose complaint is about the value\'s ' + + 'content and drops one whose only complaint is that the whole value is another type — for an object, ' + + 'the bare-field-name form\'s "expected string" — unless every form says only that.', + 'Keys the schema answers with a prescription rather than a rename, because each is real one layer ' + + 'out: on the aggregate, `dateGranularity` and `alias` go inside the structured `groupBy`, `filter` ' + + 'and `objectName` are props of the chart itself, and `measures` is the dataset path\'s (an inline ' + + 'aggregate is one function over one field); inside `groupBy`, `function` belongs on the aggregate ' + + 'and a nested `groupBy` is the one you are already in.', + 'Every refusal the schema, the published react-blocks type and objectui\'s renderer agree on is an ' + + 'error. An absent `groupBy` is a warning, and it is a tolerance, not a supported shape: the ' + + 'aggregate returns one ungrouped row and the chart plots a single point; a single number belongs ' + + 'in an object-metric block.', + ], +}; + +const REACT_CHART_FIELD_UNPROVISIONED_EXPLANATION: RuleExplanation = { + rule: 'react-chart-field-unprovisioned', + covers: 'why a chart over an unprovisioned anchor shows nothing', + paragraphs: [ + UNPROVISIONED_ANCHOR_CAUSE, + 'An `` aggregate whose `field` or `groupBy` names such an anchor reads a column that ' + + 'is empty on every row: grouping by it puts every row into one empty bucket, and aggregating it ' + + 'yields nothing. The query does not fail, so the chart renders and says nothing true.', + 'Only a name that resolved BECAUSE it is injected is judged: an author-declared column of the same ' + + 'name maps a remote column the author vouches for and is never reported. The finding is a warning ' + + 'because the rule cannot see the remote table, only that the platform provisions no storage for ' + + 'the anchor.', + ], +}; + +const REACT_BLOCK_NEEDS_RECORD_CONTEXT_EXPLANATION: RuleExplanation = { + rule: 'react-block-needs-record-context', + covers: 'why a record block renders empty on a react page', + paragraphs: [ + 'The `record:*` blocks — ``, ``, ``, ' + + '`` and the rest, or `` — render the record that a record ' + + 'page puts in context. A `kind:\'react\'` page never mounts that context, so the block renders ' + + 'empty however it is bound: the renderer does not read its `objectName` or `recordId`.', + 'They were withdrawn from the react tier for that reason, and the finding is an error because no ' + + 'binding makes the block render. A component the page declares itself under the same name ' + + 'shadows the injected one and is not judged. The `fix:` line names the block\'s react-tier ' + + 'replacement, or a record page.', + ], +}; + +const REACT_PAGE_SOURCE_UNPARSEABLE_EXPLANATION: RuleExplanation = { + rule: 'react-page-source-unparseable', + covers: 'what a react source that does not parse leaves unchecked', + paragraphs: [ + 'A `kind:\'react\'` page\'s source is parsed with TypeScript — never executed — so its component ' + + 'contract can be checked: props, field bindings, chart aggregates. A source with syntax errors is ' + + 'only partially recovered, and the checks read that recovered tree, so a problem in the part the ' + + 'parser could not read is reported by no rule.', + 'It is a warning, not a second syntax verdict. The syntax verdict on a react page is ' + + '`react-page-syntax`\'s, which transpiles the source with Sucrase, the parser that actually compiles it, ' + + 'and the two parsers accept different sets: an octal literal such as `0755` parses in Sucrase and ' + + 'not in TypeScript, and `with (o) {}` the other way round. Erroring here would fail builds the ' + + 'platform\'s own transpiler accepts.', + ], +}; + /** * Every rule explanation this package ships, keyed by rule id. `os explain * ` reads this table and nothing else. @@ -1385,6 +1784,21 @@ export const RULE_EXPLANATIONS: Readonly> = Obje [SHARING_RULE_RUNTIME_VARIABLE_CONDITION_EXPLANATION.rule]: SHARING_RULE_RUNTIME_VARIABLE_CONDITION_EXPLANATION, [SHARING_RULE_OBJECT_NOT_SHAREABLE_EXPLANATION.rule]: SHARING_RULE_OBJECT_NOT_SHAREABLE_EXPLANATION, [SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT_EXPLANATION.rule]: SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT_EXPLANATION, + [FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID_EXPLANATION.rule]: FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID_EXPLANATION, + [FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE_EXPLANATION.rule]: FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE_EXPLANATION, + [FLOW_TRIGGER_UNROUTABLE_EXPLANATION.rule]: FLOW_TRIGGER_UNROUTABLE_EXPLANATION, + [FLOW_TRIGGER_UNKNOWN_EVENT_EXPLANATION.rule]: FLOW_TRIGGER_UNKNOWN_EVENT_EXPLANATION, + [FLOW_API_TRIGGER_SECRET_MISSING_EXPLANATION.rule]: FLOW_API_TRIGGER_SECRET_MISSING_EXPLANATION, + [VALIDATION_RULE_REGEX_UNCOMPILABLE_EXPLANATION.rule]: VALIDATION_RULE_REGEX_UNCOMPILABLE_EXPLANATION, + [VALIDATION_RULE_JSON_SCHEMA_UNCOMPILABLE_EXPLANATION.rule]: VALIDATION_RULE_JSON_SCHEMA_UNCOMPILABLE_EXPLANATION, + [VALIDATION_RULE_JSON_SCHEMA_UNKNOWN_FORMAT_EXPLANATION.rule]: VALIDATION_RULE_JSON_SCHEMA_UNKNOWN_FORMAT_EXPLANATION, + [MEASURE_AGGREGATE_FIELD_TYPE_REFUSED_EXPLANATION.rule]: MEASURE_AGGREGATE_FIELD_TYPE_REFUSED_EXPLANATION, + [DIMENSION_JSON_STORED_FIELD_REFUSED_EXPLANATION.rule]: DIMENSION_JSON_STORED_FIELD_REFUSED_EXPLANATION, + [REACT_CHART_DRILLDOWN_INVALID_EXPLANATION.rule]: REACT_CHART_DRILLDOWN_INVALID_EXPLANATION, + [REACT_CHART_AGGREGATE_INVALID_EXPLANATION.rule]: REACT_CHART_AGGREGATE_INVALID_EXPLANATION, + [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, }); /** The explanation for `rule`, or `undefined` when the rule has none. Exact id match. */ diff --git a/packages/lint/src/validate-dataset-measure-aggregates.test.ts b/packages/lint/src/validate-dataset-measure-aggregates.test.ts index f08031d104a..4b9f0256ef1 100644 --- a/packages/lint/src/validate-dataset-measure-aggregates.test.ts +++ b/packages/lint/src/validate-dataset-measure-aggregates.test.ts @@ -19,6 +19,7 @@ import { BOOLEAN_VALUE_TYPES, FieldType, MULTI_CAPABLE_TYPES, + MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES, STRUCTURED_JSON_TYPES, isAggregateCompatibleWithFieldType, @@ -26,12 +27,29 @@ import { } from '@objectstack/spec/data'; import { runAuthoringRules } from './authoring-rules.js'; +import { explainRule } from './rule-explanations.js'; import { DIMENSION_JSON_STORED_FIELD_REFUSED, MEASURE_AGGREGATE_FIELD_TYPE_REFUSED, - validateDatasetMeasureAggregates, + validateDatasetMeasureAggregates as validateDatasetMeasureAggregatesUnrecorded, } from './validate-dataset-measure-aggregates.js'; +// [#22161] Each finding of the two 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 fired: Array<{ rule: string; message: string }> = []; +const validateDatasetMeasureAggregates: typeof validateDatasetMeasureAggregatesUnrecorded = (...args) => { + const found = validateDatasetMeasureAggregatesUnrecorded(...args); + fired.push(...found); + return found; +}; + +/** The `os explain` text of `rule`, one string. */ +const explanationOf = (rule: string): string => explainRule(rule)?.paragraphs.join('\n') ?? ''; + const RULE = MEASURE_AGGREGATE_FIELD_TYPE_REFUSED; /** Every aggregate the spec table declares a row for. */ @@ -93,13 +111,16 @@ describe('measure-aggregate-field-type-refused — refuses a pair the spec table expect(issue.rule).toBe(RULE); expect(issue.path).toBe('datasets[0].measures[0].aggregate'); // The author must be able to act without opening the spec: the AGGREGATE, - // the FIELD, its declared TYPE, and the set that aggregate accepts. - expect(issue.message).toContain('avg'); - expect(issue.message).toContain('measured'); - expect(issue.message).toContain('datetime'); - for (const accepted of AGGREGATE_FIELD_TYPE_COMPATIBILITY.avg) { - expect(issue.message, `accepted type ${accepted} named`).toContain(accepted); - } + // the FIELD and its declared TYPE… + expect(issue.message).toBe( + 'aggregate "avg" over field "measured" (`datetime` on object "crm_opportunity") is refused by the ' + + 'aggregate × field-type table: the number would depend on the SQL dialect', + ); + // …and [#22161] the set that aggregate accepts is the table's row, which + // `os explain` writes out (held to the table below). + expect(explanationOf(RULE)).toContain( + `\`avg\`: ${AGGREGATE_FIELD_TYPE_COMPATIBILITY.avg.join(', ')};`, + ); // And the way out, computed from the same table rather than prose: which // aggregates WOULD accept a `datetime`. expect(issue.hint).toContain('min'); @@ -315,7 +336,7 @@ describe('measure-aggregate-field-type-refused — the positions only authoring- expect(found[0].message).toContain('datetime'); // The message must say which object the LEAF lives on, or the author reads // the type as a claim about the dataset's own object. - expect(found[0].message).toContain('crm_account'); + expect(found[0].message).toContain('on joined object "crm_account"'); }); it('accepts an accepted pair on that same joined field', () => { @@ -419,7 +440,9 @@ describe('measure-aggregate-field-type-refused — reads the declaration, not th // The DECLARATION is named, flag included, or the author reads the verdict // as a claim about `select` that the table plainly does not make. expect(issue.message).toContain('`select` with `multiple: true`'); - expect(issue.message).toContain('none of them with `multiple: true`'); + expect(issue.message).toContain('a list stored as JSON'); + // [#22161] "none of the accepted types with `multiple: true`" is `os explain`'s now. + expect(explanationOf(RULE)).toContain('flagged that way is refused by the declaration'); // The way out is computed from the same predicate: only `count` is left. expect(issue.hint).toContain('accepts: count.'); @@ -509,9 +532,13 @@ describe('dimension-json-stored-field-refused — a dimension over a JSON-stored expect(issue.rule).toBe(DIMENSION_RULE); expect(issue.path).toBe('datasets[0].dimensions[0].field'); expect(issue.where).toBe('dataset "opportunity_metrics" › dimension "the_dimension"'); - // The author can act without opening the analytics service: the DIMENSION, - // the FIELD, its declared TYPE, the class, and what the door answers. - expect(issue.message).toContain('"the_dimension"'); + // The author can act without opening the analytics service: the DIMENSION + // (`where`, above), the FIELD, its declared TYPE, the class, and what the + // door answers. + expect(issue.message).toBe( + 'groups by field "grouped" (`json` on object "crm_opportunity"), a structured-JSON value, so every ' + + 'query grouping by it is refused (400 INVALID_FIELD)', + ); expect(issue.message).toContain('"grouped"'); expect(issue.message).toContain('`json`'); expect(issue.message).toContain('structured-JSON'); @@ -590,7 +617,7 @@ describe('dimension-json-stored-field-refused — a dimension over a JSON-stored const found = dimensionFindings(joined('json')); expect(found).toHaveLength(1); expect(found[0].message).toContain('account.hq'); - expect(found[0].message).toContain('crm_account'); + expect(found[0].message).toContain('on joined object "crm_account"'); expect(dimensionFindings(joined('text'))).toEqual([]); }); @@ -694,10 +721,10 @@ describe('the cube leg — an analyticsCubes member the analytics door refuses i expect(issue.rule).toBe(DIMENSION_RULE); expect(issue.path).toBe('analyticsCubes[0].dimensions.meta.sql'); expect(issue.where).toBe('cube "fx_cube" › dimension "meta"'); - expect(issue.message).toContain('object "fx_ledger" declares as `json`'); + expect(issue.message).toContain('(`json` on object "fx_ledger")'); expect(issue.message).toContain('structured-JSON'); expect(issue.message).toContain('400 INVALID_FIELD'); - expect(issue.message).toContain('so a query that selects it gets that refusal'); + expect(issue.message).toContain('every query grouping by it is refused'); expect(issue.hint).toContain('scalar value'); expect(validateDatasetMeasureAggregates(cubeStack({ dimensions: { name: dim('name') } }))).toEqual([]); @@ -720,7 +747,7 @@ describe('the cube leg — an analyticsCubes member the analytics door refuses i expect(json?.severity).toBe('error'); expect(json?.path).toBe('analyticsCubes[0].measures.distinct_meta.type'); expect(json?.where).toBe('cube "fx_cube" › measure "distinct_meta"'); - expect(json?.message).toContain('aggregate "count_distinct" to field "meta"'); + expect(json?.message).toContain('aggregate "count_distinct" over field "meta"'); expect(json?.message).toContain('`json`'); // The door that refuses it later is the cube's, never the dataset compile leg. expect(json?.hint).toContain('on the cube with `400 INVALID_FIELD`'); @@ -792,7 +819,7 @@ describe('the cube leg — an analyticsCubes member the analytics door refuses i it('reads a relationship path on the object the last hop reaches — the lookup\'s reference, or the join the cube declares for it', () => { // No declared join: the lookup's `reference` (`fx_account.hq` is json). const [byReference] = cubeDimensionFindings(cubeStack({ dimensions: { acct_hq: dim('account.hq') } })); - expect(byReference?.message).toContain('object "fx_account" (reached through this cube\'s join chain)'); + expect(byReference?.message).toContain('on joined object "fx_account"'); expect(cubeDimensionFindings(cubeStack({ dimensions: { acct_region: dim('account.region') } }))).toEqual([]); // A declared join wins over the reference, as at the door: keyed `account`, @@ -911,9 +938,9 @@ describe('the cube leg — every cube measure is judged by the aggregate × fiel expect(issue.rule).toBe(RULE); expect(issue.path).toBe('analyticsCubes[0].measures.sum_name.type'); expect(issue.where).toBe('cube "fx_cube" › measure "sum_name"'); - expect(issue.message).toContain('aggregate "sum" to field "name"'); - expect(issue.message).toContain('object "fx_ledger" declares as `text`'); - expect(issue.message).toContain(`"sum" accepts: ${AGGREGATE_FIELD_TYPE_COMPATIBILITY.sum.join(', ')}.`); + expect(issue.message).toContain('aggregate "sum" over field "name"'); + expect(issue.message).toContain('(`text` on object "fx_ledger")'); + expect(explanationOf(RULE)).toContain(`\`sum\`: ${AGGREGATE_FIELD_TYPE_COMPATIBILITY.sum.join(', ')};`); // The door that refuses it later is the cube's, never the dataset compile leg. expect(issue.hint).toContain('on the cube with `400 INVALID_FIELD`'); expect(issue.hint).not.toContain('DATASET_INVALID'); @@ -930,7 +957,7 @@ describe('the cube leg — every cube measure is judged by the aggregate × fiel for (const [type, column] of [['avg', 'name'], ['min', 'name'], ['max', 'name'], ['sum', 'opened_at'], ['avg', 'opened_at']]) { const [issue] = findings(cubeStack({ measures: { m: measure(type, column) } }, {}, datetime)); expect(issue?.path, `${type}(${column})`).toBe('analyticsCubes[0].measures.m.type'); - expect(issue?.message, `${type}(${column})`).toContain(`aggregate "${type}" to field "${column}"`); + expect(issue?.message, `${type}(${column})`).toContain(`aggregate "${type}" over field "${column}"`); } for (const type of ['min', 'max', 'count', 'count_distinct']) { expect(findings(cubeStack({ measures: { m: measure(type, 'opened_at') } }, {}, datetime)), type).toEqual([]); @@ -991,7 +1018,7 @@ describe('the cube leg — every cube measure is judged by the aggregate × fiel // No declared join: the lookup's `reference`, `fx_account`, where `name` is text and `revenue` a number. const [byReference] = findings(cubeStack({ measures: { s: measure('sum', 'account.name') } })); expect(byReference?.path).toBe('analyticsCubes[0].measures.s.type'); - expect(byReference?.message).toContain('object "fx_account" (reached through this cube\'s join chain)'); + expect(byReference?.message).toContain('on joined object "fx_account"'); expect(findings(cubeStack({ measures: { s: measure('sum', 'account.revenue') } }))).toEqual([]); // A declared join wins over the reference, as at the door: keyed `account`, @@ -999,7 +1026,7 @@ describe('the cube leg — every cube measure is judged by the aggregate × fiel const joined = { joins: { account: { name: 'fx_branch' } } }; const [byJoin] = findings(cubeStack({ measures: { s: measure('sum', 'account.revenue') } }, joined)); expect(byJoin?.message).toContain('object "fx_branch"'); - expect(byJoin?.message).toContain('declares as `text`'); + expect(byJoin?.message).toContain('(`text` on joined object "fx_branch")'); }); it('never hands the predicate a guess: no type, a type outside the table, the row wildcard, an unresolved column', () => { @@ -1057,3 +1084,59 @@ describe('the cube leg — every cube measure is judged by the aggregate × fiel ).toEqual([]); }); }); + +describe('[#22161] one-line verdicts — the two ids', () => { + const IDS = [MEASURE_AGGREGATE_FIELD_TYPE_REFUSED, DIMENSION_JSON_STORED_FIELD_REFUSED]; + + 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([...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); + } + }); + + // What each verdict stopped saying, which `os explain RULE_ID` now prints. + const MOVED: Record = { + [MEASURE_AGGREGATE_FIELD_TYPE_REFUSED]: ['property of the SQL dialect', 'average YEAR', 'COMPARES', '400 DATASET_INVALID', '`measure-aggregate-incoherent`'], + [DIMENSION_JSON_STORED_FIELD_REFUSED]: ['GROUP KEY', 'one groups each serialized value apart', 'before any SQL is built', 'instead of an answer'], + }; + + it('covers exactly the two ids', () => { + expect(Object.keys(MOVED).sort()).toEqual([...IDS].sort()); + }); + + it.each(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); + }); + + // `rule-explanations.ts` imports nothing, so the lists it spells out are held + // to the spec's constants here. + it("the measure explanation writes out the table's rows exactly", () => { + const text = explanationOf(MEASURE_AGGREGATE_FIELD_TYPE_REFUSED); + const table = AGGREGATE_FIELD_TYPE_COMPATIBILITY; + // `count` is every type; `count_distinct` every type but the JSON-stored ones. + expect([...table.count].sort()).toEqual([...FieldType.options].sort()); + const jsonStored = new Set([...STRUCTURED_JSON_TYPES, ...MULTI_OPTION_TYPES]); + expect([...table.count_distinct].sort()).toEqual(table.count.filter((t) => !jsonStored.has(t)).sort()); + expect(text).toContain('`count` accepts every type'); + expect(text).toContain('`count_distinct` every type but the JSON-stored ones'); + expect(text).toContain(`\`sum\`: ${table.sum.join(', ')};`); + expect(text).toContain(`\`avg\`: ${table.avg.join(', ')};`); + expect(table.max).toEqual(table.min); + expect(text).toContain(`\`min\` and \`max\`: ${table.min.join(', ')}.`); + }); + + it('both explanations name the JSON-stored types the spec declares', () => { + for (const rule of IDS) { + const text = explanationOf(rule); + expect(text, rule).toContain(`a structured-JSON type — ${[...STRUCTURED_JSON_TYPES].join(', ')} —`); + expect(text, rule).toContain(`an inherently multi option type (${[...MULTI_OPTION_TYPES].join(', ')})`); + expect(text, rule).toContain(`a multi-capable type (${[...MULTI_CAPABLE_TYPES].join(', ')}) flagged`); + } + }); +}); diff --git a/packages/lint/src/validate-dataset-measure-aggregates.ts b/packages/lint/src/validate-dataset-measure-aggregates.ts index a05e0b25100..2181da111d2 100644 --- a/packages/lint/src/validate-dataset-measure-aggregates.ts +++ b/packages/lint/src/validate-dataset-measure-aggregates.ts @@ -262,16 +262,20 @@ const strName = (v: unknown): string | undefined => typeof v === 'string' && v.length > 0 ? v : undefined; /** - * The table as a Map, built once — for the MESSAGE only. + * The table as a Map, built once — for the vocabulary skip (skip 5) and the + * words only. * * A Map rather than property access on the frozen record because a * property-key lookup also answers for `Object.prototype` members, so an * author's `aggregate: 'toString'` would read as a declared row. The VERDICT * is never taken from here: it is * {@link isAggregateCompatibleWithFieldType}'s, which is fail-closed on both - * vocabulary and shape. This Map only PRESENTS the same table — the accepted - * set for the refused aggregate, and the aggregates that would accept this - * field — so the message cannot name a set the verdict was not taken from. + * vocabulary and shape. This Map only PRESENTS the same table — the + * aggregates that would accept this field, in the hint — so the words cannot + * name a set the verdict was not taken from. [#22161] The accepted set for + * the refused aggregate moved out of the message: `os explain + * measure-aggregate-field-type-refused` writes the table's rows out, and this + * rule's test holds them to `AGGREGATE_FIELD_TYPE_COMPATIBILITY`. */ const ACCEPTED_TYPES_BY_AGGREGATE: ReadonlyMap = new Map( Object.entries(AGGREGATE_FIELD_TYPE_COMPATIBILITY).map( @@ -311,15 +315,14 @@ function shapeOf(verdict: Extract, fieldType: return { type: fieldType, multiple: verdict.meta?.multiple === true }; } -/** Which object the words say declares the leaf: the dataset's (or cube's) own, or the joined one. */ -function declarerOf( - verdict: Extract, - baseObject: string, - owner: 'dataset' | 'cube', -): string { - return verdict.object === baseObject - ? `object "${baseObject}"` - : `object "${verdict.object}" (reached through this ${owner}'s join chain)`; +/** + * Which object the words say declares the leaf: the dataset's (or cube's) own, + * or the one its join chain reaches. [#22161] One word marks the second + * ("joined"), so the verdict stays one line; the walk itself is + * `os explain`'s. + */ +function declarerOf(verdict: Extract, baseObject: string): string { + return verdict.object === baseObject ? `object "${baseObject}"` : `joined object "${verdict.object}"`; } /** @@ -341,8 +344,6 @@ interface MemberSite { where: string; /** The finding's `path`. */ path: string; - /** The member's name, as the words say it. */ - name: string; /** The column reference the author wrote: a dataset member's `field`, a cube member's `sql`. */ column: string; /** Who declares the resolved column, as the words say it ({@link declarerOf}). */ @@ -365,38 +366,26 @@ const CUBE_QUERY_DOOR = /** * [#20890] The finding for a dimension that groups by a JSON-stored column, * in one set of words for a dataset dimension and a cube dimension alike. - * `selector` names who selects the dimension: a dataset's reports and - * dashboards, a cube's queries. + * + * [#22161] One verdict sentence: the column, its declaration, its class and + * the refusal. `where` names the dimension; why a JSON value is no group key + * and which door refuses it is `os explain dimension-json-stored-field-refused`. */ function jsonStoredDimensionFinding( site: MemberSite, leaf: Extract, shape: ValueShapeFieldDef, cls: 'structured-json' | 'multi-value', - selector: string, ): DatasetMeasureAggregateFinding { - const head = - `dimension "${site.name}" groups by field "${site.column}", which ` + - `${site.declarer} declares as ${declaredAs(shape)} — `; - const door = - 'The analytics door refuses every query that groups by this dimension with ' + - `\`400 INVALID_FIELD\` before any SQL is built, so ${selector} that selects it gets ` + - 'that refusal instead of an answer.'; return { severity: 'error', rule: DIMENSION_JSON_STORED_FIELD_REFUSED, where: site.where, path: site.path, message: - cls === 'structured-json' - ? head + - 'a structured-JSON value, which analytics does not group by. A JSON document is no ' + - 'group key the SQL dialects share: one groups each serialized document apart, ' + - `another refuses the statement. ${door}` - : head + - 'a multi-value field, which analytics does not group by. A list of values is no ' + - 'group key the SQL dialects share: one groups each serialized list apart, another ' + - `refuses the statement. ${door}`, + `groups by field "${site.column}" (${declaredAs(shape)} on ${site.declarer}), ` + + `${cls === 'structured-json' ? 'a structured-JSON value' : 'a multi-value field'}, ` + + 'so every query grouping by it is refused (400 INVALID_FIELD)', hint: cls === 'structured-json' ? 'Group by a field that stores one scalar value: store the part of the document you ' + @@ -414,11 +403,15 @@ function jsonStoredDimensionFinding( * carry ({@link acceptsDeclaration} said no), in one set of words for a * dataset measure and a cube measure alike. `door` is the hint's last * sentence ({@link DATASET_COMPILE_DOOR} or {@link CUBE_QUERY_DOOR}). + * + * [#22161] One verdict sentence: the pair, the declaration and why it is + * refused. `where` names the measure; how backends answer the pair, and each + * aggregate's accepted types (the table's rows, written out), are + * `os explain measure-aggregate-field-type-refused`. */ function refusedMeasureFinding( site: MemberSite, aggregate: string, - accepted: readonly string[], fieldType: string, shape: ValueShapeFieldDef, door: string, @@ -426,24 +419,15 @@ function refusedMeasureFinding( // [#20890] The row accepts the TYPE and the declaration is what refuses: // a multi-capable field flagged `multiple: true` under `count_distinct`. const flaggedList = isAggregateCompatibleWithFieldType(aggregate, fieldType); + const pair = `aggregate "${aggregate}" over field "${site.column}" (${declaredAs(shape)} on ${site.declarer})`; return { severity: 'error', rule: MEASURE_AGGREGATE_FIELD_TYPE_REFUSED, where: site.where, path: site.path, message: flaggedList - ? `measure "${site.name}" applies aggregate "${aggregate}" to field "${site.column}", which ` + - `${site.declarer} declares as ${declaredAs(shape)} — a list of values ` + - `stored as JSON. "${aggregate}" COMPARES the stored values for equality, and no two ` + - `backends compare a JSON-stored value alike: one counts every row apart, one compares ` + - `the serialized text, another has no equality for the type and fails at query time. ` + - `"${aggregate}" accepts: ${accepted.join(', ')}, none of them with \`multiple: true\`.` - : `measure "${site.name}" applies aggregate "${aggregate}" to field "${site.column}", which ` + - `${site.declarer} declares as \`${fieldType}\`. That pair is refused by the aggregate × ` + - `field-type compatibility table in @objectstack/spec, so the number a backend returns ` + - `for it is a property of the SQL dialect rather than of the data — one coerces the ` + - `stored form and answers something plausible, another has no such function and fails ` + - `at query time. "${aggregate}" accepts: ${accepted.join(', ')}.`, + ? `${pair} is refused: a list stored as JSON, which no two backends compare alike` + : `${pair} is refused by the aggregate × field-type table: the number would depend on the SQL dialect`, hint: `Either point "${aggregate}" at a field of an accepted type, or aggregate ` + `"${site.column}" with one its ${declaredAs(shape)} ${shape.multiple === true ? 'declaration' : 'type'} accepts: ` + @@ -534,14 +518,12 @@ function cubeMemberFindings(cube: AnyRec, cubePath: string, graph: ObjectGraph): { where: `cube "${cubeName}" › dimension "${name}"`, path: `${path}.sql`, - name, column: sql, - declarer: declarerOf(column.verdict, object, 'cube'), + declarer: declarerOf(column.verdict, object), }, column.verdict, column.shape, cls, - 'a query', ), ); } @@ -568,12 +550,10 @@ function cubeMemberFindings(cube: AnyRec, cubePath: string, graph: ObjectGraph): { where: `cube "${cubeName}" › measure "${name}"`, path: `${path}.type`, - name, column: sql, - declarer: declarerOf(column.verdict, object, 'cube'), + declarer: declarerOf(column.verdict, object), }, aggregate, - accepted, column.fieldType, column.shape, CUBE_QUERY_DOOR, @@ -637,14 +617,12 @@ export function validateDatasetMeasureAggregates(stack: unknown): DatasetMeasure { where: `dataset "${dsName}" › dimension "${dimensionName}"`, path: `datasets[${di}].dimensions[${k}].field`, - name: dimensionName, column: field, - declarer: declarerOf(verdict, object, 'dataset'), + declarer: declarerOf(verdict, object), }, verdict, shape, cls, - 'a report or dashboard', ), ); }); @@ -676,12 +654,10 @@ export function validateDatasetMeasureAggregates(stack: unknown): DatasetMeasure { where: `dataset "${dsName}" › measure "${measureName}"`, path: `datasets[${di}].measures[${k}].aggregate`, - name: measureName, column: field, - declarer: declarerOf(verdict, object, 'dataset'), + declarer: declarerOf(verdict, object), }, aggregate, - accepted, fieldType, shape, DATASET_COMPILE_DOOR, diff --git a/packages/lint/src/validate-flow-trigger-readiness.test.ts b/packages/lint/src/validate-flow-trigger-readiness.test.ts index cf9e17e9476..91001ef96e5 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.test.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.test.ts @@ -3,7 +3,7 @@ import { describe, it, expect } from 'vitest'; import { TimeRelativeTriggerSchema } from '@objectstack/spec/automation'; import { - validateFlowTriggerReadiness, + validateFlowTriggerReadiness as validateFlowTriggerReadinessUnrecorded, FLOW_TRIGGER_UNKNOWN_OBJECT, FLOW_DRAFT_STATUS_AMBIGUOUS, FLOW_TRIGGER_UNKNOWN_EVENT, @@ -11,10 +11,40 @@ import { FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE, FLOW_TRIGGER_UNROUTABLE, FLOW_API_TRIGGER_SECRET_MISSING, - validateFlowApiTriggerSecret, + validateFlowApiTriggerSecret as validateFlowApiTriggerSecretUnrecorded, + schemaRefusalHead, } from './validate-flow-trigger-readiness.js'; import { AUTHORING_COMMANDS, AUTHORING_RULES, runAuthoringRules } from './authoring-rules.js'; import { runRuntimeAuthoringRules } from './runtime-gate.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding of the five never-fire 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 NEVER_FIRE_IDS: readonly string[] = [ + FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID, + FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE, + FLOW_TRIGGER_UNROUTABLE, + FLOW_API_TRIGGER_SECRET_MISSING, + FLOW_TRIGGER_UNKNOWN_EVENT, +]; +const fired: Array<{ rule: string; message: string }> = []; +const validateFlowTriggerReadiness: typeof validateFlowTriggerReadinessUnrecorded = (...args) => { + const findings = validateFlowTriggerReadinessUnrecorded(...args); + fired.push(...findings); + return findings; +}; +const validateFlowApiTriggerSecret: typeof validateFlowApiTriggerSecretUnrecorded = (...args) => { + const findings = validateFlowApiTriggerSecretUnrecorded(...args); + fired.push(...findings); + return findings; +}; + +/** The `os explain` text of `rule`, one string. */ +const explanationOf = (rule: string): string => explainRule(rule)?.paragraphs.join('\n') ?? ''; /** * [#20553] The flow-trigger family as the CLI table runs it: two registry @@ -267,32 +297,36 @@ describe('validateFlowTriggerReadiness', () => { expect(f.path).toBe('flows[0].nodes[0].config.timeRelative'); expect(f.message).toContain('config.timeRelative'); expect(f.where).toBe('flow "task_due_reminder" › start node'); - // …and carries zod's own key names: the missing `dateField`, the scalar - // `offsetDays`, and the unrecognized `field` with the schema's suggestion. - expect(f.message).toContain('dateField: Invalid input: expected string, received undefined'); - expect(f.message).toContain('offsetDays: Invalid input: expected array, received number'); - expect(f.message).toContain('Unrecognized key(s)'); - expect(f.message).toContain('`field`'); - expect(f.message).toContain('Did you mean `field` → `dateField`?'); + // …and carries zod's own words for one issue — [#22161] the unrecognized + // `field` with the schema's suggestion first, since that rename is also + // the missing `dateField` — and counts the other two (the missing + // `dateField`, the scalar `offsetDays`), which the bind-time warn lists. + expect(f.message).toBe( + 'config.timeRelative never binds, so the flow never runs: Unrecognized key(s) on this flow start ' + + "node's `config.timeRelative` descriptor: `field`. Did you mean `field` → `dateField`? (and 2 more)", + ); // The consequence, which is the whole reason this is not just a log line. expect(f.message).toMatch(/never runs/); expect(f.hint).toContain('TimeRelativeTriggerSchema'); }); it('forwards the schema verbatim rather than restating it (anti-drift pin)', () => { - // Every problem segment is `TimeRelativeTriggerSchema`'s own text, rendered - // the way `TimeRelativeTrigger.start()` renders the identical issue list at - // bind time. Derived from the schema HERE too, so this assertion tracks the - // contract instead of freezing today's wording: if the schema's message for - // a rejected descriptor changes, the rule's output changes with it and this - // test keeps passing — but a hand-written copy in the rule would not. + // The quoted issue is `TimeRelativeTriggerSchema`'s own text — [#22161] + // cut to its verdict (the key and the rename), the cut the module's + // `schemaRefusalHead` makes for every forwarded refusal. Derived from the + // schema HERE too, so this assertion tracks the contract instead of + // freezing today's wording: if the schema's message for a rejected + // descriptor changes, the rule's output changes with it and this test + // keeps passing — but a hand-written copy in the rule would not. const parsed = TimeRelativeTriggerSchema.safeParse(badDescriptor); expect(parsed.success).toBe(false); - const expected = parsed.error!.issues - .map((i) => `${i.path.join('.') || '(root)'}: ${i.message.replace(/\s+/g, ' ').trim()}`) - .join('; '); + const issues = parsed.error!.issues; + const quoted = issues.find((i) => i.code === 'unrecognized_keys')!; + const head = schemaRefusalHead(quoted.message); + // The head is a PREFIX of the schema's own sentence — nothing reworded. + expect(quoted.message.replace(/\s+/g, ' ').startsWith(head)).toBe(true); const [f] = validateFlowTriggerReadiness(timeRelativeStack(badDescriptor)); - expect(f.message).toContain(expected); + expect(f.message).toContain(`: ${head} (and ${issues.length - 1} more)`); // Single-line, so the CLI's bulleted list stays aligned (the schema's // guidance bullets arrive with newlines in them). expect(f.message).not.toContain('\n'); @@ -353,7 +387,7 @@ describe('validateFlowTriggerReadiness', () => { expect(findings[0].message).not.toContain('dateField'); // …and the shape warning only about the shape (it never echoes the name). expect(findings[1].message).toContain('dateField'); - expect(findings[1].message).not.toContain("'contract'"); + expect(findings[1].message).not.toContain('contract'); }); it('forwards the exactly-one-window rule (both modes, and neither)', () => { @@ -380,7 +414,18 @@ describe('validateFlowTriggerReadiness', () => { }), ); expect(findings.map((f) => f.rule)).toEqual([FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID]); - expect(findings[0].message).toContain('`schedule` is a sibling of `timeRelative`'); + // [#22161] The verdict names the key; the schema's wrong-layer + // prescription for it is longer than one line, so `os explain` carries it… + expect(findings[0].message).toContain("descriptor: `schedule`"); + expect(explanationOf(FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID)).toContain( + '`schedule` is a sibling of `timeRelative` on the START node\'s `config`', + ); + // …and the schema still answers this key with that prescription, so the + // explanation is not describing a guidance entry the spec has dropped. + const refusal = TimeRelativeTriggerSchema.safeParse({ + object: 'task', dateField: 'due_at', withinDays: 3, schedule: { type: 'cron', expression: '0 8 * * *' }, + }); + expect(refusal.error!.issues.map((i) => i.message).join('\n')).toContain('`schedule` is a sibling of `timeRelative`'); }); it('flags an array descriptor — the engine routes it, so the trigger refuses it', () => { @@ -456,12 +501,15 @@ describe('validateFlowTriggerReadiness', () => { expect(f.message).toContain('(a string)'); expect(f.message).toContain('config.timeRelative'); // The consequence chain, which is the whole reason this is not a nit: - // never routed → sweep never installed → this flow never fires at all, - // and no layer says so. - expect(f.message).toMatch(/never routed/); - expect(f.message).toMatch(/sweep is never\s+installed/); - expect(f.message).toMatch(/binds to NOTHING and\s+never fires/); - expect(f.message).toMatch(/zero diagnostics at any layer/); + // not an object → sweep never installed → this flow never fires at all… + expect(f.message).toBe( + 'has config.timeRelative = "daily" (a string), not a descriptor object, so the sweep is never ' + + 'installed and with no other trigger declared the flow binds to nothing and never fires', + ); + // …and [#22161] that no layer says so, and why the engine never routes + // it, is `os explain`'s. + expect(explanationOf(FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE)).toMatch(/is never routed to the sweep/); + expect(explanationOf(FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE)).toMatch(/nothing at any layer says a word/); // Single-line, like every other finding this rule emits (the CLI prints // `• where: message` on one line). expect(f.message).not.toContain('\n'); @@ -528,10 +576,14 @@ describe('validateFlowTriggerReadiness', () => { const [f] = validateFlowTriggerReadiness(withSchedule); expect(f.rule).toBe(FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE); expect(f.message).toContain('config.schedule'); - expect(f.message).toMatch(/silently DROPPED/); + expect(f.message).toMatch(/dropping the descriptor/); expect(f.message).not.toMatch(/never fires/); // …and the certain half is still stated. - expect(f.message).toMatch(/never routed/); + expect(f.message).toMatch(/sweep is never installed/); + // The terms it fires on are `os explain`'s. + expect(explanationOf(FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE)).toContain( + 'once per firing, with no record on the context', + ); }); it('names each fallback the engine would actually reach', () => { @@ -806,12 +858,17 @@ describe('validateFlowTriggerReadiness', () => { // whose flow DOES bind and DOES get a bind-time warn. The distinguishing // claim is that nothing names this one, and the rule has to make it. const [f] = validateFlowTriggerReadiness(unroutable({ triggerType: 'onCreate' })); - expect(f.message).toMatch(/record_change/); - expect(f.message).toMatch(/never fires/i); - expect(f.message).toMatch(/audit/i); - // …and it must NOT overclaim: the flow count line does move, so the - // message says "nothing NAMES it" rather than "no signal anywhere". - expect(f.message).toMatch(/count/i); + expect(f.message).toBe( + "declares type: 'record_change' but its start node's triggerType is 'onCreate', which the engine " + + 'routes to no trigger, so the flow is demoted to a manual one and never fires', + ); + // [#22161] The distinguishing claim — nothing NAMES this flow — is the + // id's `os explain` text now… + const explanation = explanationOf(FLOW_TRIGGER_UNROUTABLE); + expect(explanation).toMatch(/audit/i); + // …and it must NOT overclaim: the flow count line does move, so it says + // "nothing names it" rather than "no signal anywhere". + expect(explanation).toMatch(/count/i); expect(f.hint).toMatch(/record-\{before,after\}/); expect(f.hint).toMatch(/autolaunched/); }); @@ -875,11 +932,12 @@ describe('validateFlowTriggerReadiness', () => { expect(absent.rule).toBe(present.rule); expect(absent.severity).toBe(present.severity); expect(absent.path).toBe(present.path); - // Both still carry the measured audit claim from 1f's comment. - expect(absent.message).toMatch(/record_change/); - expect(absent.message).toMatch(/never fires/i); - expect(absent.message).toMatch(/audit/i); - expect(absent.message).toMatch(/count/i); + // Both state the same outcome; the measured audit claim from 1f's comment + // is their one `os explain` entry [#22161]. + for (const f of [present, absent]) { + expect(f.message).toMatch(/record_change/); + expect(f.message).toMatch(/demoted to a manual one and never fires$/); + } }); describe('does NOT flag (each paired with the mutation that makes it fire)', () => { @@ -1589,3 +1647,70 @@ describe('validateFlowTriggerReadiness', () => { expect(findings).toEqual([]); }); }); + +describe('[#22161] one-line verdicts — the never-fire family', () => { + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: each of the five ids fired at least once, so + // the shape assertion below cannot pass over an empty record. + const firedIds = new Set(fired.map((f) => f.rule)); + for (const id of NEVER_FIRE_IDS) expect(firedIds.has(id), `${id} fired`).toBe(true); + for (const f of fired.filter((x) => NEVER_FIRE_IDS.includes(x.rule))) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + describe('schemaRefusalHead — the verdict half of a forwarded spec refusal', () => { + const SURFACE = "Unrecognized key(s) on this flow start node's `config.timeRelative` descriptor: `field`."; + const HISTORY = 'Until this shape was closed, these were dropped silently — the descriptor still parsed.'; + + it('keeps the key sentence and the rename question, and drops the history', () => { + expect(schemaRefusalHead(`${SURFACE} Did you mean \`field\` → \`dateField\`? ${HISTORY}`)).toBe( + `${SURFACE} Did you mean \`field\` → \`dateField\`?`, + ); + }); + + it('drops a prescription bullet and the history after it, keeping the key', () => { + expect(schemaRefusalHead(`${SURFACE}\n • \`field\` is a sibling of the descriptor. ${HISTORY}`)).toBe( + SURFACE.slice(0, -1), + ); + }); + + it('quotes a one-sentence refusal whole', () => { + for (const message of [ + 'Invalid input: expected string, received undefined', + 'Provide exactly one of `withinDays` (range mode) or `offsetDays` (offset mode).', + ]) { + expect(schemaRefusalHead(message)).toBe(message); + } + }); + }); + + // What each verdict stopped saying, which `os explain RULE_ID` now prints. + const MOVED: Record = { + [FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID]: ['TimeRelativeTriggerSchema', 'one warn in the server log', 'the bind-time warn prints the whole list', 'FLOW-level key', 'partition'], + [FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE]: ['`typeof` says \'object\'', 'nothing at any layer says a word', 'once per firing, with no record on the context', 'silently dropped', '`resolveTriggerBinding`'], + [FLOW_TRIGGER_UNROUTABLE]: ['`resolveTriggerBinding`', 'manual — nothing to bind', 'one higher than its bound count', 'RESOLVED type', 'record-{before,after}-{create,insert,update,delete,write}'], + [FLOW_TRIGGER_UNKNOWN_EVENT]: ['record-{before,after}-{create,insert,update,delete,write}', 'bind-time warn', 'multi-event shape', '`record-after-write`'], + [FLOW_API_TRIGGER_SECRET_MISSING]: ['ADR-0041', 'HMAC', '`registerFlow`', 'whatever the flow\'s `status`', 'never receives a post', 'withholds'], + }; + + it('covers exactly the five ids', () => { + expect(Object.keys(MOVED).sort()).toEqual([...NEVER_FIRE_IDS].sort()); + }); + + it.each([...NEVER_FIRE_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 runAs prescription the explanation names is still the schema's answer for that key", () => { + const refusal = TimeRelativeTriggerSchema.safeParse({ + object: 'task', dateField: 'due_at', withinDays: 3, runAs: 'system', + }); + expect(refusal.success).toBe(false); + expect(refusal.error!.issues.map((i) => i.message).join('\n')).toContain('`runAs` is a FLOW-level key'); + expect(explanationOf(FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID)).toContain('`runAs` is a FLOW-level key'); + }); +}); diff --git a/packages/lint/src/validate-flow-trigger-readiness.ts b/packages/lint/src/validate-flow-trigger-readiness.ts index 41e4be0bce5..94be00fdba1 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.ts @@ -29,7 +29,8 @@ // judge it, and until this rule the only place it ran was BIND time, so an // unparseable descriptor produced one warn in a server log and nothing at // all in `os validate`. The judgement is not re-implemented here: the rule -// runs that schema and forwards its issue list verbatim. +// runs that schema and forwards its refusal — one issue, to its verdict, +// and a count of the rest (see `descriptorRefusal`). // // 4. A `config.timeRelative` that is not an OBJECT at all — `timeRelative: // 'daily'` (#5647). One step worse than 3, and it needs its own criterion @@ -258,6 +259,12 @@ export const FLOW_API_TRIGGER_SECRET_MISSING = 'flow-api-trigger-secret-missing' type AnyRec = Record; +/** + * [#22161] The outcome every never-fire verdict in this file closes on. Why + * nothing reports it at run time is each id's `os explain` entry. + */ +const NEVER_FIRES = 'never fires'; + /** * The record-change trigger fires only for a `triggerType` matching this exact * grammar — the same set its `triggerTypeToHookEvents` maps to ObjectQL hooks. @@ -340,6 +347,50 @@ function isArrayRecordTriggerType(config: AnyRec): boolean { ); } +/** + * [#22161] The verdict half of a refusal `@objectstack/spec` wrote: its first + * sentence — which key or value is wrong — and, when the next sentence is the + * schema's rename (`Did you mean \`field\` → \`dateField\`?`), that question + * too. A strict shape orders its refusal "which key → how to fix it → why it + * used to be silent" so a one-line reader can stop early; what follows the + * rename — a wrong-layer prescription bullet, then the sentence on why the key + * used to be dropped — is the reasoning, and each id that forwards one names + * those keys in its `os explain` entry. Whitespace is collapsed first (a + * prescription arrives on its own bulleted line). A refusal of one sentence is + * quoted whole. + * + * Exported for `validate-react-page-props.ts`, which forwards the same + * producer's refusals for `` and `aggregate`. + */ +export function schemaRefusalHead(message: string): string { + const text = message.replace(/\s+/g, ' ').trim(); + const cut = text.indexOf('. '); + if (cut === -1) return text; + const rest = text.slice(cut + 2); + const rename = rest.startsWith('Did you mean ') ? rest.slice(0, rest.indexOf('?') + 1) : ''; + return rename.endsWith('?') ? `${text.slice(0, cut + 1)} ${rename}` : text.slice(0, cut); +} + +/** The issues after the first, which a verdict counts rather than quotes. */ +function moreIssues(count: number): string { + return count > 1 ? ` (and ${count - 1} more)` : ''; +} + +/** + * [#22161] `TimeRelativeTriggerSchema`'s refusal of a descriptor, as the + * verdict quotes it: ONE issue, at its path (none for the descriptor itself), + * cut to its head ({@link schemaRefusalHead}), and a count of the rest. An + * unrecognized key is quoted first when there is one, because its rename + * usually explains the others — a misspelled `field` is also the missing + * `dateField`. The full list is what the trigger's bind-time warn prints. + */ +function descriptorRefusal(issues: ReadonlyArray<{ code: string; path: ReadonlyArray; message: string }>): string { + const first = issues.find((i) => i.code === 'unrecognized_keys') ?? issues[0]; + if (!first) return 'refused'; + const at = first.path.length ? `${first.path.join('.')}: ` : ''; + return `${at}${schemaRefusalHead(first.message)}${moreIssues(issues.length)}`; +} + /** The start node of a flow definition, if any. */ function startNodeOf(flow: AnyRec): { node: AnyRec; index: number } | undefined { const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : []; @@ -472,14 +523,6 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines // when the descriptor gains a key. const parsed = TimeRelativeTriggerSchema.safeParse(tr); if (!parsed.success) { - // Rendered exactly as the bind-time warn renders the same issue list - // (`TimeRelativeTrigger.start`), so an author who sees both channels sees - // one story told twice, not two dialects. Whitespace is collapsed because - // a finding is one line here (the CLI prints `• where: message`) while a - // log line is free to wrap — the schema's guidance bullets carry newlines. - const problems = parsed.error.issues - .map((i) => `${i.path.join('.') || '(root)'}: ${i.message.replace(/\s+/g, ' ').trim()}`) - .join('; '); findings.push({ // `error` (#5762): the verdict is `TimeRelativeTriggerSchema`'s, and it // is the same schema the trigger safeParses at bind time. A descriptor @@ -490,10 +533,11 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines rule: FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID, where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.timeRelative`, - message: - `has a config.timeRelative descriptor the time-relative trigger REFUSES at bind time, so the ` + - `sweep is never installed — the flow declares a time-relative trigger and then never runs ` + - `(the only trace is one warn in the server log). ${problems}`, + // [#22161] One verdict sentence, quoting the schema's own words for one + // issue: the bind-time warn (`TimeRelativeTrigger.start`) prints the + // whole list, and why the sweep is never installed is + // `os explain flow-time-relative-descriptor-invalid`. + message: `config.timeRelative never binds, so the flow never runs: ${descriptorRefusal(parsed.error.issues)}`, // [#22161] `hint` is the CLI's `fix:` line: the instruction first, the // reason it is sufficient after it. hint: @@ -525,9 +569,11 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines rule: FLOW_TRIGGER_UNKNOWN_EVENT, where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`, + // [#22161] One verdict sentence; the token grammar and the silence are + // `os explain flow-trigger-unknown-event`. message: - `triggerType '${triggerType}' is not a recognized record trigger — the flow binds to the ` + - `record-change trigger but never fires (the runtime stays silent about it).`, + `triggerType '${triggerType}' is not a recognized record trigger, so the flow binds to the ` + + `record-change trigger and ${NEVER_FIRES}`, hint: `Use record-{before,after}-{create,update,delete,write}. 'write' fires on create OR update in one ` + `flow; create/insert are synonyms. There is no "any change" token — pick the specific event(s).`, @@ -553,8 +599,8 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`, message: - `triggerType is an array (${JSON.stringify(config.triggerType)}), which is not supported — a start ` + - `node takes a single trigger event, so the flow binds to nothing and never fires (the runtime stays silent about it).`, + `triggerType is an array (${JSON.stringify(config.triggerType)}), but a start node takes one ` + + `trigger event, so the flow binds to nothing and ${NEVER_FIRES}`, hint: `Use one triggerType string. For "created or updated" use record-after-write (one flow, both events). ` + `For any other combination, author one flow per event — multi-event arrays are deferred until ` + @@ -604,17 +650,16 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines isRecordTriggered || isArrayRecordTriggered ? 'its record-change trigger' : config.schedule != null || flow.type === 'schedule' - ? 'its plain `config.schedule` cadence' + ? 'its `config.schedule` cadence' : triggerType === 'api' || flow.type === 'api' ? 'its api trigger' : undefined; + // [#22161] The consequence closes the one verdict sentence; what each + // means — the firing terms of the fallback trigger, and the silence at + // every layer — is `os explain flow-time-relative-descriptor-unroutable`. const consequence = fallback - ? `The flow still binds through ${fallback}, so the descriptor is silently DROPPED — it fires on ` + - `that trigger's terms (once per firing, with no record on the context) instead of once per ` + - `matching record, and nothing anywhere reports the difference.` - : `Nothing else on this start node declares a trigger either, so the flow binds to NOTHING and ` + - `never fires — with zero diagnostics at any layer, not even the one bind-time warn a ` + - `descriptor that IS an object gets when the trigger refuses it.`; + ? `the flow fires through ${fallback}, dropping the descriptor` + : `with no other trigger declared the flow binds to nothing and ${NEVER_FIRES}`; findings.push({ // `error` (#5762). The criterion IS the engine's routing predicate, so a // value that fails it is not routed to the time-relative trigger by any @@ -628,10 +673,8 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.timeRelative`, message: - `has config.timeRelative = ${renderNonObject(config.timeRelative)}, which is not the descriptor ` + - `OBJECT this slot takes — the engine routes a flow to the time-relative sweep only when ` + - `config.timeRelative is an object, so this one is never routed there and the sweep is never ` + - `installed. ${consequence}`, + `has config.timeRelative = ${renderNonObject(config.timeRelative)}, not a descriptor object, so ` + + `the sweep is never installed and ${consequence}`, hint: `config.timeRelative describes WHICH records to sweep — an object: ` + `{ object, dateField, and exactly one of withinDays | offsetDays } (plus optional filter / ` + @@ -732,16 +775,15 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines rule: FLOW_TRIGGER_UNROUTABLE, where: `flow "${flowName}" › start node`, path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`, + // [#22161] One verdict sentence; the routing chain and why no runtime + // channel names the flow are `os explain flow-trigger-unroutable`. message: `declares type: 'record_change' but ` + (hasTriggerType ? `its start node's triggerType is ${renderTriggerToken(config.triggerType)}, which the engine ` + - `routes to NO trigger` - : `its start node has no triggerType at all, so there is nothing for the engine to route`) + - ` — it binds a record-change flow only for a token starting with 'record-', so this flow is demoted ` + - `to a manual one and never fires. Nothing NAMES it: the unbound-flow audit resolves the same binding ` + - `and skips the flow as "manual — nothing to bind", so neither the boot warning nor the startup ` + - `summary lists it; the only trace is the banner's flow count being one higher than its bound count.`, + `routes to no trigger` + : `its start node has no triggerType at all, so the engine routes it to no trigger`) + + `, so the flow is demoted to a manual one and ${NEVER_FIRES}`, hint: `Use record-{before,after}-{create,update,delete,write} ('write' is create OR update in one flow; ` + `create/insert are synonyms). If the flow really is launched by hand or from a screen, ` + @@ -885,10 +927,11 @@ export function validateFlowApiTriggerSecret( rule: FLOW_API_TRIGGER_SECRET_MISSING, where: start ? `flow "${flowName}" › start node` : `flow "${flowName}"`, path: secretPath ?? `flows[${flowIndex}].nodes`, + // [#22161] One verdict sentence; why the engine refuses a secretless + // inbound hook, and where, is `os explain flow-api-trigger-secret-missing`. message: - `binds the inbound api trigger (${binds.join(' and ')}) but ${secretProblem}. An inbound hook ` + - `is armed only with a per-flow secret that every post is HMAC-verified against (ADR-0041), so the ` + - `automation engine refuses to register this flow, whatever its status, and it never receives a post.`, + `binds the inbound api trigger (${binds.join(' and ')}) but ${secretProblem}, so the engine ` + + `refuses to register the flow`, hint: `Set a non-blank string config.secret on the start node, and sign each post with it: the ` + `x-objectstack-signature header carries 'sha256=' and the hex HMAC-SHA256 of the raw body. A flow that is ` + diff --git a/packages/lint/src/validate-react-page-props.test.ts b/packages/lint/src/validate-react-page-props.test.ts index 1923d9e102f..73ddfc69706 100644 --- a/packages/lint/src/validate-react-page-props.test.ts +++ b/packages/lint/src/validate-react-page-props.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; import { - validateReactPageProps, + validateReactPageProps as validateReactPagePropsUnrecorded, REACT_CHART_FIELD_UNKNOWN, REACT_CHART_FIELD_UNPROVISIONED, REACT_CHART_AGGREGATE_INVALID, @@ -25,7 +25,31 @@ import { PAGE_FIELD_UNKNOWN, PAGE_FIELD_UNPROVISIONED } from './validate-page-fi // The gate PARSES `ChartAggregateSchema` since #5020, so the function // vocabulary lives in the schema and nowhere in the rule. Imported here to pin // the test table against it — see the `aggregate` block near the bottom. -import { ChartAggregateFunctionSchema } from '@objectstack/spec/ui'; +import { ChartAggregateFunctionSchema, ChartAggregateSchema, ChartDrillDownSchema } from '@objectstack/spec/ui'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding of the five ids below is one verdict sentence; the +// reasoning it used to carry is the id's `os explain` entry. Every call in this +// file records what it fired, and the last cases hold each recorded verdict of +// those ids 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 ONE_LINE_IDS: readonly string[] = [ + REACT_CHART_DRILLDOWN_INVALID, + REACT_CHART_AGGREGATE_INVALID, + REACT_CHART_FIELD_UNPROVISIONED, + REACT_BLOCK_NEEDS_RECORD_CONTEXT, + REACT_PAGE_SOURCE_UNPARSEABLE, +]; +const fired: Array<{ rule: string; message: string }> = []; +const validateReactPageProps: typeof validateReactPagePropsUnrecorded = (...args) => { + const findings = validateReactPagePropsUnrecorded(...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 page = (source: string) => ({ pages: [{ name: 'p', kind: 'react', source }] }); @@ -791,6 +815,8 @@ describe('validateReactPageProps — record:* blocks need a context this surface expect(rejected[0].severity).toBe('error'); expect(rejected[0].where).toBe(`page "p" › <${tag}>`); expect(rejected[0].message).toContain('renders empty'); + // [#22161] Why binding it does not help is `os explain`'s. + expect(explanationOf(REACT_BLOCK_NEEDS_RECORD_CONTEXT)).toContain('does not read its `objectName` or `recordId`'); } }); @@ -1002,6 +1028,10 @@ describe('validateReactPageProps — (#5022)', () => { expect(hit, 'the unknown key must be reported').toBeTruthy(); expect(hit!.message).toContain('chart drill-down block'); expect(hit!.message, 'the schema’s suggester reaches the author through the gate').toContain('`maxrows` → `maxRows`'); + // [#22161] Cut to its verdict: the key and the rename, not the history after it. + expect(hit!.message).toBe( + 'drillDown: Unrecognized key(s) on this chart drill-down block: `maxrows`. Did you mean `maxrows` → `maxRows`?', + ); expect(hit!.severity).toBe('error'); }); @@ -1027,17 +1057,37 @@ describe('validateReactPageProps — (#5022)', () => { expect(hit!.severity).toBe('error'); }); + // [#22161] The verdict names the key; the schema's reason for a key that is + // real on another widget is longer than one line, so `os explain` names it — + // and each pin below also holds that the schema still answers the key with a + // prescription, so the explanation never describes a dropped guidance entry. + const drillRefusal = (block: Record) => { + const parsed = ChartDrillDownSchema.safeParse(block); + expect(parsed.success).toBe(false); + return parsed.error!.issues.map((i) => i.message).join('\n'); + }; + it('rejects a key that belongs to another widget, with the reason rather than a rename', () => { const f = validateReactPageProps(drill(`{ mode: 'record' }`)); const hit = f.find((x) => x.rule === REACT_CHART_DRILLDOWN_INVALID); - expect(hit!.message).toContain('object-data-table'); + expect(hit!.message).toBe('drillDown: Unrecognized key(s) on this chart drill-down block: `mode`'); + expect(drillRefusal({ mode: 'record' })).toContain('object-data-table'); + expect(explanationOf(REACT_CHART_DRILLDOWN_INVALID)).toContain('`mode` is objectui\'s `object-data-table` drill key'); }); it('rejects the report near-key spelling and names the type difference', () => { const f = validateReactPageProps(drill(`{ drilldown: true }`)); const hit = f.find((x) => x.rule === REACT_CHART_DRILLDOWN_INVALID); - expect(hit!.message).toContain('ReportSchema.drilldown'); - expect(hit!.message).toContain('BOOLEAN'); + expect(hit!.message).toBe('drillDown: Unrecognized key(s) on this chart drill-down block: `drilldown`'); + expect(drillRefusal({ drilldown: true })).toContain('ReportSchema.drilldown'); + expect(explanationOf(REACT_CHART_DRILLDOWN_INVALID)).toContain('`drilldown` (all lowercase) is `ReportSchema.drilldown`, a boolean'); + }); + + it('every other key the explanation names is one the schema answers with a prescription', () => { + for (const key of ['report', 'view', 'sort']) { + expect(drillRefusal({ [key]: true }), key).toMatch(/\n {2}• /); + expect(explanationOf(REACT_CHART_DRILLDOWN_INVALID), key).toContain(`\`${key}\``); + } }); it('rejects a non-object drillDown and shows the two shapes that work', () => { @@ -1219,7 +1269,12 @@ describe('validateReactPageProps — PARSED (#5020)', () const hit = aggFindings(`{ function: 'count', groupBy: 42 }`); expect(hit[0].severity).toBe('error'); expect(hit[0].message).toContain('aggregate.groupBy'); - expect(hit[0].message).toContain('received 42'); + // [#22161] Every form's complaint is about the whole value's type here, so + // the verdict keeps them all — each naming what it received. + expect(hit[0].message).toBe( + 'aggregate.groupBy matches no accepted form: (1) Invalid input: expected string, received number ' + + '(2) Invalid input: expected object, received number', + ); }); // ── The zod-4 union collapse, unpacked ───────────────────────────────── @@ -1232,9 +1287,14 @@ describe('validateReactPageProps — PARSED (#5020)', () // can act on, which is the class of diagnostic this gate exists to replace. const hit = aggFindings(`{ function: 'count', groupBy: { dateGranularity: 'day' } }`); expect(hit[0].severity).toBe('error'); - expect(hit[0].message).toContain('no accepted form matched'); - expect(hit[0].message, "the structured arm's own complaint").toContain('field'); - expect(hit[0].message, 'the bare-field-name arm reports too').toContain('expected string'); + expect(hit[0].message).toContain('matches no accepted form'); + // [#22161] The structured arm's own complaint, at its key. The bare-field- + // name arm's — "expected string, received object" — says only that the + // value is an object, which the author wrote on purpose, so the one-line + // verdict keeps the arm that speaks to the content. + expect(hit[0].message).toBe( + 'aggregate.groupBy matches no accepted form: field — Invalid input: expected string, received undefined', + ); expect(hit[0].message, 'the collapsed message must not be the whole report').not.toBe( 'aggregate.groupBy: Invalid input', ); @@ -1265,6 +1325,10 @@ describe('validateReactPageProps — PARSED (#5020)', () expect(hit[0].message).toContain('Unrecognized key'); expect(hit[0].message, "the schema's own surface name").toContain('this chart aggregate'); expect(hit[0].message, 'the rename').toContain('`groupby` → `groupBy`'); + // [#22161] Cut to its verdict: the history sentence and the echoed value go. + expect(hit[0].message).toBe( + 'aggregate: Unrecognized key(s) on this chart aggregate: `groupby`. Did you mean `groupby` → `groupBy`?', + ); // The parse still runs on everything else, and the two findings coexist — // the #5046 trap in reverse: a green "it rejects" proves nothing unless the @@ -1290,7 +1354,7 @@ describe('validateReactPageProps — PARSED (#5020)', () const hit = aggFindings(`{ function: 'count', groupBy: { field: 'closed_at', dateGranularty: 'month' } }`); expect(hit.length).toBe(1); expect(hit[0].severity).toBe('error'); - expect(hit[0].message, 'the collapsed union was unpacked').toContain('no accepted form matched'); + expect(hit[0].message, 'the collapsed union was unpacked').toContain('matches no accepted form'); expect(hit[0].message, "the strict arm's surface").toContain('this chart groupBy'); expect(hit[0].message, 'and its rename').toContain('`dateGranularty` → `dateGranularity`'); expect(hit[0].message, 'the bare collapsed message must not be the whole report').not.toBe( @@ -1345,8 +1409,12 @@ describe('validateReactPageProps — unprovisioned injected anchors (#8340)', () expect(warned).toHaveLength(1); expect(warned[0].severity).toBe('warning'); expect(warned[0].message).toContain('aggregate.groupBy "owner_id"'); - expect(warned[0].message).toContain('external object (ADR-0015)'); - expect(warned[0].message).toContain('one empty bucket'); + expect(warned[0].message).toBe( + 'aggregate.groupBy "owner_id": \'owner_id\' is an injected column with no storage on external object ' + + "'ext_customer', so the chart groups everything into one empty bucket", + ); + // [#22161] ADR-0015's federation is `os explain`'s. + expect(explanationOf(REACT_CHART_FIELD_UNPROVISIONED)).toContain('ADR-0015 external object'); expect(warned[0].hint).toContain('columnMap'); }); @@ -1585,3 +1653,53 @@ describe('an unparseable react source is reported, not scored clean (#10653)', ( expect(validateReactPageProps(page(' \n '))).toEqual([]); }); }); + +describe('[#22161] one-line verdicts — the five react-page ids', () => { + it('every verdict the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: each of the five ids fired at least once, so + // the shape assertion below cannot pass over an empty record. + const firedIds = new Set(fired.map((f) => f.rule)); + for (const id of ONE_LINE_IDS) expect(firedIds.has(id), `${id} fired`).toBe(true); + for (const f of fired.filter((x) => ONE_LINE_IDS.includes(x.rule))) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + // What each verdict stopped saying, which `os explain RULE_ID` now prints. + const MOVED: Record = { + [REACT_CHART_DRILLDOWN_INVALID]: ['ChartDrillDownSchema', 'ignored at click time', 'fully static literal', '`ReportSchema.drilldown`'], + [REACT_CHART_AGGREGATE_INVALID]: ['ChartAggregateSchema', 'single ungrouped point', 'whole value is another type', '`dateGranularity` and `alias` go inside', 'object-metric block'], + [REACT_CHART_FIELD_UNPROVISIONED]: ['ADR-0015', 'without provisioning a column', 'The query does not fail', 'vouches for'], + [REACT_BLOCK_NEEDS_RECORD_CONTEXT]: ['never mounts that context', '`objectName` or `recordId`', 'shadows the injected one'], + [REACT_PAGE_SOURCE_UNPARSEABLE]: ['partially recovered', 'Sucrase', '`react-page-syntax`', '`0755`', 'not a second syntax verdict'], + }; + + it('covers exactly the five ids', () => { + expect(Object.keys(MOVED).sort()).toEqual([...ONE_LINE_IDS].sort()); + }); + + it.each([...ONE_LINE_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('every wrong-layer key the aggregate explanation names is one the schema answers with a prescription', () => { + const refusalOf = (aggregate: Record) => { + const parsed = ChartAggregateSchema.safeParse(aggregate); + expect(parsed.success).toBe(false); + // A key inside `groupBy` is refused inside the union's object arm. + return JSON.stringify(parsed.error!.issues); + }; + const text = explanationOf(REACT_CHART_AGGREGATE_INVALID); + for (const key of ['dateGranularity', 'alias', 'filter', 'objectName', 'measures']) { + expect(refusalOf({ function: 'count', groupBy: 'status', [key]: 'x' }), key).toContain('•'); + expect(text, key).toContain(`\`${key}\``); + } + for (const key of ['function', 'groupBy']) { + expect(refusalOf({ function: 'count', groupBy: { field: 'status', [key]: 'x' } }), key).toContain('•'); + expect(text, key).toContain(`\`${key}\``); + } + }); +}); diff --git a/packages/lint/src/validate-react-page-props.ts b/packages/lint/src/validate-react-page-props.ts index 23d223d27bb..9f1728087ba 100644 --- a/packages/lint/src/validate-react-page-props.ts +++ b/packages/lint/src/validate-react-page-props.ts @@ -62,14 +62,17 @@ import { } from './validate-page-field-bindings.js'; // #5020's zod-rejection renderer, shared with the SDUI component-props gate // since #5068 — see `zod-issue-format.ts` for why one copy matters here. -import { describeIssue } from './zod-issue-format.js'; +import { describeIssue, type LintZodIssue } from './zod-issue-format.js'; import { createSourceFileChecked, describeParseFailure, PARSE_FAILURE_HINT } from './checked-parse.js'; +// [#22161] The verdict half of a forwarded `@objectstack/spec` refusal — one +// cut for every rule that forwards the same producer's words. +import { schemaRefusalHead } from './validate-flow-trigger-readiness.js'; import { SYSTEM_FIELDS, indexUnprovisionedAnchors, - unprovisionedAnchorCause, unprovisionedAnchorHint, + unprovisionedAnchorVerdict, } from './system-fields.js'; import { collectionEntries } from './collection-entries.js'; @@ -370,7 +373,10 @@ function checkChartDrillDown( push( 'error', REACT_CHART_DRILLDOWN_INVALID, - `${at}: ${issue.message}`, + // [#22161] The schema's refusal to its verdict: which key or value, and + // its rename. What the schema adds after that is + // `os explain react-chart-drilldown-invalid`'s. + `${at}: ${schemaRefusalHead(issue.message)}`, 'The drill config is declared by ChartDrillDownSchema (@objectstack/spec/ui) — the rejection above carries the fix.', ); } @@ -471,12 +477,44 @@ function checkChartAggregate( push( 'error', REACT_CHART_AGGREGATE_INVALID, - `${at}: ${describeIssue(issue, raw)}`, + // [#22161] One verdict line: the schema's refusal cut to its verdict, and + // a union's arms to the ones that speak to the value's content. + issue.code === 'invalid_union' + ? `${at} matches no accepted form: ${unionRefusal(issue as LintZodIssue, raw)}` + : `${at}: ${schemaRefusalHead(describeIssue(issue, raw))}`, 'The aggregate is declared by ChartAggregateSchema (@objectstack/spec/ui) — the rejection above carries the fix.', ); } } +/** + * [#22161] A union's refusal, as the verdict quotes it. `describeIssue` lists + * every arm's complaint after "no accepted form matched"; the verdict keeps + * the arms that speak to the value's CONTENT and drops an arm whose one + * complaint is that the whole value is another type — the bare-field-name + * arm's `expected string, received object`, for an object — unless every arm + * says only that. Each kept complaint is rendered the way `describeIssue` + * renders an arm's (its path, then the issue), cut to its verdict + * ({@link schemaRefusalHead}). + */ +function unionRefusal(issue: LintZodIssue, raw: unknown): string { + const arms = (issue.errors ?? []).filter((arm) => arm.length > 0); + const wholeValueType = (arm: ReadonlyArray) => + arm.length === 1 && arm[0]!.code === 'invalid_type' && arm[0]!.path.length === 0; + const content = arms.filter((arm) => !wholeValueType(arm)); + const kept = content.length > 0 ? content : arms; + const texts = kept.map((arm) => + arm + .map((inner) => { + const where = inner.path.length ? `${inner.path.join('.')} — ` : ''; + return `${where}${schemaRefusalHead(describeIssue(inner, raw, 1))}`; + }) + .join('; '), + ); + if (texts.length === 0) return schemaRefusalHead(describeIssue(issue, raw)); + return texts.length === 1 ? texts[0]! : texts.map((text, i) => `(${i + 1}) ${text}`).join(' '); +} + const isRec = (v: unknown): v is Record => !!v && typeof v === 'object' && !Array.isArray(v); @@ -557,13 +595,14 @@ function checkObjectChart( // or grouping by one returns a single empty bucket rather than an // error, so the chart renders and says nothing true. if (anchors?.has(name)) { + // [#22161] One verdict sentence, in the shared short form of the + // anchor cause; ADR-0015's federation and why the query does not fail + // are `os explain react-chart-field-unprovisioned`. push( 'warning', REACT_CHART_FIELD_UNPROVISIONED, - `aggregate.${prop} "${name}" resolves on object "${objectName}", but ` + - `${unprovisionedAnchorCause(objectName, name)} — the aggregate query reads a column ` + - `that is empty on every row, so the chart ${prop === 'groupBy' ? 'groups everything into one empty bucket' : 'aggregates nothing'} ` + - `instead of failing.`, + `aggregate.${prop} "${name}": ${unprovisionedAnchorVerdict(objectName, name)}, so the chart ` + + (prop === 'groupBy' ? 'groups everything into one empty bucket' : 'aggregates nothing'), unprovisionedAnchorHint(objectName, name), ); } @@ -1013,10 +1052,11 @@ function recordContextFinding( rule: REACT_BLOCK_NEEDS_RECORD_CONTEXT, where, path, + // [#22161] One verdict sentence; why binding the block does not help is + // `os explain react-block-needs-record-context`. message: - `<${tag}> renders "${schemaType}", which reads its record from the record context a ` + - `record page mounts — a kind:'react' page never mounts one, so the block renders empty ` + - `no matter how it is bound (its objectName/recordId are not read by the renderer).`, + `<${tag}> renders "${schemaType}", which reads its record from a record page's context; a ` + + `kind:'react' page mounts none, so the block renders empty`, hint: `On a react page bind the record yourself: ` + `${REACT_RECORD_BLOCK_ALTERNATIVES[schemaType] ?? RECORD_BLOCK_GENERIC_FIX}`, @@ -1088,9 +1128,11 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { rule: REACT_PAGE_SOURCE_UNPARSEABLE, where: `page "${name}"`, path: `${pagePath}.source`, + // [#22161] One verdict sentence; what a partially recovered tree means + // for the checks is `os explain react-page-source-unparseable`. message: - `kind:'react' source did not parse (${describeParseFailure(failure)}), so the component-contract ` + - `checks read a partially recovered tree and may have missed real problems.`, + `kind:'react' source did not parse (${describeParseFailure(failure)}), so the ` + + `component-contract checks may have missed problems in it`, hint: PARSE_FAILURE_HINT, }); } diff --git a/packages/lint/src/validate-rule-compilability.test.ts b/packages/lint/src/validate-rule-compilability.test.ts index 01610ecd9f1..74e221d043d 100644 --- a/packages/lint/src/validate-rule-compilability.test.ts +++ b/packages/lint/src/validate-rule-compilability.test.ts @@ -23,13 +23,31 @@ import { ObjectStackSchema } from '@objectstack/spec'; import { ObjectSchema } from '@objectstack/spec/data'; import { - validateRuleCompilability, + validateRuleCompilability as validateRuleCompilabilityUnrecorded, MAX_RULE_NESTING_DEPTH, RUNTIME_AJV_OPTIONS, VALIDATION_RULE_REGEX_UNCOMPILABLE, VALIDATION_RULE_SCHEMA_UNCOMPILABLE, } from './validate-rule-compilability.js'; import { AUTHORING_COMMANDS, AUTHORING_RULES, authoringRulesFor, runAuthoringRules } from './authoring-rules.js'; +import { explainRule } from './rule-explanations.js'; + +// [#22161] Each finding of the two 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 COMPILE_IDS: readonly string[] = [VALIDATION_RULE_REGEX_UNCOMPILABLE, VALIDATION_RULE_SCHEMA_UNCOMPILABLE]; +const fired: Array<{ rule: string; message: string }> = []; +const validateRuleCompilability: typeof validateRuleCompilabilityUnrecorded = (...args) => { + const findings = validateRuleCompilabilityUnrecorded(...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 repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..', '..', '..'); const RUNTIME_VALIDATOR = 'packages/objectql/src/validation/rule-validator.ts'; @@ -80,19 +98,23 @@ describe('validateRuleCompilability — the fail-open artifacts go RED', () => { // Named: which rule, on which object, and where in the config. expect(f.where).toBe("object 'account' · validation 'tax_id_format'"); expect(f.path).toBe('objects.account.validations.tax_id_format.regex'); - expect(f.message).toContain("'tax_id_format'"); - expect(f.message).toContain("object 'account'"); // The compiler's own text, not a paraphrase — this is what an author acts on. + // [#22161] Its reason, that is: V8 echoes the source (`/([/: `), which the + // `path` and the `fix:` line already carry. let native = ''; try { new RegExp('(['); } catch (err) { native = (err as Error).message; } - expect(native).not.toBe(''); - expect(f.message).toContain(native); + expect(native).toBe('Invalid regular expression: /([/: Unterminated character class'); + expect(f.message).toBe( + '`regex` does not compile (Unterminated character class), so the write path skips the rule and it ' + + 'enforces nothing on any record', + ); // And the consequence is stated, not implied. expect(f.message).toContain('enforces nothing'); + expect(f.hint).toContain("new RegExp('([')"); expect(f.hint).toMatch(/escaped|format/); }); @@ -113,10 +135,10 @@ describe('validateRuleCompilability — the fail-open artifacts go RED', () => { expect(f.rule).toBe(VALIDATION_RULE_SCHEMA_UNCOMPILABLE); expect(f.where).toBe("object 'account' · validation 'support_config_shape'"); expect(f.path).toBe('objects.account.validations.support_config_shape.schema'); - expect(f.message).toContain("'support_config_shape'"); - expect(f.message).toContain("object 'account'"); - // ajv's own wording, whatever this ajv version words it as. + // ajv's own wording, whatever this ajv version words it as — [#22161] to its + // first violation, with the rest counted. expect(f.message).toMatch(/schema is invalid/i); + expect(f.message).toMatch(/^ajv cannot compile `schema` \(schema is invalid: data\/type [^,]+ \(and \d+ more\)\), /); expect(f.message).toContain('enforces nothing'); }); @@ -944,3 +966,41 @@ describe('validateRuleCompilability — every branch is reachable from a PARSING expect([...emitted].sort()).toEqual([...new Set(pushedRuleIds())].sort()); }); }); + +describe('[#22161] one-line verdicts — the two compile 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([...COMPILE_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 regex refusal V8 does not spell the usual way is quoted whole', () => { + // The source is cut only when the engine's text starts with the exact echo + // of it; nothing else is parsed out of the refusal. + const [f] = validateRuleCompilability( + objectWith({ type: 'format', name: 'fmt', field: 'tax_id', regex: '(?x)(?y)', message: 'm' }), + ); + expect(f.message).toContain('(Duplicate capture group name)'); + expect(f.message).not.toContain('(?x)'); + }); + + // What each verdict stopped saying, which `os explain RULE_ID` now prints. + const MOVED: Record = { + [VALIDATION_RULE_REGEX_UNCOMPILABLE]: ['`checkFormat`', 'new RegExp(rule.regex)', 'skipped', 'enforces nothing on any record, forever', 'no flags'], + [VALIDATION_RULE_SCHEMA_UNCOMPILABLE]: ['`checkJsonSchema`', 'new Ajv({ allErrors: true, strict: false })', '`ajv-formats`', 'duplicate `$id`', 'counts the rest'], + }; + + it('covers exactly the two ids', () => { + expect(Object.keys(MOVED).sort()).toEqual([...COMPILE_IDS].sort()); + }); + + it.each([...COMPILE_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-rule-compilability.ts b/packages/lint/src/validate-rule-compilability.ts index 23f2bc9b8bb..4114a3d4973 100644 --- a/packages/lint/src/validate-rule-compilability.ts +++ b/packages/lint/src/validate-rule-compilability.ts @@ -317,6 +317,40 @@ function errorText(err: unknown): string { return err instanceof Error ? err.message : String(err); } +/** + * [#22161] What a skipped rule leaves — the clause both compile verdicts close + * on. Why the write path skips rather than refuses (the fail-open `try/catch` + * in `rule-validator.ts`) is their `os explain` text. + */ +const SKIPPED_ON_EVERY_WRITE = 'so the write path skips the rule and it enforces nothing on any record'; + +/** + * [#22161] The verdict half of `new RegExp(source)`'s refusal. V8 spells it + * `Invalid regular expression: /SOURCE/: REASON`, and the source is the + * finding's `path` (and the `fix:` line quotes it), so only the reason is + * quoted. Any other spelling is quoted whole. + */ +function regexRefusal(source: string, err: unknown): string { + const text = errorText(err); + const echo = `Invalid regular expression: /${source}/: `; + return text.startsWith(echo) ? text.slice(echo.length) : text; +} + +/** + * [#22161] ajv's refusal, to its first error. Under `allErrors: true` a + * metaschema failure lists every violation in one sentence (`schema is + * invalid: data/required must be array, data/type must be …`); the verdict + * quotes the first, which names the offending keyword, and counts the rest. + * Any other refusal is quoted whole. + */ +function schemaRefusal(err: unknown): string { + const text = errorText(err); + const lead = 'schema is invalid: '; + if (!text.startsWith(lead)) return text; + const violations = text.slice(lead.length).split(/, (?=data\b)/); + return `${lead}${violations[0]}${violations.length > 1 ? ` (and ${violations.length - 1} more)` : ''}`; +} + /** * Depth cap on `conditional` nesting. Parsed metadata is a tree, so this is not * a cycle guard — it is the same cheap promise `flow-walk.ts`'s @@ -418,7 +452,7 @@ export function walkObjectValidationRules(stack: unknown): WalkedValidationRule[ export function validateRuleCompilability(stack: unknown): RuleCompilabilityFinding[] { const findings: RuleCompilabilityFinding[] = []; - for (const { rule, objectName, label, where, basePath } of walkObjectValidationRules(stack)) { + for (const { rule, where, basePath } of walkObjectValidationRules(stack)) { if (rule.type === 'format' && typeof rule.regex === 'string' && rule.regex !== '') { try { // The exact call `checkFormat` makes — no flags, same constructor. @@ -429,11 +463,10 @@ export function validateRuleCompilability(stack: unknown): RuleCompilabilityFind rule: VALIDATION_RULE_REGEX_UNCOMPILABLE, where, path: `${basePath}.regex`, - message: - `\`format\` validation ${label} on object '${objectName}' declares a \`regex\` that does not ` + - `compile: ${errorText(err)}. The write path builds it with \`new RegExp(rule.regex)\` and ` + - `SKIPS the rule when that throws (rule-validator.ts \`checkFormat\`), so the rule is declared, ` + - `listed in the metadata, and enforces nothing on any record.`, + // [#22161] One verdict sentence; `where` names the rule and its + // object. How the write path compiles it and why it skips is + // `os explain validation-rule-regex-uncompilable`. + message: `\`regex\` does not compile (${regexRefusal(rule.regex, err)}), ${SKIPPED_ON_EVERY_WRITE}`, hint: `Fix the pattern so \`new RegExp('${rule.regex}')\` compiles — a literal \`(\`, \`[\` or \`\\\` ` + `must be escaped (\`\\\\(\`, \`\\\\[\`, \`\\\\\\\\\`), and the source is a STRING, so a backslash ` + @@ -453,12 +486,10 @@ export function validateRuleCompilability(stack: unknown): RuleCompilabilityFind rule: VALIDATION_RULE_SCHEMA_UNCOMPILABLE, where, path: `${basePath}.schema`, - message: - `\`json_schema\` validation ${label} on object '${objectName}' declares a \`schema\` ajv cannot ` + - `compile: ${errorText(err)}. The write path compiles it with the same ajv ` + - `(\`new Ajv({ allErrors: true, strict: false })\` + \`ajv-formats\`) and SKIPS the rule when that throws ` + - `(rule-validator.ts \`checkJsonSchema\`), so the rule is declared and enforces nothing on any ` + - `record.`, + // [#22161] One verdict sentence; the runtime's ajv environment and + // why the write path skips is + // `os explain validation-rule-json-schema-uncompilable`. + message: `ajv cannot compile \`schema\` (${schemaRefusal(err)}), ${SKIPPED_ON_EVERY_WRITE}`, hint: `Correct the schema so ajv compiles it — the message above names the offending keyword. ` + `\`type\` must be one of null|boolean|object|array|number|string|integer (or an array of ` + diff --git a/packages/lint/src/validate-rule-schema-formats.test.ts b/packages/lint/src/validate-rule-schema-formats.test.ts index 34062476fcf..85c5bf535c5 100644 --- a/packages/lint/src/validate-rule-schema-formats.test.ts +++ b/packages/lint/src/validate-rule-schema-formats.test.ts @@ -25,13 +25,30 @@ import { describe, it, expect } from 'vitest'; import { ObjectStackSchema } from '@objectstack/spec'; import { - validateRuleSchemaFormats, + validateRuleSchemaFormats as validateRuleSchemaFormatsUnrecorded, nearestRegisteredFormat, MAX_SCHEMA_WALK_DEPTH, VALIDATION_RULE_SCHEMA_UNKNOWN_FORMAT, } from './validate-rule-schema-formats.js'; import { registeredFormatNames, validateRuleCompilability } from './validate-rule-compilability.js'; import { AUTHORING_COMMANDS, AUTHORING_RULES, authoringRulesFor, runAuthoringRules } from './authoring-rules.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 validateRuleSchemaFormats: typeof validateRuleSchemaFormatsUnrecorded = (...args) => { + const findings = validateRuleSchemaFormatsUnrecorded(...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 srcDir = dirname(fileURLToPath(import.meta.url)); const MANIFEST = { id: 'com.example.schema-format-probe', name: 'schema_format_probe', version: '1.0.0', type: 'app' } as const; @@ -75,10 +92,16 @@ describe('validateRuleSchemaFormats — the dropped keyword goes RED (#5178)', ( // Named: which rule, on which object, and exactly where in the schema. expect(f.where).toBe("object 'account' · validation 'support_shape'"); expect(f.path).toBe('objects.account.validations.support_shape.schema#/properties/email/format'); - expect(f.message).toContain("format: 'emial'"); - expect(f.message).toContain('#/properties/email/format'); - // The consequence, in the direction that matters: the record is accepted. + // [#22161] The verdict names the format; `where` and `path` carry the rule, + // the object and the pointer, so the message does not repeat them. + expect(f.message).toBe( + "`format: 'emial'` is not a registered format, so ajv drops the keyword and the rule enforces it on " + + 'no record: every write is ACCEPTED', + ); + // The consequence, in the direction that matters: the record is accepted… expect(f.message).toContain('ACCEPTED'); + // …and how ajv gets there is `os explain`. + expect(explanationOf(VALIDATION_RULE_SCHEMA_UNKNOWN_FORMAT)).toContain('DROPS the keyword'); // The fix, concretely — an author cannot act on "unknown format". expect(f.hint).toContain("Did you mean `format: 'email'`?"); // …and the full vocabulary, so a name that is merely unfamiliar is @@ -397,3 +420,31 @@ describe('validateRuleSchemaFormats is wired into every authoring command', () = expect(mine[0].severity).toBe('error'); }); }); + +describe('[#22161] one-line verdicts — validation-rule-json-schema-unknown-format', () => { + 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([VALIDATION_RULE_SCHEMA_UNKNOWN_FORMAT]); + for (const f of fired) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + } + }); + + it('`os explain` carries what the verdict no longer says', () => { + const text = explanationOf(VALIDATION_RULE_SCHEMA_UNKNOWN_FORMAT); + for (const fact of [ + 'unknown format "NAME" ignored', + 'DROPS the keyword', + 'at the publish gate alike', + '`type`, `required`', + 'The record is ACCEPTED', + 'read off a live instance', + '`default`, `const`, `enum` or `examples`', + 'new Ajv({ allErrors: true, strict: false })', + ]) { + expect(text, `explanation names ${fact}`).toContain(fact); + } + }); +}); diff --git a/packages/lint/src/validate-rule-schema-formats.ts b/packages/lint/src/validate-rule-schema-formats.ts index 64192d5e4f3..de884f7522b 100644 --- a/packages/lint/src/validate-rule-schema-formats.ts +++ b/packages/lint/src/validate-rule-schema-formats.ts @@ -292,19 +292,19 @@ export function nearestRegisteredFormat(name: string, registered: readonly strin export function validateRuleSchemaFormats(stack: unknown): RuleSchemaFormatFinding[] { const findings: RuleSchemaFormatFinding[] = []; - const pending: Array<{ use: FormatUse; where: string; label: string; objectName: string; basePath: string }> = []; - for (const { rule, objectName, label, where, basePath } of walkObjectValidationRules(stack)) { + const pending: Array<{ use: FormatUse; where: string; basePath: string }> = []; + for (const { rule, where, basePath } of walkObjectValidationRules(stack)) { if (rule.type !== 'json_schema' || !isRec(rule.schema)) continue; const uses: FormatUse[] = []; collectFormatUses(rule.schema, '', uses, 0); - for (const use of uses) pending.push({ use, where, label, objectName, basePath }); + for (const use of uses) pending.push({ use, where, basePath }); } if (pending.length === 0) return findings; const registered = registeredFormatNames(); const known = new Set(registered); - for (const { use, where, label, objectName, basePath } of pending) { + for (const { use, where, basePath } of pending) { if (known.has(use.name)) continue; const suggestion = nearestRegisteredFormat(use.name, registered); const pointer = `#${use.pointer}`; @@ -313,13 +313,13 @@ export function validateRuleSchemaFormats(stack: unknown): RuleSchemaFormatFindi rule: VALIDATION_RULE_SCHEMA_UNKNOWN_FORMAT, where, path: `${basePath}.schema${pointer}`, + // [#22161] One verdict sentence. `where` names the rule and its object and + // `path` the JSON Pointer, so neither is echoed; what ajv does with an + // unknown name, on which doors, and why the gap is silent is + // `os explain validation-rule-json-schema-unknown-format`. message: - `\`json_schema\` validation ${label} on object '${objectName}' names \`format: '${use.name}'\` at ` + - `\`${pointer}\`, which is not a registered format. ajv logs \`unknown format "${use.name}" ignored\` ` + - `once at compile time and DROPS the keyword — in the write path (rule-validator.ts, ` + - `\`strict: false\`) and in the publish gate alike — so the schema compiles, the rule ships and runs ` + - `on every write, its \`type\`/\`required\` keywords are enforced, and this constraint is enforced ` + - `on no record, ever. The record is ACCEPTED, so nothing downstream reports the gap either.`, + `\`format: '${use.name}'\` is not a registered format, so ajv drops the keyword and the rule ` + + `enforces it on no record: every write is ACCEPTED`, hint: (suggestion ? `Did you mean \`format: '${suggestion}'\`? ` : '') + `The registered names are: ${registered.join(', ')} — the default \`ajv-formats\` set, the one ` + From 1a40fc23e911c57b303444dcfba65e660a343a33 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 16:13:32 +0000 Subject: [PATCH 2/3] wip(lint): slice 5 tests green on the five rule files Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude --- .../src/validate-dataset-measure-aggregates.test.ts | 2 +- .../lint/src/validate-dataset-measure-aggregates.ts | 10 +++++++--- .../lint/src/validate-flow-trigger-readiness.test.ts | 2 +- packages/lint/src/validate-rule-compilability.test.ts | 7 +++++-- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/lint/src/validate-dataset-measure-aggregates.test.ts b/packages/lint/src/validate-dataset-measure-aggregates.test.ts index 4b9f0256ef1..d5f8b464bd2 100644 --- a/packages/lint/src/validate-dataset-measure-aggregates.test.ts +++ b/packages/lint/src/validate-dataset-measure-aggregates.test.ts @@ -1100,7 +1100,7 @@ describe('[#22161] one-line verdicts — the two ids', () => { // What each verdict stopped saying, which `os explain RULE_ID` now prints. const MOVED: Record = { - [MEASURE_AGGREGATE_FIELD_TYPE_REFUSED]: ['property of the SQL dialect', 'average YEAR', 'COMPARES', '400 DATASET_INVALID', '`measure-aggregate-incoherent`'], + [MEASURE_AGGREGATE_FIELD_TYPE_REFUSED]: ['property of the SQL dialect', 'average YEAR', 'compares values for equality', '400 DATASET_INVALID', '`measure-aggregate-incoherent`'], [DIMENSION_JSON_STORED_FIELD_REFUSED]: ['GROUP KEY', 'one groups each serialized value apart', 'before any SQL is built', 'instead of an answer'], }; diff --git a/packages/lint/src/validate-dataset-measure-aggregates.ts b/packages/lint/src/validate-dataset-measure-aggregates.ts index 2181da111d2..f9b7c1614b3 100644 --- a/packages/lint/src/validate-dataset-measure-aggregates.ts +++ b/packages/lint/src/validate-dataset-measure-aggregates.ts @@ -419,15 +419,19 @@ function refusedMeasureFinding( // [#20890] The row accepts the TYPE and the declaration is what refuses: // a multi-capable field flagged `multiple: true` under `count_distinct`. const flaggedList = isAggregateCompatibleWithFieldType(aggregate, fieldType); - const pair = `aggregate "${aggregate}" over field "${site.column}" (${declaredAs(shape)} on ${site.declarer})`; + // The declaration as the verdict needs it: the flag where it is what refuses + // the pair, the type alone where the table's row does. + const pair = (declaration: string) => + `aggregate "${aggregate}" over field "${site.column}" (${declaration} on ${site.declarer})`; return { severity: 'error', rule: MEASURE_AGGREGATE_FIELD_TYPE_REFUSED, where: site.where, path: site.path, message: flaggedList - ? `${pair} is refused: a list stored as JSON, which no two backends compare alike` - : `${pair} is refused by the aggregate × field-type table: the number would depend on the SQL dialect`, + ? `${pair(declaredAs(shape))} is refused: a list stored as JSON, which no two backends compare alike` + : `${pair(`\`${fieldType}\``)} is refused by the aggregate × field-type table: the number would depend ` + + 'on the SQL dialect', hint: `Either point "${aggregate}" at a field of an accepted type, or aggregate ` + `"${site.column}" with one its ${declaredAs(shape)} ${shape.multiple === true ? 'declaration' : 'type'} accepts: ` + diff --git a/packages/lint/src/validate-flow-trigger-readiness.test.ts b/packages/lint/src/validate-flow-trigger-readiness.test.ts index 91001ef96e5..de7258806b4 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.test.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.test.ts @@ -1075,7 +1075,7 @@ describe('validateFlowTriggerReadiness', () => { expect(new Set(findings.map((f) => f.path)).size).toBe(2); expect( findings.find((f) => f.rule === FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE)!.message, - ).toMatch(/binds to NOTHING/); + ).toMatch(/binds to nothing and never fires$/); }); it('the id is the published slug and is distinct from the routed-token id', () => { diff --git a/packages/lint/src/validate-rule-compilability.test.ts b/packages/lint/src/validate-rule-compilability.test.ts index 74e221d043d..b558f31713e 100644 --- a/packages/lint/src/validate-rule-compilability.test.ts +++ b/packages/lint/src/validate-rule-compilability.test.ts @@ -858,8 +858,11 @@ describe('validateRuleCompilability — reads only keys the spec declares (meta- // metadata. `walked` and `names` are the two accumulators #5178 added when // the rule walk and the registered-format vocabulary became shared handles // (`walkObjectValidationRules`, `registeredFormatNames`) — array `.push` / - // `.length` / `.sort`, never a key off authored metadata. - const PLUMBING = new Set(['findings', 'out', 'walked', 'names']); + // `.length` / `.sort`, never a key off authored metadata. [#22161] `text`, + // `echo`, `lead` and `violations` are the verdict helpers' strings — a + // compiler's refusal and the prefixes cut off it (`regexRefusal`, + // `schemaRefusal`): `.startsWith` / `.slice` / `.split` / `.length`. + const PLUMBING = new Set(['findings', 'out', 'walked', 'names', 'text', 'echo', 'lead', 'violations']); expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]); // …and no excuse outlives the read it excuses. A stale name in either list From 858cc2140028faeabf9f07b986d0d3a17845b06c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 16:28:44 +0000 Subject: [PATCH 3/3] wip(lint): slice 5 changeset Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude --- .changeset/22161-lint-slice-5-one-line.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 .changeset/22161-lint-slice-5-one-line.md diff --git a/.changeset/22161-lint-slice-5-one-line.md b/.changeset/22161-lint-slice-5-one-line.md new file mode 100644 index 00000000000..02eb0ecbc39 --- /dev/null +++ b/.changeset/22161-lint-slice-5-one-line.md @@ -0,0 +1,18 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): the flow-trigger, react-page, dataset-measure and validation-rule findings print one verdict line, and `os explain ` carries their reasoning + +Clause-②: no + +- **Shorter verdicts.** Each finding of these 15 rule ids now prints a `message` of one verdict sentence. Every finding the rules' own test suites and the runtime publish gate's suites fire is at most 194 characters; before, the longest of each ran from 211 to 1,049 characters. The ids: + - flow triggers (`flows[].nodes[]` start node): `flow-time-relative-descriptor-invalid`, `flow-time-relative-descriptor-unroutable`, `flow-trigger-unroutable`, `flow-api-trigger-secret-missing`, `flow-trigger-unknown-event`; + - react pages (`pages[].source`): `react-chart-drilldown-invalid`, `react-chart-aggregate-invalid`, `react-chart-field-unprovisioned`, `react-block-needs-record-context`, `react-page-source-unparseable`; + - dataset and cube members: `measure-aggregate-field-type-refused`, `dimension-json-stored-field-refused`; + - object validation rules: `validation-rule-json-schema-uncompilable`, `validation-rule-regex-uncompilable`, `validation-rule-json-schema-unknown-format`. + + A verdict no longer repeats what the finding's `where` and `path` already name: the validation rule and its object, the JSON Pointer of a `format`, the measure or dimension name, the source of an uncompilable `regex` (the `fix:` line still quotes it). A refusal the verdict quotes is quoted to its verdict. `TimeRelativeTriggerSchema`'s refusal of a descriptor is quoted for ONE issue (an unrecognized key first) and the rest are counted, `(and N more)`; ajv's metaschema refusal is quoted to its first violation, with the rest counted; a `@objectstack/spec` unknown-key refusal is quoted to the key and the schema's rename (`Did you mean …?`), without the wrong-layer prescription and the history sentence that follow it; and when no form of an `` `groupBy` union matches, the verdict keeps the forms whose complaint is about the value's content. A measure or dimension over a column its join chain reaches names a `joined object` (it read "(reached through this dataset's join chain)"). 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 15 ids**, for example `os explain flow-trigger-unroutable`. It prints the reasoning the verdicts no longer carry: the engine's trigger routing chain, the record-trigger token grammar and the time-relative routing predicate, what nothing reports at run time, and where the publish gate refuses the flow; the inbound api secret (ADR-0041) and why the engine refuses to register a flow without one; the write path's fail-open skip of a rule whose `regex` or `schema` does not compile, the runtime's ajv environment, and what ajv does with an unregistered `format` name; each aggregate's accepted field types (the `AGGREGATE_FIELD_TYPE_COMPATIBILITY` rows, written out), what a JSON-stored column is, and why analytics refuses to group by one; how a forwarded `` or `aggregate` refusal is quoted and which keys the schema answers with a prescription instead of a rename (`schedule` and `runAs` on a time-relative descriptor, `drilldown`, `mode`, `report`, `view` and `sort` on a drill block, five aggregate and two `groupBy` keys); what an unprovisioned anchor is; why a `record:*` block renders empty on a react page; and why an unparseable react source is a warning. 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 15 ids, and so does the unknown-id error's `Rules with an explanation:` line. `RULE_EXPLANATIONS` in `@objectstack/lint` gains the 15 entries. +- **Where the new text prints.** On the CLI, all 15 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): a `flow` write carries the five flow ids (all errors, refusing an active flow's publish), a `dataset` write the two dataset ids, and an `object` write the three validation-rule ids — the 422 issue's `message` and the refusal log line under `OS_ALLOW_UNLINTED_METADATA_WRITES` change. Each issue's `hint` is unchanged. +- **Never at the runtime gate:** the five `react-*` ids. Their rule runs at that door only for a flow write, whose snapshot carries no pages, so they speak only on the CLI doors above.