Uh oh!
There was an error while loading. Please reload this page.
console: read the converged FormFieldSpec instead of a third copy of it - #5597
Merged
Merged
Conversation
…it (#5542) objectui#5040 was not a missing key — it was that two hand-written descriptions of one contract drifted, and nothing could notice, because each was only ever checked against itself. PR #5537 converged the two app-shell descriptions into views/metadata-admin/form-spec.ts. A third survived: apps/console FormPage.tsx declared its own nine-key `interface FormFieldSpec`, under the same name, in a different package, leaving that failure mode fully available. Measured key by key before picking a route. The console's copy was a strict subset — 9 of the shared type's 26 keys, every one identical in type, none console-only — sitting in a position that describes an AUTHORED DOCUMENT: `FormSectionSpec.fields`, read straight off the /meta/view/:name payload, the same spec FormView metadata-admin renders (both files spell the same six-member `type` union and call the element type `FormFieldSpec`). The narrow, renderer-honoured shape is a different type that already exists in that file, `RenderableField`. So this was one contract described twice, and the console's description was wrong about the document: `visibleWhen`, `dependsOn`, `type`, `options`, `immutable`, the recursive `fields` and ten more legal keys were undeclared there — #5040's own symptom, "the type rejects the configuration the runtime accepts". Route: import, not rename. @object-ui/app-shell re-exports FormFieldSpec from its package root (type-only, erased at build), FormPage.tsx imports it and deletes the local declaration. Reachability is load-bearing: a type that cannot be imported is a type that gets retyped. form-spec.ts itself is untouched. FormPage.fieldSpec.test.ts is the pin. It reads the field-spec type back out of the exported buildSections signature rather than naming it, so re-inlining a local copy fails type-check even if the copy agrees on every key the day it is written. Liveness controls keep it from being a phantom check: the removed nine-key shape is pinned NOT equal to the shared type, RenderableField is pinned not equal either, and an undeclared key is still rejected. Fixes#5542 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
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
|
os-sales
commented
Aug 21, 2026
CollaboratorAuthor
PM review — ACCEPT (card #5542)Gates. 22 named check runs read individually for Merits.
PM fence error, disclosed. I fenced Clause ② — not tripped. The two added lines are type-only: no contract accept/reject behaviour changes, no runtime surface widens, and the barrel already re-exports from Flagged to the maintainer. Exporting these two types from the app barrel is a boundary call I made, not one the card asked for. It is cheaply reversible later — either a subpath export, or moving the type into a shared package — if you would rather the console barrel stay closed. Generated by Claude Code |
os-sales
marked this pull request as ready for review
August 21, 2026 16:47
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 21, 2026
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#5542
The comparison, key by key — the route is a measurement
Triage ruled that both outcomes close the class and that which one applies is a
measurement, not a preference. Here is the measurement.
The console's declaration (
FormPage.tsx:278oncad512fe1) had 9 keys. Theconverged app-shell declaration (
form-spec.ts:26) has 26. Every one of theconsole's 9 is present in the shared type with an identical type, and the console
had zero keys of its own:
form-spec.tsfieldstring(required)string(required)labelstring?string?placeholderstring?string?helpTextstring?string?requiredboolean?boolean?readonlyboolean?boolean?hiddenboolean?boolean?colSpan1 | 2 | 3 | 4optional1 | 2 | 3 | 4optionalwidgetstring?string?type,options,reference,dependsOn,maxLength,minLength,min,max,precision,scale,multiple,immutable,disclosure,language,visibleWhen,visibleOn,fieldsZero disagreements, zero console-only keys, strict 9-of-26 subset.
Which route the comparison supports, and why
Same contract — import it. A subset relation alone does not settle it; a narrower
authoring layer would also look like a subset. Three further facts do settle it:
The type sits on
FormSectionSpec.fields, andsec.fieldsis read straight off the/meta/view/:namepayload (resolveInternalFormcasts the response intoFormViewSpec). That is the same specFormViewmetadata-admin renders — app-shell'sown container says so in as many words, "Lightweight shape of the spec
FormViewweconsume". Both files even spell the same character-identical six-member
typeunionand both call the element type
FormFieldSpec.FormPage.tsxdeclares
RenderableField— the normalized rowbuildSectionsemits, which really is"what this renderer honours". The narrowing role was already filled, so the local
FormFieldSpecwas not filling it.metadata the spec accepts and the runtime passes through —
visibleWhen,dependsOn,type,options,immutable, the recursivefields, 17 keys in all — wasundeclared here. That is objectui#5040's own symptom, "the type rejects the
configuration the runtime accepts", which no runtime test can see.
A rename would have declared a narrowing that the code does not perform, so it was not
the honest route even though it is the smaller diff.
The change
@object-ui/app-shellre-exportsFormFieldSpecfrom its package root. Type-only— erased at build, nothing added to the bundle (the eager-closure gate confirms it
below). Reachability is the load-bearing half of the fix: the type was previously
unnameable outside
views/metadata-admin/, and a type that cannot be imported is atype that gets retyped.
FormPage.tsxalready documents this exact unreachability fora sibling case (
urlParams), which is how the third copy came to exist.FormPage.tsximports it and deletes the local declaration.FormPage.fieldSpec.test.tsis the pin.packages/app-shell/src/views/metadata-admin/form-spec.tsas a read-only reuse targetand it is untouched — confirmed, it is not in this diff. But reuse needed
reachability, so this PR does add two additive
export typelines to app-shell'sbarrels (
views/metadata-admin/index.tsandsrc/index.ts). That is beyond the letterof the declared file surface. Flagging rather than burying it: if the reviewer would
rather not widen app-shell's public type surface, the alternative is a subpath export or
moving the type to a shared package, both larger than this card.
The pin, and proof that it bites
The pin does not name the console's type — it reads it back out of the exported
buildSectionssignature, walking spec → sections → fields → the non-string arm. So itfollows whatever
FormPage.tsxactually uses in that position and cannot be satisfied bya decoy:
(Written with parens above because the write path eats angle brackets; the file uses real
generics.)
Liveness probe — a type assertion no project compiles is a phantom check. Rather than
flip the assertion to a trivially false thing, I simulated the actual regression: dropped
the shared import and re-inlined the nine-key local
interface FormFieldSpec, exactly asa future agent would.
FormPage.tsxstill compiles on its own, so the redness is the pinand nothing else:
Line 81 is the pin. Note the TS2353 pair: a re-inlined copy immediately re-creates
#5040's exact symptom, and the pin reports it in #5040's own words. Restored via
trap ... EXIT INT TERM;git status --porcelainempty afterwards and both anchoredcounts back to 1 / 0.
Three further controls stop the pin being vacuous, and they compile on every run
rather than being a one-time ritual:
Equalhelper is proven to still discriminate;
RenderableFieldis pinned not equal to it either, so the honoured-row andauthored-document types cannot be quietly collapsed again;
@ts-expect-error), so the import did not smugglein an index signature or
any— the failure mode a widening reaches for first.check-type-check-coverageindependently confirms a project really compiles this file:41/41 packages compile their tests, 0 with a narrow type-assertion project.Gate verdicts — exit codes captured before any pipe, each gate's own verdict line quoted
All run at
5536bd387, the final commit; the probe restored the tree to that exact shabefore this list was taken.
@object-ui/consoletype-checkos-verify-lock: VERDICT command-exit 0 · held the lock 21s@object-ui/app-shelltype-checktsc --noEmitthentsc -p tsconfig.test.json, no output)@object-ui/app-shellbuildtsc, no output)@object-ui/consolelint✖ 200 problems (0 errors, 200 warnings)@object-ui/app-shelllint✖ 2509 problems (0 errors, 2509 warnings)vitest run(repo root cwd)Test Files 202 passed (202)/Tests 2191 passed | 1 skipped (2192)vitest— the new pin file, verboseTest Files 1 passed (1)/Tests 2 passed (2)check-control-bytes✅ check-control-bytes: OK (scanned 4647 tracked text file(s); skipped 85 binary).check-changeset-presence✅ 3 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major✅ No changeset declares a 'major' bump.check-changeset-fixed✅ All workspace packages are in the changeset fixed group.check-type-check-coverage✅ type-check coverage: 45/46 via 'type-check' … 41/41 packages compile their testscheck-lint-coverage✅ lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).@object-ui/consolebuildtsc,vite build,tsc -p tsconfig.plugin.json)check-eager-closure-budget✅ Console eager closure is 3784.9 KB gzipped across 52 of 508 chunks (budget: 3867.2 KB, headroom: 82.3 KB).Two notes on how that list was built rather than taken on trust:
the CI workflows against my actual diff, which added four gates the dispatch did not
name:
check-type-check-coverage,check-lint-coverage,check-eager-closure-budget,and the console
buildthe last of those needs.check-eager-closure-budgetwas redbefore the build —
No eager-closure report … This is a broken gauge, not a passing budget— so rather than argue that a type-only import must be erased, I built theconsole and read the real number.
vitestreporter names only failing files, so a green run is not byitself evidence that the new pin file ran at all — the same shape as a
--filterthatmatches nothing. Re-run alone with
--reporter=verbose, which names it under the@object-ui/consoleproject with both tests passing.Declared narrowing:
vitestwas scoped toapps/console/src/components/andpackages/app-shell/src/views/metadata-admin/rather than the whole repo. Evidence itcannot hide a failure: the app-shell half of this diff is two
export typelines, whichemit no JavaScript at all, so no module outside those two paths can observe a runtime
difference; the console half is an
import typeplus a deletedinterface, both fullyerased. CI runs the full farm regardless.
Out of scope, filed separately
Three findings from the same read, all unassigned, none fixed here:
visibleWhen/visibleOnat all.objectui#2212 recorded that symptom and PR fix(form): evaluate view-level FormField.visibleOn with the canonical CEL engine #2214 fixed it in the other form chain
(
plugin-formsectionFields.tspluscomponentsrenderers/form/form.tsx);FormPage.tsxis not on that chain, so the fix never reached it. User-reachable.buildSectionsdrops the field-levelmaxLengthoverride while its owndocstring promises overrides win. Pre-existing and unchanged by this PR, but newly
expressible because the shared type declares the key. The new test records the
current answer explicitly rather than assuming it.
FormViewSpecandFormSectionSpecare eachhand-declared twice, and unlike the field spec those two have already drifted in
both directions.
finding, not queued: converging the containers is a realarchitectural call, not a mechanical one.