From 04cada7936a6f1bb477be66e594e387fe422c6d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 16:05:13 +0000 Subject: [PATCH] fix(plugin-detail): drop the two undeclared arms from the drawer's target chain `RecordDetailDrawer` resolved a relationship target through four spellings: def.reference_to ?? def.reference ?? def.referenceTo ?? def.target Two of them are declared by no contract. `@objectstack/spec`'s `FieldSchema` refuses both `referenceTo` and `target` by name with `unrecognized_keys`, each carrying its own "did you mean `reference`" rename; `referenceTo` is also stripped at the designer read door (`RETIRED_FIELD_KEYS`). So they were not redundant fallbacks but invented tolerance surface -- a silent absorption point for a producer that ought to fail visibly (AGENTS.md #0.1). A repo-wide structure-walk producer census (TypeScript compiler API over every tracked source, plus parsed JSON/YAML, recording each hit's ancestor property chain; emit positions only) found 0 producers 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. The two surviving arms are deliberately untouched: choosing between them per reader is the open scope of objectui#6837, whose triage refused a single mechanical sweep because the ~20 remaining readers are fed by different contracts. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB --- .../6837-recorddrawer-invented-target-arms.md | 36 +++ .../plugin-detail/src/RecordDetailDrawer.tsx | 28 ++- ...rdDetailDrawer.referenceArms-6837.test.tsx | 220 ++++++++++++++++++ 3 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 .changeset/6837-recorddrawer-invented-target-arms.md create mode 100644 packages/plugin-detail/src/__tests__/RecordDetailDrawer.referenceArms-6837.test.tsx 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 da15724a89..ec43039c0e 100644 --- a/packages/plugin-detail/src/RecordDetailDrawer.tsx +++ b/packages/plugin-detail/src/RecordDetailDrawer.tsx @@ -263,8 +263,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> = { + // Live arms — the two spellings a contract actually carries at this seam. + canonical: { type: 'lookup', label: 'Canonical', reference_to: 'crm_account' }, + spec_spelling: { type: 'lookup', label: 'Spec', reference: 'crm_account' }, + // Deleted arms — refused by name, zero producers in the cell. + legacy_camel: { type: 'lookup', label: 'Legacy camel', referenceTo: 'crm_account' }, + invented: { type: 'lookup', label: 'Invented', target: 'crm_account' }, + // Non-relation control: carries no target spelling at all. + plain_text: { type: 'text', label: 'Plain' }, +}; + +const RECORD: Record = { + id: 'r1', + canonical: 'acc-1', + spec_spelling: 'acc-1', + legacy_camel: 'acc-1', + invented: 'acc-1', + plain_text: 'hello', +}; + +/** Render the drawer over `fields` and return the list handed to DetailView. */ +function resolveFields(fields: Record) { + capturedFields.current = []; + render( + {}} + title="Probe" + record={RECORD} + recordId="r1" + objectName="probe" + objectSchema={{ name: 'probe', fields } as any} + onFieldSave={async () => {}} + />, + ); + return capturedFields.current; +} + +/** The `reference_to` the drawer resolved for one field name. */ +function resolvedTarget(name: string, fields: Record = FIELD_DEFS) { + return resolveFields(fields).find((f: any) => f.name === name)?.reference_to; +} + +describe('RecordDetailDrawer resolves only contract-declared target spellings (objectui#6837)', () => { + describe('live arms — the value still arrives (without these, a drawer that stopped resolving anything would pass the refusals too)', () => { + it("resolves `reference_to`, ObjectUI's own view/field key", () => { + expect(resolvedTarget('canonical')).toBe('crm_account'); + }); + + it('resolves `reference`, the spelling `FieldSchema` accepts', () => { + expect(resolvedTarget('spec_spelling')).toBe('crm_account'); + }); + + it('still derives the whole field list, relations and non-relations alike', () => { + // Guards the refusals below against the degenerate pass: a drawer that + // produced no fields at all would satisfy every `toBeUndefined()`. + const names = resolveFields(FIELD_DEFS).map((f: any) => f.name); + expect(names).toEqual( + expect.arrayContaining(['canonical', 'spec_spelling', 'legacy_camel', 'invented', 'plain_text']), + ); + }); + + it('marks the relation fields readonly, so the list is genuinely populated', () => { + const f = resolveFields(FIELD_DEFS).find((x: any) => x.name === 'canonical'); + expect(f?.readonly).toBe(true); + }); + }); + + describe('refusals — one named case per deleted key', () => { + it("does NOT read `referenceTo` (RETIRED_FIELD_KEYS, objectui#6041/#6519; `FieldSchema` refuses it by name)", () => { + expect(resolvedTarget('legacy_camel')).toBeUndefined(); + }); + + it("does NOT read `target` (no contract declares it; 0 producers in the cell against controls of 92 and 52)", () => { + expect(resolvedTarget('invented')).toBeUndefined(); + }); + }); + + describe('the ingestion choke point is what makes the `referenceTo` deletion lossless', () => { + it('a `referenceTo`-only def that came through `normalizeSchemaReferenceKeys` STILL resolves', () => { + // This is the mechanism, not a formality: the normalizer reads + // `reference_to ?? reference ?? referenceTo` and stamps both snake_case + // keys, so every def that entered through MetadataProvider or + // ObjectStackAdapter.getObjectSchema already carries `reference_to` by + // the time the drawer sees it. The deleted arm was dead weight for those. + const schema = { name: 'probe', fields: { legacy_camel: { ...FIELD_DEFS.legacy_camel } } }; + normalizeSchemaReferenceKeys(schema); + expect(resolvedTarget('legacy_camel', schema.fields)).toBe('crm_account'); + }); + + it('a `target`-only def does NOT resolve even through the normalizer — nothing in the stack ever declared it', () => { + // `normalizeFieldReferenceKeys` never read `target` either. So unlike + // `referenceTo`, `target` had no home anywhere in the stack: not in the + // spec, not at the choke point, not in the retirement registry. + const schema = { name: 'probe', fields: { invented: { ...FIELD_DEFS.invented } } }; + normalizeSchemaReferenceKeys(schema); + expect(resolvedTarget('invented', schema.fields)).toBeUndefined(); + }); + }); +});