diff --git a/.changeset/7650-retired-dialect-choke-point.md b/.changeset/7650-retired-dialect-choke-point.md index b80619820e..210287cfb4 100644 --- a/.changeset/7650-retired-dialect-choke-point.md +++ b/.changeset/7650-retired-dialect-choke-point.md @@ -8,8 +8,26 @@ Canonicalize the retired object-schema dialect once, at the ingestion choke poin `normalizeSchemaReferenceKeys` now has two arms. The `reference` / `reference_to` pair is unchanged. The new arm folds any key a served field def carries that `@objectstack/spec`'s `FieldSchema` does **not** declare, but whose snake/camel twin it -does — `display_field` onto `displayField`, `description_field` onto `descriptionField`, -`lookup_filters` onto `lookupFilters`, and `lookup_columns` onto `lookupColumns`. +does — `display_field` onto `displayField`, `lookup_filters` onto `lookupFilters`, and so +on. + +**The accepted set is MEASURED, not enumerated — and it grows with the linked spec** +(corrected on objectui#8938; this paragraph previously read as though the keys the cards +in this family happened to name were the whole of it). One spelling rule is applied to +`FieldSchema`'s **entire** declared key set at run time, so the accepted set is a property +of the installed `@objectstack/spec` and widens the moment the spec grows a camel key. +Besides the four above, today's spec puts the gate keys `visible_when` / `readonly_when` / +`required_when`, `default_value`, `required_permissions`, `masking_rule`, `track_history`, +`delete_behavior`, `external_id`, `depends_on`, `lookup_page_size`, the `related_list*` and +`inline_*` families, and the managed-by lock keys `_lock_reason` / `_lock_source` / +`_lock_docs_url` / `_package_id` / `_package_version` inside it — and case / kebab variants +of every one of them fold too. A stored legacy spelling of any of these is therefore +**active** on the client where consumers previously ignored it. + +No count of that surface is written here on purpose: it is derived from the installed spec, +and a number in this paragraph would be derived once and never again. The instrument that +re-derives it on every run is the pin named `the width IS the spec's declared key set, not +a list anyone typed`, beside the classes above as a live membership assertion. **Why this is needed at all.** The object-schema serve path never parses: `ObjectStackAdapter.getObjectSchema` fetches the document, applies two mutations and @@ -35,8 +53,10 @@ share. And it does not "correct" anything: a key that probes onto no declared ke alone, so a typo (`sortible`) stays a typo and `id_field` — which has no declared successor — stays as it is. -**Not covered.** `id_field` needs a `@objectstack/spec` release carrying the -`FIELD_KEY_GUIDANCE.id_field` row before its diagnostic can quote the contract rather than -a copy of it; that row is in no published version yet. `title_format` is out of scope -pending a separate maintainer ruling. Both land in the leave arm by the same rule, with no -special case. +**Not covered.** `id_field` and `title_format` are not folded — `id_field` has no declared +successor and `title_format` is out of scope pending a separate maintainer ruling. Both land +in the leave arm by the same rule, with no special case. What still waits on a +`@objectstack/spec` release carrying the `FIELD_KEY_GUIDANCE.id_field` row is the +**successor guidance** for `id_field`, which no published version carries; the leave arm +itself is no longer silent (objectui#8938 — the diagnostic states what it measured against +the linked spec rather than quoting a copy of contract prose). diff --git a/.changeset/8938-retired-dialect-measured-surface.md b/.changeset/8938-retired-dialect-measured-surface.md new file mode 100644 index 0000000000..08d9b1315b --- /dev/null +++ b/.changeset/8938-retired-dialect-measured-surface.md @@ -0,0 +1,39 @@ +--- +'@object-ui/core': minor +--- + +The ingestion choke point says out loud when it CANNOT fold a retired spelling +(objectui#8938) + +Maintainer ruling item 3 on objectui#7650 asked the retired-dialect fold for "a loud +diagnostic (not a silent drop) for a spelling the choke point cannot fold". What shipped +warned about the spelling it CAN fold and said nothing about the three it cannot, which is +the half that matters to a reader: a key that folds reaches every consumer, and a key that +does not reaches none of them — the retirement cards in this family (objectui#7155, #7166, +#7435) narrowed those consumers to the canonical spelling. + +`normalizeSchemaReferenceKeys` now names all three refusals, in dev only and memoised per +(object, field, spelling, reason), the discipline the two existing warnings already use: + +- **no declared twin** — `FieldSchema` declares neither the key nor anything sharing its + alias spelling, so there is nothing to fold onto (`id_field`, `title_format`, and a typo + such as `sortible`). The message ⛔ never offers a near match: the refused alternative + was the spec's `lintAuthoredRecordKeys`, whose Levenshtein fall-through answers "did you + mean `sortable`?" for that input, and a serve path that suggests a correction is one + revision away from applying it. +- **ambiguous probe** — two or more declared keys share the alias spelling, so the fold + refuses to choose. Unreachable against a spec with no collision; the pin that exercises + it substitutes a colliding `FieldSchema`. +- **occupied canonical** — the declared twin is on the def carrying a **different** value. + The producer's value stands (this choke point never overwrites one) and the retired value + is inert. Same value under both spellings is deliberately silent: that is the state the + pass leaves behind on its own second run, and the adapter re-serves a cached schema. + +**Nothing about which keys fold changes.** The diagnostic fires only on the paths that +already left the key alone, the leave arm stays lossless, and it is a no-op under +`NODE_ENV=production`. Whether the fold's full width — lock and gate keys included — is +the intended accept set is an open decision on objectui#7650, untouched here. + +Landed with the width pin objectui#8938 asked for, which drives every snake twin the +linked `@objectstack/spec` implies through the public choke point, and with the correction +to the objectui#7650 changeset that presented a handful of keys as the accepted set. diff --git a/packages/core/src/utils/__tests__/reference-keys.measuredSurface-8938.test.ts b/packages/core/src/utils/__tests__/reference-keys.measuredSurface-8938.test.ts new file mode 100644 index 0000000000..45a1d46e57 --- /dev/null +++ b/packages/core/src/utils/__tests__/reference-keys.measuredSurface-8938.test.ts @@ -0,0 +1,358 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * ⭐ THE MEASURED SURFACE OF THE RETIRED-DIALECT FOLD — objectui#8938. + * + * objectui#8873 landed the fold at the ingestion choke point per the route + * ruling, losslessly. What it declared was not what it measured: the changeset, + * the pull request body and the acceptance all presented a handful of keys as + * the accepted set, while the fold derives its width at RUN TIME from + * `FieldSchema.shape` and folds the snake twin of every declared key that has + * one — the gate keys (`visible_when` / `readonly_when` / `required_when`), + * `default_value`, `required_permissions`, `delete_behavior`, `external_id`, + * `depends_on`, and the managed-by lock keys `_lock_*` / `_package_id` / + * `_package_version` among them. + * + * ## Why this file does not write the number down + * + * A count in prose is derived once and derived never again (AGENTS.md #9), and + * this card exists because exactly that happened. So the correction states the + * RULE and points at this file, and the pins below re-derive the surface from + * the linked spec on every run: + * + * - the width pin is UNIVERSALLY QUANTIFIED over `FieldSchema`'s declared key + * set. Add a camel key to the spec and this file demands its snake twin + * fold, with no edit here; replace the derivation with a hand table and it + * goes red on the first key the table forgets. + * - the enumerated keys of the original declaration are asserted to be a + * STRICT SUBSET, so "these four are the accepted set" can never read true + * again. + * + * ⛔ This file records the width; it does not rule on it. Whether the full + * width — lock and gate keys included — is intended is the open decision on + * objectui#8938 (c), which belongs to the director seat on objectui#7650. + * Narrowing the fold means editing the class pins below on purpose, which is + * the point of having them. + * + * ## The diagnostic — maintainer ruling item 3 (objectui#7650, comment 5572018999) + * + * "A loud diagnostic (not a silent drop) for a spelling the choke point cannot + * fold." The fold's own warning covers the spelling it CAN fold; the three + * refusals — no declared twin, an ambiguous probe, an occupied canonical key — + * were silent, and each of them ends with a value no consumer reads. The pins + * below measure all three, plus the two silences that must stay silent: a + * spelling that folded (that is the other warning's case) and the reference + * arm's own keys (that is objectui#6837's). + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { FieldSchema } from '@objectstack/spec/data'; +import { + normalizeFieldReferenceKeys, + normalizeSchemaReferenceKeys, + resetReferenceKeyWarnings, +} from '../reference-keys'; + +/** The spec's alias probe, restated so this file does not import the implementation's copy. */ +const probe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g, ''); + +/** The snake spelling of a declared camel key — a SPELLING rule, never a key list. */ +const snakeTwin = (key: string): string => key.replace(/([a-z0-9])([A-Z])/g, '$1_$2').toLowerCase(); + +const declared = Object.keys(FieldSchema.shape as Record); + +/** + * Every declared key whose snake spelling is a DIFFERENT key the spec does not + * declare — i.e. every snake twin this choke point folds. Derived from the + * linked spec on every run; this is the width. + */ +const foldingTwins: ReadonlyArray = declared + .map((key) => [key, snakeTwin(key)] as const) + .filter(([key, twin]) => twin !== key && !declared.includes(twin)); + +/** + * The keys the original changeset enumerated when it presented the accepted set. + * A historical fact about that text, not a measurement — it is here only so the + * pins can assert the measured set is strictly WIDER than it. + */ +const ENUMERATED_BY_THE_ORIGINAL_DECLARATION = [ + 'display_field', + 'description_field', + 'lookup_filters', + 'lookup_columns', +] as const; + +let warn: ReturnType; + +beforeEach(() => { + resetReferenceKeyWarnings(); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + warn.mockRestore(); + resetReferenceKeyWarnings(); +}); + +const messages = (): string[] => warn.mock.calls.map((c: unknown[]) => String(c[0])); + +describe('the fold WIDTH is measured against the linked spec (objectui#8938)', () => { + it('the instrument is not dark — the spec is read, and it implies twins to fold', () => { + // Without this, every universally quantified pin below is satisfied by an + // empty key set: a vacuous green that reads exactly like a passing measurement. + expect(declared.length).toBeGreaterThan(50); + expect(foldingTwins.length).toBeGreaterThan(0); + // The twin spelling must land on its key's probe, or the derivation here is + // measuring something the implementation never looks up. + for (const [key, twin] of foldingTwins) expect(probe(twin)).toBe(probe(key)); + }); + + it('the width IS the spec’s declared key set, not a list anyone typed', () => { + // One def carrying every twin the live spec implies, driven through the + // public choke point. A hand-written table passes this only for as long as + // nobody adds a key to `FieldSchema`. + const def: Record = { type: 'text' }; + for (const [, twin] of foldingTwins) def[twin] = `v:${twin}`; + normalizeSchemaReferenceKeys({ name: 'measured', fields: { probe_field: def } }); + for (const [canonical, twin] of foldingTwins) expect(def[canonical]).toBe(`v:${twin}`); + // Lossless: the retired spelling is still on the def, every one of them. + for (const [, twin] of foldingTwins) expect(def[twin]).toBe(`v:${twin}`); + }); + + it('NEGATIVE CONTROL — an undeclared key with no declared twin is still not folded', () => { + // Without this, an implementation that stamped every key it saw would pass + // the width pin above. + const def: Record = { type: 'text', zzz_not_a_real_key: 1 }; + normalizeFieldReferenceKeys(def, 'owner', 'account'); + expect(Object.keys(def).sort()).toEqual(['type', 'zzz_not_a_real_key']); + }); + + it('the keys the original declaration enumerated are a STRICT SUBSET of the measured width', () => { + const twins = foldingTwins.map(([, twin]) => twin); + for (const key of ENUMERATED_BY_THE_ORIGINAL_DECLARATION) expect(twins).toContain(key); + expect(twins.length).toBeGreaterThan(ENUMERATED_BY_THE_ORIGINAL_DECLARATION.length); + }); + + it('the classes that declaration never named are inside the measured width', () => { + // ⛔ Not a ruling that they SHOULD be — objectui#8938 (c) is the open + // decision on that, and it is the director seat's. This pin makes the + // subject of that decision a measured fact instead of a sentence, and makes + // any later narrowing a deliberate edit here rather than a quiet drift. + const twins = foldingTwins.map(([, twin]) => twin); + for (const key of [ + 'visible_when', + 'readonly_when', + 'required_when', + 'default_value', + 'required_permissions', + 'delete_behavior', + 'external_id', + 'depends_on', + '_lock_reason', + '_package_id', + '_package_version', + ]) { + expect(twins).toContain(key); + } + }); + + it('a gate key and a lock key really do arrive canonical through the choke point', () => { + // The membership pin above reads the spec; this one reads the BEHAVIOUR, so + // the two cannot drift apart. + const schema = { + name: 'crm_account', + fields: { + stage: { type: 'text', visible_when: '${data.stage}', _package_id: 'pkg_crm' }, + }, + }; + normalizeSchemaReferenceKeys(schema); + const f = schema.fields.stage as Record; + expect(f.visibleWhen).toBe('${data.stage}'); + expect(f._packageId).toBe('pkg_crm'); + }); +}); + +describe('the ruling’s diagnostic for a spelling the choke point CANNOT fold (objectui#8938)', () => { + it('speaks up for a key with NO declared twin — `id_field`, and leaves it exactly as served', () => { + const f: Record = { type: 'text', id_field: 'code' }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(f.idField).toBeUndefined(); + expect(f.id_field).toBe('code'); + expect(warn).toHaveBeenCalledTimes(1); + const m = messages()[0] as string; + expect(m).toContain('CANNOT'); + expect(m).toContain('`account`'); + expect(m).toContain('`owner`'); + expect(m).toContain('id_field'); + expect(m).toContain('objectui#8938'); + }); + + it('⛔ never offers a near match for a TYPO — it says it cannot fold `sortible`, not "did you mean"', () => { + // The refused alternative on objectui#7650 was the spec's + // `lintAuthoredRecordKeys`, which falls through to a Levenshtein matcher and + // answers "did you mean `sortable`?" here. A diagnostic that names a + // correction is one revision away from applying it on a serve path. + const f: Record = { type: 'text', sortible: true }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(f.sortable).toBeUndefined(); + expect(warn).toHaveBeenCalledTimes(1); + expect(messages()[0]).toContain('sortible'); + expect(messages()[0]).not.toContain('sortable'); + }); + + it('speaks up when the canonical key is OCCUPIED — the retired value is inert, and that was silent', () => { + const f: Record = { + type: 'text', + display_field: 'legacy_name', + displayField: 'canonical_name', + }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + // Behaviour unchanged: the producer's value stands and nothing is dropped. + expect(f.displayField).toBe('canonical_name'); + expect(f.display_field).toBe('legacy_name'); + expect(warn).toHaveBeenCalledTimes(1); + const m = messages()[0] as string; + expect(m).toContain('CANNOT'); + expect(m).toContain('display_field'); + expect(m).toContain('displayField'); + expect(m).toContain('never overwrites'); + }); + + it('names the colliding declared keys when the probe is AMBIGUOUS', async () => { + // `FieldSchema` has no probe collision today — the sibling pin + // `has NO probe collision among declared keys — the precondition of the guard` + // asserts that against the real spec — so this refusal is unreachable + // without substituting a colliding shape. Unexercised, the guard could + // report a collision as an unrecognised spelling and nobody would know. + vi.resetModules(); + vi.doMock('@objectstack/spec/data', () => ({ + FieldSchema: { shape: { type: {}, visibleWhen: {}, visible_when: {} } }, + })); + try { + const fresh = await import('../reference-keys'); + const f: Record = { type: 'text', 'visible-when': '${data.x}' }; + fresh.normalizeFieldReferenceKeys(f, 'owner', 'account'); + // The guard still folds NOTHING — this slice adds the sentence, not a choice. + expect(f.visibleWhen).toBeUndefined(); + expect(f.visible_when).toBeUndefined(); + expect(f['visible-when']).toBe('${data.x}'); + expect(warn).toHaveBeenCalledTimes(1); + const m = messages()[0] as string; + expect(m).toContain('MORE THAN ONE'); + expect(m).toContain('visibleWhen'); + expect(m).toContain('visible_when'); + } finally { + vi.doUnmock('@objectstack/spec/data'); + vi.resetModules(); + } + }); +}); + +describe('the two silences the diagnostic must KEEP (objectui#8938)', () => { + it('⛔ does NOT fire on a spelling that DID fold — that is the other warning’s case', () => { + // Firing here too would report a def that works exactly as loudly as one + // that does not, which is the failure the fold's own warning already covers. + const f: Record = { type: 'text', display_field: 'name' }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(f.displayField).toBe('name'); + expect(warn).toHaveBeenCalledTimes(1); + expect(messages()[0]).toContain('carries the retired spelling'); + expect(messages()[0]).not.toContain('CANNOT'); + }); + + it('⛔ does NOT fire for `reference_to` / `referenceTo` — the reference arm owns those', () => { + // Both probe onto no declared key, so without the exclusion each would be + // reported as unfoldable in the same breath as the reference arm stamping it. + const f: Record = { type: 'lookup', reference_to: 'crm_account' }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(f.reference).toBe('crm_account'); + expect(warn).toHaveBeenCalledTimes(1); + expect(messages()[0]).toContain('objectui#6837'); + expect(messages()[0]).not.toContain('CANNOT'); + + resetReferenceKeyWarnings(); + warn.mockClear(); + const g: Record = { type: 'lookup', referenceTo: 'crm_account' }; + normalizeFieldReferenceKeys(g, 'owner', 'account'); + expect(warn).toHaveBeenCalledTimes(1); + expect(messages()[0]).not.toContain('CANNOT'); + }); + + it('⛔ does NOT fire on the canonical THIS PASS stamped — a second run is not a producer conflict', () => { + // The adapter re-serves a cached schema and `MetadataProvider` + // re-normalizes on every metadata refresh, so every folded def is + // re-examined with its canonical key already set — by this pass, carrying + // the very same value by reference. Reporting that state would turn the + // diagnostic into a line per folded field per refresh. The occupied-canonical + // refusal is therefore about a DIFFERENT value, not about an occupied key. + const f: Record = { type: 'text', display_field: 'name' }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + const afterFirst = warn.mock.calls.length; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(warn.mock.calls.length).toBe(afterFirst); + expect(messages().join('\n')).not.toContain('CANNOT'); + }); + + it('⛔ does NOT fire when both spellings carry the SAME value — nothing is being lost', () => { + const f: Record = { type: 'text', display_field: 'name', displayField: 'name' }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(warn).not.toHaveBeenCalled(); + }); + + it('⛔ does NOT fire for an undeclared key carrying `undefined` — no value, no lost read', () => { + const f: Record = { type: 'text', id_field: undefined }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(warn).not.toHaveBeenCalled(); + }); + + it('is silent under NODE_ENV=production, and the def comes out identical', () => { + vi.stubEnv('NODE_ENV', 'production'); + try { + const f: Record = { type: 'text', id_field: 'code', display_field: 'name' }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(warn).not.toHaveBeenCalled(); + // The diagnostic is a dev affordance; the fold is the contract. + expect(f.displayField).toBe('name'); + expect(f.id_field).toBe('code'); + } finally { + vi.unstubAllEnvs(); + } + }); +}); + +describe('flood control for the diagnostic — a console that scrolls is one nobody reads (objectui#8938)', () => { + it('fires once per (object, field, spelling) and again for a SECOND object', () => { + normalizeSchemaReferenceKeys({ name: 'crm_account', fields: { owner: { type: 'text', id_field: 'a' } } }); + normalizeSchemaReferenceKeys({ name: 'crm_account', fields: { owner: { type: 'text', id_field: 'a' } } }); + expect(warn).toHaveBeenCalledTimes(1); + normalizeSchemaReferenceKeys({ name: 'crm_contact', fields: { owner: { type: 'text', id_field: 'a' } } }); + expect(warn).toHaveBeenCalledTimes(2); + expect(messages().join('\n')).toContain('`crm_contact`'); + }); + + it('the REASON is in the memo — one field refused for two different reasons gets two lines', () => { + // Collapse the reason out of the key and the second line disappears, taking + // the occupied-canonical fix with it. + const f: Record = { + type: 'text', + id_field: 'code', + display_field: 'legacy', + displayField: 'canonical', + }; + normalizeFieldReferenceKeys(f, 'owner', 'account'); + expect(warn).toHaveBeenCalledTimes(2); + const all = messages().join('\n'); + expect(all).toContain('id_field'); + expect(all).toContain('display_field'); + }); + + it('a def with no object in scope warns under a placeholder rather than merging into a real object', () => { + normalizeFieldReferenceKeys({ type: 'text', id_field: 'code' }, 'owner'); + expect(messages()[0]).toContain('(unknown object)'); + }); +}); diff --git a/packages/core/src/utils/__tests__/reference-keys.retiredDialect-7650.test.ts b/packages/core/src/utils/__tests__/reference-keys.retiredDialect-7650.test.ts index 179693515f..39fe16b751 100644 --- a/packages/core/src/utils/__tests__/reference-keys.retiredDialect-7650.test.ts +++ b/packages/core/src/utils/__tests__/reference-keys.retiredDialect-7650.test.ts @@ -40,6 +40,20 @@ * (it needs an eight-key maintainer ruling), and it too lands there by the * rule rather than by an exclusion. * + * ## ⭐ THE THREE INVERTED PINS — objectui#8938 + * + * Three assertions in this file used to read `expect(warn).not.toHaveBeenCalled()`, + * which pinned the ABSENCE of maintainer ruling item 3's diagnostic (objectui#7650, + * comment 5572018999: a loud diagnostic, not a silent drop, for a spelling the + * choke point cannot fold). Each is now inverted in place and tightened rather + * than removed — the negative behaviour each one guards (no fold, no typo + * correction, no overwrite) is asserted exactly as before, and the refusal is + * additionally required to be audible. The diagnostic's own surface — its three + * refusals, its two deliberate silences and its memo — is pinned in the sibling + * file named by the describe `the ruling’s diagnostic for a spelling the choke + * point CANNOT fold (objectui#8938)`, together with the width measurement that + * card asked for. + * * ## The contract-derivation pins * * `describe('derives the fold from the contract, not from a table')` asserts the @@ -135,7 +149,15 @@ describe('the NEGATIVE pins the ruling required (objectui#7650)', () => { normalizeFieldReferenceKeys(f, 'owner', 'account'); expect(f.idField).toBeUndefined(); expect(f.id_field).toBe('code'); - expect(warn).not.toHaveBeenCalled(); + // objectui#8938: this asserted `warn` was NOT called, and that silence was + // the defect — maintainer ruling item 3 (objectui#7650, comment 5572018999) + // asked for a loud diagnostic on exactly the spelling the choke point + // cannot fold, and `id_field` is its type case. The pin is not dropped, it + // is INVERTED and tightened: the fold must still not happen (the two + // assertions above) and the refusal must now be audible. + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]?.[0])).toContain('CANNOT'); + expect(String(warn.mock.calls[0]?.[0])).toContain('id_field'); }); it('does NOT fold a TYPO — `sortible` never becomes `sortable`', () => { @@ -143,7 +165,15 @@ describe('the NEGATIVE pins the ruling required (objectui#7650)', () => { normalizeFieldReferenceKeys(f, 'owner', 'account'); expect(f.sortable).toBeUndefined(); expect(f.sortible).toBe(true); - expect(warn).not.toHaveBeenCalled(); + // objectui#8938: was `not.toHaveBeenCalled()`. A typo is a spelling the + // choke point cannot fold, so the ruling's diagnostic covers it too — and + // the assertion below is the load-bearing half of this test's own point: + // the message says it cannot fold `sortible` and ⛔ never names `sortable`, + // because a serve path that suggests a correction is one revision away from + // applying it. + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]?.[0])).toContain('sortible'); + expect(String(warn.mock.calls[0]?.[0])).not.toContain('sortable'); }); it('does NOT fold `title_format` — out of this card, and out of the rule', () => { @@ -161,7 +191,14 @@ describe('the NEGATIVE pins the ruling required (objectui#7650)', () => { normalizeFieldReferenceKeys(f, 'owner', 'account'); expect(f.displayField).toBe('canonical_name'); expect(f.display_field).toBe('legacy_name'); - expect(warn).not.toHaveBeenCalled(); + // objectui#8938: was `not.toHaveBeenCalled()`. The producer's value still + // stands — that is what the two assertions above pin, and it is unchanged — + // but the retired spelling beside it reaches no consumer, and that was the + // third silent refusal. The diagnostic names the occupied canonical key; + // the behaviour here is untouched. + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]?.[0])).toContain('CANNOT'); + expect(String(warn.mock.calls[0]?.[0])).toContain('displayField'); }); it('is idempotent — a second pass changes nothing and does not warn twice', () => { @@ -174,7 +211,10 @@ describe('the NEGATIVE pins the ruling required (objectui#7650)', () => { expect(warn.mock.calls.length).toBe(warnsAfterFirst); }); - it('leaves a declared key alone even when a snake twin of it exists on the def', () => { + it('leaves a DECLARED key alone — it is never itself a fold source', () => { + // objectui#8938: the title used to say "even when a snake twin of it exists + // on the def", and no twin is on this def — the same declared-versus-measured + // drift the card is about, one file down. The fixture is what it always was. // `displayField` is declared, so it is never itself a fold SOURCE. Without // this the pass could re-enter on its own output. const f: Record = plainField({ displayField: 'name' }); diff --git a/packages/core/src/utils/reference-keys.ts b/packages/core/src/utils/reference-keys.ts index 24648ab61f..5f2c8d6206 100644 --- a/packages/core/src/utils/reference-keys.ts +++ b/packages/core/src/utils/reference-keys.ts @@ -144,6 +144,23 @@ import { FieldSchema } from '@objectstack/spec/data'; * REFUSED — a stored legacy document would lose the value instead of arriving * canonical, and silent data loss on a serve path is the worst of the shapes. * + * ⭐ The leave arm is no longer SILENT, which is the other half of that + * sentence: the document keeps the value, and the consumers — narrowed to the + * canonical spelling by objectui#7155 / #7166 / #7435 — read none of it. See + * {@link warnUnfoldableRetiredKey} (objectui#8938) for the diagnostic maintainer + * ruling item 3 asked for, and for the two cases it deliberately stays out of. + * + * ## ⛔ The WIDTH of this arm is measured, never declared + * + * This arm accepts one spelling rule applied to `FieldSchema`'s WHOLE declared + * key set, so its width is a property of the linked `@objectstack/spec` and + * grows with it — ⛔ it is not the handful of keys the cards that asked for it + * happened to name, and ⛔ no count of it belongs in prose here or in a + * changeset (objectui#8938; AGENTS.md #9). The instrument that re-derives it is + * the pin named `the width IS the spec's declared key set, not a list anyone + * typed`, which drives every snake twin the live spec implies through this + * choke point. + * * ⛔ The `id_field` slice is BLOCKED, and not on a card. `@objectstack/spec`'s * `FIELD_KEY_GUIDANCE` grew an `id_field` row explaining why the key has no * successor, but that row is in NO PUBLISHED version — measured against 17.3.0 @@ -172,6 +189,7 @@ const aliasProbe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g /** Memoized `FieldSchema` readings — see {@link fieldKeyFolds}. */ let declaredFieldKeys: ReadonlySet | null = null; let probeFolds: ReadonlyMap | null = null; +let collidingProbes: ReadonlyMap | null = null; /** * `probe -> canonical declared key`, derived once from `FieldSchema.shape`. @@ -191,14 +209,24 @@ let probeFolds: ReadonlyMap | null = null; function fieldKeyFolds(): ReadonlyMap { if (probeFolds) return probeFolds; const declared = Object.keys(FieldSchema.shape as Record); - const seen = new Map(); + const seen = new Map(); for (const key of declared) { const probe = aliasProbe(key); - seen.set(probe, seen.has(probe) ? null : key); + seen.set(probe, [...(seen.get(probe) ?? []), key]); } const folds = new Map(); - for (const [probe, key] of seen) if (key !== null) folds.set(probe, key); + const collisions = new Map(); + // Same partition as before — a probe two declared keys share folds NOTHING — + // but the colliding keys are now kept rather than flattened to a `null`, so + // the diagnostic below can say WHICH declared keys the probe could not choose + // between instead of reporting the refusal as an unrecognised spelling + // (objectui#8938). + for (const [probe, keys] of seen) { + if (keys.length === 1) folds.set(probe, keys[0] as string); + else collisions.set(probe, keys); + } declaredFieldKeys = new Set(declared); + collidingProbes = collisions; probeFolds = folds; return folds; } @@ -209,6 +237,90 @@ function fieldKeyFolds(): ReadonlyMap { */ const warnedRetiredSpelling = new Set(); +/** + * Warn once per (OBJECT, field, spelling, REASON) — see + * {@link warnUnfoldableRetiredKey}. The reason is in the key because the three + * refusals are three different fixes, and a def can hit more than one of them. + */ +const warnedUnfoldableSpelling = new Set(); + +/** Why the choke point left an undeclared key where it was. */ +type UnfoldableReason = 'no-declared-twin' | 'ambiguous-probe' | 'canonical-occupied'; + +/** + * ## ⭐ THE DIAGNOSTIC FOR A SPELLING THIS CHOKE POINT CANNOT FOLD — objectui#8938 + * + * Maintainer ruling item 3 (objectui#7650, comment 5572018999) asked the + * execution for "a loud diagnostic (not a silent drop) for a spelling the choke + * point cannot fold". The fold arm shipped with a warning for the spelling it + * CAN fold ({@link canonicalizeRetiredFieldKeys}) and nothing for the one it + * cannot, which is the half the ruling named: a key that folds still reaches + * every consumer, while a key that does not fold reaches none of them. + * + * ⛔ "Not a silent DROP" is about the reader, not about the document. Nothing + * here removes a key or a value — the leave arm is lossless and stays lossless. + * What is lost is the READ: the retirement cards in this family (objectui#7155, + * #7166, #7435) narrowed the consumers to the canonical spelling, so a value + * that never reaches a canonical key is a value nothing reads. That is the + * silence this makes audible. + * + * ## The three refusals, and why each is a different sentence + * + * - `no-declared-twin` — `FieldSchema` declares neither the key nor anything + * sharing its alias probe. `id_field` and `title_format` land here by the + * derived rule rather than by an exclusion, and so does a typo (`sortible`): + * ⛔ the diagnostic deliberately does NOT offer a near match, for the same + * reason the fold does not — the spec's `lintAuthoredRecordKeys` falls + * through to a Levenshtein matcher, and a serve path that suggests a + * correction is one revision away from applying it. + * - `ambiguous-probe` — two or more declared keys share the probe, so the + * fold refuses to choose. Unreachable against a spec with no collision; + * the pin that exercises it substitutes a colliding `FieldSchema`. + * - `canonical-occupied` — the declared twin is already on the def carrying a + * DIFFERENT value, and the fold never overwrites a value the producer set. + * The producer's value is served, the retired one is inert, and NOTHING said + * so before this. ⛔ Same value under both spellings is NOT this case: that + * is the state this pass leaves behind on its own second run, and reporting + * it would make every re-normalization of a correctly folded def noisy. + * + * ## ⛔ What it does NOT fire on — the two exclusions that keep it honest + * + * - A key that DID fold. That case is {@link canonicalizeRetiredFieldKeys}'s + * own warning, and firing here as well would report a def that works + * exactly as loudly as one that does not. + * - `reference_to` / `referenceTo`, which probe onto no declared key and + * would otherwise report as `no-declared-twin` while the reference arm was + * in the middle of stamping them. They have their own arm and their own + * diagnostic ({@link warnOnLegacyOnlyReference}, objectui#6837). + * + * Dev-only and memoised, the discipline both existing warnings already use: the + * adapter re-serves a cached schema and `MetadataProvider` re-normalizes on + * every metadata refresh, so a warning that floods is a warning that gets muted. + */ +function warnUnfoldableRetiredKey( + reason: UnfoldableReason, + key: string, + detail: string, + fieldName: string | undefined, + objectName: string | undefined, + f: Record, +): void { + if (!isDev()) return; + const named = fieldName ?? (typeof f.name === 'string' ? f.name : '(unnamed field)'); + const owner = objectName ?? '(unknown object)'; + const memo = `${owner}:${named}:${key}:${reason}`; + if (warnedUnfoldableSpelling.has(memo)) return; + warnedUnfoldableSpelling.add(memo); + console.warn( + `[ObjectUI] Object \`${owner}\`, field \`${named}\`: the ingestion choke point CANNOT ` + + `canonicalize the retired spelling \`${key}\`. ${detail} The key and its value are LEFT ` + + `on the def exactly as served — nothing is dropped here — but the consumers read only the ` + + `spelling \`@objectstack/spec\`'s \`FieldSchema\` declares, so this value reaches no ` + + `reader. Fix the PRODUCER, or migrate the stored document. (maintainer ruling item 3 on ` + + `objectui#7650; objectui#8938)`, + ); +} + /** * Stamp the canonical spelling for every retired-dialect key on one field def. * @@ -227,11 +339,63 @@ function canonicalizeRetiredFieldKeys( const folds = fieldKeyFolds(); for (const key of Object.keys(f)) { if (declaredFieldKeys!.has(key)) continue; - const canonical = folds.get(aliasProbe(key)); - if (canonical === undefined) continue; const value = f[key]; + // Nothing to carry across and nothing to lose: no fold, and no diagnostic + // either — the reader is not missing a value that was never there. if (value === undefined) continue; - if (f[canonical] !== undefined) continue; + // The reference arm's own keys are handled (and warned about) below; they + // probe onto no declared key, so without this they would report here as an + // unfoldable spelling while that arm was about to stamp them. + if (REFERENCE_ARM_KEYS.has(key)) continue; + const probe = aliasProbe(key); + const canonical = folds.get(probe); + if (canonical === undefined) { + const colliding = collidingProbes!.get(probe); + if (colliding) { + warnUnfoldableRetiredKey( + 'ambiguous-probe', + key, + `Its alias spelling is shared by MORE THAN ONE key \`FieldSchema\` declares ` + + `(${colliding.map((c) => `\`${c}\``).join(', ')}), so the fold refuses to choose ` + + `between them rather than picking one.`, + fieldName, + objectName, + f, + ); + } else { + warnUnfoldableRetiredKey( + 'no-declared-twin', + key, + `\`FieldSchema\` declares neither \`${key}\` nor any key sharing its alias spelling ` + + `(lowercased, with \`_\`, \`-\` and spaces removed), so there is no canonical key ` + + `to fold it onto — and this path deliberately does not guess a near match.`, + fieldName, + objectName, + f, + ); + } + continue; + } + if (f[canonical] !== undefined) { + // ⛔ Only when a value is actually being lost. `f[canonical] === value` is + // the state this pass itself leaves behind — it stamps by reference and + // the adapter re-serves a cached schema, so reporting it would fire on + // every re-normalization of a def that folded perfectly the first time. + // The idempotence pin in the objectui#7650 suite is what measures this. + if (f[canonical] !== value) { + warnUnfoldableRetiredKey( + 'canonical-occupied', + key, + `\`FieldSchema\` declares \`${canonical}\`, and this def already carries a ` + + `DIFFERENT value under it; the producer's value stands, because this choke point ` + + `never overwrites one.`, + fieldName, + objectName, + f, + ); + } + continue; + } f[canonical] = value; if (!isDev()) continue; const named = fieldName ?? (typeof f.name === 'string' ? f.name : '(unnamed field)'); @@ -281,6 +445,7 @@ const warnedLegacyOnly = new Set(); export function resetReferenceKeyWarnings(): void { warnedLegacyOnly.clear(); warnedRetiredSpelling.clear(); + warnedUnfoldableSpelling.clear(); } const isDev = (): boolean => @@ -290,6 +455,13 @@ const isDev = (): boolean => /** The two spellings no contract declares, in the order the stamp prefers them. */ const LEGACY_REFERENCE_KEYS = ['reference_to', 'referenceTo'] as const; +/** + * The same two, as a set the retired-dialect arm tests before it reports a key + * as unfoldable — they are handled by the reference arm below, not left + * (objectui#8938). + */ +const REFERENCE_ARM_KEYS: ReadonlySet = new Set(LEGACY_REFERENCE_KEYS); + /** * Dev-mode only: say out loud that a def arrived spelling ONLY a legacy key. *