Extract buildSubmitText and inline #buildStatusLine (step 5e)

.claude/sessions/2026-04-07.md

@@ -137,3 +137,45 @@ Continuation of the same session after context compaction.
 **Preview gate moved inside renderer:** In AppLayout, `buildPreviewRows` was only called when `previewMode && commandMode`. Moving that guard inside `renderCommandMode` makes the function self-contained — callers don't need to know the precondition.
 
 **`isLikelyPath` and async clipboard/stat code stay in AppLayout:** These are I/O operations, not state. They belong with `#handleCommandKey` which stays in AppLayout as part of the event dispatch layer. Step 5e will decide whether this warrants further extraction.
+
+
+---
+
+## Step 5e — ScreenCoordinator cleanup (PR #199)
+
+Final step in the AppLayout refactor series.
+
+**New `buildSubmitText.ts`** — pure function, no I/O:
+- `buildSubmitText(text: string, attachments: readonly Attachment[] | null): string`
+- Assembles final message string from editor text + optional attachment blocks
+- Each attachment becomes an `[attachment #N]...[/attachment]` block appended after the main text
+- Returns `text` unchanged when no attachments (null or empty), so callers don't special-case the zero-attachment path
+- Extracted from the ctrl+enter handler in AppLayout — was ~25 inline lines with no tests
+
+**`AppLayout.ts` changes:**
+- Inline `#buildStatusLine`: was a one-liner delegation to `renderStatus()`. Removed the method, call site now reads `renderStatus(this.#statusState, cols)` directly.
+- ctrl+enter handler: 28 lines of attachment serialization → `buildSubmitText(text, attachments)` (3 lines)
+- Net: AppLayout loses 34 lines; all remaining code is coordination glue (screen, event loop, state wiring)
+
+**Tests:** 19 new in `buildSubmitText.spec.ts`. Total: 338 (up from 319).
+
+## What AppLayout now contains
+
+- Screen management: `#screen`, enter/exit, resize listener
+- Mode flag (`editor` / `streaming`)
+- Five state objects: `#conversationState`, `#editorState`, `#statusState`, `#toolApprovalState`, `#commandModeState`
+- Two promise holders: `#editorResolve`, `#cancelFn`
+- Render debounce: `#renderPending` + `#scheduleRender()`
+- `render()`: orchestrates all five renderers + screen write
+- `#flushToScroll()`: exits alt buffer, writes sealed blocks, re-enters
+- `handleKey()`: routes keys to the right state objects
+- `#handleCommandKey()`: async I/O (clipboard reads, stat) for the `t`/`f` command mode keys
+- `isLikelyPath()`: pure helper for path detection in `#handleCommandKey`
+
+## Decisions
+
+**Inline `#buildStatusLine`**: It was a one-liner with no logic of its own. The only reason to have a private method is to give a name to something complex enough to warrant one. `renderStatus(this.#statusState, cols)` is already self-explanatory at the call site.
+
+**`isLikelyPath` stays in AppLayout**: Used only in `#handleCommandKey`. Moving it to clipboard.ts or a utils file would create a dependency for 8 lines that only ever live in one context. The benefit doesn’t justify the churn.
+
+**togglePreview invariant not addressed**: `CommandModeState.togglePreview()` can set `previewMode = true` when `commandMode = false`. The renderer handles this correctly (returns empty previewRows), the original code had the same behaviour, and fixing it would change the state contract in a way that breaks existing tests. Reviewer noted this as non-blocking; leaving as-is.

CLAUDE.md

@@ -53,9 +53,9 @@ Refactoring `AppLayout.ts` into focused, testable units (milestone 1.0 prerequis
 | 5b ConversationState + renderConversation | ✅ Done | #196 |
 | 5c ToolApprovalState + renderToolApproval | ✅ Done | #197 |
 | 5d CommandModeState + renderCommandMode | ✅ Done | #198 |
-| 5e ScreenCoordinator cleanup | ⏳ Next | — |
+| 5e ScreenCoordinator cleanup | ✅ Done | #199 |
 
-Test count: 319 across 13 spec files.
+Test count: 338 across 14 spec files. Refactor series complete.
 
 ## Recent Decisions
 

apps/claude-sdk-cli/src/AppLayout.ts

@@ -6,6 +6,7 @@ import { sanitiseLoneSurrogates } from '@shellicar/claude-core/sanitise';
 import type { Screen } from '@shellicar/claude-core/screen';
 import { StdoutScreen } from '@shellicar/claude-core/screen';
 import type { SdkMessageUsage } from '@shellicar/claude-sdk';
+import { buildSubmitText } from './buildSubmitText.js';
 import { CommandModeState } from './CommandModeState.js';
 import type { Block, BlockType } from './ConversationState.js';
 import { ConversationState } from './ConversationState.js';
@@ -280,37 +281,9 @@ export class AppLayout implements Disposable {
       return;
     }
     const attachments = this.#commandModeState.takeAttachments();
-    const parts: string[] = [text];
-    if (attachments) {
-      for (let n = 0; n < attachments.length; n++) {
-        const att = attachments[n];
-        if (!att) {
-          continue;
-        }
-        if (att.kind === 'text') {
-          const showSize = att.sizeBytes >= 1024 ? `${(att.sizeBytes / 1024).toFixed(1)}KB` : `${att.sizeBytes}B`;
-          const fullSize = att.fullSizeBytes >= 1024 ? `${(att.fullSizeBytes / 1024).toFixed(1)}KB` : `${att.fullSizeBytes}B`;
-          const truncPrefix = att.truncated ? `// showing ${showSize} of ${fullSize} (truncated)\n` : '';
-          parts.push(`\n\n[attachment #${n + 1}]\n${truncPrefix}${att.text}\n[/attachment]`);
-        } else {
-          const lines: string[] = [`path: ${att.path}`];
-          if (att.fileType === 'missing') {
-            lines.push('// not found');
-          } else {
-            lines.push(`type: ${att.fileType}`);
-            if (att.fileType === 'file' && att.sizeBytes !== undefined) {
-              const sz = att.sizeBytes;
-              const sizeStr = sz >= 1024 ? `${(sz / 1024).toFixed(1)}KB` : `${sz}B`;
-              lines.push(`size: ${sizeStr}`);
-            }
-          }
-          parts.push(`\n\n[attachment #${n + 1}]\n${lines.join('\n')}\n[/attachment]`);
-        }
-      }
-    }
     const resolveInput = this.#editorResolve;
     this.#editorResolve = null;
-    resolveInput(parts.join(''));
+    resolveInput(buildSubmitText(text, attachments));
   }
 
   #flushToScroll(): void {
@@ -351,7 +324,7 @@ export class AppLayout implements Disposable {
     const visibleRows = overflow > 0 ? allContent.slice(overflow) : [...new Array<string>(contentRows - allContent.length).fill(''), ...allContent];
 
     const separator = buildDivider(null, cols);
-    const statusLine = this.#buildStatusLine(cols);
+    const statusLine = renderStatus(this.#statusState, cols);
     const allRows = [...visibleRows, separator, statusLine, approvalRow, commandRow, ...expandedRows];
 
     let out = syncStart + hideCursor;
@@ -439,8 +412,4 @@ export class AppLayout implements Disposable {
     }
     // All other keys silently consumed
   }
-
-  #buildStatusLine(cols: number): string {
-    return renderStatus(this.#statusState, cols);
-  }
 }

apps/claude-sdk-cli/src/buildSubmitText.ts

@@ -0,0 +1,44 @@
+import type { Attachment } from './AttachmentStore.js';
+
+/**
+ * Assemble the final message string from editor text and optional attachments.
+ *
+ * Each attachment becomes an [attachment #N] block appended after the main text.
+ * The format is part of the system prompt contract — the agent is told how these
+ * blocks are structured so it can reference attached content correctly.
+ *
+ * Returns plain text unchanged when there are no attachments, so callers do not
+ * need to handle the zero-attachment case specially.
+ */
+export function buildSubmitText(text: string, attachments: readonly Attachment[] | null): string {
+  if (!attachments || attachments.length === 0) {
+    return text;
+  }
+  const parts: string[] = [text];
+  for (let n = 0; n < attachments.length; n++) {
+    const att = attachments[n];
+    if (!att) {
+      continue;
+    }
+    if (att.kind === 'text') {
+      const showSize = att.sizeBytes >= 1024 ? `${(att.sizeBytes / 1024).toFixed(1)}KB` : `${att.sizeBytes}B`;
+      const fullSize = att.fullSizeBytes >= 1024 ? `${(att.fullSizeBytes / 1024).toFixed(1)}KB` : `${att.fullSizeBytes}B`;
+      const truncPrefix = att.truncated ? `// showing ${showSize} of ${fullSize} (truncated)\n` : '';
+      parts.push(`\n\n[attachment #${n + 1}]\n${truncPrefix}${att.text}\n[/attachment]`);
+    } else {
+      const lines: string[] = [`path: ${att.path}`];
+      if (att.fileType === 'missing') {
+        lines.push('// not found');
+      } else {
+        lines.push(`type: ${att.fileType}`);
+        if (att.fileType === 'file' && att.sizeBytes !== undefined) {
+          const sz = att.sizeBytes;
+          const sizeStr = sz >= 1024 ? `${(sz / 1024).toFixed(1)}KB` : `${sz}B`;
+          lines.push(`size: ${sizeStr}`);
+        }
+      }
+      parts.push(`\n\n[attachment #${n + 1}]\n${lines.join('\n')}\n[/attachment]`);
+    }
+  }
+  return parts.join('');
+}

apps/claude-sdk-cli/test/buildSubmitText.spec.ts

@@ -0,0 +1,198 @@
+import { describe, expect, it } from 'vitest';
+import type { Attachment } from '../src/AttachmentStore.js';
+import { buildSubmitText } from '../src/buildSubmitText.js';
+
+// ---------------------------------------------------------------------------
+// No attachments
+// ---------------------------------------------------------------------------
+
+describe('buildSubmitText — no attachments', () => {
+  it('returns text unchanged when attachments is null', () => {
+    const expected = 'hello world';
+    const actual = buildSubmitText('hello world', null);
+    expect(actual).toBe(expected);
+  });
+
+  it('returns text unchanged when attachments is empty', () => {
+    const expected = 'hello world';
+    const actual = buildSubmitText('hello world', []);
+    expect(actual).toBe(expected);
+  });
+
+  it('returns empty string unchanged when no attachments', () => {
+    const expected = '';
+    const actual = buildSubmitText('', null);
+    expect(actual).toBe(expected);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Text attachments
+// ---------------------------------------------------------------------------
+
+describe('buildSubmitText — text attachment', () => {
+  const textAtt: Attachment = {
+    kind: 'text',
+    hash: 'abc123',
+    text: 'some content',
+    sizeBytes: 12,
+    fullSizeBytes: 12,
+    truncated: false,
+  };
+
+  it('appends [attachment #1] block', () => {
+    const result = buildSubmitText('prompt', [textAtt]);
+    const expected = true;
+    const actual = result.includes('[attachment #1]');
+    expect(actual).toBe(expected);
+  });
+
+  it('includes the attachment text content', () => {
+    const result = buildSubmitText('prompt', [textAtt]);
+    const expected = true;
+    const actual = result.includes('some content');
+    expect(actual).toBe(expected);
+  });
+
+  it('includes [/attachment] closing tag', () => {
+    const result = buildSubmitText('prompt', [textAtt]);
+    const expected = true;
+    const actual = result.includes('[/attachment]');
+    expect(actual).toBe(expected);
+  });
+
+  it('starts with the main prompt text', () => {
+    const result = buildSubmitText('my prompt', [textAtt]);
+    const expected = true;
+    const actual = result.startsWith('my prompt');
+    expect(actual).toBe(expected);
+  });
+
+  it('does not include truncation notice when not truncated', () => {
+    const result = buildSubmitText('prompt', [textAtt]);
+    const expected = false;
+    const actual = result.includes('truncated');
+    expect(actual).toBe(expected);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Truncated text attachment
+// ---------------------------------------------------------------------------
+
+describe('buildSubmitText — truncated text attachment', () => {
+  const truncAtt: Attachment = {
+    kind: 'text',
+    hash: 'xyz',
+    text: 'stored text',
+    sizeBytes: 1024,
+    fullSizeBytes: 2048,
+    truncated: true,
+  };
+
+  it('includes truncation notice', () => {
+    const result = buildSubmitText('prompt', [truncAtt]);
+    const expected = true;
+    const actual = result.includes('truncated');
+    expect(actual).toBe(expected);
+  });
+
+  it('includes showing size in truncation notice', () => {
+    const result = buildSubmitText('prompt', [truncAtt]);
+    const expected = true;
+    const actual = result.includes('showing');
+    expect(actual).toBe(expected);
+  });
+
+  it('formats size in KB when >= 1024 bytes', () => {
+    const result = buildSubmitText('prompt', [truncAtt]);
+    const expected = true;
+    const actual = result.includes('KB');
+    expect(actual).toBe(expected);
+  });
+
+  it('formats size in bytes when < 1024', () => {
+    const smallTrunc: Attachment = {
+      kind: 'text',
+      hash: 'small',
+      text: 'x',
+      sizeBytes: 500,
+      fullSizeBytes: 800,
+      truncated: true,
+    };
+    const result = buildSubmitText('prompt', [smallTrunc]);
+    const expected = true;
+    const actual = result.includes('500B');
+    expect(actual).toBe(expected);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// File attachments
+// ---------------------------------------------------------------------------
+
+describe('buildSubmitText — file attachment', () => {
+  it('includes path for file attachment', () => {
+    const att: Attachment = { kind: 'file', path: '/tmp/report.txt', fileType: 'file', sizeBytes: 2048 };
+    const result = buildSubmitText('prompt', [att]);
+    const expected = true;
+    const actual = result.includes('path: /tmp/report.txt');
+    expect(actual).toBe(expected);
+  });
+
+  it('includes type and size for file attachment', () => {
+    const att: Attachment = { kind: 'file', path: '/tmp/report.txt', fileType: 'file', sizeBytes: 2048 };
+    const result = buildSubmitText('prompt', [att]);
+    const expected = true;
+    const actual = result.includes('type: file') && result.includes('size:');
+    expect(actual).toBe(expected);
+  });
+
+  it('includes type: dir for directory attachment', () => {
+    const att: Attachment = { kind: 'file', path: '/tmp/mydir', fileType: 'dir' };
+    const result = buildSubmitText('prompt', [att]);
+    const expected = true;
+    const actual = result.includes('type: dir');
+    expect(actual).toBe(expected);
+  });
+
+  it('includes // not found for missing file', () => {
+    const att: Attachment = { kind: 'file', path: '/tmp/gone.txt', fileType: 'missing' };
+    const result = buildSubmitText('prompt', [att]);
+    const expected = true;
+    const actual = result.includes('// not found');
+    expect(actual).toBe(expected);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Multiple attachments
+// ---------------------------------------------------------------------------
+
+describe('buildSubmitText — multiple attachments', () => {
+  const att1: Attachment = { kind: 'text', hash: 'h1', text: 'first', sizeBytes: 5, fullSizeBytes: 5, truncated: false };
+  const att2: Attachment = { kind: 'file', path: '/tmp/second.txt', fileType: 'file', sizeBytes: 100 };
+
+  it('numbers attachments starting at #1', () => {
+    const result = buildSubmitText('prompt', [att1, att2]);
+    const expected = true;
+    const actual = result.includes('[attachment #1]');
+    expect(actual).toBe(expected);
+  });
+
+  it('numbers second attachment #2', () => {
+    const result = buildSubmitText('prompt', [att1, att2]);
+    const expected = true;
+    const actual = result.includes('[attachment #2]');
+    expect(actual).toBe(expected);
+  });
+
+  it('both attachments appear in order', () => {
+    const result = buildSubmitText('prompt', [att1, att2]);
+    const pos1 = result.indexOf('[attachment #1]');
+    const pos2 = result.indexOf('[attachment #2]');
+    const expected = true;
+    const actual = pos1 < pos2;
+    expect(actual).toBe(expected);
+  });
+});