Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): diagnose non-literal elements in predicate in sets - #5265
Merged
os-support-ai merged 1 commit intoAug 18, 2026
Merged
Conversation
An `in` predicate's array branch JSON-parses the bracketed text and returns an empty array on any parse failure, with no diagnostic. A non-literal element (a path, a bare identifier, a trailing comma) collapses the WHOLE set to [], so membership silently reads false for every row -- even discarding good literal elements sitting next to the bad one. Diagnose only, mirroring objectui#4049's ruling: zero semantic change. The catch still returns [], the verdict is untouched; a dev-mode console.warn now names the predicate and, best-effort, the element that broke the parse. Fixes#4266
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 18, 2026 23:05
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-4266-in-predicate-array-diagnostic
branch
August 18, 2026 23:06
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
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#4266
What
predicate.ts'sin [...]array branch JSON-parses the bracketed text and, on any parse failure (a non-literal element — a path, a bare identifier, a trailing comma), thecatchreturned[]with nothing in the console.[].includes(anything)isfalse, so the predicate silently read FALSE FOR EVERY ROW — and because the parse is whole-set, not per-element, one bad element discarded every good literal sitting next to it too:data.type in ['text', data.a]collapsed exactly as hard asdata.type in [data.a]alone.Fix
Diagnose only, zero semantic change — the ruling this card asked me to measure against #4049's precedent, and matched it: the
catchstill returns[]; aninset that fails to parse is still, and remains, the empty set. A dev-modeconsole.warnnow names the predicate and, best-effort, the element that broke the parse (findUnparseableSetElement, which re-parses each top-level comma-separated element with the same quote-normalisingJSON.parsethe array branch already uses — for the warning text only, never to change the return value). Falls back to naming the whole set when no single element is individually at fault (e.g. a bare trailing comma). Warn-once keyed on(raw set text, predicate), same discipline as the two existing diagnostics in this file (#6936, #4049).⛔ Does not widen the declared subset — a path inside
in [...]still does not resolve; it is still reported as broken, just no longer silently.A louder alternative (throw so the whole predicate fails open
true, mirroring #6936) was considered and rejected — see the new header section inpredicate.tsfor the reasoning:falsehere is the documented behavior for an unparseable set, not a bug the way an unresolved path was, so flipping it to fail-open would be an unruled semantic change.Premise re-verification
Triage verified on
origin/main@b954120(a week old). Re-verified on currentmain@bc2922a8(post the two merges from #5227/PR5263 that touchedwidgets.tsx/loadState.tsin the same directory) — the array branch and its whole-setcatch { return []; }were still exactly as described. Also checked whether ROADMAP M9 (real CEL) has landed:@objectstack/formula'scel-engine.tsexists and is already used elsewhere inapp-shell(ExpressionProvider.tsx,celAuthoring.ts,clientValidation.ts, …), butSchemaForm.tsxis still the sole consumer ofpredicate.ts'sevaluatePredicate— M9 has not replaced this specific evaluator, so the card is live.Tests
predicate.test.ts§8 (new): non-literal element fires + names it (bare identifier, path-shaped, amid good literals, trailing-comma fallback), warn-once per (predicate, set) pair, dev-mode-only, and a§8.3zero-semantics table pinning verdicts identical to the pre-fix[]shape — the same proof-shape as #4049's §7.3. §8.4 pins literal-onlyinsets completely unaffected (same verdict, silent).Reverse-verification: restored the pre-fix two-line
catch { return []; }, predicted exactly the 6 new assertions that reference the warning would go red (the twotoHaveBeenCalledTimeswarn-count tests and the fourtoContain(...)on warning text) while the rest of the file (88 tests, including the new §8.3/§8.4 verdict-only pins) stays green. Observed: exactly those 6 red, 88 green — restored the fix afterward from the committed branch state and re-ran to confirm 94/94 green again.Gates run locally
pnpm --filter '@object-ui/app-shell^...' build(dependency closure) — cleanpnpm exec vitest run packages/app-shell/src/views/metadata-admin/predicate.test.ts— 94/94 passed (run from repo root per the package-level-vitest guard in this repo, objectui#3378)pnpm --filter @object-ui/app-shell type-check— cleanpnpm --filter @object-ui/app-shell exec eslint src/views/metadata-admin/predicate.ts src/views/metadata-admin/predicate.test.ts— 0 errors (1 pre-existingno-explicit-anywarning, unrelated line inresolveValue)node scripts/check-control-bytes.mjs+ explicit control-byte grep on both changed files — cleannode scripts/check-changeset-presence.mjs— 1 changeset present for the 1 released package touchedcheck:i18n-keys/check:i18n-drift): N/A — this diagnostic is a dev-modeconsole.warn, same as the two existing diagnostics in this file, neither of which goes throught()/i18nFinal commit:
bb75ca165.Generated by Claude Code