Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): make the Action designer's two panes agree on the param type vocabulary - #6554
Conversation
…am type vocabulary
`ActionDefaultInspector`'s `PARAM_TYPE_OPTS` offers eight `type` spellings.
`ActionPreview.renderFieldMock` switched on the RAW authored spelling over a
private table of five, so three of the eight previewed as a control
`ActionParamDialog` does not render:
datetime -> plain text box (runtime: DateTimeField)
lookup + reference -> plain text box (runtime: LookupField record picker)
select, no options -> plain text box (runtime: SelectField, empty picker)
and the options-first branch inverted the same defect: a `text` param carrying
`options` previewed as a select the runtime never draws, because
`resolveFormWidgetType('text')` is `text` and `TextField` reads no options.
The mock now resolves each param through `paramToField`'s
`resolveParamWidgetType` / `paramDegradesWithoutTarget` -- the same adapter the
dialog renders through -- so the two panes read one member set. The preview owns
exactly two crossings and restates no table: the authoring `reference` key is
renamed to the resolved `referenceTo` the predicate reads (what
`resolveActionParams` does before the dialog sees a param), and a field-backed
param is never previewed as degraded because its target is inherited from the
bound field at runtime. A param with no declared `type` at all is field-backed
and unresolvable here, so it keeps inferring from its options.
The per-param editor gains an `options` control for `select`. Triage allowed a
visible pointer instead, but the pointer lost on measurement: `params` is in
`CURATED_FIELDS`, so the collapsed "More fields" SchemaForm hides the whole
array too, and the only surface that could author `options` was the raw JSON
source tab.
The population stays the eight `PARAM_TYPE_OPTS` offers -- no `FieldType` mock
gallery, and `long_text` / `integer` stay deleted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q✅ 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-support-ai
commented
Aug 26, 2026
ACCEPT — objectui#6538 ( ⭐⭐⭐ The counts held; the membership did not nestThe card said eight offered, five branched — and both numbers are right. But:
"Eight offered, five branched" invites the reading that the five are a subset of the eight. They are not, and nobody would have noticed from the counts alone. Two numbers agreeing is not the two sets agreeing — this is the same shape as the parity test in #6317 that compared method names while field declarations drifted. ⭐⭐⭐ A fourth disagreement, pointing the other wayResolving each spelling through the runtime's own adapter (
The fourth is the find. Every case the card named is the preview showing less than the runtime; this one is the preview showing more — a control the runtime will never render. Same defect, opposite direction, previously unnamed. A card that fixed only the named three would have left the mirror-image bug in the same function. The fix removes the second table rather than correcting it
That is the structural answer. The defect class here was two hand-maintained vocabularies drifting; correcting the second table would have restored agreement until the next Exactly two crossings are owned and named: authoring Fences held, verified
⭐⭐ The select/options half falsified the card's own bodyMy order said to pick between a real control and a visible pointer by measurement — does the author have anywhere to put options today? Measured:
So the only remaining surface was the raw JSON tab, and a "pointer" would have read "leave the designer." That settles it: a real Choosing the heavier option because the lighter one was measured impossible is the right order of operations. Choosing it because it seemed nicer would not have been. Reverse verification: the red is the disagreement, not a countPin written first, run against the untouched tree — That last one is the summary assertion done right — it names the surviving set rather than counting it. After the fix: Gates
Lint narrowing measured over the whole changed package (993 files, 0 errors), with the population from eslint's own config resolution and the count from its JSON reporter. CI: 29 checks, zero failed, on the head reported. Landing on green. Generated by Claude Code |
os-support-ai
commented
Aug 26, 2026
CI red — diagnosed and routed to the dev holding the branch. Recording it here so the state is on GitHub rather than only in a dispatch message. The failure is this PR'sJob
Everything else is green: 26 success, 3 skipped, this one failure. ⛔ The fix is convergence, not a raised ceilingThe obvious way to make this green is to edit the inventory from 1 to 2. That is gate weakening — raising a ratchet's ceiling — and it is a human-floor decision in this lane, not available to the dev or to me. Also excluded: skipping, disabling or quarantining the test, adding an exemption entry, or an empty commit to re-run. ⭐ The gate names its own remedy in the failure text: "converge it onto The dev has been asked to reproduce the red locally first, converge the new dual read onto The ACCEPT above stands on the substance. This is a mechanical debt-gate collision the rewrite tripped, not a problem with the approach.
Generated by Claude Code |
The column-identity ratchet (objectui#3104) counts ActionPreview.tsx at ONE `two-layer` dual read — "Action param name, same layering as resolveActionParams" — and the preceding commit added a second: a `label` derivation inside `renderFieldMock` that re-open-coded the param-name chain `DialogMock` had already computed one function up. Removed rather than re-triaged. `renderFieldMock` now receives the `fieldLabel` its caller derives and renders directly above the control, so the line is gone from the source instead of hidden from the matcher, and the mock's placeholders name the param exactly as its visible label does. NOT converged onto `columnIdentity()`, and the inventory says why in its own words: that reader is canonical-first because a column IS the object field it shows, whereas a param merely BINDS one, so borrowing it would invert the param-name precedence and rename every field-backed param that also names itself. The route taken is the one objectui#3174 took for `resolveActionParams` — every call site in the file through one named reader, so the count is the number of concepts and not the number of open-codings. The scanner is a line-level heuristic and comment-blind: a first attempt quoted the chain in prose and was itself counted. The TSDoc now says so, next to the place the next editor would write it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
os-support-ai
commented
Aug 26, 2026
Ratchet collision resolved — and my routing instruction was wrong. The dev was right not to follow it. My errorI relayed the gate's failure text as this file's remedy:
That message is generic to the ratchet, not specific to this file. Read the inventory entry the gate is checking against — 'app-shell/src/views/metadata-admin/previews/ActionPreview.tsx': {count: 1,verdict: 'two-layer',why: 'Action param name, same layering as resolveActionParams.',},The verdict is
So following my order literally would have shipped a rendering change on a card whose entire subject is that the preview and the runtime must agree — the exact regression my own acceptance criterion was written to catch. The ⭐ The lesson for this seat: a ratchet's failure text is a generic remedy; the inventory entry is the file-specific ruling. I read the error and not the gate. When a debt gate names a file, read that file's row before relaying what to do about it. What was done insteadThe route #3174 took for Concretely, Verified:
⭐⭐ A trap in the gate itself, worth recording
A debt scanner that counts a comment describing the pattern as an instance of the pattern will punish the person documenting why they did not use it. The warning now sits where the next editor would trip it. That is the right place for it — better than a card nobody reads before writing a docblock. The acceptance criterion I set was met, and met the honest wayI asked for proof that the convergence did not change what the preview draws. The card's six suites: Red reproduced locally first, byte-identical to CI job Substance unchanged from the first ACCEPT, which stands. Landing on green. 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
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6538
Branched from
mainatbf3a03c1d, which contains11f23e9a5(the #6329 / PR #6536 convergence). Both files were re-read on that tip before planning.The disagreement, re-derived on current
mainWhat
PARAM_TYPE_OPTSoffers — eight, unchanged by this PR:text·textarea·number·boolean·select·date·datetime·lookupWhat
renderFieldMockbranched on — five rawtypespellings:boolean·textarea·html·number·date, plus one branch keyed onoptionsrather than ontypeat all.The card's counts (eight offered / five branched) hold, but the membership of the five is not what the card's table implies. Two corrections, both measured:
htmlis one of the five branched spellings and the inspector does not offer it — so the two sets are not nested in either direction. Offered∖branched ={text, select, datetime, lookup}; branched∖offered ={html}.textandselectare in that difference because neither is type-branched:textis the untyped fallback, andselectis drawn only whenoptionsare present, whatever the declared type.What the runtime draws is not a hand-read table either —
ActionParamDialogrenders every param throughparamToField()→resolveParamWidgetType()→getLazyFieldWidget()(ADR-0059), so it is a pure function this PR calls. Resolved over the eight:textTextFieldtext+optionsTextField(options ignored)textareaTextAreaFieldnumberNumberFieldbooleanBooleanField(widget: checkbox)select+optionsSelectFieldselect, nooptionsSelectField(empty picker)dateDateFielddatetimeDateTimeFieldlookup+referenceLookupFieldrecord pickerlookup, noreferenceSo four disagreeing cases across three spellings, not two — the card named
datetimeandlookup;select-without-optionswas its second observation, andtext-carrying-optionsis the same defect pointing the other way and had not been named.The fix — one member set, answered once
renderFieldMockno longer switches on the raw authored spelling. It resolves each param throughparamToField'sresolveParamWidgetType/paramDegradesWithoutTarget— the same adapter the dialog renders through — and switches on the resulting widget key. It restates no membership table (the forkparamToField's own TSDoc records for objectui#5654), and owns exactly two crossings:reference→referenceTo.referenceis the authoring spelling (ActionParamSchema);referenceTois the resolved one the degradation predicate reads.resolveActionParams()performs that copy before the dialog sees a param; this preview reads the unresolved draft, so the rename happens here.param.reference ?? field.reference_to), and the designer has not resolved that field — claiming the degradation would be the same lie pointing the other way.A param with no declared
typeat all is field-backed and genuinely unresolvable here, so it keeps inferring from itsoptions— pinned, so the type-driven switch cannot quietly take that case with it.Resulting mocks:
datetime→ a date/time control ·lookup+reference→ a disabled record-picker combobox (the same shape the designer's other pickers use) · targetlesslookup→ the record-id text box the dialog degrades to, plus the reason ·select→ always a picker, with a note when no choices are authored yet ·text+options→ a text input, because that is whatTextFieldrenders.The population stays eight. No
FieldTypemock gallery;long_text/integerstay deleted.htmlrides along withtextareabecause that is what the tree already drew for it — dropping it while narrowing to the population would be a regression wearing a narrowing's clothes. Every other widget key still falls to the text box,master_detailincluded: it resolves to its own widget key, is outside the eight, and extending the mock vocabulary pastPARAM_TYPE_OPTSis deliberately not this card.ActionParamDialogis untouched — no spelling needed it. No fork to report.The
select/optionshalf — a control, decided by measurementTriage allowed either a real control or a visible pointer to where options are authored. The pointer lost:
field,name,label,type,placeholder,required,defaultFromRow— and none foroptions;paramsis listed inCURATED_FIELDS, so the collapsed "More fields"SchemaFormhides the whole array. The issue body's "reachable only through the collapsed More fields SchemaForm" is false — it is not reachable there at all;optionsis the raw JSON source tab (JsonSourceEditor).A pointer would therefore have had to read "leave the designer", i.e. the designer conceding it cannot author its own offered type. So: a real
ParamOptionsEditor, scoped toselect— the one offered spelling whose runtime widget readsoptions. Atextparam carryingoptionsis not an author mistake to be enabled here;TextFieldignores them, which is exactly what the preview now shows.Reverse verification
The pin was written first and run against the untouched tree (only
PARAM_TYPE_OPTS'exportadded, which fixes nothing). 7 failed | 12 passed, and the failures are the disagreement itself, not a branch count:plus both inspector cases (
getByRole('group', { name: 'Options' })finds nothing). After the fix: 19 passed.The pin reads the population from
PARAM_TYPE_OPTSby reference, so a ninth offered spelling fails it. Its per-case expectations are hand-declared from the runtime on purpose — deriving them from the same resolver the preview calls could never catch the preview calling that resolver wrongly — with a separate drift guard asserting the literal table still matches what the resolver says.Verification
Union re-run at
ac2a2023a, the final commit, all exit codes captured before any pipe:column-identity.ratchet(the check that was red — see Follow-up)Test Files 1 passed (1)·Tests 7 passed (7)@object-ui/app-shelltype-check(tsc --noEmitandtsc -p tsconfig.test.json)TYPECHECK_EXIT=0eslinton the three changed/added filesESLINT_EXIT=0, no outputTest Files 6 passed (6)·Tests 108 passed (108)check:control-bytes✅ check-control-bytes: OK (scanned 5409 tracked text file(s))check:vi-mock-specifiers✅ check-vi-mock-specifiers: OK (…457 carry a mock…)check:designer-field-key-paritydesigner-field-key-parity: OKcheck:action-forward-parity✅ action forward parity: 5 surfaces checked…lint:coverage✅ lint coverage: 46/46 packages linted, 0 with outstanding errorstype-check:coverage✅ test type-check coverage: 41/41 packages compile their testschangeset:check✅ All workspace packages are in the changeset fixed group.check:i18n-keys/check:i18n-drift0 en value(s) changedSuites:
ActionDesigner.paramVocabulary(new) ·ActionParam.one-authority·ActionPreview.locations·ActionDefaultInspector.celGate·paramToField·ActionParamDialog.The package
type-checkcompilestsconfig.test.jsonas well, so the new pin is type-checked rather than merely executed.Declared narrowing: the repo-wide
pnpm lint(turbo run lint, 46 packages) was not run;eslintwas run over the whole changed package first — 993 files,errors: 0, counted from--format json— and then over the three changed files after a comment-only follow-up edit. The narrowing is measurable rather than assumed: the population comes from eslint's own config resolution, the count from its JSON reporter, andeslint.config.jsdeclares noproject/projectService/parserOptions, so type-aware linting is off and this diff cannot move the verdict on any file it does not touch. Not run locally and left to CI:check:eager-closure(needs a console build to emiteager-closure.json) — the new import ispackages/app-shell/src/utils/paramToField.js, already imported byActionParamDialogin the same package, so it adds the module itself and no new package edge.Follow-up — the column-identity ratchet (
ac2a2023a)CI came back red on one check of 7079:
column-identity.ratchet.test.ts(objectui#3104) countsActionPreview.tsxat onetwo-layerdual read, and the rewrite in8c545bf1fadded a second — alabelderivation insiderenderFieldMockthat re-open-coded the param-name chainDialogMockhad already computed one function up. Reproduced locally before touching anything, byte-identical to CI:The inventory count is unchanged at 1 and the ratchet was not touched. The added read is gone from the source, not hidden from the matcher:
renderFieldMocknow takes thefieldLabelits caller already derived and renders directly above the control, so the mock's placeholders name the param exactly the way its visible label does.Not converged onto
columnIdentity(), and the inventory refuses that borrowing in its own words. This file's verdict istwo-layer, "Action param name, same layering as resolveActionParams" — and theresolveActionParamsentry states thatcolumnIdentity()is canonical-first because a column is the object field it shows, whereas a param merely binds one, so borrowing it "would invert the param-name precedence and rename every field-backed param that also names itself". The route taken is instead the one objectui#3174 took for that same file: every call site in the file goes through one named reader, so the count is the number of concepts rather than the number of open-codings.One trap worth recording, because the first attempt hit it: the scanner is a line-level heuristic and blind to comments, so a TSDoc line quoting the chain in prose is itself counted. The rewritten TSDoc says so, next to the place the next editor would write it.
The card's own suites were re-run to prove the convergence changed nothing about what the preview draws —
Tests 108 passed (108), including all 19 pin cases, which assert the rendered control kind per offered spelling.Scope
File face exactly as dispatched:
previews/ActionPreview.tsx,inspectors/ActionDefaultInspector.tsx, one new pin, one changeset.previews/index.tsandinspectors/index.tsuntouched;ActionParamDialoguntouched. Nothing in #6465 / #6332 / #6534 / #6541 or PRs #6543 / #6544 / #6546 was touched.PARAM_TYPE_OPTS' newexportcosts this panel fast refresh; thereact-refresh/only-export-componentsdirective accepts that by name, next to the reason.Generated by Claude Code
Generated by Claude Code