Uh oh!
There was an error while loading. Please reload this page.
fix(core,app-shell): resolve a relationship target from reference only - #6649
Merged
Merged
Conversation
`resolveReferenceTo` (dataset designer) and its sibling
`resolveRelationshipTarget` (`chart-series.ts`) each read a relationship
field's target through a four-spelling tolerant chain:
reference ?? reference_to ?? referenceTo ?? reference_to_object
Censused against every producer that can reach them, with `reference` as the
positive control: `ObjectSchema.safeParse` (spec 17.2.0) ACCEPTS `reference`
and REFUSES the other three BY NAME, and none of the three has a producer on
the object-metadata surface. `reference_to` is live only on ObjectUI's own
view/field schema (a different contract, translated INTO from `reference`);
`referenceTo`'s two producers were retired by objectui#6041 and it is stripped
by the read door since objectui#6519; `reference_to_object` occurs nowhere in
either tree outside the chain and the test that called it.
Narrowed in BOTH places in one pass so the two canonicalizations cannot
diverge. The dead-branch unit tests are updated with the helpers and gain
refusal pins, so re-widening the chain turns red.
`chart-series.ts` reads `GET /meta/object/:name` directly, with no read door,
so this is a real behaviour change there: a stored pre-objectui#6041 row
spelling the target `referenceTo` no longer resolves. The walk is best-effort
by construction, so it degrades visibly rather than being absorbed — per
AGENTS.md #0.1 such a row is a producer-side defect, and a lenient consumer is
where it would have stayed hidden. `reference` was already head of the old
chain, so a document carrying both is unaffected.
Carriers (string / array / `{ object }`) are untouched — a separate axis.
Part of #6528Contributor
✅ 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
|
os-sales
marked this pull request as ready for review
August 28, 2026 08:51
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6528
resolveReferenceTo(dataset designer) and its siblingresolveRelationshipTarget(packages/core/src/utils/chart-series.ts) each read a relationship field's target through a four-spelling tolerant chain:The card asked for the dead
referenceTobranch. Following the triage charter, the spellings were measured first and three of the four turn out to be unfounded, so the chain is narrowed toreferencein both places in one pass — they must not diverge, since a fix leaving them disagreeing recreates the defect one file over.1. Census — which spellings can a served document actually carry
Every zero carries a positive control in the same query shape, so it is a measurement rather than a broken pattern.
referenceis that control throughout.ObjectSchema.safeParse(spec 17.2.0)referenceMetadataService.toFieldPayload,MetadataFieldsPage.fromDesignerField, eachreference: target); 445 of 565 lookup / master_detail defs in the framework treereference_tounrecognized_keys, "Did you meanreference_to->reference?"referenceTounrecognized_keys, "Did you meanreferenceTo->reference?"reference_to_objectunrecognized_keys, and not even a recognised aliasPopulation census behind the producer column — every lookup / master_detail field def in each tree, and which spelling names its target:
referencereference_toreferenceToreference_to_objectobjectstack)objectui)Both trees' non-
referencehits were read individually rather than counted and trusted:referenceTo: 'reference', the machinery that PRODUCES the refusal hint). Zero production emit sites.driver-sqlstates it outright — "reference_tois a REJECTED ALIAS, not a normalised one".reference_tohits are almost entirely a different surface.@object-ui/types'views.zod.tsdeclaresreference_toon ObjectUI's own view/field schema, andplugin-detailtranslates INTO it from the servedreference(RecordDetailDrawer.tsx,RelatedList.tsx,buildDefaultPageSchema.ts). That is a separate contract; an object metadata document never carries it. ThereferenceTohits are ObjectUI's own TS property names (DatasetRelationship.referenceTo, the resolver's OUTPUT), not wire keys.reference_to_objectoccurs nowhere in either tree outside the two chains and the two unit tests that called them. It was only ever produced by its own test.2. Premise verified on the merged ref
The cut rests on "the
referenceTobranch is unreachable since #6519 strips the key". Confirmed onorigin/mainat 5361083, not taken from the card:packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts—RETIRED_FIELD_KEYS = ['indexed', 'referenceTo', 'isSystem'], stripped byreadFields.normalizeObjectbuilds every def throughreadFields, and it isresolveReferenceTo's only production caller.So
referenceTocould not reach that helper even from a stored pre-#6041 row. Premise holds.3. The fork did NOT fire
No real producer emits a legacy spelling onto a document either consumer reads. The two historical
referenceToproducers were already retired at the producer by #6041 — which is the remedy the fork prescribes, applied before this card existed.One producer-side finding did turn up and is filed rather than fixed here: #6647 — the console
objectpreview sample spells a lookup targetreference_to, a second off-spec key itsKNOWN_STALEreason does not record. It is dev-only and unreachable from both consumers, so it neither blocks nor is blocked by this cut.4. One real behaviour change, and it is deliberate
The two consumers are not symmetric, and the difference is load-bearing:
resolveReferenceTosits behind the read door — the strip makes its narrowing cosmetic.resolveRelationshipTargetreadsGET /meta/object/:namedirectly (ObjectChart.tsx,useDatasetDimensionLabels.tsboth fetch and pass the raw doc). No door strips retired keys on that path.So a stored pre-#6041 row spelling the target
referenceTono longer resolves in the chart path. The walk is best-effort by construction — no entry is yielded and the caller keeps the raw value — so such a row degrades visibly instead of being silently absorbed. Per AGENTS.md #0.1 that row is a producer-side defect, and a lenient consumer is exactly where it would have stayed hidden.referencewas already head of the old chain, so any document carrying both is unaffected (pinned by a new test).5. Tests — updated with the helpers, not preserved as cover
The dead-branch pins asserted the removed branches. They are rewritten, and the refusal is now pinned in both files so a silent re-widening turns red:
Fixture triage across the rule's consumption radius, not just the edited packages: fixtures feeding
normalizeObject/useDatasetFieldCatalogwere re-spelled toreference(they used an alias). Every dimension-walk exerciser inplugin-dashboard,plugin-report,reactandplugin-chartswas checked and carries zero legacy spellings, with livereferencecontrols proving the query.column-sortability.test.tscarries areference_tofixture but its module reads no reference key at all — out of radius, left alone.Verification
All readings below are on the final commit,
4ef4cc7c.pnpm exec vitest runoverpackages/core/src/utils/__tests__/,packages/app-shell/src/views/metadata-admin/inspectors/, and the dataset widget / report renderer walk exercisers —Test Files 97 passed (97),Tests 1334 passed | 1 skipped (1335).EXIT/INT/TERMtrap with absolute paths) givesTests 6 failed | 51 passed— exactly the 6 refusal pins, 3 per file, nothing else. The restore leg is proven bygit diff HEADempty and blob-hash equality againstHEAD(which carries the fix, since the fix was committed first). No rebuild step is involved: this repo resolves both vitest andtsctosrc, so there is nodistfor a stale artifact to hide in.@object-ui/core(--filter '...@object-ui/core', prefix form = consumers): 32 Done, 1 Failed, the failure beingapps/siteunable to resolve an unbuilt@object-ui/example-schema-catalog. Building that example and re-runningapps/sitealone passes, so the closure is 33/33 clean. Every consumer of the two changed functions is green:core,app-shell,plugin-charts,plugin-dashboard,plugin-report,react.pnpm lint(eslint .over the whole repo, no narrowing):47 successful, 47 total,0 errors(2773 pre-existingno-explicit-anywarnings, unchanged in kind; this diff removes twoas anycasts).check-changeset-presence.mjs— "5 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)";check-changeset-no-major.mjs— "No changeset declares amajorbump." The bump is an honestpatchon both packages; the gate ruled it and was not pre-empted.Scope
Confined to
packages/coreandpackages/app-shell.MetadataServicewas read but never touched (#6490 is live inapp-shellonsaveObject); nothing here reaches it.#6597is not addressed here and nothing in this PR settles it. It concernsFieldMeta.referenceToinplugin-dashboard— same word, different surface, still awaiting a ruling.plugin-dashboardandFieldMetaare untouched. One census result is input to that pending ruling and no more: spec 17.2.0 refusesreferenceToby name on aFieldSchemawith an explicit rename hint toreference, and thereferenceTooccurrences in this repo's own TS interfaces are property names rather than wire keys.The string / array /
{ object }carriers are untouched — a separate axis from the spelling, whose own census is not finished. Filed as #6648.Generated by Claude Code