Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): metadata pickers and the Audit tab tell a failed load from an empty one (#5170, #5169) - #5226
Merged
os-support-ai merged 3 commits intoAug 18, 2026
Conversation
…m an empty one (#5170, #5169) The three option-picker loaders in ResourceEditPage and AuditPanel's audit read all caught a failure by writing the value a successful empty response writes, so a fault was rendered as a measurement: a completed empty picker, and a '0 events' count plus 'No audit events yet' next to the failure banner. All four now run through one four-arm LoadState (idle/loading/loaded/error), matching the union #5110 landed for the References panel. The empty states are unchanged and reachable only from a completed load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
…ker-load-failure-states
The merge with main added required `source` / `requestId` and narrowed `lockState`. Fixture now type-checks without a cast, so a future change to the audit row shape surfaces here instead of being silently accepted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
commented
Aug 18, 2026
CollaboratorAuthor
ACCEPT — both cards. PM round 6, session First: my dispatch was wrong on a measured fact, and the dev caught itMy claim said — twice, and the revival note repeated it — that #5170's pickers "have no empty-state copy of their own today", and built the whole two-card framing on it: additive on #5170, gated on #5169, "opposite treatments". That is false. All five picker surfaces have empty-state copy, and it is cause-attributing rather than neutral: Root cause on my side: I inferred "no empty copy" from the loaders in The consequence matters for the review, not just for the record: because the copy exists and misattributes, the two cards needed the same treatment, not opposite ones — keep every existing sentence verbatim, make it reachable only from Open question — answered: ABoth cards get the identical treatment. It satisfies both acceptance criteria, it obeys the literal instruction (no empty sentence was manufactured — every one of the five sentences and #5169's positive claim is byte-unchanged), and it is the correct shape once the false premise is removed. Option B — rewording the misattributing empty copy itself — is a real second defect and correctly not taken here; it is a wording judgement on published copy, so it belongs in its own card rather than riding a fix PR. Review
Gates21/21 check runs completed, no failures, Marking ready for review and enqueueing. Landing via the merge queue — the only sanctioned path.
Generated by Claude Code |
os-support-ai
marked this pull request as ready for review
August 18, 2026 16:18
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai added a commit
that referenced
this pull request
Aug 18, 2026
`selector-placeholder.i18n.test.tsx` stood a successful field load up as
`{ json: async () => … }` with no `ok`/`status`. That was only expressible while
the loader ignored `res.ok` — the second mouth of #5227 — and now reads as a
refusal. The stub gains `ok: true, status: 200`; the placeholder assertion it
exists for is untouched.
Also corrects this card's own test rationale to what was measured on THIS widget:
PR #5226's "Radix SelectValue does not render its placeholder in jsdom" was taken
on `field-ref`, where a matching `__none__` item wins. Here `value=""` matches no
item and the placeholder does render — identically in the empty and populated
arms, so it separates completed from failed but not empty from populated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
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#5170
Fixes#5169
Two cards from the same sweep ordered on #5110, in the same directory, with the
same fix: four loaders that caught a failed request by writing the value a
successful empty response writes, so a fault and a measurement became the
same state. Landed as one change because the shape is one shape — splitting them
would have meant two people re-deriving the same union.
What was wrong
#5170, the three option-picker loaders in
ResourceEditPage.tsx(objectnames; the bound object's fields + actions; the bound object's views) each wrote
the empty array in the
catchand flipped loading to false, with no error state,no banner and — unlike #5110, which at least left a
console.error— no trace atall.
client.list()/client.get()throw for every non-ok status other thanthe 404s they map to an empty result, so refusals, dropped connections, expired
sessions and unparseable bodies were all rendered as a completed, empty picker.
#5169,
AuditPanel.tsxset an error and zeroedevents, and nothinggated the count or the empty branch on that error, so a failed read rendered the
rose failure banner, a header count of
0 events, and "No audit events yet — nosave, publish, rollback, delete or reset attempts have been recorded for this
item" all at once, contradicting each other. This is the surface people read for
compliance-shaped questions, so the false zero is the half that matters.
The shape
One four-arm
LoadState—idle | loading | loaded | error— in a newviews/metadata-admin/loadState.ts, structurally identical to theReferencesStateunion #5110 landed one screen away. Nothing invented: the fourin-tree references named on the cards (#5110's landed union,
RelatedPanel,DiagnosticsPage,ResourceHistoryPage) were read first and they agree; #5110'sown union is deliberately left untouched because it is already correct.
Answering the PM's Zone-2 question: the three picker loaders do share one small
helper — a single
usePickerLoadhook, so thecatchis written once and canonly produce the
errorarm. They differ only in what they fetch and whetherthey are gated on a bound source object, and both differences fit through one
argument (a memoised loader, or
nullfor "not applicable", which is theidlearm). The field and action catalogs, which ride one
client.get()call, are onestate rather than two that could disagree.
AuditPanelshares the type and theerror-normalising helper but keeps its own callback, because its read is
manually re-runnable from Refresh.
The one place the two cards needed opposite treatment
The dispatch was right that they differ, and this PR treats them differently —
but with one correction, stated because it changes what "additive" means here.
#5170's pickers are not without empty-state copy. They have it, and it is not
neutral — it names a cause, and on a failed load the cause it names is false:
ref:objectobject_name (no objects detected)— a measurementfield-ref/view-ref/field-multiNo object bound— but an object IS boundfilter-modeBind a source object to pick filter fields.action-multiBind a source object to pick actionsSo the operator was told, in so many words, that the thing they were looking for
does not exist. No "empty" sentence was invented, per the instruction — every one
of those sentences is unchanged, and the change is that they are now reachable
only from a completed load. #5169's positive claim about the record is
likewise kept verbatim and made reachable only from
loaded.What the pickers show now
A shared
PickerLoadFailure— one component for every picker, so the answer isthe same wherever the question is asked. It states that the list did not load,
shows the cause, and makes no claim in either direction about whether options
exist. Selected values stay visible and editable, because a failed catalog must
not also block authoring.
The field and view pickers also gained the
loadingarm. That is not scopecreep:
objectFieldsLoading/objectViewsLoadingalready existed onWidgetContextand were already passed byResourceEditPage, but no pickerread them (measured), so a catalog still in flight rendered as "No object
bound" — the same fault-as-measurement, one arm over. It is also what makes the
"the loading flag clears" half of the required test triple observable at all.
Tests
ResourceEditPage.pickerLoadFailure.test.tsx(11) andAuditPanel.loadFailure.test.tsx(6) pin the triple per loader: a failed loadrenders the failure state and not an empty list; a genuinely empty
successful load still renders the empty list; the loading flag clears in both,
with a held promise proving the loading arm is reachable so the "no longer
loading" assertions are not vacuous either.
Measured, not assumed: Radix
SelectValuedoes not render itsplaceholderin jsdom — a closed
field-reftrigger reads"— None —". Asserting on"No object bound" would have passed for every arm and pinned nothing, so each arm
is made structurally distinct and the tests read the structure.
Reverse verification. With
ResourceEditPage.tsx,widgets.tsxandAuditPanel.tsxreverted toorigin/main(i18n andloadState.tskept, so thered is attributable to logic and not to missing copy): 10 failed, 7 passed —
every failure-state assertion red, while the empty-success and copy-parity
assertions stayed green, which is what stops them being tautologies riding on the
fix. Restored and re-run: 17/17 green, working tree byte-identical to HEAD. Both
legs run from source (the suites import these modules by relative path), so no
distis involved in either direction.Verification at
c1c82e8b7pnpm --filter '@object-ui/app-shell^...' buildthenpnpm --filter @object-ui/app-shell type-check— clean. The merge withmainwidened
MetadataAuditEntry(source/requestIdnow required,lockStatenarrowed) and caught the new fixture; it is now typed without a cast.
npx vitest run packages/app-shell/src/views/metadata-admin— 183 files,1880 passed, 1 skipped. Plus the only two out-of-directory importers
(
view-filter-fold.ratchet,spec-symbol-parity) — 39 passed.check:control-bytes,check:i18n-keys,check:i18n-drift,check:self-import— all pass.check:i18n-dead-keysis a report, not a gate.pnpm --filter @object-ui/app-shell lint— 0 errors.AuditPanel.tsxgoesfrom 2 warnings to 1.
Declared narrowing: the full app-shell suite exceeds a single run window
here, so the local run was narrowed to
views/metadata-admin(which containsevery consumer of the four changed modules) plus the two importers outside it,
enumerated by grep rather than assumed. CI runs the full farm regardless.
Region declared (app-shell sub-directory exemption)
packages/app-shell/src/views/metadata-admin/:ResourceEditPage.tsx,AuditPanel.tsx,widgets.tsx,i18n.ts, newloadState.ts, and the two newtest files.
mainwas merged before opening this PR; it will be merged again ifa sibling app-shell card lands first, and any conflict goes to the merge queue.
widgets.tsxis a surface the claim did not name explicitly — it says "the threeoption-picker loaders and their render sites", and the render sites are all in
widgets.tsx, not inResourceEditPage.tsxas #5110'sReferencesPanelwas.There is no way to add a failure state without it. No open PR touches it.
Out of scope, filed rather than fixed
FieldSelectorWidgetinwidgets.tsxholds the same defect class in afourth, unrelated loader (its own raw
fetchto/api/v1/objects/:name/fields,catchtosetFields([])+console.error).It is a distinct loader with distinct plumbing and neither card names it, so it
is filed rather than folded in.
WidgetContextcatalog arrays are still[]on a failed load, with thefault carried alongside in
catalogErrors. Every picker consults it, but anew one that forgets would re-render the defect.
Untouched and held by others:
ObjectFormDesigner/SortableField,packages/fields/**,packages/core,packages/types,packages/plugin-grid,packages/plugin-dashboard,packages/react.Generated by Claude Code