From 1838c5d0a7bd1ff41f9ffdcb8818904875c9e913 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 12:31:51 -0700 Subject: [PATCH 1/2] docs(renderer): plan the pane isolation review follow-ups Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013SULm3ApxebET2a8eLxKHd --- .../2026-09-07-pane-isolation-review-notes.md | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-07-pane-isolation-review-notes.md diff --git a/docs/superpowers/plans/2026-09-07-pane-isolation-review-notes.md b/docs/superpowers/plans/2026-09-07-pane-isolation-review-notes.md new file mode 100644 index 00000000..6df69197 --- /dev/null +++ b/docs/superpowers/plans/2026-09-07-pane-isolation-review-notes.md @@ -0,0 +1,171 @@ +### Task 2: Land Review B's three cosmetic notes from the #810 review + +**Files:** +- Modify: `src/renderer/src/features/reply-to-selection/lib/selectionStash.ts:42` +- Modify: `src/renderer/src/workspace/hook/helpers.ts:19-25` (the `EMPTY_RUNTIME` comment) +- Modify: `src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx:163-174` (the related-agent status dot) +- Modify: `src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx:61,77` +- Create: `docs/superpowers/plans/2026-09-07-pane-isolation-review-notes.md` (this section) + +**Interfaces:** +- Produces: a `data-related-status` attribute on the related-agent chip's status dot with values `'error' | 'attention' | 'running' | 'idle'`. + +Review B's notes, verbatim from the closed session: (1) "`selectionStash.ts:42` still names `setDraftVersion` in a WHY comment; the function is now `bumpDraftChanges`." (2) "The last sentence of the new `EMPTY_RUNTIME` comment in `helpers.ts` reads awkwardly." (3) "The new PaneHeader test detects the running state through a `.bg-accent` class query, which couples it to styling." + +- [ ] **Step 1: Create the worktree and commit the plan** + +```bash +cd /Users/juliusolsson/Desktop/Development/agent-code +git worktree add .worktrees/pane-isolation-review-notes -b chore/pane-isolation-review-notes origin/main +cd .worktrees/pane-isolation-review-notes +git submodule update --init && ln -s ../../node_modules node_modules +# copy this Task 2 section into docs/superpowers/plans/2026-09-07-pane-isolation-review-notes.md +git add docs/superpowers/plans/2026-09-07-pane-isolation-review-notes.md +git commit -m "docs(renderer): plan the pane isolation review follow-ups + +Co-Authored-By: Claude Fable 5.1 +Claude-Session: https://claude.ai/code/session_013SULm3ApxebET2a8eLxKHd" +``` + +- [ ] **Step 2: Fix the stale function name** + +In `src/renderer/src/features/reply-to-selection/lib/selectionStash.ts`, the comment currently reads: + +``` +// quote" from last week would be a bug, not a feature). Runtime writes +// also bump setDraftVersion, which would dirty the autosave path on +// every mouse drag. codeBlockRegistry.ts sets the precedent for +``` + +Change the middle line to: + +``` +// also call bumpDraftChanges, which would dirty the autosave path on +``` + +Confirm the name is current: `git grep -n "bumpDraftChanges" -- src/renderer/src/workspace/hook/actions/draft.ts` must list the declaration at line 39. + +- [ ] **Step 3: Rewrite the awkward invariant sentence** + +In `src/renderer/src/workspace/hook/helpers.ts`, replace the whole `EMPTY_RUNTIME` comment block (the lines from `// Missing sessions need a stable read-only fallback` through the line before `const EMPTY_RUNTIME = emptyRuntime()`) with: + +```ts +// Missing sessions need a stable read-only fallback so merely asking for their +// state cannot invalidate a memo boundary. Reducers still allocate their own. +// +// INVARIANT: this object is SHARED by every session that has no stored runtime +// yet and must never be mutated in place. Update paths always build fresh +// reducer-owned runtimes (spread + patch), so a `getRuntime()` result is only +// ever read. An in-place edit here would leak into every runtime-less session +// at once, because they all read this one object; the corruption would surface +// only after one of them was created, which is the worst moment to debug it. +``` + +- [ ] **Step 4: Give the status dot a semantic hook and use it in the test** + +In `src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx`, the dot inside the related-agent chip is: + +```tsx + +``` + +Add one attribute, computed from the same branches so the test and the color can never disagree: + +```tsx + +``` + +In `PaneHeader.phoneCoupling.renderer.test.tsx`, replace both occurrences of + +```ts + expect(chip!.querySelector('.bg-accent')).not.toBeNull() +``` + +with + +```ts + expect(chip!.querySelector('[data-related-status="running"]')).not.toBeNull() +``` + +- [ ] **Step 5: Run the affected tests and the type gate** + +```bash +source /opt/homebrew/opt/nvm/nvm.sh && nvm use 24 +NODE_ENV=test npx vitest run --project renderer src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx +npm run typecheck +``` +Expected: 1 file / 3 tests passed; typecheck exit 0. + +- [ ] **Step 6: Commit, push, open the PR** + +```bash +git add src/renderer/src/features/reply-to-selection/lib/selectionStash.ts src/renderer/src/workspace/hook/helpers.ts src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx +git commit -m "chore(renderer): tidy the comments and test hook noted in the pane isolation review + +The #810 review left three non-blocking notes: a comment naming a renamed +function, an invariant sentence that read badly, and a regression test that +detected the running state through a Tailwind class. The test now reads a +data attribute computed from the same branches as the color, so styling +changes cannot break it and the attribute cannot drift from the color. + +Refs #763 + +Co-Authored-By: Claude Fable 5.1 +Claude-Session: https://claude.ai/code/session_013SULm3ApxebET2a8eLxKHd" +git push -u origin chore/pane-isolation-review-notes +gh pr create --title "chore(renderer): tidy the comments and test hook noted in the pane isolation review" --body-file - <<'EOF' +## Problem +Review B of #810 left three non-blocking notes that were never actioned: a WHY comment in `selectionStash.ts` still named `setDraftVersion` (renamed to `bumpDraftChanges` in #810), the new `EMPTY_RUNTIME` invariant comment ended in a sentence that read badly, and the new `PaneHeader.phoneCoupling` test detected the running state through a `.bg-accent` class query. + +## Change +- Comment names the current function. +- Invariant comment rewritten; same invariant, plain sentences. +- The related-agent status dot gets `data-related-status="error|attention|running|idle"` computed from the same branches as its color; the test queries that attribute. + +## Verification +- `PaneHeader.phoneCoupling.renderer.test.tsx`: 3 tests pass on Node 24. +- `npm run typecheck` clean. + +Refs #763 + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +https://claude.ai/code/session_013SULm3ApxebET2a8eLxKHd +EOF +``` + +CHECKPOINT: report the PR link; merge only on confirmation. + +--- + From e06de832f65a4aed40e9c46430b8971c1bf55bb0 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 12:33:13 -0700 Subject: [PATCH 2/2] chore(renderer): tidy the comments and test hook noted in the pane isolation review The #810 review left three non-blocking notes: a comment naming a renamed function, an invariant sentence that read badly, and a regression test that detected the running state through a Tailwind class. The test now reads a data attribute computed from the same branches as the color, so styling changes cannot break it and the attribute cannot drift from the color. Refs #763 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013SULm3ApxebET2a8eLxKHd --- .../reply-to-selection/lib/selectionStash.ts | 2 +- src/renderer/src/workspace/hook/helpers.ts | 12 ++++++------ .../PaneHeader.phoneCoupling.renderer.test.tsx | 4 ++-- .../src/workspace/tile-tree/TileLeaf/PaneHeader.tsx | 9 +++++++++ 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/renderer/src/features/reply-to-selection/lib/selectionStash.ts b/src/renderer/src/features/reply-to-selection/lib/selectionStash.ts index efd97180..eb96a5c9 100644 --- a/src/renderer/src/features/reply-to-selection/lib/selectionStash.ts +++ b/src/renderer/src/features/reply-to-selection/lib/selectionStash.ts @@ -39,7 +39,7 @@ import type { SessionId } from '@renderer/workspace/types' // of those things — it is ephemeral and DOM-derived, and it must never // be autosaved or rehydrated (resuming a session with a stale "pending // quote" from last week would be a bug, not a feature). Runtime writes -// also bump setDraftVersion, which would dirty the autosave path on +// also call bumpDraftChanges, which would dirty the autosave path on // every mouse drag. codeBlockRegistry.ts sets the precedent for // DOM-derived data living in module scope. // diff --git a/src/renderer/src/workspace/hook/helpers.ts b/src/renderer/src/workspace/hook/helpers.ts index bc73a38b..cc6726cf 100644 --- a/src/renderer/src/workspace/hook/helpers.ts +++ b/src/renderer/src/workspace/hook/helpers.ts @@ -17,12 +17,12 @@ import { commandTargetSessionIdForState } from '@renderer/workspace/hook/selecto // Missing sessions need a stable read-only fallback so merely asking for their // state cannot invalidate a memo boundary. Reducers still allocate their own. // -// INVARIANT: this object is SHARED across every missing session and must never -// be mutated in place. Update paths always build fresh reducer-owned runtimes -// (spread + patch), so a `getRuntime()` result is only ever read; an in-place -// edit on it would silently contaminate every session that has no stored -// runtime yet — until one session is created, then exactly one shared object -// is shared by all of them at once (the worst possible time to corrupt it). +// INVARIANT: this object is SHARED by every session that has no stored runtime +// yet and must never be mutated in place. Update paths always build fresh +// reducer-owned runtimes (spread + patch), so a `getRuntime()` result is only +// ever read. An in-place edit here would leak into every runtime-less session +// at once, because they all read this one object; the corruption would surface +// only after one of them was created, which is the worst moment to debug it. const EMPTY_RUNTIME = emptyRuntime() // ----------------------------------------------------------------------------- diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx index 52f3b185..6bec420a 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.phoneCoupling.renderer.test.tsx @@ -58,7 +58,7 @@ describe('PaneHeader related-status store coupling', () => { ) const chip = container.querySelector('button') expect(chip).not.toBeNull() - expect(chip!.querySelector('.bg-accent')).not.toBeNull() + expect(chip!.querySelector('[data-related-status="running"]')).not.toBeNull() }) it('derives related status from the store when the key exists (desktop path)', () => { @@ -74,6 +74,6 @@ describe('PaneHeader related-status store coupling', () => { ) const chip = container.querySelector('button') expect(chip).not.toBeNull() - expect(chip!.querySelector('.bg-accent')).not.toBeNull() + expect(chip!.querySelector('[data-related-status="running"]')).not.toBeNull() }) }) \ No newline at end of file diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx index 8ddf3eae..b3fc0590 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf/PaneHeader.tsx @@ -161,6 +161,15 @@ export function PaneHeader({ ].join(' ')} >