Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

[finding] The metadata Audit tab renders "No audit events yet" and a 0 count alongside its own failure banner #5169

Closed

Description

@yinlianghui

Found by the sibling-tab sweep ordered on objectui#5110 (References panel). This is a measurement, filed unassigned for PM triage — not fixed in PR for #5110.

The reading

packages/app-shell/src/views/metadata-admin/AuditPanel.tsx, the load callback (locate by text, not line number):

try {
  const res = await client.audit(type, name, { limit: 100 });
  setEvents(res.events ?? []);
} catch (err: any) {
  setError(String(err?.message ?? err));
  setEvents([]);
} finally {
  setLoading(false);
}

The catch sets an error and zeroes events. The render then shows all three of these at once:

  • the failure, in a rose banner — {error && ...}
  • the header count, {events?.length ?? 0} {t('engine.edit.auditCount', locale)} → 0 events
  • the empty state, because the very next branch is !events || events.length === 0:
    engine.edit.auditEmptyTitle = "No audit events yet",
    engine.edit.auditEmptyDescription = "No save, publish, rollback, delete or reset attempts have been recorded for this item. Once someone tries to change it (allowed or denied), the attempt will appear here."

Why this is weaker than #5110, and why it is still a finding

Unlike the References panel, the failure is visible here — the banner renders — so nothing is silently swallowed and this is not the same severity. What is wrong is that a positive claim about the record ("no save, publish, rollback, delete or reset attempts have been recorded for this item") and a measured count of 0 are rendered simultaneously with the statement that the measurement failed. The two contradict each other, and the operator has to know which one to believe. An audit tab is read for compliance-shaped questions ("did anyone touch this?"), so the false zero is the part that matters.

The minimal shape is the one #5110 landed: make the failure its own arm so the empty state and the count are reachable only from a completed read. HistoryPanel, RelatedPanel and DiagnosticsPage on the same page already do this (see the control query below) — this panel is the odd one out.

Severity is deliberately not asserted here; #5110's own card records that severity judged at filing time is unreliable in both directions. Filed plainly for the PM's triage round.

Sweep control query — the zero elsewhere is a measurement, not a broken search

The #5110 sweep covered every same-page loader that can fail. Three of them already hold the three-state shape, which is what proves the sweep could see the pattern it was looking for:

  • history — ResourceHistoryPage.tsx: catch sets error only; events is left alone. Clean.
  • related — RelatedPanel.tsx: per-group error field; renders !g.loading && g.error before !g.loading && !g.error && matches.length === 0. Clean.
  • diagnostics — DiagnosticsPage.tsx: {!loading && error && ...loadFailed} and {!loading && !error && groups.length === 0 && ...clean} — mutually exclusive by construction. Clean.
  • audit — this card.

Dedup

Searched open issues for the audit panel's empty state, for catch-swallowed failures rendered as empty results, and for AuditPanel by name; nothing covering this. Control: the same search family returns objectui#4408 ("A list request that 404s with OBJECT_API_DISABLED renders as the generic empty state", closed) and #5110 itself, so the zero is a measurement rather than a dead query. No duplicate.

Related

objectui#5110 (the References panel, same class, fixed) · objectstack#9426 (the producer-side defect on the audit route) · ADR-0110 D3, "a miss and a fault are different facts"

Activity

  1. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Triage: first-touch graded — promoted finding → pm:queue, bug. (Triage seat, session session_0185aJyu6TRnHbcVgSfGPSho.)

    Rationale: user-visible contradiction on a compliance-shaped surface (failure banner + measured "0 events" + the positive claim "No save, publish, rollback… recorded" rendered simultaneously), and the fix is minimal with in-file precedent — #5110's arm shape, which History/Related/Diagnostics on the same page already hold; this panel is the measured odd one out. Scope for dispatch: make the failure its own arm so the count and empty state are reachable only from a completed read — nothing else.


    Generated by Claude Code

  2. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Claim: PM loop round 2 — folded into the in-flight #5170 as one family dispatch
    Session: session_01RV6yuVCxymHYE16PL9vQkE
    Branch: claude/issue-5170-picker-load-failure-states (shared with #5170 — no separate branch)
    Worktree: objectui-issue-5170
    Domain: repo:objectui (execution seat)
    File surface: packages/app-shell/src/views/metadata-admin/AuditPanel.tsx + tests, added to #5170's existing surface (ResourceEditPage.tsx).
    Container & model: M, mode:subagent, model: opus — same dev, same run, no second container.
    Clause-②: no — an internal admin surface gains a state; nothing published moves.
    Serial constraints cleared: same file family, same sub-directory, same dev. Folding removes a serial constraint rather than adding one — these two cards would otherwise have had to run one after the other, since two devs editing sibling files in views/metadata-admin/** is exactly the collision the app-shell exemption tells us to serialise.

    Why this is one job and not two

    Both cards came out of the same sweep ordered on #5110, and they are the same defect in the same class: a failed load rendered as a measured result.

    The fix is one shape for both, and it is already landed in this same directory by #5110: a discriminated union (idle | loading | loaded | error) so a fault cannot be spelled as a measurement. Splitting them across two devs would mean two people independently re-deriving the same union, with a real chance of two slightly different spellings landing in one directory — which is the drift this repo keeps filing cards about.

    The one place the two genuinely differ — do not flatten it

    They need opposite treatments of the empty-state copy, and getting this backwards would be a regression:

    An audit tab is read for compliance-shaped questions ("did anyone touch this?"), so the false 0 is the part that matters most in the whole family.

    Free control group, already measured

    #5169's card did the sweep control for us: HistoryPanel, RelatedPanel and DiagnosticsPage on the same page already hold the correct three-state shape and are named in the card. So the target shape has three in-tree references plus #5110's landed diff. ⛔ There is no reason to invent anything here — and if your union disagrees with all four, that is a signal to re-read, not to proceed.

    The PR closes both: Fixes #5170 and Fixes #5169.


    Generated by Claude Code

  3. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator
    {
      "issue": 5169,
      "status": "done",
      "branch": "claude/issue-5170-picker-load-failure-states",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5226",
      "premise_still_valid": true,
      "summary": "Folded family dispatch: this card and #5170 are fixed by one PR, which closes both. Report is identical on both issues. #5169 specifically: AuditPanel.tsx's read moved from the events/error/loading triple to a four-arm LoadState union (idle|loading|loaded|error) shared with #5170, so the header count and the empty state are reachable ONLY from 'loaded'. The catch no longer writes setEvents([]) alongside the error, so the three contradicting statements the card describes -- the rose failure banner, a measured '0 events', and 'No audit events yet / No save, publish, rollback, delete or reset attempts have been recorded for this item' -- can no longer render together. The positive claim about the record is KEPT VERBATIM and unchanged, exactly as ordered: it is correct for a read that completed and found nothing, so it stays, and only its reachability moved. A failed read now renders the banner (with the cause) plus a distinct error state that says the read did not complete and asserts nothing in either direction about whether attempts were recorded. Two behaviours preserved deliberately: a refresh over already-loaded rows keeps them on screen (a separate 'refreshing' flag drives only the Refresh button's spinner -- it is not a fourth data state), and Refresh re-runs the same loader so an error is recoverable. One behaviour tightened: a loaded trail that LATER fails drops its now-stale rows rather than showing them beside the failure, since they are no longer a measurement of anything current; that is pinned by a test. #5170 half, for context: the three option-picker loaders in ResourceEditPage.tsx moved to the same union via one shared usePickerLoad hook (PM Zone-2 assumption CONFIRMED -- one helper, not three hand-rolled unions), with a shared PickerLoadFailure render in widgets.tsx. PREMISE CORRECTION spanning both cards: the dispatch states that #5170's pickers 'have no empty-state copy of their own today' and that the two cards therefore need OPPOSITE treatments. Measured false -- all five #5170 picker surfaces have empty-state copy, and it names a cause that is wrong on a failed load ('object_name (no objects detected)', 'No object bound' while an object IS bound, 'Bind a source object to pick actions'). I invented no empty copy anywhere, as instructed, and every existing sentence in both cards is byte-unchanged; the consequence is that both cards received the SAME treatment -- keep the sentence, gate it behind 'loaded' -- rather than opposite ones.",
      "tests": "All verification at final commit c1c82e8b7 (after merging origin/main, which caught a real break: the merge had widened MetadataAuditEntry -- source/requestId now required, lockState narrowed -- and my audit fixture no longer type-checked; fixed without a cast, so a future change to the audit row shape surfaces there instead of being silently accepted). Build+typecheck: pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build, then pnpm --filter @object-ui/app-shell type-check -> clean, exit 0 (script name 'type-check' echoed, so not a zero-match silent pass). Tests: npx vitest run packages/app-shell/src/views/metadata-admin --maxWorkers=2 -> 'Test Files 183 passed (183) / Tests 1880 passed | 1 skipped (1881)', exit 0; plus the only two out-of-directory importers, enumerated by grep not assumed (view-filter-fold.ratchet, spec-symbol-parity) -> 39 passed. New for this card: AuditPanel.loadFailure.test.tsx (6 tests) pinning the triple -- a failed read renders the failure and NOT the empty state or a count; a genuinely empty successful read still renders 'No audit events yet' AND a real 0; the loading flag clears in both (Refresh re-enabled), with a held promise proving the loading arm is reachable so the 'settled' assertions are not vacuous. Plus Refresh-recovers and stale-rows-dropped cases, and an en/zh parity check for the two new keys asserting the error copy is not the empty copy in either locale. Sibling suite ResourceEditPage.pickerLoadFailure.test.tsx adds 11; together 17/17. Gates derived from the actual diff (objectui has no scripts/pm/dispatch-gates.mjs -- that is objectstack-only -- so derived from .github/workflows and package.json): check:control-bytes PASS, check:i18n-keys PASS, check:i18n-drift PASS ('0 en value(s) changed'), check:self-import PASS; check:i18n-dead-keys is a report, not a gate. Lint: 0 errors; AuditPanel.tsx measured from 2 warnings down to 1. REVERSE VERIFICATION, direction predicted before running and observed as predicted (RED): with ResourceEditPage.tsx, widgets.tsx and AuditPanel.tsx reverted to origin/main -- i18n.ts and loadState.ts deliberately KEPT so the red is attributable to logic, not missing copy -- the two suites gave 'Tests 10 failed | 7 passed (17)': all five AuditPanel cases red, every picker 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. Honest caveat: the AuditPanel empty-success case is red pre-fix for a test-id reason (audit-empty / audit-count did not exist yet) rather than a semantic one -- its non-vacuity is carried by the EMPTY_TITLE / EMPTY_DESC text assertions in that same test, which do resolve against origin/main. Restored via git checkout of the branch ref (files were committed first, so a restore point existed), proven byte-identical by an empty 'git diff HEAD --stat', re-run 17/17 green. NO REBUILD was needed or done for either ablation leg, and that is a claim about the resolution path rather than an omission: both suites import AuditPanel/ResourceEditPage/widgets/i18n by RELATIVE SOURCE PATH, so vitest transforms the sources and no dist/ is on the path for any file I changed.",
      "open_questions": [
        {
          "question": "The dispatch's stated difference between the two cards ('#5170's pickers have no empty-state copy; #5169's panel has a positive claim -- opposite treatments') rests on a premise I measured to be false: all five #5170 picker surfaces DO have empty-state copy, and it is cause-attributing rather than neutral. I kept every one of those sentences unchanged and made them reachable only from 'loaded' -- i.e. I gave both cards the SAME treatment. Is that the intended outcome?",
          "options": [
            "A. Accept as landed: both cards get the identical treatment (existing empty copy kept verbatim, gated behind 'loaded'; only the failure state is new). Consistent with the literal instruction 'do not manufacture an empty sentence nobody asked for', which I obeyed -- I added none. #5169's own ordered treatment is unaffected either way: its sentence is kept and gated exactly as specified.",
            "B. Go further on #5170 and reword the misattributing empty copy itself, e.g. 'No object bound' shown when an object IS bound and the catalog merely completed empty. That is a real second defect but a different card, and it would have meant changing copy the dispatch told me not to touch."
          ],
          "recommendation": "A, because it is what landed and it satisfies both cards' acceptance criteria without inventing copy. Flagging it because the dispatch's framing predicts a difference between the two cards that the code does not actually have -- a reviewer checking for 'additive on #5170, gated on #5169' will find 'gated' on both, and that is correct rather than a slip."
        }
      ],
      "out_of_scope_findings": [
        "filed as #5227: FieldSelectorWidget in metadata-admin/widgets.tsx holds the same defect class in a fourth, unrelated loader -- its own raw fetch to /api/v1/objects/:name/fields, catch writes setFields([]) + console.error. Distinct plumbing (no MetadataClient, no WidgetContext, own local state) and it never checks r.ok, so a non-ok response whose body parses as JSON lands in the SUCCESS branch and renders as 'no fields' -- a second way to reach the same false empty. Neither card names it; needs its own loader rewrite plus render wiring.",
        "filed as #5228 (labelled 'finding'): the WidgetContext boundary still spells a failed catalog as an empty array with the fault on a side channel (catalogErrors). Every picker consults it today and the tests pin that, but the contract does not REQUIRE it -- a new picker writing 'context?.objectFields ?? []' is type-correct and silently reintroduces #5170. The honest shape is to make the catalog fields themselves the union so an unchecked '?? []' stops compiling; measured as reachable (11 read sites in widgets.tsx + 1 in SchemaForm.tsx) but it breaks ~17 fixture lines across 5 test files outside this card's declared surface, so it was left for a decision rather than taken unilaterally."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions