diff --git a/.changeset/6648-relationship-target-carriers.md b/.changeset/6648-relationship-target-carriers.md new file mode 100644 index 0000000000..05f4bef52c --- /dev/null +++ b/.changeset/6648-relationship-target-carriers.md @@ -0,0 +1,52 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/core': patch +--- + +Resolve a relationship target from a `reference` STRING only — the carrier axis +(objectui#6648). + +objectui#6528 narrowed both relationship-target resolvers to the single spec +SPELLING `reference` and left the CARRIER — the shape the value may take — +explicitly for its own census. That census is done, and it says the same thing: +`resolveReferenceTo` (dataset designer) and its sibling +`resolveRelationshipTarget` (`chart-series.ts`) each accepted three carriers on +the canonical key, two of which `FieldSchema` never declared. Both are removed +in BOTH files in one pass (they must not diverge — a fix leaving them +disagreeing recreates the defect one file over). + +The measurement, with the bare string as the positive control every zero is +measured against: + +| carrier | `ObjectSchema.safeParse` (spec 17.2.0) | producers at the field-def key position | +|---|---|---| +| `reference: 'crm_account'` | ACCEPTED | live — 587 across both trees | +| `reference: ['crm_account']` | REFUSED — `expected string, received array` | 0 | +| `reference: { object: 'crm_account' }` | REFUSED — `expected string, received object` | 0 | + +The census walked STRUCTURE, not text: JSON/YAML parsed and walked, TS/TSX read +through the TypeScript compiler API, each hit recorded with its ancestor +property chain and its enclosing object's sibling keys so a FIELD DEF is +separated from the other tiers that also spell `reference` (a form field +literally named `reference`, its translation entries, a JSON-Schema property +descriptor, a liveness-ledger row). Every dynamic initializer at the field-def +position resolved to a string-typed source, and every `reference` TYPE +declaration in either tree declares `string`. The detector is not blind to the +shape it hunted — it DID report array and `{ object }` carriers, and every one +was a test asserting this very tolerance plus one framework lint fixture whose +own rule already reads string-only. + +The array branch was also a silent PRODUCT decision: handed a multi-target +value it returned element zero and DISCARDED the rest. Nothing declares such a +value. Polymorphic lookup is an open, unbuilt gap in the spec's own audit report +("Current `reference` only supports a single target", Tier 3), and the +platform's one polymorphic reference (ADR-0018 `xRef`) is a STRING with a +sibling discriminator, never a list. A multi-target lookup, if it lands, lands +as a declared spec shape — not as a carrier a consumer guesses at. + +Behaviour change, and it is deliberate: a field def whose `reference` is not a +non-empty string now resolves to `undefined` in both helpers. Such a document is +already refused by `ObjectSchema`, so per AGENTS.md #0.1 it is a producer-side +defect, and a lenient consumer is exactly where it would have stayed hidden. The +two unit assertions that pinned the tolerant reads are converted to refusal +pins, so re-widening the carrier turns red. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.test.ts index 5935f9fb25..44c25bdde1 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.test.ts @@ -27,11 +27,32 @@ describe('resolveReferenceTo', () => { expect(resolveReferenceTo({ reference: 'account' })).toBe('account'); }); - it('reads array + object carriers off `reference`', () => { - // The CARRIER (bare name / one-element array / `{ object }`) is a separate - // axis from the SPELLING, and objectui#6528 narrowed only the latter. - expect(resolveReferenceTo({ reference: ['account', 'lead'] })).toBe('account'); - expect(resolveReferenceTo({ reference: { object: 'account' } })).toBe('account'); + /** + * objectui#6648 — the CARRIER axis, pinned exactly like the SPELLING axis + * below and for the same reason. These two assertions used to read + * `.toBe('account')`: they were the tolerance written down, not evidence for + * it. + * + * `FieldSchema.reference` is a plain `z.string()`, and `ObjectSchema.safeParse` + * (spec 17.2.0) REFUSES both carriers — `invalid_type: expected string, + * received array` / `received object` — while ACCEPTING the bare name + * asserted above as the positive control. A structure-walking, + * key-position-aware census of both trees found 587 bare-string carriers at + * the field-def key position and ZERO producers of either carrier below, so + * resolving one could only re-hide a producer defect (AGENTS.md #0.1). + * + * The array case was the worse of the two: it returned element zero and + * silently DISCARDED the rest of a multi-target value. No such value is + * declared anywhere — polymorphic lookup is an open, unbuilt spec gap — so + * the branch was data loss wearing a compatibility shim. Deleting the + * narrowing turns this RED. + */ + it.each([ + { carrier: 'array', reference: ['account', 'lead'] as unknown }, + { carrier: 'multi-element array (the discarded-rest case)', reference: ['account', 'lead', 'case'] as unknown }, + { carrier: '`{ object }`', reference: { object: 'account' } as unknown }, + ])('refuses the $carrier carrier on `reference` — a producer emitting it is the bug', ({ reference }) => { + expect(resolveReferenceTo({ reference })).toBeUndefined(); }); /** diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.ts b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.ts index f424f2db02..c3985b3f2f 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.ts @@ -115,18 +115,40 @@ export function resolveLabel(label: unknown, fallback: string): string { * target any other way is a PRODUCER defect, and must fail visibly here so it is * fixed there. * - * The string / array / `{ object }` CARRIERS are untouched: they are a separate - * axis from the spelling, and narrowing them needs its own census. + * The CARRIER — the shape the value may take — is narrowed on the same evidence + * standard as of objectui#6648. `FieldSchema.reference` is a plain `z.string()`, + * and `ObjectSchema.safeParse` (spec 17.2.0) REFUSES the other two carriers on + * the canonical key while ACCEPTING the bare name (the positive control): + * `reference: ['crm_account']` is `invalid_type: expected string, received + * array`, `reference: { object: 'crm_account' }` is `expected string, received + * object`. + * + * The carrier census walked STRUCTURE, not text — JSON/YAML parsed and walked, + * TS/TSX through the TypeScript compiler API — recording each hit's ancestor + * property chain and its enclosing object's sibling keys, so a FIELD DEF is + * separated from the other tiers that also spell `reference` (a form field + * literally named `reference`, its translation entries, a JSON-Schema property + * descriptor, a liveness-ledger row). Across both trees: 607 hits at the + * field-def key position, 587 of them the bare-string carrier, and ZERO array + * or `{ object }` carriers from any producer. Every dynamic initializer at that + * position resolved to a string source, and every `reference` TYPE declaration + * in either tree declares `string`. The detector is not blind to the shape it + * hunted: it DID report array and `{ object }` carriers — the two green pins + * that asserted this tolerance, and one framework lint FIXTURE whose own rule + * (`refOf`) already reads string-only. + * + * The array branch was also a silent PRODUCT decision: handed a multi-target + * value it returned `raw[0]` and discarded the rest. Nothing declares such a + * value — polymorphic lookup is an open Tier-3 gap in the spec's own audit + * report ("Current `reference` only supports a single target"), and the + * platform's one polymorphic reference (ADR-0018 `xRef`) is a STRING with a + * sibling discriminator, never a list. If multi-target lookups land they land + * as a declared spec shape; until then, silently taking element zero is data + * loss wearing a compatibility shim. */ export function resolveReferenceTo(def: Record): string | undefined { const raw = def.reference; - if (typeof raw === 'string' && raw) return raw; - if (Array.isArray(raw) && typeof raw[0] === 'string') return raw[0]; - if (raw && typeof raw === 'object') { - const obj = (raw as { object?: unknown }).object; - if (typeof obj === 'string' && obj) return obj; - } - return undefined; + return typeof raw === 'string' && raw ? raw : undefined; } /** Map a framework field type onto a dataset dimension type. */ diff --git a/packages/core/src/utils/__tests__/chart-series.test.ts b/packages/core/src/utils/__tests__/chart-series.test.ts index 0d580af87e..89773b624f 100644 --- a/packages/core/src/utils/__tests__/chart-series.test.ts +++ b/packages/core/src/utils/__tests__/chart-series.test.ts @@ -196,11 +196,30 @@ describe('relabelDimensions + buildChartSeries (the value≠label chart bug, clo describe('resolveRelationshipTarget (objectui#4053)', () => { it('reads the target off the spec spelling `reference`', () => { + // The positive control: the one carrier `ObjectSchema.safeParse` ACCEPTS, + // and the one every zero below is measured against. expect(resolveRelationshipTarget({ type: 'lookup', reference: 'crm_account' })).toBe('crm_account'); - // Array and `{ object }` carriers. The CARRIER is a separate axis from the - // SPELLING, and objectui#6528 narrowed only the latter. - expect(resolveRelationshipTarget({ type: 'lookup', reference: ['crm_account'] })).toBe('crm_account'); - expect(resolveRelationshipTarget({ type: 'lookup', reference: { object: 'crm_account' } })).toBe('crm_account'); + }); + + /** + * objectui#6648 — the mirror of the dataset designer's CARRIER refusal pin, + * kept in lockstep for the same reason the spelling pins are (see below): the + * two canonicalizations are one doctrine in two files, and a divergence + * recreates the defect one file over. + * + * These two used to assert `.toBe('crm_account')` — the tolerance written + * down. The census that removed them walked STRUCTURE, not text, across both + * trees and found ZERO producers of either carrier at the field-def key + * position, against 587 bare-string carriers there. `ObjectSchema.safeParse` + * (spec 17.2.0) REFUSES both as `invalid_type`, and the array branch also + * discarded every element after the first. + */ + it.each([ + { carrier: 'array', reference: ['crm_account'] as unknown }, + { carrier: 'multi-element array (the discarded-rest case)', reference: ['crm_account', 'crm_lead'] as unknown }, + { carrier: '`{ object }`', reference: { object: 'crm_account' } as unknown }, + ])('refuses the $carrier carrier on `reference` — a producer emitting it is the bug', ({ reference }) => { + expect(resolveRelationshipTarget({ type: 'lookup', reference })).toBeUndefined(); }); /** diff --git a/packages/core/src/utils/chart-series.ts b/packages/core/src/utils/chart-series.ts index 91c12d7d0f..e17f3c0e43 100644 --- a/packages/core/src/utils/chart-series.ts +++ b/packages/core/src/utils/chart-series.ts @@ -942,8 +942,10 @@ function fieldDefsOf(schema: unknown): Record | null { * * The target lives under `reference` — the ONLY spelling read here, and the same * canonicalization as the dataset designer's `resolveReferenceTo`, which is - * narrowed to match in the same pass (objectui#6528). The value may still carry - * a bare name, a one-element array, or `{ object }`. + * narrowed to match in the same pass (objectui#6528). As of objectui#6648 the + * CARRIER matches too: a bare string and nothing else. The two helpers are one + * doctrine in two files and must never drift — a divergence recreates the + * defect one file over. * * The three legacy spellings this dropped (`reference_to` / `referenceTo` / * `reference_to_object`) were censused against every producer, with `reference` @@ -967,6 +969,25 @@ function fieldDefsOf(schema: unknown): Record | null { * `reference` was already HEAD of the old chain, so any doc carrying both is * unaffected. * + * The array and `{ object }` CARRIERS this used to accept were censused on the + * same standard and removed with the same reasoning (objectui#6648). + * `FieldSchema.reference` is a plain `z.string()`; `ObjectSchema.safeParse` + * (spec 17.2.0) ACCEPTS `reference: 'crm_account'` and REFUSES both + * `['crm_account']` and `{ object: 'crm_account' }` as `invalid_type`. A + * structure-walking, key-position-aware census of both trees found 587 + * bare-string carriers at the field-def key position and ZERO array or + * `{ object }` carriers from any producer — the only ones in either tree were + * the green pins asserting this very tolerance. See `resolveReferenceTo` for + * the full census, including why a multi-target lookup is NOT what the array + * branch was serving: it returned `raw[0]` and discarded the rest, and + * polymorphic lookup remains an undeclared, open spec gap. + * + * This path matters MORE than the designer's for the same reason the spelling + * narrowing did: it reads the metadata document directly, with no `readFields` + * door in between, so a non-string carrier now degrades visibly (the walk + * yields no entry and the caller keeps the raw value) instead of being absorbed + * here. + * * The **type gate is deliberate**: only a declared relationship is walked, so a * path segment naming a plain field can never be turned into an object name and * fetched speculatively. @@ -980,13 +1001,7 @@ export function resolveRelationshipTarget(fieldDef: unknown): string | undefined const type = typeof def.type === 'string' ? def.type.toLowerCase() : ''; if (!RELATIONSHIP_FIELD_TYPES.has(type)) return undefined; const raw = def.reference; - if (typeof raw === 'string' && raw) return raw; - if (Array.isArray(raw) && typeof raw[0] === 'string' && raw[0]) return raw[0]; - if (raw && typeof raw === 'object') { - const obj = (raw as { object?: unknown }).object; - if (typeof obj === 'string' && obj) return obj; - } - return undefined; + return typeof raw === 'string' && raw ? raw : undefined; } /**