From 55223df14f785b2dc4949fc4c48d0058c8d49c19 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 11:09:28 +0000 Subject: [PATCH 1/2] fix(lint): one-line fix hints for the six data-model-rules ids (WIP) Part of the #22161 slice-18 work. Claude-Session: https://claude.ai/code/session_013jYeZqax65nQWvPNayiCT3 Co-authored-by: Claude --- .../src/data-model-rules.fix-lines.test.ts | 212 ++++++++++++++++++ packages/lint/src/data-model-rules.ts | 54 ++--- packages/lint/src/rule-explanations.ts | 14 +- 3 files changed, 249 insertions(+), 31 deletions(-) create mode 100644 packages/lint/src/data-model-rules.fix-lines.test.ts diff --git a/packages/lint/src/data-model-rules.fix-lines.test.ts b/packages/lint/src/data-model-rules.fix-lines.test.ts new file mode 100644 index 00000000000..03ee7fe969e --- /dev/null +++ b/packages/lint/src/data-model-rules.fix-lines.test.ts @@ -0,0 +1,212 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// [#22161] Every `data-model-rules.ts` finding's `fix` is one remedy line of at +// most 200 characters. `os lint --fix` prints it as the `→ fix:` line, and the +// three ADR-0120 uniqueness ids carry it as the `hint` `os validate` and +// `os build` print. What the hints used to explain is `os explain` text. +// +// The three `lintDataModel` sweep ids (`object/missing-name-field`, +// `field/select-missing-options`, `rollup/non-numeric-aggregand`) are never +// adapted into an `AuthoringFinding`, so a census that reads `hint` never sees +// them. This file reads `fix` directly, over every arm each rule has, with long +// object and field names where a name is interpolated. + +import { describe, expect, it } from 'vitest'; + +import { + lintDataModel, + lintLegacyOrganizationComposites, + lintUniqueDeclarations, + lintUnscopedDeclaredIndexes, + ROLLUP_NON_NUMERIC_AGGREGAND, + UNIQUE_DOUBLE_DECLARATION, + UNIQUE_LEGACY_ORGANIZATION_COMPOSITE, + UNIQUE_UNSCOPED_DECLARED_INDEX, + type LintIssue, +} from './data-model-rules.js'; +import { explainRule } from './rule-explanations.js'; + +const MISSING_NAME_FIELD = 'object/missing-name-field'; +const SELECT_MISSING_OPTIONS = 'field/select-missing-options'; + +const IDS = [ + UNIQUE_UNSCOPED_DECLARED_INDEX, + UNIQUE_DOUBLE_DECLARATION, + UNIQUE_LEGACY_ORGANIZATION_COMPOSITE, + MISSING_NAME_FIELD, + SELECT_MISSING_OPTIONS, + ROLLUP_NON_NUMERIC_AGGREGAND, +]; + +const LONG_OBJECT = 'acme_customer_purchase_order_header'; +const LONG_FIELD = 'primary_contact_email_address'; + +const fired: LintIssue[] = []; +const record = (issues: T[]): T[] => { + fired.push(...issues.filter((i) => IDS.includes(i.rule))); + return issues; +}; +const fixOf = (issues: LintIssue[], rule: string): string => { + const hit = issues.find((i) => i.rule === rule); + expect(hit, `${rule} fired`).toBeDefined(); + return hit!.fix ?? ''; +}; + +const twice = (fieldUnique: unknown, indexUnique: unknown, field = 'sku') => [ + { + name: LONG_OBJECT, + fields: { [field]: { type: 'text', unique: fieldUnique } }, + indexes: [{ name: 'uniq_sku', fields: [field], unique: indexUnique }], + }, +]; + +const rollup = (childType: string, parent = 'invoice', fn = 'max') => [ + { + name: parent, + fields: { + name: { type: 'text' }, + rolled_up: { type: 'summary', summaryOperations: { object: 'invoice_line', field: 'f', function: fn } }, + }, + }, + { + name: 'invoice_line', + fields: { + name: { type: 'text' }, + inv: { type: 'master_detail', reference: parent, required: true, deleteBehavior: 'cascade' }, + f: { type: childType }, + }, + }, +]; + +describe('[#22161] data-model-rules.ts — one remedy line per fix', () => { + it('unique/unscoped-declared-index: the two scope words, nothing else', () => { + const issues = record( + lintUnscopedDeclaredIndexes([ + { + name: LONG_OBJECT, + fields: {}, + indexes: [ + { name: 'uniq_org_email', fields: ['organization_id', 'email'], unique: true }, + { fields: ['provider_id', 'account_id', 'region', 'channel'], unique: true }, + ], + }, + ]), + ); + expect(issues).toHaveLength(2); + for (const issue of issues) { + expect(issue.fix).toBe( + "State the scope: `unique: 'global'` (installation-wide — the exact index bare `true` built) or " + + "`unique: 'organization'` (one holder per organization).", + ); + } + }); + + it('unique/double-declaration: one line per quadrant', () => { + expect(fixOf(record(lintUniqueDeclarations(twice('global', 'global'))), UNIQUE_DOUBLE_DECLARATION)).toBe( + "Keep ONE spelling of installation-wide uniqueness: `unique: 'global'` on 'sku', or the declared index — not both.", + ); + expect(fixOf(record(lintUniqueDeclarations(twice(true, 'organization'))), UNIQUE_DOUBLE_DECLARATION)).toBe( + "Keep ONE spelling of per-organization uniqueness: `unique: 'organization'` on 'sku' (preferred), or the " + + 'declared index — not both.', + ); + const contradiction = + "Pick ONE scope and say it once: keep `unique: 'global'` and drop the per-organization side, or set " + + "`unique: 'organization'` and drop the installation-wide one."; + expect(fixOf(record(lintUniqueDeclarations(twice(true, 'global'))), UNIQUE_DOUBLE_DECLARATION)).toBe(contradiction); + expect(fixOf(record(lintUniqueDeclarations(twice('global', 'organization'))), UNIQUE_DOUBLE_DECLARATION)).toBe( + contradiction, + ); + // Long column names, on the two arms that name the column. + record(lintUniqueDeclarations(twice('global', 'global', LONG_FIELD))); + record(lintUniqueDeclarations(twice('organization', 'organization', LONG_FIELD))); + record(lintUniqueDeclarations(twice(true, 'global', LONG_FIELD))); + }); + + it('unique/legacy-organization-composite: the respelling and its cost, whatever the columns', () => { + const issues = record( + lintLegacyOrganizationComposites([ + { + name: LONG_OBJECT, + fields: {}, + indexes: [{ name: 'uk_team', fields: ['name', 'organization_id'], unique: true }], + }, + { + name: LONG_OBJECT, + tenancy: { tenantField: 'tenant_reference_column' }, + fields: {}, + indexes: [{ fields: ['tenant_reference_column', 'code', 'region', 'channel'], unique: 'global' }], + }, + ]), + ); + expect(issues).toHaveLength(2); + for (const issue of issues) { + expect(issue.fix).toBe( + "Respell it `unique: 'organization'` and keep `fields` exactly as they are; opting in is a " + + '`recreate_index` gated by the duplicate pre-flight probe (ADR-0120 D4).', + ); + } + }); + + it('object/missing-name-field: the pointer, and where the titleFormat reasoning lives', () => { + const fix = fixOf(record(lintDataModel([{ name: LONG_OBJECT, fields: { amount: { type: 'number' } } }])), MISSING_NAME_FIELD); + expect(fix).toBe( + "Set `nameField: ''` (ADR-0079's title pointer) to a stored text/autonumber field or a " + + "`returnType: 'text'` formula; `titleFormat` does not count (`os explain title-format-retired`).", + ); + // The command the line names answers, and says why `titleFormat` does not count. + expect(explainRule('title-format-retired')!.paragraphs.join('\n')).toContain('render-only'); + }); + + it('field/select-missing-options: one line on every option-field type', () => { + for (const type of ['select', 'multiselect', 'radio', 'enum']) { + const fix = fixOf( + record(lintDataModel([{ name: LONG_OBJECT, nameField: 'name', fields: { name: { type: 'text' }, [LONG_FIELD]: { type } } }])), + SELECT_MISSING_OPTIONS, + ); + expect(fix, type).toBe( + "Give the field ONE options source: inline `options: [{ label: '…', value: '…' }]` or " + + "`picklist: ''` naming a shared list — never both: `options` beside `picklist` is refused.", + ); + } + }); + + it('rollup/non-numeric-aggregand: one line naming the child type, not the parent', () => { + expect(fixOf(record(lintDataModel(rollup('datetime', LONG_OBJECT))), ROLLUP_NON_NUMERIC_AGGREGAND)).toBe( + "Aggregate a numeric or boolean child field, or use function: 'count'; a roll-up cannot store a " + + 'datetime, so declare a datetime field and maintain it from a flow.', + ); + for (const type of ['date', 'time', 'text', 'select', 'multiselect', 'lookup', 'json', 'autonumber', 'textarea']) { + for (const fn of ['min', 'max']) record(lintDataModel(rollup(type, LONG_OBJECT, fn))); + } + }); + + it('every fix the cases above fired is one line of at most 200 characters', () => { + // The coverage control first: all six ids fired. + expect([...new Set(fired.map((f) => f.rule))].sort()).toEqual([...IDS].sort()); + for (const f of fired) { + expect(f.fix, f.rule).toBeTruthy(); + expect(f.fix, f.rule).not.toContain('\n'); + expect(f.fix!.length, `${f.rule}: ${f.fix}`).toBeLessThanOrEqual(200); + } + }); + + it('`os explain` carries what the uniqueness fixes no longer say', () => { + const facts: Record = { + [UNIQUE_UNSCOPED_DECLARED_INDEX]: [ + 'prepend the NULL-safe organization key part at registration', + 'Run `os migrate meta --from 17` to list the mechanical edits for existing sources', + ], + [UNIQUE_DOUBLE_DECLARATION]: ['preferably field-level on the column, or on the declared index'], + [UNIQUE_LEGACY_ORGANIZATION_COMPOSITE]: [ + 'NULL-safe in place rather than prepending a second organization key part', + '"one email per organization"', + '`recreate_index` drift op', + 'block it with a report rather than failing a boot', + ], + }; + for (const [rule, list] of Object.entries(facts)) { + const text = explainRule(rule)!.paragraphs.join('\n'); + for (const fact of list) expect(text, `${rule} explanation names ${fact}`).toContain(fact); + } + }); +}); diff --git a/packages/lint/src/data-model-rules.ts b/packages/lint/src/data-model-rules.ts index f9652215756..dd03cc6b34c 100644 --- a/packages/lint/src/data-model-rules.ts +++ b/packages/lint/src/data-model-rules.ts @@ -464,15 +464,12 @@ export function lintUnscopedDeclaredIndexes(objects: any[]): LocatedLintIssue[] `"${obj.name}" index${indexLabel} [${cols}] has bare \`unique: true\`, an unstated scope protocol 18 ` + `refuses (ADR-0120): it built the index installation-wide`, path: `objects[${i}].indexes[${j}]`, + // [#22161] One remedy line. What `'organization'` makes the driver + // build, and the `os migrate meta` route for existing sources (the + // house sentence), are `os explain` text. fix: - `State the scope: \`unique: 'global'\` (installation-wide — the exact index bare \`true\` built) or ` + - `\`unique: 'organization'\` (one holder per organization — the driver prepends the NULL-safe ` + - `organization key part at registration). ` + - // The house sentence, plain-quoted (not a template literal) so this site - // is judged by `retired-key-migrate-sentence.test.ts`'s scan of this - // package: escaped template backticks hide it from that scan. - 'Run `os migrate meta --from 17` to list the mechanical edits for existing sources; ' - + '`--write` applies the ones it can prove, and you apply the rest by hand.', + "State the scope: `unique: 'global'` (installation-wide — the exact index bare `true` built) or " + + "`unique: 'organization'` (one holder per organization).", }); } } @@ -554,17 +551,18 @@ export function lintUniqueDeclarations(objects: any[]): LocatedLintIssue[] { fix = fScope === 'global' ? `Keep ONE spelling of installation-wide uniqueness: \`unique: 'global'\` on '${name}', or the declared index — not both.` - : `Keep ONE spelling of per-organization uniqueness: \`unique: 'organization'\` on '${name}' (preferred), or the declared \`'organization'\` index — not both.`; + : `Keep ONE spelling of per-organization uniqueness: \`unique: 'organization'\` on '${name}' (preferred), or the declared index — not both.`; } else { // Different scopes — the installation-wide side is physically stricter // and wins; the per-organization intent is dead on arrival. message = `"${obj.name}.${name}" field ${fieldSpelling} and index${indexLabel} ${indexSpelling} CONTRADICT: ` + `the installation-wide one wins, so the per-organization intent is silently dead`; + // [#22161] One remedy line; where each spelling may live is + // `os explain` text. fix = - `Pick ONE scope and say it once: for installation-wide uniqueness keep \`unique: 'global'\` and drop the ` + - `per-organization declaration; for per-organization uniqueness set \`unique: 'organization'\` (field-level on ` + - `'${name}', or on the declared index) and drop the installation-wide one.`; + "Pick ONE scope and say it once: keep `unique: 'global'` and drop the per-organization side, or set " + + "`unique: 'organization'` and drop the installation-wide one."; } issues.push({ @@ -632,7 +630,6 @@ export function lintLegacyOrganizationComposites(objects: any[]): LocatedLintIss if (!cols.includes(tenantColumn)) continue; const indexLabel = typeof idx?.name === 'string' && idx.name.trim() ? ` '${idx.name.trim()}'` : ''; - const rest = cols.filter((c: string) => c !== tenantColumn); issues.push({ severity: 'warning', rule: UNIQUE_LEGACY_ORGANIZATION_COMPOSITE, @@ -643,13 +640,11 @@ export function lintLegacyOrganizationComposites(objects: any[]): LocatedLintIss `"${obj.name}" index${indexLabel} [${cols.join(', ')}] lists the organization column, and SQL ` + `UNIQUE is NULL-distinct: on every row whose '${tenantColumn}' is NULL it enforces nothing`, path: `objects[${i}].indexes[${j}]`, + // [#22161] One remedy line; what the respelling builds, what the + // probe reports and why leaving it is valid are `os explain` text. fix: - `State the scope instead: \`unique: 'organization'\` on this index (keep \`fields\` exactly as they are — the ` + - `driver makes the listed '${tenantColumn}' NULL-safe in place rather than prepending a second organization key ` + - `part). ${rest.length > 0 ? `The constraint then really is "one ${rest.join(' + ')} per organization". ` : ''}` + - `Opting in is a physical tightening: it surfaces as a \`recreate_index\` drift op gated by the duplicate ` + - `pre-flight probe (ADR-0120 D4), so pre-existing duplicate NULL-organization rows block it with a report ` + - `rather than failing a boot. Leaving it as-is stays valid indefinitely and forces no drift.`, + "Respell it `unique: 'organization'` and keep `fields` exactly as they are; opting in is a " + + '`recreate_index` gated by the duplicate pre-flight probe (ADR-0120 D4).', }); } } @@ -737,11 +732,11 @@ export function lintDataModel(objects: any[]): LintIssue[] { rule: 'object/missing-name-field', message: `Object "${obj.name}" has no nameField and no name-like field — records will display as raw IDs`, path: `${objPath}.fields`, + // [#22161] One remedy line; why `titleFormat` does not count is + // `title-format-retired`'s `os explain` text. fix: - `Set \`nameField: ''\` — ADR-0079's canonical primary-title pointer — to a stored ` + - `text/autonumber field, or to a formula field with \`returnType: 'text'\` for a composite ` + - `title. A \`titleFormat\` template does NOT count: it is retired (ADR-0079) and render-only, ` + - `so the server can neither return nor query the title it renders.`, + "Set `nameField: ''` (ADR-0079's title pointer) to a stored text/autonumber field or a " + + "`returnType: 'text'` formula; `titleFormat` does not count (`os explain title-format-retired`).", }); } @@ -771,9 +766,8 @@ export function lintDataModel(objects: any[]): LintIssue[] { message: `${type} field "${obj.name}.${fieldName}" has no options`, path: `${fieldPath}.options`, fix: - "Give the field ONE options source: inline `options: [{ label: '…', value: '…' }]`, or " + - "`picklist: ''` naming a shared list — never both, because a field declaring " + - '`options` beside `picklist` is refused.', + "Give the field ONE options source: inline `options: [{ label: '…', value: '…' }]` or " + + "`picklist: ''` naming a shared list — never both: `options` beside `picklist` is refused.", }); } } @@ -832,11 +826,11 @@ export function lintDataModel(objects: any[]): LintIssue[] { `${fn}(${childName}.${childFieldName}), a ${childType} field: ${fn} answers in the ` + `child field's own type, but a summary field stores a finite number`, path: `${fieldPath}.summaryOperations.field`, + // [#22161] One remedy line; why min/max over those answer with a + // number and `count` reads no value is `os explain` text. fix: - `Aggregate a numeric or boolean child field instead (min/max over those answer ` + - `with a number), or use function: 'count' — which reads no value off the field. ` + - `To carry a ${childType} on "${obj.name}", declare a ${childType} field and ` + - `maintain it from a flow; a roll-up cannot store one.`, + `Aggregate a numeric or boolean child field, or use function: 'count'; a roll-up cannot ` + + `store a ${childType}, so declare a ${childType} field and maintain it from a flow.`, }); } } diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 8f76e4d0ea9..88ce9f4017e 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -2575,7 +2575,12 @@ const UNIQUE_LEGACY_ORGANIZATION_COMPOSITE_EXPLANATION: RuleExplanation = { 'row.', 'Respelled `unique: \'organization\'`, the NULL rows become one platform bucket ' + '(`COALESCE(organization_id, \'__global__\')`) that is unique among themselves, which is what ' + - 'closes the hole.', + 'closes the hole. The respelling keeps `fields` exactly as they are: the driver makes the listed ' + + 'organization column NULL-safe in place rather than prepending a second organization key part, ' + + 'so `[organization_id, email]` then really is "one email per organization".', + 'Opting in is a physical tightening. It surfaces as a `recreate_index` drift op gated by the ' + + 'duplicate pre-flight probe (ADR-0120 D4), so pre-existing duplicate NULL-organization rows ' + + 'block it with a report rather than failing a boot.', 'Advisory, and never auto-fixed (ADR-0120 D5c): the legacy spelling stays valid indefinitely ' + 'and forces no drift, and opting in is a real physical tightening that goes through the D4 ' + 'ceremony, because rows the void constraint admitted may still be there. Not reported: ' + @@ -2600,6 +2605,10 @@ const UNIQUE_UNSCOPED_DECLARED_INDEX_EXPLANATION: RuleExplanation = { '`ObjectSchema.create` and the runtime save door — and this rule refuses it where nothing ' + 'parses: `os lint`, which judges the normalized stack. Stored metadata that still carries it ' + 'converts to `unique: \'global\'`, which builds the same physical index.', + 'Either word is a complete fix. `\'global\'` keeps the index exactly as bare `true` built it; ' + + '`\'organization\'` makes the driver prepend the NULL-safe organization key part at ' + + 'registration, so each organization may hold the value once. ' + + 'Run `os migrate meta --from 17` to list the mechanical edits for existing sources; `--write` applies the ones it can prove, and you apply the rest by hand.', ], }; @@ -2615,6 +2624,9 @@ const UNIQUE_DOUBLE_DECLARATION_EXPLANATION: RuleExplanation = { 'the per-organization constraint can never be tripped and one of the two declared intents is ' + 'silently dead. The same scope on both sides is REDUNDANT — the same index declared twice — ' + 'and dropping one gives the intent a single home.', + 'Either way the fix leaves one spelling. Installation-wide uniqueness is `unique: \'global\'`, on ' + + 'the field or on the declared index; per-organization uniqueness is `unique: \'organization\'`, ' + + 'preferably field-level on the column, or on the declared index.', 'A composite declared index is not judged here: listing the organization column is the legacy ' + 'organization spelling, `unique/legacy-organization-composite`\'s question. Advisory: the ' + 'stack is well-defined, and the cost is an intent that never takes effect.', From 492ab98ddb649fdde0c8ae08a35d497efb1792e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 11:14:36 +0000 Subject: [PATCH 2/2] chore(changeset): lint patch for the data-model-rules fix lines (WIP) Claude-Session: https://claude.ai/code/session_013jYeZqax65nQWvPNayiCT3 Co-authored-by: Claude --- .changeset/22161-s18-data-model-fix-lines.md | 22 ++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .changeset/22161-s18-data-model-fix-lines.md diff --git a/.changeset/22161-s18-data-model-fix-lines.md b/.changeset/22161-s18-data-model-fix-lines.md new file mode 100644 index 00000000000..8ad20981170 --- /dev/null +++ b/.changeset/22161-s18-data-model-fix-lines.md @@ -0,0 +1,22 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): the six `data-model-rules.ts` rule ids print a fix line of one remedy sentence, and `os explain RULE_ID` carries what those lines used to explain + +Clause-②: no + +- **Shorter fix lines.** Each finding of these six rule ids now carries a `fix` of one remedy sentence of at most 200 characters on every variant measured, including long object and column names. The three ADR-0120 uniqueness ids carry it as the `hint` that `os validate` and `os build` print after `fix: `; all six print it as `os lint --fix`'s `→ fix:` line. The longest of each, FROM → TO (characters of the fix text alone): + - `unique/legacy-organization-composite` 592 → 161; + - `unique/unscoped-declared-index` 380 → 150; + - `object/missing-name-field` 327 → 188; + - `unique/double-declaration` 303 → 159 (the contradiction arm; the two redundancy arms were already short and now drop one word); + - `rollup/non-numeric-aggregand` 300 → 165 (the line no longer names the parent object, so its length no longer grows with that name); + - `field/select-missing-options` 207 → 180. +- **What moved.** The text the fix lines stopped saying is in each id's `RULE_EXPLANATIONS` entry, which `os explain RULE_ID` prints: + - `unique/unscoped-declared-index`: what `unique: 'organization'` makes the driver build, and the `os migrate meta --from 17` route for existing sources (one new paragraph); + - `unique/legacy-organization-composite`: that the respelling makes the listed organization column NULL-safe in place, what the constraint then means, and that duplicate NULL-organization rows block the `recreate_index` with a report rather than failing a boot (one paragraph extended, one new); + - `unique/double-declaration`: where each scope word may be written (one new paragraph). + - `object/missing-name-field` now points at `os explain title-format-retired`, whose entry already says why a `titleFormat` template does not count. `field/select-missing-options` and `rollup/non-numeric-aggregand` lost only wording; the rollup entry already carries the reasoning. +- **Runtime publish gate:** none of the six runs there. The three uniqueness entries are `cli`-only registry rules, and the other three run only in `os lint`'s `lintDataModel` sweep. So no 422 `issues[].hint` and no 2xx `advisories[].hint` changes. +- **Unchanged:** every `message` (the verdict), rule id, severity and `path`, and what each rule accepts or refuses.