From 8e05ae8136fdd78785e70977db2988da8fbd9344 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 00:03:06 +0000 Subject: [PATCH 1/2] fix(react): report a faulting node-gate predicate in production, once per predicate source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- ...6038-production-predicate-fault-warning.md | 70 +++ content/docs/guide/metadata-diagnostics.md | 24 + ...e-tabs-visible-when-fault-warning.test.tsx | 196 ++++++++ .../src/renderers/layout/containers.tsx | 20 +- .../core/src/evaluator/ExpressionEvaluator.ts | 100 +++- .../ExpressionEvaluator.onFault.test.ts | 187 ++++++++ packages/react/src/SchemaRenderer.tsx | 51 +- ...r.productionPredicateFaultWarning.test.tsx | 437 ++++++++++++++++++ packages/react/src/index.ts | 14 + .../react/src/utils/visibilityDiagnostic.ts | 34 +- 10 files changed, 1108 insertions(+), 25 deletions(-) create mode 100644 .changeset/6038-production-predicate-fault-warning.md create mode 100644 packages/components/src/__tests__/page-tabs-visible-when-fault-warning.test.tsx create mode 100644 packages/core/src/evaluator/__tests__/ExpressionEvaluator.onFault.test.ts create mode 100644 packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx diff --git a/.changeset/6038-production-predicate-fault-warning.md b/.changeset/6038-production-predicate-fault-warning.md new file mode 100644 index 0000000000..8110de9e00 --- /dev/null +++ b/.changeset/6038-production-predicate-fault-warning.md @@ -0,0 +1,70 @@ +--- +'@object-ui/core': patch +'@object-ui/react': patch +'@object-ui/components': patch +--- + +A node-gate visibility predicate that FAULTS now says so in a production build, once per +distinct predicate source (objectui#6038, maintainer ruling 2026-08-25, option B: "the +silence is no longer an accepted property"). Observability only — no verdict moves. + +`SchemaRenderer`'s visibility chain is fail-open: a 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 (objectui#5454 / +objectui#5687) sat behind a `__DEV__` short-circuit, because the only fault-detection +channel available was `throwOnError`, and on the CEL branch `evaluateCelCondition` +implements that by evaluating **twice** — too expensive to ship for every predicate of +every node. + +**What production actually printed before, measured per dialect on the built evaluator** +— the card's premise held for one dialect of three, and the other two failed in opposite +directions: + +| dialect | production console, before | +|---|---| +| bare string | **nothing** | +| `{ dialect: 'cel' }` envelope | one generic line, deduped per source | +| `${…}` template | one generic line **per evaluation**, never deduped | + +So the dialect objectstack#11254 measured a live gate breaking on was the silent one, +while the template dialect was the console flood the ruling's rate-limit clause exists to +prevent. + +**The fix reports the fault the evaluator already detected, at the same number of engine +calls.** `EvaluationOptions.onFault` is a new passback on `@object-ui/core`'s +`ExpressionEvaluator`: every fault site is already inside a `catch`, or already holds the +canonical engine's failure reason, so nothing is evaluated twice. It mirrors, one layer +up, the seam `FieldPredicateDiagnostic` already documents (`warn: false` plus a reason +passback), and supplying it transfers reporting to the caller so one fault stays one +line. Pinned: the CEL branch performs the same number of record reads with the passback +as without it, and strictly fewer than the `throwOnError` probe. + +`SchemaRenderer` passes it in production and reports through the **same** reporter the dev +branch uses — same message, same severity, same dedupe `Set`, same key. Development and +production now print the identical line for the identical fault; the `__DEV__` gate no +longer decides *whether* a fault is reported, only *how* it is detected. + +`page:tabs` item-level `visibleWhen` (`@object-ui/components`) is covered by the same +reporter and the same rate limit. It swallowed the identical fault under a different +helper, 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. + +**Rate limit:** deduped per (node type, gate key, predicate source) — never per render and +never per node instance. A two-hundred-row list of one broken predicate is one line; a +second distinct predicate source still gets its own line. Both halves are pinned, because +a test that asserts only "a warning was emitted" is equally green on an implementation +that emitted fifty, and one that asserts only "exactly one" is equally green on an +implementation that suppresses everything. + +**Not changed by this card, deliberately:** the fail-open semantics themselves; the +objectui#5687 adapter-only `data.*` report, which stays development-only under its own +2026-08-22 ruling (that path is not a fault — the predicate evaluated perfectly, against +the wrong object); and the `/forms/:name` scope wiring of objectui#6262, which lands in +its own PR. + +`reportUnresolvableVisibilityPredicate`, `formatUnresolvableVisibilityMessage`, +`UNRESOLVABLE_VISIBILITY_PREFIX` and `__resetVisibilityPredicateWarnings` are now exported +from `@object-ui/react` so every surface that evaluates a node `visibleWhen` shares one +reporter and one rate limit — a second copy would mean a second dedupe `Set`, and one +authored predicate would be entitled to one line per package instead of one line. diff --git a/content/docs/guide/metadata-diagnostics.md b/content/docs/guide/metadata-diagnostics.md index 1661b0b0d6..27ab97bd36 100644 --- a/content/docs/guide/metadata-diagnostics.md +++ b/content/docs/guide/metadata-diagnostics.md @@ -168,6 +168,30 @@ fires while its field stays visible is the classic symptom — open the browser console and the broken predicate identifies itself (most often a bare field name where `record.` was meant). +The same is now true of a **component node's own gate** — `visibleWhen` on a +page component (and its `visible` / `visibleOn` / `visibility` / `hidden` / +`hiddenOn` siblings), plus a `page:tabs` item's `visibleWhen`. These used to +report in a development build only, so a gate that stopped biting in +production left nothing on the console at all. They now warn in **both** +builds, with the node type, the node id, the gate key, the predicate source +and the engine's reason: + +```text +[ObjectUI] A visibility predicate could not be evaluated - node "record:alert" (id: "a1") + visibleWhen: "nosuchroot.status == 'draft'" + Reason: Failed to evaluate expression "nosuchroot.status == 'draft'": nosuchroot is not defined +The node was treated as its safe default, which on this surface means the +gate did NOT bite - a predicate that cannot be evaluated reads on screen +exactly like one that said yes. +``` + +The line is **rate limited to one per distinct predicate source**, so a broken +predicate rendered down two hundred rows of a list is one line, not two +hundred — while a second, differently-broken predicate still gets its own. +The verdict is unchanged in every case: this is a diagnostic about a +predicate, not a change to what the gate decides. A node gate that fails open +renders exactly as it always did; the difference is that it now says so. + ### 4. Governance overview page `/apps//metadata/_diagnostics` — a single sortable table of every diff --git a/packages/components/src/__tests__/page-tabs-visible-when-fault-warning.test.tsx b/packages/components/src/__tests__/page-tabs-visible-when-fault-warning.test.tsx new file mode 100644 index 0000000000..e5f959c81f --- /dev/null +++ b/packages/components/src/__tests__/page-tabs-visible-when-fault-warning.test.tsx @@ -0,0 +1,196 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6038, census site 2 — a `page:tabs` item's `visibleWhen` that FAULTS + * is reported, through the same reporter and the same rate limit as the node + * gate in `SchemaRenderer`. + * + * ## Why this file exists at all + * + * The card is written about `evaluateVisibilityPredicate`'s `__DEV__` + * short-circuit, and the dispatch's census clause is explicit that the shape — + * "a predicate evaluation caught and swallowed" — is what has to be covered, + * not the symbol. `PageTabsRenderer.isItemVisible` is that shape under another + * name: it calls the same `evaluateCondition`, on the same canonical key + * (`visibleWhen`), with the same fail-open contract its own comment declares + * ("the same semantics SchemaRenderer applies to component-level + * `visibleWhen`"). + * + * It was in fact the WORSE of the two. The node gate at least reported in a + * development build; an item-level `visibleWhen` that faulted here was silent + * in BOTH builds — and its false verdict removes an entire tab, header and + * panel, rather than one block. A tab that quietly stops disappearing (or + * quietly stops appearing) is the failure an author is least likely to notice, + * because a tab strip looks correct in every arrangement. + * + * ## One reporter, one rate limit — not one per package + * + * The report goes through `reportUnresolvableVisibilityPredicate`, exported + * from `@object-ui/react` for this card. A local copy would mean a second + * dedupe `Set`, and one authored predicate would then be entitled to one line + * per package instead of one line. The last case here is what pins that. + * + * ## Reverse verification (direction predicted BEFORE running) + * + * Dropping the `onFault` option from `isItemVisible` turns RED exactly the + * report cases and leaves every VERDICT case green — the tabs render + * identically either way, which is this card's observability-only constraint + * restated on this surface. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import React from 'react'; +import { + SchemaRenderer, + UNRESOLVABLE_VISIBILITY_PREFIX, + __resetVisibilityPredicateWarnings, +} from '@object-ui/react'; +import '../renderers'; + +const tabsSchema = (items: any[]) => ({ type: 'page:tabs', id: 'tabs', items }); + +/** Faults on every dialect this surface accepts (measured on the built evaluator). */ +const FAULT_BARE = 'nosuchroot.x > 1'; +const FAULT_BARE_2 = 'anotherbadroot.y == 3'; + +type WarnSpy = { mock: { calls: unknown[][] } }; +const spyWarn = () => vi.spyOn(console, 'warn').mockImplementation(() => {}); +const reports = (warn: WarnSpy): string[] => + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX)); +const allWarnings = (warn: WarnSpy): string[] => warn.mock.calls.map((c) => String(c[0])); + +beforeEach(() => { + __resetVisibilityPredicateWarnings(); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe('objectui#6038 — a faulting `page:tabs` item predicate is reported', () => { + it('POSITIVE CONTROL: the spy observes a line carrying the prefix', () => { + // Without it, the `toHaveLength(0)` cases below are equally green on a + // capture that observes nothing. + const warn = spyWarn(); + console.warn(`${UNRESOLVABLE_VISIBILITY_PREFIX} - synthetic control line`); + expect(reports(warn)).toHaveLength(1); + }); + + it('DEGENERATE CONTROL: unrelated console output does not satisfy the pin', () => { + const warn = spyWarn(); + console.warn('[object-ui] an entirely unrelated warning'); + expect(allWarnings(warn)).toHaveLength(1); + expect(reports(warn)).toHaveLength(0); + }); + + it('THE acceptance criterion: a faulting item `visibleWhen` warns, and the tab still renders', () => { + const warn = spyWarn(); + const { getByText } = render( + , + ); + // VERDICT UNCHANGED — fail-open, so the tab is still there. This card does + // not get to move that; it only gets to say so. + expect(getByText('Contracts')).toBeTruthy(); + + const lines = reports(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('page:tabs'); + expect(lines[0]).toContain('visibleWhen'); + expect(lines[0]).toContain(FAULT_BARE); + expect(lines[0]).toContain('Reason:'); + }); + + it('a HEALTHY item predicate stays silent, on both verdicts', () => { + // The half that makes the loud half mean something: a false predicate is a + // verdict, not a fault, and must print nothing. + const warn = spyWarn(); + // THREE items, two of them surviving: `alwaysShowStrip` defaults to false, + // so a strip down to a single tab hides its header entirely and renders the + // panel bare — the assertion below would then be measuring that rule + // instead of this one. (Measured: this case first failed on exactly that.) + const { getByText, queryByText } = render( + , + ); + expect(getByText('Details')).toBeTruthy(); + expect(getByText('Related')).toBeTruthy(); + expect(queryByText('Contracts')).toBeNull(); + expect(allWarnings(warn)).toHaveLength(0); + }); + + it('deduped per predicate SOURCE: eight tabs sharing one broken predicate produce ONE line', () => { + // The "not per call-site instance" half — eight distinct items, eight + // evaluations, one authored mistake. + const warn = spyWarn(); + render( + ({ + label: `Tab ${i}`, + value: `t${i}`, + visibleWhen: FAULT_BARE, + children: [], + })), + )} + />, + ); + expect(reports(warn)).toHaveLength(1); + expect(allWarnings(warn)).toHaveLength(1); + }); + + it('a SECOND distinct predicate source still warns — a dedupe that suppressed everything would look identical', () => { + const warn = spyWarn(); + render( + , + ); + const lines = reports(warn); + expect(lines).toHaveLength(2); + expect(lines.some((l) => l.includes(FAULT_BARE))).toBe(true); + expect(lines.some((l) => l.includes(FAULT_BARE_2))).toBe(true); + }); + + it('ONE rate limit across packages: the node gate and the tab gate share a dedupe `Set`', () => { + // The property the shared export exists for. The SAME predicate source on + // the SAME node type must not be entitled to a second line just because a + // second package evaluated it. Here the tab strip reports first; a node + // gate of type `page:tabs` carrying the same `visibleWhen` source then + // finds the entry already present. + // + // (A different node TYPE legitimately reports again — the key is + // (type, key, source), and two types are two places an author has to go + // and fix. This case holds the type fixed, which is what isolates the + // cross-package question from the key-shape question.) + const warn = spyWarn(); + render( + , + ); + expect(reports(warn)).toHaveLength(1); + cleanup(); + render(); + expect(reports(warn)).toHaveLength(1); + }); +}); diff --git a/packages/components/src/renderers/layout/containers.tsx b/packages/components/src/renderers/layout/containers.tsx index 62297dc72c..f3c8e995e9 100644 --- a/packages/components/src/renderers/layout/containers.tsx +++ b/packages/components/src/renderers/layout/containers.tsx @@ -22,7 +22,7 @@ import React from 'react'; import { ComponentRegistry, ExpressionEvaluator, evalRowPredicate, getRecordDisplayName, toPredicateRecord } from '@object-ui/core'; import type { ComponentInput } from '@object-ui/core'; import { actionRendersAt } from '@object-ui/types'; -import { useRecordContext, useAction, useCapabilityGate, usePredicateScope, usePageVariables, useInlineEdit, useActionTextLocalizer } from '@object-ui/react'; +import { useRecordContext, useAction, useCapabilityGate, usePredicateScope, usePageVariables, useInlineEdit, useActionTextLocalizer, reportUnresolvableVisibilityPredicate } from '@object-ui/react'; import { renderChildren, cn } from '../../lib/utils'; import { LazyIcon } from '../../lib/lazy-icon'; import { RelatedCountStore, useRelatedCountVersion } from '../../hooks/related-count-store'; @@ -456,8 +456,22 @@ const PageTabsRenderer: React.FC = ({ schema, className, ...props }) => { page: pageVariables, }); // evaluateCondition is fail-open (unparseable predicate → visible) — the - // same semantics SchemaRenderer applies to component-level `visibleWhen`. - return evaluator.evaluateCondition(it.visibleWhen); + // same semantics SchemaRenderer applies to component-level `visibleWhen`, + // and objectui#6038 gives it the same VOICE. The verdict is untouched: a + // faulting predicate still resolves to `true` and the tab still renders. + // + // This site is in the census for the reason the card's census clause names + // — it swallows the identical fault under a different helper. It is worse + // than the node gate was, in fact: `SchemaRenderer` at least reported in + // development, while an item-level `visibleWhen` that faulted here was + // silent in BOTH builds, on a gate whose false verdict removes an entire + // tab (header and panel) rather than one block. Reported through the SAME + // reporter and the SAME dedupe `Set` as the node gate, so one authored + // predicate is one line no matter which surface evaluates it. + return evaluator.evaluateCondition(it.visibleWhen, { + onFault: (reason) => + reportUnresolvableVisibilityPredicate('page:tabs', schema?.id, 'visibleWhen', it.visibleWhen, reason), + }); }; const visibleFlags = rawItems.map(isItemVisible); // Keep the filtered array's identity stable while visibility is unchanged diff --git a/packages/core/src/evaluator/ExpressionEvaluator.ts b/packages/core/src/evaluator/ExpressionEvaluator.ts index 9b0ca4f555..298b84bc05 100644 --- a/packages/core/src/evaluator/ExpressionEvaluator.ts +++ b/packages/core/src/evaluator/ExpressionEvaluator.ts @@ -41,6 +41,70 @@ export interface EvaluationOptions { * @default true */ sanitize?: boolean; + + /** + * Fault passback: called with the failure reason when this evaluation could + * not be performed, at the moment the evaluator ALREADY knows it faulted. + * + * ## Why a passback and not a second evaluation (objectui#6038) + * + * The only fault-detection channel this class used to offer a fail-soft + * caller was `throwOnError`, and on the CEL branch `evaluateCelCondition` + * implements that by evaluating TWICE (once with each fallback — a value + * that tracks the fallback both times is a fault). A caller that wants to + * *observe* a fault while keeping the fail-soft verdict therefore had to pay + * for a second engine call per predicate per node per render. That price is + * exactly why the node gate in `SchemaRenderer` bought its diagnostic with a + * `__DEV__` gate and shipped production silent. + * + * This option costs nothing: every fault site below is already inside a + * `catch`, or already holds the engine's own failure reason. The verdict is + * untouched on every path — `onFault` is invoked for its side effect and its + * return value is ignored. + * + * ## Supplying it TRANSFERS reporting to the caller + * + * The built-in `console.warn`s on these paths are suppressed while it is + * set, so one fault stays one line — the caller's, which can name the node + * the predicate belongs to. This mirrors, one layer up, the contract + * `FieldPredicateDiagnostic` (`fieldRules.ts`) already documents for the + * canonical CEL engine: `warn: false` plus an `onFault` passback, so + * silencing the generic line never discards the description of *why* the + * predicate failed. On the CEL branch this option is forwarded to exactly + * that seam rather than reimplementing it. + * + * Independent of {@link throwOnError}, which converts a fault into a throw + * and is the fail-CLOSED contract; this one keeps the historical fail-soft + * answer and merely says so out loud. `throwOnError` still wins where both + * are set: the throw happens first and is the caller's own signal. + * + * Must not throw — it is invoked outside the evaluation guard, so an + * exception here propagates to the caller rather than being reported as an + * evaluation fault. + */ + onFault?: (reason: string) => void; +} + +/** + * One fault, one report: hand the reason to the caller's {@link + * EvaluationOptions.onFault} when it supplied one, otherwise fall back to this + * class's historical `console.warn`. + * + * Kept as a module-local function rather than repeating the ternary at each + * catch, so the "supplying `onFault` suppresses the built-in line" rule is + * stated once and cannot drift between the three sites that implement it. + */ +function reportEvaluationFault( + onFault: ((reason: string) => void) | undefined, + builtinMessage: string, + error: unknown, +): void { + const reason = error instanceof Error ? error.message : String(error); + if (onFault) { + onFault(reason); + return; + } + console.warn(builtinMessage, error); } /** @@ -91,7 +155,7 @@ export class ExpressionEvaluator { return expression; } - const { defaultValue, throwOnError = false, sanitize = true } = options; + const { defaultValue, throwOnError = false, sanitize = true, onFault } = options; try { // Check if string contains template expressions @@ -117,7 +181,7 @@ export class ExpressionEvaluator { if (throwOnError) { throw error; } - console.warn(`Expression evaluation failed for: ${expr}`, error); + reportEvaluationFault(onFault, `Expression evaluation failed for: ${expr}`, error); return match; // Return original if evaluation fails } }); @@ -125,7 +189,7 @@ export class ExpressionEvaluator { if (throwOnError) { throw error; } - console.warn(`Failed to evaluate expression: ${expression}`, error); + reportEvaluationFault(onFault, `Failed to evaluate expression: ${expression}`, error); return defaultValue ?? expression; } } @@ -266,6 +330,17 @@ export class ExpressionEvaluator { if (options.throwOnError) { throw error; } + // objectui#6038 — the dialect that reported NOTHING. Measured on the + // built evaluator against the other two: a `{ dialect: 'cel' }` envelope + // already warns here in production (`evalFieldPredicate`, deduped per + // source) and a `${…}` template already warns (the generic line above), + // while a BARE-STRING predicate that faults returned its fail-soft `true` + // in complete silence. That is the dialect objectstack#11254 measured a + // real gate breaking on, and the reason a node gate could stop biting + // with nothing on the console to say so. `onFault` is the only new + // channel: absent it this catch behaves exactly as it always has, so no + // existing caller's console output moves. + options.onFault?.(error instanceof Error ? error.message : String(error)); return true; } } @@ -287,8 +362,23 @@ export class ExpressionEvaluator { ? (rec as Record) : (bag as Record); if (!options.throwOnError) { - // Fast path: one evaluation, fail-soft to visible/enabled (legacy parity). - return evalFieldPredicate(source, record, true, undefined, bag); + // Fast path: ONE evaluation, fail-soft to visible/enabled (legacy parity). + // + // objectui#6038: when the caller passes `onFault`, forward it to the seam + // `evalFieldPredicate` already exposes for exactly this — `warn: false` + // plus the reason passback — rather than adding a second reporter. The + // caller then emits one line that can name the node; without `onFault` + // the built-in warning fires exactly as before. Either way this stays a + // SINGLE engine call: the `throwOnError` double-evaluation below is what + // this branch exists to avoid paying in production. + return evalFieldPredicate( + source, + record, + true, + undefined, + bag, + options.onFault ? { warn: false, onFault: options.onFault } : undefined, + ); } // Fail-closed callers need to tell a genuine `false` from a fault. The // canonical helper fails soft to the fallback, so a value that tracks the diff --git a/packages/core/src/evaluator/__tests__/ExpressionEvaluator.onFault.test.ts b/packages/core/src/evaluator/__tests__/ExpressionEvaluator.onFault.test.ts new file mode 100644 index 0000000000..a7704a5509 --- /dev/null +++ b/packages/core/src/evaluator/__tests__/ExpressionEvaluator.onFault.test.ts @@ -0,0 +1,187 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6038 — `EvaluationOptions.onFault`: observe a predicate fault + * WITHOUT a second evaluation and WITHOUT moving a verdict. + * + * ## The problem this seam exists to solve + * + * Before it, the only fault-detection channel a fail-soft caller had was + * `throwOnError`, and on the CEL branch `evaluateCelCondition` implements that + * by evaluating TWICE. So a caller that wanted to *report* a broken predicate + * while keeping the historical fail-open answer had to double the engine calls + * for every predicate of every node of every render — which is precisely why + * `SchemaRenderer`'s node gate bought its diagnostic with a `__DEV__` gate and + * shipped production silent. The maintainer's 2026-08-25 ruling (option B) + * retired that silence and kept the negligible-cost requirement, so the fault + * has to become observable at the SAME number of engine calls. + * + * ## What was actually silent — measured, per dialect + * + * Against `origin/main`, on the built evaluator, a faulting predicate with no + * options produced: bare string -> NOTHING; `{ dialect: 'cel' }` -> one generic + * line, deduped; `${…}` template -> one generic line PER EVALUATION. The first + * group of cases below pins all three converging on the passback, which is what + * lets one caller print one line for a fault in any dialect. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ExpressionEvaluator } from '../ExpressionEvaluator.js'; + +const SCOPE = { record: { status: 'open' }, data: { total: 99 }, page: {} }; +const evaluator = () => new ExpressionEvaluator(SCOPE); + +/** Faulting predicates, one per dialect. */ +const FAULT_BARE = 'nosuchroot.x > 1'; +const FAULT_TEMPLATE = '${nosuchroot.x > 1}'; +const FAULT_CEL = { dialect: 'cel', source: 'record.bad(' }; + +describe('#6038 — onFault fires on every dialect that can fault', () => { + it('BARE STRING: the dialect that reported nothing at all now hands back a reason', () => { + // objectstack#11254 measured a real gate breaking on exactly this dialect + // and produced no console line anywhere. + const reasons: string[] = []; + const verdict = evaluator().evaluateCondition(FAULT_BARE, { onFault: (r) => reasons.push(r) }); + expect(verdict).toBe(true); // fail-open, unchanged + expect(reasons).toHaveLength(1); + expect(reasons[0]).toContain('nosuchroot'); + }); + + it('CEL ENVELOPE: forwarded to `evalFieldPredicate`\'s existing passback, and its generic line is suppressed', () => { + // One fault, one report. Without `warn: false` the canonical engine would + // print its own line beside the caller's, so a production console would + // show two lines for one broken predicate. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const reasons: string[] = []; + const verdict = evaluator().evaluateCondition(FAULT_CEL, { onFault: (r) => reasons.push(r) }); + expect(verdict).toBe(true); + expect(reasons).toHaveLength(1); + expect(reasons[0]).toContain('parse'); + expect(warn).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); + + it('${…} TEMPLATE: the generic per-evaluation line is replaced by one passback per evaluation', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const reasons: string[] = []; + const verdict = evaluator().evaluateCondition(FAULT_TEMPLATE, { onFault: (r) => reasons.push(r) }); + expect(verdict).toBe(true); + expect(reasons).toHaveLength(1); + expect(warn).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); + + it('a HEALTHY predicate never calls it — on either verdict', () => { + // The silence that makes the signal mean something. A `false` verdict is an + // answer, not a fault, and a passback that fired on it would report every + // hiding gate in the repository. + const reasons: string[] = []; + const e = evaluator(); + expect(e.evaluateCondition("record.status == 'open'", { onFault: (r) => reasons.push(r) })).toBe(true); + expect(e.evaluateCondition("record.status == 'closed'", { onFault: (r) => reasons.push(r) })).toBe(false); + expect(e.evaluateCondition({ dialect: 'cel', source: "record.status == 'closed'" }, { onFault: (r) => reasons.push(r) })).toBe(false); + expect(reasons).toHaveLength(0); + }); +}); + +describe('#6038 — the seam moves no verdict and adds no evaluation', () => { + it('every predicate shape reaches the identical verdict with and without onFault', () => { + // The card is observability-only. This is that constraint as a measurement + // rather than a claim: same inputs, both call shapes, byte-identical answers. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const cases: Array<[unknown, string]> = [ + [FAULT_BARE, 'bare fault'], + [FAULT_CEL, 'cel fault'], + [FAULT_TEMPLATE, 'template fault'], + ["record.status == 'open'", 'bare true'], + ["record.status == 'closed'", 'bare false'], + [{ dialect: 'cel', source: "record.status == 'open'" }, 'cel true'], + [{ dialect: 'cel', source: "record.status == 'closed'" }, 'cel false'], + ['${record.status == "open"}', 'template true'], + ['${record.status == "closed"}', 'template false'], + [true, 'literal true'], + [false, 'literal false'], + [undefined, 'absent'], + ['', 'empty'], + [' ', 'whitespace'], + ]; + for (const [pred, label] of cases) { + const without = evaluator().evaluateCondition(pred as never); + const with_ = evaluator().evaluateCondition(pred as never, { onFault: () => {} }); + expect(`${label}: ${String(with_)}`).toBe(`${label}: ${String(without)}`); + } + } finally { + warn.mockRestore(); + } + }); + + it('the CEL branch still makes ONE engine call — the `throwOnError` probe is not smuggled in', () => { + // The whole point of the passback. `throwOnError` detects a CEL fault by + // evaluating twice (once with each fallback); if `onFault` had been built on + // top of it, production would pay double for every predicate of every node. + // A throwing getter on the record counts the reads the engine performs. + let reads = 0; + const probed = new ExpressionEvaluator({ + record: { + get status() { + reads += 1; + return 'open'; + }, + }, + }); + reads = 0; + probed.evaluateCondition({ dialect: 'cel', source: "record.status == 'open'" }, { onFault: () => {} }); + const withPassback = reads; + + reads = 0; + probed.evaluateCondition({ dialect: 'cel', source: "record.status == 'open'" }, { throwOnError: true }); + const withProbe = reads; + + // The passback path reads the record the same number of times a plain + // fail-soft call does; the `throwOnError` probe reads it strictly more. + reads = 0; + probed.evaluateCondition({ dialect: 'cel', source: "record.status == 'open'" }); + const plain = reads; + + expect(withPassback).toBe(plain); + expect(withProbe).toBeGreaterThan(plain); + }); + + it('without onFault, nothing about the existing console output moves', () => { + // The compatibility half: every caller that does not opt in must see the + // exact lines it saw before. A bare-string fault stays silent (it always + // was), and the two loud dialects keep their own generic lines. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect(evaluator().evaluateCondition(FAULT_BARE)).toBe(true); + expect(warn).not.toHaveBeenCalled(); + + expect(evaluator().evaluateCondition(FAULT_TEMPLATE)).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toContain('Failed to evaluate expression'); + } finally { + warn.mockRestore(); + } + }); + + it('`throwOnError` still wins where both are set — the fail-closed contract is unmoved', () => { + // Documented precedence. A caller that asked for a throw gets a throw; the + // passback is the fail-SOFT observation channel, not a second one. + expect(() => + evaluator().evaluateCondition(FAULT_BARE, { throwOnError: true, onFault: () => {} }), + ).toThrow(); + }); +}); diff --git a/packages/react/src/SchemaRenderer.tsx b/packages/react/src/SchemaRenderer.tsx index 619f23374b..1b16513bd8 100644 --- a/packages/react/src/SchemaRenderer.tsx +++ b/packages/react/src/SchemaRenderer.tsx @@ -623,7 +623,20 @@ export const SchemaRenderer: ForwardRefExoticComponent< * on which dialect they happened to write it in. * * Deduped per (node type, key, predicate source): a broken predicate is - * re-evaluated on every render, and the point is one line, not a wall. + * re-evaluated on every render, and the point is one line, not a wall. The + * key is the predicate SOURCE TEXT plus the gate it was authored on — never + * the render and never the schema object — so the same broken predicate + * rendered over two hundred rows reports once, and a SECOND distinct + * predicate still reports (objectui#6038 pins both halves). + * + * ## Production is loud too, since objectui#6038 + * + * It was `__DEV__`-only, and the maintainer's 2026-08-25 ruling retired + * that silence: a gate that stops biting in production used to leave + * nothing on the console for the bare-string dialect, so a class-1 defect + * could sit live and undiscovered (measured in objectstack#11254). The + * `__DEV__` gate below no longer decides WHETHER the fault is reported, + * only HOW it is detected — see the two branches. * * ## Defined HERE, ahead of the `properties` evaluation loop below * @@ -635,15 +648,33 @@ export const SchemaRenderer: ForwardRefExoticComponent< * on the POST-evaluation, POST-hoist schema, for the real verdict. */ const evaluateVisibilityPredicate = (raw: VisibilityPredicate, key: string): boolean => { - // PRODUCTION IS THE UNTOUCHED CALL. `throwOnError` is how the fault is - // detected, and on the CEL branch `evaluateCelCondition` implements it by - // evaluating TWICE (once with each fallback — a value that tracks the - // fallback both times is a fault). Spec-parsed metadata normalizes - // `visibleWhen` into a `{ dialect: 'cel' }` envelope, so that branch is - // the common one in production: paying for the probe unconditionally - // would double the engine calls for every predicate of every node, to - // build a message no production build ever prints. - if (!__DEV__) return evaluator.evaluateCondition(raw); + // PRODUCTION STILL MAKES THE SINGLE CALL — it just no longer makes it + // in silence (objectui#6038, maintainer ruling 2026-08-25, option B). + // + // `throwOnError` remains the DEV probe and remains too expensive to ship: + // on the CEL branch `evaluateCelCondition` implements it by evaluating + // TWICE (once with each fallback — a value that tracks the fallback both + // times is a fault), and spec-parsed metadata normalizes `visibleWhen` + // into a `{ dialect: 'cel' }` envelope, so that branch is the common one + // in production. Paying for the probe here would double the engine calls + // for every predicate of every node. + // + // `onFault` is the way out of that trade: the evaluator hands back the + // reason at the point it ALREADY knows the predicate faulted, inside the + // catch it already runs, so the fault becomes observable at ONE engine + // call. The verdict is `evaluateCondition(raw)`'s, unchanged — this card + // is observability only, and the fail-open semantics are not its to move. + // + // The reporter is the SAME one the dev branch below uses: same message, + // same severity, same dedupe `Set`, same key. Production and development + // now print the identical line for the identical fault, which is the + // property the `__DEV__` gate used to cost us. + if (!__DEV__) { + return evaluator.evaluateCondition(raw, { + onFault: (reason) => + reportUnresolvableVisibilityPredicate(newSchema.type, newSchema.id, key, raw, reason), + }); + } try { const verdict = evaluator.evaluateCondition(raw, { throwOnError: true }); // objectui#5687 — the NON-throwing half of the same silence, and it is diff --git a/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx b/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx new file mode 100644 index 0000000000..97460f05b8 --- /dev/null +++ b/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx @@ -0,0 +1,437 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6038 — a node-gate predicate that FAULTS is loud in a PRODUCTION + * build, once per distinct predicate source. + * + * 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. […] A is rejected — the silence is no longer an + * accepted property." Observability only: the fail-open semantics are NOT this + * card's to change, so every case below pins the verdict alongside the line. + * + * ## What "the silence" actually was — measured, not assumed + * + * The card states production prints nothing. Measured on the built evaluator + * against `origin/main`, that is true of ONE dialect of three, and the other + * two fail in opposite directions: + * + * | dialect | production console, before | + * |------------------------|----------------------------| + * | bare string | NOTHING | + * | `{ dialect: 'cel' }` | 1 generic line, deduped | + * | `${…}` template | 1 generic line PER EVALUATION (never deduped) | + * + * So the bare-string dialect — the one objectstack#11254 measured a real gate + * breaking on — was silent, while the template dialect was the flood the + * ruling's rate-limit clause exists to prevent. Both are now ONE node-bearing + * line per distinct source, which is why several cases below pin the TOTAL + * `console.warn` count and not merely the count of matching lines: an + * implementation that added our line beside the generic one would satisfy + * "warned once" while doubling what a production console actually shows. + * + * ## The two halves of a rate limit, and why one of them is not optional + * + * "A warning was emitted" is green on an implementation that emitted fifty, and + * "exactly one warning was emitted" is green on an implementation that + * suppresses EVERYTHING after the first line ever printed. Neither pin alone + * can tell a working dedupe from a broken one, so every dedupe case here pins + * both: N evaluations of one source => exactly 1 line, AND a second DISTINCT + * source still => its own line. + * + * ## Controls + * + * - **Positive control** (`capture` group): the spy sees a line this test emits + * itself. Without it, every `toHaveLength(0)` in the file is also green when + * the capture is simply broken. + * - **Degenerate control** (`capture` group): unrelated `console.warn` output + * does NOT satisfy the pin — the assertions read a PREFIX-FILTERED view, so + * noise from anywhere else in the render cannot stand in for the diagnostic. + * + * ## Reverse verification (direction predicted BEFORE running) + * + * Restoring `if (!__DEV__) return evaluator.evaluateCondition(raw);` — the line + * this card replaces — turns RED every production case in groups 1-3 (their + * report counts fall to 0, except the CEL and template cases, whose GENERIC + * lines reappear and whose total-warn pins therefore also move), and leaves + * every verdict assertion and the whole of group 4 GREEN. That asymmetry is + * the card restated: the change moves the silence, not the answer. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import React from 'react'; +import { UNRESOLVABLE_VISIBILITY_PREFIX } from '../utils/visibilityDiagnostic'; + +const NAME = 'probe-6038'; +const TYPE = 'element:probe-6038'; + +const Probe = () =>
; + +/** The ambient scope app-shell's `ExpressionProvider` really mounts. */ +const APP_SCOPE = { + current_user: { id: 'u1' }, + user: { id: 'u1' }, + app: {}, + data: {}, + features: {}, +}; +const ADAPTER = { total: 99 }; +const ROW = { id: 'r1', status: 'open' }; + +/** + * Predicates that genuinely FAULT, one per dialect, measured on the built + * evaluator. An unbound ROOT identifier is the shape #11254 hit and the shape + * an AI-authored predicate reaches for; `record.bad(` is a parse error, which + * is the only fault the CEL engine reports as `[parse]`. + */ +const FAULT_BARE = 'nosuchroot.x > 1'; +const FAULT_BARE_2 = 'anotherbadroot.y == 3'; +const FAULT_TEMPLATE = '${nosuchroot.x > 1}'; +const FAULT_CEL = { dialect: 'cel', source: 'record.bad(' }; +/** A predicate that resolves cleanly — the silence that makes the noise mean something. */ +const HEALTHY = "record.status == 'open'"; + +type WarnSpy = { mock: { calls: unknown[][] } }; +const spyWarn = () => vi.spyOn(console, 'warn').mockImplementation(() => {}); +/** The PREFIX-FILTERED view every assertion reads. */ +const reports = (warn: WarnSpy): string[] => + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX)); +/** Everything the console was asked to print, filtered by nothing. */ +const allWarnings = (warn: WarnSpy): string[] => warn.mock.calls.map((c) => String(c[0])); + +/** + * Mount `schemas` in a PRODUCTION module graph. + * + * `__DEV__` in `SchemaRenderer` is an IIFE evaluated at module load, so the env + * has to be stubbed before the import — hence `resetModules` and a dynamic + * import in the test BODY (the case `object-ui/no-dynamic-import-in-test-hook` + * exempts). The dedupe `Set` is module state of that same fresh graph, so the + * reset has to come from the fresh graph too: resetting the statically-imported + * copy would clear a DIFFERENT `Set` and every count below would be measuring + * leakage from the previous case. + */ +async function inProduction( + fn: (mount: (schemas: Record[]) => void) => void | Promise, +): Promise { + vi.resetModules(); + vi.stubEnv('NODE_ENV', 'production'); + try { + const [core, prod, ctx, rec, expr, diag] = await Promise.all([ + import('@object-ui/core'), + import('../SchemaRenderer'), + import('../context/SchemaRendererContext'), + import('../context/RecordContext'), + import('../hooks/useExpression'), + import('../utils/visibilityDiagnostic'), + ]); + diag.__resetVisibilityPredicateWarnings(); + core.ComponentRegistry.register(NAME, Probe as never, { + namespace: 'element', + skipFallback: true, + } as never); + const mount = (schemas: Record[]) => + render( + + + + {schemas.map((s, i) => ( + + ))} + + + , + ); + await fn(mount); + core.ComponentRegistry.unregister?.(NAME, 'element'); + } finally { + cleanup(); + vi.unstubAllEnvs(); + vi.resetModules(); + } +} + +const shownCount = () => screen.queryAllByTestId('probe').length; + +beforeEach(() => { + vi.restoreAllMocks(); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/* -------------------------------------------------------------------------- * + * Group 0 — the controls that decide whether the rest of the file means + * anything. Both are about the CAPTURE, not about the renderer. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 0 — capture controls', () => { + it('POSITIVE CONTROL: the spy really observes a line carrying the prefix', () => { + // Without this, every `toHaveLength(0)` in this file is equally green on a + // capture that observes nothing at all. + const warn = spyWarn(); + console.warn(`${UNRESOLVABLE_VISIBILITY_PREFIX} - synthetic control line`); + expect(reports(warn)).toHaveLength(1); + }); + + it('DEGENERATE CONTROL: unrelated console output does NOT satisfy the pin', () => { + // The assertions read a prefix-filtered view precisely so that noise — a + // React key warning, a deprecation notice, another diagnostic — can never + // stand in for this diagnostic. A test that counted raw `console.warn` + // calls would pass on a build that emitted only the noise. + const warn = spyWarn(); + console.warn('[object-ui] some entirely unrelated warning'); + console.warn('Warning: each child in a list should have a unique "key" prop.'); + expect(allWarnings(warn)).toHaveLength(2); + expect(reports(warn)).toHaveLength(0); + }); +}); + +/* -------------------------------------------------------------------------- * + * Group 1 — THE acceptance criterion, per dialect. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 1 — a faulting node gate is loud in production', () => { + it('THE acceptance criterion: a BARE-STRING `visibleWhen` fault warns in a production build (it printed NOTHING before)', async () => { + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + + // Verdict FIRST — observability only. The gate still fails open, which + // is exactly the property that made the silence dangerous. + expect(shownCount()).toBe(1); + + const lines = reports(warn); + expect(lines).toHaveLength(1); + // The line has to identify the predicate, the gate and the node, or it + // is not a diagnostic — it is an alarm with no address. + expect(lines[0]).toContain(TYPE); + expect(lines[0]).toContain('n1'); + expect(lines[0]).toContain('visibleWhen'); + expect(lines[0]).toContain(FAULT_BARE); + expect(lines[0]).toContain('Reason:'); + // ONE fault, ONE line: nothing else was printed alongside it. + expect(allWarnings(warn)).toHaveLength(1); + }); + }); + + it('a `{ dialect: "cel" }` envelope fault warns ONCE — the generic line is replaced, not joined', async () => { + // Before this card the CEL branch already printed the canonical engine's + // own generic line here. The risk of this card was two lines for one fault; + // the total-count pin is what refuses it. + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: FAULT_CEL }]); + expect(shownCount()).toBe(1); + const lines = reports(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('record.bad('); + expect(allWarnings(warn)).toHaveLength(1); + }); + }); + + it('a `${…}` template fault warns ONCE across many evaluations — it used to print one line PER evaluation', async () => { + // The flood direction. Three nodes carrying the same broken template used + // to produce three generic lines and would produce three per re-render. + await inProduction((mount) => { + const warn = spyWarn(); + mount([ + { id: 'n1', visibleWhen: FAULT_TEMPLATE }, + { id: 'n2', visibleWhen: FAULT_TEMPLATE }, + { id: 'n3', visibleWhen: FAULT_TEMPLATE }, + ]); + expect(shownCount()).toBe(3); + expect(reports(warn)).toHaveLength(1); + expect(allWarnings(warn)).toHaveLength(1); + }); + }); + + it('the `hidden` leg — the NON-negated polarity — reports too, and still hides', async () => { + // `hidden` / `hiddenOn` are the two legs whose fail-soft `true` means HIDE. + // A fault there removes the node, which is the louder screen symptom and + // the quieter console one; it must not be a second silence. + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', hidden: FAULT_BARE }]); + expect(shownCount()).toBe(0); + const lines = reports(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('hidden'); + }); + }); +}); + +/* -------------------------------------------------------------------------- * + * Group 2 — the rate limit: BOTH halves. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 2 — deduped per predicate SOURCE, not per render and not per node instance', () => { + it('three nodes with DIFFERENT ids and the SAME predicate source produce exactly ONE line', async () => { + // This is the "not the call-site instance" half. The ids differ, so a key + // that included the node instance (or the schema object) would emit three. + await inProduction((mount) => { + const warn = spyWarn(); + mount([ + { id: 'alpha', visibleWhen: FAULT_BARE }, + { id: 'beta', visibleWhen: FAULT_BARE }, + { id: 'gamma', visibleWhen: FAULT_BARE }, + ]); + expect(shownCount()).toBe(3); + expect(reports(warn)).toHaveLength(1); + }); + }); + + it('re-rendering the same faulting predicate does NOT add a line — the "not per render" half', async () => { + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + cleanup(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + cleanup(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + expect(reports(warn)).toHaveLength(1); + }); + }); + + it('a SECOND distinct predicate source still warns — without this, a dedupe that suppresses everything looks identical', async () => { + // The half that a "warned at least once" test cannot see. An implementation + // that printed one line ever, for the life of the page, passes every pin + // above and fails this one. + await inProduction((mount) => { + const warn = spyWarn(); + mount([ + { id: 'n1', visibleWhen: FAULT_BARE }, + { id: 'n2', visibleWhen: FAULT_BARE_2 }, + ]); + expect(shownCount()).toBe(2); + const lines = reports(warn); + expect(lines).toHaveLength(2); + expect(lines.some((l) => l.includes(FAULT_BARE))).toBe(true); + expect(lines.some((l) => l.includes(FAULT_BARE_2))).toBe(true); + }); + }); + + it('a two-hundred-row list of ONE broken predicate is one line, not two hundred', async () => { + // The scenario the ruling's own cost table names ("一个列表里几百行会淹没 + // 控制台"). It is the same property as the case above, at the scale that + // decides whether option B is usable. + await inProduction((mount) => { + const warn = spyWarn(); + mount(Array.from({ length: 200 }, (_, i) => ({ id: `row-${i}`, visibleWhen: FAULT_BARE }))); + expect(shownCount()).toBe(200); + expect(reports(warn)).toHaveLength(1); + expect(allWarnings(warn)).toHaveLength(1); + }); + }); +}); + +/* -------------------------------------------------------------------------- * + * Group 3 — the silence that makes the noise mean something. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 3 — nothing else became loud', () => { + it('a HEALTHY predicate prints nothing, on either verdict', async () => { + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: HEALTHY }]); + expect(shownCount()).toBe(1); + cleanup(); + mount([{ id: 'n2', visibleWhen: "record.status == 'closed'" }]); + expect(shownCount()).toBe(0); + expect(allWarnings(warn)).toHaveLength(0); + }); + }); + + it('a node with NO gate at all prints nothing and renders', async () => { + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1' }]); + expect(shownCount()).toBe(1); + expect(allWarnings(warn)).toHaveLength(0); + }); + }); + + it('objectui#5687 stays DEV-ONLY: an adapter-only `data.*` predicate is silent in production', async () => { + // Deliberately NOT extended by this card. That leg reports a predicate that + // evaluated perfectly against the wrong object — not a fault — and its own + // ruling (2026-08-22, option A) scoped it to development. Pinned here so + // "production is louder" cannot quietly become "production reports + // everything the dev build reports". + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: "data.status == 'draft'" }]); + // Verdict: the constant-false still hides, exactly as documented. + expect(shownCount()).toBe(0); + expect(allWarnings(warn)).toHaveLength(0); + }); + }); +}); + +/* -------------------------------------------------------------------------- * + * Group 4 — development is unchanged, and says the same words. + * -------------------------------------------------------------------------- */ + +describe('#6038 group 4 — dev and production print the IDENTICAL line', () => { + it('the same fault produces the same message in a development build', async () => { + // Two builds, one message. This is what makes "the `__DEV__` gate now + // decides HOW the fault is detected, not WHETHER it is reported" a fact + // rather than a comment: a production-only message would be a second + // diagnostic to keep in sync, and they would drift. + const { __resetVisibilityPredicateWarnings } = await import('../utils/visibilityDiagnostic'); + const { SchemaRenderer } = await import('../SchemaRenderer'); + const { SchemaRendererContext } = await import('../context/SchemaRendererContext'); + const { RecordContextProvider } = await import('../context/RecordContext'); + const { PredicateScopeProvider } = await import('../hooks/useExpression'); + const { ComponentRegistry } = await import('@object-ui/core'); + ComponentRegistry.register(NAME, Probe as never, { + namespace: 'element', + skipFallback: true, + } as never); + __resetVisibilityPredicateWarnings(); + + const warn = spyWarn(); + render( + + + + + + + , + ); + const devLines = reports(warn); + expect(devLines).toHaveLength(1); + expect(devLines[0]).toContain(TYPE); + expect(devLines[0]).toContain(FAULT_BARE); + // Dev still fails open too — the verdict is one behaviour, not two. + expect(shownCount()).toBe(1); + ComponentRegistry.unregister?.(NAME, 'element'); + + // Restore before spying again: `vi.spyOn` on an already-spied method hands + // back the SAME spy, so a second `spyWarn()` here would carry the dev call + // into the production reading and every count below would be off by the + // lines this half already made. (Measured — it is how this case first + // failed, at 2 lines instead of 1.) + vi.restoreAllMocks(); + + // …and the production graph's line for the same fault, word for word. + let prodLine = ''; + await inProduction((mount) => { + const prodWarn = spyWarn(); + mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + const lines = reports(prodWarn); + expect(lines).toHaveLength(1); + prodLine = lines[0]; + }); + expect(prodLine).toBe(devLines[0]); + }); +}); diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index be8ea1124b..559790ede3 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -20,6 +20,20 @@ export * from './element-data-source/ElementDataSourceGate.js'; // i18n utilities export { resolveKeyedI18nLabel } from './utils/i18n.js'; +// Node-gate predicate diagnostics. Exported at the package entry (objectui#6038) +// so every surface that evaluates a node `visibleWhen` reports a fault through +// ONE reporter and ONE dedupe `Set` — `page:tabs` item predicates live in +// `@object-ui/components`, which depends on this package. A second copy of the +// reporter would mean a second rate limit, and the same broken predicate would +// then be entitled to one line per package instead of one line, which is the +// property the 2026-08-25 ruling asked for. +export { + reportUnresolvableVisibilityPredicate, + formatUnresolvableVisibilityMessage, + UNRESOLVABLE_VISIBILITY_PREFIX, + __resetVisibilityPredicateWarnings, +} from './utils/visibilityDiagnostic.js'; + // Write-error surfacing utilities (shared by drag-write plugins so a failed // PATCH — e.g. an RLS 403 — is never silently swallowed). export { extractWriteErrorMessage, isPermissionError, extractFieldErrors, classifyLoadError, declaredUserMessage } from './utils/error-message.js'; diff --git a/packages/react/src/utils/visibilityDiagnostic.ts b/packages/react/src/utils/visibilityDiagnostic.ts index f8aa316fe8..62130273cc 100644 --- a/packages/react/src/utils/visibilityDiagnostic.ts +++ b/packages/react/src/utils/visibilityDiagnostic.ts @@ -7,8 +7,9 @@ */ /** - * Dev-build diagnostic: a visibility predicate could not be evaluated - * (objectui#5454, leg 3 of the 2026-08-21 ruling). + * Diagnostic: a visibility predicate could not be evaluated (objectui#5454, + * leg 3 of the 2026-08-21 ruling; production coverage added by objectui#6038, + * maintainer ruling 2026-08-25 option B). * * ## The defect this names * @@ -93,15 +94,34 @@ export function formatUnresolvableVisibilityMessage( * repeat the line. Keyed on the predicate TEXT rather than the schema object: * the same broken predicate authored once and rendered over many rows is ONE * authoring bug, and an object key would report it once per row. + * + * This is the RATE LIMIT the 2026-08-25 ruling requires of the production leg, + * and it is why that leg can be a plain `console.warn`: the ceiling is not "one + * line per render" but "one line per distinct authored predicate", for the + * lifetime of the page. Two properties have to hold together, and a test that + * pins only the first cannot tell a working dedupe from one that suppresses + * everything — so objectui#6038 pins both: N renders of ONE faulting predicate + * emit exactly one line, and a SECOND distinct predicate source still emits. */ const _warnedVisibilityPredicates = new Set(); /** - * Dev-build only; the caller applies the gate, so this module is dead code in a - * production build. `console.warn`, not `error`: the verdict is unchanged and - * the page still renders, so this is a diagnostic about a predicate — not the - * refusal `reportUnevaluatedExpressions` emits once raw source has reached the - * DOM. + * Reports a visibility predicate that could not be evaluated — in DEVELOPMENT + * AND IN PRODUCTION since objectui#6038 (maintainer ruling 2026-08-25, option + * B: "the silence is no longer an accepted property"). + * + * `console.warn`, not `error`: the verdict is unchanged and the page still + * renders, so this is a diagnostic about a predicate — not the refusal + * `reportUnevaluatedExpressions` emits once raw source has reached the DOM. + * + * ## `err` takes a reason, not only an `Error` + * + * The dev caller catches a throw and passes the `Error`; the production caller + * is handed the evaluator's own reason STRING through `EvaluationOptions.onFault` + * (no throw is raised there, because raising one would cost a second + * evaluation). `String(err)` already covered that shape, so both callers reach + * the same `Reason:` text and the same dedupe entry — which is what makes "dev + * and production print the identical line" true rather than approximately true. */ export function reportUnresolvableVisibilityPredicate( type: unknown, From 3893507fc60a1e5ef278adf755b29060b3494000 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 00:41:16 +0000 Subject: [PATCH 2/2] test(react): rewrite the #5454 production-silence pin to the ruled property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- ...emaRenderer.nodeGateDataPredicate.test.tsx | 11 ++++++++-- ...Renderer.visibleWhenRecordBinding.test.tsx | 22 +++++++++++++++++-- .../react/src/utils/visibilityDiagnostic.ts | 20 ++++++++++++++--- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx b/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx index 66ce91480b..5bf006c48a 100644 --- a/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx +++ b/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx @@ -399,8 +399,15 @@ describe('#5687 group 3 — the `record.*` bucket is NOT this card, and cannot b describe('#5687 group 4 — production is untouched', () => { it('a PRODUCTION build reaches the SAME verdicts and prints nothing', async () => { - // The diagnostic is dev-only by the same `__DEV__` gate objectui#5454 put - // in front of the probe. A gate that changed the ANSWER would be a fork. + // THIS leg — objectui#5687's adapter-only `data.*` report — is dev-only, + // and stays so: its own ruling (2026-08-22, option A) scoped it there, and + // it does not describe a fault at all (the predicate evaluated perfectly, + // against the wrong object). The SIBLING leg no longer is: objectui#6038 + // took the `__DEV__` gate off `reportUnresolvableVisibilityPredicate`, so + // a FAULTING predicate is reported in production too. That is why every + // assertion below filters on `ADAPTER_ONLY_DATA_PREDICATE_PREFIX` rather + // than counting raw `console.warn` calls. A gate that changed the ANSWER + // would be a fork, and none of these verdicts moved. // // The dynamic import lives in the test BODY, not a hook: it has to read // module state that only exists after `resetModules` + `stubEnv`, which is diff --git a/packages/react/src/__tests__/SchemaRenderer.visibleWhenRecordBinding.test.tsx b/packages/react/src/__tests__/SchemaRenderer.visibleWhenRecordBinding.test.tsx index af90f8531c..4e8e956a12 100644 --- a/packages/react/src/__tests__/SchemaRenderer.visibleWhenRecordBinding.test.tsx +++ b/packages/react/src/__tests__/SchemaRenderer.visibleWhenRecordBinding.test.tsx @@ -364,8 +364,26 @@ describe('#5454 leg 3 — an unresolvable predicate is loud, and its verdict is cleanup(); mountProd({ hidden: cel('record.nope.deeper == 1') }, undefined); expect(shown()).toBe(false); - // ...but with none of this module's diagnostic, which is dev-only. - expect(warn.mock.calls.map(c => String(c[0])).filter(m => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX))).toHaveLength(0); + // ...and, since objectui#6038, WITH this module's diagnostic — one line + // per distinct faulting predicate source, in production too. + // + // This assertion used to read `toHaveLength(0)`, and the change is the + // whole of the maintainer's 2026-08-25 ruling (option B): "A is rejected + // — the silence is no longer an accepted property." The pin is rewritten + // rather than deleted, because what it was really guarding is the half + // that did NOT move: every verdict above is untouched, and the branch is + // still the SINGLE-evaluation one (production never pays for the + // `throwOnError` probe — `EvaluationOptions.onFault` reports the fault + // the evaluator had already caught). + // + // TWO lines, not one: the two mounts above carry two DIFFERENT predicate + // sources (`record.status == 'in_review'` on `visibleWhen`, and + // `record.nope.deeper == 1` on `hidden`). One line each is the dedupe + // working; one line total would mean it had swallowed the second. + const produced = warn.mock.calls.map(c => String(c[0])).filter(m => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX)); + expect(produced).toHaveLength(2); + expect(produced.some(m => m.includes("record.status == 'in_review'"))).toBe(true); + expect(produced.some(m => m.includes('record.nope.deeper == 1'))).toBe(true); core.ComponentRegistry.unregister?.(NAME, 'element'); } finally { vi.unstubAllEnvs(); diff --git a/packages/react/src/utils/visibilityDiagnostic.ts b/packages/react/src/utils/visibilityDiagnostic.ts index 62130273cc..673b2fe521 100644 --- a/packages/react/src/utils/visibilityDiagnostic.ts +++ b/packages/react/src/utils/visibilityDiagnostic.ts @@ -159,8 +159,20 @@ export function __resetVisibilityPredicateWarnings(): void { * dev-only unresolvable-predicate report". What is load-bearing there — and * what the dispatch restated — is the reporter's POSTURE: same module, same * severity (`console.warn`), same dedupe key shape, same dedupe Set, same - * dev-only gate at the same call site, same test-only reset. All of that is - * shared below. The first LINE is not reused, because on this path it would + * gate at the same call site, same test-only reset. All of that is shared + * below. + * + * ⚠️ The two legs stopped sharing that gate's VALUE in objectui#6038, and only + * that. The unresolvable leg now reports in production as well, because the + * 2026-08-25 ruling retired its silence; THIS leg stays dev-only under its own + * 2026-08-22 ruling, and the difference is not an oversight. A fault is a + * predicate that could not be evaluated, and shipping a live gate that has + * stopped biting is the class-1 defect production has to be able to see. This + * leg reports something else: a predicate that evaluated perfectly, against the + * wrong object. Its trigger is a LEXICAL scan of the predicate source with a + * stated false-positive residue (the deliberate-absence idioms below), which is + * a cost worth paying for an author at their keyboard and not for every user of + * every production page. The first LINE is not reused, because on this path it would * state something false: the predicate did not fail to evaluate. It evaluated * perfectly, against the wrong object, and produced a constant. Telling an * author "could not be evaluated" would send them hunting for a syntax error @@ -314,7 +326,9 @@ export function formatAdapterOnlyDataMessage( * objectui#5454 existed to remove, one path further along. * * Sharing the module means sharing the LIFECYCLE, which is the part that has to - * match: one dedupe Set, one reset, one severity, one dev-only gate. The dedupe + * match: one dedupe Set, one reset, one severity. (One gate, too, until + * objectui#6038 — see the prefix constant above for why only the sibling leg + * crossed into production.) The dedupe * key is tagged with this leg's name so the two diagnostics cannot silence each * other for the same (type, key, source) triple — they are different faults, * and a node that faults one way is not evidence about the other.