diff --git a/.changeset/6837-recorddrawer-invented-target-arms.md b/.changeset/6837-recorddrawer-invented-target-arms.md new file mode 100644 index 0000000000..cc27971316 --- /dev/null +++ b/.changeset/6837-recorddrawer-invented-target-arms.md @@ -0,0 +1,36 @@ +--- +'@object-ui/plugin-detail': minor +--- + +`RecordDetailDrawer` resolves a relationship target only from the two spellings +a contract carries, dropping the two no contract declares (objectui#6837, first +slice). + +The chain was `def.reference_to ?? def.reference ?? def.referenceTo ?? +def.target`; it is now `def.reference_to ?? def.reference`. + +**Accept-set move — a def carrying ONLY `referenceTo`, or ONLY `target`, stops +resolving a target** and the field renders without one (the drawer already marks +every reference-bearing field readonly, so nothing becomes editable that was +not). Two things bound that: + +- Any def that entered through the ingestion choke point is unaffected. + `normalizeSchemaReferenceKeys` reads `reference_to ?? reference ?? + referenceTo` and stamps both snake_case keys, so a `referenceTo`-only def + arriving via `MetadataProvider` or `ObjectStackAdapter.getObjectSchema` + already carries `reference_to` before the drawer sees it. Only a def that + bypassed that door entirely is affected. +- `target` was never read anywhere else in the stack — not by the normalizer, + not by the spec. `@objectstack/spec`'s `FieldSchema` refuses both deleted + spellings by name with `unrecognized_keys`, each carrying its own "did you + mean `reference`" rename, and `referenceTo` is additionally stripped at the + designer read door (`RETIRED_FIELD_KEYS`, objectui#6041 / #6519). + +A repo-wide structure-walk producer census found **0** emitters of `target` and +**0** reaching this seam for `referenceTo`, measured in the cell the drawer +reads (a value inside an object schema's `fields` container) against controls +`reference` (92 hits / 36 files) and `reference_to` (52 / 36) hot in the same +pass over the same cells. + +Pinned by `RecordDetailDrawer.referenceArms-6837.test.tsx`, which keeps the live +arms green beside a named refusal per deleted key. diff --git a/packages/plugin-detail/src/RecordDetailDrawer.tsx b/packages/plugin-detail/src/RecordDetailDrawer.tsx index 81e41d75c0..69b27fb6b2 100644 --- a/packages/plugin-detail/src/RecordDetailDrawer.tsx +++ b/packages/plugin-detail/src/RecordDetailDrawer.tsx @@ -272,8 +272,32 @@ export function RecordDetailDrawer({ format: def.format, // Served schemas key the target as `reference` (ObjectStack // convention, #2407); the drawer can receive a raw schema from any - // DataSource, so resolve every spelling. - reference_to: def.reference_to ?? def.reference ?? def.referenceTo ?? def.target, + // DataSource, so both snake_case spellings are resolved here. + // + // Two further arms stood here until objectui#6837 — `def.referenceTo` + // and `def.target` — and they were NOT redundant-but-harmless: no + // contract declares either spelling. `FieldSchema` refuses BOTH by name + // with `unrecognized_keys`, each carrying its own "did you mean + // `reference`" rename; `referenceTo` is additionally stripped at the + // designer read door (`RETIRED_FIELD_KEYS`, objectui#6041 / #6519), so + // that arm could never hit. A structure-walk producer census found ZERO + // emitters of either at THIS cell — a value inside an object schema's + // `fields` container, which is what `objectSchema.fields[name]` reads — + // while the controls `reference` (92 hits / 36 files) and `reference_to` + // (52 / 36) were hot in the same pass over the same cells. So the two + // arms were invented tolerance surface: a silent absorption point for a + // producer that should fail visibly (AGENTS.md #0.1). + // + // ⛔ Do not re-add a spelling arm here. A producer emitting a refused + // spelling is fixed AT THE PRODUCER, or canonicalised once at the + // ingestion choke point (`normalizeSchemaReferenceKeys`, which stamps + // both snake_case keys from whichever spelling arrived) — never by a + // renderer-side alias. + // + // The two remaining arms are deliberately left standing: deciding + // between them per reader is objectui#6837's OPEN scope (the + // classification table over ~20 more readers), not this slice's. + reference_to: def.reference_to ?? def.reference, reference_field: def.reference_field ?? def.referenceField, required: def.required, validation: def.validation, diff --git a/packages/plugin-detail/src/__tests__/RecordDetailDrawer.referenceArms-6837.test.tsx b/packages/plugin-detail/src/__tests__/RecordDetailDrawer.referenceArms-6837.test.tsx new file mode 100644 index 0000000000..b8c1f36623 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/RecordDetailDrawer.referenceArms-6837.test.tsx @@ -0,0 +1,220 @@ +/** + * 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. + */ + +/** + * objectui#6837 (first slice) — the drawer's relationship-target chain drops the + * two arms NO CONTRACT DECLARES, and keeps the two that carry the value. + * + * Before: `def.reference_to ?? def.reference ?? def.referenceTo ?? def.target` + * After: `def.reference_to ?? def.reference` + * + * ## 1. The measurement this pin stands on (not just its conclusion) + * + * THE CELL: a value inside an object schema's `fields` container — literally + * what this component reads, `objectSchema.fields[name]`. Producer census by + * STRUCTURE WALK (TypeScript compiler API over every tracked `.ts`/`.tsx`, plus + * parsed JSON/YAML), recording each hit's ancestor property chain; EMIT + * positions only, so `def.target` (a read) is never counted as a producer. + * Subject and control terms were extracted BY THE SAME PASS, FROM THE SAME + * CELLS, IN THE SAME UNITS — the control sits on the JOIN, not merely on the + * terms. + * + * | term | role | repo-wide emits | IN THE CELL | + * |----------------|---------|-----------------|---------------| + * | `target` | SUBJECT | 1329 / 366 files| **0** / 0 | + * | `referenceTo` | SUBJECT | 80 / 41 files| **2** / 2 | + * | `reference` | CONTROL | 194 / 75 files| 92 / 36 | + * | `reference_to` | CONTROL | 136 / 87 files| 52 / 36 | + * + * A second, INDEPENDENT cell test that does not rely on a `fields` ancestor — + * "the enclosing object's own `type` is reference-bearing" + * (`EXPANDABLE_FIELD_TYPES`) — agrees: `target` 0, and every one of the 29 + * `referenceTo` hits is a test fixture at OTHER seams (action params, the + * filter builder, the retirement machinery), none in `plugin-detail` and none + * reaching this drawer. + * + * The two in-cell `referenceTo` hits are NEGATIVE fixtures of the retirement + * machinery itself (`object-fields-io.spec-keys`, `MetadataFieldsPage + * .specKeyReference`): they poison a draft with the retired key precisely to + * assert the read door STRIPS it. A fixture asserting removal is not a producer. + * + * Why `target`'s repo-wide 1329 is not a counter-example: every one of those + * emits belongs to a DIFFERENT TIER. Attributed by the enclosing object's own + * `type` value they are `api` (137), `url` (35), `script` (32), `form` (16), + * `flow` (13), `back` (9), `modal` (6), `fault` (5) — action and navigation + * nodes — plus 1062 with no sibling `type` at all (DOM event targets, link + * targets). Not one lands on a field definition. + * + * ## 2. Why refusal is correct, not merely unused-today + * + * `@objectstack/spec`'s `FieldSchema` refuses BOTH deleted spellings BY NAME + * with `unrecognized_keys`, each carrying its own "did you mean `reference`" + * rename — measured two-directionally against the installed spec, alongside + * `reference` parsing clean. `referenceTo` is additionally in + * `RETIRED_FIELD_KEYS` (objectui#6041 / #6519), so the designer read door + * strips it before any draft round-trips. So these were not "redundant" + * fallbacks: they were INVENTED tolerance surface — a silent absorption point + * for a producer that ought to fail visibly (AGENTS.md #0.1). + * + * ## 3. No precedence inversion exists here — stated rather than fabricated + * + * Both deleted arms sat at the END of the chain + * (`reference_to ?? reference ?? referenceTo ?? target`), so neither could ever + * preempt a contract-carrying spelling. There is therefore NO inversion case to + * pin, and this file deliberately does not invent one: a + * `{ reference: 'a', target: 'b' }` case would resolve to `'a'` both before and + * after the change and would measure nothing. (Copied from PR #6916 / card + * #6840, which set this form and made the same call for `value`.) + * + * ## 4. THE FLOOR, restated where someone would try to re-widen it + * + * ⛔ Do not re-add a spelling arm to this chain. A producer emitting a refused + * spelling is fixed AT THE PRODUCER, or canonicalised ONCE at the ingestion + * choke point — `normalizeSchemaReferenceKeys`, which stamps both snake_case + * keys from whichever spelling arrived. Never a renderer-side alias: that is + * how twenty per-consumer dual-key fallbacks got written under a normalizer + * whose own docstring says it exists "so per-consumer dual-key fallbacks can't + * drift". + * + * ⛔ The two SURVIVING arms are out of this slice's scope. Deciding between + * `reference_to` and `reference` per reader is objectui#6837's OPEN scope (the + * classification table over ~20 more readers across eight other packages), and + * triage refused a single mechanical sweep because those readers are fed by + * different contracts. #6837 stays open. + * + * ## 5. Ablation direction, predicted before running + * + * Restore either deleted arm on the committed tree and the matching refusal + * goes RED while every live-arm control stays GREEN — that contrast is what + * makes the controls controls rather than duplicates of the pins. The pin + * imports the component by RELATIVE SOURCE PATH (`../RecordDetailDrawer`), so + * no package `exports` hop and no `dist` leg is involved. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render } from '@testing-library/react'; +import React from 'react'; +import { normalizeSchemaReferenceKeys } from '@object-ui/core'; +import { RecordDetailDrawer } from '../RecordDetailDrawer'; + +/** + * The drawer hands its derived field list to `DetailView` as `schema.fields`. + * Standing in for `DetailView` captures that list without rendering the whole + * detail tree — the resolved `reference_to` under test is a property of the + * list, not of how DetailView paints it. (Same harness as + * `expandableFamily.identity-5874.test.tsx`, which pins the sibling rule + * derived from the very same `.map()`.) + */ +const capturedFields: { current: any[] } = { current: [] }; +vi.mock('../DetailView', () => ({ + DetailView: ({ schema }: any) => { + capturedFields.current = schema?.fields ?? []; + return
; + }, +})); + +/** Every probe is a `lookup`, so only the target SPELLING varies between them. */ +const FIELD_DEFS: Record