Uh oh!
There was an error while loading. Please reload this page.
fix(components): stop the sparse-predicate warning blaming hidden: true (#5399) - #5592
Merged
Merged
Conversation
…rue` (#5399) The `[page:header]` missing-field diagnostic ended with a causal sentence that named a mechanism this repo does not own, and named it wrongly: Hidden (hidden: true) fields are stripped from detail payloads server-side, so a predicate gating on one may evaluate to a hide-by-default verdict. `hidden` is a UI concern, not a projection rule. Verified against the framework checkout rather than taken on trust: the spec describes the key as "Hidden from default UI"; ObjectQL's dedicated strip for the `__search` companion documents that the `hidden` / `readonly` / `system` markers are "None of them is a PROJECTION rule" — which is exactly why that one column needed a purpose-built strip; drivers answer a query with no `fields` using `SELECT *`; and metadata-protocol enumerates what the read path does drop (`internal: true` columns and the `__search` companion, nothing else). The only read-side uses of `field.hidden` in the framework are auto-view/auto-form column generation and companion-source eligibility, neither of which removes a key from a record body. So the diagnostic sent authors hunting for a `hidden` flag they would not find, or would find on a field the payload still returns, while the real source of the sparseness — a projected or partial read — went unexamined. The replacement states the fact this surface can see and the consequence it owns: the page bound a payload without those keys, a projected or partial read will not carry them, so the predicate fails closed and the action stays hidden. The measured half of the message (action name, missing fields, predicate source) is unchanged, and what triggers the warning is untouched. Message text only, no behaviour change. The same false claim also sat in this warner's own doc comments and in the comments of the test that pins the message; both are corrected, and the docstring now carries an explicit note against re-attributing the cause. No other call site was swept. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
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-sales
marked this pull request as ready for review
August 21, 2026 16:20
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#5399
The defect
packages/components/src/renderers/layout/containers.tsx— the[page:header]missing-field diagnostic named the action, the missing fields and the predicate
source (all measured, all correct), then closed with a causal sentence:
That cause is false, and it names a mechanism this repo does not own. An author who
read it went looking for a
hidden: trueon the field and found either none, or oneon a field the payload demonstrably still returns — while the real source of the
sparseness (a projected or partial read) went unexamined.
Premise check — confirmed false, not taken on trust
Verified against the framework checkout at
112a8c6, four independent ways. Threeof these are my own reading of the running framework, not a restatement of triage:
packages/spec/src/data/field.zod.ts:1234—hidden: z.boolean().default(false).describe('Hidden from default UI').packages/objectql/src/search-companion.ts:370and its conformance suite:the companion column is declared
hidden+readonly+system, and"None of them is a PROJECTION rule". A query naming no
fieldsreaches thedriver with
ast.fieldsundefined and every driver answers that withSELECT *.The one column that is withheld needed a purpose-built strip rule — which is
only necessary because
hiddendoes not project.packages/metadata-protocol/src/protocol.ts:8073names the complete set:omitInternalFieldsforinternal: truecolumns, andstripSearchCompanionFromReadfor the__searchcompanion.hiddenis not in it.field.hiddenare something else entirely.protocol.ts:7020/:7053are auto-view and auto-form column generation (a UIconcern, exactly as the spec describe says), and
search-companion.ts:136is aneligibility gate for what may be denormalized into the companion. Neither removes
a key from a record body.
Triage's live-server probe (both
hidden: truebusiness fields present in the payload,__searchthe only absent key) is consistent with all four and I did not re-run it.The fix
Message text only. The measured half is untouched; the causal sentence is replaced with
the fact this surface can actually see plus the consequence it does own:
No behaviour change. What triggers the warning is byte-identical; the only changed
executable line in the diff is the string literal inside
console.warn.The two things the card asked me to check
1. Does a test assert this message? Yes —
page-header-actions.test.tsx:842asserts/not present in the record payload/andtoContain('secret_level_2358'). Both sit in themeasured half of the message, which is unchanged, so no assertion went red. Confirmed
by the suite passing, not by inspection alone.
2. Is the same claim repeated elsewhere? Yes, in four places, all of them annotations of
this same call site — not other warners, not prose docs. Because they document the exact
function being corrected, leaving them would ship the fix underneath a comment restating the
falsehood, and the next reader would revert the message to match. All four are inside the two
files this card's scope already names, and each is called out here rather than swept silently:
containers.tsx:977— the paragraph introducing the warner ("fields stripped from thepayload server-side")
containers.tsx:983— the docstring ofwarnMissingRecordFieldsitself ("the server stripshidden: truefields from detail payloads"); it now also carries an explicit note againstre-attributing the cause, so this cannot quietly regress
page-header-actions.test.tsx:828and:837— the comments of the test that pins the messageNothing outside those two files was touched. The sweep found no other site carrying this
claim: every other "server strips" hit in the repo is about write stripping (readonly
fields dropped from a save), which is a different and real mechanism. One adjacent site worth
recording but not a duplicate:
apps/console/src/pages/system/ApprovalsInboxPage.rawPayloadGate.test.tsx:39explicitly saystrimming a payload by
hidden: trueis not asserted there and is tracked elsewhere — whichis consistent with this finding rather than a repeat of it. No new issue filed; nothing found
that needs one.
Verification
There is no meaningful ablation for a message-text change and I did not manufacture one —
mutating a string literal to watch an assertion flip would be theatre, and the assertion that
matters deliberately does not cover the changed sentence.
All gates run at the tree now committed as
059cb503d(working tree was clean at commit),exit codes captured before any pipe, each gate's own verdict line quoted:
pnpm --filter @object-ui/components type-checkVERDICT command-exit 0(script echoedtsc --noEmit && tsc -p tsconfig.test.json, so not a zero-match)pnpm --filter @object-ui/components lint896 problems (0 errors, 896 warnings)— all pre-existing warningspnpm exec vitest run packages/components/(repo root)Test Files 174 passed (174)·Tests 1576 passed (1576)node scripts/check-control-bytes.mjscheck-control-bytes: OK (scanned 4645 tracked text file(s); skipped 85 binary)node scripts/check-changeset-presence.mjs2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjsNo changeset declares a major bump.node scripts/check-changeset-fixed.mjsAll workspace packages are in the changeset fixed group.Re-derived from the actual changed paths rather than trusting the dispatch list, and ran the
additional gates that could plausibly be implicated:
check-i18n-call-site-keys,check-i18n-en-drift,check-i18n-dead-keys,check-action-forward-parity,check-lint-coverage,check-type-check-coverage,check-doc-links— all exit 0.Declared narrowing: lint was run package-scoped rather than repo-wide. It cannot hide a
failure for this diff —
pnpm --filter @object-ui/components lintrunseslint .across thewhole package, and both changed files are inside
packages/componentsand appear by name inits output. The dependency closure was built first
(
pnpm --workspace-concurrency=2 --filter "@object-ui/components^..." build, exit 0), so thetype-check read fresh
.d.tsrather than stale or absent output. CI runs the full farm.Generated by Claude Code