Uh oh!
There was an error while loading. Please reload this page.
fix(react): a faulting node-gate predicate is loud in production, once per predicate source - #6446
Merged
Merged
Conversation
… per predicate source
A `visibleWhen` (and `visible` / `visibleOn` / `visibility` / `hidden` /
`hiddenOn`) predicate that cannot be evaluated resolves to the same answer as
one that said yes, so a gate that stops biting looks exactly like a gate the
author got right. The diagnostic that names it sat behind a `__DEV__`
short-circuit, because the only fault-detection channel was `throwOnError`,
which the CEL branch implements by evaluating twice.
Measured on the built evaluator, the silence was dialect-dependent: a bare
string printed nothing, a `{ dialect: 'cel' }` envelope printed one generic
deduped line, and a `${…}` template printed one generic line PER EVALUATION.
The dialect a live gate was measured breaking on was the silent one.
`EvaluationOptions.onFault` reports the fault the evaluator has already
detected — every fault site is already inside a `catch`, or already holds the
canonical engine's reason — so nothing is evaluated twice, and supplying it
transfers reporting to the caller so one fault stays one line. `SchemaRenderer`
passes it in production and reports through the same reporter, message, dedupe
`Set` and key the dev branch uses; `page:tabs` item predicates join the same
rate limit.
Observability only: no verdict moves, on any path or dialect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q…operty
`SchemaRenderer.visibleWhenRecordBinding` asserted that a production build
printed none of this diagnostic. That is precisely the property the maintainer's
2026-08-25 ruling retired ("A is rejected — the silence is no longer an accepted
property"), so the pin is rewritten rather than deleted: the two faulting
predicates it mounts now assert one line each, and every verdict it was really
guarding is unchanged and still asserted.
Also corrects two neighbouring comments that described the #5687 adapter-only
leg's dev-only gate as shared by both legs. It is not shared any more, and the
asymmetry is deliberate: a fault is a predicate that could not be evaluated,
while that leg reports a predicate that evaluated perfectly against the wrong
object, on a lexical scan with a stated false-positive residue.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4qContributor
✅ 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 26, 2026 01:00
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#6038
Maintainer ruling, 2026-08-25, in-session batch adjudication (verbatim: 「就全部接受，然后继续下一批」) ⇒ Option B: production bundles emit a rate-limited, deduplicated warning (one per distinct failing predicate) when a
visibleWhen/ node-gate predicate faults, replacing the current__DEV__-only silence. Observability only — the fail-open semantics are not this card's to change.The premise, verified — and it holds for one dialect of three
The card says a faulting node-gate predicate prints nothing in production. Measured on the built evaluator (
packages/core/dist/evaluator/ExpressionEvaluator.js) at the branch point, that is true of one dialect, and the other two fail in opposite directions:{ dialect: 'cel' }envelopeevalFieldPredicate)${…}templateSo the silent dialect is the bare string — exactly the one objectstack#11254 measured a live gate breaking on — while the template dialect was already the console flood the ruling's rate-limit clause exists to prevent. The card's premise stands; this table is what it looks like once measured, and it is why several tests here pin the total
console.warncount rather than just the count of matching lines.The design constraint, and the way out of it
throwOnErrorwas the only fault-detection channel a fail-soft caller had, and on the CEL branchevaluateCelConditionimplements it by evaluating twice (once with each fallback). Spec-parsed metadata normalisesvisibleWheninto a{ dialect: 'cel' }envelope, so that is the common production branch — paying for the probe would double the engine calls for every predicate of every node of every render. That trade is the whole reason the diagnostic shipped behind__DEV__.EvaluationOptions.onFault(new,@object-ui/core) removes the trade: every fault site is already inside acatch, or already holds the canonical engine's failure reason, so the fault is reported at the same number of engine calls. It mirrors one layer up the seamFieldPredicateDiagnosticalready documents (warn: falseplus a reason passback), and on the CEL branch it is forwarded to that seam rather than reimplemented. Supplying it transfers reporting to the caller, so one fault stays one line — the caller's, which can name the node.Pinned rather than asserted: with a counting getter on the record, the CEL branch performs the same number of reads with the passback as a plain fail-soft call, and the
throwOnErrorprobe performs strictly more.SchemaRenderer's production branch passes it and reports through the same reporter the dev branch uses — same message, same severity, same dedupeSet, same key. Dev and production now print the identical line for the identical fault (pinned by string equality across the two module graphs). The__DEV__gate no longer decides whether a fault is reported, only how it is detected.Census — every predicate-fault catch site, and what happened to it
Censused by shape (a predicate evaluation caught and swallowed), not by symbol.
SchemaRenderer.evaluateVisibilityPredicate—visibleWhen/visible/visibleOn/visibility/hidden/hiddenOn, plus #5756's pre-hoistpropertiescallPageTabsRenderer.isItemVisible(page:tabsitemvisibleWhen,@object-ui/components)ExpressionProvider.evaluateVisibility(nav/area itemvisible,@object-ui/app-shell)ExpressionEvaluator.evaluate's two generic warns (props / content /properties)onFaultis passed)SchemaRenderer'sdisabled/disabledOngateuseConditionfail-closed legevalFieldPredicate)Why
page:tabsis fixed here and the other three are not. It is the same canonical key (visibleWhen) with the same declared contract — its own comment says "the same semantics SchemaRenderer applies to component-levelvisibleWhen" — and it was the worse of the two: the node gate at least reported in development, while a faulting item predicate was silent in both builds on a gate whose false verdict removes an entire tab, header and panel. Covering it is five lines and one import that already existed. The other three are a different key, a different tier, or a non-gate path; each is stated in its own issue rather than quietly widened into this PR's file surface.Declared file-surface amendment (the claim comment named
packages/reactonly): this PR also touchespackages/core/src/evaluator/ExpressionEvaluator.tsandpackages/components/src/renderers/layout/containers.tsx.Rate limit — both halves, because either alone is untestable
Deduped per (node type, gate key, predicate source) — never per render, never per node instance. Proven, not asserted:
console.warn;The last one is not optional: "a warning was emitted" is green on an implementation that emitted fifty, and "exactly one was emitted" is green on one that suppresses everything after the first line ever printed.
Controls
Every assertion reads a prefix-filtered view of captured
console.warnoutput, plus, in both new test files:toHaveLength(0)is equally green on a broken capture);[object-ui] …, a React key warning) does not satisfy the pin.And the silences that make the noise mean something: a healthy predicate on either verdict, a node with no gate, and #5687's adapter-only
data.*report — which stays development-only under its own 2026-08-22 ruling and is pinned silent in production, so "production is louder" cannot drift into "production reports everything dev does".Reverse verification — direction predicted before each run, restored by hash
Two isolated legs, each restoring one file to
origin/mainand running its own pins. No rebuild leg: vitest resolves every workspace specifier to<pkg>/srcvia the rootvitest.config.mtsalias table, sodist/takes no part in any reading. Each leg verified the mutation landed (hash-object== theorigin/mainblob, marker count → 0) before measuring, and restored withgit checkout HEAD -- <abs path>, verified by hash equality with theHEADblob and an emptygit diff HEAD.packages/react/src/SchemaRenderer.tsx→origin/mainpackages/components/.../containers.tsx→origin/mainBoth readings on the acceptance criterion, quoted: after —
reports(warn)has length 1; before —AssertionError: expected [] to have a length of 1 but got +0.The asymmetry is the card restated: every verdict assertion stayed green in both legs. The change moves the silence, not the answer.
Bundle cost — measured A/B, not argued
The reporter module was already in every production bundle: it is imported at
SchemaRenderer's module top level and__DEV__is a runtime IIFE readingglobalThis.process?.env?.NODE_ENV(not theprocess.env.NODE_ENVmember expression bundlers constant-fold), so it was never tree-shaken. Only the wiring is new.Measured by building
apps/consoletwice with identical tooling — once in a detached compare worktree pinned to this branch's base commit9602dc820(not movingorigin/main), once here — then running the repo's ownBundle Analysisgate on each:9602dc820framework(SchemaRenderer + evaluator)ui-components(containers.tsx)vendor-objectstack+107 bytes gzipped, across the two chunks that carry the change. Every per-chunk ceiling unmoved at the gate's 0.1 KB resolution; the checker exits ✅ on both trees.
An existing pin was rewritten, deliberately
SchemaRenderer.visibleWhenRecordBinding.test.tsxasserted that a production build printed none of this diagnostic — the exact property the ruling retires ("A is rejected — the silence is no longer an accepted property"). It now asserts two lines, one per distinct faulting source, with every verdict it was really guarding unchanged and still asserted. Two neighbouring comments that described #5687's dev-only gate as shared by both legs are corrected: they are no longer shared, and that asymmetry is deliberate — a fault is a predicate that could not be evaluated, while that leg reports one that evaluated perfectly against the wrong object, on a lexical scan with a stated false-positive residue.Verification
Union run at
3893507fc, after the final commit.pnpm exec vitest run packages/core/ packages/react/→ 158 files, 2866 tests, all passedpnpm exec vitest run packages/components/→ 194 files, 1754 tests, all passed3893507fc)pnpm exec turbo run type-check --filter=@object-ui/core --filter=@object-ui/react --filter=@object-ui/components→ 11 successful, 11 total, each package echoingtsc --noEmit && tsc -p tsconfig.test.json. All three new test files confirmed inside the typecheck file set via--listFiles(1 hit each), so "typecheck is clean" actually covers them.pnpm lint(full repo, not narrowed) → 47/47 tasks successful, 0 cached, 0 errors repo-wide (pre-existingno-explicit-anywarnings only)pnpm check:control-bytes→ ✅ 5317 tracked text files; plus a directgrep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over the diff — cleanpnpm check:readme-exports→ ✅ 378/378 self-imports real, 0 fabricated ·pnpm check:doc-snippets→ ✅ 267/267 blocks ·check:doc-fences,check:phantom-deps,check:self-import,check:esm-specifiers→ ✅node scripts/check-changeset-presence.mjs→ ✅ 5 source files of 3 released packages, 1 changesetBoth
check:readme-exportsandcheck:doc-snippetsfirst exited non-zero withrun the build first— a prerequisite, not a verdict. Re-run afterturbo run build --filter='./packages/*'(39/39 successful); the readings above are the post-build ones.Out of scope, by the ruling
/forms/:namepublishescurrent_userbut notfeatures, so afeatures.*predicate still resolves differently there than inside an app #6262's/forms/:namescope wiring — explicitly excluded; it lands in its own PR./forms/:namepublishescurrent_userbut notfeatures, so afeatures.*predicate still resolves differently there than inside an app #6262 is not addressed here and remains open.sendDefaultPii: trueis committed inapps/console/.env.productionand baked into every production build — theVITE_SENTRY_ENABLEDswitch is build-time and absent from the inlined env, so a shipped bundle cannot be silenced #5522) and D (host switch, new public config surface) — rejected.⛔ Draft on purpose: the PM lands this. Not marked ready, not self-merged, auto-merge not enabled.
Generated by Claude Code
Generated by Claude Code