Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-protocol): honour hidden on getUiView's list priority pass - #13329
Conversation
…pass `FieldSchema.hidden` is declared "Hidden from default UI" and `getUiView` is the default UI, but its list branch applied `!fields[k].hidden` to the fill pass only. A field declared `hidden: true` was therefore withheld for eight of nine spellings and served — with its authored label — for the ninth, whenever the author happened to name it one of `name`, `title`, `label`, `subject`, `email`, `status`, `type`, `category`, `created_at`. The `form` branch of the same function already filtered hidden uniformly, so two branches of one producer disagreed about what `hidden` means. Bring the priority pass to the side that already honoured the declaration. This restores a stated invariant; it does not redesign what `hidden` governs and adds no way to declare a column list. The pin drives three arms in one case — a hidden priority-named field, a hidden non-priority field, and a visible priority field as the control that stops "nothing is emitted" from passing vacuously — then sweeps all nine priority names, and pins the form branch's exact output as unchanged.
📓 Docs Drift CheckThis PR changes 1 package(s): 6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 23ed64ff46d0552d4be93fb188eb382f9f92d999 && git checkout 23ed64ff46d0552d4be93fb188eb382f9f92d999
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0ae9e1e16e34db799cb717ad254165a2b4cef243 21aee57fa674e1abab115d0d18b2cf129de4848a && git checkout -B drift-repro 0ae9e1e16e34db799cb717ad254165a2b4cef243 && git merge --no-ff 21aee57fa674e1abab115d0d18b2cf129de4848a
node scripts/docs-audit/affected-docs.mjs --json 0ae9e1e16e34db799cb717ad254165a2b4cef243
|
os-zhuang
commented
Aug 30, 2026
PM review — accepted for landing (held in draft until CI is complete and green)Reviewed at head ScopeThree files: the changeset, Clause-② does not bind, and it is worth being precise about why rather than resting on the narrative: the path limb is mechanical and does not fire ( The fixOne line: I verified the load-bearing claim myself rather than accepting the report. The argument is "two branches of ONE producer disagreed about what
So this is restoring a stated invariant, exactly as the in-code comment claims, not a redesign of what The test, and one finding inside itThe three-arm structure is right, and the third arm is the one that matters: ⭐ The ablation produced a finding, not just a red. The direction was predicted in writing and deliberately mixed, and the two predicted-GREEN legs are the informative half: the form-branch pin stayed green (it measures the branch this card does not touch), and — the real one — the pre-fix body parsed GREEN against Mutation proven on disk before any reading (grep counts plus blob ⭐ The discarded first measurementThe blast-radius sweep returned This is the single most important thing in the report. A zero from an instrument that was never shown to be able to produce a non-zero is not evidence, and reporting the first pass would have been a defensible-looking lie.
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#13259
FieldSchema.hiddenis declared "Hidden from default UI", andgetUiViewis the default UI — it is the producer behindGET /api/v1/ui/view/:object/:type. Its list branch chose columns in two passes and applied the visibility filter to the second one only, so a field declaredhidden: truewas withheld for eight of nine spellings and served, with its authored label, for the ninth: whenever the author happened to name it one ofname,title,label,subject,email,status,type,category,created_at.The
formbranch of the same function already filtered every hidden field uniformly, so two branches of one producer disagreed about whathiddenmeans. This brings the priority pass to the side that already honoured the declaration.The change
One line in
packages/metadata-protocol/src/protocol.ts:plus the comment block explaining why the filter belongs on both passes. Per the triage ruling this is option 1 — restoring a stated invariant. ⛔ Option 2 ("make the priority pass respect a declared column list") is a new authoring capability, needs its own card and the maintainer's decision, and is not attempted here.
Because
searchableFieldsis derived fromcolumns, a hidden priority-named field also stops being offered as a search affordance. That is a transitive consequence of the same line and is pinned.Clause-②: no
The path limb does not fire: the diff lands in
packages/metadata-protocol/src/, notpackages/spec/src/**, and touches no*.zod.tscontract schema or the error-code ledger. On the content limb both halves read no:getUiViewis a pure derivation of a presentation payload from already-validated metadata. It parses nothing, validates nothing, rejects nothing. The set of authorable spellings is byte-identical either side of the change.hiddenis the contract being honoured, not altered. Had the repair gone the other way it would have added an authoring capability and I would be declaringClause-②: yes.Verification
New pin:
packages/metadata-protocol/src/protocol.ui-view-hidden-columns.test.ts(7 cases).Three arms in one case, and what each rules out:
status— hidden and a priority namebeta_secret— hidden, not a priority namename— a priority name, not hiddenEach case also asserts the name-agnostic form — no emitted column is declared hidden, computed from the fixture — so a tenth priority name added later without the filter fails even though the test's restated list would not know about it.
The class, not the field. A sweep drives all nine priority names hidden at once, plus one visible non-priority field, and pins the answer as exactly
['visible_note']— a specific non-empty set rather than "empty".Form-branch agreement, pinned two ways. One case drives both branches from the same fixture and asserts they withhold the same set. A second pins the form branch's exact output (
['name', 'plain_note']) — if the repair had over-reached into the branch this card does not touch, that is what says so.Reverse verification (ablation). Direction predicted in writing before the run, deliberately mixed so both halves are falsifiable. Mutation: revert
protocol.tsto the merge base. Proven on disk before reading anything — injected filter grep count 0, pre-fix line count 1, on-disk blob3e5f6838differing from HEAD blob71c528b6. Restore leg namesHEADexplicitly (a baregit checkout -- pathrestores from the index, which the mutation had already written) and is verified by observed state, not an exit code:git diff HEADempty,git status --porcelainempty, on-disk blob back to71c528b6exactly, both anchored greps back to the fixed state.Predicted RED and observed RED — the three-arm case, the label case, the searchable case, the nine-name sweep, the agreement case:
Predicted GREEN and observed GREEN under the same ablation: the form-branch pin (it measures the branch this card does not touch), and the schema-conformance case. ⭐ That second one is a finding in its own right — the pre-fix body parsed GREEN against
GetUiViewResponseSchema, so the existing conformance test could never have caught this defect. That is why a dedicated pin had to exist rather than leaning on conformance.No
distleg is involved in the ablation: the test imports./protocol.js, a relative specifier that vitest resolves to the source file, and the package declares no vitest config and no alias.Checks — all run on the final commit
21aee57fa, quoting each gate's own verdict lineRepo-wide, not narrowed:
Type-check ratchet (both halves, after building the closure exactly as
lint.ymldoes):Measured directly as well, because
@objectstack/metadata-protocolcarries a shrink-only DEBT entry frozen at 63 and itstsconfig.jsonincludessrc/**/*, so a new test file is inside that program:tsc --noEmit -p tsconfig.jsoncounts 63 — the frozen number exactly — with 0 errors inprotocol.tsand 0 in the new test file. The per-file distribution matches the ledger note (27 inprotocol.stored-migration.test.ts, 10 inseed-loader-multi-value-reference.test.ts), confirming the same population the ratchet freezes.Gate family derived from the real change set with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(no hand-built diff list) — all green:Two report NOT MEASURED, in their own words, and neither is a red:
scripts/check-test-completeness.mjs(exit 3) — "There is no local log to hand it, so the local reading for this gate is NOT MEASURED. ⛔ It is not a red." It needs a savedturbo run testlog that only CI tees.scripts/pm/check-half-states.mjs(exit 3) — "PREREQUISITE NOT MET — the token in the environment is not a valid GitHub credential." A board sweep, unrelated to this diff.Changeset —
minor, with the blast radius measured.changeset/ui-view-hidden-priority-columns.md, gradedminorand declared BREAKING.Why
minorand notpatch. This repo uses exactly two grades — 341patch, 257minor, zeromajor— and 48 changesets state the convention verbatim: breaking changes ship asminorunder the launch-window convention. The twopatchprecedents I read both justify themselves with "the set of accepted metadata is byte-identical", i.e. nothing a consumer observes moves. Here something a consumer observes does move: a column, its label and its search affordance disappear from a published route's body, and the consequence is a removal. Grading itpatchwould keep a behaviour removal out of the changelog an upgrading agent greps.Why not
major. Nothing authorable is removed or renamed, no stored metadata is invalidated, and there is no FROM/TO mapping to carry — the metadata was already correct and now simply takes effect. The ADR-0087 disposition is thereforenot-required (no-migration-prescription), and the gate accepted it on its own terms: "✓ check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition."Blast radius, measured rather than assumed. Every⚠️ The first pass of this walk returned "(none)" with a failed control — it resolved zero keys, because it only knew
hidden: truedeclaration site across all tracked files was resolved to the field key it attaches to.key: {while this repo writes fields askey: Field.textarea({ … }). That zero was discarded, not reported. The corrected walk resolves 22 distinct keys includingprevious_password_hashes,tokenandkey, so its zero is a reading:packages/platform-objects,examples/**orpackages/plugins/**are both hidden and priority-named. The three real hidden fields there are all non-priority names and were already dropped.created_at+hiddenpair in the tree is a@objectstack/objectqlunit fixture that never callsgetUiView(grep count 0).getUiViewreference outside this package is a mock (vi.fn(),undefined as any, a thrower).⇒ No in-repo consumer loses a column.⚠️ That is a measurement of this repo, not of the class — a downstream app declaring
status: { hidden: true }is exactly the ordinary shape this fixes, which is why it is declared rather than filed as invisible.Serial constraints cleared: #13214
Verified disjoint rather than assumed. PR #13258 is confined to
packages/restand its own harness; this diff is confined topackages/metadata-protocol/src/plus.changeset/. No shared file.expect(columns).toContain('status')for a field declared hidden. When this PR lands, that assertion is what turns red, by design. It belongs to #13214/#13258 and is ⛔ deliberately not edited here; I read it only at that PR's head for reference. Whichever of the two lands second needs the sibling's expectation updated in its own PR. Nothing onmaintoday asserts the pre-fix behaviour, so this branch's own CI is unaffected.Out of scope, filed not fixed
#13328 — after this fix, a hidden
created_atis correctly withheld fromcolumnswhilesortstill names it, because the sort directive is built from presence alone. Driven with a control arm (the sort block is byte-identical whethercreated_atis hidden or visible, whilecolumnsandsearchableFieldsdiffer, so the probe can observe a difference where one exists). Left unfixed on purpose: whetherhiddenshould govern an ordering directive is a judgment call, and #13259's triage ruled that this card restores a stated invariant and smuggles in nothing else. Filed unassigned withfinding.Generated by Claude Code
Generated by Claude Code