refactor(gui): react-doctor cleanup for Startup/Debug/Storage/Usage

[Wibias]

Summary

• Split Startup and Debug page modules for react-doctor maintainability.
• Harden Storage/Usage loading resets and related fetch status handling for react-doctor 0.9.1.

Stack

• Depends on chore(gui): pin react-doctor to 0.9.1 #465 and fix(gui): react-doctor shared foundations #466.

Test plan

• bun x tsc -b in gui/
• Smoke Startup, Debug, Storage, and Usage pages

[coderabbitai]

Warning

Review limit reached

@Wibias, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 97f99134-eb32-4348-84d7-ba6bea62e280

📥 Commits

Reviewing files that changed from the base of the PR and between 803807a and fd5ad59.

📒 Files selected for processing (15)

• gui/src/pages/Debug.tsx
• gui/src/pages/Startup.tsx
• gui/src/pages/Storage.tsx
• gui/src/pages/Usage.tsx
• gui/src/pages/debug-claude-inbound-panel.tsx
• gui/src/pages/debug-log-viewer.tsx
• gui/src/pages/debug-settings-panel.tsx
• gui/src/pages/debug-shared.ts
• gui/src/pages/startup-sections.tsx
• gui/src/pages/startup-shared.ts
• gui/tests/debug-claude-inbound-keys.test.tsx
• gui/tests/debug-mutation-busy.test.tsx
• gui/tests/storage-loading-race.test.tsx
• src/claude/inbound-debug.ts
• tests/claude-inbound-debug.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[lidge-jun]

🔒 Under maintainer review — detailed feedback incoming

@lidge-jun (maintainer) has this PR in an active review pass. Please do not merge, rebase, or
close it until the detailed review lands; a full comment with specific file:line findings,
failure modes, and suggested fixes is being prepared.

This is a claim marker so two maintainers do not review or land the same PR at once. If you are a
maintainer and need this PR urgently, please reply here first instead of merging.

Baseline for this pass: dev is at bef0dfbe (yesterday's integration batch ebc62d1f plus
#458, #456, #391, #392, #463, #465). If this PR touches the same files, the upcoming review will
name the exact overlapping paths so you only have to rebase once.

No action needed from you until then. Thanks for the contribution and for your patience.

_{Review tracker: devlog/_plan/260726_pr_review_pass · marker posted by the maintainer review pass}

[lidge-jun]

Thanks for the react-doctor cleanup across Startup/Debug/Storage/Usage. Typecheck, lint, and matrix CI are green on the exact head, but this is labelled a refactor and three of the changes are behavior-changing rather than behavior-preserving. Those are what I'd like resolved before merge.

1. Aborted requests now clear shared loading state unconditionally — gui/src/pages/Startup.tsx:113, gui/src/pages/Storage.tsx:130, gui/src/pages/Usage.tsx:567. When apiBase, the range, or the surface changes, the superseded request's finally can run after the replacement request has started, setting loading=false while the newer request is still in flight. The UI then presents stale data as settled and re-enables controls early. Please restore latest-request ownership with a generation or request ID, and clear loading only when the completing request is still the current one. The abort guard shouldn't be weakened just to satisfy the linter.

2. gui/src/pages/Debug.tsx:125 and :161 — mutation busy state was removed. debugBusy is gone and hard-coded to false, so the switches and Reset stay enabled during in-flight PUTs. Rapid toggling or a toggle-plus-reset then resolves to whichever response lands last, which is a real correctness problem for a debug-flag surface. Restore the mutation state, disable controls while a mutation is pending, and refresh from the accepted response or after the latest mutation settles.

3. gui/src/pages/debug-claude-inbound-panel.tsx:30 — the new row key is not unique. The key ${entry.at}:${entry.endpoint}:${entry.model} collides because the producer stamps Date.now() at src/claude/inbound-debug.ts:85, and several same-model requests can be captured within one millisecond. Duplicate keys give React incorrect reconciliation and visibly stale rows. Add a monotonic sequence ID to captured entries and key on it.

Overlap with dev: gui/src/pages/Usage.tsx is the only touched path that also moved on current dev (later Usage layout/accessibility work). Three-way merge inspection is clean so a rebase isn't strictly required, but please revalidate the merged Usage page so both sets of changes survive.

Test gap: add component-level regressions for (a) an aborted request completing after its replacement starts, (b) rapid Debug flag/reset interactions, and (c) multiple Claude inbound entries sharing a timestamp, endpoint, and model. The current green typecheck/lint/matrix runs do not exercise any of these timing or reconciliation failures — which is exactly why the refactor reads as safe today.

---

_{Reviewed as part of a maintainer review pass against dev at bef0dfbe. Verification: exact head, fresh local bun run typecheck + bun run lint:gui, plus hosted matrix CI.}

[Wibias]

Review follow-up (46bef6cf)

Rebased onto current upstream/dev, dropping the bundled foundations commit (9c7e922e) now that #466 is merged.

Fixed (Jun)

1. Startup / Storage / Usage loading races — generation/request ownership so a superseded aborted request cannot clear loading while a newer fetch is in flight.
2. Debug mutation busy — restored debugBusy + mutation generation; switches/Reset disable while PUT pending; apply accepted response via setClientResourceData for the latest mutation only.
3. Claude inbound row keys — monotonic id on capture entries; panel keys on entry.id.

Tests

• gui/tests/storage-loading-race.test.tsx
• gui/tests/debug-mutation-busy.test.tsx
• gui/tests/debug-claude-inbound-keys.test.tsx
• extended tests/claude-inbound-debug.test.ts

Skipped

• Codex comments on gui/src/api.ts / client-resource foundations — those land via fix(gui): react-doctor shared foundations #466 after the rebase; out of scope for this PR.

Verification locally: focused GUI + inbound tests, bun x tsc -b, bun run lint in gui/.

[lidge-jun]

Post-merge note: I put this through an independent review pass after it landed, and the five production fixes all hold up. Startup, Storage, and Usage each own loading through a complete generation sequence (increment, result guard, current-generation finally, pre-abort invalidation), every Debug mutation control is gated by debugBusy with the PUT response installed before busy clears, and nextCaptureId stays unique across ring eviction and debug toggles. The extraction is behavior-preserving — conditionals, disabled states, ARIA roles, and i18n keys all survived.

Your entry.id approach is better than what I had planned. I was going to key on ${entry.at}-${index}, which still shifts when the ring reorders; a producer-side monotonic id has no such problem.

Three test-quality gaps did turn up, and I've closed them in bcaf029e rather than sending this back for another round — they're the kind of hardening a maintainer can write, and the implementation itself was done.

debug-claude-inbound-keys passed against the defect it was written to catch. It asserts row content only, and with the old colliding composite key React still renders both rows correctly while merely warning on the console. Reproduced it directly. It now captures console.error and asserts the duplicate-key warning is absent.

claude-inbound-debug never reached RING_LIMIT. Three captures don't exercise eviction. Added a 25-capture case asserting ids stay unique and descending afterwards — a counter reset would resurrect a discarded id and bring the collision straight back.

Startup and Usage had no race regression. storage-loading-race is a genuine one, but the other two carry the same guard with nothing protecting it. Added equivalent delayed-settlement tests and verified they go red by reverting each guard individually.

Neither Debug test proved response-install ordering. Both flows end at the same state they start from, so removing setClientResourceData() still satisfies their final assertions. Added a false → true transition where every GET keeps returning the pre-mutation state, so the only way the switch can read true is the PUT response itself. It's in a separate file because sharing a fetch stub with the serialization tests perturbs them.

Verification: GUI suite 178/178, the five affected files 8/8, backend batches clean, typecheck / lint:gui / privacy scan all green.

One governance note for the record, not a criticism of the change: GitHub shows this merged by its own author with no recorded review decision, and MAINTAINERS.md asks for maintainer approval with no self-approval. Treat this review plus the follow-up evidence as the post-merge gate.

---

_{Reviewed as part of a maintainer PABCD pass against dev at 4980410e. Plan unit: devlog/_plan/260726_pr_close_rework/050_wp5_gui_pages_cleanup_468.md.}