Uh oh!
There was an error while loading. Please reload this page.
fix(react): make a node-gate data.* predicate the adapter cannot answer loud (#5687) - #5757
Merged
os-zhuang merged 1 commit intoAug 23, 2026
Merged
Conversation
…swer loud (#5687) At the node visibility tier `data` is the data-source ADAPTER — what `${data.total}` in a props bag resolves against — and it has never been the row. A predicate written with the deprecated `data.*` spelling therefore resolves `undefined == 'draft'`, which is a clean `false`: the block is hidden on every row, and because nothing throws, the unresolvable-predicate reporter added for #5454 never fired. Measured on this base, the same predicate written as a `{ dialect: 'cel' }` envelope DOES throw and IS already reported. So the card's premise ("this path never throws") holds for the bare-string and `${…}` template dialects only — whether an author heard about the identical mistake depended on which dialect they wrote it in, which is the arbitrariness #5454 existed to remove. Per the maintainer ruling of 2026-08-22 (option A): no verdict changes and no interpolation changes. The evaluator's answer is returned exactly as computed; this adds a dev-only `console.warn` on the non-throwing branch, through the same module, dedupe Set and lifecycle as the #5454 reporter. The trigger is a `data.*` read the bound adapter answers with `undefined`, not the spelling: a genuine adapter read, a canonical `record.*` predicate and a correctly-hiding gate all stay silent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
Contributor
✅ 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-zhuang
marked this pull request as ready for review
August 23, 2026 05:06
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang
deleted the
claude/issue-5687-node-gate-data-predicate-report
branch
August 23, 2026 05:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5687
Maintainer ruling, 2026-08-22, option A: the node tier keeps its documented
data= adapter semantics; the deprecated spelling's constant-false gets loud. No verdict changes, no interpolation changes — #5330's row binding does not extend here, and the docblock's "silent interpolation change nobody asked for" stays refused. What lands instead is the #5454 reporter posture, extended to this non-throwing path.The defect, re-measured on this base (not inherited from the card)
The card measured on
3b147a367; this branch is off3ece13e33. All three of its load-bearing facts were re-checked, and one of them has changed:3ece13e33SchemaRendererhoists everyproperties.*value onto the node, soproperties.visiblearrives asschema.visibledata: dataSource, the adapter, defaulting toNO_DATA_SOURCE = {}data: dataSourceis still written last, after the ambient spread and afterrecordMeasured directly against
@object-ui/core's built evaluator withdata: {}:So the CEL envelope already reports today, through
reportUnresolvableVisibilityPredicate. The bare-string and template dialects do not. That is precisely the arbitrariness #5454 existed to remove — "whether an author heard about their own typo depended on which dialect they happened to write it in" — surviving one path further along. The card's repro shape is the bare string, so the gap is real and this is its narrowest closure.What changed
Two files, one call, ~200 lines of which most are the reasoning.
packages/react/src/utils/visibilityDiagnostic.ts—reportAdapterOnlyDataPredicate()beside the existing reporter. Same module, same dedupeSet, same__resetVisibilityPredicateWarnings(), sameconsole.warnseverity, same dev-only gate at the same call site. The dedupe key is tagged with this leg's name so the two diagnostics cannot silence each other for one(type, key, source)triple — they are different faults.packages/react/src/SchemaRenderer.tsx— one call, on the non-throwing branch only:verdictis returned exactly as computed. "No verdict changes" is true by construction here, not by review.The trigger, and why it is this one
The ruling's trigger is not references
data.— a genuine${data.total}adapter read must stay silent, and that half is what makes the loud half mean anything. Four candidate definitions were measured, not argued:data.total > 0(adapter hastotal)data.at allfalsedata.total > 100against{total:99}fires, and so does a correctrecord.status == 'draft'against a non-draft rowundefined@object-ui/core's evaluator to expose its operands — an engine change for a diagnostic on a card that dissolvesdata.*read the bound object does not answerThe last one is also structurally incapable of firing on the
record.*bucket (#5401 → #5454): a predicate with nodata.root has nothing for it to look up. The two adjacent cards cannot bleed into each other through this code.Residue, stated rather than hidden: deliberate absence idioms (
data.status == null,!data.status) are reported. They are not really false positives at this tier — an adapter with nostatusmakes those constants too, just constant-true — but they are shapes an author could have meant.The scan is lexical, so string literals are stripped before it runs (
record.status == 'data.status'reads nothing and says nothing). Not hypothetical:examples/schema-catalog/src/schemas/components-complex-scroll-area/code-preview.jsoncarriesdata.featuresinside a JS snippet in acontentstring.The pin is a triple
packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx, 18 cases. Drop any leg and the file goes green on an implementation that reports everything, or nothing.properties: { visible: "data.status == 'draft'" }warns, names the node/key/predicate and the unanswered read, and still hides, on both rows;record.*stays silent — opposite verdicts on the two row polarities, zero diagnostics;${data.total}still interpolates to99in a props bag, anddata.total > 0as a visibility gate is silent on both polarities.The case that decides which discriminator is implemented is 2c: same predicate text, same
falseverdict, adapter now has the key → silent. It holds the source and the verdict fixed and moves only the adapter.Also pinned: the non-negated
hiddenleg keeps its inverted answer;metadata.*andrecord.data.*are notdata.*; a CEL envelope readingdata.*is reported once, by #5454, never twice; production takes the single-evaluation branch, reaches the same four verdicts and prints nothing.Ablation — direction predicted before running, both legs restored under
trap … EXIT INT TERMBoth mutations were confirmed on disk by anchored counts in both directions (the deleted text at 0, an injected marker at 1) before any test was read. The mutated subject is imported by the tests via a relative source path (
../SchemaRenderer), not through a packageexports→dist, so no rebuild is involved and no stale artifact can make a mutation read as green.A — delete the reporter call. Predicted: red exactly the positive cases, green every silence case and every verdict assertion. Observed: 6 red, 32 green, and the entire #5454 pin file green.
B — delete the
unresolved.length === 0silence guard (i.e. report on anydata.mention). Predicted: red the silence cases that actually reach the reporter. Observed: 5 red — 2b, 2c, 2d, 3c, 3d — group 1 and production untouched.The two brackets are what make the discriminator checkable: A kills a "report nothing" implementation, B kills a "report everything" one.
Ablation B found a bad control, which is the point of running it. Case 3c (
metadata.*is notdata.*) survived B on the first run. Measured cause: withmetadataunbound the predicate throws (metadata is not defined), lands in the #5454 catch, and never reaches this leg at all — so it was green for a reason unrelated to the boundary it claims to check.metadatais now bound in that case's scope; it takes the non-throwing path, and it dies under B as it should.Controls that correctly survive both ablations, and why: 3a/3b (
record.*) — neither mutation touches a path without adata.root, which is exactly the evidence the two cards don't bleed; 3e (CEL envelope) — it throws first, so it is #5454's case in both worlds; group 4 (production) —__DEV__short-circuits ahead of both mutations.Verification
Union run on
06561bde3, the final commit, after the rebase.pnpm exec vitest run packages/react/→ 52 files, 706 tests, 0 failures.pnpm exec vitest list packages/react/collects 706 cases, 18 of them this file's — the default reporter names no passing file, so inclusion is read from vitest's own collection rather than assumed.pnpm exec vitest run packages/react/ packages/components/ packages/plugin-grid/ packages/types/→ 363 files, 3635 tests, 0 failures.pnpm --filter @object-ui/react type-check→ exit 0.pnpm --filter @object-ui/react lint→✖ 347 problems (0 errors, 347 warnings); all 347 are pre-existingno-explicit-any, and none sits in either of this diff's two hunks inSchemaRenderer.tsx(warning lines vs. hunk ranges compared).check-control-bytes✅ OK (scanned 4801 tracked text file(s))·check-changeset-presence✅ … declares 1 changeset(s)·changeset-fixed·changeset-no-major·lint-coverage✅ 46/46 packages linted, 0 with outstanding errors·type-check-coverage·phantom-deps·self-import·esm-specifiers·spec-symbols·action-forward-parity— all exit 0, quoting each gate's own verdict line.check-eager-closurefails with its own "No eager-closure report … the console was not built … This is a broken gauge, not a passing budget" — the known worktree gauge. It cannot move here regardless: the module specifiers imported bySchemaRenderer.tsxare byte-identical before and after (10 before, 10 after, empty diff) — the change widens an existing named import rather than adding an edge.Declared narrowing:
packages/app-shell/andapps/console/exceed the 10-minute foreground cap as whole suites, so the run was narrowed to the actual blast surface — every file carrying adata.*visibility predicate —packages/app-shell/src/views/metadata-admin/+apps/console/src/components/FormPage.fieldSpec.test.ts→ 195 files, 1979 passed, 1 skipped. The residual risk a newconsole.warnposes is a test asserting exact console call counts; the vitest config declares noonConsoleLog/failOnConsole(grep exit 1, controlled against a term known to be in the file), so an unexpected warn cannot fail a suite by configuration. CI runs the full farm regardless.Deprecated-spelling inventory (#5687's open question, answered)
The triage analysis flagged what it could not see: whether node-tier
data.*predicates exist in real metadata. Controlled sweep ofexamples/,apps/,content/,packages/:data.*visibility predicates in non-test source: zero.data.*visibility predicates in the repo —packages/app-shell/src/views/metadata-admin/anchors.ts— are at the SchemaForm field tier, whereevaluatePredicate(visibility, { data: value })bindsdatato the form draft values. A correct, different binding on a different evaluator, which this change cannot reach.Controls for those zeros, since a zero hit is not a reading: the same shape spelled
record.*→ 246 hits; the visibility keys at all → 1273 hits;data.anywhere → 744 hits; and the sweep demonstrably entersexamples/schema-catalog(5 visibility-key hits there, allrecord.*/current_user.*). Nobody is standing in the blast radius — useful for whoever owns #5330's deprecation window.Filed, not fixed here
#5756 — a
${…}predicate written insidepropertiesis interpolated to a boolean before the hoist, so it reaches the chain as a barefalseand no gate-side diagnostic can ever see it (this one or #5454's). It hides the block on every row, silently. Measured, and pinned in this PR as an explicit known limit rather than papered over. Closing it means adding a diagnostic inside the interpolation loop — the surface this ruling fences off — and it carries two real choices, so it belongs to triage, not to this card.One deliberate reading of the ruling, flagged for review
The ruling says this path "emits the dev-only unresolvable-predicate report". I read that as the reporter — its module, severity, dedupe and lifecycle, all of which are shared verbatim — and gave this leg its own first line (
ADAPTER_ONLY_DATA_PREDICATE_PREFIX) rather than reusing'A visibility predicate could not be evaluated'. On this path that sentence would be false: the predicate evaluated perfectly, against the wrong object, and produced a constant. Telling an author "could not be evaluated" sends them hunting for a syntax error that is not there. Dispatch guidance was that posture matters more than message text, so this is where the flexibility was spent — and if the literal string was meant, it is a one-line change.Temporary by design
This is loudness, not a feature. It dissolves when #5330's deprecation window for the
data.*row spelling closes; the changeset says so.Generated by Claude Code