Uh oh!
There was an error while loading. Please reload this page.
fix(react,app-shell,components): choose the visibility-fault advice paragraph by scope tier - #6495
Merged
Merged
Conversation
…y scope tier
`formatUnresolvableVisibilityMessage` closed every report with the NODE tier's
roots — `record`, `current_user`, `page.<var>`. Since the app-shell chrome gate
was wired onto the same reporter, that paragraph also printed for an evaluator
built from `{ current_user, user, ctx: { user }, os: { user }, app, data,
features }`, which binds neither `record` nor `page`. A nav author whose
`visible` predicate faulted was sent to check two roots that cannot exist at
their tier, while the roots that do exist went unnamed.
Both exported functions now take an optional `PredicateScopeTier` selecting the
closing paragraph, and all three in-repo call sites state their tier rather than
inherit the default. Each tier's root set is derived from the code that builds
the bag: `SchemaRenderer` and the `page:tabs` item gate bind the three roots the
spec declares for a page component, `ExpressionProvider` binds the ADR-0068
identity aliases plus `app` and `features`.
The concrete root names are deliberately kept at both tiers. Generalising the
copy would have made one paragraph true everywhere and useful nowhere: an author
who mistyped a root needs to know which roots exist at their tier.
No verdict moves, on any surface. Everything above the advice paragraph — the
prefix, the node, the gate key, the predicate source, the engine's reason and
the "gate did NOT bite" sentence — is unchanged, and so is the objectui#6038
dedupe key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4qContributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 26, 2026 06:04
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6487
formatUnresolvableVisibilityMessageclosed every report with the node tier's roots. Since objectui#6443 wired the app-shell chrome gate onto the same reporter, that paragraph also printed for an evaluator that binds neither of the two roots it names — so a nav, area or fieldvisiblepredicate that faulted told its author to go and checkrecordand page state, at a tier where neither exists, while the roots that do exist went unnamed.Both exported functions now take an optional sixth argument, a
PredicateScopeTier, selecting the closing paragraph. No verdict moves on any surface, and everything above that paragraph is unchanged.The root sets were derived from the code that binds the scope, not from the card
The card asked for this and it was worth doing — one of its numbers does not survive the measurement.
'page-component'—packages/react/src/SchemaRenderer.tsx:585:record,current_userand page state — the three roots@objectstack/spec'sui/page.zod.tsdeclares for the tier.page:tabsitem predicates sit here too, measured rather than assumed:packages/components/src/renderers/layout/containers.tsx:451builds the same three roots (plus the row spread flat anddataaliased to the row — breadth that is undeclared on both surfaces, so it is advertised on neither). This paragraph is byte-for-byte what it was.'app-shell'—packages/app-shell/src/providers/ExpressionProvider.tsx:59:No
record, nopage. The line now namescurrent_userwith its three ADR-0068 alias spellings,app, andfeatures.Where the measurement disagrees with the card
The card says five bound roots go unnamed. Measured, six are:
user,ctx.user,os.user,app,features— anddata, which the card's own quoted context bag contains but its "go unnamed" list omits.datais still deliberately not advertised, for a reason the card did not state: every mount ofExpressionProviderin this repo passesdata={{}}(console/AppContent.tsx:925,views/RecordFormPage.tsx:270) or omits it (apps/console/.../InternalFormRoute.tsx:81). Naming it would point an author at a root that answers nothing. So the card's advice is right and its count is off by one; the difference is recorded here rather than smoothed over.Why parameterised, and why the copy was not generalised
Three callers, and the tiers genuinely differ —
recordand page state are bound at one and absent at the other. Generalising would have made one paragraph true everywhere at the cost of making it useful nowhere: an author who mistyped a root needs to know which roots exist at their tier. A test cell pins that refusal directly (still names CONCRETE roots), and the ablation below shows the generalisation turning both tiers red.Deducing the tier from the existing
typeslot was also refused:typecarries the schema node type at the node tier and a surface label at the other two, andpage:tabsis a real registry key — so apage:tabsnode faulting its ownvisibleWheninsideSchemaRendereris indistinguishable bytypefrom one of its item predicates. An explicit argument cannot collide that way.tieris not in the dedupe key: the objectui#6038 rate limit is untouched. Adding it could only loosen the limit, and loosening needs onetypeshared across two tiers — which production cannot produce, since the app-shell site'stypeis the constant'app-shell:visible'.Published-surface check (clause ②) — reachable, and the shape grew
Followed to the built entry rather than inferred from an
exportkeyword.@object-ui/react'sexportsmap declares exactly one entry ("." -> ./dist/index.d.ts), andpackages/react/src/index.ts:30-35re-exports both functions by name (no wildcard hop is involved for this module — grep confirmsutils/visibilityDiagnostic.jsis imported from exactly two places, the entry andSchemaRenderer.tsx).Yes, both are reachable from the package entry, and yes, the shape grew. From the rebuilt
packages/react/dist/utils/visibilityDiagnostic.d.ts:Nothing breaks: the argument is optional and defaults to
'page-component', and a cell pins that the five-argument call is byte-identical to the explicit node-tier call. The default is a compatibility shim for callers outside this repo — all three in-repo call sites state their tier.Tests
Everything below was run at
df9de7836, the final commit. Vitest resolves@object-ui/reacttopackages/react/src(vitest.config.mts:262), so the suites read source and no rebuild sits between a source edit and a test verdict — that is why the ablations below rebuild nothing and why saying so is part of the claim.packages/react/(whole package)Test Files 58 passed (58)/Tests 854 passed (854)packages/app-shell/src/providers/Test Files 10 passed (10)/Tests 73 passed (73)page:tabs/ page-container component suitesTest Files 7 passed (7)/Tests 49 passed (49)New:
packages/react/src/utils/visibilityDiagnostic.adviceTier.test.ts(17 cells) — the tier matrix, with load-bearing assertions placed only where the tiers disagree.current_useris named at both tiers, so the cell asserting it is kept but labelled degenerate, because on its own it is green against the defect too.Extended, one end-to-end cell per real call site, each asserting a fault reached the reporter in the same run so none can pass on a reporter that stopped reporting:
ExpressionProvider.visibleFaultDiagnostic.test.ts,page-tabs-visible-when-fault-warning.test.tsx,SchemaRenderer.visibleWhenRecordBinding.test.tsx.Ablations — direction predicted before each run, mutation proven on disk, restore proven
Each leg pinned its restore to the literal SHA
df9de783675bbf3b27aa90124f25086e7cb7e00c(neverorigin/main, which is shared across worktrees), verified the mutation with agrep -con the exact injected and deleted text, and proved the restore withgit diff HEAD --quiet->0.A. Revert the app-shell call site to
'page-component'(literally the pre-fix behaviour). Predicted: the app-shell tier cells red, everything else green. Measured:Test Files 1 failed | 3 passed (4)/Tests 2 failed | 61 passed (63)— red were exactlya faulting nav predicate is NOT told to check ...andit names the roots this provider really binds — including features. The two app-shell cells labelled controls stayed green, as labelled.B. Delete the concrete root names from both paragraphs (the fork the card fenced off). Predicted: both tiers red, at all three call sites. Measured:
Test Files 4 failed (4)/Tests 14 failed | 49 passed (63).C. Type ablation — pass a bogus tier at the app-shell call site. Predicted red, and specifically that
@object-ui/app-shell'stscreads the rebuilt.d.tsrather than a cached five-argument one. MeasuredExit status 2:The error names the new union by name, so the green cross-package type-check above is a measurement of the new signature, not of a stale declaration.
Gates
Quoted from each gate's own verdict line; exit codes captured before any pipe.
pnpm --filter @object-ui/react|@object-ui/app-shell|@object-ui/components run type-check—EXIT=0for all three (tsc --noEmit && tsc -p tsconfig.test.json, so the new tests are in the program:--listFilesreports the new file present, 1 match).check:control-bytes—check-control-bytes: OK (scanned 5369 tracked text file(s); skipped 85 binary).check:readme-exports— first run exited 1 on its own non-vacuity guard (36 unbuilt, census collapsed belowFLOORS), which is a missing prerequisite and not measured, not red. After the build the workflow itself runs (turbo run build --filter='./packages/*'):check-readme-exports: OK (... 378 self-imports judged (378 real, 0 wrong-path, 0 fabricated); 3257 export symbol(s) read from 37 of 40 package(s) ... 0 unbuilt ...). No baseline or floor was touched.check:self-import,check:phantom-deps,check:vi-mock-specifiers— allEXIT=0with a real census.check-changeset-presence—8 source file(s) of 3 released package(s) changed, and this change declares 1 changeset(s);check-changeset-no-major—No changeset declares a major bump.Lint, narrowed — and the narrowing measured
eslinton the 10 changed files: 0 errors, 122 warnings, and 0 of those warnings land on a line this commit added (checked by intersecting the--format jsonmessage line numbers with the commit's added-line ranges). Every warning is pre-existing@typescript-eslint/no-explicit-anyin files this PR only appends to. The lint job blocks on errors only, by its own workflow comment.The narrowing is a measurement, not an omission:
ESLint#isPathIgnoredover every tracked file with a lintable extension: 4462 candidates, 3818 not ignored. That is what a full run checks.--format jsonarray.eslint.config.jsdeclares noparserOptions, noprojectand noprojectService, so type-aware linting is off; and none of the 8 customobject-ui/*rules reads the filesystem (grep -l readFileSync|require('fs')|from 'fs'|from 'node:fs'|globSync eslint-rules/*.js-> no non-test match). Every rule judges one file's own AST, so a file this PR does not touch cannot change verdict because of this PR.Filed, not fixed
#6493 (
finding, unassigned) —evaluateVisibilityis reached by three evaluators, not one.views/RecordFormPage.tsx:183andconsole/AppContent.tsx:671build ad-hocExpressionEvaluators binding only{ user, app, data }, bypassing the provider they sit beneath, socurrent_user, the alias spellings andfeaturesare unbound for an object field'svisiblethere — and the app-shell paragraph this PR lands over-promises at those two sites. The repair belongs at the producer, and bindingcurrent_useris a behaviour change (objectui#6110's changeset documents inert gates going live and asks for a metadata audit), so it needs its own card and its own changeset warning rather than a rider here. Not a duplicate of objectui#6262, which is the provider being under-fed on/forms/:namerather than bypassed.Note for #6445
objectui#6445 adds a third caller — the
disabled/disabledOngate inpackages/react/src/SchemaRenderer.tsx. This shape barely constrains it: that gate uses the sameevaluatoras the visibility chain, so its roots are identical and it passes'page-component', or nothing at all. What #6445 changes is the first paragraph's polarity, which is per-message and not per-tier, so the two cards do not collide in the advice table. A cell in the app-shell suite pins that the first paragraph is unchanged by this card, so if #6445 does re-tier it, that cell is where it will say so.Generated by Claude Code