diff --git a/.changeset/field-notnull-prescribes-storage-not-required.md b/.changeset/field-notnull-prescribes-storage-not-required.md new file mode 100644 index 00000000000..30bd42c35e4 --- /dev/null +++ b/.changeset/field-notnull-prescribes-storage-not-required.md @@ -0,0 +1,19 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): `FieldSchema` no longer prescribes `required` for `notNull` / `not_null` — the flattened column-constraint spellings now name `storage: { notNull: true }` (#16867) + +Writing `notNull: true` (or `not_null: true`) on a field was refused — correctly — and then told to write `required` instead, via a rename row in `FieldSchema`'s alias table. `required` is the one key ADR-0113 exists to say is **not** the column constraint. `required`'s own description in the same file states the opposite of what the rename prescribed: *"NOT a column constraint — the physical NOT NULL is a separate explicit opt-in (`storage.notNull`)"*. + +The failure mode was not the refusal — that fired, loudly, and did its job. It was the **remedy**: an author reaching for a NOT NULL column complied, wrote `required: true`, and received a nullable column plus a write-time gate, with nothing downstream to refuse it. The refusal read as though it had been satisfied. + +All three flattened spellings — `notNull`, `not_null`, and `storageNotNull`, which already carried the correct sentence — now get one prescription naming the real key: + +> physical column constraints live under `storage` — write `storage: { notNull: true }` (ADR-0113). There is no flat spelling of it: post-17 a column is NOT NULL because its author wrote that nested key, and for no other reason. It is NOT `required`, which is the WRITE contract (an insert must provide a value; an update may not null it out) and deliberately does NOT imply the column constraint — `required: true` alone leaves the column nullable. Write whichever of the two you meant, or both. + +Both halves are named on purpose: the defect being repaired is that the author cannot tell which of the two axes they are getting, so a prescription naming only the column half would have fixed the measured direction and opened the mirror-image one. + +**No accepted key moves.** `notNull` and `not_null` were refused before this change and are refused after it — a `guidance` / `guidanceSets` table decorates a rejection and never admits a key. Only the sentence attached to the refusal changed. `storage: { notNull: true }` parsed before and parses now; `isRequired` and `mandatory` are genuine spellings of the write contract, ADR-0113 moved neither, and both still rename onto `required`. + +One mechanical note for anyone repairing a table like this: the entry moved from `aliases` to `guidanceSets`, not to exact `guidance`. `aliases` is indexed by `aliasProbe` (case- and separator-folded, so one row covered `not_null` too) while exact `guidance` is matched case-sensitively on the authored spelling — a lone `guidance.notNull` row would have quietly dropped `not_null` onto the edit-distance fallback. The two spellings are pinned separately for exactly that reason. diff --git a/packages/spec/src/data/field.test.ts b/packages/spec/src/data/field.test.ts index de2ee1b18c7..66306c92e2a 100644 --- a/packages/spec/src/data/field.test.ts +++ b/packages/spec/src/data/field.test.ts @@ -1905,6 +1905,91 @@ describe('ADR-0113 — required is a write contract; storage.notNull is the colu }); }); +/** + * #16867 — the flattened column-constraint spellings must not be renamed onto + * `required`. + * + * The defect these pin against was not a silent one: the refusal fired, loudly, + * and then prescribed `required` — the one key ADR-0113 exists to say is NOT the + * column constraint. The author complied and got a nullable column plus a write + * gate, with nothing downstream to refuse it. So the assertions come in pairs: + * the ADR-0113 sentence is PRESENT, and the rename to `required` is ABSENT. + * Asserting only the first would pass on a message that carried both. + * + * `not_null` is pinned separately from `notNull` on purpose. The two channels + * fold differently — `aliases` is indexed by `aliasProbe` (case and `_`/`-` + * folded), exact `guidance` is not — so a repair that moved the entry between + * them without a pattern would keep `notNull` green while `not_null` fell + * through to the edit-distance fallback, and no single-spelling pin would see it. + */ +describe('ADR-0113 / #16867 — flat `notNull` spellings prescribe `storage.notNull`, never `required`', () => { + const refusalMessage = (field: Record): string => { + const r = FieldSchema.safeParse({ type: 'text', label: 'F', ...field }); + expect(r.success).toBe(false); + if (r.success) throw new Error('unreachable — the key was ADMITTED, not refused'); + const issue = r.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(issue, 'the key must be refused as unrecognized, not accepted').toBeDefined(); + return String(issue!.message); + }; + + // The three flattened spellings an author actually reaches for. `notNull` and + // `not_null` are what the card measured being renamed onto `required`; + // `storageNotNull` already carried the sentence and must keep it. + for (const spelling of ['notNull', 'not_null', 'storageNotNull']) { + it(`\`${spelling}\` is refused WITH the ADR-0113 sentence and WITHOUT a rename to \`required\``, () => { + const message = refusalMessage({ [spelling]: true }); + + // Lit: the correct target is named, nested spelling and all. + expect(message).toContain('storage: { notNull: true }'); + expect(message).toContain('ADR-0113'); + + // Dark: the rename channel did not answer. `Did you mean` is the rename + // channel's own template (`suggestions.zod.ts`), so its absence is what + // proves the alias row is gone rather than merely outvoted. + expect(message).not.toMatch(/Did you mean/); + expect(message).not.toMatch(/→ `required`/); + }); + } + + it('names the write contract too, so an author who meant `required` is not sent the other way', () => { + // The defect is that the author cannot tell which of the two axes they are + // getting. A prescription naming only the column half would fix the + // measured direction and open the mirror-image one. + const message = refusalMessage({ notNull: true }); + expect(message).toContain('`required`'); + expect(message).toMatch(/WRITE contract/); + }); + + it('the genuine write-contract synonyms still RENAME onto `required` (the set is anchored)', () => { + // Guards the blast radius of the pattern: ADR-0113 moved neither of these, + // and `/^(?:storage[_-]?)?not[_-]?null$/i` must not reach them. + for (const spelling of ['isRequired', 'mandatory']) { + const message = refusalMessage({ [spelling]: true }); + expect(message).toMatch(/Did you mean/); + expect(message).toContain('`required`'); + expect(message).not.toContain('ADR-0113'); + } + }); + + it('clause \u2461 — both spellings are REFUSED, before and after; only the sentence moved', () => { + // A `guidance` / `guidanceSets` table decorates a rejection and never + // admits a key (`strict-object.ts`: it "runs only from the + // `unrecognized_keys` path"). This pin is what would go red if a future + // edit turned either spelling into an accepted key. + for (const spelling of ['notNull', 'not_null', 'storageNotNull']) { + const r = FieldSchema.safeParse({ type: 'text', label: 'F', [spelling]: true }); + expect(r.success, `\`${spelling}\` must stay REFUSED — it is not an authorable key`).toBe(false); + } + }); + + it('the instrument can still say "fine" — the real keys parse', () => { + // Negative controls, so a pin above cannot pass by refusing everything. + expect(FieldSchema.parse({ type: 'text', label: 'F' }).storage).toBeUndefined(); + expect(FieldSchema.parse({ type: 'text', label: 'F', storage: { notNull: true } }).storage?.notNull) + .toBe(true); + }); +}); + describe('FieldSchema — authored `radio` + `multiple: true` is REFUSED (#11437, maintainer ruling 2026-08-22 on objectui#4015, Option C)', () => { // Option C rejects the contradiction at the entrance: the data layer // honoured the flag while the widget rendered a single-value radio group — diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index a05e28e318b..977b998b548 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { retiredKey } from '../shared/retired-key'; import { strictObject } from '../shared/strict-object'; +import type { KeySetGuidance } from '../shared/suggestions.zod'; // Package-internal, like `strict-object` itself — the `shared/index.ts` barrel // deliberately does not re-export it, so nothing about the public API surface // moves. No cycle back into this file: that module's only runtime import is @@ -846,6 +847,68 @@ export const InlineGridColumnSchema = lazySchema(() => strictObject({ requiredWhen: ExpressionInputSchema.optional().describe('Predicate (CEL) — the cell is required when TRUE. Same `record` + `parent` scope as `readonlyWhen`. PRESENTATION ONLY: this flags the cell inline-invalid in the grid; nothing on the write path reads it. The server-enforced contract is the child FIELD\'s own `requiredWhen` — a transition gate, see `Field.requiredWhen` — which hydration copies onto an identity-only column, so declaring the requirement here alone enforces nothing.'), })); +/** + * The FLATTENED spellings of the column constraint — `notNull`, `not_null`, + * `storageNotNull` — answered with one prescription that names the real key. + * + * ## Why this is prose and not a rename + * + * ADR-0113 split one knob into two axes, and adjudicated the spelling of each: + * `required` is the write-time contract, and the physical constraint is + * `storage: { notNull: true }` (Q1, decided 2026-07-30). The target is + * therefore a NESTED key, and `aliases` renames onto a flat one — the same + * reason `currency` is answered in prose a few lines below. + * + * ## Why it may not rename onto `required` (#16867) + * + * It used to: `notNull: 'required'` sat in the alias table beside `isRequired` + * and `mandatory`, and because `aliases` is consulted only AFTER this channel + * declines, an author who wrote `notNull` was told to write the one key ADR-0113 + * exists to say is not the column constraint. `required`'s own `.describe()` + * below states the opposite in the same file: *"NOT a column constraint — the + * physical NOT NULL is a separate explicit opt-in (`storage.notNull`)"*. + * + * The cost of that rename was not a wording nit. The refusal was loud and did + * its job; its REMEDY produced the wrong end state, and that end state was + * SILENT — the author complied, got `required: true`, and received a nullable + * column plus a write gate with nothing downstream to refuse it. The same + * conflation was withdrawn from the conversion registry on the same reading + * (`conversions/registry.ts`, maintainer ruling 2026-09-08): *"A conversion + * cannot be what decides a column constraint — that is the author's explicit + * act."* + * + * ## Why a SET and not three `guidance` rows + * + * `guidance` is matched case-sensitively on the exact authored spelling, while + * `aliases` is indexed by `aliasProbe` (case- and separator-folded). Moving + * `notNull` from one channel to the other therefore silently narrows what it + * covers: the single alias row answered `not_null` too, and a lone + * `guidance.notNull` row would not have. The pattern restores the fold, and + * fires once per message however many members were written. + * + * ## Both halves are named on purpose + * + * The prescription states what `required` is as well as what `storage.notNull` + * is, because the defect being repaired is precisely that the author cannot + * tell which of the two they are getting — answering only one half would leave + * the author who meant the write contract to guess in the other direction. + */ +const COLUMN_CONSTRAINT_FLAT_KEYS: KeySetGuidance = { + name: 'COLUMN_CONSTRAINT_FLAT_KEYS', + // Anchored, so it claims only the flattened column-constraint spellings and + // cannot reach a declared key: `required`, `requiredWhen` and `storage` + // itself are all outside it. + keys: /^(?:storage[_-]?)?not[_-]?null$/i, + examples: ['notNull', 'not_null', 'storageNotNull'], + prescription: + 'physical column constraints live under `storage` — write `storage: { notNull: true }` ' + + '(ADR-0113). There is no flat spelling of it: post-17 a column is NOT NULL because its ' + + 'author wrote that nested key, and for no other reason. It is NOT `required`, which is ' + + 'the WRITE contract (an insert must provide a value; an update may not null it out) and ' + + 'deliberately does NOT imply the column constraint — `required: true` alone leaves the ' + + 'column nullable. Write whichever of the two you meant, or both.', +}; + export const FieldSchema = lazySchema(() => { const base = strictObject({ surface: 'this field', @@ -857,7 +920,10 @@ export const FieldSchema = lazySchema(() => { title: 'label', displayName: 'label', help: 'inlineHelpText', helpText: 'inlineHelpText', hint: 'inlineHelpText', tooltip: 'inlineHelpText', default: 'defaultValue', initialValue: 'defaultValue', - isRequired: 'required', mandatory: 'required', notNull: 'required', + // `notNull` is NOT here, deliberately — see COLUMN_CONSTRAINT_FLAT_KEYS + // below. `isRequired` / `mandatory` stay: both are genuine spellings of the + // WRITE contract, and ADR-0113 moved neither. + isRequired: 'required', mandatory: 'required', isUnique: 'unique', values: 'options', choices: 'options', picklist: 'options', selectOptions: 'options', relatedTo: 'reference', referenceTo: 'reference', target: 'reference', targetObject: 'reference', lookupObject: 'reference', @@ -908,13 +974,12 @@ export const FieldSchema = lazySchema(() => { referenceFilters: '`referenceFilters` (string[]) was removed in the 16.x line — the lookup picker only ' + 'ever read the structured form. Use `lookupFilters: [{ field, operator, value }]`.', - // `notNull` is aliased to `required` above for the common case, but ADR-0113 - // makes the two deliberately distinct and the distinction IS the point, so - // the flattened spelling gets its own sentence rather than a rename. - storageNotNull: - 'physical column constraints live under `storage` — write `storage: { notNull: true }` ' - + '(ADR-0113). `required` is the WRITE contract and deliberately does not imply the column ' - + 'constraint.', + // `notNull` / `not_null` / `storageNotNull` are answered by + // COLUMN_CONSTRAINT_FLAT_KEYS (a `guidanceSets` entry, declared above this + // schema) rather than by a row here: exact `guidance` is matched + // CASE-SENSITIVELY on the authored spelling, so a row per spelling is the + // only way this channel can cover a family, and `not_null` was the spelling + // it would have missed. tracked: '`tracked` is not a field key — per-field timeline tracking is `trackHistory: true` (ADR-0052 §5b).', // Prose rather than a rename, because this surface declares BOTH forms and // the two answers have opposite polarity: renaming onto `visibleWhen` sends @@ -927,6 +992,7 @@ export const FieldSchema = lazySchema(() => { + 'per-record CEL predicate is `visibleWhen` (shown only when TRUE). Its siblings are ' + '`readonlyWhen` and `requiredWhen`.', }, + guidanceSets: [COLUMN_CONSTRAINT_FLAT_KEYS], }, { /** Identity */ name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Machine name (snake_case)').optional(),