Uh oh!
There was an error while loading. Please reload this page.
refactor(plugin-dashboard): one relation predicate, not two that agree by coincidence - #6099
Merged
Merged
Conversation
…e by coincidence `computeLookupExpand` in `ObjectDataTable.tsx` carried its own `isLookup`, byte-identical to the exported `isLookupType` in `recordFields.tsx` once objectui#5692 had pointed both at `@object-ui/core`'s `EXPANDABLE_FIELD_TYPES`. Nothing kept them aligned. `computeLookupExpand` now calls `isLookupType` — its first production consumer — and the module no longer imports the shared family or the retirement gate. Behaviour-preserving, measured rather than assumed: identical bodies give identical boolean answers, and `reportRetiredFieldType` dedupes per SPELLING in one module-level set in `@object-ui/core` that both bodies already shared, so the warning count cannot move either. A refactor with no observable delta cannot be pinned by a behavioural test, so the pin is identity: `computeLookupExpand` is observed CALLING `isLookupType`, and `ObjectDataTable.tsx` is read at source level to confirm no second body survives for it to call instead.
…r its own ablation
Measured on the first ablation leg: a genuine revert failed the pin on its
CONTROL line ('probe stripped the code away'), which is the message a broken
probe would give. Control and subject were indistinguishable. The controls are
now text present in BOTH versions of the file, and the isLookupType call count
moved to the subject where it belongs.Contributor
✅ 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
|
yinlianghui
marked this pull request as ready for review
August 24, 2026 17:09
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#5876
Option B, triage's preference:
computeLookupExpandnow callsisLookupTypeinstead of carrying its own copy of it. The package holds one relation predicate.
What was there
ObjectDataTable.tsxheld a localisLookupinsidecomputeLookupExpandthat wasidentical to the exported
isLookupTypeinrecordFields.tsx— same retirementgate, same
EXPANDABLE_FIELD_TYPES.has, verified line-for-line on the merge-base(
7c96c9420). They agreed because #5692 had just pointed both at the same set, andnothing kept them agreeing afterwards.
computeLookupExpandnow callsisLookupTypeat both call sites, and the module nolonger imports
EXPANDABLE_FIELD_TYPES,isRetiredFieldTypeorreportRetiredFieldTypeat all.isLookupTypegains its first production consumer,so #4914's item-9 coverage in
lookupTypes.retiredFieldType.test.tskeeps workinguntouched — the asymmetry that makes B cheaper than A.
Behaviour-preserving, and that is measured, not assumed
Triage called this "behaviour-affecting". With identical bodies it is not, and both
candidate deltas were checked:
treeis stillexpanded,
referenceis still refused, ordinary relations are still carried.reportRetiredFieldTypededupes per spelling, inone module-level
Setinside@object-ui/core(
retired-field-types.ts: "the dedupe is per SPELLING, not per face"). Both bodiesalready shared that Set, so routing two callers through one function cannot move
the count. If the key had been per call site, this would have been a real delta.
Nothing published moves:
isLookupTypeis not re-exported fromindex.tsx, so it isnot on
@object-ui/plugin-dashboard's published surface.patch, notminor.renderFieldValuewas deliberately not moved (#6014 reads that seam).Why the pin is identity, not behaviour
A refactor with no observable delta cannot be pinned by a behavioural test. Every
assertion in the three pre-existing describes —
treeexpands,referencedoes not,ordinary relations do — passes identically on a revert. They are controls for this
change, not pins of it.
The new pin in
expandableFamily.identity-5692.test.tshas two halves that fail fordifferent reasons:
`computeLookupExpand` CALLS `isLookupType` — in BOTH column modes— avi.doMockdouble (surgical: not hoisted, so the rest of the file stays on the real module)
records the call in the explicit-whitelist and auto-derive paths.
`ObjectDataTable.tsx` holds no second predicate body— source read with commentsstripped, so the convergence note that names those symbols cannot fake a hit.
Ablation, direction predicted before running: restore the merge-base
ObjectDataTable.tsxand both new pins go RED while everything else stays GREEN.Observed exactly that —
2 failed | 18 passed, failing onexplicit whitelist answered the relation question without asking isLookupTypeandcomputeLookupExpand stopped calling the shared predicate. Mutation was proven ondisk before the run (local body restored: 1 hit;
isLookupType(: 0 hits; controlcomputeLookupExpand: 5 hits), and restored undertrap … EXIT INT TERMwithgit diff HEAD --statconfirmed empty afterwards.The first ablation leg found a real flaw and it was fixed in a second commit: the
source pin failed on its control line ("probe stripped the code away"), the same
message a broken probe would give — control and subject were indistinguishable. The
controls are now text present in both versions of the file.
Gates — all at
42c38c2a4, the final commitpnpm --filter @object-ui/plugin-dashboard type-checktsc --noEmitandtsc -p tsconfig.test.json)packages/plugin-dashboard/srcTest Files 80 passed (80),Tests 754 passed (754)pnpm --filter '...@object-ui/plugin-dashboard' type-check(downstream)Scope: 7 of 47, all 7Donepnpm lint(repo-wideeslint ., no narrowing)47 successful, 47 totalnode scripts/check-changeset-presence.mjsnode scripts/check-changeset-no-major.mjsnode scripts/check-control-bytes.mjspnpm check:esm-specifiersThe downstream sweep is the prefix form — the package and its dependents
(
@object-ui/app-shell,apps/console,apps/site, three examples), i.e. thecontract direction. It went red once on
Cannot find module '@object-ui/example-schema-catalog'— the unbuilt-closure class, not this diff:building
examples/**flipped it green with the diff unchanged.check:esm-specifierswas run because an import was added toObjectDataTable.tsx;it is a step inside the
Type Checkjob, derived by enumerating that job's own steplist rather than top-level script names.
Generated by Claude Code