Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): compile a reference typed into ConditionBuilder's value box as a reference - #6384
Merged
os-support-ai merged 1 commit intoAug 25, 2026
Conversation
…ue box as a reference `fmtValue` quoted anything that was not a number / `true` / `false` / `null`, and the row builder's value box is free text — so "this field differs from its prior value", the idiom that defines a change-detection predicate, compiled to `previous == 'previous.status'`. Valid CEL, `previous` is a declared root, and a string literal's contents are deliberately not scanned for references by `flow-ref-check` or the server-side validator: the predicate parsed, registered, evaluated, and was always false with no author-time signal at any layer, at all five surfaces that mount this builder. A value matching a declared root prefix now emits as the reference it plainly is. The root set is this builder's own vocabulary — `record` / `user` / `org` are what its subject dropdown offers one control to the left, `previous` and `parent` are bound by `evalFieldPredicate` and by the server-side hook / validation evaluators, and `current_user` is the ADR-0068 spelling of the identity `user` also names. Roots this builder never offers are excluded on purpose: `data` IS bound and `data.csv` is a plausible literal, so capturing it would swap one silently-false predicate for another rather than for a loud one. The test is "a dotted path under a declared root", not "contains a dot": `1.2.3` and paths under unbound roots stay literal text, and the literal / number controls are unchanged. Nothing already stored is rewritten — a persisted `previous == 'previous.status'` no longer round-trips, so the builder's existing safety rule hands it to the raw CEL editor instead of reinterpreting it. Pinned by mounting the real component and reading what `onCommit` emits, because the commit path above `fmtValue` is part of the defect. 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
|
os-support-ai
marked this pull request as ready for review
August 25, 2026 19:13
12 tasks
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-6293-conditionbuilder-reference-quoting
branch
August 25, 2026 19:24
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#6293
fmtValuequoted anything that was not a number /true/false/null, and the row builder's value box is free text — so "this field differs from its prior value", the idiom that defines a change-detection predicate, compiled toprevious == 'previous.status'. That parses, registers and evaluates, and is always false. Nothing objects at any layer:previousis a declared root, and a string literal's contents are deliberately not scanned for references byflow-ref-checkor by the server-side validator.The ruling: compile the reference
Of the two ruled arms — compile it, or refuse loudly — this takes compile. The measurement against the five consumers is below; no consumer breaks, and compiling is the better outcome because it makes the reference checkable: it is now an identifier the existing reference checkers can see, where a string literal's contents were invisible to them.
A value matching a declared root prefix now emits as the reference. The root set is
record,previous,parent,user,current_user,org— this builder's own vocabulary:record/user/org— exactly what the subject dropdown offers one control to the left (record.<field>from the field catalog, plusCONTEXT_SUBJECTS).previous/parent— bound byevalFieldPredicate(packages/core/src/evaluator/fieldRules.ts) and by the server-side hook / validation evaluators.current_user— the ADR-0068 spelling of the same identity objectusernames; which alias the author typed must not decide whether it reads as a reference.Roots this builder never offers (
data,os,app,features,input,vars,page) are excluded on purpose:data.csvis a plausible literal anddatais bound at the shell surfaces, so capturing it would swap one silently-false predicate for another rather than for a loud one. Declaring which roots a mounting surface actually binds is caller-supplied vocabulary and belongs to objectui#6296 — see the note at the bottom.The test is "a dotted path under a declared root", not "contains a dot":
1.2.3, filenames, and paths under unbound roots all stay literal text.The five consumers — measured, one line each
views/metadata-admin/inspectors/HookDefaultInspector.tsx(hookcondition)previous— the showcase app shipsprevious.done != true && record.done == true— soprevious.statusnow compiles to the comparison the author meant, on the surface the defect was measured on.views/metadata-admin/widgets.tsx(ConditionWidget→ fieldvisibleWhen/readonlyWhen/requiredWhen)evalFieldPredicatebindsrecord,previous, and thescopeextra (parent) — every emitted root this surface can see. A root it cannot see resolves to a not-ok verdict that is reported (warnPredicateFailure+ theonFaultpassback), never a silentfalse.views/metadata-admin/inspectors/PageBlockInspector.tsx(blockvisibleWhen)record(viaRecordContextProvider, perpage.zod.ts's "Bindsrecord,current_user,page.<var>") plus the shell's predicate scope; arecord.*value is the same identifier the subject dropdown already emits.previousis not bound here, but an unbound root is a loud not-ok verdict, not the silent always-false it replaces.views/metadata-admin/inspectors/ActionDefaultInspector.tsx(actionvisible+disabled, two mount sites)ExpressionProviderbindscurrent_user/user/ctx.user/os.user/app/data/features— plusrecordfrom the record context. Identical reading to #3.views/studio-design/ObjectValidationsPanel.tsx(validation rulecondition)record.x != previous.x) is the defining validation-rule idiom; the panel stores the emitted CEL through the samewriteExpressionSourceenvelope as the other four and post-processes nothing.Two facts hold across all five, and both are pinned by tests:
record.status != previous.statusre-opens in ROW mode, so the builder does not flip to the raw editor the moment the author reopens what they just authored. (It could not round-trip before this change.)previous == 'previous.status'no longer round-trips byte-for-byte, so the component's existing safety rule hands it to the raw CEL editor rather than reinterpreting it — the author sees both readings and decides. No stored predicate's meaning changes.No consumer-side tolerance is added anywhere: the repair is at the authoring surface, where the ambiguity is.
Ghost-assertion guard — both readings
Tests mount the real component and read what
onCommitemits; the commit path abovefmtValueis part of the defect, so a unit test of the formatter alone would not cover it.Before — the new test file alone, on unmodified
origin/main(2e11c8c5b), source untouched:The
Receivedline on the first assertion is the card's measurement, reproduced.After — same file, with the repair:
The other four assertions are the probe and the controls, and they pass on both readings by design — that is what makes them controls, and what makes the reading attributable to the input rather than to a constant: the row-mode probe (
previous == nullopens with a live value box), the literal control (done→previous == 'done'), the number control (42→previous == 42, unquoted), and an undeclared-root control (foo.barand1.2.3stay quoted).Gates run locally
Union re-run on the final commit
b43eca6f3:pnpm exec vitest runover all five consumers' suites (metadata-admin/inspectors/,ResourceEditPage.celGate,CelPredicateField.labelBinding,EmbeddedItemEditor.preview,studio-design/ObjectValidationsPanel.celGate) — 62 files, 662 passed, 1 skipped, 0 failed.pnpm --filter @object-ui/app-shell run type-check(tsc --noEmit && tsc -p tsconfig.test.json) — clean. Confirmed the program actually contains both edited files viatsc -p tsconfig.test.json --listFiles(1 hit each), so "typecheck clean" is a statement about this diff.eslint .over the whole@object-ui/app-shellpackage — 966 files, 0 errors. The one warning onConditionBuilder.tsxis the pre-existingreact-hooks/exhaustive-depson the// first mount onlyuseMemo, present unchanged onorigin/main(line 151 there).check-changeset-presence,check-changeset-no-major,check-control-bytes— all green.packages/andapps/for any fixture pinning a quoted reference-shaped literal on a comparison — zero hits outside the new test file.Scope
.changeset/6293-conditionbuilder-reference-value.md—patch,@object-ui/app-shell.flow-ref-check, the server-side expression validator andpackages/specare untouched, and no predicate's meaning changes anywhere else.objectui#6296 (the first-class "another field" value control) is not built here, not even partially. The repair does make it easier:
REFERENCE_ROOTSis now the single place this builder states which prefixes read as references, which is exactly the seam a caller-supplied subject vocabulary would replace — a surface that declares its roots would feed that constant instead of the builder assuming them. objectui#6296 remains open and is out of scope for this PR.Generated by Claude Code