Uh oh!
There was an error while loading. Please reload this page.
fix(designer): retire the formula-expression control instead of renaming its key - #6312
Merged
Merged
Conversation
…ing its key The Field Designer's formula textarea wrote `formula`, a key `FieldSchema` refuses BY NAME, so `PUT /api/v1/meta/object/:name` returned a hard 422 `INVALID_METADATA` that blocked every later save of the object. The spec spells the concept `expression`, and the rename was deliberately NOT taken. `FieldSchema` judges the key and never the expression LANGUAGE: measured on @objectstack/spec 17.2.0 it accepts `expression: 'price * quantity'` and even `expression: '!!!not cel at all!!!'`. Spec `expression` is CEL rooted at `record`, while this control's own placeholder taught `price * quantity` — bare field refs that evaluate to null silently under the scope formulas bind. A rename would have converted a loud 422 into a formula that saves clean and computes nothing. Making refusals loud in the control needs CEL lint and returnType inference, i.e. CelPredicateField — which lives in @object-ui/app-shell, and app-shell depends on @object-ui/plugin-designer, so it cannot be imported back without a cycle. Expressions are authored in metadata-admin's ObjectFieldInspector, which lints against the real @objectstack/formula engine. The field TYPE `formula` stays: it is a valid spec FieldType and only the expression key was refused. `formula` joins RETIRED_FIELD_KEYS so an object already carrying it is stripped clean on its next save rather than staying blocked forever — with the control gone there is no other way left to clear it. A stored `expression` is untouched. Also drops the key from object-fields-bridge.ts, a third emit site the parity gate does not cover, and clears the ledger entry, which ratchets both ways. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
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
|
This was referenced Aug 25, 2026
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-support-ai
marked this pull request as ready for review
August 25, 2026 12:39
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#6043
The Field Designer's formula textarea wrote
formula, a keyFieldSchemarefuses by name, soPUT /api/v1/meta/object/:namereturned a hard 422INVALID_METADATA— and because the key was then stored, it blocked every later save of that object, not just the one that introduced it.Surfaced by the key-level parity gate built for #5761. Verified on⚠️
origin/main@7ccb53a(mainmoved from theba306e3the card was triaged on; every premise was re-derived). All measurements are against the installed@objectstack/spec17.2.0.The premise holds
Why the obvious rename was refused — the measurement that decides this card
The spec ships a rename hint, and taking it would have been a two-line change. It is wrong, and this is the evidence:
FieldSchemajudges the key name, never the expression language. Only the empty string is refused, so a rename buys a green parse for arbitrary garbage.That matters here specifically because spec
expressionis CEL rooted atrecord(record.amount * 0.1), while this control's own placeholder taughte.g. price * quantity— bare field refs. This repo has already measured what those do;celAuthoring.tsstates it about the exact scope formulas bind:So a bare rename converts a loud, immediate 422 into a formula that saves clean and then quietly computes nothing — strictly worse than the bug.
Why the control was removed rather than taught CEL
Making refusals loud inside the control needs CEL lint, autocomplete and
returnTypeinference. That isCelPredicateField, and it is not reachable from here:@object-ui/app-shelldepends on@object-ui/plugin-designer(declared in itspackage.json), so importing the editor back is a dependency cycle.@object-ui/plugin-designerdoes not depend on@objectstack/formulaat all — it has no CEL engine.Building a second formula-authoring surface inside plugin-designer is a feature, not this fix. The control is removed instead, and the capability is not lost: metadata-admin's
ObjectFieldInspectorauthorsexpressionthroughCelPredicateFieldwith live lint and stampsreturnTypefrom the inferred CEL type, labelledFormula (CEL). Its own comment records that it "replaces the formula textarea that previously edited aformulakey the runtime never read" — this repo has retired a formula textarea once already, for this reason.⭐ Nothing that worked is being taken away. Every save carrying
formulareturned 422, so no author can have been successfully using this control — it has only ever failed.returnType: omitted, deliberately. It is derivable only by inferring the CEL result type, which needs the engine this package does not have; and with no expression control there is nothing to infer from. The surface that can infer it already stamps it.The field TYPE
formulais kept. Measured:FieldSchema.safeParse({ type:'formula', label:'Tax' })succeeds — it is a valid specFieldType. Only the expression key was ever refused, so removing the type would have broken formula fields outright. Authors still declare a field computed; they author its expression where it gets checked.What changed
types/src/designer.tsDesignerFieldDefinition.formularemoved + tombstoneplugin-designer/src/FieldDesigner.tsxplugin-designer/src/MetadataFieldsPage.tsxServerFieldSchemakey, both converters,formulaadded toRETIRED_FIELD_KEYSapp-shell/src/services/MetadataService.tsFieldMetadataPayloadkey +toFieldPayloadapp-shell/…/previews/object-fields-bridge.tsscripts/check-designer-field-key-parity.mjsThe tombstone is half the fix. With the control gone, an author has no way left to clear a stored
formula, so withoutRETIRED_FIELD_KEYSan already-blocked object would stay blocked forever —carryOverspreads the previous server def verbatim. It is dropped, not migrated toexpression, for the same reason the rename was refused: laundering a non-CEL string into a valid key name is the silent failure this card exists to prevent. A storedexpressionis not stripped — it is a realFieldSchemakey and rides through untouched.⭐ A third emit site, named by neither the card nor the gate.⚠️ Correction to my claim comment: I called it a live third write site — measured, it is dead code, zero importers repo-wide, so this edit is behaviourally inert. Filed as #6309, together with a lossy round-trip in its
object-fields-bridge.tsalso read and wroteformula;FrameworkFieldDefis not one of the parity gate'sPAYLOAD_SHAPES, so the gate was green over it by construction. It is in this PR by necessity (it consumesDesignerFieldDefinition).serializeDesignerFieldthat contradicts its own header.Verification
All commands from the repo root per AGENTS.md;
pnpm exec vitest run <paths>, never--filterorcd packages/x.Tests — green at
9798788b7, the final commit (22 files / 178 tests, the affected-set regression run; the 3 new files contribute 21):Assertions are on the bytes actually PUT (
JSON.parseof the captured request body through a realMetadataClient/ObjectStackAdapterover a fetch double), not on the argument handed to the client — a property whose value isundefinedis a key zod's strict object COUNTS butJSON.stringifyDROPS.Ablation — each repair point reverted alone, mutation proven on disk in both directions, restored under
trap … EXIT INT TERMwith absolute paths,git diff HEADempty after each leg. No rebuild leg is needed and that is measured, not assumed: the root vitest config aliases every@object-ui/*topackages/*/src, so these tests import the mutated source directly — nodist/in the path.'formula'out ofRETIRED_FIELD_KEYSformula: designed.formulaback intofromDesignerFieldformulasmuggled onto a designer fieldformula: field.formulaback intotoFieldPayloadformulasmuggled onto the fieldformulafield in the create drawer + …edit drawergrep -cFwith a multi-line pattern does not search for that block,-Ftreats each line as a separate alternative, so" };\n}"matched every closing brace and the "removed text" count came back 59 instead of 0. The guard refusing to read a test result from an unverified mutation is the behaviour working. The underlying error was mis-classification: A2/A3 are pure insertions like A4, so a "removed text" direction cannot exist for them; their honest proof is before-count 0 / after-count 1 on a single-line marker.Trap species addressed by name: a degenerate control was caught live — the grid mock fills rows from a promise, so the first draft queried row buttons before mount. Had that file only asserted absences, the empty tree would have made every one of them pass. Every absence in the control test is now paired with a positive control (
drawer-field-referenceTo, the sibling control in the same section through the same testid channel). Ghost assertions are labelled where they exist: theexpression/returnTyperound-trip row and the lookup round-trip row also pass on a revert and say so in comments — they are must-not-change pins, not two-world assertions.Gates — derived by enumerating each CI job's own step list, not from memory. Run locally, each reading its own verdict line (exit codes captured before any pipe):
check:designer-field-key-parity✅ (ledger entry gone, gate still OK) ·check:i18n-keys✅ ·check:control-bytes✅ ·check:vi-mock-specifiers✅ ·check:spec-symbols✅ ·check:self-import✅ ·check:phantom-deps✅ ·check-changeset-presence✅ ·check-changeset-no-major✅ ·check-changeset-fixed✅ ·type-check(types + plugin-designer + app-shell) ✅Type-check genuinely covers the new tests — each package's
type-checkrunstsc -p tsconfig.test.json, and--listFilesconfirms all three new files are in the set (not excluded).Lint, declared narrowing.
pnpm linthere isturbo run lint(per-packageeslint .), so linting the three packages I touched is the complete lint for every package I touched — run with--forceto defeat turbo cache: 4 tasks successful, 0 errors (2678 warnings = the repo's pre-existing baseline;check-lint-coverage.mjsreports 46/46 packages, 0 outstanding errors). Per-file--format jsonover my 8 changed files: 0 errors, 13 warnings, all 13 on lines I did not touch. Invariance justifying the narrowing:eslint.config.jssets noparserOptions.project/projectService, so linting is not type-aware and this diff cannot move the verdict on any untouched file.Changeset:
minoron all three packages, stating in words that this is a behaviour change on an authoring surface (a control is removed). Nevermajor, per the version-alignment rule.Known leftover, declared
appDesigner.fieldDesigner.formulais now a dead key in ten locale packs plus the designer defaults map. Left in place deliberately:check:i18n-dead-keysis report-only and wired into no workflow, and removing it is an 11-file translation-hygiene diff coupled to the defaults-map mirror gate. Filed as #6310 rather than folded in here.Out-of-scope findings filed
metadata-admin/previews/object-fields-bridge.tshas zero importers, and itsserializeDesignerFieldwould destroy every key the designer does not model #6309 —object-fields-bridge.tshas zero importers, and itsserializeDesignerFieldwould destroy every key the designer does not model (contradicting its own header)appDesigner.fieldDesigner.formulais now a dead key — 10 locale packs plus the designer defaults map, zero call sites #6310 — the dead i18n key above⛔ Left as draft: the PM lands it.
Generated by Claude Code