diff --git a/.changeset/22161-functional-completeness-one-line.md b/.changeset/22161-functional-completeness-one-line.md new file mode 100644 index 00000000000..6addb30406e --- /dev/null +++ b/.changeset/22161-functional-completeness-one-line.md @@ -0,0 +1,13 @@ +--- +"@objectstack/spec": patch +"@objectstack/lint": patch +--- + +fix(spec, lint): the functional-completeness findings print one verdict line, and `os explain ` carries their reasoning + +Clause-②: no + +- **Shorter verdicts.** The nine rule ids the ADR-0078 completeness predicate emits (`field/summary-without-operations`, `field/formula-without-expression`, `field/relationship-without-reference`, `field/choice-without-options`, `view/layout-without-binding`, `view/tree-without-parent-field`, `view/row-color-without-colors`, `view/row-color-unresolvable-value`, `webhook/without-triggers`) printed a `message` of 239 to 793 characters. Each `message` is now one verdict sentence of at most 200 characters (for `view/row-color-unresolvable-value`, plus the offending `key` = "value" entries it lists). It still names the runtime site that makes the verdict true, for example `` A `summary` field with no `summaryOperations` reads 0/null everywhere: the engine's summary index skips it (`engine.ts` — `if (!d.summaryOperations) continue`) ``. The `fix` (the CLI's `fix:` line, the runtime issue's `hint`) is unchanged, and so are every rule id, severity, `path`, and what each rule accepts or refuses. For `view/layout-without-binding`, the keys the old body told the author to declare are on that `fix:` line. A tool that matched the old message text should match on `rule` and `path` instead. +- **`os explain ` takes these nine ids**, for example `os explain view/layout-without-binding`. It prints the reasoning the verdicts no longer carry: each view type's measured renderer path, refusal screen and required keys, the exemptions (`user` relationships, `multiselect`, `gallery`), the hex trap behind the two `rowColor` rules, and why an empty `triggers` is not an off switch. 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 nine ids, and so does the unknown-id error's `Rules with an explanation:` line. `RULE_EXPLANATIONS` in `@objectstack/lint` gains the nine entries. +- **Where the new text prints.** On the CLI, `os validate`, `os build` (and `os compile`, which `os dev` runs on every compile), `os lint`, `os verify` and `os init`'s scaffold check print the new `message` on the text face. `os validate --json` and `os build --json` carry it in their `warnings` and author-time `issues`. At the runtime publish gate, the rule runs on `object` writes only (Studio, REST `/meta`, MCP), so only the `field/*` ids reach it. A `summary` field with no `summaryOperations` or a `formula` field with no `expression` is an `error`: the 422 issue's `message` changes, and so does the server's refusal log line under the unlinted-writes hatch. A `checkboxes` field with no `options` is a `warning`: the `message` in the 2xx response's `advisories` changes, and so does the deduped `[Protocol] authoring advisory` server log line. Each issue's `hint` is unchanged. A `lookup`/`master_detail` with no `reference` and a `select`/`radio` with no `options` never reach this gate: the object schema's own parse refuses them first, with its own message, which this change does not touch. +- **Unchanged doors.** The registry's boot-time line (`[Registry] Object "…" registered with N functionally-incomplete field(s) …`) prints each finding's rule id and `fix`, never its `message`, so it reads as before. The webhook auto-enqueuer's `webhook/without-triggers` boot warning has its own text, also unchanged. The `view/*` and `webhook/*` ids do not run at the runtime publish gate. diff --git a/packages/lint/src/rule-explanations.test.ts b/packages/lint/src/rule-explanations.test.ts index c4d8ac14972..a5a301aaf21 100644 --- a/packages/lint/src/rule-explanations.test.ts +++ b/packages/lint/src/rule-explanations.test.ts @@ -10,15 +10,21 @@ import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; +import { FUNCTIONAL_COMPLETENESS_RULES } from '@objectstack/spec/kernel'; + import * as indexBarrel from './index.js'; import { RULE_EXPLANATIONS, explainRule } from './rule-explanations.js'; import { CARRIER_ROOTS, CONSUMER_ROOTS, FIELD_NO_CONSUMERS } from './validate-field-consumers.js'; const HERE = dirname(fileURLToPath(import.meta.url)); -/** Every rule id constant the root barrel exports, by value. */ +/** + * Every rule id constant the root barrel exports, by value, plus the ids of the + * functional-completeness predicate: those rules live in `@objectstack/spec/kernel` + * and `validate-functional-completeness.ts` forwards their findings unchanged. + */ function exportedRuleIds(): Set { - const ids = new Set(); + const ids = new Set(FUNCTIONAL_COMPLETENESS_RULES); for (const [name, value] of Object.entries(indexBarrel)) { if (/^[A-Z][A-Z0-9_]*$/.test(name) && typeof value === 'string') ids.add(value); } diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 7ac9bfdd52a..a515b043ba5 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -14,7 +14,10 @@ * * ⛔ One copy. The text below is not repeated in the rule's message or hint, * and a rule's message does not paraphrase it: an author who wants the "why" - * runs the command. + * runs the command. One deliberate overlap: a functional-completeness verdict + * (`@objectstack/spec/kernel`'s `FUNCTIONAL_COMPLETENESS_RULES`) still names + * the runtime site that makes it true, because ADR-0078 §6 keeps that + * citation in the finding's own message. * * ## Why this module imports nothing * @@ -26,8 +29,10 @@ * every other command would otherwise pay. So the keys are the rule ids * spelled as literals, and a list a rule computes from data (the roots a scan * reads) is written out here — `rule-explanations.test.ts` holds each key to a - * rule id constant some rule file exports and each such list to the rule's own - * constant, so neither can drift from the rule it explains. + * rule id constant some rule file exports (or to an id in the completeness + * predicate's `FUNCTIONAL_COMPLETENESS_RULES`, whose rules live in + * `@objectstack/spec`) and each such list to the rule's own constant, so + * neither can drift from the rule it explains. * * Adding an entry: key it by the rule's id, write `covers` so that it * completes the sentence "`os explain ` for …", and move the long text out @@ -111,6 +116,194 @@ const SECURITY_OWD_UNSET_EXPLANATION: RuleExplanation = { ], }; +// ── Functional completeness (ADR-0078) ────────────────────────────────────── +// The verdicts live in `packages/spec/src/kernel/functional-completeness.ts`; +// each names the runtime site that makes it true, and the measured reading +// behind it is written out below. A mechanism corrected there is corrected +// here in the same edit. + +const FIELD_SUMMARY_WITHOUT_OPERATIONS_EXPLANATION: RuleExplanation = { + rule: 'field/summary-without-operations', + covers: 'why a summary field needs summaryOperations', + paragraphs: [ + 'A `summary` field is a roll-up over child records, and `summaryOperations` says which child ' + + 'object, which child field and which function to roll up. Without it the engine\'s summary ' + + 'index skips the field (`engine.ts` — `if (!d.summaryOperations) continue`), so it computes ' + + 'nothing: it reads 0/null everywhere, and anything derived from it is stuck at 0.', + 'Nothing else notices: the field parses, publishes and is served, and every authoring surface ' + + 'reports success. This is the shape ADR-0078 was written for — metadata that is valid and uses ' + + 'only live properties, yet is dead because it omits the sibling config its consumer needs.', + 'To fix it, declare `summaryOperations` with the child object, the child field and the function ' + + '(the `fix:` line shows `sum`), or remove the field.', + ], +}; + +const FIELD_FORMULA_WITHOUT_EXPRESSION_EXPLANATION: RuleExplanation = { + rule: 'field/formula-without-expression', + covers: 'why a formula field needs an expression', + paragraphs: [ + 'A `formula` field computes its value from `expression`. The engine builds its formula plan ' + + 'only from fields that have one (`engine.ts` — `if (def?.type === \'formula\' && ' + + 'def.expression)`), so a formula with no `expression` never computes: the field is ' + + 'permanently empty, while parsing and publishing both succeed.', + 'To fix it, write the expression (the `fix:` line shows the shape, reading other fields through ' + + '`record`), or remove the field.', + ], +}; + +const FIELD_RELATIONSHIP_WITHOUT_REFERENCE_EXPLANATION: RuleExplanation = { + rule: 'field/relationship-without-reference', + covers: 'why a relationship field needs a reference', + paragraphs: [ + 'A `lookup` or `master_detail` field is a relationship, and `reference` names the object it ' + + 'points at. With no `reference` it is a relationship to nowhere: `$expand` silently skips it ' + + '(`engine.ts` — `if (!referenceObject) continue`), and the record picker has no object to ' + + 'search, so the column stores raw ids that never resolve.', + 'A `user` field is not judged: its target is implicitly `sys_user`. To fix it, declare ' + + '`reference` with the name of the target object.', + ], +}; + +const FIELD_CHOICE_WITHOUT_OPTIONS_EXPLANATION: RuleExplanation = { + rule: 'field/choice-without-options', + covers: 'why a choice field needs options', + paragraphs: [ + 'A `select` or `radio` field with no `options` is a choice with nothing to choose, and it fails ' + + 'twice: the form control is empty, AND server-side value validation is disabled — ' + + '`record-validator.ts` skips the check when the allowed list is empty — so any value writes ' + + 'through the API. That is an error.', + 'A `picklist` reference is an option source too: the served field carries the shared list\'s ' + + 'options, so `picklist` naming a declared list satisfies the rule as well as an `options` list.', + 'A `checkboxes` field with no `options` renders zero checkboxes. The validator\'s multi-value ' + + 'branch tolerates it as free-form (the `multiselect` tags mode), so it is a warning, not an ' + + 'error — but a checkbox group is almost never meant to be free-form: declare the boxes, or use ' + + '`multiselect` if free-form tags were the intent.', + 'A `multiselect` with no `options` is not judged: `record-validator.ts` accepts it as free-form ' + + 'tags by design.', + ], +}; + +const VIEW_LAYOUT_WITHOUT_BINDING_EXPLANATION: RuleExplanation = { + rule: 'view/layout-without-binding', + covers: 'what each view type renders without its block', + paragraphs: [ + 'Six list-view types read their layout from a config block named after the type: `kanban`, ' + + '`calendar`, `gantt`, `timeline`, `map` and `tree`. A view of one of them with no block parses ' + + 'and publishes clean, but the surface the author asked for is not the one that renders. What ' + + 'renders instead differs by type, as read from the renderer below; the rule is a warning on all six.', + '`calendar`: no date axis, and the renderer does not invent one. objectui\'s `ListView.tsx` ' + + 'calendar branch forwards only the bindings the view declared, so `getCalendarConfig` ' + + '(`ObjectCalendar.tsx`) resolves `null` and the view renders its "Calendar configuration ' + + 'required" refusal screen instead of records — on any object, not just on one that happens to ' + + 'lack a field. Declare `calendar.startDateField`, the block\'s one required key; the event title ' + + 'resolves through the ADR-0079 record display-name chain when `titleField` is omitted.', + '`gantt`: the `ListView.tsx` gantt branch forwards only the bindings the view declared, so ' + + '`getGanttConfig` (`ObjectGantt.tsx`) resolves `null` without both dates and the view renders ' + + 'its "Gantt configuration required" refusal screen instead of tasks, on any object. Declare ' + + '`gantt.startDateField`, `gantt.endDateField` and `gantt.titleField`, the three keys ' + + '`GanttConfigSchema` requires; `progressField` and `dependenciesField` are optional and stay ' + + 'unbound when omitted.', + '`timeline`: the `ListView.tsx` timeline branch forwards a start date only when the view ' + + 'declared one, so `ObjectTimeline.tsx` resolves no date field and renders its "Timeline date ' + + 'axis required" refusal instead of records, on any object. Only the title has a renderer ' + + 'default (`name`); the date axis never does. Declare `timeline.startDateField` and ' + + '`timeline.titleField`, the two keys `TimelineConfigSchema` requires.', + '`map`: no coordinate binding, and the renderer does not guess one. The `ListView.tsx` map ' + + 'branch forwards only the keys the view declared, and `ObjectMap.tsx` no longer guesses ' + + '`location` / `latitude` / `longitude` field names, so its `hasCoordinateBinding` gate fails ' + + 'and the view renders its "Map configuration required" refusal instead of markers, plotting ' + + 'nothing on any object. The gate treats a declared block exactly as an absent one, so a `map` ' + + 'block that binds neither form is reported too. Declare `map.locationField`, or both ' + + '`map.latitudeField` and `map.longitudeField` (half a pair is not a binding); ' + + '`ListMapConfigSchema` requires neither form, so this warning is where the requirement is stated.', + '`kanban` and `tree`: the renderer falls back to literal default field names, which works only ' + + 'if the object happens to declare them; on any other object the view renders empty while ' + + 'authoring reports success. For `tree` a block is not the whole story either: every `tree` key ' + + 'is optional, and a tree with no resolvable parent pointer renders flat — that is ' + + '`view/tree-without-parent-field`.', + 'A `gallery` view is not judged: no `gallery` key is required, and its fallback mis-titles ' + + 'cards rather than emptying the view.', + ], +}; + +const VIEW_TREE_WITHOUT_PARENT_FIELD_EXPLANATION: RuleExplanation = { + rule: 'view/tree-without-parent-field', + covers: 'how a tree view finds its parent pointer', + paragraphs: [ + 'A `tree` view nests each record under its parent, so it needs a parent pointer: ' + + '`tree.parentField`, or a field the renderer can auto-detect on the bound object (objectui ' + + '`ObjectTree.tsx` — `detectParentField`). Auto-detection accepts a `tree` field with no ' + + '`reference` or with one naming this object, or a `lookup` / `master_detail` whose `reference` ' + + 'is this object.', + 'When `parentField` is undeclared and the bound object has none of those, auto-detection finds ' + + 'nothing and `buildForest` makes every record a root at depth 0. The view renders FLAT, not ' + + 'empty: a complete, correct-looking table with an expand slot that never opens, while ' + + 'authoring reports success.', + 'The rule fires only when both halves fail, so a view that renders correctly by auto-detection ' + + 'is never warned about; it stays silent when the stack does not declare the view\'s object. To ' + + 'fix it, declare `tree.parentField`, or add a self-referencing field to the object.', + ], +}; + +const VIEW_ROW_COLOR_WITHOUT_COLORS_EXPLANATION: RuleExplanation = { + rule: 'view/row-color-without-colors', + covers: 'how a grid colours its rows', + paragraphs: [ + 'A grid\'s `rowColor` colours each row from a field\'s value: `field` says which value to look ' + + 'up, and the `colors` map says which colour each value gets — the map does the colouring. With ' + + 'no `colors` map the grid\'s row-className resolver returns before it reads a record (objectui ' + + '`useRowColor.ts` — `if (!config?.field || !config.colors) return undefined`), so every row ' + + 'keeps the default background while parsing and publishing report success.', + 'An empty `colors: {}` is the same dead shape spelled out: it passes that guard and then matches ' + + 'no value. To turn row colouring off, omit the `rowColor` block or leave the toolbar toggle ' + + '`userActions.rowColor` off.', + 'Each value in the map is a colour NAME from the resolver\'s own vocabulary (`red`, `blue`, ' + + '`slate`, …) or a complete Tailwind background class (`bg-red-200`). A hex parses, publishes and ' + + 'silences this rule while still colouring nothing — that is `view/row-color-unresolvable-value`.', + 'Only `grid` views are judged: the other list-view types never read `rowColor`, so a `colors` ' + + 'map would not make them colour anything.', + ], +}; + +const VIEW_ROW_COLOR_UNRESOLVABLE_VALUE_EXPLANATION: RuleExplanation = { + rule: 'view/row-color-unresolvable-value', + covers: 'which colour values a grid can resolve', + paragraphs: [ + 'objectui\'s `useRowColor.ts` resolves a `rowColor.colors` value in `colorToClass`: a ' + + '`bg-`-prefixed literal is handed through untouched, and anything else is lower-cased, trimmed and ' + + 'looked up in the resolver\'s own closed vocabulary of colour NAMES, returning `undefined` for ' + + 'everything else. Tailwind v4 has no runtime, so no class can be fabricated from a hex.', + 'A map holding such values CLEARS the `!config.colors` guard, so ' + + '`view/row-color-without-colors` goes quiet — and every row still keeps its default background ' + + 'while parsing and publishing report success. A hex is the obvious value to copy, because a ' + + 'select field\'s own option colours are hexes; the warning lists each offending entry with its ' + + 'value so the author does not have to re-read the map.', + 'The rule judges a value\'s SHAPE, not the vocabulary: a value that is neither `bg-`-prefixed nor ' + + 'a bare alphabetic word once lower-cased and trimmed cannot be a colour name, whatever the ' + + 'vocabulary holds. So it never reports a value the renderer would resolve (`RED` and a padded ' + + '`red` resolve), and a misspelled name such as `chartreuse` passes it. Whether a `bg-…` class was ' + + 'actually compiled into the stylesheet is not judged.', + 'To fix it, write a colour name (`red`, `blue`, `slate`, …) or a complete Tailwind background ' + + 'class (`bg-red-200`).', + ], +}; + +const WEBHOOK_WITHOUT_TRIGGERS_EXPLANATION: RuleExplanation = { + rule: 'webhook/without-triggers', + covers: 'why an empty trigger list is not an off switch', + paragraphs: [ + 'A webhook fires on the record events its `triggers` name. With none, the auto-enqueuer drops it ' + + 'while building its subscription cache (`auto-enqueuer.ts` — `if (triggers.size === 0) … return ' + + 'null`), and there is no manual fire path to reach it either: `webhook.zod.ts` records that the ' + + '`api` trigger was removed because "no manual fire path exists — the only webhook HTTP surface ' + + 're-queues already-failed deliveries". So it never fires on any path.', + 'The webhook still materializes into `sys_webhook`, looks armed in Setup, and delivers nothing. ' + + 'An empty `triggers: []` is judged the same as an omitted list.', + 'To disable a webhook use `isActive: false`; an empty `triggers` is not an off switch, just a ' + + 'dead one. To fix it, name the events it should fire on.', + ], +}; + /** * Every rule explanation this package ships, keyed by rule id. `os explain * ` reads this table and nothing else. @@ -118,6 +311,15 @@ const SECURITY_OWD_UNSET_EXPLANATION: RuleExplanation = { export const RULE_EXPLANATIONS: Readonly> = Object.freeze({ [FIELD_NO_CONSUMERS_EXPLANATION.rule]: FIELD_NO_CONSUMERS_EXPLANATION, [SECURITY_OWD_UNSET_EXPLANATION.rule]: SECURITY_OWD_UNSET_EXPLANATION, + [FIELD_SUMMARY_WITHOUT_OPERATIONS_EXPLANATION.rule]: FIELD_SUMMARY_WITHOUT_OPERATIONS_EXPLANATION, + [FIELD_FORMULA_WITHOUT_EXPRESSION_EXPLANATION.rule]: FIELD_FORMULA_WITHOUT_EXPRESSION_EXPLANATION, + [FIELD_RELATIONSHIP_WITHOUT_REFERENCE_EXPLANATION.rule]: FIELD_RELATIONSHIP_WITHOUT_REFERENCE_EXPLANATION, + [FIELD_CHOICE_WITHOUT_OPTIONS_EXPLANATION.rule]: FIELD_CHOICE_WITHOUT_OPTIONS_EXPLANATION, + [VIEW_LAYOUT_WITHOUT_BINDING_EXPLANATION.rule]: VIEW_LAYOUT_WITHOUT_BINDING_EXPLANATION, + [VIEW_TREE_WITHOUT_PARENT_FIELD_EXPLANATION.rule]: VIEW_TREE_WITHOUT_PARENT_FIELD_EXPLANATION, + [VIEW_ROW_COLOR_WITHOUT_COLORS_EXPLANATION.rule]: VIEW_ROW_COLOR_WITHOUT_COLORS_EXPLANATION, + [VIEW_ROW_COLOR_UNRESOLVABLE_VALUE_EXPLANATION.rule]: VIEW_ROW_COLOR_UNRESOLVABLE_VALUE_EXPLANATION, + [WEBHOOK_WITHOUT_TRIGGERS_EXPLANATION.rule]: WEBHOOK_WITHOUT_TRIGGERS_EXPLANATION, }); /** The explanation for `rule`, or `undefined` when the rule has none. Exact id match. */ diff --git a/packages/lint/src/validate-functional-completeness.test.ts b/packages/lint/src/validate-functional-completeness.test.ts index cf9831220a2..7c4d224143f 100644 --- a/packages/lint/src/validate-functional-completeness.test.ts +++ b/packages/lint/src/validate-functional-completeness.test.ts @@ -15,7 +15,10 @@ import { describe, expect, it } from 'vitest'; +import { FUNCTIONAL_COMPLETENESS_RULES } from '@objectstack/spec/kernel'; + import { runAuthoringRules, splitBySeverity } from './authoring-rules.js'; +import { explainRule } from './rule-explanations.js'; import { validateFunctionalCompleteness } from './validate-functional-completeness.js'; const bareSummary = { type: 'summary' }; @@ -425,3 +428,43 @@ describe('#14320 acceptance — object-nested `list` / `listViews` reach `valida }); } }); + +// [#22161] Each completeness finding prints ONE verdict sentence and one fix; +// the reasoning the verdict used to carry is the rule id's `os explain` entry. +// The predicate's own suite pins the short verdicts. This pins the other half: +// every id the predicate can emit has an explanation, and each explanation +// still names what the verdict stopped saying — the renderer screens and +// required keys per view type, the exemptions, the off switch. +describe('every completeness rule id has an `os explain` entry carrying what its verdict no longer does', () => { + const MOVED: Record = { + 'field/summary-without-operations': ['engine.ts', 'summaryOperations', 'ADR-0078'], + 'field/formula-without-expression': ['engine.ts', 'expression'], + 'field/relationship-without-reference': ['$expand', 'never resolve', '`user`', 'sys_user'], + 'field/choice-without-options': ['record-validator.ts', '`picklist`', '`multiselect`', '`checkboxes`'], + 'view/layout-without-binding': [ + 'Calendar configuration required', 'ObjectCalendar.tsx', 'calendar.startDateField', 'ADR-0079', + 'Gantt configuration required', 'ObjectGantt.tsx', + 'gantt.startDateField', 'gantt.endDateField', 'gantt.titleField', 'GanttConfigSchema', + 'Timeline date axis required', 'ObjectTimeline.tsx', + 'timeline.startDateField', 'timeline.titleField', 'TimelineConfigSchema', + 'Map configuration required', 'ObjectMap.tsx', 'hasCoordinateBinding', + 'map.locationField', 'map.latitudeField', 'map.longitudeField', 'ListMapConfigSchema', + 'falls back to literal default field names', 'ListView.tsx', + ], + 'view/tree-without-parent-field': ['ObjectTree.tsx', 'detectParentField', 'buildForest', 'depth 0', 'tree.parentField'], + 'view/row-color-without-colors': ['useRowColor.ts', 'colors: {}', 'bg-red-200', 'view/row-color-unresolvable-value'], + 'view/row-color-unresolvable-value': ['colorToClass', 'Tailwind v4', 'view/row-color-without-colors', 'bg-red-200'], + 'webhook/without-triggers': ['auto-enqueuer.ts', 'webhook.zod.ts', 'no manual fire path exists', 'sys_webhook', 'isActive'], + }; + + it('covers exactly the ids the predicate can emit', () => { + expect(Object.keys(MOVED).sort()).toEqual([...FUNCTIONAL_COMPLETENESS_RULES].sort()); + }); + + it.each([...FUNCTIONAL_COMPLETENESS_RULES])('%s', (rule) => { + const explanation = explainRule(rule); + expect(explanation, `no \`os explain ${rule}\` entry`).toBeDefined(); + const text = explanation!.paragraphs.join('\n'); + for (const fact of MOVED[rule]) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + }); +}); diff --git a/packages/spec/src/kernel/functional-completeness.test.ts b/packages/spec/src/kernel/functional-completeness.test.ts index 36b604d46c2..45d9a5c5322 100644 --- a/packages/spec/src/kernel/functional-completeness.test.ts +++ b/packages/spec/src/kernel/functional-completeness.test.ts @@ -173,9 +173,11 @@ describe('checkViewCompleteness — layout bindings', () => { expect(f.message).toContain('ObjectCalendar.tsx'); // ⛔ The half the correction had to PRESERVE: a warning an author meets at // authoring time earns its place by naming the key to declare, not by - // reporting that something is missing. - expect(f.message).toContain('calendar.startDateField'); - expect(f.fix).toContain('startDateField'); + // reporting that something is missing. [#22161] The verdict is one + // sentence now, so the key rides the `fix:` line printed under it; that + // `startDateField` is the block's one REQUIRED key is the rule's + // `os explain` entry (pinned in @objectstack/lint). + expect(f.fix).toContain('calendar: { startDateField:'); // Severity is untouched because it is already RULED, not because the // question is open: #16577 ruled B (comment `5634033966`, card closed // `completed` 2026-09-11) — a `calendar` route that refuses BY NAME @@ -206,9 +208,15 @@ describe('checkViewCompleteness — layout bindings', () => { expect(f.message).toContain(screen); expect(f.message).toContain(component); // ⛔ The half the correction had to PRESERVE: the prescription. Each key - // the renderer's refusal screen names is the key the body tells the - // author to declare. - for (const key of keys) expect(f.message).toContain(key); + // the renderer's refusal screen names is a key the author is told to + // declare — [#22161] on the `fix:` line now, which prints directly under + // the one-sentence verdict (`gantt.startDateField` is spelled + // `gantt: { startDateField: … }` there). + for (const key of keys) { + const [block, prop] = key.split('.'); + expect(f.fix.startsWith(`${block}: {`)).toBe(true); + expect(f.fix).toContain(`${prop}:`); + } // Severity is untouched and CONSISTENT with #16577's ruling B (comment // `5634033966`): a route that refuses BY NAME stays warning-class under // ADR-0078 §1 because both doors are loud. These rows now measure that @@ -684,4 +692,32 @@ describe('registry hygiene', () => { expect(f.message.length).toBeGreaterThan(60); } }); + + // [#22161] `os validate` / `os build` / `os dev` print a finding on every + // run, so its message is ONE verdict sentence: what is inert and the runtime + // site that makes it so (ADR-0078 §6 keeps the citation here). The long + // reasoning is the rule id's `os explain` entry in `@objectstack/lint`, + // whose own suite pins that every id below has one. Every firing variant of + // every rule, so a long body cannot come back on one branch. + it('every firing variant prints one verdict line of at most 200 characters', () => { + const object = { name: 'unit', fields: { name: { type: 'text' } } }; + const variants = [ + ...['summary', 'formula', 'lookup', 'master_detail', 'select', 'radio', 'checkboxes'] + .flatMap((type) => checkFieldCompleteness({ type })), + ...['kanban', 'calendar', 'gantt', 'timeline', 'map', 'tree'].flatMap((type) => checkViewCompleteness({ type })), + ...checkViewCompleteness({ type: 'map', map: {} }), + ...checkViewCompleteness({ type: 'tree', tree: {} }, object), + ...checkViewCompleteness({ type: 'grid', rowColor: { field: 'status' } }), + // One offending value: the list the verdict closes on is the author's data. + ...checkViewCompleteness({ type: 'grid', rowColor: { field: 'status', colors: { open: '#0f0' } } }), + ...checkWebhookCompleteness({}), + ]; + expect(new Set(variants.map((f) => f.rule))).toEqual(new Set(FUNCTIONAL_COMPLETENESS_RULES)); + expect(variants).toHaveLength(18); + for (const f of variants) { + expect(f.message, f.rule).not.toContain('\n'); + expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200); + expect(f.fix, f.rule).not.toContain('\n'); + } + }); }); diff --git a/packages/spec/src/kernel/functional-completeness.ts b/packages/spec/src/kernel/functional-completeness.ts index 40595e83ada..892aa425472 100644 --- a/packages/spec/src/kernel/functional-completeness.ts +++ b/packages/spec/src/kernel/functional-completeness.ts @@ -73,6 +73,20 @@ * * Severity follows ADR-0078 decision 1: `error` when the instance is fully * inert, `warning` when it degrades to something that partially works. + * + * ## One verdict per finding — the reasoning is `os explain ` + * + * `os validate`, `os build` and `os dev` print every finding on every run, + * and the `field/*` findings also ride the metadata write door's 422 issues + * and 2xx advisories. So `message` is ONE verdict sentence — what is inert, + * and the runtime site that makes it so (ADR-0078 §6 keeps that citation in + * the message) — and `fix` is one pastable line. The rest of what a rule + * knows — the full mechanism, each view type's measured reading, the + * carriers and exemptions — is that rule id's entry in `@objectstack/lint`'s + * `RULE_EXPLANATIONS` (`packages/lint/src/rule-explanations.ts`), which + * `os explain ` prints. ⛔ A mechanism corrected here is corrected + * in that entry in the same edit: it is the long-form carrier of the same + * reading. */ /** One completeness violation on one instance. */ @@ -83,7 +97,11 @@ export interface CompletenessFinding { severity: 'error' | 'warning'; /** Path of the omitted config relative to the item (e.g. `summaryOperations`). */ path: string; - /** What is inert, the runtime line that makes it so, and what to add. */ + /** + * One verdict sentence: what is inert and the runtime line that makes it so + * (ADR-0078 §6). The long reasoning is the rule's `os explain` entry — see + * the module doc. + */ message: string; /** One-line prescription, machine-pastable where possible. */ fix: string; @@ -153,10 +171,8 @@ export function checkFieldCompleteness(def: unknown): CompletenessFinding[] { severity: 'error', path: 'summaryOperations', message: - 'A `summary` field with no `summaryOperations` computes nothing: the engine\'s ' - + 'summary index skips it (`engine.ts` — `if (!d.summaryOperations) continue`), so it ' - + 'reads 0/null everywhere and anything derived from it is stuck at 0 — while every ' - + 'authoring surface reports success. This is the shape ADR-0078 was written for.', + 'A `summary` field with no `summaryOperations` reads 0/null everywhere: the engine\'s ' + + 'summary index skips it (`engine.ts` — `if (!d.summaryOperations) continue`)', fix: "summaryOperations: { object: '', field: '', function: 'sum' }", }); } @@ -167,9 +183,8 @@ export function checkFieldCompleteness(def: unknown): CompletenessFinding[] { severity: 'error', path: 'expression', message: - 'A `formula` field with no `expression` never computes: the engine builds its formula ' - + 'plan only from fields that HAVE one (`engine.ts` — `if (def?.type === \'formula\' && ' - + 'def.expression)`), so this field is permanently empty while parsing and publishing succeed.', + 'A `formula` field with no `expression` is always empty: the engine plans formulas only ' + + 'for fields that have one (`engine.ts` — `if (def?.type === \'formula\' && def.expression)`)', fix: 'expression: F`record. * record.`', }); } @@ -180,9 +195,8 @@ export function checkFieldCompleteness(def: unknown): CompletenessFinding[] { severity: 'error', path: 'reference', message: - `A \`${type}\` field with no \`reference\` is a relationship to nowhere: \`$expand\` ` - + 'silently skips it (`engine.ts` — `if (!referenceObject) continue`) and the record ' - + 'picker has no object to search, so the column stores raw ids that never resolve.', + `A \`${type}\` field with no \`reference\` points nowhere: \`$expand\` skips it ` + + '(`engine.ts` — `if (!referenceObject) continue`) and the record picker has no object to search', fix: "reference: ''", }); } @@ -197,9 +211,8 @@ export function checkFieldCompleteness(def: unknown): CompletenessFinding[] { severity: 'error', path: 'options', message: - `A \`${type}\` field with no \`options\` is a choice with nothing to choose: the form ` - + 'control is empty AND server-side value validation is disabled (`record-validator.ts` ' - + 'skips the check when the allowed list is empty), so any value writes through the API.', + `A \`${type}\` field with no \`options\` offers nothing to pick and validates nothing: ` + + '`record-validator.ts` skips the value check on an empty list, so any value writes through', fix: "options: [{ label: '…', value: '…' }] — or picklist: '' for a shared list", }); } else if (DEAD_WITHOUT_OPTIONS_WARNING.has(type) && !hasOptionSource) { @@ -208,10 +221,8 @@ export function checkFieldCompleteness(def: unknown): CompletenessFinding[] { severity: 'warning', path: 'options', message: - 'A `checkboxes` field with no `options` renders zero checkboxes. The validator\'s ' - + 'multi-value branch tolerates it as free-form (the `multiselect` tags mode), but a ' - + 'checkbox group is almost never meant to be free-form — declare the boxes, or use ' - + '`multiselect` if free-form tags were the intent.', + 'A `checkboxes` field with no `options` renders zero checkboxes (`record-validator.ts` ' + + 'accepts it as free-form tags); declare the boxes, or use `multiselect` if tags were meant', fix: "options: [{ label: '…', value: '…' }]", }); } @@ -641,44 +652,31 @@ const VIEW_BINDING_FIX: Readonly> = { * ⚠️ What the correction must PRESERVE is the prescription. The old sentence * was wrong about the mechanism and still right about the remedy, and the * remedy is the whole value of a warning an author meets at authoring time: - * it says which key to declare, not merely that something is missing. + * it says which key to declare, not merely that something is missing. The + * keys ride {@link VIEW_BINDING_FIX}, the `fix:` line printed directly under + * this verdict. + * + * Each entry is ONE verdict sentence: the refusal screen the author will see + * and the renderer symbol that shows it (ADR-0078 §6). The measured path + * behind it — the `ListView.tsx` branch that forwards only declared bindings, + * which keys the schema requires and which stay optional — is the + * `view/layout-without-binding` entry in `@objectstack/lint`'s + * `RULE_EXPLANATIONS`, one paragraph per type: the long-form carrier of + * each reading, corrected together with this map and the table above. */ const VIEW_BINDING_MESSAGE: Readonly> = { calendar: - 'A `calendar` view with no `calendar` block declares no date axis, and the renderer does ' - + 'not invent one: objectui\'s `ListView.tsx` calendar branch forwards only the bindings the ' - + 'view DECLARED, so `getCalendarConfig` (objectui `ObjectCalendar.tsx`) resolves `null` and ' - + 'the view renders its "Calendar configuration required" refusal screen instead of records ' - + '— it parses and publishes clean, then shows no event on any object, not just on one that ' - + 'happens to lack a field. Declare `calendar.startDateField`, the block\'s one required key; ' - + 'the event title resolves through the ADR-0079 record display-name chain when `titleField` ' - + 'is omitted.', + 'A `calendar` view with no `calendar` block shows the "Calendar configuration required" ' + + 'screen on every object (`ObjectCalendar.tsx` — `getCalendarConfig` resolves `null`)', gantt: - 'A `gantt` view with no `gantt` block declares no date axis, and the renderer does not ' - + 'invent one: objectui\'s `ListView.tsx` gantt branch forwards only the bindings the view ' - + 'DECLARED, so `getGanttConfig` (objectui `ObjectGantt.tsx`) resolves `null` without both ' - + 'dates and the view renders its "Gantt configuration required" refusal screen instead of ' - + 'tasks — it parses and publishes clean, then shows no task on any object, not just on one ' - + 'that happens to lack a field. Declare `gantt.startDateField`, `gantt.endDateField` and ' - + '`gantt.titleField`, the three keys `GanttConfigSchema` requires; `progressField` and ' - + '`dependenciesField` are optional and stay unbound when omitted.', + 'A `gantt` view with no `gantt` block shows the "Gantt configuration required" screen on ' + + 'every object (`ObjectGantt.tsx` — `getGanttConfig` resolves `null`)', timeline: - 'A `timeline` view with no `timeline` block declares no date axis, and the renderer does not ' - + 'invent one: objectui\'s `ListView.tsx` timeline branch forwards a start date only when the ' - + 'view declared one, so `ObjectTimeline.tsx` resolves no date field and renders its ' - + '"Timeline date axis required" refusal instead of records — it parses and publishes clean, ' - + 'then shows no item on any object. Only the title has a renderer default (`name`); the date ' - + 'axis never does. Declare `timeline.startDateField` and `timeline.titleField`, the two keys ' - + '`TimelineConfigSchema` requires.', + 'A `timeline` view with no `timeline` block shows the "Timeline date axis required" screen ' + + 'on every object (`ObjectTimeline.tsx` resolves no date field)', map: - 'A `map` view with no `map` block declares no coordinate binding, and the renderer does not ' - + 'guess one: objectui\'s `ListView.tsx` map branch forwards only the keys the view declared, ' - + 'and `ObjectMap.tsx` no longer guesses `location` / `latitude` / `longitude` field names, so ' - + 'its `hasCoordinateBinding` gate fails and the view renders its "Map configuration required" ' - + 'refusal instead of markers — it parses and publishes clean, then plots nothing on any ' - + 'object. Declare `map.locationField`, or both `map.latitudeField` and `map.longitudeField`; ' - + '`ListMapConfigSchema` requires neither form, so this warning is where the requirement is ' - + 'stated.', + 'A `map` view with no `map` block shows the "Map configuration required" screen on every ' + + 'object (`ObjectMap.tsx` — its `hasCoordinateBinding` gate fails)', }; /** @@ -691,9 +689,8 @@ const VIEW_BINDING_MESSAGE: Readonly> = { * an entry above, not a reworded sentence here. */ const unboundBlockMessage = (type: string, block: string): string => - `A \`${type}\` view with no \`${block}\` block is bound to nothing: the renderer falls ` - + 'back to literal default field names, which works only if the object happens to declare ' - + 'them — on any other object the view renders empty while authoring reports success.'; + `A \`${type}\` view with no \`${block}\` block falls back to literal default field names, ` + + 'so it renders empty on any object that does not declare them'; const isNonEmptyString = (v: unknown): v is string => typeof v === 'string' && v.length > 0; @@ -922,13 +919,9 @@ export function checkViewCompleteness(view: unknown, boundObject?: unknown): Com severity: 'warning', path: 'map.locationField', message: - 'A `map` view whose `map` block declares neither `locationField` nor the `latitudeField`/' - + '`longitudeField` pair is bound to nothing, and the renderer does not guess: objectui ' - + '`ObjectMap.tsx` applies its `hasCoordinateBinding` gate to a declared block exactly as ' - + 'to an absent one, so the view renders its "Map configuration required" refusal instead ' - + 'of markers — while the block parses and publishes clean, because `ListMapConfigSchema` ' - + 'requires neither form. Declare `map.locationField`, or both `map.latitudeField` and ' - + '`map.longitudeField` (half a pair is not a binding).', + 'A `map` block with neither `map.locationField` nor both `map.latitudeField` and ' + + '`map.longitudeField` shows the "Map configuration required" screen (`ObjectMap.tsx` — ' + + '`hasCoordinateBinding`)', fix: VIEW_BINDING_FIX.map, }); } @@ -941,13 +934,9 @@ export function checkViewCompleteness(view: unknown, boundObject?: unknown): Com severity: 'warning', path: 'tree.parentField', message: - 'A `tree` view with no resolvable parent pointer renders FLAT, not empty: `parentField` is ' - + 'undeclared and the bound object declares neither a `tree` field (with no `reference`, or one ' - + 'naming this object) nor a lookup/master_detail back to itself, so the renderer\'s ' - + 'auto-detection finds nothing (objectui `ObjectTree.tsx` — ' - + '`detectParentField`) and `buildForest` makes every record a root at depth 0. The result is ' - + 'a complete, correct-looking table with an expand slot that never opens, while authoring ' - + 'reports success. Declare `tree.parentField`, or add a self-referencing field to the object.', + 'A `tree` view with no resolvable parent pointer renders flat, every record at depth 0: ' + + '`parentField` is undeclared and `detectParentField` (objectui `ObjectTree.tsx`) finds no ' + + 'self-reference', fix: "tree: { parentField: '' }", }); } @@ -962,14 +951,7 @@ export function checkViewCompleteness(view: unknown, boundObject?: unknown): Com path: 'rowColor.colors', message: `A \`${type}\` view whose \`rowColor\` binds \`${field}\` and declares no \`colors\` map never ` - + 'colours a row: the grid\'s row-className resolver returns before it reads a record (objectui ' - + '`useRowColor.ts` — `if (!config?.field || !config.colors) return undefined`), so every row keeps ' - + 'the default background while parsing and publishing report success. An empty `colors: {}` is the ' - + 'same dead shape spelled out — it passes that guard and then matches no value. The map is what ' - + 'does the colouring; the field only says which value to look up. Each value is a colour NAME from ' - + 'the resolver\'s own vocabulary (`red`, `blue`, `slate`, …) or a complete Tailwind background class ' - + '(`bg-red-200`) — a hex parses, publishes and silences this very rule while still colouring ' - + `nothing (\`${VIEW_ROW_COLOR_UNRESOLVABLE_VALUE}\`).`, + + 'colours a row (`useRowColor.ts` — `if (!config?.field || !config.colors) return undefined`)', // ⛔ The prescription must name a spelling that RESOLVES. It used to // read `''`, which put the one spelling the renderer // cannot resolve in first position: the gate fired, handed the author a @@ -990,16 +972,12 @@ export function checkViewCompleteness(view: unknown, boundObject?: unknown): Com rule: VIEW_ROW_COLOR_UNRESOLVABLE_VALUE, severity: 'warning', path: 'rowColor.colors', + // The offending entries close the sentence: the list is data, as long as + // the author's map, and the fixed text before it stays one short line. message: - `A \`${type}\` view whose \`rowColor\` binds \`${field}\` declares ${dead.length} colour ` - + `value${dead.length === 1 ? '' : 's'} the renderer resolves to nothing: ${dead.join(', ')}. ` - + 'objectui `useRowColor.ts` — `colorToClass` — hands a `bg-`-prefixed literal through untouched ' - + 'and otherwise looks the lower-cased, trimmed value up in its own closed vocabulary of colour ' - + 'NAMES, returning `undefined` for everything else; Tailwind v4 has no runtime, so no class can ' - + 'be fabricated from a hex. A map like this CLEARS the `!config.colors` guard, so ' - + `\`${VIEW_ROW_COLOR_WITHOUT_COLORS}\` goes quiet, and every row still keeps its default ` - + 'background while parsing and publishing report success. Write a colour name (`red`, `blue`, ' - + '`slate`, …) or a complete Tailwind background class (`bg-red-200`).', + `A \`${type}\` view's \`rowColor\` on \`${field}\` has ${dead.length} colour ` + + `value${dead.length === 1 ? '' : 's'} the renderer resolves to nothing (\`useRowColor.ts\` — ` + + `\`colorToClass\`), which silences \`${VIEW_ROW_COLOR_WITHOUT_COLORS}\`: ${dead.join(', ')}`, fix: `rowColor: { field: '${field}', colors: { '': 'red' } }`, }); } @@ -1053,13 +1031,8 @@ export function checkWebhookCompleteness(webhook: unknown): CompletenessFinding[ severity: 'error', path: 'triggers', message: - 'A webhook with no `triggers` never fires on any path. The auto-enqueuer drops it while ' - + 'building its subscription cache (`auto-enqueuer.ts` — `if (triggers.size === 0) … return ' - + 'null`), and there is no manual fire path to reach it either: `webhook.zod.ts` ' - + 'records that the `api` trigger was removed because "no manual fire path exists — the only ' - + 'webhook HTTP surface re-queues already-failed deliveries". The webhook materializes into ' - + '`sys_webhook`, looks armed in Setup, and delivers nothing. To disable a webhook use ' - + '`isActive: false`; an empty `triggers` is not an off switch, just a dead one.', + 'A webhook with no `triggers` never fires: `auto-enqueuer.ts` drops it (`if (triggers.size ' + + '=== 0) … return null`) and no manual fire path exists; to turn it off, use `isActive: false`', fix: "triggers: ['create', 'update']", }]; }