Skip to content

fix(ui): adopt AnalyticsCardShell in 5 analytics cards that hand-roll their own chrome - #6543

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
galuis116:fix/analytics-card-shell-adoption
Jul 16, 2026
Merged

fix(ui): adopt AnalyticsCardShell in 5 analytics cards that hand-roll their own chrome#6543
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
galuis116:fix/analytics-card-shell-adoption

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Closes #6175

Summary

apps/loopover-ui/src/components/site/app-panels/analytics-card-shell.tsx is the documented (#2200) shared "titled card with loading/empty/ready" treatment. Five sibling analytics cards never adopted it, instead hand-rolling the same <section className="rounded-token border-hairline..."> header/title markup: cycle-time-card.tsx, gate-precision-card.tsx, reversal-health-card.tsx, gate-outcome-card.tsx, slop-duplicate-trend-card.tsx. This is the root cause of a real, user-visible bug: cycle-time-card.tsx's no-data case rendered a bare <p> instead of the shared EmptyState every properly-adopted sibling (acceptance-rate-card.tsx, queue-health-card.tsx) gets for free from AnalyticsCardShell's state="empty".

Each of the 5 cards has a different relationship to "empty," so each migrates differently rather than mechanically:

  • CycleTimeCard and SlopDuplicateTrendCard are true binary states — when there's no signal, nothing else renders (both previously fell back to a bare <p>). Both now use the shell's state={hasSignal ? "ready" : "empty"} directly, with the previous <p> copy moved into emptyTitle/emptyHint.
  • GatePrecisionCard keeps its documented "render nothing when there are no evaluated rows" early return (if (report.rows.length === 0) return null;) completely unchanged — only the rows > 0 branch is wrapped in the shell, with state="ready" always.
  • ReversalHealthCard and GateOutcomeCard always render their Stat tiles regardless of whether the list/mix content has data — using the shell's own state="empty" there would also hide those always-present Stats, which isn't the current (or wanted) behavior. Both stay in state="ready" and keep their existing inner list-vs-EmptyState toggle exactly as before, the same shape acceptance-rate-card.tsx/queue-health-card.tsx already established for content that doesn't cleanly fit the shell's binary split.

AnalyticsCardShell gains an optional action header-right slot (status pill / boundary badge / freshness stamp), rendered across every state via the header's existing justify-between layout, so each card keeps its existing header signal instead of losing it in the move.

Data-fetching logic and the shape of ready-state content are unchanged — only the chrome/state-handling wrapper moved.

Note on the prior closed attempt

#6472 attempted this same issue and was auto-closed: CI genuinely failed (validate, validate-code, validate-tests, and the UI preview build) despite the PR claiming a clean typecheck/lint/test/build. This PR verifies every claim below directly rather than assuming — including deliberately reverting the fix files via git stash to confirm the new/strengthened tests actually fail without them before restoring.

Scope

Validation

  • npm --workspace @loopover/ui run typecheck — clean.
  • npm run ui:lint — 0 errors, only pre-existing warnings in unrelated files.
  • npm --workspace @loopover/ui run test (full workspace) — 47 files, 294 tests, all passing.
  • Strengthened cycle-time-card.test.tsx's and slop-duplicate-trend-card.test.tsx's empty-state tests to assert the real EmptyState title/description (not just the header pill text, which was the only thing the old assertions checked) — verified both genuinely fail without the fix via git stash on the three changed component files, then restored and re-confirmed passing.
  • Added 2 new tests to analytics-card-shell.test.tsx for the new action slot: renders across all three states, and is omitted entirely when not provided.
  • git diff --check
  • npm run actionlint
  • npm run docs:drift-check / manifest:drift-check / command-reference:check
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • npm run ui:build — fails in this sandbox on an unrelated, pre-existing issue (@scalar/api-reference fails to resolve from @scalar/api-reference-react's dist output), reproduced identically on a clean, unmodified main checkout this same session (unrelated to this diff).
  • Root npm run typecheck / npm run test:coverage — this sandbox's root tsc --noEmit reliably OOMs regardless of diff content (reproduced repeatedly this session); the workspace-scoped @loopover/ui typecheck (clean) plus the full workspace test run (294/294 passing) cover this diff's actual surface. apps/** is excluded from Codecov's coverage.include, so no codecov/patch check applies to this diff.

If any required check was skipped, explain why:

  • ui:build: pre-existing sandbox environment issue, confirmed unrelated to this diff.
  • Root typecheck/test:coverage: established OOM pattern in this sandbox, unrelated to diff content; superseded by the scoped checks above.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no such changes.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no API/MCP surface touched; all 5 cards keep the exact same prop/data shape.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — Every card already renders from live dashboard payloads; this only fixes/unifies how each card's existing states are presented, per the issue's own scope.
  • Visible UI changes include a UI Evidence section below with screenshots. — See below.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — CHANGELOG.md untouched.

UI Evidence

The visible change is CycleTimeCard's and SlopDuplicateTrendCard's empty states now show the shared EmptyState treatment (icon + title + description, matching every sibling card) instead of a bare paragraph. The other 3 cards (GatePrecisionCard, ReversalHealthCard, GateOutcomeCard) render identically to before — only their internal chrome now comes from the shared shell rather than hand-rolled markup. ui:build couldn't run in this sandbox (see Validation) to produce a live screenshot; covered instead by the new/strengthened regression tests asserting the exact rendered EmptyState title/description text per card.

Notes

None.

cycle-time-card, gate-precision-card, reversal-health-card,
gate-outcome-card, and slop-duplicate-trend-card each hand-rolled the
same "titled card with loading/empty/ready" chrome AnalyticsCardShell
(JSONbored#2200) already provides, matching acceptance-rate-card/queue-health-
card. Fixes the real user-visible bug this caused: CycleTimeCard's
no-samples branch rendered a bare <p> instead of the shared EmptyState
every other adopted card gets for free.

- CycleTimeCard and SlopDuplicateTrendCard are true binary states
  (nothing else renders when empty) -- both now use the shell's
  state="empty"/"ready" toggle directly.
- GatePrecisionCard keeps its documented "render nothing when there
  are no evaluated rows" early return unchanged, wrapping only the
  rows>0 branch in the shell.
- ReversalHealthCard and GateOutcomeCard always render their Stat
  tiles regardless of the list/mix content, so both stay in the
  shell's "ready" state and keep their own inner list-vs-EmptyState
  toggle, matching acceptance-rate-card's shape for content that
  doesn't fit the shell's own binary empty/ready split.

AnalyticsCardShell gains an optional `action` header-right slot (status
pill / boundary badge / freshness stamp), rendered across every state
via the header's existing justify-between layout, so each card keeps
its existing header signal.

Data-fetching logic and ready-state content are unchanged -- only the
chrome/state-handling wrapper moved.

Closes JSONbored#6175
@galuis116
galuis116 requested a review from JSONbored as a code owner July 16, 2026 12:43
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 12:52:10 UTC

9 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR migrates 5 analytics cards to the shared AnalyticsCardShell component, adding an `action` header-slot prop to the shell so status pills/badges can render across all states, and fixes the real bug (bare `<p>` instead of shared `EmptyState`) in CycleTimeCard's empty case. The differentiated migration strategy is sound: cards with a true binary empty state (CycleTimeCard, SlopDuplicateTrendCard) use `state={hasSignal ? "ready" : "empty"}`, while cards with always-present Stats (ReversalHealthCard, GateOutcomeCard) correctly stay in `state="ready"` and keep their own inner empty toggle to avoid hiding the Stats, and GatePrecisionCard's early-return null behavior is left untouched. Tests cover the new action slot across all three states and the CycleTimeCard EmptyState fix; CI is green.

Nits — 5 non-blocking
  • The external brief flags magic-number `6175` issue-reference comments in 4 files (cycle-time-card.tsx:9, gate-outcome-card.tsx:15, gate-precision-card.tsx:13, reversal-health-card.tsx:14) — these are just doc-comment issue citations, not real magic numbers, so no action needed, but worth confirming that's the intent.
  • gate-outcome-card.tsx and reversal-health-card.tsx keep their own inner `hasSamples`/`reversedTargets.length > 0` EmptyState toggle inside the shell's 'ready' state, which is correct per the PR's stated design but slightly duplicates the shell's own empty-rendering logic — worth a short code comment on shell itself noting this dual-pattern exists (already added inline in the two card files, so this is minor).
  • Test-to-code ratio (0.25) is on the lower side given 5 files changed, though the shell's own new action-slot tests and the CycleTimeCard EmptyState assertions do cover the two behaviorally new paths.
  • Consider adding a one-line note in analytics-card-shell.tsx's docblock cross-referencing when callers should use the inner list-vs-EmptyState pattern (ReversalHealthCard/GateOutcomeCard) versus the shell's own `state="empty"` (CycleTimeCard/SlopDuplicateTrendCard), since future contributors adding new analytics cards will face the same choice.
  • gate-outcome-card.tsx and reversal-health-card.tsx tests don't appear to add new assertions for the action slot rendering in the 'ready' state beyond what's implicit — a targeted test asserting the BoundaryBadge/StatusPill still renders in the header after migration would close the loop explicitly.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6175
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 1945 registered-repo PR(s), 1279 merged, 54 issue(s).
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1945 PR(s), 54 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
All five named cards (cycle-time, gate-precision, reversal-health, gate-outcome, slop-duplicate-trend) are migrated to render through AnalyticsCardShell, with CycleTimeCard now rendering the shared EmptyState instead of a bare <p>, and tests verify this behavior.

Review context
  • Author: galuis116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, Python, Dart, TypeScript, HTML, MDX, Rust, C++
  • Official Gittensor activity: 1945 PR(s), 54 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 0acedd3 into JSONbored:main Jul 16, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ui): adopt AnalyticsCardShell in 5 analytics cards that hand-roll their own chrome (fixes CycleTimeCard's broken empty state)

1 participant