2 commits

Author	SHA1	Message	Date
Bryan Helmkamp	62f0b3e7d1	refactor(web): improve React Doctor score (#405) ... ## Summary Improves the web UI's React Doctor audit score by separating reusable helpers from React component modules, tightening effect/state ownership, and extracting real component boundaries in the install wizard, stage activity view, run-files diff browser, RunDetail route, and Runs workspace. The branch removes the previously deferred RunDetail and Runs giant-component diagnostics without changing RunDetail UX, route contracts, action ordering, or Runs workspace behavior. | Metric | Main baseline | Initial PR | Current PR | |--------|---------------|------------|------------| | React Doctor score | 63 | 71 | 99 | | React Doctor errors | 123 | 0 | 0 | | React Doctor warnings | 241 | 163 | 3 | | React Doctor diagnostics | 364 | 163 | 3 | ## Changes - Moves exported helper logic out of component files so Fast Refresh/component-export rules no longer dominate the audit. - Adds a targeted React Doctor config exception for React Router route modules, where non-component exports like route metadata are intentional. - Refactors low-risk state/effect patterns: keyed interview question state, reducer-backed editable run title state, event-owned preview opening, route-keyed insights editor initialization, refresh timer ownership, and selection/derived list cleanup. - Reworks `InstallApp` around an install reducer, a controller hook for install lifecycle state, and focused wizard step components for LLM, server, object-store, sandbox, and GitHub setup. - Moves `RunStages` selected-stage activity into a keyed boundary for panel/debug detail state while preserving stage activity filters across navigation. - Extracts the `RunFiles` loaded diff-browser view from route/query coordination so the route owns data/URL state and the loaded view owns rendering. - Splits `RunDetail` into route-local header, actions, tab shell, docked controls, model, and lifecycle-toast modules; the actions menu now uses grouped descriptors instead of a large boolean/callback prop matrix. - Extracts Runs workspace preference ownership into `useRunsWorkspacePreferences` and moves toolbar rendering into `RunsToolbar`, leaving the route focused on data, DnD state, filtering, and view selection. - Guards `InsightsEditor` query execution with a latest-run id and timeout cleanup so stale or unmounted mock query runs cannot overwrite newer results. - Adds regression coverage for archived-run deletion from RunDetail and stale-result handling in InsightsEditor. - Improves semantic/accessibility coverage with labeled controls, native meter/section semantics, decorative status dots, and clearer unavailable copy. - Removes dead UI code and applies local suppressions only where the rule is a documented false positive or an intentional imperative integration boundary. ## Remaining React Doctor warnings Current score is 99 with 0 errors and 3 warnings. The remaining warnings are intentionally left for separate judgment rather than mechanical churn: - `prefer-useReducer` (3): `AutomationsNew`, `InsightsEditor`, and `CreateSecretForm` need reducers only if they encode real coupled transitions, not simple field setters. ## Verification - `cd apps/fabro-web && bun test app/routes/run-detail.test.ts` -> `22 pass`, `0 fail` - `cd apps/fabro-web && bun test app/routes/insights-editor.test.tsx app/routes/runs.preferences.test.tsx` -> `7 pass`, `0 fail` - `cd apps/fabro-web && bun run typecheck` - `cd apps/fabro-web && bun test --isolate` -> `490 pass`, `0 fail` - `cd apps/fabro-web && bunx react-doctor@latest --full --json > /tmp/fabro-react-doctor-runs-insights.json` -> score `99`, `0` errors, `3` warnings - Earlier branch verification also included `cd apps/fabro-web && bun run build` - `git diff --check` --- [](https://github.com/EveryInc/compound-engineering-plugin) 🤖 Generated with GPT-5 (context not reported, default reasoning) via [Codex](https://openai.com/codex)	2026-05-25 22:41:37 -04:00
Bryan Helmkamp	7ac15b28f0	feat(fabro-web): port /chats/new + /chats/:id from prototype (#289) ... ## Summary Ports the validated `/chats/new` and `/chats/:id` chat surface from `docs/superpowers/prototypes/2026-05-16-chats-new/` into `apps/fabro-web`. Client-side scripted prototype mounted inside the existing `AppShell`; replaces `/start` as the planned new "kick off agent work" entry point (but does not delete `/start` in this phase). - New routes: `/chats/new` (empty-state composer) and `/chats/:chatId` (active conversation with assistant-ui's `<Thread>`, scripted streaming replies, markdown + tool-call rendering). - Drives `@assistant-ui/react` + `@assistant-ui/react-ui` via `useLocalRuntime` and a custom `ChatModelAdapter` that cycles a 6-entry scripted reply bank. - Tailwind v4 cascade fix: assistant-ui CSS is now imported via `@layer assistant-ui` so v4 utilities cascade above the package's unlayered scoped preflight. Includes a discovered Bun-specific tweak — see Notable Deviations below. - StrictMode-safe first-message handoff: store seeds the user message into `seedMessages` with a `pendingResponse: true` flag, and `chats-detail` triggers a single `runtime.thread.startRun({ parentId: null })` then consumes the flag. Avoids the prototype's autorespond-lost-stream race under React 19 StrictMode. The Ask-Fabro right sidebar (also in the prototype) is **out of scope** for this PR. Companion spec: [`docs/superpowers/specs/2026-05-16-chats-new-prototype-design.md`](../tree/chats-new-port/docs/superpowers/specs/2026-05-16-chats-new-prototype-design.md) Implementation plan: [`docs/superpowers/plans/2026-05-16-chats-new-fabro-web-port.md`](../tree/chats-new-port/docs/superpowers/plans/2026-05-16-chats-new-fabro-web-port.md) ## Screenshots Captured from a local debug `fabro server` running this branch's binary, signed in via GitHub. ### `/chats/new` (empty state)  ### `/chats/:chatId` (active conversation)  ## Files **New** (under `apps/fabro-web/`): - `app/lib/chats-types.ts` — `Chat` wrapper + `ChatContentPart` discriminated union over the API client's `CompletionContentPart` - `app/lib/chats-script.ts` — 6-entry scripted reply bank (`CompletionMessage[]`) - `app/lib/chats-store.tsx` — Context + `useReducer` for chat metadata, `pendingResponse` flag, scriptIndex - `app/lib/chats-runtime.ts` — `createScriptedAdapter` + `toThreadMessages` boundary converter - `app/lib/test-utils.tsx` — minimal `renderHook` shim (lifts the duplicated `IS_REACT_ACT_ENVIRONMENT` + dep-warning silencing pattern out of `install-app.test.tsx`) - `app/components/chats/{tool-fallback,composer-chips,custom-composer}.tsx` - `app/routes/{chats-layout,chats-new,chats-detail}.tsx` - Tests: `chats-store.test.tsx` (5), `chats-runtime.test.ts` (4), `chats-router.test.tsx` (3) **Modified:** - `package.json` — adds `@assistant-ui/{react,react-ui,react-markdown}` (pinned exactly to versions verified in the prototype) - `app/app.css` — `@layer` declaration + assistant-ui CSS imports into `layer(assistant-ui)` + `.fabro-chat` `--aui-*` variable overrides mapping to the Fabro palette - `app/root.tsx` — removed `import "./app.css"` (see Notable Deviations) - `app/router.tsx` — wires the chats routes under the AppShell tree ## Notable deviations from the plan Two intentional deviations, both explained in their commit bodies: 1. **`apps/fabro-web/app/root.tsx` no longer imports `./app.css`.** Bun's CSS bundler (used by `Bun.build` on `entry.tsx`) rejects spec-valid `@layer name, name;` ordering between `@import` rules, even though Tailwind's CLI accepts it. The CSS is built standalone by the Tailwind CLI step in `scripts/build.ts` and linked from `index.template.html`, so dropping the JS-side import bypasses Bun's parser without any runtime change. A safety-net comment at the top of `app.css` warns future engineers against re-adding the import. Commit: `c37690be9`. 2. **`!` non-null assertions removed** in two places where the verbatim prototype copy violated the global CLAUDE.md rule banning `!` in production code: `chats-script.ts` now uses a typed `FALLBACK_REPLY` and `??` coalescing; `composer-chips.tsx` lifts the first option of each chip into a `DEFAULT_*` constant. `chats-runtime.test.ts`'s `for await` drain loops were also replaced with `Array.fromAsync(...)` per the no-loops-in-tests rule. Commits: `ace6ac6d4`, `652ad97af`. ## Test plan - [x] `cd apps/fabro-web && bun run typecheck` — clean - [x] `bun test` — 383 pass / 0 fail (12 new tests for chats) - [x] `cd apps/fabro-web && bun run build` — succeeds; assistant-ui CSS bundled into `dist/assets/app.css` - [x] **Manual browser smoke test** — debug `fabro` binary running this branch served `/chats/new` and `/chats/seed_email` correctly inside the real AppShell with GitHub-OAuth auth (screenshots above). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-16 17:10:16 -04:00