feat(ui): add network-vs-API error distinction and loading skeletons to StateBoundary - #5620
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-13 14:24:54 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 6 non-blocking
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands Visual preview
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.
|
…to StateBoundary Several routes had minimal loading/error handling: no retry, no way to tell a connectivity failure apart from a server-side error, and a loading state that rendered identically to a genuinely-empty one. useApiResource now surfaces the apiFetch failure kind and status; StateBoundary/ErrorState use it to show distinct copy and iconography for "can't reach the server" vs "the server returned an error", and accept an optional content-shaped loadingSkeleton in place of the generic spinner. Both are additive and backward-compatible -- every existing StateBoundary/ErrorState/useApiResource consumer keeps its current default copy unless it opts in. Wires the new primitives into app.runs.tsx, the most minimal of the routes named in the issue: a proper StateBoundary (retry, network-aware error, loading skeleton) replaces the old ad-hoc warning banner and the misleading loading state that showed "no runs match filters" before the first fetch even completed, plus a RefreshMeta "last refresh" label in the header.
8f3f533 to
66c36b6
Compare




Summary
app.runs.tsxin particular rendered its loading state identically to a genuinely-empty one (it showed "No runs match these filters" before the first fetch had even completed).useApiResourcenow surfaces the underlyingapiFetchfailurekind/status, andStateBoundary/ErrorStateuse it to render distinct copy + iconography for a connectivity failure (WifiOff, "Can't reach the server") vs a server-side error (AlertTriangle, generic "Couldn't load data").StateBoundaryalso accepts an optionalloadingSkeletonto replace the generic spinner with a content-shaped placeholder.StateBoundary/ErrorState/useApiResourceconsumer (app.analytics, app.operator, the maintainer/owner/miner/digest/commands panels, dead-letter-queue-panel, notification-readiness-card) keeps its current default copy unless it opts in to the new props.app.runs.tsx, the most minimal of the routes named in the issue: replaced the old ad-hoc "Live runs are unavailable" banner with a properStateBoundary(retry, network-aware error copy, loading skeleton), and added aRefreshMeta"last refresh" label + manual refresh button to the header.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Closes #793
Validation
git diff --checknpm run actionlint— ran as part ofnpm run test:ci, passednpm run db:migrations:check/db:schema-drift:check/selfhost:env-reference:check— ran as part ofnpm run test:ci, passed (no DB/env changes in this PR)npm run typechecknpm run test:coveragelocally (this PR only touchesapps/gittensory-ui/**, which Codecov'ssrc/**-only patch rule does not gate, but the new/changed logic is covered by new and updated tests)npm run ui:lintnpm run ui:typechecknpm run ui:testnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run cf-typegen:check(part ofnpm run test:ci) fails locally on this Windows dev machine with awrangler ENOENT—scripts/gen-cf-typegen.mjscallsexecFileSync("wrangler", ...)withoutshell: true, which cannot invoke npm's.cmdshim on Windows (confirmed in isolation, independent of any change in this PR — reproduces with a bareexecFileSync("wrangler", ["--version"])on a clean checkout). This PR makes no Cloudflare/wrangler.jsoncchanges; the check runs fine on the actual CI (Linux runners).Safety
errorKind/errorStatusare client-side fields derived from the existingapiFetchresult shape.)UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
All captured against
app.runs.tsxwith a mocked authenticated session and a mocked/v1/agent/runsresponse, so each state is genuinely reachable rather than staged.Notes
state-views.tsx,use-api-resource.ts) plusapp.runs.tsx, the route with the most clearly minimal state handling of the ones named in the issue.app.workbench.tsx's panels (miner/playground/commands/digest) andapp.operator.tsx/app.analytics.tsxalready useStateBoundary; they pick up the network-vs-API distinction and skeleton support automatically the next time they're touched, with no migration required since both additions are opt-in.