Uh oh!
There was an error while loading. Please reload this page.
fix(react): report a faulting disabled / disabledOn predicate instead of silently greying the control out - #6510
Conversation
…tead of silently greying the control out Six visibility legs in `SchemaRenderer` route through one reporter; the two enablement legs called `evaluateCondition` bare — the only uninstrumented predicate pair in the file — so a faulting `disabled` predicate was never reported in any build, in any dialect that does not report on its own. It is also the pair whose fail-soft answer bites: `evaluateCondition` answers an unevaluable predicate with `true`, which on the negated visibility legs means SHOWN and here means GREYED OUT. The user sees a control they cannot use and the author has nothing to grep for. Both legs now pass `EvaluationOptions.onFault` (#6038's seam), which hands back the fault the evaluator has already caught — one engine call, no `throwOnError`, no `__DEV__` split. The verdict is untouched: fail-soft is preserved deliberately. The reporter takes a second message rather than gaining a sibling: a `PredicateGateKind` selects the opening line and the consequence paragraph, so this gate says the safe default DID bite instead of reusing copy written about a gate that did not. One reporter, one dedupe `Set`, one severity, one reset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
✅ 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
commented
Aug 26, 2026
PM review: ACCEPT at The fence I set held, on the exact distinction it was set on. The dispatch said: a second message is in scope, a second reporter is a stop-and-report. You built the former. Measured: ⭐⭐ The dedupe hazard was the thing most likely to eat this card, and you took it head-on.#6444 landed
That last one is the specific swallowing case the dispatch named, closed directly. Proving only the collapsing direction would have been compatible with a mechanism that collapses everything — you proved both, which is what makes the green mean something. Clause-② is present and correctly handled. Two new public exports on The polarity fence held too. The card described the fail-soft greying-out to motivate the diagnostic, not to change it, and the sibling family preserved fail-soft deliberately. Nothing here flips it — this adds a console line and nothing else about what the user sees. The message text was yours to decide and you used that latitude correctly. The visibility reporter's copy is written for a gate that did not bite; this one does, and the new prefix and gate kind exist so the two lines can say true things about opposite polarities without a second reporter to maintain. One test worth calling out because it answers a question nobody asked: "the same fault produces the same bytes in development and in production." The card was explicitly about a diagnostic missing in both builds, so pinning byte-equality across them closes the card's actual scope rather than half of it. Landing: Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6445
Six visibility legs in
SchemaRenderer.tsxroute throughevaluateVisibilityPredicateand report a fault; the two enablement legs calledevaluateConditionbare — the only uninstrumented predicate pair in the file. Both legs now passEvaluationOptions.onFault(#6038's seam), so the fault the evaluator has already caught is reported at the same number of engine calls.Verified on
cbb77c304(the head this PR was pushed at).Scope: three files, one package
packages/react/src/SchemaRenderer.tsxevaluateEnablementPredicatehelper beside its visibility sibling; bothdisabledlegs route through itpackages/react/src/utils/visibilityDiagnostic.tsPredicateGateKind+ aGATE_KIND_COPYtablepackages/react/src/index.tsNo
throwOnError, no second evaluation, no reach into@object-ui/core.The verdict does not move — deliberately
evaluateConditionansweringtruefor an unevaluable predicate, and therefore greying the control out, is existing shipped behaviour, preserved.onFaultis invoked for its side effect and its return value is ignored. Every case in the new suite pins the verdict beside the line (data-disabled-prop === 'true'), so a run that made the file green by flipping fail-soft would fail those pins instead.The message decision (the card's design content)
Triage delegated the message text with the #3862/#3955 asymmetry as the constraint. The shipped copy is written about a gate that did not bite:
On this gate the safe default is the one that bites, so that sentence would tell an author the opposite of what is on their screen and send them hunting for a rendering bug. This gate prints:
A second message, not a second reporter — the dispatch's caveat. One reporter, one dedupe
Set, one severity, one test-only reset; aPredicateGateKindparameter selects the opening line and the consequence paragraph from a table indexed without a??fallback, mirroring howPredicateScopeTieralready works in this module. The six visibility legs keep their bytes: a regression pin asserts the visibility line still starts and ends exactly as it did (#6487 landed those bytes hours ago).The reporter's name is now narrower than the function. Kept: it is exported from the package entry and called from
@object-ui/app-shelland@object-ui/components, so renaming would be a cross-package edit changing no behaviour, on a card scoped to one file's wiring. Stated in its docblock rather than left for a reader to notice.The #6444 dedupe, proved in both directions — and a correction to the dispatch's framing
Correction, measured:
warnedEvaluationFaultsinExpressionEvaluatorgoverns the evaluator's built-in line only, and only when the caller supplies noonFault—reportEvaluationFaultreturns immediately after invoking the callback, and the CEL branch forwards{ warn: false, onFault }toevalFieldPredicate, whose own passback is documented as firing on every fault. So reports from this new site do not flow through it. The rate limit that governs them is_warnedVisibilityPredicatesinvisibilityDiagnostic.ts, keyed(type, key, source). Both directions are pinned there anyway, because that Set is the one that matters here:console.warncalls = 1).disabledanddisabledOn→ 2 lines; the same source ondisabledandvisibleWhen→ 2 lines, in both mount orders. That last pair is the direction this new call site created: the two gates share oneSet, and without it a key that collapsed them would look exactly like a working rate limit.The reverse audit of the existing suite, both answers
Existing cells: nothing can now pass while measuring nothing. Every test file that exercises a
disabled/disabledOngate and watches the console was read:packages/components/.../page-header-predicate-dialect.test.tsx(its faulting-disabledcell goes through the page-header action gate andevalRowPredicate's own warn-once machinery — a different site, a different Set, and its fail direction is the opposite one: enabled, not disabled) andpackages/app-shell/.../resolveActionParams.test.ts(literaldisabled: false, no node gate). No existing cell asserts on console output while rendering a node-level faultingdisabled, so none can read another cell's dedupe entry. Both files are in the run below and stayed green.My own file: the hazard was real there, and is fixed.
inProductiongets a fresh module graph per case, so core'swarnedEvaluationFaultsstarts empty — but the development cells run on the static graph and share one Set for the whole file. A dev cell reusing another dev cell's fault source could have the evaluator's built-in line suppressed on its behalf, and the total-count pin would then read1on a build that reported the fault twice. Each dev cell now uses its own source (FAULT_BARE_DEV,FAULT_BARE_PARITY), with the reasoning written next to the constants.Related: the dev-build console also carries a pre-existing false positive on these nodes — core's
BASE_SCHEMA_RULESdeclaresdisabled"must be a boolean", so every expression-valued gate is reported as an invalid schema. Filed as #6505; the new suite subtracts it by name, never by count, so a doubled report of our own line could not hide inside the allowance.Reverse verification (direction predicted before running)
Prediction: restoring the two bare
evaluator.evaluateCondition(newSchema.disabled…)calls turns RED every report-count pin in groups 1, 2, 4 and 5, and leaves every verdict pin, all of group 3, and the visibility-copy regression guard GREEN.Observed, with the mutation proved on disk (wired call sites 2 → 0, bare 0 → 2, blob hash moved) and restored by
git checkout HEAD -- <abs path>against the pinned commit, verified by blob-hash equality and an emptygit diff HEAD:15 red, exactly the predicted set: 5/5 group 1, 7/7 group 2, 2/3 group 4 (the visibility-copy guard stayed green — it does not depend on this wiring), 1/1 group 5. Groups 0 and 3 green, every verdict pin green, and both existing files green — which is the card restated: the existing suite could not see this defect, and the change moves the silence, not the answer.
(No rebuild step: the root
vitest.config.mtsaliases@object-ui/reactand@object-ui/coretosrc, so the mutated source is what ran — nothing resolves throughdiston this path.)Verification
All runs through the shared validation lock (
os-verify-lock.sh), narrowly scoped.pnpm exec vitest run packages/react/ packages/components/src/__tests__/page-tabs-visible-when-fault-warning.test.tsx packages/components/src/__tests__/page-header-predicate-dialect.test.tsx packages/app-shell/src/providers/ExpressionProvider.visibleFaultDiagnostic.test.tsoncbb77c304→ 62 files / 928 tests passed,VERDICT command-exit 0.pnpm --filter @object-ui/react type-check(tsc --noEmit && tsc -p tsconfig.test.json) → exit 0.--listFilesconfirms the new test file is inside the program (1 hit), so "typecheck is clean" covers it rather than merely excluding it.pnpm --filter '@object-ui/react^...' build→VERDICT command-exit 0.no-explicit-anywarnings inSchemaRenderer.tsxare pre-existing and none falls inside the changed hunks (718-778, 1018-1031). Narrowing evidence: file count read from--format json(4 linted, none ignored), andeslint.config.jsconfigures noparserOptions.project/projectServiceand no import resolver, so no rule's verdict on an untouched file can depend on this diff. The repo-wideeslint .is CI's run.Filed rather than folded in
hidden/hiddenOnauthors the gate did NOT bite — on those two legs it did, and the node vanished #6503 — the same reporter tellshidden/hiddenOnauthors the gate did NOT bite, when on those two legs it did and the node vanished. Pre-existing shipped copy on the surfacereportUnresolvableVisibilityPredicate's advice paragraph names node-tier roots (record,page.<var>) that no app-shellvisiblepredicate binds #6487 just landed; its fix is a third entry in the new table, which the type's docblock records.data.*diagnostic (#5687) covers the visibility gate only — the same constant predicate ondisabledgreys a control out in silence #6504 — SchemaRenderer's node visibility gate resolves adata.*predicate against the data-source ADAPTER, so a hoistedproperties.visiblewritten that way hides the block on every row, silently #5687's adapter-onlydata.*diagnostic covers the visibility gate only; the same constant predicate ondisabledgreys a control out with no report. Needs its own ruling and its own copy.validateSchemadeclaresdisabled/visible"must be a boolean", so every expression-valued gate is reported invalid — the console line sits right beside the real diagnostic #6505 — core's devvalidateSchemadeclaresdisabled/visiblemust be booleans, so every expression-valued gate is reported invalid, right beside the real diagnostic.Generated by Claude Code