perf(sessions): drop spinner-only screen frames and alias duplicate strings on the wire

docs/superpowers/plans/2026-09-03-screen-frame-spinner-gate.md

@@ -0,0 +1,89 @@
+# Screen frames: drop spinner-only repaints and alias duplicate strings
+
+## 2026-09-04 review correction
+
+The original global duration/token/whitespace substitutions below are superseded:
+only complete recognized status/footer lines are normalized; multiline composer
+regions and ordinary content remain verbatim. The analogous headless package
+fix retains attribute-aware composer/condition parsing on every emitted frame.
+Successful session recovery now seeds the requesting renderer from the latest
+raw screen cache. This bypasses no ownership/recovery failure and does not reset
+the shared gate or broadcast to unrelated windows. Twenty-six focused gate,
+manager, IPC recovery and wire tests pass; full CI runs on the integrated head.
+
+Fixes #746. Refs #390, #103.
+
+## Problem
+
+`session:screen` is the highest-volume IPC channel and almost all of it is
+redundant. Recording `794dafdd` (13.2 min, one Claude session): 4,728
+frames at 6 Hz, 8.3 KB each; `recent === plain` in every frame; 99% of
+consecutive frames differ only in the spinner line (glyph rotation, the
+elapsed timer, the token counter) or the `/rc connecting…` blink. Today's
+recordings confirm the shape: 750/750 Claude frames with `recent === plain`
+and 533/750 consecutive pairs differing in one line. Every one of those
+frames is cloned through the coalescer, sent to the renderer, committed as
+a new runtime object (re-rendering the workspace), forwarded to the remote
+client and JSON-stringified by the session recorder.
+
+The headless terminal already gates on EXACT equality of `plain`/`recent`
+(`HeadlessTerminal.ts`), which never fires while a spinner animates.
+
+## Design
+
+1. **Volatile-text gate at the manager's `screen` re-emit**
+   (`sessionManager.ts`, the one point feeding the forwarder, the remote
+   server and the recorder). `normalizeVolatileScreenText` rewrites the
+   known-volatile shapes — spinner glyphs at line start (Claude `·✢✳✶✻✽`,
+   Codex braille), elapsed timers (`5s`, `1m 9s`), token counters
+   (`↓ 1.2k tokens`), the `/rc connecting…` suffix, trailing whitespace —
+   to fixed tokens. A frame whose normalized `plain` and `recent` equal the
+   last EMITTED frame's is dropped; anything else is emitted RAW (the
+   renderer still sees the real spinner on every real change).
+   `lastScreenSnapshot` keeps the raw latest for MCP/debug readers and the
+   provider sessions' own consumers (composer-ready, prompt gate) run before
+   the manager and are untouched. Condition parsers run in the headless
+   package on every raw frame.
+
+   WHY normalize-and-compare rather than a time throttle: a throttle still
+   emits N frames per second of pure spinner while an agent thinks for
+   minutes; the gate emits zero and still passes a composer keystroke or a
+   new output line within the same 100 ms tick.
+
+2. **Alias duplicate strings on the renderer wire.** At the IPC edge the
+   forwarder omits `recent`/`recentMarkdown` when they equal
+   `plain`/`markdown` (`aliasScreenSnapshotForWire`); the preload's
+   `onSessionScreen` expands them back (`expandScreenSnapshotFromWire`) so
+   every renderer type and consumer is unchanged. The remote server takes
+   the full payload from the manager. The session recorder taps the IPC
+   send, so recordings carry the wire form; replay treats `session:screen`
+   as a no-op tick (`reconstructSlices.ts`) and the redaction pass caps
+   fields only when present, so nothing downstream reads the omitted keys.
+
+Not in scope: removing the markdown walks from the headless package
+(submodule), and not sending `markdown` unless a debug consumer is mounted.
+
+## Verification
+
+- `screenFrameGate.test.ts`: real spinner/timer/rc lines from recordings
+  normalize to the same string across ticks; a composer keystroke, a new
+  output line and a picker-only change (selection moves, picker closes)
+  are emitted; the first frame is always emitted; a session's state is
+  dropped on removal; a frame that changes and then ticks emits once.
+- `sessionManager.screenGate.test.ts`: a spinner tick does not reach
+  `manager.on('screen')` listeners but does update `getScreenSnapshot`;
+  a real change reaches them again.
+- `screenSnapshotWire.test.ts`: the alias/expand helpers drop duplicate
+  fields, keep a different window verbatim and round-trip. The forwarder
+  and preload changes are one-line delegations to those helpers and have
+  no suites of their own.
+- `npx tsc -b`; `src/main/sessions`, `sessionManager.wake` and the
+  session-feed renderer suite.
+
+## Known limitations
+
+- The remote client renders `recent` as a terminal mirror; while an agent
+  thinks it now shows the spinner/timer frozen at the last real change
+  rather than animating. `workingStatus` (process-state) is unaffected.
+- Gate state is reset when a session is removed and when a superseded
+  backend is replaced under the same id (the `lastGateEvaluation` site).

src/main/ipc/session.test.ts

@@ -17,8 +17,38 @@ vi.mock('@providers/registry.main.js', () => ({
   getMainProvider: () => ({ listSessions: harness.listSessions }),
 }))
 
+vi.mock('@main/window/windowRegistry.js', () => ({
+  claimSessionForWindow: vi.fn(),
+  releaseSession: vi.fn(),
+  windowIdFor: () => 'requesting-window',
+}))
+
 const { registerSessionIpc } = await import('./session.js')
 
+describe('recovered renderer screen seed', () => {
+  it.each([
+    { ok: true, destroyed: false, available: true, sends: 1 },
+    { ok: false, destroyed: false, available: true, sends: 0 },
+    { ok: true, destroyed: true, available: true, sends: 0 },
+    { ok: true, destroyed: false, available: false, sends: 0 },
+  ])('seeds only successful live requesters ($ok/$destroyed/$available)', async ({ ok, destroyed, available, sends }) => {
+    const screen = { plain: 'latest raw tick', markdown: 'latest raw tick', recent: 'latest raw tick', recentMarkdown: 'latest raw tick' }
+    const recover = vi.fn(async () => ({ ok }))
+    const getScreenSnapshot = vi.fn(() => available ? screen : null)
+    registerSessionIpc({ recover, getScreenSnapshot } as never, {} as never)
+    const sender = { isDestroyed: () => destroyed, send: vi.fn() }
+    await expect(harness.handlers.get('session:recover')!({ sender }, { sessionId: 's1' })).resolves.toEqual({ ok })
+    expect(sender.send).toHaveBeenCalledTimes(sends)
+    if (sends) {
+      expect(getScreenSnapshot).toHaveBeenCalledWith('s1')
+      expect(recover.mock.invocationCallOrder[0]).toBeLessThan(getScreenSnapshot.mock.invocationCallOrder[0]!)
+      expect(sender.send).toHaveBeenCalledWith('session:screen', {
+        sessionId: 's1', plain: screen.plain, markdown: screen.markdown,
+      })
+    }
+  })
+})
+
 describe('session input transcript observations', () => {
   beforeEach(() => {
     harness.handlers.clear()

src/main/ipc/session.ts

@@ -21,6 +21,7 @@ import type {
   SessionRecoveryCancellationOptions,
   SessionRecoverOptions,
 } from '@shared/types/session.js'
+import { aliasScreenSnapshotForWire } from '@shared/types/session.js'
 import {
   claimSessionForWindow,
   releaseSession,
@@ -95,7 +96,19 @@ export function registerSessionIpc(
     // id it is restoring — so the claim can happen before the call rather than
     // through a mint hook.
     claimSessionForWindow(options.sessionId, windowIdFor(evt.sender))
-    return await manager.recover(options)
+    const result = await manager.recover(options)
+    // A fresh/reloaded renderer has no previous screen even when this backend
+    // is already live. The spinner gate may now suppress every subsequent
+    // repaint, and an idle backend may emit none. Seed this requesting renderer
+    // from the latest RAW cache after successful recovery; do not reset the
+    // global gate or broadcast to unrelated windows just to satisfy one joiner.
+    // Read after await so a frame received during recovery cannot be replayed
+    // behind a newer cached value. Failed/conflicting recoveries reveal nothing.
+    if (result.ok && !evt.sender.isDestroyed()) {
+      const screen = manager.getScreenSnapshot(options.sessionId)
+      if (screen) evt.sender.send('session:screen', aliasScreenSnapshotForWire({ sessionId: options.sessionId, ...screen }))
+    }
+    return result
   })
 
   ipcMain.handle(

src/main/sessionManager.screenGate.test.ts

@@ -0,0 +1,82 @@
+import { EventEmitter } from 'node:events'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { createSession, createTerminalSession } = vi.hoisted(() => ({
+  createSession: vi.fn(),
+  createTerminalSession: vi.fn(),
+}))
+
+vi.mock('@providers/registry.main.js', () => ({
+  getMainProvider: () => ({
+    name: 'Claude',
+    createSession,
+    createTerminalSession,
+  }),
+}))
+
+vi.mock('@main/setup/toolchain.js', () => ({
+  getToolPath: () => '/usr/bin/true',
+}))
+
+vi.mock('@main/performance/PerformanceService.js', () => ({
+  performanceService: {
+    mark: vi.fn(),
+    record: vi.fn(),
+    error: vi.fn(),
+    metric: vi.fn(),
+    span: () => ({ end: vi.fn(), fail: vi.fn() }),
+  },
+}))
+
+vi.mock('@main/storage/feedDebugLog.js', () => ({
+  forgetFeedDebugSession: vi.fn(),
+}))
+
+class FakeAgentSession extends EventEmitter {
+  async start(): Promise<void> {
+    this.emit('started', { projectDir: '/tmp/project' })
+  }
+
+  async stop(): Promise<void> {}
+
+  write(): void {}
+
+  resize(): void {}
+}
+
+// #746: spinner-only repaints must not reach the manager's listeners (the
+// forwarder, the remote server, the recorder) but must still update the raw
+// snapshot cache that MCP/debug readers consult.
+
+const THINKING = "> fix it\n\n✻ Beboppin'… (5s · thinking…)\n"
+const THINKING_TICK = "> fix it\n\n✽ Beboppin'… (6s · thinking…)\n"
+const OUTPUT = THINKING_TICK + '⏺ Read(README.md)\n'
+
+function frame(plain: string) {
+  return { plain, markdown: plain, recent: plain, recentMarkdown: plain, picker: { visible: false, items: [] } }
+}
+
+describe('SessionManager screen-frame gate', () => {
+  beforeEach(() => {
+    createSession.mockReset()
+    createTerminalSession.mockReset()
+  })
+
+  it('drops a spinner tick for listeners but keeps the raw snapshot current', async () => {
+    const { SessionManager } = await import('./sessionManager')
+    const session = new FakeAgentSession()
+    createSession.mockImplementation(() => session)
+    const manager = new SessionManager()
+    const seen: string[] = []
+    manager.on('screen', payload => seen.push((payload as { plain: string }).plain))
+
+    const { sessionId } = await manager.spawn({ kind: 'claude', cwd: '/tmp/project' })
+    session.emit('screen', frame(THINKING))
+    session.emit('screen', frame(THINKING_TICK))
+    expect(seen).toEqual([THINKING])
+    expect(manager.getScreenSnapshot(sessionId)?.plain).toBe(THINKING_TICK)
+
+    session.emit('screen', frame(OUTPUT))
+    expect(seen).toEqual([THINKING, OUTPUT])
+  })
+})

src/main/sessionManager.ts

@@ -40,6 +40,7 @@ import { resolveToolPath } from '@main/setup/binaryResolver.js'
 import { updateToolPaths } from '@main/setup/setupState.js'
 import { forgetFeedDebugSession } from '@main/storage/feedDebugLog.js'
 import { CappedTextBuffer } from '@main/sessions/cappedTextBuffer.js'
+import { ScreenFrameGate } from '@main/sessions/screenFrameGate.js'
 import type {
   ConditionCustomAction,
   ProviderConditionSnapshot,
@@ -469,6 +470,8 @@ export class SessionManager extends EventEmitter {
   // per event (a reference store, no clone); entries die with the session
   // in cleanupSessionState.
   private readonly lastScreenSnapshot = new Map<string, AgentScreenSnapshot>()
+  // See screenFrameGate.ts — drops spinner-only repaints before they fan out.
+  private readonly screenFrameGate = new ScreenFrameGate()
   private readonly lastConditionsSnapshot = new Map<string, ProviderConditionSnapshot>()
   private readonly lastInputReadiness = new Map<string, SessionInputReadiness>()
   // WHY this is one manager-global sequence instead of a bounded per-id map:
@@ -856,6 +859,7 @@ export class SessionManager extends EventEmitter {
     // session's screen/conditions to a late subscriber would present it as
     // live (the exact stale-state bug the remote late-joiner replay had).
     this.lastScreenSnapshot.delete(sessionId)
+    this.screenFrameGate.forget(sessionId)
     this.lastConditionsSnapshot.delete(sessionId)
     this.lastTranscriptFile.delete(sessionId)
     this.codexCandidateObservationEdges.delete(sessionId)
@@ -2671,7 +2675,25 @@ export class SessionManager extends EventEmitter {
       session.on('screen', (snap: AgentScreenSnapshot) => {
         if (!ownsEntry()) return
         this.markActivity(sessionId)
+        // Raw latest for MCP/debug readers (getScreenSnapshot) — kept even
+        // for frames the gate below drops, so a reader always sees the
+        // current spinner state.
         this.lastScreenSnapshot.set(sessionId, snap)
+        // WHY frames are gated HERE (#746): this emit is the single feed for
+        // the renderer forwarder, the remote server and the session
+        // recorder, and 99% of consecutive frames while an agent thinks
+        // differ only in spinner chrome. Everything that must see every raw
+        // frame (condition parsers, composer-ready, the prompt gate) already
+        // ran inside the headless package or the provider session.
+        const droppedBefore = this.screenFrameGate.droppedBeforeLastEmit(sessionId)
+        if (!this.screenFrameGate.shouldEmit(sessionId, snap)) return
+        // One journal sample per EMITTED frame carrying the run of drops it
+        // ended; a per-drop record would cost more than the frames saved.
+        if (droppedBefore > 0) {
+          performanceService.metric('session.screen.gate.dropped', droppedBefore, 'sample', {
+            sessionId,
+          })
+        }
         this.emit('screen', { sessionId, ...snap })
       })
       session.on('jsonl-entry', (
@@ -2825,6 +2847,10 @@ export class SessionManager extends EventEmitter {
       // inherit the dead one's `since` (fabricating elapsed) and, because the
       // first verdict then looked unchanged, skip arming the sampler entirely.
       this.lastGateEvaluation.delete(sessionId)
+      // Same reasoning for the screen-frame gate (#761 review): a superseded
+      // backend's last normalized frame must not swallow the fresh one's
+      // identical-looking first frame.
+      this.screenFrameGate.forget(sessionId)
       this.rememberSessionId(sessionId)
       this.throwIfSpawnCancelled(recoveryClaim, codexReplacementHandoff)
       // Hoisted out of the try so the catch can report a duration too: a

src/main/sessions/forwarder.ts

@@ -1,4 +1,6 @@
 import type { SessionManager } from '@main/sessionManager.js'
+import { aliasScreenSnapshotForWire } from '@shared/types/session.js'
+import type { AgentScreenSnapshot } from '@shared/types/session.js'
 import type { LspManager } from '@main/lspManager.js'
 
 import {
@@ -53,8 +55,14 @@ export function wireSessionForwarder(
   const subAgents = new SubAgentWatcherManager((sessionId, map) =>
     sendToSessionWindow(sessionId, 'session:sub-agents', { sessionId, subAgents: map }),
   )
-  const screens = new LatestSessionIpcCoalescer(payload =>
-    sendToSessionWindow(payload.sessionId, 'session:screen', payload),
+  // WHY the alias happens here and not at the manager (#746): the remote
+  // server and the recorder-independent readers take the full payload from
+  // the manager; only the renderer IPC edge pays structured-clone bytes for
+  // the duplicate `recent` strings, and only the preload expands them back.
+  // (The session recorder taps this send, so recordings carry the wire form;
+  // replay treats screen frames as no-op ticks and never reads the fields.)
+  const screens = new LatestSessionIpcCoalescer<{ sessionId: string } & AgentScreenSnapshot>(payload =>
+    sendToSessionWindow(payload.sessionId, 'session:screen', aliasScreenSnapshotForWire(payload)),
   )
   const processStates = new LatestSessionIpcCoalescer(payload =>
     sendToSessionWindow(payload.sessionId, 'session:process-state', payload),