Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-admin): diagnose a path on the right side of ==/!= in a visibility predicate (#4049) - #4264
Merged
Conversation
…visibility predicate (#4049) The evaluator resolves paths only on the LEFT of ==/!=. The right side goes to parseLiteral, whose tail returns anything it does not recognise as a literal verbatim, so `data.a == data.b` compares against the string "data.b" and is false however equal the two sides are — silently. objectstack#6936's warning hangs on resolveValue, which the right side never enters. Option B per the ruling on #4049: a dev-mode warning at that tail when the returned text is path-shaped, ZERO semantic change. Verdicts pinned identical before and after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 09:57
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#4049
The predicate evaluator in
packages/app-shell/src/views/metadata-admin/predicate.tsresolves paths only on the left of==/!=. The right-hand side goes throughparseLiteral, whose tailreturn shands back anything it does not recognise as a literal verbatim — sodata.a == data.bcompares the value ofdata.aagainst the seven-character string"data.b". objectstack#6936's unresolved-path warning cannot see this: it hangs onresolveValue, which the right side never enters (measured: 0 warnings across the whole truth table).Per the binding ruling on the card: option B only — a dev-mode diagnostic, zero semantic change. Option A (resolving the right side) was rejected because it flips
data.type == text— the unquoted-string spelling that works today by accident — into a fail-opentrue.Truth table: verdicts are identical before and after
Measured on this branch's tip, first with a temporary probe on the unmodified evaluator, then pinned mechanically in
predicate.test.tssection 7.3.data.a == data.b{a:'x', b:'x'}falsefalsedata.a != data.b{a:'x', b:'x'}truetruedata.a == data.b{a:'x', b:'y'}falsefalsedata.a == 'x'{a:'x', b:'y'}truetruedata.type == text{type:'text'}truetrueThe only change is that the console stops being silent.
data.a == data.bis still false when both sides hold the same value.Trigger grammar
The diagnostic fires when the tail returns text matching the dot-separated identifier chain the left side itself accepts:
That covers both halves of the ruling's description — a dotted path (
data.b) and a bare identifier (text).One deliberate, stated narrowing of the ruling's parenthetical "contains a
.": a dotted non-identifier such as1.2.3reaches the same tail (viaresolveValue's digit-leading literal shortcut) and is likewise compared as text, but it is a malformed number, not a path — announcing it as a path would be a false statement about the author's code. It is pinned silent, with the reasoning in the code comment. Flagging it here rather than deciding it silently; happy to widen if the seat prefers the literal reading.Controls pinned silent: quoted strings (both quote styles), numbers, negatives, decimals,
true,false,null, arrays, and a quoted string that itself contains dots.Operator coverage (measured, not assumed)
The diagnostic sits at the tail
return s, so every route to that line carries it by construction. The three call sites ofparseLiteral:==/!=— reaches the tail. This is the card's case and the only route that fires in practice.resolveValue's literal shortcut — used by the left side of==/!=, the left side ofin, and a bare truthy check. It reaches the tail only for digit-leading operands (1.2.3,12abc), because quoted /true/false/nullreturn from their own branches. Covered by the diagnostic, but never identifier-shaped, so it does not fire.in— provably cannot reach the tail. The branch regex requires(\[.*\]), soparseLiteralalways takes the array branch and returns an array.Route 3 has its own silent shape, which is out of scope here and filed separately rather than folded in: a non-JSON element such as
data.type in [data.a]makesJSON.parsethrow, the branch catches and returns[], and membership is then false for every row with no diagnostic. Measuredfalse, silent. Different code path (the array branch'scatch), not the tail this card rules on.Producer-side half (for the seat's D relay)
The publish-time-validation companion card of objectstack#6936 is objectstack#7010 — "Publish-time validation of predicate path references". Note for the relay: it is already closed as completed, landed by objectstack PR #7214 (
feat(lint): publish-time resolution of metadata-form predicate paths). Its delivered scope is predicate paths that do not resolve against the target schema; "the right-hand side is a path at all" is a different refusal and does not appear to be covered by it, so the D half likely needs a fresh card against that landed check rather than a comment on a closed one. Flagging the state rather than acting on it — the relay is the seat's call.Retirement pointer (C)
The diagnostic's comment and the file header both record that this file is the interim stand-in for
@objectstack/formula(ROADMAP M9): when CEL lands, the diagnostic retires with the file. It is deliberately not grown into a second evaluator.Verification
expected "warn" to be called 1 times, but got 0 times); the controls and semantics pins passed from the start, since they describe unchanged behavior.npx vitest run packages/app-shell/src/views/metadata-admin/predicate.test.ts— 81 passed (81).npx vitest run packages/app-shell/— 312 files, 2944 passed, 1 skipped.pnpm --filter @object-ui/app-shell type-check— exit 0.npx eslinton both changed files — 0 errors (1 pre-existingno-explicit-anywarning atpredicate.ts:308, untouched).node scripts/check-control-bytes.mjs— OK, 3923 files; plus a targeted control-byte self-scan of both changed files, clean.git checkout HEAD~1 -- predicate.ts, tests kept): predicted and confirmed — the 5 diagnostic pins go red, all 76 others including every semantics pin and control stay green. That green-through-the-revert is the mechanical zero-semantics proof. Restored afterwards.No i18n: dev-mode diagnostics are not user copy, matching objectstack#6936's landed
warnUnresolvedPathidiom in the same file (same channel, sameNODE_ENVgating, same warn-once memo keyed on the pair, sameresetPredicateWarningstest hook).Generated by Claude Code