fix(app-shell): canonicalize reference keys on the by-name object-schema serve path (objectui#7650) - #8868
Conversation
…ema serve path (objectui#7650)
`MetadataProvider` has two paths that hand an object schema to a reader and only
one of them normalized. `ensureType('object')` — the LIST path — has run
`normalizeSchemaReferenceKeys` over every fetched item since objectui#2407.
`getItem('object', name)` — the BY-NAME path, behind the published
`useMetadataItem` hook — ran `extractItem`, which unwraps the `{ item }`
envelope and normalizes nothing.
Which spelling a reader saw therefore depended on cache order, not on the
document: warm it got the stamped def, cold it got whichever single key the
producer stored. The serve path never parses, so `FieldSchema` strictness is no
evidence a legacy spelling cannot arrive — a document stored before the key was
tightened is served back verbatim, forever.
`getItem` now applies the same idempotent, in-place stamp for `type === 'object'`.
No other metadata type is touched, nothing is dropped, and a spelling the
producer already set keeps its own value.
The coverage comment in `packages/core/src/utils/reference-keys.ts` said this
file runs at "exactly two production call sites". That was true about where it
RAN and false about the serve surface it was cited for; it now names three and
says how to re-derive the list.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01611D6ZaRaMmwTNQmSbk8MH
PM 评审 —— PR #8868（卡 #7650 的半 C）：通过，⛔ 保持 draft，停队等契约评审
⛔ 我在裁决里给的 Clause-② 论证是错的，dev 的是对的我在裁决（5605081157）里写过：C「是真正的 Clause-② dev 的反驳逐字：
⇒ 我把「某条路径服务过这个键」和「这个消费者见过这个键」混为一谈了。 一个只走 by-name 的消费者，此前拿到的载荷里确实没有那个键。机械底线因此成立， ⭐ 而这正是派发里那两句话存在的理由：「⛔ 不要继承 PM 的那句话」「拿不准就按 缺陷形状本身值得记下来⇒ 一个读者看到哪种拼法，取决于缓存顺序，⛔ 不取决于文档本身。 缓存热（列表先取过， ⭐ 这是这张卡的置信缺口给出的答案的实际形状 —— 卡片问「入口是不是只有一条」，答案是「不是」，而第二条的后果是非确定性，比「少了一次规范化」严重。
|
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…t two (objectui#7650) Comment only. The `reference_to` warning pin's docblock stated that `normalizeSchemaReferenceKeys` runs at "exactly the two ingestion choke points". The commit before this one added a third — `MetadataProvider.getItem` — so that sentence became false in the same change that made it false. Fixed here rather than left for a later reader to trust. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01611D6ZaRaMmwTNQmSbk8MH
|
Follow-up commit on this branch: The docblock of No test result in the body above moves: the runs quoted there were taken at
Generated by Claude Code |
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
|
Contract review at
|
|
Landing provenance — director seat, summon #18 segment 6 ( Generated by Claude Code |
3 participants
Part of objectui#7650 — this is the first of the two halves the ruling on that card
(comment 5605081157) ordered. It shuts a hole in behaviour that is already shipped;
the four-key canonicalisation the card is really about is the second half and rides its
own pull request. Neither half completes the card on its own, so nothing here uses a closing
keyword.
What was wrong
MetadataProviderhas two paths that hand an object schema to a reader, and only one ofthem normalised:
ensureType('object')— the LIST pathnormalizeSchemaReferenceKeysper item, since objectui#2407 / PR #2587getItem('object', name)— the BY-NAME path, behind the publisheduseMetadataItemhookextractItem, which only unwraps the{ item }envelopeSo which spelling a reader saw depended on cache order, not on the document. Warm — the
list had already been fetched, so
entry.byNamewas populated by the normalising path — itgot the def with both keys stamped. Cold — a
useMetadataItem('object', name)that ranbefore any list fetch — it got whichever single key the producer had stored. A consumer that
reads one key rendered a raw id in the cold case and the relation in the warm one: the
objectui#2407 bug, still reachable through the one door left open.
Why it matters that this is a SERVE path
objectui#7650 measured that the object-schema serve path never parses:
ObjectStackAdapter.getObjectSchemaapplies exactly two mutations and runs noObjectSchema.parseat all (lit control in the same file: it does import and useDroppedFieldsEventSchemaandisFilterAST, so the zero is a real absence, not a failedgrep).
FieldSchemastrictness therefore gates the write door only — a document storedbefore a key was tightened is served back verbatim, forever, and a host with its own
getObjectSchemais served straight through.useMetadataItemis exported from@object-ui/app-shell, so the unnormalised def reachedout-of-repo consumers too, not only the five in-repo
objectcallers.What changed
packages/app-shell/src/providers/MetadataProvider.tsx—getItemapplies the sameidempotent, in-place stamp for
type === 'object'before it caches. Nothing else istouched: no other metadata type, no drop, no overwrite of a spelling the producer set.
packages/core/src/utils/reference-keys.ts— comment only. Its coverage block said thisfile runs at "exactly two production call sites". That was true about where it ran and
false about the serve surface it was cited for. It now names three, and says to
re-derive the list from the serve paths rather than by grepping for the call.
Clause-② declaration
Clause-②: yes— measured on this diff, not inherited.The mechanical floor in the contract-review rules is "a new exported symbol or a new key on
a published payload is always yes". No new exported symbol. But a key is new on a
published payload: before this change a def served through the by-name path came back
carrying only the spelling its producer stored, and after it carries both. The tempting
argument against — "those keys are already served on the other two paths, so no consumer
sees a key it was not already seeing" — is about a different path, and a consumer that
only ever reads by name did previously get a payload without the key. Arguable is not
self-evident, and the rule for arguable is
yes.needs:contract-reviewis hung on this PRand on the card; this PR parks until that review is reachable.
Tests
New pins in
packages/app-shell/src/providers/__tests__/MetadataProvider.itemReferenceKeys-7650.test.tsx(6 cases). Every assertion reads the value a consumer gets back from
getItem— neverthe provider's internals, because a re-plumbing that still served an unstamped def by some
other route would satisfy an internals assertion and violate the card.
Four positive pins (both fold directions, the array field-container shape, and cache
stickiness on a repeat read) and two negative pins that are the load-bearing half: a
normaliser that fired for every metadata type, or that overwrote a key the producer
had already set, would pass every positive assertion.
Runs, all in the dedicated worktree at commit
20890fe13:Test Files 1 passed (1),Tests 6 passed (6)MetadataProvider,useMetadataItemornormalizeSchemaReferenceKeys, plus the new one:Test Files 77 passed (77),Tests 836 passed (836)turbo run type-check --filter=@object-ui/app-shell --filter=@object-ui/core:Tasks: 31 successful, 31 total— and each package'stype-checkrunstsc --noEmit && tsc -p tsconfig.test.json, so the new test file is type-checked toocheck:designer-field-key-parity,check:handler-key-reads,check:spec-symbols,check:control-bytes,check:unreferenced-sources,check:vi-mock-specifiers,check:vi-mock-inherit,check:comment-mask-corpus,check:changeset-presence— allexit 0 on their own printed verdict line
Declared narrowing: the full
packages/app-shell/suite (668 test files) does not fitthe container's foreground budget under lock contention — one attempt acquired the lock
after 126s and was still running at the 560s cap. It is left to CI. The narrowing is stated
here rather than presented as coverage.
Ablation
Mutation: delete the one added line from the committed tree, then re-run the new pins.
d9a75c607c0f2427…, mutated worktreeblob
d65fb48ae3dff8dc…, marker count 1 to 0reference_toon a by-name objectdef that spells only
reference", "stampsreferenceon a by-name object def that spellsonly the legacy
reference_to", "normalizes the ARRAY field-container shape the metadataAPI also serves", "serves the SAME canonical shape on a repeat read, without a second
fetch". The two that stayed green are exactly the two negative pins, which assert that
something is not normalised — the expected direction, and the reason they are listed
rather than glossed
d9a75c607c0f2427…,git diff HEADempty,marker count back to 1. The restore is
git checkout HEAD -- PATH, never a baregit checkout -- PATH, and the script carries atrap … EXIT INT TERMNo build was involved: the mutated file is app-shell's own source and the test imports it by
relative path, so nothing resolved through a
dist/that could have gone stale.验收备注
Recorded, nothing filed, and none of it rides this PR:
objectcallers ofuseMetadataItemnumber five, not three:components/src/renderers/basic/metadata-viewer.tsx,components/src/renderers/layout/containers.tsx,plugin-detail/src/renderers/record-quick-actions.tsx,plugin-detail/src/renderers/record-alert.tsxandapp-shell/src/views/DeclaredActionsBar.tsx. The earlier count of three, reported on thecard, was an undercount by me; correcting it here so the card's record is right. Four of
the five read
.actionsoff the def rather than a relation, which is why nothing in-repowas visibly broken — the published hook is the surface that was.
mergeViewsIntoObjects, soobjectDef.listViewsispopulated on the list path only. That is a different key and a different decision; not
touched here, and no card filed, because no reader was measured taking the by-name path
for
listViews. Successor: whoever next widens the by-name path.Session, written as prose because a markdown-link footer does not reliably survive a body
PATCHin this repository: this pull request was produced by the Claude Code seathttps://claude.ai/code/session_01611D6ZaRaMmwTNQmSbk8MH.Generated by Claude Code