diff --git a/plans/2026-06-11-f10-interactive-review.md b/plans/2026-06-11-f10-interactive-review.md new file mode 100644 index 0000000..6aa3da0 --- /dev/null +++ b/plans/2026-06-11-f10-interactive-review.md @@ -0,0 +1,995 @@ +# F10 — Interactive Track-Changes Review in the Markdown Preview — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the rendered markdown preview the single interactive review surface — a clean (zero-annotation) editor, an annotations on/off toggle, one combined per-author + strikethrough render, and ✓/✗ accept/reject of Claude's pending F4 proposals from inside the preview. + +**Architecture:** Mostly assembly of shipped features. F3 attribution (`spansFor`), F4 propose/accept (`acceptById`/`rejectById`), and the F6 baseline (`DiffViewController`) stay as **data layers**; their in-editor visuals are removed. One new pure, vscode-free function `renderReview(baselineText, currentText, authorSpans, proposals)` overlays the F7 block/word diff (author-colored via the salvaged F9 PUA-sentinel technique, struck deletions) **and** F4 pending proposals (blue blocks with ✓/✗). The `TrackChangesPreviewController` collapses F9's two modes into one `"on"|"off"` toggle, gains a `ProposalController` dependency, and routes ✓/✗ webview messages through the F4 seam. The webview never mutates the document (INV-20/21/34). + +**Tech Stack:** TypeScript, VS Code extension API, `markdown-it` + `diff` (jsdiff) + `mermaid` (webview-only), esbuild (webview bundle), vitest (vscode-free unit), `@vscode/test-electron` (host E2E). + +**Spec:** `specs/coauthoring-interactive-review.md` (F10, `#29`). Parent invariants INV-1..31 carry over except where F10 supersedes (F10 adds INV-32/33/34; **reverses F9 INV-26**). + +--- + +## File Structure + +**Modify:** +- `src/attributionController.ts` — `render()` stops applying editor decorations (keep `spansFor`); retire decoration types. +- `src/proposalController.ts` — stop creating in-editor comment threads + pending decorations; refactor bookkeeping to plain maps; add `listProposals()` + `onDidChangeProposals`. +- `src/trackChangesModel.ts` — add `renderReview` + `renderPlain` + `colorByAuthor` + `ProposalView`; remove public `renderAuthorship`. +- `src/trackChangesPreview.ts` — collapse mode to `"on"|"off"`; take `ProposalController`; subscribe to `onDidChangeProposals`; new render path; inbound `accept`/`reject`/`setMode`; own a status-bar item. +- `src/extension.ts` — reorder construction (proposals before preview); inject `ProposalController` into the preview. +- `media/preview.ts` — replace segmented control with on/off switch; add ✓/✗ click → `postMessage`. +- `media/preview.css` — proposal-block + ✓/✗ button styles; on/off switch style. +- `package.json` — hide `toggleDiffView`/`ctrl+alt+d`, `toggleAttribution`, in-editor `acceptProposal`/`rejectProposal` (`when:false`); retitle `showTrackChangesPreview`. + +**Create:** +- `docs/MANUAL-SMOKE-F10.md` — the live webview smoke checklist. + +**Test:** +- `test/trackChangesModel.test.ts` — extend (vitest) for `renderReview`/`renderPlain`/`colorByAuthor`. +- `test/e2e/suite/trackChangesPreview.test.ts` and/or a new `f10Review.test.ts` — host E2E. + +--- + +## Conventions for this plan + +- **Unit tests** (the pure `trackChangesModel` engine) are strict TDD: failing test first, run it, minimal impl, run green, commit. Command: `npm test` (vitest) — to run one file: `npx vitest run test/trackChangesModel.test.ts`. +- **vscode-layer changes** (controllers, webview, package.json) cannot be unit-tested vscode-free; they are verified by **host E2E** (`npm run test:e2e`) and the manual smoke. For those tasks the "test" step is the E2E assertion added in SLICE-4 plus a `npm run build` typecheck. +- Build/typecheck after vscode-layer edits: `npm run build` (esbuild + bundles `media/preview.ts`). E2E precompiles via `npm run pretest:e2e`. +- Commit after each task. + +--- + +## SLICE-1 — Clean editor (INV-32) + +Strip F3 attribution decorations and F4 in-editor proposal threads/decorations; hide the F6 two-pane diff command/keybinding and the attribution toggle. Keep `spansFor`, accept/reject logic, and the baseline store. *Mostly deletion.* + +### Task 1: Strip F3 attribution editor decorations + +**Files:** +- Modify: `src/attributionController.ts:62-63` (decoration types), `:354-373` (`render()`), `:82` (dispose list) + +- [ ] **Step 1: Read the current `render()` and decoration setup.** Confirm `render()` computes `agentRanges`/`humanRanges` and calls `editor.setDecorations(this.agentType, …)` / `(this.humanType, …)` at lines 366-367, and that `spansFor` (≈line 410) is independent of these. + +- [ ] **Step 2: Remove the editor-painting side effect from `render()`.** Edit `render()` so it no longer calls `setDecorations` and no longer builds `agentRanges`/`humanRanges`. F10 keeps `render()` as a method (callers exist) but it becomes a no-op for decorations — leave any non-decoration bookkeeping intact. Concretely, delete the two `editor.setDecorations(...)` calls (lines 366-367) and the range-building that feeds only them. + +- [ ] **Step 3: Retire the decoration types.** Delete the `agentType`/`humanType` `createTextEditorDecorationType(...)` fields (lines 62-63), the `AGENT_DECO`/`HUMAN_DECO` constants (≈lines 47-56), and remove `this.agentType, this.humanType` from the dispose push at line 82. Keep `this.statusItem, this.output, this.applyEmitter` in the dispose list. + +- [ ] **Step 4: Typecheck.** + +Run: `npm run build` +Expected: no TypeScript errors; no remaining references to `agentType`/`humanType`/`AGENT_DECO`/`HUMAN_DECO`. + +- [ ] **Step 5: Confirm `spansFor` is untouched.** Grep that `spansFor(` still exists and returns `AuthorSpan[]`. + +Run: `grep -n 'spansFor' src/attributionController.ts` +Expected: the method signature is present and unchanged. + +- [ ] **Step 6: Commit.** + +```bash +git add src/attributionController.ts +git commit -m "F10 SLICE-1: strip F3 attribution editor decorations (keep spansFor) (#29)" +``` + +### Task 2: Strip F4 in-editor proposal threads + decorations; refactor bookkeeping + +The editor must carry **no** F4 visuals. Remove the `CommentController`, the per-proposal comment threads, and the amber `pendingType` decoration — but keep resolving each proposal's anchor into `state.live`/`state.unresolved` so `getRendered` (existing tests) and SLICE-3's `listProposals` still work. The in-editor `acceptThread`/`rejectThread` paths become dead and are removed; `acceptById`/`rejectById` (the public seams) are kept. + +**Files:** +- Modify: `src/proposalController.ts` — `DocState` (`:32-41`), constructor (`:49-71`), `renderAll` (`:187-210`), `renderProposal` (`:243-274`), `renderDecorations` (`:276-285`), `byThread`/`acceptThread`/`rejectThread` (`:137-144`, `:304-314`), `getRendered` (`:323-339`). + +- [ ] **Step 1: Drop the CommentController + pending decoration + thread bookkeeping.** + - Remove `private readonly controller = vscode.comments.createCommentController(...)` (≈line 64) and `private readonly pendingType = ...createTextEditorDecorationType(PENDING_DECO)` (line 53) and the `PENDING_DECO` constant (lines 43-47). + - Remove `this.controller` and `this.pendingType` from the dispose push (line 65) — keep `this.statusItem`. + - Remove the `acceptProposal`/`rejectProposal` command registrations (lines 67-68) — these are the in-editor thread-menu commands; the seams move to the preview in SLICE-3. + - In `DocState` remove `vsThreads: Map` (line 36); keep `live` and `unresolved`. + +- [ ] **Step 2: Replace `renderProposal` (thread+UI) with `recordProposal` (bookkeeping only).** New method records resolved offsets without any vscode UI: + +```ts +/** Record a proposal's resolved offsets (no editor UI — F10 INV-32). */ +private recordProposal(proposal: Proposal, offsets: OffsetRange, pending: boolean): void { + this.docs.get(this.keyOf2(proposal))?.live; // placeholder removed below +} +``` + + …actually keep it simple and stateful via the passed `state`: + +```ts +private recordProposal(state: DocState, proposal: Proposal, offsets: OffsetRange, pending: boolean): void { + state.live.set(proposal.id, offsets); + if (!pending) state.unresolved.add(proposal.id); +} +``` + +- [ ] **Step 3: Rewrite `renderAll` to use `recordProposal` and drop thread/decoration disposal.** Keep the anchor resolve-or-flag loop; remove `for (const vsThread of state.vsThreads.values()) vsThread.dispose();` and `state.vsThreads.clear();` and the `this.renderDecorations(...)` call: + +```ts +renderAll(document: vscode.TextDocument): void { + if (!this.isTracked(document)) return; + const docPath = this.keyOf(document); + const state = this.ensureState(document); + state.artifact = this.store.load(docPath) ?? emptyArtifact(docPath); + state.live.clear(); + state.unresolved.clear(); + const text = document.getText(); + for (const proposal of state.artifact.proposals) { + const fp = state.artifact.anchors[proposal.anchorId]?.fingerprint; + const resolved = fp ? resolve(text, fp) : "orphaned"; + if (resolved === "orphaned") { + const line = fp ? Math.min(fp.lineHint, Math.max(0, document.lineCount - 1)) : 0; + const off = document.offsetAt(new vscode.Position(line, 0)); + this.recordProposal(state, proposal, { start: off, end: off }, false); + } else { + this.recordProposal(state, proposal, resolved, true); + } + } + this.renderStatus(state); +} +``` + +- [ ] **Step 4: Remove `renderDecorations` and the thread-following branch in `onDidChange`.** Delete `renderDecorations` (lines 276-285). In `onDidChange` (lines 222-239) keep the `state.live` shift loop but remove the `vsThread.range = ...` update and the trailing `this.renderDecorations(...)` call: + +```ts +private onDidChange(e: vscode.TextDocumentChangeEvent): void { + const state = this.docs.get(this.keyOf(e.document)); + if (!state || state.live.size === 0) return; + for (const change of e.contentChanges) { + const edit = { start: change.rangeOffset, end: change.rangeOffset + change.rangeLength, newLength: change.text.length }; + for (const [id, range] of state.live) state.live.set(id, shift(range, edit)); + } +} +``` + +- [ ] **Step 5: Remove `acceptThread`/`rejectThread`/`byThread`.** Delete those three methods (lines 137-144, 304-314). `acceptById`/`rejectById`/`byId` stay. `accept`/`reject` keep their `renderAll(document)` re-render calls (now thread-free). + +- [ ] **Step 6: Repoint `getRendered` at `live`/`unresolved`.** It read from `vsThreads`; read from `live` instead: + +```ts +getRendered(docPath: string): RenderedProposal[] { + const state = this.docs.get(docPath); + if (!state) return []; + const out: RenderedProposal[] = []; + for (const [id, off] of state.live) { + const p = state.artifact.proposals.find((x) => x.id === id)!; + out.push({ + id, + pending: !state.unresolved.has(id), + canReply: false, + turnId: p.turnId, + range: { start: off.start, end: off.end }, + }); + } + return out; +} +``` + +- [ ] **Step 7: Typecheck.** + +Run: `npm run build` +Expected: no errors; no remaining references to `vsThreads`, `pendingType`, `PENDING_DECO`, `this.controller`, `acceptThread`, `rejectThread`, `byThread`, `renderDecorations`, `renderProposal`. + +- [ ] **Step 8: Run the existing F4 unit tests (proposalModel — vscode-free) to confirm no regression.** + +Run: `npx vitest run test/proposalModel.test.ts` +Expected: PASS (proposalModel is untouched; this is a guard). + +- [ ] **Step 9: Commit.** + +```bash +git add src/proposalController.ts +git commit -m "F10 SLICE-1: remove F4 in-editor proposal threads/decorations; bookkeeping via live/unresolved (#29)" +``` + +> NOTE: the existing host E2E `test/e2e/suite/proposals.test.ts` may assert in-editor thread/`getRendered` behavior. Do **not** fix it here — SLICE-4 updates the E2E suite. If `npm run test:e2e` is run before SLICE-4, expect known proposal-thread failures. + +### Task 3: Hide F6 two-pane diff + attribution toggle in package.json + +**Files:** +- Modify: `package.json` — keybindings (`:154-165`), `contributes.menus.commandPalette` (`:93-152`), command titles (`:48-91`) + +- [ ] **Step 1: Hide the `ctrl+alt+d` keybinding.** In `contributes.keybindings`, set the `toggleDiffView` entry's `when` to `false`: + +```json +{ "command": "cowriting.toggleDiffView", "key": "ctrl+alt+d", "when": "false" } +``` + +- [ ] **Step 2: Hide `toggleDiffView`, `pinDiffBaseline`, and `toggleAttribution` from the command palette.** In `contributes.menus.commandPalette`, add (or set) `when: "false"` entries: + +```json +{ "command": "cowriting.toggleDiffView", "when": "false" }, +{ "command": "cowriting.pinDiffBaseline", "when": "false" }, +{ "command": "cowriting.toggleAttribution", "when": "false" } +``` + +- [ ] **Step 3: Confirm the in-editor `acceptProposal`/`rejectProposal` palette entries are already `when:false`** (lines 104-110) and **remove their `editor/context`/`comments/commentThread` menu contributions** that referenced the now-deleted comment controller (the `commentController == cowriting.proposals` menu items, lines ≈143-151), since SLICE-1 deleted that controller. + +- [ ] **Step 4: Retitle `showTrackChangesPreview` and rebind to the review verb.** Set its title to `"Cowriting: Open Review Preview"` (line 89) and keep `ctrl+alt+r` / `when: editorLangId == markdown` (lines 161-164). + +- [ ] **Step 5: Validate JSON + typecheck.** + +Run: `node -e "JSON.parse(require('fs').readFileSync('package.json','utf8')); console.log('ok')" && npm run build` +Expected: `ok` then a clean build. + +- [ ] **Step 6: Commit.** + +```bash +git add package.json +git commit -m "F10 SLICE-1: hide F6 diff command/keybinding + attribution toggle; retitle preview to 'Open Review Preview' (#29)" +``` + +--- + +## SLICE-2 — Combined render engine (pure, vscode-free; INV-33) + +Add `renderReview` (the on-state) + `renderPlain` (the off-state) to `trackChangesModel.ts`, extracting the F9 PUA-sentinel coloring into a reusable `colorByAuthor`, and removing the now-superseded public `renderAuthorship`. Strict TDD — this is the pure core. + +### Task 4: Extract `colorByAuthor` from `renderAuthorship`'s inline sentinels + +**Files:** +- Modify: `src/trackChangesModel.ts:326-352` (sentinel helpers), `:360-386` (`renderAuthorship`) +- Test: `test/trackChangesModel.test.ts` + +- [ ] **Step 1: Write a failing unit test for `colorByAuthor`.** It should color a prose block's HTML by author spans (clipped to the block) using the existing sentinel technique. + +```ts +import { colorByAuthor, type AuthorSpan } from "../src/trackChangesModel"; + +test("colorByAuthor wraps human-authored prose in cw-by-human spans", () => { + const raw = "hello world"; + const spans: AuthorSpan[] = [{ start: 0, end: 5, author: "human" }]; + const render = (src: string) => `

${src}

`; + const html = colorByAuthor(raw, 0, spans, render); + expect(html).toContain('hello'); + expect(html).toContain("world"); +}); +``` + +- [ ] **Step 2: Run it — expect failure (`colorByAuthor` not exported).** + +Run: `npx vitest run test/trackChangesModel.test.ts -t colorByAuthor` +Expected: FAIL — `colorByAuthor is not a function` / not exported. + +- [ ] **Step 3: Implement `colorByAuthor` by lifting the sentinel logic.** Export a function that injects sentinels for the block's overlapping spans, runs the provided render, and maps sentinels → spans: + +```ts +/** + * Color one prose block's HTML by F3 author spans (PUA-sentinel technique, + * salvaged from F9 renderAuthorship). `blockStart` is the block's char offset in + * the source so spans map correctly. Pure; deterministic. + */ +export function colorByAuthor( + raw: string, + blockStart: number, + spans: AuthorSpan[], + render: (src: string) => string, +): string { + const overlapping = spans.filter((s) => s.end > blockStart && s.start < blockStart + raw.length); + const injected = injectSentinels(raw, blockStart, overlapping); + return sentinelsToSpans(render(injected)); +} +``` + + Keep `injectSentinels`, `sentinelsToSpans`, `SENT`, `isCloseSentinel`, `authorBadge` as the private helpers they already are. + +- [ ] **Step 4: Run the test — expect PASS.** + +Run: `npx vitest run test/trackChangesModel.test.ts -t colorByAuthor` +Expected: PASS. + +- [ ] **Step 5: Refactor `renderAuthorship` to call `colorByAuthor` (no behavior change yet — it's removed in Task 7).** In its prose branch replace the inline `sentinelsToSpans(safe(injectSentinels(...)))` with `colorByAuthor(b.raw, b.start, overlapping, safe)`. + +- [ ] **Step 6: Run the full model suite — expect PASS (no regression).** + +Run: `npx vitest run test/trackChangesModel.test.ts` +Expected: PASS (all existing renderAuthorship/renderTrackChanges cases green). + +- [ ] **Step 7: Commit.** + +```bash +git add src/trackChangesModel.ts test/trackChangesModel.test.ts +git commit -m "F10 SLICE-2: extract colorByAuthor from renderAuthorship sentinels (#29)" +``` + +### Task 5: Add `renderPlain` (the off-state) + +**Files:** +- Modify: `src/trackChangesModel.ts` +- Test: `test/trackChangesModel.test.ts` + +- [ ] **Step 1: Write a failing test.** + +```ts +import { renderPlain } from "../src/trackChangesModel"; + +test("renderPlain renders current buffer as plain markdown (no marks)", () => { + const html = renderPlain("# Title\n\nhello"); + expect(html).toContain("

Title

"); + expect(html).toContain("hello"); + expect(html).not.toContain("cw-"); +}); +``` + +- [ ] **Step 2: Run it — expect FAIL (not exported).** + +Run: `npx vitest run test/trackChangesModel.test.ts -t renderPlain` +Expected: FAIL. + +- [ ] **Step 3: Implement.** + +```ts +/** Off-state body: the current buffer as plain markdown, no annotations (INV-33). */ +export function renderPlain(currentText: string, opts: RenderOptions = {}): string { + const render = opts.render ?? defaultRender; + try { + return render(currentText); + } catch (err) { + return chip(err instanceof Error ? err.message : String(err)); + } +} +``` + +- [ ] **Step 4: Run — expect PASS.** + +Run: `npx vitest run test/trackChangesModel.test.ts -t renderPlain` +Expected: PASS. + +- [ ] **Step 5: Commit.** + +```bash +git add src/trackChangesModel.ts test/trackChangesModel.test.ts +git commit -m "F10 SLICE-2: add renderPlain off-state (#29)" +``` + +### Task 6: Add `ProposalView` + `renderReview` (the on-state) + +`renderReview` overlays two axes in one pass: (1) the F7 baseline block/word diff, with added/changed prose author-colored via `colorByAuthor` and deletions struck; (2) each pending proposal injected at its resolved anchor as a blue `cw-proposal` block carrying `data-proposal-id` and a ✓/✗ placeholder. Unanchored proposals render as trailing blocks (never dropped — INV-34). + +**Files:** +- Modify: `src/trackChangesModel.ts` +- Test: `test/trackChangesModel.test.ts` + +- [ ] **Step 1: Add the `ProposalView` type.** (Resolved offsets are computed by the controller; the pure engine receives them.) + +```ts +export interface ProposalView { + id: string; + /** resolved offsets in currentText; null when the anchor did not resolve. */ + anchorStart: number | null; + anchorEnd: number | null; + /** the text the proposal would replace (fp.text), for the struck "before". */ + replaced: string; + /** the proposed replacement text. */ + replacement: string; +} +``` + +- [ ] **Step 2: Write failing tests covering the on-state contract.** + +```ts +import { renderReview, type ProposalView, type AuthorSpan } from "../src/trackChangesModel"; + +test("renderReview: human addition since baseline renders green ", () => { + const html = renderReview("hello", "hello world", + [{ start: 6, end: 11, author: "human" }], []); + expect(html).toMatch(/[^<]*world[^<]*<\/ins>|cw-by-human/); +}); + +test("renderReview: deletion since baseline renders struck cw-del/", () => { + const html = renderReview("hello world", "hello", [], []); + expect(html).toMatch(/|cw-del/); +}); + +test("renderReview: a pending proposal renders a blue block with data-proposal-id and a ✓/✗ action placeholder", () => { + const proposals: ProposalView[] = [ + { id: "p1", anchorStart: 0, anchorEnd: 5, replaced: "hello", replacement: "goodbye" }, + ]; + const html = renderReview("hello", "hello", [], proposals); + expect(html).toContain('class="cw-proposal"'); + expect(html).toContain('data-proposal-id="p1"'); + expect(html).toContain("cw-actions"); + expect(html).toContain("goodbye"); + expect(html).toMatch(/[^<]*hello[^<]*<\/del>|cw-del/); +}); + +test("renderReview: an unresolved proposal renders as a trailing block (never dropped)", () => { + const proposals: ProposalView[] = [ + { id: "p2", anchorStart: null, anchorEnd: null, replaced: "x", replacement: "y" }, + ]; + const html = renderReview("a", "a", [], proposals); + expect(html).toContain('data-proposal-id="p2"'); + expect(html).toContain("cw-proposal-unanchored"); +}); + +test("renderReview is deterministic (same inputs → identical HTML)", () => { + const a = renderReview("hello", "hello world", [{ start: 6, end: 11, author: "human" }], []); + const b = renderReview("hello", "hello world", [{ start: 6, end: 11, author: "human" }], []); + expect(a).toBe(b); +}); + +test("renderReview: an atomic mermaid change is diffed whole (no inner sentinels)", () => { + const base = "```mermaid\nflowchart LR\n A --> B\n```"; + const cur = "```mermaid\nflowchart LR\n A --> C\n```"; + const html = renderReview(base, cur, [], []); + expect(html).toContain("mermaid"); + expect(html).not.toContain("cw-by-"); // fences stay atomic +}); +``` + +- [ ] **Step 3: Run — expect FAIL (renderReview not exported).** + +Run: `npx vitest run test/trackChangesModel.test.ts -t renderReview` +Expected: FAIL. + +- [ ] **Step 4: Implement `renderReview`.** Reuse `diffBlocks` + `renderOp`, but author-color added/changed prose and append proposal blocks. The simplest correct composition: render the diff body via the existing `renderOp` path **augmented** so added/changed prose `` content is author-colored, then inject proposal blocks. Add an internal `renderReviewBlock` that colors prose and a `proposalBlockHtml` emitter: + +```ts +function proposalBlockHtml(p: ProposalView, render: (src: string) => string): string { + const safe = (src: string): string => { + try { return render(src); } catch (err) { return chip(err instanceof Error ? err.message : String(err)); } + }; + const unanchored = p.anchorStart === null ? " cw-proposal-unanchored" : ""; + const before = p.replaced ? `${safe(p.replaced)}` : ""; + const after = `${safe(p.replacement)}`; + const actions = + `` + + `` + + `` + + ``; + return `
${actions}${before}${after}
`; +} + +/** + * On-state body (INV-33): the F7 baseline diff — added/changed PROSE author-colored + * via colorByAuthor (F9 sentinels), deletions struck — overlaid with F4 pending + * proposals as blue cw-proposal blocks (✓/✗). One pass, pure, vscode-free. + * Proposals are injected at their resolved anchor's block; unresolved ones append + * as trailing cw-proposal-unanchored blocks (never dropped — INV-34). + */ +export function renderReview( + baselineText: string, + currentText: string, + authorSpans: AuthorSpan[], + proposals: ProposalView[], + opts: RenderOptions = {}, +): string { + const render = opts.render ?? defaultRender; + // 1) the diff body, with prose author-coloring layered onto current-side blocks. + const ranges = splitBlocksWithRanges(currentText); + const ops = diffBlocks(baselineText, currentText); + // Map current-side blocks (added/unchanged/changed) to their source ranges for coloring. + const colored = (raw: string): string => { + const blk = ranges.find((r) => r.raw === raw); + if (!blk) return render(raw); + return colorByAuthor(raw, blk.start, authorSpans, render); + }; + const bodyParts = ops.map((op) => renderReviewOp(op, render, colored)); + // 2) proposal blocks. Resolved proposals are appended after the block containing + // their anchor; for v1 simplicity (and determinism) append all proposals in id + // order, anchored ones first, then unanchored — each carries its own marker. + const anchored = proposals.filter((p) => p.anchorStart !== null); + const unanchored = proposals.filter((p) => p.anchorStart === null); + const proposalParts = [...anchored, ...unanchored].map((p) => proposalBlockHtml(p, render)); + return [...bodyParts, ...proposalParts].join("\n"); +} +``` + + Add `renderReviewOp` — a thin wrapper over the existing `renderOp` that swaps the prose renderer for the author-coloring one on `added`/`changed`(non-atomic)/`unchanged` ops: + +```ts +function renderReviewOp( + op: BlockOp, + render: (src: string) => string, + colored: (raw: string) => string, +): string { + // Atomic fences and deletions: identical to renderOp (no author sentinels). + if (op.kind === "removed") return renderOp(op, render); + if (op.kind === "changed" && op.atomic) return renderOp(op, render); + // Prose added/unchanged: author-color the block; changed prose: keep / + // word merge (deletions struck), then we accept that word-merge is not per-author + // colored in v1 (covered by §6.7 "deletion coloring = neutral"). + if (op.kind === "changed") return renderOp(op, render); // word-merged / + // unchanged / added prose → author-colored. + return `
${colored(op.block.raw)}
`; +} +``` + + > Design note: per spec §6.7, deletion coloring is neutral and added-prose author-coloring is the primary signal; changed-prose keeps the F7 word-merge. This keeps the engine deterministic and the tests above green. If a later refinement wants author-colored `` inside changed prose, it extends `renderReviewOp`. + +- [ ] **Step 5: Run the renderReview tests — expect PASS.** + +Run: `npx vitest run test/trackChangesModel.test.ts -t renderReview` +Expected: PASS (all six). + +- [ ] **Step 6: Run the full model suite.** + +Run: `npx vitest run test/trackChangesModel.test.ts` +Expected: PASS. + +- [ ] **Step 7: Commit.** + +```bash +git add src/trackChangesModel.ts test/trackChangesModel.test.ts +git commit -m "F10 SLICE-2: add ProposalView + renderReview combined on-state render (#29)" +``` + +### Task 7: Remove the superseded public `renderAuthorship` + +**Files:** +- Modify: `src/trackChangesModel.ts` (remove `renderAuthorship`), `src/trackChangesPreview.ts` (stops importing it — done in SLICE-3, so here just remove the export and fix the model) +- Test: `test/trackChangesModel.test.ts` (drop/replace renderAuthorship-only cases salvaged into colorByAuthor) + +- [ ] **Step 1: Delete the exported `renderAuthorship` function** (lines 360-386). Keep `colorByAuthor`, `injectSentinels`, `sentinelsToSpans`, `authorBadge`, `SENT`. + +- [ ] **Step 2: Remove or repoint any test that imports `renderAuthorship`.** Any remaining authorship-only assertions are now covered by the `colorByAuthor` test (Task 4); delete the obsolete `renderAuthorship` test cases. + +- [ ] **Step 3: Typecheck — expect a known error in `trackChangesPreview.ts`** (it still imports `renderAuthorship`). That import is removed in SLICE-3 Task 9; note it and proceed (do not patch the preview here beyond removing the dangling import if convenient). + +Run: `npm run build` +Expected: error only at `trackChangesPreview.ts` `renderAuthorship` import → resolved in Task 9. (If you prefer a clean build per-task, remove the import + authorship branch now as a head-start on Task 9.) + +- [ ] **Step 4: Run the model suite — expect PASS.** + +Run: `npx vitest run test/trackChangesModel.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit.** + +```bash +git add src/trackChangesModel.ts test/trackChangesModel.test.ts +git commit -m "F10 SLICE-2: remove superseded public renderAuthorship (salvaged into colorByAuthor) (#29)" +``` + +--- + +## SLICE-3 — Interactive controller + webview (INV-34) + +Add `ProposalController.listProposals` + `onDidChangeProposals`; wire `ProposalController` into the preview; collapse mode to `"on"|"off"`; route ✓/✗/setMode messages; rebuild the webview asset; status-bar indicator. + +### Task 8: `ProposalController.listProposals` + `onDidChangeProposals` + +**Files:** +- Modify: `src/proposalController.ts` + +- [ ] **Step 1: Add the change emitter + event.** Near the other fields: + +```ts +private readonly onDidChangeProposalsEmitter = new vscode.EventEmitter<{ uri: string }>(); +/** Fires on propose / accept / reject / external sidecar change (F10). */ +readonly onDidChangeProposals = this.onDidChangeProposalsEmitter.event; +``` + + Push it into `disposables` and fire it: at the end of `propose` (after `renderAll`), `accept` (after `removeProposal`+`renderAll`), `reject` (after `renderAll`), and `handleExternalSidecarChange`/`renderAll`. Fire with the document's URI string: + +```ts +private fireChanged(document: vscode.TextDocument): void { + this.onDidChangeProposalsEmitter.fire({ uri: document.uri.toString() }); +} +``` + + Call `this.fireChanged(document)` at the end of `renderAll(document)` (it is the common funnel for propose/accept/reject/external change). + +- [ ] **Step 2: Add `listProposals`.** Return resolved views for the preview's render. Resolve each proposal against the *current* document text (same as `renderAll`), exposing `fp.text` as `replaced`: + +```ts +import type { ProposalView } from "./trackChangesModel"; + +/** Resolved proposal views for the F10 preview (anchorStart=null when unresolved). */ +listProposals(document: vscode.TextDocument): ProposalView[] { + const docPath = this.keyOf(document); + const artifact = this.store.load(docPath) ?? emptyArtifact(docPath); + const text = document.getText(); + return artifact.proposals.map((p) => { + const fp = artifact.anchors[p.anchorId]?.fingerprint; + const resolved = fp ? resolve(text, fp) : "orphaned"; + return { + id: p.id, + anchorStart: resolved === "orphaned" ? null : resolved.start, + anchorEnd: resolved === "orphaned" ? null : resolved.end, + replaced: fp?.text ?? "", + replacement: p.replacement, + }; + }); +} +``` + +- [ ] **Step 3: Typecheck.** + +Run: `npm run build` +Expected: clean (the `ProposalView` import resolves against Task 6's export). + +- [ ] **Step 4: Commit.** + +```bash +git add src/proposalController.ts +git commit -m "F10 SLICE-3: ProposalController.listProposals + onDidChangeProposals (#29)" +``` + +### Task 9: Collapse preview mode to on/off, take `ProposalController`, route messages + +**Files:** +- Modify: `src/trackChangesPreview.ts` (constructor, mode map, `refresh`, message handler, shell HTML, test seams, imports) + +- [ ] **Step 1: Update imports + mode type.** Replace the model import with the F10 surface and switch the mode union: + +```ts +import { renderReview, renderPlain, diffBlocks, type BlockOp } from "./trackChangesModel"; +import type { ProposalController } from "./proposalController"; +``` + +```ts +/** F10: per-panel annotations toggle — on (combined render) or off (plain). */ +private readonly mode = new Map(); +``` + +- [ ] **Step 2: Add the `ProposalController` constructor dependency + subscribe to its change event.** + +```ts +constructor( + private readonly diffView: DiffViewController, + private readonly extensionUri: vscode.Uri, + private readonly attribution: AttributionController, + private readonly proposals: ProposalController, +) { + this.disposables.push( + vscode.commands.registerCommand("cowriting.showTrackChangesPreview", () => + this.show(vscode.window.activeTextEditor?.document), + ), + vscode.workspace.onDidChangeTextDocument((e) => this.onEdit(e.document)), + this.diffView.onDidChangeBaseline(({ uri }) => this.refreshByUri(uri)), + this.proposals.onDidChangeProposals(({ uri }) => { this.refreshByUri(uri); this.updateStatus(uri); }), + ); +} +``` + +- [ ] **Step 3: Replace the inbound message handler** (the `setMode` block) to also handle accept/reject: + +```ts +panel.webview.onDidReceiveMessage( + (m: { type?: string; mode?: "on" | "off"; proposalId?: string }) => { + if (m?.type === "setMode" && (m.mode === "on" || m.mode === "off")) { + this.mode.set(key, m.mode); + this.refresh(document); + } else if (m?.type === "accept" && m.proposalId) { + void this.proposals + .acceptById(this.proposalKey(document), m.proposalId) + .then(() => this.refresh(document)); + } else if (m?.type === "reject" && m.proposalId) { + this.proposals.rejectById(this.proposalKey(document), m.proposalId); + this.refresh(document); + } + }, + null, + this.disposables, +); +``` + + Add a `proposalKey(document)` helper that returns the same key `ProposalController` uses (`this.store.keyOf(docIdentity(document))`). Since the preview doesn't hold the router, expose a public `keyFor(document): string` on `ProposalController` and call it: + + In `proposalController.ts`: +```ts +/** The doc key F4 uses (F8 routing) — exposed for F10's preview. */ +keyFor(document: vscode.TextDocument): string { return this.keyOf(document); } +``` + In the preview, `private proposalKey(d: vscode.TextDocument) { return this.proposals.keyFor(d); }`. + +- [ ] **Step 4: Rewrite `refresh` for on/off.** Replace the `"authorship"`/`"changes"` branches with one combined `renderReview` (on) / `renderPlain` (off): + +```ts +refresh(document: vscode.TextDocument): void { + const key = document.uri.toString(); + const panel = this.panels.get(key); + if (!panel) return; + const mode = this.mode.get(key) ?? "on"; + const current = document.getText(); + const baseline = this.diffView.getBaseline(key); + const baselineText = baseline?.text ?? current; // no baseline → no change-marks + const ops = diffBlocks(baselineText, current); + this.lastModel.set(key, ops); + if (mode === "off") { + void panel.webview.postMessage({ type: "render", mode, html: renderPlain(current) }); + return; + } + const spans = this.attribution.spansFor(document); + const proposals = this.proposals.listProposals(document); + const summary = { + added: ops.filter((o) => o.kind === "added").length, + removed: ops.filter((o) => o.kind === "removed").length, + proposals: proposals.length, + }; + void panel.webview.postMessage({ + type: "render", + mode, + html: renderReview(baselineText, current, spans, proposals), + epoch: this.epochLabel(baseline), + summary, + }); +} +``` + +- [ ] **Step 5: Update the shell HTML header** — replace the segmented `[Track changes | Authorship]` with an on/off switch: + +```ts +
+ + Review + + +
+``` + +- [ ] **Step 6: Update the test seams** for the new mode type: + +```ts +getMode(uriString: string): "on" | "off" { return this.mode.get(uriString) ?? "on"; } +setMode(uriString: string, mode: "on" | "off"): void { + this.mode.set(uriString, mode); + const doc = vscode.workspace.textDocuments.find((d) => d.uri.toString() === uriString); + if (doc) this.refresh(doc); +} +/** F10 test seam: the on-state review HTML the panel would post. */ +renderHtmlFor(uriString: string): string { + const doc = vscode.workspace.textDocuments.find((d) => d.uri.toString() === uriString); + if (!doc) return ""; + const current = doc.getText(); + const baseline = this.diffView.getBaseline(uriString); + return renderReview(baseline?.text ?? current, current, this.attribution.spansFor(doc), this.proposals.listProposals(doc)); +} +``` + +- [ ] **Step 7: Add the status-bar item + `updateStatus`** (PUC-6) — see Task 10 (implement them here or in Task 10; cross-reference). For now stub `private updateStatus(_uri: string): void {}` so this task typechecks, and fill it in Task 10. + +- [ ] **Step 8: Typecheck.** + +Run: `npm run build` +Expected: clean — no `renderAuthorship`/`renderTrackChanges` references remain in the preview. + +- [ ] **Step 9: Commit.** + +```bash +git add src/trackChangesPreview.ts src/proposalController.ts +git commit -m "F10 SLICE-3: preview takes ProposalController; on/off mode; renderReview path; accept/reject routing (#29)" +``` + +### Task 10: Status-bar indicator (PUC-6) + +A status-bar item shows the pending-proposal count when **no preview panel is open** for any doc with proposals; clicking it runs `cowriting.showTrackChangesPreview`. Hidden when count is 0 or a panel is open. + +**Files:** +- Modify: `src/trackChangesPreview.ts` + +- [ ] **Step 1: Create the status-bar item in the constructor.** + +```ts +private readonly statusItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Right, 88); +``` + In the constructor body: `this.statusItem.command = "cowriting.showTrackChangesPreview"; this.disposables.push(this.statusItem);` + +- [ ] **Step 2: Implement `updateStatus`.** Count pending proposals for the doc whose proposals changed; show the item only if that doc has no open panel: + +```ts +private updateStatus(uri: string): void { + const doc = vscode.workspace.textDocuments.find((d) => d.uri.toString() === uri); + if (!doc) { this.statusItem.hide(); return; } + const n = this.proposals.listProposals(doc).length; + if (n === 0 || this.panels.has(uri)) { this.statusItem.hide(); return; } + this.statusItem.text = `$(comment-discussion) ${n} Claude proposal${n === 1 ? "" : "s"}`; + this.statusItem.tooltip = "Cowriting: open the review preview to accept/reject Claude's proposals"; + this.statusItem.show(); +} +``` + +- [ ] **Step 3: Hide the status item when a panel opens.** In `show()` after `this.panels.set(key, panel)`, call `this.statusItem.hide()`. In `onDidDispose`, call `this.updateStatus(key)` so it reappears if proposals remain. + +- [ ] **Step 4: Dispose the status item.** Already pushed to `disposables`; confirm `dispose()` clears it. + +- [ ] **Step 5: Typecheck.** + +Run: `npm run build` +Expected: clean. + +- [ ] **Step 6: Commit.** + +```bash +git add src/trackChangesPreview.ts +git commit -m "F10 SLICE-3: status-bar proposal indicator (PUC-6) (#29)" +``` + +### Task 11: Webview asset — on/off switch + ✓/✗ click handler + CSS + +**Files:** +- Modify: `media/preview.ts`, `media/preview.css` + +- [ ] **Step 1: Update the inbound `RenderMessage` type + handler in `media/preview.ts`.** The mode is now `"on"|"off"`; `summary` carries `{added, removed, proposals}`: + +```ts +type RenderMessage = { + type: "render"; + mode: "on" | "off"; + html: string; + epoch?: string; + summary?: { added: number; removed: number; proposals: number }; +}; +``` + + On receive: set `body.innerHTML = msg.html`; reflect the checkbox (`annotationsEl.checked = msg.mode === "on"`); show/hide summary/legend; then `renderMermaid()`. + +- [ ] **Step 2: Wire the on/off checkbox → `postMessage`.** Replace the `.cw-seg` segmented-control wiring: + +```ts +const annotationsEl = document.getElementById("cw-annotations") as HTMLInputElement | null; +annotationsEl?.addEventListener("change", () => { + vscode.postMessage({ type: "setMode", mode: annotationsEl.checked ? "on" : "off" }); +}); +``` + +- [ ] **Step 3: Add the ✓/✗ delegated click handler.** Buttons are inside `.cw-proposal` blocks carrying `data-proposal-id`; read the id + `data-action` and post intent: + +```ts +document.getElementById("cw-body")?.addEventListener("click", (e) => { + const btn = (e.target as HTMLElement)?.closest(".cw-actions button"); + if (!btn) return; + const block = btn.closest(".cw-proposal"); + const id = block?.dataset.proposalId; + const action = btn.dataset.action; + if (id && (action === "accept" || action === "reject")) { + vscode.postMessage({ type: action, proposalId: id }); + } +}); +``` + +- [ ] **Step 4: Add CSS in `media/preview.css`** for the proposal block, ✓/✗ buttons, and on/off toggle, theme-variable-driven: + +```css +.cw-proposal { + position: relative; + border-left: 3px solid var(--vscode-charts-blue, #4daafc); + background: color-mix(in srgb, var(--vscode-charts-blue, #4daafc) 12%, transparent); + padding: 0.4em 0.6em; + margin: 0.4em 0; + border-radius: 3px; +} +.cw-proposal-unanchored { border-left-style: dashed; opacity: 0.85; } +.cw-actions { position: absolute; top: 0.2em; right: 0.4em; display: inline-flex; gap: 0.25em; } +.cw-actions button { + cursor: pointer; border: 1px solid var(--vscode-button-border, transparent); + border-radius: 3px; font-size: 0.9em; line-height: 1; padding: 0.1em 0.35em; + background: var(--vscode-button-secondaryBackground); color: var(--vscode-button-secondaryForeground); +} +.cw-accept:hover { background: var(--vscode-testing-iconPassed, #2ea043); color: #fff; } +.cw-reject:hover { background: var(--vscode-errorForeground, #f14c4c); color: #fff; } +#cw-toggle { display: inline-flex; align-items: center; gap: 0.35em; cursor: pointer; } +``` + +- [ ] **Step 5: Build the webview bundle + typecheck.** + +Run: `npm run build` +Expected: clean; `out/media/preview.js` rebuilt. + +- [ ] **Step 6: Commit.** + +```bash +git add media/preview.ts media/preview.css +git commit -m "F10 SLICE-3: webview on/off switch + ✓/✗ click→postMessage + proposal CSS (#29)" +``` + +### Task 12: Wire it together in `extension.ts` + +**Files:** +- Modify: `src/extension.ts:92-105` (reorder: proposals before preview; inject) + +- [ ] **Step 1: Move `ProposalController` construction above `TrackChangesPreviewController`** and pass it in: + +```ts +// --- F4: propose/accept (Feature #12) — constructed before the preview so F10 +// can route ✓/✗ through it --- +const proposalController = new ProposalController(sidecarRouter, attributionController, root, versionGuard); +context.subscriptions.push(proposalController); + +// --- F7/F10: the review preview is the single interactive review surface --- +const trackChangesPreviewController = new TrackChangesPreviewController( + diffViewController, + context.extensionUri, + attributionController, + proposalController, +); +context.subscriptions.push(trackChangesPreviewController); +``` + + Remove the now-duplicate later `proposalController` construction (old line 104). + +- [ ] **Step 2: Typecheck + full unit suite.** + +Run: `npm run build && npm test` +Expected: clean build; vitest green (model + proposalModel + all vscode-free suites). + +- [ ] **Step 3: Commit.** + +```bash +git add src/extension.ts +git commit -m "F10 SLICE-3: wire ProposalController into the review preview (#29)" +``` + +--- + +## SLICE-4 — Tests & docs + +### Task 13: Host E2E for the interactive review flow + +**Files:** +- Modify: `test/e2e/suite/trackChangesPreview.test.ts` (or create `test/e2e/suite/f10Review.test.ts`), `test/e2e/suite/proposals.test.ts` (drop in-editor-thread assertions), `test/e2e/suite/authorship.test.ts` (remove/repoint F9 authorship-mode cases) + +- [ ] **Step 1: Repoint the obsolete F9 authorship E2E.** `authorship.test.ts` exercised `setMode("authorship")` + `renderAuthorship`. Either delete the file or rewrite its cases against `setMode(uri, "on")` and `renderReview` output (author-colored spans present). Confirm the suite index still imports valid files. + +- [ ] **Step 2: Fix `proposals.test.ts`.** Remove assertions about in-editor comment threads / `getRendered.canReply` semantics that no longer hold; keep `acceptById`/`rejectById` behavior assertions (those seams are unchanged). + +- [ ] **Step 3: Add the F10 host-E2E cases** (extend `trackChangesPreview.test.ts`). Use the existing harness patterns (open a markdown fixture, run `cowriting.showTrackChangesPreview`, drive the controller via its test seams and the `applyAgentEdit`/`proposeAgentEdit` commands). Assert: + - open markdown → panel open; `getLastModel` on a fresh baseline shows no change-marks. + - type text → an `added` BlockOp; the review HTML (`renderHtmlFor`) contains a `cw-by-human` span. + - `proposeAgentEdit` seam → `listProposals` returns one view with an id; `renderHtmlFor` contains `data-proposal-id` + `cw-actions`. + - simulate accept: call `proposalController.acceptById(key, id)` → proposal gone from `listProposals`, baseline advanced, the block now ordinary; document text reflects the replacement. + - simulate reject on a second proposal → gone from `listProposals`; document text unchanged. + - **clean editor:** assert no attribution decoration types are applied — e.g. assert `vscode.window.activeTextEditor` has no cowriting decorations (or that `AttributionController.render` no longer creates them; a structural assertion that the decoration-type fields were removed). + - `toggleDiffView` is hidden: assert its keybinding `when` is `false` (read from `package.json`) or that invoking it is a no-op user-facing. + - non-markdown doc → `show` warns, no panel (`isOpen` false). + - status-bar: after a propose with no panel open for that doc, the controller's status path reports a pending count (assert via a small seam if needed, e.g. expose `statusText()` for tests). + +- [ ] **Step 4: Add a tiny test seam if needed.** If asserting the status bar is awkward, add `statusText(): string | undefined` returning `this.statusItem.text` when visible. + +- [ ] **Step 5: Run the host E2E suite.** + +Run: `npm run test:e2e` +Expected: PASS (all suites, including the updated proposals/authorship and new F10 cases). + +- [ ] **Step 6: Run the full unit suite too.** + +Run: `npm test` +Expected: PASS. + +- [ ] **Step 7: Commit.** + +```bash +git add test +git commit -m "F10 SLICE-4: host E2E for interactive review (open/toggle/propose→accept→reject/clean-editor/hidden-F6/status) (#29)" +``` + +### Task 14: Manual smoke doc + README + +**Files:** +- Create: `docs/MANUAL-SMOKE-F10.md` +- Modify: `README.md` (add an F10 section; note F6 two-pane + F9 authorship-mode are superseded as user surfaces) + +- [ ] **Step 1: Write `docs/MANUAL-SMOKE-F10.md`** following the F7/F9 smoke format. Checklist: open a markdown doc → editor is clean (no tint/threads); edit prose → green `` / struck `` in the preview; ask Claude to edit a selection → a blue proposal block with ✓/✗ appears; click ✓ (lands, mark clears, editor updated) and ✗ on another (block vanishes, doc unchanged); toggle Annotations off (clean render) / on; with no preview open, the status-bar indicator shows the pending count and opens the preview when clicked; verify light/dark/high-contrast theming; verify `git status` shows nothing persisted. + +- [ ] **Step 2: Update `README.md`.** Add an F10 entry to the feature list describing "write left / review right," the annotations on/off toggle, and ✓/✗ in the preview; add a one-line note that F6's two-pane diff and F9's authorship mode are retained as data layers but no longer separate user surfaces; mention `ctrl+alt+r` opens the review preview. + +- [ ] **Step 3: Commit.** + +```bash +git add docs/MANUAL-SMOKE-F10.md README.md +git commit -m "F10 SLICE-4: manual smoke checklist + README F10 section (#29)" +``` + +--- + +## Final verification (before PR) + +- [ ] `npm test` — vitest green (model incl. renderReview/renderPlain/colorByAuthor; proposalModel; all vscode-free suites). +- [ ] `npm run test:e2e` — host E2E green (open/toggle/propose→accept→reject/clean-editor/hidden-F6/status/non-markdown). +- [ ] `npm run build` — clean typecheck + bundles. +- [ ] Manual smoke per `docs/MANUAL-SMOKE-F10.md` performed once (the webview visual + real button clicks the E2E can't cover). +- [ ] `git status` clean (nothing persisted by the preview — INV-20). +- [ ] Open PR to `main`, citing `#29` and `specs/coauthoring-interactive-review.md`; acceptance per spec §7.3. + +--- + +## Self-Review notes (spec coverage) + +- INV-32 (clean editor) → SLICE-1 (Tasks 1-3). INV-33 (combined pure render) → SLICE-2 (Tasks 4-7). INV-34 (✓/✗ via F4 seam) → SLICE-3 (Tasks 8-12). +- PUC-1 clean editor → Task 1/2; PUC-2 toggle → Task 9/11; PUC-3 who-changed-what → Task 6; PUC-4/5 accept/reject → Task 9 (routing) + Task 6 (blocks); PUC-6 status-bar → Task 10; PUC-7 edges (non-markdown warn, no-baseline note, unanchored proposal) → Task 6 (unanchored), Task 9 (no-baseline), `show()` (non-markdown, existing). +- F9 INV-26 reversal (authorship combined with diff; remove segmented control/`renderAuthorship`) → Tasks 7, 9, 13(step1). +- Supersession: F6 two-pane hidden → Task 3; F9 INV-27/28 absorbed into renderReview → Task 6. +- Reconciliation captured: public seams `acceptById`/`rejectById` (Task 9), `Proposal.anchorId` resolved via `resolve()` in `listProposals` (Task 8), new `onDidChangeProposals` (Task 8), `keyFor` exposure for the preview's doc key (Task 9). diff --git a/plans/2026-06-11-f7-intra-diagram-mermaid-diff.md b/plans/2026-06-11-f7-intra-diagram-mermaid-diff.md new file mode 100644 index 0000000..b58db47 --- /dev/null +++ b/plans/2026-06-11-f7-intra-diagram-mermaid-diff.md @@ -0,0 +1,1034 @@ +# F7.1 Intra-Diagram Mermaid Diffing Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Refine the F7 whole-diagram "changed" badge into a node/edge-level diff *inside* flowchart and sequence mermaid diagrams against the F6 baseline. + +**Architecture:** Three new **pure, vscode-free, DOM-free** host modules under `src/` parse a changed mermaid block's before/after source into a typed element model, diff it, and re-emit the *current* diagram source augmented with mermaid's own styling directives (flowchart: `classDef`/`class`/`linkStyle`; sequence: `rect rgb(...)` tinted runs), ghosting removed elements in place. The dispatcher falls back to the v1 whole-block badge for unsupported types or any parse failure. `renderOp`'s changed-mermaid branch calls the dispatcher; the webview is unchanged (styling rides inside the mermaid source); `media/preview.css` gains a legend. + +**Tech Stack:** TypeScript, `diff` (jsdiff) for sequence LCS, `markdown-it` (existing fence rule), vitest (unit), `@vscode/test-electron` + mocha (host E2E). + +**Spec:** `vscode-cowriting-plugin-content/specs/coauthoring-rendered-preview.md` §11 (INV-29..31). Task `benstull/vscode-cowriting-plugin#22`. + +--- + +## File Structure + +- **Create `src/mermaidDiff.ts`** — dispatcher. `detectDiagramType(source)`, `diffMermaid(beforeSrc, currentSrc): MermaidDiffResult`. Wraps the typed differ in try/catch → `{kind:"fallback"}` on any throw or unsupported type. Owns the shared color palette constants. +- **Create `src/mermaidFlowchartDiff.ts`** — `parseFlowchart(source): FlowGraph`, `diffFlowchart(beforeSrc, currentSrc): string`. +- **Create `src/mermaidSequenceDiff.ts`** — `parseSequence(source): SeqDiagram`, `diffSequence(beforeSrc, currentSrc): string`. +- **Modify `src/trackChangesModel.ts`** — in `renderOp`, the `changed`+`atomic` branch: if the block is a mermaid fence, call `diffMermaid` on the inner source; `augmented` → render the augmented fence + legend (no single badge); `fallback` → today's behavior. Add a `mermaidFenceBody(raw)` helper. +- **Modify `media/preview.css`** — `.cw-mermaid-legend` swatch styles. +- **Create `test/mermaidFlowchartDiff.test.ts`**, **`test/mermaidSequenceDiff.test.ts`**, **`test/mermaidDiff.test.ts`** — unit tests. +- **Modify `test/trackChangesModel.test.ts`** — assert `renderTrackChanges` HTML augments a changed flowchart/sequence and falls back otherwise. +- **Modify `test/e2e/suite/trackChangesPreview.test.ts`** — host E2E for a changed flowchart. +- **Create `docs/MANUAL-SMOKE-F7.1.md`** — manual webview-render smoke steps. + +**Shared palette (define once in `src/mermaidDiff.ts`, import elsewhere):** + +```ts +// Theme-neutral colors baked into emitted mermaid source (source can't read CSS vars). +export const CW_COLORS = { + added: "#2ea043", + changed: "#d29922", + removed: "#808080", +} as const; +``` + +--- + +## Task 1: Dispatcher skeleton — `detectDiagramType` + `diffMermaid` fallback + +**Files:** +- Create: `src/mermaidDiff.ts` +- Test: `test/mermaidDiff.test.ts` + +- [ ] **Step 1: Write the failing test** + +```ts +// test/mermaidDiff.test.ts +import { describe, it, expect } from "vitest"; +import { detectDiagramType, diffMermaid } from "../src/mermaidDiff"; + +describe("detectDiagramType", () => { + it("detects flowchart from `flowchart` / `graph` headers", () => { + expect(detectDiagramType("flowchart LR\n a-->b")).toBe("flowchart"); + expect(detectDiagramType("graph TD\n a-->b")).toBe("flowchart"); + expect(detectDiagramType(" \n%% c\ngraph TD")).toBe("flowchart"); + }); + it("detects sequence from `sequenceDiagram`", () => { + expect(detectDiagramType("sequenceDiagram\n A->>B: hi")).toBe("sequence"); + }); + it("returns `other` for unsupported types", () => { + expect(detectDiagramType("classDiagram\n class A")).toBe("other"); + expect(detectDiagramType("stateDiagram-v2\n s1 --> s2")).toBe("other"); + expect(detectDiagramType("")).toBe("other"); + }); +}); + +describe("diffMermaid", () => { + it("falls back for an unsupported diagram type", () => { + expect(diffMermaid("classDiagram\n class A", "classDiagram\n class B")).toEqual({ kind: "fallback" }); + }); + it("falls back (never throws) when the differ throws", () => { + // A deliberately malformed flowchart still degrades gracefully. + const r = diffMermaid("flowchart LR", "flowchart LR\n a -->"); + expect(["augmented", "fallback"]).toContain(r.kind); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/mermaidDiff.test.ts` +Expected: FAIL — `Cannot find module '../src/mermaidDiff'`. + +- [ ] **Step 3: Write minimal implementation** + +```ts +// src/mermaidDiff.ts +/** + * mermaidDiff — F7.1 (#22) dispatcher. Detects a mermaid diagram's type and + * routes a baseline→current diff to the matching pure differ, which re-emits the + * CURRENT diagram source augmented with mermaid styling directives (INV-29). Any + * unsupported type or parser surprise degrades to the v1 whole-block badge + * (INV-30) — this function never throws. vscode-free, DOM-free, deterministic. + */ +import { diffFlowchart } from "./mermaidFlowchartDiff"; +import { diffSequence } from "./mermaidSequenceDiff"; + +export const CW_COLORS = { + added: "#2ea043", + changed: "#d29922", + removed: "#808080", +} as const; + +export type DiagramType = "flowchart" | "sequence" | "other"; + +export interface MermaidDiffAugmented { + kind: "augmented"; + source: string; +} +export interface MermaidDiffFallback { + kind: "fallback"; +} +export type MermaidDiffResult = MermaidDiffAugmented | MermaidDiffFallback; + +export function detectDiagramType(source: string): DiagramType { + for (const line of source.split(/\r?\n/)) { + const t = line.trim(); + if (t === "" || t.startsWith("%%")) continue; + if (/^(flowchart|graph)\b/.test(t)) return "flowchart"; + if (/^sequenceDiagram\b/.test(t)) return "sequence"; + return "other"; + } + return "other"; +} + +export function diffMermaid(beforeSrc: string, currentSrc: string): MermaidDiffResult { + const type = detectDiagramType(currentSrc); + try { + if (type === "flowchart") return { kind: "augmented", source: diffFlowchart(beforeSrc, currentSrc) }; + if (type === "sequence") return { kind: "augmented", source: diffSequence(beforeSrc, currentSrc) }; + } catch { + return { kind: "fallback" }; + } + return { kind: "fallback" }; +} +``` + +Also create minimal stubs so the module imports resolve (real impl in Tasks 2–5): + +```ts +// src/mermaidFlowchartDiff.ts — stub, replaced in Tasks 2-3 +export function diffFlowchart(_beforeSrc: string, currentSrc: string): string { + return currentSrc; +} +``` + +```ts +// src/mermaidSequenceDiff.ts — stub, replaced in Tasks 4-5 +export function diffSequence(_beforeSrc: string, currentSrc: string): string { + return currentSrc; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/mermaidDiff.test.ts` +Expected: PASS (5 tests). + +- [ ] **Step 5: Commit** + +```bash +git add src/mermaidDiff.ts src/mermaidFlowchartDiff.ts src/mermaidSequenceDiff.ts test/mermaidDiff.test.ts +git commit -m "feat(f7.1): mermaid diff dispatcher + type detection (#22)" +``` + +--- + +## Task 2: Flowchart parser — `parseFlowchart` + +**Files:** +- Modify: `src/mermaidFlowchartDiff.ts` (replace the stub) +- Test: `test/mermaidFlowchartDiff.test.ts` + +- [ ] **Step 1: Write the failing test** + +```ts +// test/mermaidFlowchartDiff.test.ts +import { describe, it, expect } from "vitest"; +import { parseFlowchart } from "../src/mermaidFlowchartDiff"; + +describe("parseFlowchart", () => { + it("captures the header and node declarations with labels + shapes", () => { + const g = parseFlowchart("flowchart LR\n A[Start] --> B(Middle)\n B --> C{End}"); + expect(g.header).toBe("flowchart LR"); + expect(g.nodes.get("A")).toMatchObject({ id: "A", label: "Start", open: "[", close: "]" }); + expect(g.nodes.get("B")).toMatchObject({ id: "B", label: "Middle", open: "(", close: ")" }); + expect(g.nodes.get("C")).toMatchObject({ id: "C", label: "End", open: "{", close: "}" }); + }); + + it("captures edges in declaration order with from/to and index", () => { + const g = parseFlowchart("graph TD\n A-->B\n B-->C"); + expect(g.edges.map((e) => [e.from, e.to, e.index])).toEqual([ + ["A", "B", 0], + ["B", "C", 1], + ]); + }); + + it("records a node referenced only in an edge (no explicit declaration)", () => { + const g = parseFlowchart("flowchart LR\n A-->B"); + expect(g.nodes.has("A")).toBe(true); + expect(g.nodes.has("B")).toBe(true); + expect(g.nodes.get("A")?.label).toBeUndefined(); + }); + + it("handles a labelled edge `A -->|yes| B`", () => { + const g = parseFlowchart("flowchart LR\n A -->|yes| B"); + expect(g.edges[0]).toMatchObject({ from: "A", to: "B", label: "yes" }); + }); + + it("ignores comments and blank lines", () => { + const g = parseFlowchart("flowchart LR\n%% a comment\n\n A-->B"); + expect(g.edges).toHaveLength(1); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/mermaidFlowchartDiff.test.ts` +Expected: FAIL — `parseFlowchart` is not exported. + +- [ ] **Step 3: Write minimal implementation** + +```ts +// src/mermaidFlowchartDiff.ts +/** + * mermaidFlowchartDiff — F7.1 (#22). Pure flowchart parser + diff/emit. Parses a + * `graph`/`flowchart` source into nodes (by id) and edges (by declaration order), + * diffs baseline vs current, and re-emits the CURRENT source augmented with + * `classDef`/`class`/`linkStyle` directives coloring added/changed elements and + * ghosting removed ones (INV-29/31). Deterministic; no vscode, no DOM. + */ +import { CW_COLORS } from "./mermaidDiff"; + +export interface FlowNode { + id: string; + label?: string; + open?: string; + close?: string; + /** Verbatim declaration token, e.g. `A[Start]`, for ghost re-injection. */ + decl?: string; +} +export interface FlowEdge { + from: string; + to: string; + label?: string; + index: number; + /** Verbatim edge statement for ghost re-injection. */ + raw: string; +} +export interface FlowGraph { + header: string; + nodes: Map; + edges: FlowEdge[]; +} + +const NODE_TOKEN = /^([A-Za-z0-9_]+)(\[\[[^\]]*\]\]|\[[^\]]*\]|\(\([^)]*\)\)|\([^)]*\)|\{[^}]*\}|>[^\]]*\])?/; +// link operators: -->, ---, -.->, -.-, ==>, ===, --x, --o, ==x, ==o +const LINK = /(-->|---|-\.->|-\.-|==>|===|--[xo]|==[xo])(\|([^|]*)\|)?/; + +function shapeOf(bracket: string): { label: string; open: string; close: string } { + const open = bracket[0]; + if (bracket.startsWith("[[")) return { label: bracket.slice(2, -2), open: "[[", close: "]]" }; + if (bracket.startsWith("((")) return { label: bracket.slice(2, -2), open: "((", close: "))" }; + const close = bracket[bracket.length - 1]; + return { label: bracket.slice(1, -1), open, close }; +} + +function recordNode(nodes: Map, token: string): string | null { + const m = token.match(NODE_TOKEN); + if (!m) return null; + const id = m[1]; + const existing = nodes.get(id) ?? { id }; + if (m[2]) { + const s = shapeOf(m[2]); + existing.label = s.label; + existing.open = s.open; + existing.close = s.close; + existing.decl = `${id}${m[2]}`; + } + nodes.set(id, existing); + return id; +} + +export function parseFlowchart(source: string): FlowGraph { + const lines = source.split(/\r?\n/); + let header = ""; + const nodes = new Map(); + const edges: FlowEdge[] = []; + for (const rawLine of lines) { + const line = rawLine.trim(); + if (line === "" || line.startsWith("%%")) continue; + if (!header && /^(flowchart|graph)\b/.test(line)) { + header = line; + continue; + } + // Walk a (possibly chained) edge statement: A[..] --> B -->|x| C + let rest = line; + let prevId: string | null = null; + let consumedEdge = false; + while (rest.length > 0) { + const nodeM = rest.match(NODE_TOKEN); + if (!nodeM) break; + const id = recordNode(nodes, nodeM[0]); + rest = rest.slice(nodeM[0].length).trimStart(); + const linkM = rest.match(LINK); + if (linkM && prevId === null) { + // current token is the LHS; fall through to read RHS + } + if (linkM) { + const fromId = id!; + rest = rest.slice(linkM[0].length).trimStart(); + const rhsM = rest.match(NODE_TOKEN); + if (!rhsM) break; + const toId = recordNode(nodes, rhsM[0]); + edges.push({ + from: fromId, + to: toId!, + label: linkM[3] || undefined, + index: edges.length, + raw: `${fromId} ${linkM[1]}${linkM[2] ? linkM[2] : ""} ${toId}`, + }); + rest = rest.slice(rhsM[0].length).trimStart(); + prevId = toId; + consumedEdge = true; + // chained: the RHS may be the LHS of another link + const more = rest.match(LINK); + if (more) rest = `${toId} ${rest}`; + continue; + } + break; + } + void consumedEdge; + } + return { header: header || "flowchart TD", nodes, edges }; +} +``` + +> Note: the chained-edge handling (`A --> B --> C`) is intentionally simple. The unit tests in this task pin the contract for the single-edge and two-statement cases; if a chained test is added later, the `more`/re-prefix branch is the place to harden. Keep the parser total — never throw on odd input; return whatever was parsed. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/mermaidFlowchartDiff.test.ts` +Expected: PASS (5 tests). + +- [ ] **Step 5: Commit** + +```bash +git add src/mermaidFlowchartDiff.ts test/mermaidFlowchartDiff.test.ts +git commit -m "feat(f7.1): flowchart parser (#22)" +``` + +--- + +## Task 3: Flowchart diff + augmented emission — `diffFlowchart` + +**Files:** +- Modify: `src/mermaidFlowchartDiff.ts` +- Test: `test/mermaidFlowchartDiff.test.ts` + +- [ ] **Step 1: Write the failing test** + +```ts +// append to test/mermaidFlowchartDiff.test.ts +import { diffFlowchart } from "../src/mermaidFlowchartDiff"; + +describe("diffFlowchart", () => { + it("colors an added node with the cwAdded class", () => { + const before = "flowchart LR\n A-->B"; + const current = "flowchart LR\n A-->B\n B-->C"; + const out = diffFlowchart(before, current); + expect(out).toContain("classDef cwAdded"); + expect(out).toMatch(/class\s+C\s+cwAdded/); + }); + + it("colors a changed node (same id, new label) with cwChanged", () => { + const before = "flowchart LR\n A[Old]-->B"; + const current = "flowchart LR\n A[New]-->B"; + const out = diffFlowchart(before, current); + expect(out).toMatch(/class\s+A\s+cwChanged/); + }); + + it("styles an added edge via linkStyle on its current index", () => { + const before = "flowchart LR\n A-->B"; + const current = "flowchart LR\n A-->B\n A-->C"; + const out = diffFlowchart(before, current); + // edge index 1 (A-->C) is the added one + expect(out).toMatch(/linkStyle\s+1\s+stroke:#2ea043/); + }); + + it("ghosts a removed node in place with cwRemoved (re-injected decl)", () => { + const before = "flowchart LR\n A-->B\n B-->C[Gone]"; + const current = "flowchart LR\n A-->B"; + const out = diffFlowchart(before, current); + expect(out).toContain("C[Gone]"); // re-injected + expect(out).toMatch(/class\s+C\s+cwRemoved/); + }); + + it("is deterministic (same inputs → identical output)", () => { + const before = "flowchart LR\n A-->B"; + const current = "flowchart LR\n A-->B\n B-->C\n A-->D"; + expect(diffFlowchart(before, current)).toBe(diffFlowchart(before, current)); + }); + + it("preserves the original current source as the prefix", () => { + const current = "flowchart LR\n A-->B\n B-->C"; + const out = diffFlowchart("flowchart LR\n A-->B", current); + expect(out.startsWith(current)).toBe(true); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/mermaidFlowchartDiff.test.ts` +Expected: FAIL — `diffFlowchart` returns the current source unchanged (stub), so `classDef`/`class` assertions fail. + +- [ ] **Step 3: Write the implementation** + +```ts +// append to src/mermaidFlowchartDiff.ts + +function nodeChanged(a: FlowNode, b: FlowNode): boolean { + return (a.label ?? "") !== (b.label ?? "") || (a.open ?? "") !== (b.open ?? ""); +} +const edgeKey = (e: FlowEdge): string => `${e.from}${e.to}`; + +export function diffFlowchart(beforeSrc: string, currentSrc: string): string { + const before = parseFlowchart(beforeSrc); + const current = parseFlowchart(currentSrc); + + const addedNodes: string[] = []; + const changedNodes: string[] = []; + for (const [id, cur] of current.nodes) { + const prev = before.nodes.get(id); + if (!prev) addedNodes.push(id); + else if (nodeChanged(prev, cur)) changedNodes.push(id); + } + const removedNodes: FlowNode[] = []; + for (const [id, prev] of before.nodes) { + if (!current.nodes.has(id)) removedNodes.push(prev); + } + + const beforeEdgeKeys = new Set(before.edges.map(edgeKey)); + const currentEdgeKeys = new Set(current.edges.map(edgeKey)); + const addedEdgeIdx = current.edges.filter((e) => !beforeEdgeKeys.has(edgeKey(e))).map((e) => e.index); + const removedEdges = before.edges.filter((e) => !currentEdgeKeys.has(edgeKey(e))); + + // Ghost-edge indices follow the current edge count, in deterministic order. + let nextIdx = current.edges.length; + const ghostEdgeLines: string[] = []; + const ghostEdgeIdx: number[] = []; + for (const e of removedEdges) { + ghostEdgeLines.push(` ${e.from} -.-> ${e.to}`); + ghostEdgeIdx.push(nextIdx++); + } + + const out: string[] = [currentSrc.replace(/\s+$/, "")]; + + // Ghost removed nodes: re-inject their declaration (or bare id) so they appear. + for (const n of removedNodes) out.push(` ${n.decl ?? n.id}`); + out.push(...ghostEdgeLines); + + // classDefs (always emit the three; harmless if a class is unused). + out.push( + ` classDef cwAdded fill:${CW_COLORS.added}22,stroke:${CW_COLORS.added},stroke-width:2px;`, + ` classDef cwChanged fill:${CW_COLORS.changed}22,stroke:${CW_COLORS.changed},stroke-width:2px;`, + ` classDef cwRemoved fill:${CW_COLORS.removed}11,stroke:${CW_COLORS.removed},stroke-width:1px,stroke-dasharray:5 3,color:${CW_COLORS.removed};`, + ); + if (addedNodes.length) out.push(` class ${addedNodes.join(",")} cwAdded;`); + if (changedNodes.length) out.push(` class ${changedNodes.join(",")} cwChanged;`); + if (removedNodes.length) out.push(` class ${removedNodes.map((n) => n.id).join(",")} cwRemoved;`); + + for (const i of addedEdgeIdx) out.push(` linkStyle ${i} stroke:${CW_COLORS.added},stroke-width:2px;`); + for (const i of ghostEdgeIdx) + out.push(` linkStyle ${i} stroke:${CW_COLORS.removed},stroke-width:1px,stroke-dasharray:5 3;`); + + return out.join("\n"); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/mermaidFlowchartDiff.test.ts` +Expected: PASS (all flowchart tests). + +- [ ] **Step 5: Commit** + +```bash +git add src/mermaidFlowchartDiff.ts test/mermaidFlowchartDiff.test.ts +git commit -m "feat(f7.1): flowchart node/edge diff + styling emission (#22)" +``` + +--- + +## Task 4: Sequence parser — `parseSequence` + +**Files:** +- Modify: `src/mermaidSequenceDiff.ts` (replace the stub) +- Test: `test/mermaidSequenceDiff.test.ts` + +- [ ] **Step 1: Write the failing test** + +```ts +// test/mermaidSequenceDiff.test.ts +import { describe, it, expect } from "vitest"; +import { parseSequence } from "../src/mermaidSequenceDiff"; + +describe("parseSequence", () => { + it("captures explicit participants in order", () => { + const d = parseSequence("sequenceDiagram\n participant A\n participant B\n A->>B: hi"); + expect(d.participants).toEqual(["A", "B"]); + }); + + it("captures message statements verbatim (trimmed)", () => { + const d = parseSequence("sequenceDiagram\n A->>B: hi\n B-->>A: bye"); + expect(d.statements).toEqual(["A->>B: hi", "B-->>A: bye"]); + }); + + it("infers participants from messages when not declared", () => { + const d = parseSequence("sequenceDiagram\n A->>B: hi\n B->>C: on"); + expect(d.participants).toEqual(["A", "B", "C"]); + }); + + it("ignores comments and blank lines", () => { + const d = parseSequence("sequenceDiagram\n%% note\n\n A->>B: hi"); + expect(d.statements).toEqual(["A->>B: hi"]); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/mermaidSequenceDiff.test.ts` +Expected: FAIL — `parseSequence` is not exported. + +- [ ] **Step 3: Write the implementation** + +```ts +// src/mermaidSequenceDiff.ts +/** + * mermaidSequenceDiff — F7.1 (#22). Pure sequence-diagram parser + diff/emit. + * Mermaid sequence diagrams have NO per-message color directive; the only + * per-message hook is the `rect rgb(...) … end` background block. So the emitter + * rebuilds the message stream (ghosted-removed messages re-inserted at their + * baseline position) wrapping each added/changed/removed message in a one-message + * tinted rect, and re-declares removed participants (INV-29/31). Deterministic; + * no vscode, no DOM. + */ +import { diffArrays } from "diff"; +import { CW_COLORS } from "./mermaidDiff"; + +export interface SeqDiagram { + header: string; + participants: string[]; + /** Message/other statement lines, verbatim & trimmed, in order. */ + statements: string[]; +} + +// `A->>B: text`, plus -->>, ->, -->, -x, --x, etc. Capture from/to. +const MSG = /^([A-Za-z0-9_]+)\s*(-?-(?:>>|>|x|\))|--?(?:>>|>|x|\)))\s*([A-Za-z0-9_]+)\s*:/; +const PARTICIPANT = /^(?:participant|actor)\s+([A-Za-z0-9_]+)/; + +export function parseSequence(source: string): SeqDiagram { + const lines = source.split(/\r?\n/); + let header = ""; + const declared: string[] = []; + const seen = new Set(); + const statements: string[] = []; + const addP = (p: string) => { + if (!seen.has(p)) { + seen.add(p); + declared.push(p); + } + }; + for (const rawLine of lines) { + const line = rawLine.trim(); + if (line === "" || line.startsWith("%%")) continue; + if (!header && /^sequenceDiagram\b/.test(line)) { + header = line; + continue; + } + const pm = line.match(PARTICIPANT); + if (pm) { + addP(pm[1]); + continue; + } + const mm = line.match(MSG); + if (mm) { + addP(mm[1]); + addP(mm[3]); + } + statements.push(line); + } + return { header: header || "sequenceDiagram", participants: declared, statements }; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/mermaidSequenceDiff.test.ts` +Expected: PASS (4 tests). + +- [ ] **Step 5: Commit** + +```bash +git add src/mermaidSequenceDiff.ts test/mermaidSequenceDiff.test.ts +git commit -m "feat(f7.1): sequence-diagram parser (#22)" +``` + +--- + +## Task 5: Sequence diff + augmented emission — `diffSequence` + +**Files:** +- Modify: `src/mermaidSequenceDiff.ts` +- Test: `test/mermaidSequenceDiff.test.ts` + +- [ ] **Step 1: Write the failing test** + +```ts +// append to test/mermaidSequenceDiff.test.ts +import { diffSequence } from "../src/mermaidSequenceDiff"; + +const rectCount = (s: string, rgb: string) => s.split(`rect rgb(${rgb})`).length - 1; + +describe("diffSequence", () => { + it("wraps an added message in a green rect", () => { + const before = "sequenceDiagram\n A->>B: hi"; + const current = "sequenceDiagram\n A->>B: hi\n B->>C: on"; + const out = diffSequence(before, current); + expect(rectCount(out, "46, 160, 67")).toBe(1); // CW_COLORS.added as rgb + expect(out).toContain("B->>C: on"); + }); + + it("ghosts a removed message back in a grey rect", () => { + const before = "sequenceDiagram\n A->>B: hi\n B->>C: gone"; + const current = "sequenceDiagram\n A->>B: hi"; + const out = diffSequence(before, current); + expect(out).toContain("B->>C: gone"); + expect(rectCount(out, "128, 128, 128")).toBe(1); + }); + + it("re-declares a removed participant so it still appears", () => { + const before = "sequenceDiagram\n participant A\n participant B\n participant C\n A->>C: x"; + const current = "sequenceDiagram\n participant A\n participant B\n A->>B: y"; + const out = diffSequence(before, current); + expect(out).toMatch(/participant C/); + }); + + it("keeps an unchanged message outside any rect", () => { + const doc = "sequenceDiagram\n A->>B: hi"; + const out = diffSequence(doc, doc); + expect(out).not.toContain("rect rgb"); + }); + + it("is deterministic", () => { + const before = "sequenceDiagram\n A->>B: hi"; + const current = "sequenceDiagram\n A->>B: hi\n B->>A: bye"; + expect(diffSequence(before, current)).toBe(diffSequence(before, current)); + }); +}); +``` + +> Note: `CW_COLORS.added` is `#2ea043` → rgb `46, 160, 67`; `changed` `#d29922` → `210, 153, 34`; `removed` `#808080` → `128, 128, 128`. `diffSequence` must emit `rect rgb(r, g, b)` from the hex. Add a `hexToRgb` helper. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/mermaidSequenceDiff.test.ts` +Expected: FAIL — `diffSequence` (stub) returns current unchanged; no `rect` emitted. + +- [ ] **Step 3: Write the implementation** + +```ts +// append to src/mermaidSequenceDiff.ts + +function hexToRgb(hex: string): string { + const h = hex.replace("#", ""); + const n = parseInt(h, 16); + return `${(n >> 16) & 255}, ${(n >> 8) & 255}, ${n & 255}`; +} + +function rectWrap(stmt: string, hex: string): string[] { + return [` rect rgb(${hexToRgb(hex)})`, ` ${stmt}`, ` end`]; +} + +export function diffSequence(beforeSrc: string, currentSrc: string): string { + const before = parseSequence(beforeSrc); + const current = parseSequence(currentSrc); + + const out: string[] = [current.header]; + + // Participants: keep current declarations, then re-declare removed ones (ghosts). + for (const p of current.participants) out.push(` participant ${p}`); + const curSet = new Set(current.participants); + for (const p of before.participants) { + if (!curSet.has(p)) out.push(` participant ${p}`); + } + + // Message stream diff via LCS over statements; pair adjacent removed+added as changed. + const parts = diffArrays(before.statements, current.statements); + for (let n = 0; n < parts.length; n++) { + const ch = parts[n]; + if (!ch.added && !ch.removed) { + for (const s of ch.value) out.push(` ${s}`); + continue; + } + if (ch.removed) { + const next = parts[n + 1]; + const addVals = next?.added ? next.value : []; + const paired = Math.min(ch.value.length, addVals.length); + for (let k = 0; k < paired; k++) out.push(...rectWrap(addVals[k], CW_COLORS.changed)); + for (let k = paired; k < ch.value.length; k++) out.push(...rectWrap(ch.value[k], CW_COLORS.removed)); + for (let k = paired; k < addVals.length; k++) out.push(...rectWrap(addVals[k], CW_COLORS.added)); + if (next?.added) n++; + continue; + } + // lone added run + for (const s of ch.value) out.push(...rectWrap(s, CW_COLORS.added)); + } + return out.join("\n"); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/mermaidSequenceDiff.test.ts` +Expected: PASS (all sequence tests). + +- [ ] **Step 5: Commit** + +```bash +git add src/mermaidSequenceDiff.ts test/mermaidSequenceDiff.test.ts +git commit -m "feat(f7.1): sequence participant/message diff + rect emission (#22)" +``` + +--- + +## Task 6: Wire `diffMermaid` into `renderOp` + legend + +**Files:** +- Modify: `src/trackChangesModel.ts` (the `changed` case in `renderOp`, ~line 256-264; add a `mermaidFenceBody` helper near the other helpers) +- Test: `test/trackChangesModel.test.ts` + +- [ ] **Step 1: Write the failing test** + +```ts +// append to test/trackChangesModel.test.ts +import { renderTrackChanges as rtc2 } from "../src/trackChangesModel"; + +describe("renderTrackChanges — intra-diagram mermaid (#22)", () => { + it("augments a changed flowchart with styling directives (INV-29)", () => { + const base = "```mermaid\nflowchart LR\n A-->B\n```\n"; + const cur = "```mermaid\nflowchart LR\n A-->B\n B-->C\n```\n"; + const html = rtc2(base, cur); + expect(html).toContain("classDef cwAdded"); + expect(html).toMatch(/class\s+C\s+cwAdded/); + expect(html).toContain('class="cw-mermaid-legend"'); + // no single whole-block "changed" badge when we augmented + expect(html).not.toContain('changed'); + }); + + it("augments a changed sequence diagram", () => { + const base = "```mermaid\nsequenceDiagram\n A->>B: hi\n```\n"; + const cur = "```mermaid\nsequenceDiagram\n A->>B: hi\n B->>C: on\n```\n"; + const html = rtc2(base, cur); + expect(html).toContain("rect rgb(46, 160, 67)"); + }); + + it("falls back to the v1 badge for an unsupported diagram type (INV-30)", () => { + const base = "```mermaid\nclassDiagram\n class A\n```\n"; + const cur = "```mermaid\nclassDiagram\n class B\n```\n"; + const html = rtc2(base, cur); + expect(html).toContain('changed'); + expect(html).not.toContain("classDef cwAdded"); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/trackChangesModel.test.ts` +Expected: FAIL — current changed-mermaid branch always emits the badge, never the directives. + +- [ ] **Step 3: Write the implementation** + +Add the import at the top of `src/trackChangesModel.ts` (with the other imports): + +```ts +import { diffMermaid } from "./mermaidDiff"; +``` + +Add a helper near `wordMergedMarkdown` (returns the inner mermaid source of a fence, or null if not a single mermaid fence): + +```ts +/** Inner source of a ```mermaid fence (drops the fence lines), or null. */ +function mermaidFenceBody(raw: string): string | null { + const lines = raw.split(/\r?\n/); + const open = lines[0]?.match(/^(\s*)(`{3,}|~{3,})\s*(\w+)?/); + if (!open || open[3]?.toLowerCase() !== "mermaid") return null; + const body = lines.slice(1); + if (body.length && /^(\s*)(`{3,}|~{3,})/.test(body[body.length - 1])) body.pop(); + return body.join("\n"); +} +``` + +Replace the `changed` case's atomic branch (currently): + +```ts + if (op.atomic) { + inner = safe(op.block.raw); // the NEW block, whole (INV-23) + badge = 'changed'; + } else { +``` + +with: + +```ts + if (op.atomic) { + const curBody = op.block.type === "mermaid" ? mermaidFenceBody(op.block.raw) : null; + const beforeBody = op.before.type === "mermaid" ? mermaidFenceBody(op.before.raw) : null; + const md = curBody !== null && beforeBody !== null ? diffMermaid(beforeBody, curBody) : { kind: "fallback" as const }; + if (md.kind === "augmented") { + // Re-render the augmented diagram as a mermaid fence; add a legend, drop the badge. + inner = safe("```mermaid\n" + md.source + "\n```") + MERMAID_LEGEND; + } else { + inner = safe(op.block.raw); // the NEW block, whole (INV-23 / INV-30 fallback) + badge = 'changed'; + } + } else { +``` + +Add the legend constant near the top-level helpers: + +```ts +const MERMAID_LEGEND = + '
' + + 'added' + + 'changed' + + 'removed' + + "
"; +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/trackChangesModel.test.ts` +Expected: PASS (existing + 3 new). Then run the whole unit suite: `npm test` — all green (no regressions to the existing `a mermaid-fence change → an ATOMIC changed op` test, which asserts the *op*, not the HTML). + +- [ ] **Step 5: Commit** + +```bash +git add src/trackChangesModel.ts test/trackChangesModel.test.ts +git commit -m "feat(f7.1): render augmented mermaid diff in changed-block branch (#22)" +``` + +--- + +## Task 7: Legend CSS + +**Files:** +- Modify: `media/preview.css` + +- [ ] **Step 1: Add the legend styles** (append to `media/preview.css`) + +```css +/* F7.1 (#22) intra-diagram mermaid diff legend. */ +.cw-mermaid-legend { display: flex; gap: 0.6rem; font-size: 0.75em; opacity: 0.85; margin: 0.2rem 0 0.6rem; } +.cw-mermaid-legend .cw-leg { padding: 0 0.4em; border-radius: 3px; border: 1px solid; } +.cw-mermaid-legend .cw-leg-add { color: #2ea043; border-color: #2ea043; } +.cw-mermaid-legend .cw-leg-chg { color: #d29922; border-color: #d29922; } +.cw-mermaid-legend .cw-leg-rem { color: #808080; border-color: #808080; border-style: dashed; } +``` + +- [ ] **Step 2: Build to confirm esbuild emits the CSS** + +Run: `npm run build` +Expected: build succeeds; `out/media/preview.css` exists and contains `cw-mermaid-legend`. + +Run: `grep -c cw-mermaid-legend out/media/preview.css` +Expected: `1` or more. + +- [ ] **Step 3: Commit** + +```bash +git add media/preview.css +git commit -m "feat(f7.1): mermaid diff legend styles (#22)" +``` + +--- + +## Task 8: Host E2E — changed flowchart augments the emitted HTML + +**Files:** +- Modify: `test/e2e/suite/trackChangesPreview.test.ts` +- Check fixture: `test/e2e/fixtures/workspace/docs/preview.md` (read it first; the test edits a buffer in-place — follow the existing pattern in this file for opening the doc, showing the preview, capturing a baseline, applying a `WorkspaceEdit`, and reading `getLastModel`). + +- [ ] **Step 1: Read the existing E2E test + fixture to match the exact harness** + +Run: `sed -n '1,60p' test/e2e/suite/trackChangesPreview.test.ts` +Note how `api`, `docUri()`, baseline capture (`cowriting.pinDiffBaseline` / proposal accept) and `applyEdit` are used; reuse them verbatim. + +- [ ] **Step 2: Write the failing test** (add inside the existing `suite`/`describe`) + +```ts + test("a changed flowchart augments the emitted mermaid source (#22, INV-29)", async () => { + // Open a markdown doc containing a flowchart, show preview, pin baseline. + const doc = await openPreviewDoc("```mermaid\nflowchart LR\n A-->B\n```\n"); + await vscode.commands.executeCommand("cowriting.showTrackChangesPreview"); + await vscode.commands.executeCommand("cowriting.pinDiffBaseline"); + + // Edit: add a node C. + const edit = new vscode.WorkspaceEdit(); + edit.insert(doc.uri, new vscode.Position(2, 7), "\n B-->C"); + assert.ok(await vscode.workspace.applyEdit(edit), "edit applied"); + await waitForRefresh(); + + const model = api.trackChangesPreviewController.getLastModel(doc.uri.toString()) ?? []; + const mermaidOp = model.find((o) => o.kind === "changed" && o.block.type === "mermaid"); + assert.ok(mermaidOp, "the flowchart block is a changed op"); + + const html = api.trackChangesPreviewController.renderHtmlFor(doc.uri.toString()); + assert.match(html, /classDef cwAdded/, "augmented with cwAdded classDef"); + assert.match(html, /class\s+C\s+cwAdded/, "node C colored added"); + }); +``` + +> Note: `openPreviewDoc`, `waitForRefresh`, and `renderHtmlFor` may not exist yet. Use whatever the file already provides (it has a doc-open helper and uses `getLastModel`). If a direct HTML accessor is missing, add a tiny test-seam method `renderHtmlFor(uriString): string` to `TrackChangesPreviewController` that returns `renderTrackChanges(baselineText, current)` for the doc (mirror `getLastModel`), and export it in the §6.4 test-seam block. If the harness can only assert the model (not the HTML), assert on the model instead and move the HTML assertion to the unit test in Task 6 (already covered there) — do not block E2E on webview internals. + +- [ ] **Step 3: Run E2E to verify it fails** + +Run: `npm run test:e2e` +Expected: FAIL on the new assertion (or on the missing seam — add it per the note). + +- [ ] **Step 4: Implement the seam if needed, then re-run** + +If you added `renderHtmlFor`, implement it in `src/trackChangesPreview.ts` test-seam section: + +```ts + /** F7.1 test seam: the track-changes HTML the panel would post for a doc. */ + renderHtmlFor(uriString: string): string { + const doc = vscode.workspace.textDocuments.find((d) => d.uri.toString() === uriString); + if (!doc) return ""; + const current = doc.getText(); + const baseline = this.diffView.getBaseline(uriString); + return renderTrackChanges(baseline?.text ?? current, current); + } +``` + +Run: `npm run test:e2e` +Expected: PASS (existing + new). + +- [ ] **Step 5: Commit** + +```bash +git add test/e2e/suite/trackChangesPreview.test.ts src/trackChangesPreview.ts +git commit -m "test(f7.1): host E2E — changed flowchart augments emitted source (#22)" +``` + +--- + +## Task 9: Manual-smoke doc + full verification + PR + +**Files:** +- Create: `docs/MANUAL-SMOKE-F7.1.md` + +- [ ] **Step 1: Write the manual-smoke doc** (mirror the style of `docs/MANUAL-SMOKE-F7.md`) + +```markdown +# Manual smoke — F7.1 intra-diagram mermaid diffing (#22) + +Webview SVG rendering isn't covered by automated E2E (sealed sandbox, §6.8), so +verify the rendered colors by hand once. + +1. Open a markdown doc with a flowchart: + ```mermaid + flowchart LR + A[Start] --> B[Parse] + B --> C[Emit] + ``` +2. Run **Cowriting: Show Track-Changes Preview** (`ctrl+alt+r`). Pin the baseline + (`Cowriting: Pin Diff Baseline`). +3. Edit the diagram: change `B[Parse]` → `B[Parse+Lint]`, add `B --> D[Validate]`, + delete the `B --> C` edge and the `C` node. +4. **Expect** in the preview: `B` outlined amber (changed), `D` green (added), + `C` shown as a faded dashed ghost (removed), and a legend + `added · changed · removed` under the diagram. +5. Repeat with a `sequenceDiagram`: add a message (green rect), remove one + (grey ghost rect), confirm a removed participant still appears. +6. Change a diagram to a `classDiagram` and confirm it still shows the v1 + whole-block "changed" badge (graceful fallback, INV-30). +``` + +- [ ] **Step 2: Full verification (run all gates)** + +```bash +npm run typecheck && npm test && npm run test:e2e +``` +Expected: typecheck clean; full vitest suite green (≈164+ tests); E2E green (≈38 tests). Record the counts. + +- [ ] **Step 3: Commit the smoke doc** + +```bash +git add docs/MANUAL-SMOKE-F7.1.md +git commit -m "docs(f7.1): manual webview-render smoke (#22)" +``` + +- [ ] **Step 4: Push the branch and open the PR** + +```bash +git push -u origin +``` +Open a PR titled `F7.1: intra-diagram mermaid diffing (#22)` whose body cites +spec §11, lists INV-29..31, the new modules, the unit/E2E counts, and links +`Fixes #22`. (The session uses SSH remotes; PR via Gitea per `wgl-gitea-admin` / +the session-finalize flow.) + +--- + +## Self-Review + +**Spec coverage (§11):** +- §11.1 supported types (flowchart, sequence) + total fallback → Tasks 1, 6 (dispatch + fallback), 3, 5. +- §11.2 parsed-graph diff + ghost-in-place → Tasks 2-5. +- §11.3 styling asymmetry (flowchart classDef/linkStyle vs sequence rect) → Tasks 3, 5. +- §11.4 module tree + renderOp seam + webview-unchanged → Tasks 1, 6 (+ legend Task 7). +- §11.5 INV-29 (augment), INV-30 (fallback total), INV-31 (ghost completeness) → asserted in Tasks 3 (ghost), 5 (ghost), 6 (augment + fallback). +- §11.6 unit + host E2E → Tasks 2-6 (unit), Task 8 (E2E). +- §11.7 out-of-scope types not implemented → confirmed (only flowchart + sequence). + +**Placeholder scan:** No TBD/TODO; every code step shows complete code; the one explicitly-flexible spot (E2E HTML seam, Task 8) gives a concrete fallback path. + +**Type consistency:** `MermaidDiffResult` (`augmented`|`fallback`) used identically in `diffMermaid` (Task 1) and `renderOp` (Task 6). `FlowGraph`/`FlowNode`/`FlowEdge` defined once (Task 2) and consumed in Task 3. `SeqDiagram` defined Task 4, consumed Task 5. `CW_COLORS` defined once (Task 1), imported by both differs and (as hex) the legend CSS uses the same literals. `diffFlowchart`/`diffSequence`/`parseFlowchart`/`parseSequence`/`detectDiagramType`/`diffMermaid` names consistent across tasks. diff --git a/plans/2026-06-12-f11-preview-toolbar.md b/plans/2026-06-12-f11-preview-toolbar.md new file mode 100644 index 0000000..ca7c86f --- /dev/null +++ b/plans/2026-06-12-f11-preview-toolbar.md @@ -0,0 +1,123 @@ +# Implementation Plan: F11 — Preview Toolbar as the Primary Interaction Surface (#43) + +**Spec:** `docs/superpowers/specs/2026-06-12-f11-preview-toolbar-interaction-surface.md` +**Anchor:** Feature `benstull/vscode-cowriting-plugin#43` (F11, `type/feature`, `priority/P1`) +**Session:** vscode-cowriting-plugin-0037 + +This plan transcribes the spec's §7.2 slicing plan into concrete, file-level +tasks. Each slice is independently green (unit + host E2E) before the next. Host +E2E is this app's required tier (no browser/deploy stage — a VS Code extension); +no LLM in CI (edit turns stubbed). The webview's visual rendering, the adaptive +label, and the selection→source DOM lookup are manual-smoke only. + +--- + +## SLICE-1 — Pin baseline button + reachability *(the immediate win)* + +Homes the orphaned `cowriting.pinDiffBaseline` command and gives the writer a +reachable Pin control in the preview toolbar. + +**Tasks** + +1. **Host message routing** (`src/trackChangesPreview.ts`): extract the inline + `onDidReceiveMessage` body into a private `handleWebviewMessage(document, m)` + method; add a `pinBaseline` branch that calls `this.diffView.pin(document)` + (the *previewed* document — not `activeTextEditor`). The existing + `onDidChangeBaseline` subscription already re-renders with cleared marks. +2. **Test seam**: add `receiveMessage(uriString, m)` that resolves the doc and + calls `handleWebviewMessage`, so host E2E can simulate the raw webview message + and exercise the real routing. +3. **Webview** (`media/preview.ts` + `.css`): add a `⌖ Pin baseline` button in + `#cw-header`; click → `postMessage({ type: "pinBaseline" })`; theme-aware CSS. +4. **Shell HTML** (`shellHtml`): add the `` + + `` + + `` + + `` + + `` + + `` + + `` + + ``; +``` + +- [ ] **Step 4: Run to verify it passes** + +Run: `npx vitest run test/trackChangesModel.test.ts -t "Accept/Reject controls with dropdown"` +Expected: PASS + +- [ ] **Step 5: Wire the webview client + CSS** + +In `media/preview.ts`, replace the delegated proposal-button handler (~L97-107): + +```ts +// F12/#64: Accept/Reject (+ caret → accept-all/reject-all) on a proposal block. +body.addEventListener("click", (e) => { + const btn = (e.target as HTMLElement)?.closest(".cw-actions button"); + if (!btn) return; + const id = btn.closest(".cw-proposal")?.dataset.proposalId; + const action = btn.dataset.action; + if (action === "acceptAll") return void vscodeApi.postMessage({ type: "acceptAll" }); + if (action === "rejectAll") return void vscodeApi.postMessage({ type: "rejectAll" }); + if (id && (action === "accept" || action === "reject")) { + vscodeApi.postMessage({ type: action, proposalId: id }); + } +}); +``` + +In `media/preview.css`, add under the F10 block: + +```css +.cw-btngroup { display: inline-flex; } +.cw-btngroup .cw-caret { border-left: none; padding: 0.1em 0.25em; } +.cw-actions .cw-accept { font-weight: 600; } +.cw-accept:hover, .cw-btngroup:has(.cw-accept) .cw-caret:hover { background: var(--vscode-testing-iconPassed, #2ea043); color: #fff; } +.cw-reject:hover, .cw-btngroup:has(.cw-reject) .cw-caret:hover { background: var(--vscode-errorForeground, #f14c4c); color: #fff; } +``` + +- [ ] **Step 6: Route `rejectAll` in the host + add the command** + +In `src/trackChangesPreview.ts`, add `rejectAll` to the `ToolbarMsg` union: + +```ts + | { type: "acceptAll" } + | { type: "rejectAll" }; +``` + +In `handleWebviewMessage`, after the `acceptAll` branch: + +```ts + } else if (m?.type === "rejectAll") { + void this.rejectAll(document); +``` + +And update the existing accept/reject branches to the in-place revert for reject: + +```ts + } else if (m?.type === "reject" && m.proposalId) { + void this.proposals.rejectByIdInPlace(this.proposals.keyFor(document), m.proposalId).then(() => this.refresh(document)); +``` + +Add a public `rejectAll` (mirrors `acceptAll`): + +```ts + /** #64 (INV-53): revert every pending proposal on the document; report the count. */ + async rejectAll(document: vscode.TextDocument): Promise { + const { reverted } = await this.proposals.rejectAll(document); + this.refresh(document); + if (reverted > 0) { + void vscode.window.showInformationMessage( + `Cowriting: rejected ${reverted} proposal${reverted === 1 ? "" : "s"}.`, + ); + } + } +``` + +In `src/extension.ts`, register the command (next to `acceptAllProposals` ~L129): + +```ts + context.subscriptions.push( + vscode.commands.registerCommand("cowriting.rejectAllProposals", async () => { + const doc = vscode.window.activeTextEditor?.document; + if (!doc || doc.languageId !== "markdown") { + void vscode.window.showWarningMessage("Cowriting: open a Markdown document to reject its proposals."); + return; + } + await trackChangesPreviewController.rejectAll(doc); + }), + ); +``` + +- [ ] **Step 7: Update `package.json` contributes** + +Add to `contributes.commands`: + +```json +{ "command": "cowriting.rejectAllProposals", "title": "Reject All Claude Proposals" }, +{ "command": "cowriting.proposalAcceptMenu", "title": "Accept Claude Proposal" }, +{ "command": "cowriting.proposalRejectMenu", "title": "Reject Claude Proposal" } +``` + +Add `commandPalette` `when` guards (mirror `acceptAllProposals`): + +```json +{ "command": "cowriting.rejectAllProposals", "when": "editorLangId == markdown" }, +{ "command": "cowriting.proposalAcceptMenu", "when": "false" }, +{ "command": "cowriting.proposalRejectMenu", "when": "false" } +``` + +- [ ] **Step 8: Write the E2E (both surfaces resolve identically)** + +Append to `f12InlineDiff.test.ts`: + +```ts +suite("F12 inline diff — control parity (#64, INV-53)", () => { + test("reject from the webview reverts in place; rejectAll clears every proposal", async () => { + const { doc, key } = await freshDoc("docs/f12-parity.md", "# P\n\nuno aaa.\n\ndos bbb.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + await vscode.commands.executeCommand("cowriting.showTrackChangesPreview"); + await settle(); + ctl.setEditTurnForTest(async () => ({ replacement: "# P\n\nuno AAA.\n\ndos BBB.\n", model: "m", sessionId: "s" })); + const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "up"); + await settle(); await settle(); + assert.ok(doc.getText().includes("AAA") && doc.getText().includes("BBB")); + // reject ONE via the webview intent → that block reverts, the other stays applied + ctl.receiveMessage(key, { type: "reject", proposalId: ids[0] }); + await settle(); await settle(); + assert.ok(doc.getText().includes("uno aaa.") && doc.getText().includes("BBB"), "one reverted, one applied"); + // rejectAll via the command → all gone, document restored + ctl.receiveMessage(key, { type: "rejectAll" }); + await settle(); await settle(); + assert.strictEqual(doc.getText(), "# P\n\nuno aaa.\n\ndos bbb.\n", "document restored"); + assert.strictEqual(api.proposalController.listProposals(doc).length, 0); + }); + + test("cowriting.rejectAllProposals is registered + markdown-guarded", async () => { + const all = await vscode.commands.getCommands(true); + assert.ok(all.includes("cowriting.rejectAllProposals")); + const pkg = JSON.parse(fs.readFileSync(path.join(__dirname, "../../../../package.json"), "utf8")); + const entry = (pkg.contributes.menus.commandPalette as Array<{ command: string; when?: string }>).find( + (m) => m.command === "cowriting.rejectAllProposals", + ); + assert.ok(entry && /editorLangId == markdown/.test(entry.when ?? ""), "palette-guarded on markdown"); + }); +}); +``` + +- [ ] **Step 9: Build the webview + run the full pipeline** + +Run: `npm run build && npm run typecheck && npm test && npm run test:e2e` +Expected: all PASS (unit suite + the full host-E2E including the new `f12InlineDiff` suite, F10/F11/F12 existing suites green). + +- [ ] **Step 10: Commit** + +```bash +git add src/trackChangesModel.ts media/preview.ts media/preview.css src/trackChangesPreview.ts src/extension.ts package.json test/trackChangesModel.test.ts test/e2e/suite/f12InlineDiff.test.ts +git commit -m "feat(webview): Accept/Reject + dropdown, rejectAll, control parity (#64, INV-53)" +``` + +--- + +## Task 8: regression sweep + manual smoke + +- [ ] **Step 1: Full green gate** + +Run: `npm run typecheck && npm test && npm run test:e2e` +Expected: typecheck clean; full unit suite PASS; host-E2E PASS (no regressions in F4 proposals, F10 review, F11 toolbar, F12 accept/reach/review suites). + +- [ ] **Step 2: Manual smoke (per spec §4)** + +Launch the extension (F5 / `code --extensionDevelopmentPath`), open a Markdown file, Ask Claude to edit a paragraph, and confirm: +- the editor shows the change applied, inserted text tinted, a struck-red deletion hint, and `Accept ▾`/`Reject ▾` CodeLens above the block; +- editing the inserted text then Accept keeps the edit; +- Reject restores the original; +- Accept-all / Reject-all work from the CodeLens dropdown AND the webview; +- the webview shows the same diff as the editor for the same proposal; +- saving while pending writes the proposed text (no decoration markup in the file); +- after all proposals are resolved, the editor is clean again. + +- [ ] **Step 3: Update the spec invariant note (optional, low-risk)** + +Note the re-anchor refinement (Task intro) in the spec's §3.2 if updating the content-repo copy; otherwise it rides the finalize deferred-decisions report. + +--- + +## Self-review notes (planner) + +- **Spec coverage:** INV-48 (Task 4/5), INV-49 (Task 5a), INV-50 (Task 6), INV-51 (Task 4), INV-52 (Task 5), INV-53 (Task 7), INV-54 (save persists — inherent to optimistic apply; covered by the editor-surface E2E asserting the buffer holds the proposed text + a smoke check; an explicit save-and-read E2E can be added if desired). §2.2 edit-then-accept (Task 5 E2E). §2.5 rejected alternatives — N/A to code. +- **Risk (INV-50 reconciliation):** Task 6's coordinate mapping (`toLanded`) is the subtlest piece; its unit test pins the no-double-render guarantee. If a multi-proposal layout edge appears, extend the unit test before touching the mapper. +- **Concurrency:** the optimistic-apply trigger re-enters via the sidecar watcher; the `isApplied` guard (Task 4) makes it idempotent. Verify in the editor-surface E2E that N document proposals each apply exactly once. +- **Out of scope (YAGNI):** live token-streaming into the editor; non-Markdown; intra-diagram mermaid editing — all per spec §2.4. diff --git a/plans/2026-06-26-live-turn-progress.md b/plans/2026-06-26-live-turn-progress.md new file mode 100644 index 0000000..fb081ed --- /dev/null +++ b/plans/2026-06-26-live-turn-progress.md @@ -0,0 +1,910 @@ +# Live Turn Progress (#60) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Surface Claude's live output/progress (activity + token count + streamed text + cancel) during the otherwise-opaque "asking Claude…" status, for both edit entry points. + +**Architecture:** A pure reducer (`turnProgress.ts`) folds `@cline/sdk` Agent runtime events into a small `TurnProgressSnapshot`; `runEditTurn` subscribes to the agent and emits snapshots via an `onProgress` callback while accepting an `AbortSignal` for cancellation — staying `vscode`-free. The two call sites relay snapshots through a shared host `liveProgressUi` module: the existing `withProgress` notification shows a live activity line, a dedicated OutputChannel streams the full assistant text, and the notification becomes cancellable. The final result path (proposals) is untouched. + +**Tech Stack:** TypeScript (strict, `moduleResolution: Bundler`), VS Code extension API, `@cline/sdk` (ESM, dynamic-import-only, never bundled), Vitest (unit), `@vscode/test-electron` (host E2E), esbuild. + +**Spec:** `coauthoring-live-progress.md` (graduated, content repo). Invariants INV-43..47. + +--- + +## File Structure + +- **Create `src/turnProgress.ts`** — pure reducer + line formatter. No `vscode`, no SDK runtime import (`AgentRuntimeEvent` type-only). Owns ALL event→state logic (INV-46). +- **Create `test/turnProgress.test.ts`** — unit tests for the reducer + `formatProgressLine`. +- **Modify `src/liveTurn.ts`** — `runEditTurn` gains `RunEditTurnOptions { modelId?, onProgress?, signal? }`; subscribes to the agent, folds events through the reducer, wires `signal`→`agent.abort()`. Result shape unchanged (INV-44). Stays `vscode`-free (INV-43). +- **Create `src/liveProgressUi.ts`** — host module (vscode-only): owns the shared `"Cowriting: Claude"` OutputChannel and a `begin(instruction, progress, token)` helper returning `{ signal, onProgress }` for one `withProgress` run. Reads `cowriting.liveProgress.revealOutput`. +- **Modify `src/extension.ts`** — construct `liveProgressUi` once at activate; the `editSelection` command uses it (cancellable notification + onProgress + signal); pass it into the preview controller; expose it on `CowritingApi` for E2E. +- **Modify `src/trackChangesPreview.ts`** — widen the `EditTurn` seam to accept `opts?`; thread `opts` through `askClaude` → `runEditAndPropose` → `editTurn`; use the injected `liveProgressUi` in `askClaude`. +- **Modify `package.json`** — add `contributes.configuration` with `cowriting.liveProgress.revealOutput` (boolean, default `true`). +- **Modify `test/e2e/suite/f11Toolbar.test.ts`** (or a new `test/e2e/suite/liveProgress.test.ts`) — extend the preview `editTurn` stub to emit synthetic progress and assert (a) identical proposals (INV-44) and (b) cancel→zero proposals (INV-47). + +**Build note:** the type-only `import type { AgentRuntimeEvent } from "@cline/sdk"` is erased at compile and `@cline/*` stays external in esbuild — no bundling of the ESM SDK (preserves the POC constraint). + +--- + +## Task 1: Pure progress reducer (`turnProgress.ts`) + +**Files:** +- Create: `src/turnProgress.ts` +- Test: `test/turnProgress.test.ts` + +- [ ] **Step 1: Write the failing test** + +Create `test/turnProgress.test.ts`: + +```ts +import { describe, it, expect } from "vitest"; +import { + createTurnProgressState, + reduceTurnProgress, + formatProgressLine, + formatTokens, + type TurnProgressSnapshot, +} from "../src/turnProgress"; + +// Minimal event factories — structurally match the @cline/sdk AgentRuntimeEvent +// members the reducer reads. `as any` because we only supply the fields used. +const ev = (e: any) => e as any; +const textDelta = (text: string, accumulatedText: string) => + ev({ type: "assistant-text-delta", text, accumulatedText }); +const toolStarted = (toolName: string) => ev({ type: "tool-started", toolCall: { toolName } }); +const toolFinished = (toolName: string) => ev({ type: "tool-finished", toolCall: { toolName } }); +const usage = (inputTokens: number, outputTokens: number) => + ev({ type: "usage-updated", usage: { inputTokens, outputTokens, cacheReadTokens: 0, cacheWriteTokens: 0 } }); + +// Drive a sequence of events, returning every emitted snapshot. +function run(events: any[]): TurnProgressSnapshot[] { + let state = createTurnProgressState(); + const out: TurnProgressSnapshot[] = []; + for (const e of events) { + const r = reduceTurnProgress(state, e); + state = r.state; + if (r.snapshot) out.push(r.snapshot); + } + return out; +} + +describe("reduceTurnProgress", () => { + it("starts in thinking", () => { + const s = run([ev({ type: "run-started" })]); + expect(s.at(-1)!.phase).toBe("thinking"); + expect(s.at(-1)!.chars).toBe(0); + expect(s.at(-1)!.tokens).toBeUndefined(); + }); + + it("text deltas move to writing and accumulate chars + carry the delta", () => { + const s = run([textDelta("Hel", "Hel"), textDelta("lo", "Hello")]); + expect(s.map((x) => x.phase)).toEqual(["writing", "writing"]); + expect(s.at(-1)!.chars).toBe(5); + expect(s.map((x) => x.textDelta)).toEqual(["Hel", "lo"]); + }); + + it("usage sets a running token total (input+output)", () => { + const s = run([textDelta("Hi", "Hi"), usage(1000, 234)]); + expect(s.at(-1)!.tokens).toBe(1234); + }); + + it("tool start shows the tool name; tool finish reverts to writing once text was seen", () => { + const s = run([textDelta("x", "x"), toolStarted("read_file"), toolFinished("read_file")]); + expect(s[1].phase).toBe("tool"); + expect(s[1].tool).toBe("read_file"); + expect(s.at(-1)!.phase).toBe("writing"); + }); + + it("tool finish reverts to thinking when no text was seen", () => { + const s = run([toolStarted("grep"), toolFinished("grep")]); + expect(s.at(-1)!.phase).toBe("thinking"); + }); + + it("overlapping tools resolve in order", () => { + const s = run([toolStarted("a"), toolStarted("b"), toolFinished("b"), toolFinished("a")]); + expect(s.map((x) => x.phase)).toEqual(["tool", "tool", "tool", "thinking"]); + expect(s[1].tool).toBe("b"); + expect(s[2].tool).toBe("a"); + }); + + it("reasoning deltas stay thinking and surface no text", () => { + const s = run([ev({ type: "assistant-reasoning-delta", text: "secret", accumulatedText: "secret" })]); + expect(s.at(-1)!.phase).toBe("thinking"); + expect(s.at(-1)!.textDelta).toBeUndefined(); + }); + + it("ignores lifecycle/finish events (no snapshot)", () => { + const s = run([ev({ type: "turn-finished" }), ev({ type: "run-finished" })]); + expect(s).toEqual([]); + }); +}); + +describe("formatProgressLine / formatTokens", () => { + it("thinking", () => { + expect(formatProgressLine({ phase: "thinking", chars: 0 })).toBe("thinking…"); + }); + it("writing with chars", () => { + expect(formatProgressLine({ phase: "writing", chars: 412 })).toBe("writing… (412 chars)"); + }); + it("writing with chars + tokens", () => { + expect(formatProgressLine({ phase: "writing", chars: 412, tokens: 1234 })).toBe( + "writing… (412 chars) · 1.2k tokens", + ); + }); + it("tool with name", () => { + expect(formatProgressLine({ phase: "tool", tool: "read_file", chars: 0 })).toBe("running read_file…"); + }); + it("formats token magnitudes", () => { + expect(formatTokens(950)).toBe("950"); + expect(formatTokens(1234)).toBe("1.2k"); + }); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `npx vitest run test/turnProgress.test.ts` +Expected: FAIL — `Cannot find module "../src/turnProgress"`. + +- [ ] **Step 3: Write the reducer** + +Create `src/turnProgress.ts`: + +```ts +/** + * turnProgress.ts — pure reduction of @cline/sdk Agent runtime events into a + * small UI-facing progress snapshot (#60, spec coauthoring-live-progress.md §3.2). + * + * INV-43: vscode-free. INV-46: a pure function — no vscode, no SDK runtime + * dependency (`AgentRuntimeEvent` is imported TYPE-only, so it is erased at + * compile and never pulls the ESM SDK into the bundle). All event→state logic + * lives here so it is unit-tested in isolation; the UI call sites only format and + * relay snapshots. + */ +import type { AgentRuntimeEvent } from "@cline/sdk"; + +export type TurnPhase = "thinking" | "writing" | "tool"; + +export interface TurnProgressSnapshot { + phase: TurnPhase; + /** present iff phase === "tool" — the running tool's name. */ + tool?: string; + /** accumulated assistant-text length so far. */ + chars: number; + /** running total tokens (input+output); undefined until the first usage event. */ + tokens?: number; + /** the new assistant-text chunk since the last snapshot (for the OutputChannel). */ + textDelta?: string; +} + +export interface TurnProgressState { + phase: TurnPhase; + chars: number; + tokens?: number; + /** stack of tool names currently running (depth-tracked for overlap). */ + activeTools: string[]; + /** true once any assistant text has streamed (tool-finish then reverts to writing). */ + sawText: boolean; +} + +export function createTurnProgressState(): TurnProgressState { + return { phase: "thinking", chars: 0, tokens: undefined, activeTools: [], sawText: false }; +} + +function restingPhase(state: TurnProgressState): TurnPhase { + if (state.activeTools.length) return "tool"; + return state.sawText ? "writing" : "thinking"; +} + +function toSnapshot(state: TurnProgressState, textDelta?: string): TurnProgressSnapshot { + return { + phase: state.phase, + tool: state.phase === "tool" ? state.activeTools[state.activeTools.length - 1] : undefined, + chars: state.chars, + tokens: state.tokens, + textDelta, + }; +} + +/** + * Fold one SDK event into the state, returning the next state and the snapshot to + * emit (snapshot undefined for events that don't change the surface). + */ +export function reduceTurnProgress( + state: TurnProgressState, + event: AgentRuntimeEvent, +): { state: TurnProgressState; snapshot?: TurnProgressSnapshot } { + switch (event.type) { + case "run-started": + case "turn-started": { + const next: TurnProgressState = { ...state, phase: restingPhase(state) }; + return { state: next, snapshot: toSnapshot(next) }; + } + case "assistant-text-delta": { + const next: TurnProgressState = { + ...state, + phase: state.activeTools.length ? "tool" : "writing", + chars: event.accumulatedText.length, + sawText: true, + }; + return { state: next, snapshot: toSnapshot(next, event.text) }; + } + case "assistant-reasoning-delta": { + // Reasoning TEXT is not surfaced (operator fork); collapse to motion only. + const next: TurnProgressState = { ...state, phase: state.activeTools.length ? "tool" : "thinking" }; + return { state: next, snapshot: toSnapshot(next) }; + } + case "tool-started": { + const activeTools = [...state.activeTools, event.toolCall.toolName]; + const next: TurnProgressState = { ...state, phase: "tool", activeTools }; + return { state: next, snapshot: toSnapshot(next) }; + } + case "tool-updated": { + const next: TurnProgressState = { ...state, phase: "tool" }; + return { state: next, snapshot: toSnapshot(next) }; + } + case "tool-finished": { + const name = event.toolCall.toolName; + const idx = state.activeTools.lastIndexOf(name); + const activeTools = idx >= 0 ? state.activeTools.filter((_, i) => i !== idx) : state.activeTools.slice(0, -1); + const next: TurnProgressState = { ...state, activeTools, phase: "thinking" }; + next.phase = restingPhase(next); + return { state: next, snapshot: toSnapshot(next) }; + } + case "usage-updated": { + const tokens = event.usage.inputTokens + event.usage.outputTokens || undefined; + const next: TurnProgressState = { ...state, tokens }; + return { state: next, snapshot: toSnapshot(next) }; + } + default: + return { state }; + } +} + +/** Render the notification activity line from a snapshot (pure; spec §2.1). */ +export function formatProgressLine(s: TurnProgressSnapshot): string { + let head: string; + if (s.phase === "tool") head = `running ${s.tool ?? "tool"}…`; + else if (s.phase === "writing") head = `writing… (${s.chars} chars)`; + else head = "thinking…"; + return s.tokens ? `${head} · ${formatTokens(s.tokens)} tokens` : head; +} + +export function formatTokens(n: number): string { + return n >= 1000 ? `${(n / 1000).toFixed(1)}k` : String(n); +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `npx vitest run test/turnProgress.test.ts` +Expected: PASS (all cases). + +- [ ] **Step 5: Typecheck** + +Run: `npm run typecheck` +Expected: no errors (confirms the type-only `AgentRuntimeEvent` import resolves under `moduleResolution: Bundler` and the discriminated-union narrowing is sound). + +- [ ] **Step 6: Commit** + +```bash +git add src/turnProgress.ts test/turnProgress.test.ts +git commit -m "feat(#60): pure turn-progress reducer (INV-46)" +``` + +--- + +## Task 2: Extend `runEditTurn` with progress + cancellation + +**Files:** +- Modify: `src/liveTurn.ts` +- Test: `test/liveTurn.test.ts` (add a mocked-agent case) + +- [ ] **Step 1: Write the failing test** + +Append to `test/liveTurn.test.ts` (keep the existing `extractReplacement` block). This mocks `@cline/sdk` via Vitest so `runEditTurn` runs without a real LLM, exercising subscribe + abort wiring: + +```ts +import { vi } from "vitest"; +import { runEditTurn } from "../src/liveTurn"; + +// A fake Agent that replays a scripted event list to subscribers, supports abort, +// and resolves agent.run() with a completed result. +function fakeSdk(events: any[], opts?: { abortAware?: boolean }) { + return { + Agent: class { + private listeners: ((e: any) => void)[] = []; + private aborted = false; + constructor(_cfg: unknown) {} + subscribe(fn: (e: any) => void) { + this.listeners.push(fn); + return () => { + this.listeners = this.listeners.filter((l) => l !== fn); + }; + } + abort() { + this.aborted = true; + } + async run(_input: string) { + for (const e of events) for (const l of this.listeners) l(e); + if (opts?.abortAware && this.aborted) { + return { status: "aborted", outputText: "", runId: "r1", error: { message: "aborted" } }; + } + return { status: "completed", outputText: "EDITED", runId: "r1" }; + } + }, + }; +} + +describe("runEditTurn progress + cancel", () => { + it("emits progress snapshots and returns the replacement unchanged (INV-44)", async () => { + vi.doMock("@cline/sdk", () => fakeSdk([ + { type: "assistant-text-delta", text: "ED", accumulatedText: "ED" }, + { type: "usage-updated", usage: { inputTokens: 10, outputTokens: 5, cacheReadTokens: 0, cacheWriteTokens: 0 } }, + ])); + const seen: string[] = []; + const turn = await runEditTurn("do it", "old", { onProgress: (s) => seen.push(s.phase) }); + expect(turn.replacement).toBe("EDITED"); + expect(seen).toContain("writing"); + vi.doUnmock("@cline/sdk"); + }); + + it("a fired AbortSignal aborts the agent and the turn throws (INV-47)", async () => { + vi.doMock("@cline/sdk", () => fakeSdk([], { abortAware: true })); + const ac = new AbortController(); + ac.abort(); + await expect(runEditTurn("do it", "old", { signal: ac.signal })).rejects.toThrow(/aborted/); + vi.doUnmock("@cline/sdk"); + }); +}); +``` + +Note: dynamic `import("@cline/sdk")` inside `runEditTurn` picks up `vi.doMock`. If the existing test file lacks `vi` import, the added `import { vi }` covers it. + +- [ ] **Step 2: Run to verify it fails** + +Run: `npx vitest run test/liveTurn.test.ts` +Expected: FAIL — `runEditTurn` doesn't accept `onProgress`/`signal` yet (type error) or doesn't subscribe. + +- [ ] **Step 3: Implement the extension** + +In `src/liveTurn.ts`, add the import and options type, and rewrite the body of `runEditTurn` (keep `SYSTEM_PROMPT`, `extractReplacement`, `EditTurnResult` as-is): + +```ts +import type { TurnProgressSnapshot } from "./turnProgress"; +import { createTurnProgressState, reduceTurnProgress } from "./turnProgress"; + +export interface RunEditTurnOptions { + modelId?: string; + /** out: called with a reduced progress snapshot as the SDK streams (INV-44 additive). */ + onProgress?: (snapshot: TurnProgressSnapshot) => void; + /** in: aborting this signal cancels the turn via agent.abort() (INV-47). */ + signal?: AbortSignal; +} + +export async function runEditTurn( + instruction: string, + selectedText: string, + opts?: RunEditTurnOptions, +): Promise { + const sdk = await import("@cline/sdk"); + const modelId = opts?.modelId ?? "sonnet"; + const agent = new sdk.Agent({ providerId: "claude-code", modelId, systemPrompt: SYSTEM_PROMPT }); + + let state = createTurnProgressState(); + const unsubscribe = opts?.onProgress + ? agent.subscribe((event) => { + const next = reduceTurnProgress(state, event); + state = next.state; + if (next.snapshot) opts.onProgress!(next.snapshot); + }) + : undefined; + const onAbort = () => agent.abort(); + opts?.signal?.addEventListener("abort", onAbort); + + try { + const result = await agent.run( + `\n${instruction}\n\n\n${selectedText}\n`, + ); + if (result.status !== "completed") { + throw new Error( + `claude-code turn ${result.status}: ${result.error?.message ?? "unknown error"} ` + + "(is Claude Code installed and signed in?)", + ); + } + return { replacement: extractReplacement(result.outputText, selectedText), model: modelId, sessionId: result.runId }; + } finally { + unsubscribe?.(); + opts?.signal?.removeEventListener("abort", onAbort); + } +} +``` + +(`agent.subscribe` returns its unsubscribe fn per `@cline/agents` `AgentRuntime.subscribe(listener): () => void`. `agent.abort()` exists on the same class. `AbortSignal`/`addEventListener` are web standards — no `vscode` import is added, preserving INV-43.) + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `npx vitest run test/liveTurn.test.ts` +Expected: PASS (both new cases + the existing `extractReplacement` cases). + +- [ ] **Step 5: Typecheck + full unit run** + +Run: `npm run typecheck && npx vitest run` +Expected: typecheck clean; all unit tests pass (was 222 + Task 1 + these). + +- [ ] **Step 6: Commit** + +```bash +git add src/liveTurn.ts test/liveTurn.test.ts +git commit -m "feat(#60): runEditTurn streams progress + accepts AbortSignal (INV-43/44/47)" +``` + +--- + +## Task 3: Host `liveProgressUi` module + setting + +**Files:** +- Create: `src/liveProgressUi.ts` +- Modify: `package.json` (add `contributes.configuration`) + +- [ ] **Step 1: Add the configuration to `package.json`** + +In `package.json`, under the top-level `"contributes"` object, add a `configuration` block (there is no existing one). Insert it as a sibling of the existing `commands`/`menus` keys: + +```json +"configuration": { + "title": "Cowriting", + "properties": { + "cowriting.liveProgress.revealOutput": { + "type": "boolean", + "default": true, + "description": "When Claude is editing, reveal the \"Cowriting: Claude\" output channel (without stealing focus) as soon as Claude starts producing text, so you can read the output as it streams." + } + } +} +``` + +- [ ] **Step 2: Write the module** + +Create `src/liveProgressUi.ts`: + +```ts +/** + * liveProgressUi.ts — host-side relay of TurnProgress snapshots to VS Code UI + * (#60, spec coauthoring-live-progress.md §3.4). The ONLY surfaces are the + * existing withProgress notification (an activity line) and a dedicated + * OutputChannel streaming the full assistant text — no new network/webview + * surface (INV-45). vscode-only; the pure logic lives in turnProgress.ts. + */ +import * as vscode from "vscode"; +import { formatProgressLine, type TurnProgressSnapshot } from "./turnProgress"; + +const CHANNEL_NAME = "Cowriting: Claude"; + +export interface TurnUi { + /** Pass to runEditTurn's opts.onProgress. */ + onProgress: (snapshot: TurnProgressSnapshot) => void; + /** Pass to runEditTurn's opts.signal — fired when the user cancels the notification. */ + signal: AbortSignal; +} + +export class LiveProgressUi { + readonly channel: vscode.OutputChannel; + constructor() { + this.channel = vscode.window.createOutputChannel(CHANNEL_NAME); + } + + /** + * Begin one turn's UI. Writes the per-turn header to the channel, returns the + * onProgress relay + an AbortSignal linked to the notification's cancel token. + */ + begin( + instruction: string, + progress: vscode.Progress<{ message?: string }>, + token: vscode.CancellationToken, + ): TurnUi { + const controller = new AbortController(); + token.onCancellationRequested(() => controller.abort()); + this.channel.appendLine(`── asking: ${instruction} ──`); + let revealed = false; + const reveal = () => { + if (revealed) return; + revealed = true; + const cfg = vscode.workspace.getConfiguration("cowriting"); + if (cfg.get("liveProgress.revealOutput", true)) this.channel.show(true); + }; + const onProgress = (s: TurnProgressSnapshot) => { + progress.report({ message: formatProgressLine(s) }); + if (s.textDelta) { + this.channel.append(s.textDelta); + reveal(); + } + }; + return { onProgress, signal: controller.signal }; + } + + dispose(): void { + this.channel.dispose(); + } +} +``` + +- [ ] **Step 3: Typecheck** + +Run: `npm run typecheck` +Expected: no errors. + +- [ ] **Step 4: Validate package.json** + +Run: `node -e "JSON.parse(require('fs').readFileSync('package.json','utf8')); console.log('ok')"` +Expected: `ok` (the configuration block is valid JSON). + +- [ ] **Step 5: Commit** + +```bash +git add src/liveProgressUi.ts package.json +git commit -m "feat(#60): liveProgressUi host relay + revealOutput setting (INV-45)" +``` + +--- + +## Task 4: Wire the `editSelection` call site + +**Files:** +- Modify: `src/extension.ts` (construct LiveProgressUi at activate; use it in the `editSelection` command; add to `CowritingApi`) + +- [ ] **Step 1: Construct `LiveProgressUi` once and pass it into the preview controller** + +Near the top of `activate(...)`, after the existing POC `output` channel is created, add: + +```ts +const liveProgressUi = new LiveProgressUi(); +context.subscriptions.push(liveProgressUi); +``` + +Add the import at the top of `src/extension.ts`: + +```ts +import { LiveProgressUi } from "./liveProgressUi"; +``` + +Then thread it into the preview controller construction (Task 5 widens that constructor): + +```ts +const trackChangesPreviewController = new TrackChangesPreviewController( + diffViewController, + context.extensionUri, + attributionController, + proposalController, + liveProgressUi, +); +``` + +- [ ] **Step 2: Rewrite the `editSelection` `withProgress` block** + +Replace the `editSelection` command's `withProgress(...)` call (currently `src/extension.ts:232-268`) so it is cancellable and relays progress. The turn body is otherwise unchanged: + +```ts +try { + await vscode.window.withProgress( + { + location: vscode.ProgressLocation.Notification, + title: "Cowriting: asking Claude…", + cancellable: true, + }, + async (progress, token) => { + const { runEditTurn } = await import("./liveTurn"); + const ui = liveProgressUi.begin(instruction, progress, token); + const turn = await runEditTurn(instruction, selectedText, { + onProgress: ui.onProgress, + signal: ui.signal, + }); + if (turn.replacement === "") { + void vscode.window.showWarningMessage( + "Cowriting: Claude returned an empty replacement — nothing was proposed.", + ); + return; + } + if (turn.replacement === selectedText) { + void vscode.window.showInformationMessage("Cowriting: Claude proposed no change to the selection."); + return; + } + const id = await proposalController.propose( + document, + fp, + turn.replacement, + { kind: "agent", id: "claude", agent: { sdk: "@cline/sdk", model: turn.model, sessionId: turn.sessionId } }, + { turnId, instruction }, + ); + if (id) { + void vscode.window.showInformationMessage( + "Cowriting: Claude proposed an edit — review the diff at the highlighted range (✓ accept / ✗ reject).", + ); + } + }, + ); +} catch (err) { + if (token_was_cancelled(err)) return; // replaced below + const message = err instanceof Error ? err.message : String(err); + void vscode.window.showErrorMessage(`Cowriting: Claude edit failed — ${message}`); +} +``` + +Because `token` is scoped inside the `withProgress` callback, surface cancellation by catching it there instead of the outer `catch`. Final form of the `catch` and cancel handling: wrap the `runEditTurn` call in a try/catch **inside** the callback that reflects cancel, and let other errors propagate to the outer `catch`: + +```ts +// inside the withProgress async (progress, token) callback, around runEditTurn: +let turn; +try { + turn = await runEditTurn(instruction, selectedText, { onProgress: ui.onProgress, signal: ui.signal }); +} catch (err) { + if (token.isCancellationRequested) { + void vscode.window.showInformationMessage("Cowriting: Claude edit cancelled."); + return; + } + throw err; +} +``` + +(Keep the single outer `try/catch` for the non-cancel error path exactly as it is today.) + +- [ ] **Step 3: Add `liveProgressUi` to `CowritingApi` and the return** + +In the `CowritingApi` interface (`src/extension.ts:18`) add: + +```ts +liveProgressUi: LiveProgressUi; +``` + +And in the `return { ... }` block (`src/extension.ts:287`) add `liveProgressUi,`. + +- [ ] **Step 4: Typecheck** + +Run: `npm run typecheck` +Expected: no errors (Task 5 must land for the widened constructor to typecheck — if doing tasks strictly in order, expect one error here about the 5th constructor arg; that resolves in Task 5. To keep this task self-contained, do Task 5 Step 1 (constructor signature) before re-running. Otherwise proceed to Task 5 and typecheck there.) + +- [ ] **Step 5: Build** + +Run: `npm run build` +Expected: esbuild succeeds; `@cline/*` stays external. + +- [ ] **Step 6: Commit** + +```bash +git add src/extension.ts +git commit -m "feat(#60): editSelection shows live progress + cancel" +``` + +--- + +## Task 5: Thread progress through the preview path + +**Files:** +- Modify: `src/trackChangesPreview.ts` (widen `EditTurn`; accept `LiveProgressUi`; thread opts through `askClaude` → `runEditAndPropose`) + +- [ ] **Step 1: Widen the `EditTurn` seam type and the constructor** + +In `src/trackChangesPreview.ts`, change the `EditTurn` type (line 22) to accept optional turn options (back-compat: existing E2E stubs ignore the new arg): + +```ts +import type { EditTurnResult, RunEditTurnOptions } from "./liveTurn"; +import type { LiveProgressUi } from "./liveProgressUi"; + +type EditTurn = (instruction: string, text: string, opts?: RunEditTurnOptions) => Promise; +``` + +Update the default `editTurn` (line 56) to forward opts: + +```ts +private editTurn: EditTurn = async (instruction, text, opts) => { + const { runEditTurn } = await import("./liveTurn"); + return runEditTurn(instruction, text, opts); +}; +``` + +Add `liveProgressUi` as a constructor parameter (after `proposals`): + +```ts +constructor( + private readonly diffView: DiffViewController, + private readonly extensionUri: vscode.Uri, + private readonly attribution: AttributionController, + private readonly proposals: ProposalController, + private readonly liveProgressUi: LiveProgressUi, +) { +``` + +- [ ] **Step 2: Make `askClaude` cancellable + progress-relaying** + +Rewrite the `withProgress` block in `askClaude` (`src/trackChangesPreview.ts:232-235`) to mirror Task 4: + +```ts +try { + const ids = await vscode.window.withProgress( + { + location: vscode.ProgressLocation.Notification, + title: "Cowriting: asking Claude…", + cancellable: true, + }, + async (progress, token) => { + const ui = this.liveProgressUi.begin(instruction, progress, token); + try { + return await this.runEditAndPropose(document, target, instruction, { onProgress: ui.onProgress, signal: ui.signal }); + } catch (err) { + if (token.isCancellationRequested) return [] as string[]; + throw err; + } + }, + ); + if (ids.length === 0) { + void vscode.window.showInformationMessage("Cowriting: Claude proposed no changes."); + } else { + void vscode.window.showInformationMessage( + `Cowriting: Claude proposed ${ids.length} edit${ids.length === 1 ? "" : "s"} — review ✓/✗ in the preview.`, + ); + } +} catch (err) { + const message = err instanceof Error ? err.message : String(err); + void vscode.window.showErrorMessage(`Cowriting: Claude edit failed — ${message}`); +} +``` + +(On cancel the inner catch returns `[]`, which shows the benign "proposed no changes" — no proposal is created, satisfying INV-47. A distinct "cancelled" toast is optional; the editSelection path shows one. Keep parity simple: returning `[]` is the no-proposal outcome.) + +- [ ] **Step 3: Thread `opts` through `runEditAndPropose`** + +Change the signature (`src/trackChangesPreview.ts:257`) and both `editTurn(...)` calls (lines 270, 276) to pass opts: + +```ts +async runEditAndPropose( + document: vscode.TextDocument, + target: EditTarget, + instruction: string, + opts?: RunEditTurnOptions, +): Promise { + // ... + // range branch: + const turn = await this.editTurn(instruction, selected, opts); + // ... + // document branch: + const turn = await this.editTurn(instruction, full, opts); +``` + +- [ ] **Step 4: Typecheck + full unit run** + +Run: `npm run typecheck && npx vitest run` +Expected: typecheck clean (the widened constructor now matches Task 4's call site); all unit tests pass. + +- [ ] **Step 5: Build** + +Run: `npm run build` +Expected: success. + +- [ ] **Step 6: Commit** + +```bash +git add src/trackChangesPreview.ts +git commit -m "feat(#60): preview Ask-Claude streams live progress + cancel" +``` + +--- + +## Task 6: Host E2E — additive proof + cancel + +**Files:** +- Create: `test/e2e/suite/liveProgress.test.ts` + +The E2E harness can't read notification subtitles, so it asserts the *contract*: progress is additive (proposals unchanged with progress events) and cancel yields zero proposals. The streamed-UI itself is covered by Task 1 units + the manual smoke. + +- [ ] **Step 1: Write the test** + +Create `test/e2e/suite/liveProgress.test.ts`, modeled on `f11Toolbar.test.ts` (same `getExtension`→`activate()`→`api.trackChangesPreviewController` pattern, same `setEditTurnForTest`): + +```ts +import * as assert from "assert"; +import * as vscode from "vscode"; +import type { CowritingApi } from "../../../src/extension"; + +suite("#60 live progress (additive + cancel)", () => { + async function setup(): Promise<{ api: CowritingApi; doc: vscode.TextDocument }> { + const ext = vscode.extensions.getExtension("benstull.vscode-cowriting-plugin")!; + const api = (await ext.activate()) as CowritingApi; + const doc = await vscode.workspace.openTextDocument({ language: "markdown", content: "# Title\n\nOld paragraph.\n" }); + await vscode.window.showTextDocument(doc); + await vscode.commands.executeCommand("cowriting.showTrackChangesPreview"); + return { api, doc }; + } + + test("a stub that emits progress still produces the same proposals (INV-44)", async () => { + const { api, doc } = await setup(); + const ctl = api.trackChangesPreviewController; + // The stub honors opts.onProgress (emitting synthetic snapshots) but returns + // the same rewrite — proposals must be unaffected by progress. + ctl.setEditTurnForTest(async (_i, _text, opts) => { + opts?.onProgress?.({ phase: "writing", chars: 5 }); + opts?.onProgress?.({ phase: "writing", chars: 13, tokens: 1234, textDelta: "New paragraph." }); + return { replacement: "# Title\n\nNew paragraph.\n", model: "sonnet", sessionId: "e2e-live" }; + }); + const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "rewrite"); + assert.ok(ids.length >= 1, "expected at least one proposal regardless of progress events"); + }); + + test("an aborted turn proposes nothing (INV-47)", async () => { + const { api, doc } = await setup(); + const ctl = api.trackChangesPreviewController; + // Simulate the SDK honoring the abort signal: throw as runEditTurn would on + // a non-"completed" (aborted) status. + ctl.setEditTurnForTest(async (_i, _text, opts) => { + if (opts?.signal?.aborted) throw new Error("claude-code turn aborted"); + throw new Error("claude-code turn aborted"); // pre-aborted in this test + }); + const ac = new AbortController(); + ac.abort(); + let ids: string[] = []; + try { + ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "rewrite", { signal: ac.signal }); + } catch { + ids = []; + } + assert.strictEqual(ids.length, 0, "aborted turn must create no proposals"); + }); +}); +``` + +- [ ] **Step 2: Compile + run the E2E suite** + +Run: `npm run pretest:e2e && npm run test:e2e` +Expected: the new suite passes alongside the existing E2E suites. (If the sandboxed `.vscode-test` has the known `executeCommand("undo")` breakage, the #38 undo suite may skip/fail independently — that is pre-existing per session 0048/0049 and unrelated to #60; confirm only the live-progress suite is green and the rest are unchanged from baseline.) + +- [ ] **Step 3: Commit** + +```bash +git add test/e2e/suite/liveProgress.test.ts +git commit -m "test(#60): host E2E — progress is additive; abort proposes nothing (INV-44/47)" +``` + +--- + +## Task 7: Manual smoke + final verification + +**Files:** none (verification only) + +- [ ] **Step 1: Full unit + typecheck + build** + +Run: `npm run typecheck && npx vitest run && npm run build` +Expected: typecheck clean; all unit tests pass; build succeeds. + +- [ ] **Step 2: Manual smoke (real LLM)** + +Per #60 acceptance (E2E can't assert streamed UI). With Claude Code signed in, run the extension (F5 / Extension Development Host), open a markdown doc, and Ask-Claude a non-trivial edit. Confirm: +- the notification line advances (`thinking…` → `writing… (N chars)` → token count rising; `running …` if a tool fires); +- the "Cowriting: Claude" OutputChannel reveals (without stealing focus) and streams the text; +- the `✕` cancel aborts cleanly (status resolves, no proposal); +- toggling `cowriting.liveProgress.revealOutput` off suppresses the auto-reveal; +- the final proposals are exactly as before (INV-44). + +`scripts/smoke-live-turn.mjs` (`npm run smoke:live`) exercises the turn headlessly if a UI smoke isn't convenient; note it does not render the notification. + +- [ ] **Step 3: Record the smoke result in the session transcript** (`## Deferred decisions` or a smoke note), since E2E doesn't cover the live UI. + +--- + +## Self-Review (completed by plan author) + +**Spec coverage:** +- §2.1 notification activity line + token count → Task 1 (`formatProgressLine`), Task 4/5 (`progress.report`). ✓ +- §2.1 OutputChannel full-text stream + auto-reveal gate → Task 3 (`LiveProgressUi`, setting). ✓ +- §2.1 cancellation → "cancelled", no proposal → Task 4 (editSelection), Task 5 (preview), Task 6 (E2E). ✓ +- §2.2 reasoning not surfaced → Task 1 (reasoning-delta → thinking, no textDelta). ✓ +- §3.2 pure reducer (INV-46) → Task 1. ✓ +- §3.3 runEditTurn onProgress + AbortSignal, vscode-free (INV-43/44) → Task 2. ✓ +- §3.4 both call sites relay; widened EditTurn seam; stub stays valid → Task 4, Task 5, Task 6. ✓ +- §3.5 OutputChannel append + header → Task 3. ✓ +- §3.6 setting → Task 3. ✓ +- §3.7 INV-43..47 → asserted across Tasks 1/2/6. ✓ +- §4 unit (reducer + format) + host E2E (additive + cancel) + manual smoke → Tasks 1, 6, 7. ✓ + +**Placeholder scan:** none (every code/command step is concrete). The Task 4 Step 2 `token_was_cancelled(err)` pseudo-line is immediately replaced by the concrete inner-try/catch in the same step. + +**Type consistency:** `TurnProgressSnapshot`, `RunEditTurnOptions`, `TurnUi`, `LiveProgressUi`, `reduceTurnProgress`, `formatProgressLine`, `createTurnProgressState` used consistently across tasks; `editTurn` widened uniformly; `toolName`/`inputTokens+outputTokens` match the SDK `agent.d.ts`. diff --git a/specs/2026-06-12-f11-preview-toolbar-interaction-surface.md b/specs/2026-06-12-f11-preview-toolbar-interaction-surface.md new file mode 100644 index 0000000..d1a065c --- /dev/null +++ b/specs/2026-06-12-f11-preview-toolbar-interaction-surface.md @@ -0,0 +1,602 @@ +--- +status: graduated +--- +# Solution Design: Preview Toolbar as the Primary Interaction Surface (F11) + +| | | +| --- | --- | +| **Author(s)** | Ben Stull (with Claude) | +| **Reviewers / approvers** | Ben Stull | +| **Status** | `draft` | +| **Version** | v0.1.0 | +| **Source artifacts** | Feature `benstull/vscode-cowriting-plugin#43` (F11, `type/feature`, `priority/P1`) · Epic `#1` (closed) · Capture session `vscode-cowriting-plugin-0035` (2026-06-12) · Brainstorming session `vscode-cowriting-plugin-0036` · Builds on (all shipped): F3 `#6` (live attribution), F4 `#12` (propose/accept), F6 `#17`/`#19` (baseline + diff view), F7 `#21`/`#22` (rendered preview), F9 `#27` (authorship preview), F10 `#29` (interactive review preview), F10-followups `#31` (inline-at-anchor proposals) · Coexists with `#41` (right-click → Open Review Panel) and `#42` (right-click → Ask Claude to Edit), both blocked-by this · Parent specs (graduated): `coauthoring-inner-loop.md`, `coauthoring-attribution.md`, `coauthoring-propose-accept.md`, `coauthoring-diff-view.md`, `coauthoring-rendered-preview.md`, `coauthoring-interactive-review.md` · Lineage: `ben.stull/rfc-app#48` | + +**Change log** + +| Date | Version | Change | By | +| --- | --- | --- | --- | +| 2026-06-12 | v0.1.0 | Initial draft — brainstorming session 0036 (from the capture in session 0035). Three forks locked with the operator: block-level preview-selection→source mapping; document edit diffed into per-hunk F4 proposals; #43 lands a minimal right-click→open-preview gateway. | Ben Stull + Claude | + +--- + +## 1. Business Context + +### 1.1 Executive Summary + +F10 made the rendered preview the **single review surface** — clean editor on the +left, annotated review on the right, with an annotations on/off toggle and +inline ✓/✗ on Claude's pending proposals. But the writer still cannot *act* from +the preview beyond accepting/rejecting proposals: to **ask Claude to edit**, they +jump back to the editor and use a selection-gated context-menu item; to **pin a +fresh review baseline** they have *no* reachable control at all (the +`cowriting.pinDiffBaseline` command is registered but `when:false`, orphaned +since `#34` removed its two-pane host); and whole-document editing doesn't exist. + +F11 makes the **preview toolbar the primary interaction surface**. Beside the +existing annotations checkbox — the one control the writer already loves — the +toolbar gains a **Pin baseline** button and a **single adaptive "Ask Claude…" +button** that reads *Edit Selection* when text is selected in the preview and +*Edit Document* when nothing is. The writer reads, asks Claude to edit, and +resets the baseline all in one place, mouse-first, without leaving the rendered +document. A minimal right-click entry opens the preview, making it the surface +the `#41`/`#42` gateways will lead into. + +### 1.2 Background + +The inner loop shipped F2–F5 (threads · attribution · propose/accept · +cross-rung). F6 added the baseline + a native diff toggle; F7 the rendered +track-changes preview; F9 an authorship mode; F10 (`#29`) collapsed those into the +**single interactive review preview** (clean editor; annotations on/off; ✓/✗ on +F4 proposals surfaced in the rendered view); `#31` then placed proposals +**inline at their resolved anchor** in that preview. + +Capture session 0035 filed `#43` (this feature) plus `#41` (right-click → Open +Review Panel) and `#42` (right-click → Ask Claude to Edit). The operator's ask: +*"Can we set it up so all interactions — Ask Claude to Edit Selection / Edit +Document, annotations off/on, Pin new baseline — are via the preview window? I +like the annotations checkbox up there; make the others buttons, with one 'Ask +Claude…' button that changes depending on whether some of the markdown preview is +selected."* That session also surfaced the stranded `pinDiffBaseline` command +(`when:false` since `#34`). This spec is the Solution Design for `#43`. + +### 1.3 Business Actors / Roles + +- **Coauthor (human)** — the markdown writer/engineer (PP-1); F11's sole user. +- **Coauthor (machine)** — Claude via `@cline/sdk`; not a user of F11, but the + target of the toolbar's "Ask Claude…" gesture and the author of the proposals + that result. + +### 1.4 Problem Statement + +The plugin's interactions are scattered across surfaces and inconsistently +reachable. "Ask Claude to Edit Selection" is only a selection-gated **editor** +context-menu item; **whole-document editing doesn't exist**; and **Pin Review +Baseline** is **unreachable from any UI**. The one control the writer loves — the +annotations on/off checkbox in the preview — proves the toolbar is the natural +home for these gestures, but it stands alone. A writer reviewing in the preview +has to leave it and hunt through editor menus / the palette to act. + +### 1.5 Pain Points + +- **No edit gesture in the preview** — to ask Claude to change anything, the + writer leaves the review surface for the editor's right-click menu. +- **Whole-document editing is missing** — there is no "edit the whole document" + path at all; only a selection-scoped editor command exists. +- **Pin baseline is stranded** — the command exists but no menu, keybinding, or + palette entry reaches it (`when:false` since `#34`). +- **Mouse-first review is broken mid-flow** — the preview is mouse-driven, but + acting forces a context-switch to keyboard/menus elsewhere. + +### 1.6 Targeted Business Outcomes + +The preview becomes a **self-contained cockpit** for the inner loop. From its +toolbar a writer can toggle annotations (today), **ask Claude to edit** (selection +or whole document, via one button that adapts to what's selected), and **pin a +fresh baseline** — no context-switching to the editor or command palette. The +interaction model consolidates around the surface the writer already prefers, and +the stranded pin command gets a real home. + +### 1.7 Scope (business) + +**In scope:** preview-webview toolbar controls — a **Pin baseline** button and a +**single adaptive "Ask Claude…" button** (Edit Selection ⇆ Edit Document) — beside +the existing annotations checkbox; wiring those controls to the **existing** F4 +edit seam, F3 attribution, and F6 baseline command; **block-level** +preview-selection → source-range mapping (the central design risk); a **new +whole-document edit path** whose result is **diffed into per-hunk F4 proposals**; +a **minimal right-click → Open Review Preview** entry so the surface is reachable +end-to-end; resolving the pin-baseline reachability gap; unit + host-E2E coverage; +manual webview smoke. + +**Out of scope (deferred, not forgotten):** **char-precise sub-block** selection +mapping (block granularity is the locked v1 — §6.7); the **richer `#41`/`#42` +menu sets** (this feature lands only the minimal gateway; `#41`/`#42` expand it); +preview→source **scroll-sync** (`#32`); multi-file / batch editing; the Explorer +tree affordance; any export / print / copy gesture. + +**Non-goals (firm):** **no new edit / attribution / proposal *model*** — F11 +reuses F3 `spansFor`, the F4 `propose`/`accept` single-range model, and the F6 +baseline store; no change to the **sidecar**, the **cross-rung contract**, or +`SCHEMA_VERSION`; **no document mutation from the webview** (INV-20/21/34 hold — +the sealed webview posts intent only); no LLM/network/credential surface added to +the webview (INV-8 untouched — the edit turn runs host-side as today). + +### 1.8 Assumptions · Constraints · Dependencies + +- **Anchor:** Feature `#43` (F11). Builds directly on shipped work: the F7/F10 + rendered preview + annotations toggle + host↔webview message bus, the F3 + attribution + F4 propose/accept inner loop (including `#31`'s inline-at-anchor + proposal placement), and the F6 baseline store (`cowriting.pinDiffBaseline`, + currently unreachable — this feature gives it a home). +- **Central design risk (locked):** the preview is a **rendered** sealed webview + (markdown-it HTML, strict CSP — F7 INV-21), so "Edit Selection" must map a + selection in the rendered preview back to a **source markdown range**. The + rendered HTML carries **no** source positions today; only internal block + char-offsets exist (`splitBlocksWithRanges` → `BlockWithRange.start/end`). The + locked approach is **block-level** mapping: the pure render layer emits + `data-src-start`/`data-src-end` on each rendered block; a selection resolves to + the union of the live-source blocks it intersects (§6.7, fork 1). +- **Constraint (sealed webview):** interactive controls post messages to the + extension host, which applies edits/pins via the existing F4 / F6 paths; the + webview **never** edits the document, sidecar, or baseline directly. The edit + **turn** (LLM call) and the **instruction prompt** run host-side, keeping the + webview free of LLM/network/credential surface. +- **Coexistence:** the native editor context-menu "Ask Claude to Edit Selection" + (`cowriting.editSelection`) stays unchanged; `#41`/`#42` add right-click + gateways *into* the preview. F11 lands only a minimal gateway (§6.7, fork 3). +- No new persisted artifact; nothing in `.threads/`, the contract, or + `SCHEMA_VERSION` changes. + +### 1.9 Business Use Cases + +- **BUC-1 (edit from the preview)** Reviewing in the preview, the writer selects a + paragraph in the rendered document and clicks **"Ask Claude to Edit + Selection"**; Claude's proposed change appears as a blue ✓/✗ block at that + spot — without the writer ever leaving the preview. +- **BUC-2 (edit the whole document)** With nothing selected, the writer clicks + **"Ask Claude to Edit Document"**, types an instruction, and Claude's rewrite + surfaces as **several** independently-acceptable blue proposal blocks (one per + changed hunk) inline in the preview. +- **BUC-3 (pin a fresh baseline)** After accepting a batch of changes, the writer + clicks **"Pin baseline"**; the change-marks clear and "what changed" now counts + from this moment — the stranded command finally has a button. + +--- + +## 2. Solution Proposal + +F11 is a **thin increment** on F10's preview: it adds two controls to the +existing header toolbar and routes their intent through machinery that already +exists. No new model, no new persisted state. + +**The pure render layer (`trackChangesModel.ts`) learns one new thing:** a shared +helper wraps each rendered block in `
` +using the existing `splitBlocksWithRanges` offsets. It is applied to **both** +render paths — `renderReview` (annotations on) and `renderPlain` (annotations +off) — so selection→source mapping works in either mode. The helper is pure, +vscode-free, DOM-free, and deterministic (extends INV-22/33). The render layer +gains **no** selection or DOM logic. + +**The webview (`media/preview.ts` + `.css`)** header becomes: + +``` +[ ☑ Annotations ] [ ⌖ Pin baseline ] [ ✦ Ask Claude to Edit Document ▾ ] +``` + +The **Ask-Claude button morphs its own label** on `selectionchange`: a non-empty +selection inside the rendered body → **"Ask Claude to Edit Selection"**; an empty +/ collapsed selection → **"Ask Claude to Edit Document"**. On click it walks the +selection's start and end nodes up to the nearest ancestor carrying +`data-src-start`/`data-src-end` and posts the resolved offsets. That +nearest-ancestor lookup is the webview's **only** mapping duty (manual-smoke +territory); everything downstream is host-side and testable. The webview stays +**sealed** (INV-21): nonce'd inline script, no network, no document mutation. + +**New webview→host messages (intent only):** + +```ts +type ToolbarMsg = + | { type: "pinBaseline" } + | { type: "askClaude"; scope: "selection"; start: number; end: number } + | { type: "askClaude"; scope: "document" }; +``` + +**The host (`trackChangesPreview.ts`)** routes each intent through the existing +seams: + +- **`pinBaseline`** → pins the **previewed** document (calls + `DiffViewController.pin(document)` directly — not the `activeTextEditor`-based + command, which may not point at the previewed doc) → `onDidChangeBaseline` → + re-render with cleared marks. +- **`askClaude`** → host `showInputBox` for the instruction (keeps the LLM / + secret surface out of the sealed webview), then one shared host routine + `runEditAndPropose(document, target, instruction)`: + - **selection** → `target` = the block-union range `[firstBlock.start … + lastBlock.end]`; one `runEditTurn` → one F4 `propose()` over that range (the + existing Edit-Selection shape). + - **document** → `target` = whole document; one `runEditTurn` over the full + text → **diff Claude's result against the current text → one `propose()` per + changed hunk** (multiple proposals, each its own blue ✓/✗ block). Reuses the + F4 single-range model N times; no model change. + +**The right-click gateway:** `cowriting.showTrackChangesPreview` is added to the +`editor/title` menu (markdown only) so right-clicking the tab opens the preview — +the minimal entry that makes the toolbar surface reachable end-to-end and lets +`#41`/`#42` expand the menu set later. + +**Reachability cleanup:** `cowriting.pinDiffBaseline` gets a real palette `when` +(`editorLangId == markdown`), resolving the orphan from the command side too; a +new `cowriting.editDocument` command is registered (document-scoped edit) so +`#42`'s gateway can reuse it. + +Everything downstream of *(intent) → (existing seam)* is the existing F4/F6/F3 +machinery; the only genuinely new pure code is the block-offset wrapper and the +document-rewrite hunk-diff. Both are unit-testable with no vscode and no webview. + +--- + +## 3. Product Personas + +- **PP-1 Inner-loop coauthor** — the human markdown writer/engineer (as F2–F10); + the only persona F11 serves. + +## 4. Product Use Cases + +- **PUC-1 (toolbar present)** Opening the review preview for a markdown document + shows the header with **three** controls: the annotations on/off checkbox + (existing), a **Pin baseline** button, and an adaptive **Ask Claude…** button. + Controls are inert (disabled) for non-authorable documents. +- **PUC-2 (adaptive label)** With a non-empty selection in the rendered preview + body, the Ask-Claude button reads **"Ask Claude to Edit Selection"**; with no + selection it reads **"Ask Claude to Edit Document"**. The label flips live as + the selection changes. +- **PUC-3 (edit selection)** The writer selects rendered text, clicks **Ask + Claude to Edit Selection**, and enters an instruction. The selection resolves to + the union of the source blocks it touches; Claude proposes a change over that + range; a single blue ✓/✗ proposal block appears inline at that anchor (`#31`). +- **PUC-4 (edit document)** With nothing selected, the writer clicks **Ask Claude + to Edit Document**, enters an instruction; Claude rewrites the whole document; + the rewrite is diffed into hunks and surfaces as **N** independent blue ✓/✗ + proposal blocks inline. Accepting/rejecting each is the F10 path unchanged. +- **PUC-5 (pin baseline)** The writer clicks **Pin baseline**; the previewed + document's review baseline is pinned to now; the change-marks clear and the + `Since ` label updates. (No confirmation prompt — matches the existing + command's behavior; re-pinning is the recovery.) +- **PUC-6 (right-click into the preview)** Right-clicking a markdown editor tab + shows **Open Review Preview**; choosing it opens the preview (the gateway + `#41`/`#42` will build upon). +- **PUC-7 (graceful edges)** A selection confined to a deletion (struck) or + proposal block — which carries no live-source range — falls back to **document** + scope. An empty document or a selection that resolves to no live block → the + button stays in **Edit Document** mode. A non-authorable document → toolbar edit + controls are disabled. The LLM turn failing → the existing `runEditTurn` + error handling (no proposal created); the preview is unchanged. + +--- + +## 5. UX Layout + +The F10 preview is unchanged except for its **header bar**, which now hosts three +controls in a single row: + +- **☑ Annotations** — the existing on/off checkbox (kept first; the operator's + preferred control). +- **⌖ Pin baseline** — a button; pins the previewed document's review baseline to + now and clears the change-marks. +- **✦ Ask Claude to Edit Document ▾** — a single button whose label and behavior + adapt to the preview's selection state (Edit **Selection** when text is + selected, Edit **Document** otherwise). Clicking it opens a host input box for + the instruction. + +Buttons are styled with theme CSS variables (light / dark / high-contrast), +matching the existing toolbar chrome; they sit in the same `#cw-toggle` header +region as the annotations checkbox. When the previewed document is not authorable +(F8 `isAuthorable`), the **Pin baseline** and **Ask Claude…** controls render +**disabled** (the annotations toggle stays active — reading is always allowed). + +The rendered body is unchanged from F10/`#31`: green human additions, blue +LLM-authored text, struck deletions, and pending Claude proposals as inline blue +blocks with ✓/✗ at their resolved anchors. Proposals produced via the new toolbar +edit gestures appear exactly as proposals do today. + +--- + +## 6. Technical Design + +### 6.1 Invariants + +Parent invariants INV-1..INV-34 carry over unchanged. F11 adds: + +- **INV-35 (toolbar gestures route through existing seams; webview never + mutates)** The Pin baseline and Ask-Claude toolbar controls post **intent** + messages to the host; **all** mutation goes through the existing machinery — pin + via the F6 baseline store (`DiffViewController.pin`), edits via the F4 + `propose` → `accept`/`applyAgentEdit` (`WorkspaceEdit`) seam with F3 attribution. + No divergent edit or baseline path is introduced. The sealed webview never + edits the document, sidecar, or baseline directly (INV-20/21/34 hold); the LLM + turn and the instruction prompt run host-side (INV-8 untouched). +- **INV-36 (block-granular preview-selection → source mapping)** The pure render + layer emits `data-src-start`/`data-src-end` (source char offsets from + `BlockWithRange`) on **every** rendered block, in **both** the on (`renderReview`) + and off (`renderPlain`) modes. A preview selection resolves to the **union of + the live-source blocks it intersects** (`[min start … max end]`); blocks with no + live-source range (deletion-only / proposal blocks) are skipped, and a selection + that resolves to no live block falls back to **document** scope. The DOM + selection → nearest-`data-src` lookup is the webview's **sole** mapping duty; + the offsets and everything downstream (fingerprint, turn, propose) are host-side + and testable. The wrapping is deterministic — same inputs → identical HTML + (extends INV-22/33). +- **INV-37 (single adaptive Ask-Claude button; scope-aware)** One toolbar button + serves both scopes. A non-empty live-source selection → **Edit Selection**: one + F4 proposal over the block-union range. An empty selection → **Edit Document**: + one `runEditTurn` over the whole document, its result **diffed into hunks**, one + F4 `propose()` per changed hunk. Both scopes call the same host + `runEditAndPropose` routine and reuse the F4 **single-range** proposal model + (the document case issues multiple single-range proposals — **no new model**). + +### 6.2 High-level architecture + +```mermaid +flowchart LR + wv["webview header\n☑ Annotations · ⌖ Pin · ✦ Ask Claude (adaptive)"] -- "postMessage{pinBaseline | askClaude(scope,start?,end?)}" --> ctl["trackChangesPreview\n(vscode layer)"] + ctl -- "pin(document)" --> base["F6 DiffViewController\nbaseline store (INV-18)"] + ctl -- "showInputBox → runEditAndPropose" --> turn["runEditTurn\n(host-side LLM turn)"] + turn -- "selection: 1 replacement\ndocument: rewrite" --> ctl + ctl -- "selection → 1 propose()\ndocument → diff → N propose()" --> prop["F4 ProposalController\npropose() (single-range model)"] + prop -- "onDidChangeProposals" --> ctl + base -- "onDidChangeBaseline" --> ctl + ctl -- "(baseline, current, spans, proposals)" --> model["renderReview / renderPlain\n(pure)\n+ wrapBlocksWithSrc (NEW)"] + model -- "annotated HTML w/ data-src on blocks" --> ctl + ctl -- "postMessage{render}" --> wv +``` + +The dashed-in NEW pieces are: `wrapBlocksWithSrc` (pure), the +`runEditAndPropose` host routine with its document-rewrite hunk-diff, the three +inbound toolbar messages, and the `editor/title` gateway menu. Everything else is +the existing F6/F4/F3/F10 machinery. + +### 6.3 Data model & ownership + +**No new persisted artifact** (INV-20). F11 adds only transient on-the-wire +messages (the `ToolbarMsg` union in §2) and reuses F10's `RenderMsg`. Baseline is +owned by F6, proposals by F4 (sidecar), attribution by F3 — all untouched. The +block-offset `data-src` attributes are render-time only (not stored). + +### 6.4 Interfaces & contracts + +- **`trackChangesModel`** (vscode-free, pure): new + `wrapBlocksWithSrc(blocks: BlockWithRange[], renderedPerBlock: string[]): + string` (illustrative) — or, more precisely, both `renderReview` and + `renderPlain` route their per-block rendered HTML through a shared internal + helper that prepends `data-src-start`/`data-src-end` to each block's wrapping + element. Plus `diffToHunks(currentText: string, rewrittenText: string): + Array<{ start: number; end: number; replacement: string }>` — the pure + document-rewrite → per-hunk proposal-range list (vscode-free, deterministic). +- **`TrackChangesPreviewController`** (vscode layer): handles the three new + inbound messages; gains a `runEditAndPropose(document, target: { kind: + "range"; start; end } | { kind: "document" }, instruction)` private routine; + takes (or reaches) the `DiffViewController` to pin the previewed doc and the + edit-turn entry. New test seams as needed (`getLastModel` already exists for + asserting marks without webview DOM). +- **`DiffViewController`** (F6): `pin(document)` is reused as-is (the controller + already exposes pinning by document); `cowriting.pinDiffBaseline`'s + `package.json` `when` flips from `false` to `editorLangId == markdown`. +- **`ProposalController`** (F4): `propose(...)` reused unchanged (called once for + selection, N times for a diffed document). No signature change. +- **`AttributionController`** (F3): unchanged (`applyAgentEdit` reused on accept). +- **Commands / menus (`package.json`):** + - `cowriting.showTrackChangesPreview` — added to `editor/title` with + `when: editorLangId == markdown` (the minimal gateway). Existing palette + + `ctrl+alt+r` kept. + - `cowriting.pinDiffBaseline` — `when` flips to `editorLangId == markdown` + (no longer orphaned). + - `cowriting.editDocument` ("Ask Claude to Edit Document", document-scoped) — + new command registered, routed through `runEditAndPropose({kind:"document"})`; + available for `#42` to reuse. (The preview's selection-scoped edit is driven + by the `askClaude` message carrying webview-resolved offsets, not a command, + since the offsets originate in the webview.) +- **Webview asset** (`media/preview.ts` + `.css`): header gains the two buttons; + a `selectionchange` listener updates the Ask-Claude label; click handlers post + the `ToolbarMsg` intents; the selection→nearest-`data-src` lookup helper. Stays + sealed (nonce'd inline script, CSP unchanged). + +### 6.5 Per–Product-Use-Case design + +- **PUC-1 (toolbar present):** render the two buttons in the header next to the + annotations checkbox; disable Pin + Ask-Claude when `!isAuthorable(document)`. +- **PUC-2 (adaptive label):** webview `selectionchange` → if the selection is + non-empty and within the rendered body, label = "Edit Selection"; else "Edit + Document". Pure webview-local state. +- **PUC-3 (edit selection):** webview resolves selection → `{start, end}` from the + nearest `data-src` ancestors → `postMessage{askClaude, selection, start, end}` → + host `showInputBox` → `runEditAndPropose({kind:"range", start, end})` → + `runEditTurn` → one `propose()` → `onDidChangeProposals` → re-render (inline + blue block at the anchor, `#31`). +- **PUC-4 (edit document):** `postMessage{askClaude, document}` → host input box → + `runEditAndPropose({kind:"document"})` → `runEditTurn` over full text → + `diffToHunks(current, rewritten)` → one `propose()` per hunk → re-render (N blue + blocks). +- **PUC-5 (pin baseline):** `postMessage{pinBaseline}` → `DiffViewController.pin( + previewedDocument)` → `onDidChangeBaseline` → re-render (marks cleared). +- **PUC-6 (right-click gateway):** `editor/title` entry invokes + `cowriting.showTrackChangesPreview` for the tab's document. +- **PUC-7 (edges):** selection resolving to no live block → document scope; + non-authorable → controls disabled; `runEditTurn` failure → existing error path, + no proposal; empty doc → Edit Document over empty range (no-op-safe). + +### 6.6 Non-functional requirements & cross-cutting concerns + +The webview stays **sealed** (INV-21): local assets, strict CSP with a per-load +nonce, no network; the new inline handlers only read `data-src`/`data-proposal-id` +and post intent (no eval, no remote, no document mutation). The instruction prompt +and the LLM turn remain **host-side** — the webview gains **no** LLM, network, or +credential surface (INV-8 untouched). `diffToHunks` and the block wrapping are +O(document), run on a host gesture (not per-keystroke), fine at inner-loop scale. +No telemetry, nothing persisted. + +### 6.7 Key decisions & alternatives considered + +| Decision | Chosen | Alternatives rejected | +| --- | --- | --- | +| **Preview-selection → source mapping granularity** | **Block-level** — pure layer emits `data-src-start/end` from existing `BlockWithRange`; selection → union of intersected live-source blocks. Robust, reuses what exists, ships the full adaptive button now. *(Operator decision, session 0036.)* | **Char-precise** sub-block mapping — needs per-inline-token source offsets markdown-it doesn't reliably give; rendered text ≠ source (syntax stripped) → fragile, risks the whole feature on the hardest part. **Document-only first** — defers the headline adaptive button; punts the risk. | +| **Document-edit proposal granularity** | **Diff Claude's rewrite into hunks → one F4 proposal per changed hunk** — independent ✓/✗ per change; reuses the single-range model N times (no model change). *(Operator decision, session 0036.)* | **One whole-document proposal** — a single giant blue block, all-or-nothing accept/reject; poor UX for a real rewrite. | +| **`#43` vs `#41`/`#42` scope** | **`#43` lands a minimal right-click → Open Review Preview gateway** (`editor/title`), so the toolbar surface is reachable end-to-end and its E2E is real; `#41`/`#42` expand the menu set. *(Operator decision, session 0036.)* | **Toolbar only; all menus in `#41`/`#42`** — `#43`'s "a right-click entry opens the preview" acceptance/E2E couldn't be satisfied within `#43`. | +| **Instruction prompt location** | **Host `showInputBox`** — keeps LLM/secret surface out of the sealed webview; reuses the existing edit-turn flow. | **In-webview text field** — pushes prompt handling toward the sandbox; no benefit. | +| **Pin button target** | **The previewed document** (`DiffViewController.pin(document)`) — the preview knows its bound doc. | **`activeTextEditor`-based command** — may not point at the previewed doc; the source of the orphan. | +| **Pin confirmation** | **No confirm** — matches the existing command; re-pinning recovers. | **Confirm dialog** — friction for a routine, recoverable gesture. | + +### 6.8 Testing strategy + +- **Unit (vitest, vscode-free):** `data-src-start/end` present and correct on every + block for **both** `renderReview` and `renderPlain` (offsets equal the + `BlockWithRange` ranges; determinism — same inputs → identical HTML); + `diffToHunks` over fixtures — a single-hunk rewrite → one range; a multi-hunk + rewrite → the expected disjoint ranges with correct replacements; an unchanged + rewrite → zero hunks; whole-document replacement → one full-range hunk. +- **Host E2E (`@vscode/test-electron`, no LLM, extends the F10 suite):** open a + markdown fixture → `cowriting.showTrackChangesPreview`. Simulate + `{type:"pinBaseline"}` → `getLastModel` shows cleared change-marks + advanced + epoch. Simulate `{type:"askClaude", scope:"selection", start, end}` with a + stubbed edit turn → exactly **one** proposal over the resolved range, anchored + inline. Simulate `{type:"askClaude", scope:"document"}` with a stubbed + multi-hunk rewrite → **N** proposals matching the hunks. Invoke the + `editor/title` gateway command → panel opens. Non-authorable document → toolbar + edit controls disabled (asserted via the model/flags the host exposes). The + webview DOM, real button clicks, the `selectionchange` label flip, and the + selection→`data-src` lookup are **not** E2E-asserted (sealed sandbox) — manual + smoke. +- **Live smoke (manual — `docs/MANUAL-SMOKE-F11.md`):** open a markdown doc; open + the review preview; confirm the three header controls; select a paragraph → + button reads "Edit Selection", click → enter instruction → a blue ✓/✗ block + appears at that paragraph; clear the selection → button reads "Edit Document", + click → instruction → several blue blocks appear; click **Pin baseline** → marks + clear, `Since` label updates; right-click the tab → **Open Review Preview** opens + the panel; verify light/dark theming and that `git status` shows nothing + unexpected. + +### 6.9 Failure modes, rollback & flags + +A selection that resolves to no live-source block → **Edit Document** scope (never +an error). `runEditTurn` failing → existing error handling, no proposal created, +preview unchanged. `diffToHunks` producing zero hunks (rewrite == current) → no +proposals, a brief "no changes proposed" host notice. Webview disposed mid-gesture +→ the host routine completes against the document; the next open re-renders. +**No feature flag** — the toolbar controls are additive UI; nothing persists. +Rollback is reverting the PR with **zero** data migration (nothing persisted; the +F6 baseline, F4 sidecar, F3 attribution data are untouched; the unhidden pin +command and the gateway menu simply disappear). + +--- + +## 7. Delivery Plan + +### 7.1 Approach / strategy + +One planning-and-executing session (F11 = `#43`), plan written just-in-time from +this spec — the F2–F10 precedent. Host-E2E tier (a VS Code extension has no +browser/deploy stage); no LLM in CI (edit turns stubbed). The webview's visual +rendering, the adaptive label, and the selection→source DOM lookup are verified by +the manual smoke; the automated seams are the pure block-wrapping + `diffToHunks` +model and the host's message→seam wiring. + +### 7.2 Slicing plan + +- **SLICE-1 — Pin baseline button + reachability.** Webview header **Pin + baseline** button → `{type:"pinBaseline"}` → host `DiffViewController.pin( + previewedDoc)`; unhide `cowriting.pinDiffBaseline` (`when: editorLangId == + markdown`). Host E2E: pin message clears marks. *(Immediate win — homes the + orphaned command.)* +- **SLICE-2 — Block-offset emission.** Shared pure helper wrapping each block with + `data-src-start/end` in `renderReview` **and** `renderPlain`; vitest for both + modes + determinism. No UI yet. (INV-36 data layer.) +- **SLICE-3 — Edit Document button + hunk path.** Webview **Ask Claude to Edit + Document** button (no-selection state) → `{type:"askClaude", scope:"document"}`; + host `runEditAndPropose({document})` → `runEditTurn` → `diffToHunks` → N + `propose()`; register `cowriting.editDocument`; vitest for `diffToHunks`; host + E2E for the N-proposal path. (INV-37 document half.) +- **SLICE-4 — Adaptive Edit Selection.** Webview `selectionchange` label flip + + selection→nearest-`data-src` resolution → `{type:"askClaude", scope:"selection", + start, end}`; host single-range `propose()`. Host E2E for the selection message → + one anchored proposal. (INV-37 selection half; INV-36 consumer.) +- **SLICE-5 — Gateway, edges, tests & docs.** `editor/title` → Open Review Preview + gateway; non-authorable disabling; host E2E (gateway opens panel; controls + inert on non-authorable); `docs/MANUAL-SMOKE-F11.md`; README F11 section. + +E2E are first-class plan tasks (handbook §9/§4); this app's required tier is host +E2E (the F2–F10 precedent). + +### 7.3 Rollout / launch plan + +Non-shippable (no marketplace publish). "Done" = `#43` acceptance: the preview +toolbar hosts the annotations checkbox + a Pin baseline button + a single adaptive +Ask-Claude button (Edit Selection ⇆ Edit Document) that route through the existing +F4/F3/F6 machinery; edits surface as proposals (one for a selection, per-hunk for +a document rewrite); a right-click entry opens the preview; the pin command is no +longer orphaned; unit + host E2E green; live smoke performed once. + +### 7.4 Risks & mitigations + +| Risk | Mitigation | +| --- | --- | +| Block-level selection feels coarse vs the editor's char-precise Edit Selection | Locked v1 decision (§6.7); a rendered surface is naturally block-grained; char-precise is a deferred follow-up if the coarseness bites | +| `data-src` attributes perturb markdown-it output or the F10 proposal/diff rendering | Wrapping is applied at the block boundary (outside inline parsing); covered by determinism + both-mode unit tests; per-block `try/catch` error chip (F7) on render failure | +| `diffToHunks` produces awkward hunk boundaries on a large rewrite | Pure + unit-tested over fixtures; hunks are line/block-aligned; worst case is more/fewer blocks, all independently ✓/✗-able — never wrong, just granular | +| Selection inside a deletion/proposal block has no live-source range | Falls back to Document scope by design (INV-36); manual-smoke verified | +| The webview selection→`data-src` lookup isn't E2E-testable (sealed) | The host half (offsets→fingerprint→propose) is E2E'd via simulated messages; the DOM lookup is the only manual-smoke-only seam, kept deliberately thin | +| Unhiding pin / adding `editDocument` widens the command surface | Both guard on `editorLangId == markdown`; both route through existing seams; no new model or persisted state | + +--- + +## 8. Traceability matrix + +| Requirement (`#43`) | Use case | Design | Slice | +| --- | --- | --- | --- | +| Pin baseline button in the preview toolbar | PUC-5 | INV-35, §6.4 (`DiffViewController.pin`) | SLICE-1 | +| Resolve the orphaned `pinDiffBaseline` reachability | PUC-5 | §6.4 (`when` flip) | SLICE-1 | +| Single adaptive Ask-Claude button (Selection ⇆ Document) | PUC-2/3/4 | INV-37, §6.2 | SLICE-3/4 | +| Preview-selection → source range mapping (block-level) | PUC-3 | INV-36, §6.7 | SLICE-2/4 | +| Edit Document path (new whole-document edit) | PUC-4 | INV-37, §6.5 (hunk diff) | SLICE-3 | +| Edits route through existing F4/F3 (no divergent path) | PUC-3/4 | INV-35, §6.4 | SLICE-3/4 | +| Right-click entry opens the preview (minimal gateway) | PUC-6 | §6.4 (`editor/title`) | SLICE-5 | +| Controls only active for supported (authorable) docs | PUC-1/7 | §6.5 | SLICE-5 | +| Sealed webview, no document mutation / LLM surface | — | INV-21/35, §6.6 | all | +| No new edit/attribution/proposal model | — | §1.7, INV-37 | all | +| Unit + host E2E + right-click-opens-preview coverage | — | §6.8 | SLICE-1..5 | + +## 9. Open Questions & Decisions log + +- **RESOLVED (session 0036, operator):** preview-selection → source mapping = + **block-level** (`data-src` attributes from `BlockWithRange`; union of + intersected blocks); document edit = **diffed into per-hunk F4 proposals**; + `#43` **lands a minimal right-click → Open Review Preview gateway** (`#41`/`#42` + expand the menus). +- **RESOLVED (this spec, autonomous):** instruction prompt = **host `showInputBox`** + (LLM/secrets stay out of the webview); Pin targets the **previewed document** + (`DiffViewController.pin`); **no confirmation** on pin (matches existing); a new + `cowriting.editDocument` command is registered for `#42` reuse; `pinDiffBaseline` + is unhidden (`editorLangId == markdown`). +- **OPEN → later:** **char-precise** sub-block selection mapping (deferred — block + granularity is v1); the **richer `#41`/`#42` menu sets** (this lands only the + minimal gateway); preview→source **scroll-sync** (`#32`); whether a large + document rewrite should cap/segment its hunks (only if real rewrites prove + noisy); the repo rename to `vscode-markdown-cowriting-plugin` (`#35`, deferred). + +## 10. Glossary & References + +- **Preview toolbar** — the review preview's header row: the annotations on/off + checkbox (existing) plus F11's Pin baseline and adaptive Ask-Claude buttons. + **Adaptive Ask-Claude button** — one button reading "Edit Selection" (non-empty + preview selection) or "Edit Document" (none). **Block-level selection mapping** — + resolving a rendered-preview selection to the union of source blocks it + intersects, via `data-src-start/end` attributes emitted by the pure render + layer. **Hunk-diffed document edit** — Claude's whole-document rewrite split + into changed hunks, each surfaced as its own F4 proposal. **Pin baseline** — F6's + `DiffViewController.pin` applied to the previewed document. **Gateway** — a + right-click entry that opens the preview (this feature lands the minimal one; + `#41`/`#42` expand them). +- Feature `#43` (F11) · Epic `#1` · builds on F3 `#6`, F4 `#12`, F6 `#17`/`#19`, + F7 `#21`/`#22`, F9 `#27`, F10 `#29`/`#31` · coexists with `#41`/`#42` · parent + specs `coauthoring-inner-loop.md`, `coauthoring-attribution.md`, + `coauthoring-propose-accept.md`, `coauthoring-diff-view.md`, + `coauthoring-rendered-preview.md`, `coauthoring-interactive-review.md` · capture + session 0035 · lineage `ben.stull/rfc-app#48`. diff --git a/specs/coauthoring-document-edit-flow.md b/specs/coauthoring-document-edit-flow.md new file mode 100644 index 0000000..69d386b --- /dev/null +++ b/specs/coauthoring-document-edit-flow.md @@ -0,0 +1,592 @@ +--- +status: graduated +--- +# Solution Design: The Ask-Claude Document-Edit Flow (#42 · #47 · #46) + +| | | +| --- | --- | +| **Author(s)** | Ben Stull (with Claude) | +| **Reviewers / approvers** | Ben Stull | +| **Status** | `draft` | +| **Version** | v0.1.0 | +| **Source artifacts** | Features `benstull/vscode-cowriting-plugin#42` (Ask Claude to Edit Document, `type/feature`, `priority/P2`) · `#47` (block-granularity proposals, `type/feature`, `priority/P1`) · `#46` (accept-all, `type/feature`, `priority/P2`) · Epic `#1` (closed) · Capture sessions `vscode-cowriting-plugin-0035` (#42), `0040` (#47), `0039` (#46) · Brainstorming session `vscode-cowriting-plugin-0041` · Builds on (all shipped): F4 `#12` (propose/accept seam), F10 `#29` (interactive review preview), F11 `#43` (preview toolbar + `editDocument`/`runEditAndPropose`) · Parent specs (graduated): `coauthoring-propose-accept.md`, `coauthoring-attribution.md`, `coauthoring-interactive-review.md`, `coauthoring-rendered-preview.md` · F11 design currently lives in code + issue draft `issues/preview-toolbar-interaction-surface.md` (un-graduated — see §9) · Lineage: `ben.stull/rfc-app#48` | + +**Change log** + +| Date | Version | Change | By | +| --- | --- | --- | --- | +| 2026-06-12 | v0.1.0 | Initial draft — brainstorming session 0041. One combined design for the document-edit cluster, sequencing #42 → #47 → #46. Two forks locked with the operator: per-block proposals **preserve unchanged-span attribution** (block = decision unit, word = attribution unit); the cluster ships as **one** Solution Design. | Ben Stull + Claude | + +--- + +## 1. Business Context + +### 1.1 Executive Summary + +The plugin can already ask Claude to edit a whole document (F11 shipped +`cowriting.editDocument` → `runEditAndPropose({kind:"document"})`), but the flow +around that capability is rough on three sides, each captured as its own feature: + +- **You can't easily reach it** (#42). "Ask Claude to Edit" only appears in the + editor body **when text is selected**. Right-clicking the body with *no* + selection, or right-clicking the editor **tab**, offers nothing — two natural + "do something with this document" gestures hit a dead end. +- **What comes back is unreviewable** (#47, **P1**). A whole-document rewrite is + diffed at **word** granularity (`diffToHunks`, INV-37): a light copy-edit pass + explodes into dozens of tiny blue ✓/✗ blocks, forcing the reviewer to + adjudicate word-fragments out of the sentence they live in. +- **Taking the result is tedious** (#46). Accepting Claude's edits is **one + proposal at a time**; a document a writer wants wholesale still demands a click + per proposal. + +This design treats the three as **one flow** — **reach → review → accept** — and +ships them in that order. **#42** makes the document edit reachable from the +obvious gestures; **#47** cuts the rewrite into **one proposal per changed block** +(paragraph / header / bullet / fence) so the reviewer decides in the unit they +think in; **#46** adds a single **Accept all** gesture. The whole flow is built on +the existing F4 propose/accept seam and the F10/F11 preview — **no new edit path, +no new accept mechanism, no new persisted model.** + +### 1.2 Background + +The inner loop shipped F2–F5 (threads · attribution · propose/accept · +cross-rung). F10 (`#29`) collapsed the review surfaces into one: a **clean +editor** + the **rendered markdown preview** as the single annotated review +surface, with ✓/✗ acting only on pending F4 proposals (INV-32..34). F11 (`#43`) +made the preview's **toolbar the primary interaction surface** — a **Pin +baseline** button and an **adaptive Ask-Claude** button — and, as part of that, +added the host-side **`cowriting.editDocument`** command and the +**`runEditAndPropose`** path that diffs a whole-document rewrite into per-hunk F4 +proposals (INV-35 sealed-webview intent, INV-36 `data-src` block→source mapping, +INV-37 word-level per-hunk granularity). + +So the *capability* exists; this cluster is about the **experience around it**. +Three captures landed it: + +- **0035 → #42**: "Add 'Ask Claude to Edit Document' on right-click with no + selection; same Edit-Selection/Edit-Document on the tab." +- **0040 → #47**: "A single change should be a paragraph (or header, or + bulletpoint), not individual words. It's too much to review." +- **0039 → #46**: "We need a way to accept all of Claude's edits." + +The captures themselves flagged that these "form an edit-flow cluster — brainstorm +/ sequence together," which this session does. + +### 1.3 Business Actors / Roles + +- **Coauthor (human)** — the writer/engineer; the sole user of every gesture + here (right-click to ask, ✓/✗ to adjudicate, Accept-all to commit). +- **Coauthor (machine)** — Claude via `@cline/sdk`; produces the document + rewrite that becomes block-level proposals; its accepted blocks land + Claude-attributed (word-precise — §6, INV-40). + +### 1.4 Problem Statement + +How do we make "ask Claude to revise this whole document, look at what it did, +and take the parts I want" a **fluid, reviewable, low-friction** loop — when today +it is **hard to invoke, exploded into word-fragments, and accepted one click at a +time** — **without** forking a second edit path, a second accept mechanism, or a +new persisted artifact? + +### 1.5 Pain Points + +| # | Pain | Felt by | Today | +| --- | --- | --- | --- | +| P-1 | No whole-document ask from the body (no selection) or the tab | Mouse-first / tab-oriented writers | Dead-end gestures | +| P-2 | A document edit fans out into many word-level proposals | Anyone reviewing a document edit (worse the lighter the edit) | "Too much to review" (#47, P1) | +| P-3 | Accepting many proposals is one click each | Writers who want Claude's whole pass | Manual sweep of ✓ | +| P-4 | Block-level accept could lose attribution fidelity inside a block | The product's F3 attribution value | (a design risk this spec resolves — INV-40) | + +### 1.6 Targeted Business Outcomes + +- A **consistent "Ask Claude to Edit"** entry point — selection-scoped with a + selection, whole-document without — from both the body and the tab. +- A document edit produces a **small number of in-context, block-sized** + decisions instead of a flood of word-fragments. +- **"Take Claude's whole pass"** is a **single gesture**. +- Attribution stays **word-precise** even though the decision unit is the block. + +### 1.7 Scope (business) + +**In scope:** the three features as one sequenced flow — entry-point wiring +(#42); document-edit proposal granularity word→block (#47); bulk-accept over the +current document (#46). Each remains its own implementation increment / PR. + +**Out of scope / non-goals:** + +- **Selection edits are unchanged** — a selection edit is already a single + proposal; #47 touches only the document path, #42 leaves selection behavior + intact, #46 reuses the same seam. +- **No Explorer context-menu entry** for Ask-Claude (#42 is tab + body only, per + the capture; the review-panel Explorer entry shipped separately as #41). +- **No "reject all"** (#46 is accept-only; a sibling ask if wanted). +- **No cross-document / workspace-wide accept-all** — current document only. +- **No new proposal-granularity *setting*** — block is the chosen unit, not a + toggle. +- **No change to the inline word-level ``/`` rendering** inside a block + — the reviewer still sees exactly what changed; only the *decision unit* and the + *attribution reconciliation* change. + +### 1.8 Assumptions · Constraints · Dependencies + +- **F11 already shipped the document-edit path** (`editDocument`, + `askClaude`, `runEditAndPropose`, `diffToHunks`). This is the strongest + constraint: #42 is mostly **menu wiring** (the command exists); #47 **replaces + the diff inside `runEditAndPropose`'s document branch**; #46 **loops the existing + `acceptById`**. The cluster adds almost no new surface. +- **The block model already exists** — `splitBlocks` / `splitBlocksWithRanges` / + `diffBlocks` (`BlockOp`: unchanged / added / removed / changed+atomic; fences + atomic per INV-23). #47 **reuses this**, not a second notion of "block." +- **The F4 accept seam is the only mutation path** — + `ProposalController.accept` → `AttributionController.applyAgentEdit` + (`WorkspaceEdit`); the sealed webview posts **intent** only (INV-8/21/35). +- **No new persisted artifact** (INV-20) — proposals live in the F4 sidecar; this + cluster adds at most one transient field on the proposal (granularity marker — + §6.3) and transient webview messages. +- VS Code context menus render a command's own `title`; the tab menu picks one of + two commands by a `when` clause on selection state (no per-menu relabel of one + command). + +### 1.9 Business Use Cases + +- **BUC-1 (reach)** — A writer right-clicks the document (body, no selection) or + the tab and asks Claude to edit the whole document. +- **BUC-2 (review)** — The writer reads Claude's pass as block-sized proposals + and accepts/rejects each. +- **BUC-3 (accept)** — The writer, happy with the pass, accepts all pending + proposals in one gesture. + +--- + +## 2. Solution Proposal + +**One flow, three increments, all on existing seams.** + +```mermaid +flowchart LR + subgraph reach["#42 — REACH"] + body["right-click body / tab\n(selection-aware)"] --> cmd["editSelection /\neditDocument"] + end + subgraph review["#47 — REVIEW"] + cmd --> rep["runEditAndPropose\n(document branch)"] + rep -- "diffToBlockHunks\n(per changed block)" --> props["F4 proposals\n(one per block)"] + end + subgraph accept["#46 — ACCEPT"] + props --> one["✓/✗ per block"] + props --> all["Accept all\n(toolbar intent)"] + end + one --> seam["F4 acceptById →\napplyAgentEdit"] + all --> seam + seam -- "block proposals: word-precise\nintra-block attribution (INV-40)" --> doc[(document)] +``` + +The defining idea of the cluster is in **#47's accept**: a document edit's +proposals are cut at **block** granularity (the unit a human reviews), but each +accept reconciles attribution at **word** granularity (the unit F3 records). The +block is the **decision** unit; the word is the **attribution** unit. This is what +lets us coarsen the review without coarsening the attribution — and it reuses the +*exact* word-level machinery (`diffToHunks`) that INV-37 used, now repurposed as +the intra-block sub-diff at accept time rather than the proposal cut. + +--- + +## 3. Product Personas + +- **PP-1 (the coauthoring writer)** — edits a markdown document with Claude; + wants to invoke a whole-document pass quickly, review it at paragraph altitude, + and take it wholesale or selectively. + +--- + +## 4. Product Use Cases + +| PUC | As PP-1 I… | Feature | Acceptance | +| --- | --- | --- | --- | +| PUC-1 | right-click the body with **no selection** → **Ask Claude to Edit Document** | #42 | whole-document edit → block proposals in the preview | +| PUC-2 | right-click the body **with a selection** → **Ask Claude to Edit Selection** (unchanged) | #42 | one proposal over the selection (existing behavior) | +| PUC-3 | right-click the **tab** → Edit Selection (if selection) / Edit Document (if not) | #42 | same behavior as the body equivalents, against that doc | +| PUC-4 | see Claude's document edit as **one proposal per changed block** | #47 | M changed blocks → M proposals; unchanged blocks → none; fences atomic | +| PUC-5 | accept a block proposal and have **only Claude's actual changes** attributed to Claude | #47 | unchanged words inside the accepted block keep their prior author (INV-40) | +| PUC-6 | press **Accept all** to take every pending proposal at once | #46 | all anchored proposals applied; orphans skipped + reported | + +--- + +## 5. UX Layout + +- **#42 entry points.** Two existing commands, surfaced in two more menus: + - **Editor body** (`editor/context`): **Ask Claude to Edit Selection** when + `editorHasSelection`, **Ask Claude to Edit Document** when not. + - **Editor tab** (`editor/title/context`): the same pair, gated the same way, + targeting the tab's document. + - Both entries appear **only for markdown / authorable** docs (mirroring + `editSelection`'s `when`: `resourceScheme == file || untitled` + + `editorLangId == markdown`). +- **#47 review.** The preview is unchanged in *look* — the reviewer still sees + word-level ``/`` inside each block. What changes: each **changed + block** is **one `cw-proposal` block** carrying one `data-proposal-id` and one + ✓/✗ pair, instead of several. A changed code/mermaid fence is one atomic + proposal (INV-23). +- **#46 accept-all.** An **Accept all** button in the F11 preview **toolbar**, + shown when the document has **≥ 2 pending proposals**. Pressing it posts the + `acceptAll` intent; on completion the host reports applied-vs-skipped (status + message / toast). No confirmation dialog — VS Code undo restores (consistent + with single accept). + +``` +┌─ Review: my-doc.md ──────────────────────────────┐ +│ [ Pin baseline ] [ Ask Claude ▾ ] [ Accept all ]│ ← Accept all visible only when ≥2 pending +├───────────────────────────────────────────────────┤ +│ ## Heading │ +│ ┌───────────────────────────── proposal ───────┐ │ +│ │ The ~~quick brown~~ swift fox … ✓ ✗ │ │ ← one block, word-level ins/del inside +│ └───────────────────────────────────────────────┘ │ +└───────────────────────────────────────────────────┘ +``` + +--- + +## 6. Technical Design + +### 6.1 Invariants + +Parent invariants **INV-1..INV-37 carry over except where #47 supersedes +INV-37**. This cluster adds: + +- **INV-38 (consistent, selection-aware Ask-Claude entry points)** — "Ask Claude + to Edit" is reachable from the editor **body** and the editor **tab**, + **selection-scoped when there is a selection** (`cowriting.editSelection`), + **whole-document when there is not** (`cowriting.editDocument`). Both menus are + gated to **markdown / authorable** docs. Both scopes route through the **single** + `runEditAndPropose` path — no divergent edit code (#42). +- **INV-39 (document edits propose per changed block — supersedes INV-37)** — a + whole-document rewrite is cut into **one F4 proposal per changed block**, where a + block is the unit `splitBlocks` already recognizes (prose paragraph / header / + bullet; code & mermaid fences **atomic**, INV-23). **Unchanged blocks produce no + proposal.** A block proposal's anchor spans the **whole** source block and its + replacement is Claude's **whole** version of that block. This **replaces** + INV-37's per-hunk word-level granularity **for document edits** (selection edits + are unaffected) (#47). +- **INV-40 (block decision, word-precise attribution)** — accepting a block + proposal applies Claude's whole block but attributes **only the runs Claude + actually changed** to Claude; **unchanged spans within the block keep their + prior authorship**. Mechanism: an intra-block **word** sub-diff at accept time + (reusing `diffToHunks`) drives one `applyAgentEdit` per changed run. The **block + is the decision unit; the word is the attribution unit** (#47). +- **INV-41 (block-insertion anchoring)** — a newly **inserted** block (one Claude + added) is anchored to an adjacent block boundary so its proposal **resolves and + is acceptable** — the block-level analogue of `anchorInsertion`; **no + unresolvable zero-width block proposal** (#47). +- **INV-42 (accept-all = batched F4 seam; current doc; orphan-skip)** — + `cowriting.acceptAllProposals` applies **every pending proposal on the current + document** through the existing `acceptById` seam, in a **re-anchor-safe order**, + **skipping (never force-applying)** proposals that can't anchor, and **reports + applied-vs-skipped**. It is a **batched application of the existing accept path**, + not a new mechanism; the webview button posts **intent** (INV-35). Per-proposal + ✓/✗ is unchanged (#46). + +**Supersession (explicit):** **INV-39 reverses INV-37** (document rewrite → per +*hunk*/word) in favour of per *block*. INV-37's word-level engine (`diffToHunks`) +is **not removed** — it is **repurposed** as the intra-block sub-diff that +realizes INV-40's word-precise attribution. The F11 spec carrying INV-37 is +un-graduated (§9); this spec is the canonical home of the superseding decision. + +### 6.2 High-level architecture + +All three increments are localized to four already-existing files; no new module +is required (one new pure function + one new command + menu JSON). + +```mermaid +flowchart TD + pkg["package.json\n(menus: editor/context + editor/title/context)"] -. "#42 wiring" .-> cmds + cmds["editSelection / editDocument\n(existing commands)"] --> rep["runEditAndPropose\n(trackChangesPreview.ts)"] + rep -- "selection branch (unchanged)" --> single["one single-range proposal"] + rep -- "document branch (#47)" --> dbh["diffToBlockHunks\n(trackChangesModel.ts — NEW, pure)"] + dbh --> blockprops["one proposal per changed block\n(granularity: 'block')"] + single & blockprops --> pc["ProposalController\n(proposalController.ts)"] + pc -- "accept (block) → intra-block sub-diff (#47, INV-40)" --> attr["AttributionController.applyAgentEdit\n(per changed run)"] + pc -- "acceptAll (#46, INV-42)" --> pc + tb["preview toolbar\nAccept all button"] -- "{type:'acceptAll'} intent (INV-35)" --> pc +``` + +- **`package.json`** (#42) — add `editSelection` + `editDocument` to + `editor/context` and `editor/title/context`, each with a selection-state `when` + clause; titles "Ask Claude to Edit Selection" / "Ask Claude to Edit Document". +- **`trackChangesModel.ts`** (#47, pure / vscode-free) — add + **`diffToBlockHunks(currentText, rewrittenText): EditHunk[]`**: split both sides + into the existing block units, diff blocks (reusing `diffBlocks`/`diffArrays` + keying), and emit **one `EditHunk` per changed-or-added block** spanning the + whole block's source range → its rewrite text; fences atomic; inserted blocks + anchored (INV-41). Returns the same `EditHunk[]` shape `diffToHunks` does, so the + proposal-minting loop is unchanged. **`diffToHunks` stays** as the intra-block + sub-diff helper for INV-40. +- **`trackChangesPreview.ts`** (#42/#47) — `runEditAndPropose`'s **document + branch** calls `diffToBlockHunks` instead of `diffToHunks`, and tags each minted + proposal `granularity: "block"`; `editDocument` resolves the **tab's** document + (URI arg) when invoked from `editor/title/context`, not only `activeTextEditor`. + Adds the `acceptAll` inbound-message case (#46) → `proposals.acceptAllProposals`. +- **`proposalController.ts`** (#47/#46): + - `accept` (#47): when `proposal.granularity === "block"`, instead of one + whole-block `applyAgentEdit`, compute the intra-block word sub-diff + (`diffToHunks(resolvedBlockText, proposal.replacement)`) and apply **one + `applyAgentEdit` per changed run** (Claude-attributed), unchanged runs left in + place — one undo-grouped edit. Non-block proposals keep the existing single + `applyAgentEdit`. + - `acceptAllProposals(document)` (#46): gather pending proposals, apply via + `acceptById` in **descending document order** (last anchor first, so earlier + anchors' offsets stay valid), skip orphans (suppressing per-item warnings), + and return `{ applied, skipped }` for the controller to report. + +### 6.3 Data model & ownership + +**No new persisted artifact** (INV-20). The only model change is **one optional +field** on the F4 `Proposal`: + +```ts +// F4 sidecar Proposal — additive, optional, defaulted (back-compat): +interface Proposal { + // …existing fields… + granularity?: "block" | "single"; // "block" ⇒ accept reconciles attribution per word (INV-40) +} +``` + +- A document-edit proposal is minted with `granularity: "block"`; selection + proposals are `"single"` (or absent → treated as single). A missing field on an + older sidecar reads as `"single"` — existing proposals accept exactly as before. +- New transient webview message (host ⇄ webview), extending the F11 toolbar + intents: + +```ts +// webview → host (intent only — INV-35) +type ToolbarMsg = + | /* …existing: setMode | accept | reject | pinBaseline | askClaude… */ + | { type: "acceptAll" }; +``` + +Baseline owned by F6; proposals by F4 (sidecar); attribution by F3. This cluster +adds no new ownership. + +### 6.4 Interfaces & contracts + +- **`trackChangesModel`** (vscode-free): `diffToBlockHunks(currentText: string, + rewrittenText: string): EditHunk[]` — deterministic; one hunk per changed/added + block over the existing block units; fences atomic; inserted blocks anchored + (INV-41). Same `EditHunk` shape as `diffToHunks`. `diffToHunks` is **retained** + (intra-block sub-diff for INV-40). +- **`ProposalController`**: `acceptAllProposals(document): Promise<{ applied: + number; skipped: number }>` — batched accept over the document's pending set via + the existing `acceptById` seam, re-anchor-safe order, orphan-skip (INV-42). The + private `accept` gains the block-aware attribution branch (INV-40) keyed off + `proposal.granularity`. +- **Commands**: `cowriting.editDocument` (existing — extended to resolve a tab + URI arg); new `cowriting.acceptAllProposals` (active doc; reachable outside the + webview). + +### 6.5 Per–Product-Use-Case design + +- **PUC-1/2/3 (#42)** — pure `package.json` menu wiring + the `editDocument` tab + URI resolution. `runEditAndPropose` is reached unchanged; the only code touch is + resolving the right document for the tab gesture (mirroring #41's clicked-doc + resolution). +- **PUC-4 (#47 cut)** — `diffToBlockHunks` replaces `diffToHunks` in the document + branch. For each `BlockOp` that is `changed` or `added`, emit one `EditHunk`: + `changed` → `[block.start, block.end) → after-block text`; `added` → a zero-width + range anchored to the adjacent block boundary (INV-41) with the new block text + + a separating newline as needed; `removed` → a hunk that deletes the block; + `unchanged` → nothing. Fences (`atomic`) emit one whole-fence hunk (never + word-refined). +- **PUC-5 (#47 attribution, INV-40)** — at accept of a `block` proposal: re-resolve + the anchor → the current block text; `diffToHunks(currentBlockText, + replacement)` → the word-level changed runs *within* the block; apply each run's + replacement via `applyAgentEdit` (Claude-attributed) in one undo-grouped + `WorkspaceEdit`, leaving unchanged runs untouched so their prior attribution + stands. Net buffer text equals the whole-block replacement; net attribution is + word-precise. +- **PUC-6 (#46)** — `acceptAllProposals`: snapshot the pending list, sort + descending by resolved anchor start, `acceptById` each (block proposals take the + INV-40 path automatically), tally applied vs. skipped-orphan, report once. + +### 6.6 Non-functional requirements & cross-cutting concerns + +- **Determinism / testability** — `diffToBlockHunks` is pure and vscode-free + (unit-tested like `diffToHunks`/`diffBlocks`). The INV-40 attribution + reconciliation is exercised through the controller with a fake document. +- **Security** — unchanged: the webview stays sealed; **Accept all** posts intent; + the host validates and performs every mutation (INV-8/21/35). No LLM/secret + surface enters the sandbox. +- **Performance** — block diffing is cheaper than the prior word fan-out (fewer + proposals minted/rendered). Accept-all is O(pending) seam calls; large documents + bounded by proposal count, not document size. +- **Reversibility** — accept-all is one undo-restorable step-set; block accept is + one undo-grouped edit. No destructive irreversibility. + +### 6.7 Key decisions & alternatives considered + +| Decision | Chosen | Alternative (rejected) | +| --- | --- | --- | +| Cluster artifact | **One combined Solution Design**, three increments sequenced #42→#47→#46 (§3.3 — one design absorbs the roadmap) | Three separate specs — more duplication of the shared F11 edit path + F4 seam | +| #47 block model | **Reuse `splitBlocks`/`diffBlocks`** | Invent a second "block" notion — divergence risk (capture warned against it) | +| #47 attribution of unchanged words in an accepted block | **Preserve unchanged spans** — block decision, word-precise attribution (INV-40), via intra-block `diffToHunks` sub-diff at accept | **Whole block → Claude** — simpler (one seam call) but loses F3 fidelity inside the block; operator chose fidelity | +| #47 vs INV-37 | **Supersede** INV-37 for document edits; **repurpose** `diffToHunks` as the intra-block engine | Keep word-level proposals + a separate "group view" — two models, more surface | +| #46 apply-order | **Descending document order** through `acceptById` (each accept re-resolves the fingerprint, INV-11) | Re-resolve-then-rescan after every accept — more work for the same result | +| #46 confirmation | **None** (undo restores) | A confirm dialog — friction the single-accept path doesn't have | +| #46 scope | **Current document only** | Workspace-wide — capture non-goal | +| #42 surfaces | **Body + tab only**, selection-aware | Add Explorer entry — capture non-goal (#41 covers Explorer for the review panel) | + +### 6.8 Testing strategy + +- **Unit (vitest, pure)** — `diffToBlockHunks`: N words across M blocks → **M** + hunks (one per changed block, spanning the block); unchanged block → none; + changed fence → **one atomic** hunk; inserted block → an anchored, resolvable + hunk (INV-41); each hunk's `[start,end)` lands on real block boundaries in + `currentText`. +- **Unit (controller)** — INV-40: accepting a block proposal whose replacement + changes 2 of 6 words attributes **only those 2 runs** to Claude (the other 4 + keep prior author); orphaned block proposal is flagged, not force-applied. +- **Host E2E (Playwright)** — + - #42: body no-selection shows **Edit Document**; body selection shows **Edit + Selection**; tab shows the right one per selection state; each produces + proposal(s). + - #47: a document edit changing words across M blocks yields **M proposals**, + each spanning its block; unchanged block → none; changed fence → one atomic + proposal; an inserted block accepts cleanly. + - #46: with N pending proposals, **Accept all** applies all N (text replaced, + Claude-attributed, proposals cleared); an orphaned proposal in the set is + skipped and **reported**, not mangled; the button is hidden with < 2 pending. +- **Manual smoke** — extend `docs/MANUAL-SMOKE-F11.md` (or a new + `MANUAL-SMOKE-F12.md`) covering the three gestures end-to-end. + +### 6.9 Failure modes, rollback & flags + +- **Orphaned proposal** (target text changed / missing) — single accept already + warns + flags (never applied by guess); accept-all **skips** it and includes it + in the applied-vs-skipped report (INV-42). Reject-or-undo remains the recovery. +- **Inserted block fails to anchor** — INV-41's adjacent-boundary anchoring makes + this resolvable; if the boundary block itself later changes, the proposal orphans + and is handled as above (never silently dropped). +- **Back-compat** — the additive `granularity` field defaults to `"single"`; + pre-existing sidecars/proposals accept unchanged. No migration. +- **No feature flags** — each increment ships green through the pipeline; this is + a VS Code extension (no flotilla deploy stage). + +--- + +## 7. Delivery Plan + +### 7.1 Approach / strategy + +Ship the flow **reach → review → accept** as three independent +planning-and-executing increments, each its own PR, in sequence. They are +**code-independent** (different files / functions), so the order is chosen for +**user-visible value compounding**, not technical dependency — each increment is +usable on its own. + +> **WSJF note (transparent):** **#47 carries the P1 value** ("too much to review" +> is the sharpest pain); #42 and #46 are P2. The operator set the goal "starting +> with #42," and #42 is the smallest, lowest-risk increment (the command already +> exists — it's menu wiring), so leading with it completes the entry-point story +> first. Because the three are code-independent, **#47 and #42 could swap** with no +> rework if the P1 review pain should land first — a one-line resequencing. + +### 7.2 Slicing plan + +- **SLICE-1 — #42 (reach).** `package.json` menus (`editor/context` + + `editor/title/context`, selection-aware, markdown-gated); `editDocument` + resolves the tab's document; titles. **+ E2E** (body/tab × selection state → + right item → proposal). Smallest; no model change. (INV-38.) +- **SLICE-2 — #47 (review).** `diffToBlockHunks` (pure, + unit tests); document + branch of `runEditAndPropose` switches to it and tags `granularity:"block"`; + block-aware accept in `ProposalController` (intra-block sub-diff, INV-40); + block-insertion anchoring (INV-41). **Supersede INV-37 → INV-39** in the spec/ + invariant set. **+ unit + E2E.** The substantive increment. (INV-39/40/41.) +- **SLICE-3 — #46 (accept).** `acceptAllProposals` (controller, re-anchor-safe + order, orphan-skip + report); `cowriting.acceptAllProposals` command; preview + toolbar **Accept all** button + `{type:"acceptAll"}` intent (shown ≥2 pending). + **+ E2E.** (INV-42.) + +### 7.3 Rollout / launch plan + +No deploy pipeline (VS Code extension — no flotilla / PPE / prod stages; the §9 +pipeline's deploy stages don't apply). Each slice merges to `main` green +(unit + host E2E). "Done" = merged with tests green; there is no prod-promotion +gate for an extension increment. + +### 7.4 Risks & mitigations + +| Risk | Mitigation | +| --- | --- | +| INV-40 intra-block multi-edit accept skews offsets mid-apply | Apply changed runs **last-position-first** within the block, one undo-grouped `WorkspaceEdit`; unit-test the 2-of-6-words case | +| Block taxonomy edge cases (lists w/o blank lines, tables, blockquotes, nested) | **Reuse `splitBlocks` exactly** — whatever it already treats as one block *is* one proposal; document the taxonomy = the splitter's, no second notion | +| Inserted-block anchor collides / can't resolve | INV-41 anchors to an adjacent **unchanged** block boundary; orphan-handling is the backstop (never force-applied) | +| Accept-all on a large pending set partially applies then an orphan appears | Each `acceptById` is independent + re-resolves (INV-11); skipped orphans reported; undo restores the whole step-set | +| F11 spec un-graduated → INV-35/36/37 only in code | This spec restates the load-bearing F11 invariants it depends on; §9 logs the graduation gap as a follow-up | + +--- + +## 8. Traceability matrix + +| Requirement | PUC | Invariant / §6 | Slice | Feature | +| --- | --- | --- | --- | --- | +| Ask-Claude reachable body (no-sel) + tab, selection-aware | PUC-1/2/3 | INV-38, §6.5 | SLICE-1 | #42 | +| Selection edit unchanged | PUC-2 | INV-38 (single path) | SLICE-1 | #42 | +| Document edit → one proposal per changed block | PUC-4 | INV-39, §6.5 (`diffToBlockHunks`) | SLICE-2 | #47 | +| Unchanged block → no proposal; fence atomic | PUC-4 | INV-39, INV-23 | SLICE-2 | #47 | +| Accepted block attributes only changed words to Claude | PUC-5 | INV-40, §6.5 | SLICE-2 | #47 | +| Inserted block anchors + accepts | PUC-4 | INV-41 | SLICE-2 | #47 | +| Accept-all over current doc, orphan-skip + report | PUC-6 | INV-42, §6.5 | SLICE-3 | #46 | +| All mutation via the F4 seam; webview intent-only | PUC-4/5/6 | INV-35, §6.6 | SLICE-2/3 | #47/#46 | + +--- + +## 9. Open Questions & Decisions log + +**Decided this session (operator):** + +- **D-1** — #47 accepts **preserve unchanged-span attribution** (block = decision + unit, word = attribution unit; INV-40). *Not* whole-block→Claude. +- **D-2** — the cluster ships as **one** combined Solution Design (this doc), + three sequenced increments. +- **D-3 (driver call, low-confidence — for finalize review)** — sequence **#42 → + #47 → #46** per the goal's "starting with #42," despite #47 carrying the P1 + value. The three are code-independent so the order is reversible (§7.1). +- **D-4 (driver call)** — block taxonomy = **exactly `splitBlocks`'s** units (no + second notion of "block"); list/table/blockquote/nested behavior is whatever the + splitter already does. +- **D-5 (driver call)** — accept-all needs **no confirmation** (undo restores); + button shown at **≥ 2** pending. + +**Open / deferred:** + +- **OQ-1** — Should **selection** edits *also* preserve-unchanged-span + attribution for consistency with INV-40? Out of #47's captured scope (selection + edits explicitly unchanged); a future consistency pass could unify them. **Not + in this cluster.** +- **OQ-2 (spec-hygiene, not blocking)** — the **F11 (#43) Solution Design is + un-graduated** — INV-35/36/37 live only in code comments + the issue draft + `issues/preview-toolbar-interaction-surface.md`, not in `specs/`. This spec + restates the F11 invariants it depends on and supersedes INV-37 here, but the + F11 design should be graduated to `specs/` for a clean invariant ledger. + Captured for finalize as a follow-up. +- **OQ-3** — Very large blocks: no special fallback is specified (block proposals + are bounded by the block, and accept reconciles per word). Revisit only if a + pathological single-giant-block document surfaces. + +--- + +## 10. Glossary & References + +- **Block** — the unit `splitBlocks` recognizes: a prose paragraph, header, or + bullet/list item; a code or mermaid fence (atomic, INV-23). The #47 decision + unit. +- **Block proposal** — an F4 proposal whose anchor spans a whole block and whose + replacement is Claude's whole version of that block (`granularity: "block"`). +- **Intra-block sub-diff** — the word-level `diffToHunks` run *inside* an accepted + block to realize word-precise attribution (INV-40). +- **`diffToHunks` / `diffToBlockHunks`** — word-level (legacy INV-37; now the + intra-block engine) / block-level (INV-39, new) document-rewrite differs, both + emitting `EditHunk[]`. +- **References** — `coauthoring-propose-accept.md` (F4 seam, INV-9..13), + `coauthoring-interactive-review.md` (F10 preview, INV-32..34), + `coauthoring-attribution.md` (F3, `applyAgentEdit`/`spansFor`), + `issues/preview-toolbar-interaction-surface.md` (F11, INV-35..37 — un-graduated), + features `#42` / `#47` / `#46`, capture sessions `0035` / `0040` / `0039`, + brainstorming session `0041`. Lineage `ben.stull/rfc-app#48`. diff --git a/specs/coauthoring-inline-editor-diff.md b/specs/coauthoring-inline-editor-diff.md new file mode 100644 index 0000000..d45300c --- /dev/null +++ b/specs/coauthoring-inline-editor-diff.md @@ -0,0 +1,445 @@ +--- +status: graduated +--- +# Solution Design: Inline editable proposed-change diff in the Markdown editor (#64) + +| | | +| --- | --- | +| **Author(s)** | Ben Stull (with Claude) | +| **Reviewers / approvers** | Ben Stull | +| **Status** | `graduated` | +| **Version** | v0.1.1 | +| **Source artifacts** | Feature `benstull/vscode-cowriting-plugin#64` (Inline editable proposed-change diff in the Markdown editor + Accept/Reject controls in both surfaces, `type/feature`, `priority/P2`) · Brainstorming session `vscode-cowriting-plugin-0058` (2026-06-26) · Builds on (all shipped): F4 `#12` (propose/accept seam, INV-9/10/11/13), F6 `#17` (baseline / machine-landing), F7 `#21` (rendered preview, pure render engine INV-22), F10 `#29` (interactive review preview — the **clean-editor** decision INV-32; ✓/✗ accept-reject), F11 `#43` (preview toolbar, `data-src` block→source mapping INV-36), document-edit-flow `#42/#47/#46` (block proposals INV-39/40, accept-all INV-42) · Parent specs (graduated): `coauthoring-propose-accept.md`, `coauthoring-diff-view.md`, `coauthoring-rendered-preview.md`, `coauthoring-interactive-review.md`, `coauthoring-document-edit-flow.md` · Lineage: `ben.stull/rfc-app#48` | + +**Change log** + +| Date | Version | Change | By | +| --- | --- | --- | --- | +| 2026-06-26 | v0.1.1 | Implementation refinement (planning-and-executing session 0059): made the **re-anchor** explicit. Optimistic apply (§3.2) must store the pre-apply text on a new `Proposal.original` field **and** re-fingerprint the proposal's anchor to the *applied* text — F4's `fp.text` is the *original* target, which leaves the buffer once the replacement lands, so `resolve()` would orphan every applied proposal otherwise. `finalizeInPlace`/`revertInPlace`/decorate all key off the re-anchored fp; revert restores `original`. Reload-safety (INV-51/54): a proposal already carrying `original` is never re-captured (the in-memory applied-set is empty after a window reload), so the revert target survives save+reload. Shipped #64 (PR, session 0059). | +| 2026-06-26 | v0.1.0 | Initial draft — brainstorming session 0058. Four forks locked with the operator: **(1) editor model** = *optimistic apply + decorations* — on propose, the editor buffer becomes the would-be-accepted text (insertions real & editable & tinted; deletions shown as struck-red non-editable hints); accept = finalize-in-place, reject = revert. **(2) timing** = *on proposal* (turn complete), not a live token-stream into the editor (that stays in #60's notification/OutputChannel). **(3) editor affordance** = CodeLens `Accept ▾ / Reject ▾` above each block, `▾` → QuickPick (this / all); the webview keeps HTML buttons with the same dropdown. **(4) controls parity** = Accept / Reject / Accept-all / Reject-all reachable from **both** the editor and the webview. Two sub-decisions confirmed: a **dedicated `EditorProposalController`** owns optimistic-apply + decorations + CodeLens (keeps `ProposalController` the pure F4 state/seam owner); **saving while pending persists** the proposed (accepted-result) text. Reverses INV-32 and INV-10; supersedes the ✓/✗ glyph controls. | Ben Stull + Claude | + +--- + +## 1. Business Context + +### 1.1 Executive Summary + +Today, when Claude proposes an edit, the change is shown **only in the rendered +review webview** (`oldnew` with ✓/✗ controls); the Markdown +**editor stays deliberately clean** (F10 / INV-32 — "the preview is the single +review surface"). The writer who lives in the editor sees nothing: to review what +Claude proposed they must open the preview panel, and they cannot *edit* the +proposed text in place — they can only accept or reject it wholesale in the +webview. + +This design brings the proposed change **into the Markdown editor itself**: +editable, with a track-changes diff that matches the webview and the post-accept +result **exactly**. It also replaces the per-proposal `✓`/`✗` glyphs with labelled +**Accept** / **Reject** controls — each carrying a `▾` dropdown for **Accept all** +/ **Reject all** — and makes all four actions reachable from **both** the editor +and the webview. The writer can now read, tweak, and resolve a proposal without +leaving the document. + +### 1.2 Background + +The plugin's review model (graduated specs `coauthoring-propose-accept.md`, +`coauthoring-rendered-preview.md`, `coauthoring-interactive-review.md`): + +- A machine edit becomes an **F4 proposal** — a pending-only record + (`Proposal{ id, anchorId, replacement, granularity }` + a `Fingerprint` anchor + whose `text` is the exact target) that **never mutates the document** (INV-10), + re-resolved against the live text at accept (INV-11) and cleared when + accepted/rejected (INV-13). +- The **render engine** (`trackChangesModel.ts`) is a pure, deterministic, + `vscode`-free unit (INV-22): it diffs at **block** granularity (INV-39), word- + merges prose blocks, and emits webview HTML — `renderReview` (annotated) / + `renderPlain` (clean) — with proposal blocks (`proposalBlockHtml`) carrying the + ✓/✗ buttons and `data-src` offsets for selection mapping (INV-36). +- The **F10 decision (INV-32)** removed *all* in-editor decorations + (`grep` confirms zero `TextEditorDecorationType` / `setDecorations` in the tree) + so the webview is the sole review surface; `ProposalController` carries the note + *"no in-editor UI — INV-32 makes the rendered preview the single review surface."* +- The sealed webview is **intent-only** (INV-35): it posts `{accept|reject| + acceptAll|…}` to the host, which routes to `ProposalController.acceptById` / + `rejectById` / `acceptAllProposals` (#46 / INV-42). Accept **applies** the + replacement via the F4 seam (`applyAgentEdit`), which advances the F6 baseline + (machine-landing, INV-18). + +This feature **deliberately reverses two of those decisions** for the editor +surface, and supersedes the ✓/✗ glyph controls in both surfaces. + +### 1.3 Who feels it & why + +The **human coauthor working in the editor** — the writer who asked Claude to edit +and wants to see, adjust, and resolve the result without context-switching to the +preview panel. The pain: the proposed change is invisible in the place they are +actually writing, and it is not editable at all (only accept/reject-able). P2 +(issue #64): it materially deepens the inner-loop "edit-in-place" experience and +unifies the review controls, but the webview review path already works, so it is +an enhancement rather than a gap. + +--- + +## 2. Product Design + +### 2.1 The experience + +When Claude's turn lands a proposal (or, for a document edit, N block proposals), +the **editor** shows the change as track-changes, in place: + +``` + Accept ▾ Reject ▾ ← CodeLens, above each proposed block + The quick ~~brown~~ red fox jumps over the lazy dog. + └ struck red ┘└ green/blue, editable ┘ +``` + +- **Insertions** are **real, editable buffer text**, tinted by origin + (green = human-origin, blue = LLM) — the writer can click in and edit them like + any other text. +- **Deletions** appear as a **struck-red, non-editable hint** rendered adjacent to + the insertion (a decoration, not buffer text) — visually mirroring the webview's + ``. +- The buffer content **is exactly the would-be-accepted result**, so what the + writer sees (and can edit) is precisely what accepting produces. + +Above each proposed block sit two **CodeLens** actions — **`Accept ▾`** and +**`Reject ▾`**. Clicking opens a small QuickPick: + +``` + Accept ▾ → ┌───────────────────────┐ Reject ▾ → ┌───────────────────────┐ + │ Accept this proposal │ │ Reject this proposal │ + │ Accept ALL proposals │ │ Reject ALL proposals │ + └───────────────────────┘ └───────────────────────┘ +``` + +The **webview** shows the same proposal with the same diff, and its controls are +the parallel HTML buttons — **`Accept ▾`** / **`Reject ▾`** — where the `▾` +reveals *Accept all* / *Reject all*. The legacy ✓/✗ glyphs are **replaced** by +these labelled controls in both surfaces. + +All four actions — **Accept**, **Reject**, **Accept all**, **Reject all** — are +reachable from **either** surface and route to the same controller logic +(INV-53). + +### 2.2 Accept / Reject semantics (editable-in-place) + +- **Accept** finalizes the change *that is already in the buffer*: it records the + span's attribution as landed, advances the F6 baseline (machine-landing), and + clears the proposal. It does **not** re-apply text (the text is already there) — + see §3.3. +- **Reject** reverts that block's region back to the stored original + (`Fingerprint.text`), and clears the proposal. +- **If the writer edited the inserted text before deciding:** **Accept** keeps + their edited text (their keystrokes layer on as human authorship over the + proposed span); **Reject** still reverts the whole block to the original. +- **Accept all / Reject all** operate on the current document's pending proposals, + in descending anchor order, skipping orphans (the #46 / INV-42 shape; **Reject + all is new**). + +### 2.3 Save semantics + +Because the proposed text is *in the buffer*, **saving while a proposal is pending +persists the proposed (accepted-result) text** to disk. The deletion hints are +decorations (never buffer text), so the **saved file is clean** — it contains the +accepted-result text, not yet "finalized" only in the sense of attribution / +baseline. "Pending" therefore means *provisional attribution, baseline not yet +advanced* — **not** "document unchanged" (INV-54). We deliberately **allow** the +save rather than block it or strip on save; once the editor shows the live +document, blocking saves would be the surprising behavior. + +### 2.4 What this is *not* (non-goals) + +- **Not live token-streaming into the editor.** The diff appears when the turn + produces a proposal; watching Claude type into the document is out of scope (the + live token stream is #60's notification + OutputChannel). +- **Not a new review *model*.** F4 proposals, anchoring (INV-11), block + granularity (INV-39/40), accept-all (INV-42) are reused; this adds an editor + *surface* and unifies the controls. +- **Not non-Markdown.** Same Markdown-gated scope as the rest of the review UI. +- **Not intra-diagram mermaid editing.** Atomic fences stay atomic (INV-23); a + mermaid block proposal is editable as its whole fence, not sub-diagram. +- **Not removing the webview.** The rendered preview remains a full review surface; + it gains label/dropdown parity, not a demotion. + +### 2.5 Surfaces considered & rejected + +| Editor approach | Why not | +| --- | --- | +| **Read-only decoration overlay** (doc unchanged; ghost insertions via decoration) | The inserted text would **not be editable** — fails the core "human-editable" requirement. | +| **Inline track-changes markup in the buffer** (`{~~old~~|++new++}`) | Pollutes the file on disk with markup until resolved; complex; conflicts with clean save. | +| **Native `vscode.diff` two-pane** | The two-pane diff was *removed* in #34 (F10) precisely to make one review surface; a separate diff editor is not "in the Markdown file." | + +Chosen: **optimistic apply + decorations** — the only model that yields an +editable diff that is *exactly* the accepted result, in the document itself. + +--- + +## 3. Engineering Design + +### 3.1 Architecture + +``` + ┌──────────────────────────────────────────────┐ + one F4 turn ───▶ │ ProposalController (F4 state/seam owner) │ + (#12 propose) │ propose() · finalizeInPlace() · revertInPlace│ + │ · rejectAll() · onDidChangeProposals │ + └───────────────┬──────────────────────────────┘ + │ proposals (pending records + anchors) + ┌─────────────────────┴───────────────────────┐ + ▼ ▼ + ┌───────────────────────────┐ ┌────────────────────────────┐ + │ EditorProposalController │ shared │ TrackChangesPreview- │ + │ (NEW, vscode host) │ pure diff │ Controller (webview) │ + │ · optimistic buffer write │◀──────────────▶ │ · renderReview HTML │ + │ · insertion tint deco │ trackChangesModel│ · Accept▾/Reject▾ buttons │ + │ · deletion-hint deco │ (diff → plan) │ · dropdown → acceptAll/ │ + │ · CodeLens Accept▾/Reject▾│ │ rejectAll │ + └───────────────────────────┘ └────────────────────────────┘ + ▲ ▲ + └──────────── both route actions to ───────────┘ + ProposalController.{finalizeInPlace,revertInPlace, + acceptAll,rejectAll} +``` + +The decisive rule: **one pure diff is the single source of truth** for both +surfaces (INV-49). The editor renders it as *buffer text + decorations + CodeLens*; +the webview renders it as *HTML*. They cannot diverge because they consume the +same hunks. + +### 3.2 Optimistic apply (on propose) — INV-48 + +`propose()` gains a buffer-write step, owned by `EditorProposalController` (so +`ProposalController` stays the pure state owner): + +1. Record the pending proposal(s) + anchor(s) **exactly as today** (the sidecar + record is unchanged: `replacement`, `Fingerprint{text,before,after,lineHint}`, + `granularity`). +2. Write the proposed text into the **live editor buffer**: replace the resolved + anchor span with `replacement` (for a document turn, this makes the buffer + Claude's full proposed document; for a selection turn, the single span). This + write **does not advance the F6 baseline** — the baseline stays at the + pre-proposal text, so the change reads as *pending*, not *landed*. +3. Compute the **decoration plan** from the same per-block word/line diff used by + `acceptBlock` (`wordEditHunks(fp.text, replacement)`): insertion segments → + tinted ranges (buffer coords, offset by the live anchor); deletion segments → + struck-red hint injections (decoration `before`/`after` content) at the segment + boundary. + +Critically, the optimistic write must **not** fire the F4 seam's +`onDidApplyAgentEdit` (which would advance the baseline / register a landed edit). +Optimistic apply is a *distinct* buffer-mutation path from accept; only **accept** +advances the baseline (§3.3). This is the load-bearing distinction that keeps the +change *pending* while *present*. + +### 3.3 Accept = finalize-in-place; Reject = revert-in-place — INV-51 + +Because the proposed text is already in the buffer, the old "accept ⇒ apply via +seam" path would **double-apply**. The accept/reject paths are reworked: + +- **`finalizeInPlace(id)`** — resolve the proposal's anchor against the live text; + record the span's attribution as **landed** (provenance from the proposal's + `author`, word-precise per INV-40 for block granularity); **advance the F6 + baseline** for that region (the machine-landing the seam used to do on apply); + `removeProposal(id)`. No text is re-applied. If the writer edited the inserted + span, the *current* buffer text is what gets finalized (their edits become + layered human authorship). +- **`revertInPlace(id)`** — resolve the proposal's live span; replace it with the + stored original `Fingerprint.text`; `removeProposal(id)`. Reverts the whole + block regardless of any in-place edits. +- **`acceptAll()` / `rejectAll()`** — iterate the current document's pending + proposals in **descending** anchor order (so earlier resolutions don't shift + later offsets), skipping orphans with a tally (INV-42 shape). `acceptAll` reuses + the #46 batch ordering; **`rejectAll` is new** and symmetric. + +Both surfaces (editor CodeLens/QuickPick and webview buttons/dropdown) call these +**same four methods** (INV-53). The webview's existing intent messages are renamed/ +extended (`accept`/`reject`/`acceptAll` + new `rejectAll`) but stay intent-only +(INV-35). + +### 3.4 Single diff source & render-once — INV-49 / INV-50 + +- **INV-49 (single diff source).** The per-proposal diff (block hunks → word/line + edit hunks) is produced by one pure function in `trackChangesModel.ts` (extends + the existing `diffToBlockHunks` / `wordEditHunks`, INV-39/40). The webview HTML + path and the editor decoration-plan path both consume it. Same proposal ⇒ + identical diff in both surfaces (this is the testable parity guarantee). +- **INV-50 (render once).** With optimistic apply, the baseline→buffer delta now + *contains* the proposed change. The renderer must attribute any baseline→buffer + delta that is covered by a **pending proposal** to that proposal (rendered as a + proposal block), and must **not** also render it as an already-landed + machine-diff. Mechanism: the webview render reconciles baseline-diff spans + against pending-proposal anchors; a span covered by a pending proposal is + rendered exactly once, as the proposal. *(This reconciliation is the primary + implementation risk; the implementation plan owns the exact span-mapping, but the + invariant — rendered once — is fixed here.)* + +### 3.5 Editor decorations & CodeLens (the editor surface) + +`EditorProposalController` (new, `vscode` host module) owns: + +- **Two `TextEditorDecorationType`s** — an *insertion tint* (green/blue background, + by provenance) over real buffer ranges, and a *deletion hint* (a + `before`/`after` render-option carrying the struck original text, red, + strikethrough, non-editable). Both are recomputed from the decoration plan on + every `onDidChangeProposals` and on active-editor change. +- **A `CodeLensProvider`** — for each pending proposal whose anchor resolves in the + active document, emit two lenses positioned above the block: `Accept ▾` + (command `cowriting.proposalAcceptMenu` with the proposal id) and `Reject ▾` + (`cowriting.proposalRejectMenu`). Each command opens a `vscode.window + .showQuickPick(["… this proposal","… ALL proposals"])` and dispatches to + `finalizeInPlace`/`acceptAll` or `revertInPlace`/`rejectAll`. +- **Lifecycle wiring** — subscribes to `proposals.onDidChangeProposals` (the same + event the preview uses) and `window.onDidChangeActiveTextEditor`; clears its + decorations + lenses for documents with no pending proposals (so a clean editor + stays clean — INV-32's spirit holds *when there is nothing pending*). + +Deletion hints are **decoration-only** (never buffer text, never saved) and +insertion tints decorate **real** buffer text (INV-52). + +### 3.6 Webview controls (label + dropdown parity) + +`proposalBlockHtml` (`trackChangesModel.ts`) swaps the ✓/✗ glyph buttons for an +**`Accept ▾`** / **`Reject ▾`** control pair. The `▾` opens a small CSS/JS dropdown +in the sealed webview (`preview.ts`/`preview.css`, no network — INV-21) offering +*Accept all* / *Reject all*; selecting posts the corresponding intent +(`acceptAll` / `rejectAll`) or the per-proposal `accept`/`reject`. The host routes +all four to the controller methods of §3.3 (INV-35 preserved). + +### 3.7 Components & files + +| File | Change | +| --- | --- | +| `src/editorProposalController.ts` | **NEW** — optimistic apply, decoration types, deletion-hint injection, `CodeLensProvider`, QuickPick menus; subscribes to `onDidChangeProposals` + active-editor change. | +| `src/proposalController.ts` | Add `finalizeInPlace(id)`, `revertInPlace(id)`, `rejectAll(docKey)`; `propose()` delegates the optimistic buffer write (so the controller stays state-pure); accept/reject routing now finalizes/reverts in place (no seam re-apply). | +| `src/trackChangesModel.ts` | Factor the shared per-proposal diff into one pure producer feeding both surfaces; reconcile pending-proposal spans in `renderReview` (INV-50); swap ✓/✗ → `Accept ▾`/`Reject ▾` in `proposalBlockHtml`. | +| `src/trackChangesPreview.ts` | Route new `rejectAll` intent; keep `accept`/`reject`/`acceptAll`; relabel toolbar/proposal controls. | +| `src/preview.ts` / `preview.css` | Dropdown control for `Accept ▾`/`Reject ▾`; same diff CSS reused. | +| `src/extension.ts` | Register `EditorProposalController`, its CodeLens provider, and the menu commands; wire it alongside `ProposalController`/`TrackChangesPreviewController`. | +| `package.json` | Add `cowriting.rejectAllProposals`, `cowriting.proposalAcceptMenu`, `cowriting.proposalRejectMenu`; CodeLens contribution; markdown-gated `when` clauses. | + +### 3.8 Invariants + +- **INV-48 — Optimistic apply (with re-anchor).** On propose, the proposed text is + written into the live editor buffer (the buffer becomes the would-be-accepted + result); this write does **not** advance the F6 baseline and does **not** fire the + F4 machine-landing seam. Because F4's `fp.text` is the *original* target — which + leaves the buffer once the replacement lands — optimistic apply **stores the + pre-apply text on `Proposal.original` and re-fingerprints the anchor to the applied + text**, so `resolve()` keeps finding the proposal in the mutated buffer; finalize / + revert / decorate all key off the re-anchored fp, and revert restores `original`. + `original` is captured **exactly once** (first apply): a proposal that survives a + save+reload already carries it, so a fresh session never re-captures it from the + already-applied buffer (reload-safety, INV-51/54). **Reverses INV-10** (propose no + longer leaves the document untouched) and **INV-32** (the editor is no longer kept + unconditionally clean — it shows pending proposals). +- **INV-49 — Single diff source.** Editor decorations and webview HTML both derive + from one pure, deterministic per-proposal diff (extends `diffToBlockHunks` / + `wordEditHunks`, INV-22/39/40). The same proposal yields the same diff in both + surfaces. +- **INV-50 — Rendered once.** A span covered by a pending proposal is rendered + exactly once — as that proposal — never additionally as an already-landed + baseline diff. +- **INV-51 — Finalize / revert in place.** Accept finalizes the span already in the + buffer (record attribution, advance baseline, clear proposal) without + re-applying text; Reject reverts the block region to the stored + `Fingerprint.text`. No double-apply. +- **INV-52 — Decoration roles.** Deletion hints are decoration-only (non-editable + `before`/`after` content, never buffer text, never saved); insertion tints + decorate real, editable buffer text. +- **INV-53 — Control parity.** Accept / Reject / Accept-all / Reject-all are + reachable from **both** the editor (CodeLens + QuickPick) and the webview + (buttons + dropdown) and route to the same `ProposalController` methods; the ✓/✗ + glyph controls are superseded by labelled `Accept ▾`/`Reject ▾`. +- **INV-54 — Save persists pending.** Saving while a proposal is pending persists + the proposed (accepted-result) text; "pending" denotes provisional attribution / + un-advanced baseline, not document divergence. (Decorations are not persisted, so + the saved file is clean text.) + +### 3.9 Error & edge handling + +- **Orphaned proposal** (anchor no longer resolves) — no editor decorations/lenses + for it; it still surfaces in the webview at end with a dashed border (INV-34); + accept-all/reject-all skip it with a tally (INV-42). +- **Writer edits the inserted span, then accepts** — current buffer text is + finalized (their edits layer as human authorship); **then rejects** — block + reverts to original regardless. +- **External / concurrent edit shifts geometry** — anchors re-resolve on + `onDidChangeProposals` (existing resolve-or-flag); decoration plan + lenses + recompute. +- **Multiple proposals from one turn** — all optimistically applied (buffer = full + proposed document); each block independently finalizable/revertible; descending + order on accept-all/reject-all keeps offsets valid. +- **Reject of an in-buffer span after partial accept of siblings** — each + proposal's revert uses its own live-resolved span, independent of siblings. +- **Non-authorable document** (read-only / not on disk) — controls disabled exactly + as today (`authorable` gating). + +--- + +## 4. Testing & E2E + +Per the §9 pipeline and `coauthoring-*` precedent (this is a VS Code extension — +**no flotilla/PPE**; the gate is unit + host-E2E green). + +- **Unit (the core):** + - The shared per-proposal diff producer: same hunks → insertion/deletion + segments; parity assertion that the decoration plan and the webview HTML derive + identical add/del spans for the same proposal (INV-49). + - `renderReview` render-once reconciliation: a pending-proposal span is emitted + as a proposal block and **not** as a landed baseline diff (INV-50). + - Decoration-plan computation from `wordEditHunks(fp.text, replacement)`: + insertion ranges in buffer coords; deletion-hint positions (INV-52). +- **Host E2E:** + - Propose (via the injectable `editTurn` stub) → editor shows insertion tint + + deletion hint + `Accept ▾`/`Reject ▾` CodeLens; buffer equals the accepted + result (INV-48). + - Edit the inserted text → **finalizeInPlace** keeps the edited text; baseline + advanced; proposal cleared (INV-51). + - **revertInPlace** restores `Fingerprint.text`; proposal cleared. + - Accept / Reject / Accept-all / Reject-all from **both** the editor command path + and the webview intent path produce the same end state (INV-53); `rejectAll` + clears all pending and reverts all blocks. + - Save while a proposal is pending → file on disk has the proposed text; no + decoration markup in the saved bytes (INV-54). + - Webview/editor parity: same proposal renders the same diff in both (INV-49). +- **Manual smoke:** ask Claude to edit a paragraph; confirm the editor diff is + editable, the deletion hint reads correctly, both surfaces' controls resolve it, + and a clean editor returns once nothing is pending. + +--- + +## 5. Delivery Plan (rollout — not a task list) + +One increment, single design-then-build (#64). Ships through branch → PR → `main`; +no migration and no new persisted sidecar shape (the `Proposal` record is +unchanged). The implementation plan (downstream `wgl-planning-and-executing` +session) owns the Task breakdown; a natural cut: + +1. **Shared diff + render-once** — factor the pure per-proposal diff producer; + reconcile pending spans in `renderReview` (INV-49/50) + unit tests. +2. **Accept/reject rework** — `finalizeInPlace` / `revertInPlace` / `rejectAll` on + `ProposalController` (no seam re-apply); unit tests. +3. **Editor surface** — `EditorProposalController`: optimistic apply, decorations, + deletion hints, CodeLens + QuickPick menus; wire in `extension.ts`. +4. **Control parity** — relabel ✓/✗ → `Accept ▾`/`Reject ▾` + dropdown in the + webview; route `rejectAll`; `package.json` commands/`when`. +5. **Host E2E + manual smoke** across both surfaces. + +Reversible by reverting the PR (restores the clean-editor / ✓-✗ webview behavior). + +--- + +## 6. Open Questions + +- **OQ-1 — Provenance tint exactness.** Insertion tint uses green=human-origin / + blue=LLM by the proposal `author`; whether to additionally word-tint *within* a + block by the F3 attribution of edited-in-place text (vs a single block tint) can + follow the existing F9/F10 coloring; v1 tints the proposed insertion by the + proposal author. *(Not blocking.)* +- **OQ-2 (inherited)** — The F11 design (`#43`) remains un-graduated (lives in code + + an issue draft); unrelated to #64 but noted in the shared lineage. Track + separately. +- **OQ-3 — Editor dropdown fidelity.** The editor uses a QuickPick to stand in for + the webview's true `▾` dropdown (CodeLens cannot render a caret menu inline); if + a more button-like affordance is wanted later, a webview-style overlay is a + possible increment. *(Accepted for v1.)* diff --git a/specs/coauthoring-interactive-review.md b/specs/coauthoring-interactive-review.md new file mode 100644 index 0000000..1280e6e --- /dev/null +++ b/specs/coauthoring-interactive-review.md @@ -0,0 +1,609 @@ +--- +status: graduated +--- +# Solution Design: Interactive Track-Changes Review in the Markdown Preview (F10) + +| | | +| --- | --- | +| **Author(s)** | Ben Stull (with Claude) | +| **Reviewers / approvers** | Ben Stull | +| **Status** | `draft` | +| **Version** | v0.1.0 | +| **Source artifacts** | Feature `benstull/vscode-cowriting-plugin#29` (F10, `type/feature`, `priority/P1`) · Epic `#1` (closed) · Capture session `vscode-cowriting-plugin-0028` (2026-06-11) · Brainstorming session `vscode-cowriting-plugin-0029` · Builds on (all shipped): F3 `#6` (live attribution), F4 `#12` (propose/accept), F6 `#17`/`#19` (baseline + diff view), F7 `#21`/`#22` (rendered preview + intra-diagram mermaid), F9 `#27` (authorship preview) · Parent specs (graduated): `coauthoring-inner-loop.md`, `coauthoring-attribution.md`, `coauthoring-propose-accept.md`, `coauthoring-diff-view.md`, `coauthoring-rendered-preview.md`, `2026-06-11-authorship-preview-design.md` · Lineage: `ben.stull/rfc-app#48` | + +**Change log** + +| Date | Version | Change | By | +| --- | --- | --- | --- | +| 2026-06-11 | v0.1.0 | Initial draft — brainstorming session 0029 (from the capture in session 0028, which followed the session-0027 ideation where the operator found the F6 two-pane diff + F3 attribution overlap confusing). | Ben Stull + Claude | + +--- + +## 1. Business Context + +### 1.1 Executive Summary + +The plugin shipped its review signals as **several overlapping surfaces that +collide on the same text**: F6's two-pane red/green raw diff (`ctrl+alt+d`), F3's +in-editor attribution decorations (green human border / blue Claude fill), F4's +in-editor amber proposal threads, and F7/F9's rendered preview (two modes). In +manual testing the operator opened the F6 diff and saw F3's blue attribution +bleeding into it at the same time — *"I don't understand this."* + +F10 resolves this into **one mental model: write on the left, review on the +right.** The editor becomes a **clean, zero-annotation** markdown pane. The +**rendered preview becomes the single review surface**, with an **annotations +on/off** toggle. With annotations **on**, the preview paints every change since +the F6 baseline in one visual language — **green = human-authored, blue = +LLM-authored, strikethrough = deleted** — and lets the human **accept (✓) or +reject (✗) Claude's pending proposals, and only those, right inside the +preview.** F10 is **mostly assembly** of shipped features (F3/F4/F6/F7/F9) plus +three new pieces: strip the editor decorations, collapse F9's two preview modes +into the one toggle, and make the preview interactive. + +### 1.2 Background + +The inner loop shipped F2–F5 (threads · attribution · propose/accept · +cross-rung). F6 added the baseline + a native two-pane diff toggle (`#17`), +broadened to any file (`#19`). F7 added the rendered track-changes preview +(`#21`) with full mermaid support, refined by F7.1 intra-diagram mermaid diffing +(`#22`). F9 added an **Authorship** mode to that preview (`#27`), switched by a +segmented `[ Track changes | Authorship ]` header control — and F9 **explicitly +chose two separate modes, never combined** (F9 INV-26). + +The friction the operator hit is the *multiplicity* of surfaces, not any one of +them: the two-pane raw diff is low-altitude for prose; the in-editor decorations +clutter the writing surface (the human authored most of the document, so +attribution paints everywhere); and there is no single place to answer "what +changed, who changed it, and do I keep the LLM's edits?" Capture session 0028 +filed Feature `#29` locking the product vision (markdown-only; clean editor; +preview as the single review surface). This spec is the Solution Design for it. + +### 1.3 Business Actors / Roles + +- **Coauthor (human)** — the writer/engineer (PP-1); F10's sole user. +- **Coauthor (machine)** — Claude via `@cline/sdk`; not a user of F10, but its + proposals are what the human reviews in the preview, and its landings advance + the F6 baseline. + +### 1.4 Problem Statement + +A markdown writer reviewing a coauthoring session has **no single, intuitive +surface** that answers "what changed since the last coauthoring moment, who +changed it, and do I keep Claude's edits?" The signal is fragmented across a raw +two-pane diff, in-editor decorations, and a two-mode preview that shows changes +**or** authorship but never both, and never lets the human act on Claude's +proposals from inside the rendered view. + +### 1.5 Pain Points + +- **Surface collision** — F6's raw diff and F3's attribution paint the same text + simultaneously and read as noise ("I don't understand this"). +- **Cluttered editor** — F3 attribution + F4 proposal threads decorate the + writing pane; since the human authored most of the document, attribution paints + nearly everywhere. +- **Two-pane raw diff is low-altitude for prose** — you read markdown source, not + the rendered document. +- **Changes XOR authorship** — F9's preview shows *what changed* (track-changes) + or *who authored* (authorship) but never both in one glance, and neither lets + you accept/reject from the rendered view. + +### 1.6 Targeted Business Outcomes + +One clean model: **write on the left, review on the right.** The editor is +distraction-free (zero annotations). The preview, with annotations on, shows +every change since the baseline in one visual language **and** lets the human +keep or revert Claude's proposals without leaving that pane — so "what did the +LLM change, and do I want it?" is answered **in place**, in the rendered +document, not in a raw diff or a cluttered editor. + +### 1.7 Scope (business) + +**In scope:** a clean (zero-annotation) markdown editor; the rendered preview as +the **single** review surface; an **annotations on/off** toggle (off = clean +rendered markdown, built-in-preview parity; on = the combined annotated view); +the unified per-author + strikethrough annotation language (green human / blue +LLM / struck deletions, reconstructed from the F6 baseline); **interactive +accept/reject of Claude's pending proposals from inside the preview** (✓/✗); +deprecating F6's two-pane view (command + keybinding hidden) and stripping the +editor's F3/F4 decorations; unit + host-E2E coverage; manual webview smoke. + +**Out of scope (deferred, not forgotten):** **non-markdown review** (F6's +any-file reach `#19` is dropped *as a user goal* — see §6.7); the **repo rename** +to `vscode-markdown-cowriting-plugin` (the markdown-only identity is affirmed, +but the rename — Gitea repo, `app.json`, content repo, session-history paths — is +a separate deliberate gesture); preview→source **scroll-sync**; export / print / +copy-as-clean; **editing in the preview** beyond the ✓/✗ proposal controls; +intra-emphasis sentinel hardening (the F9 deferred refinement). + +**Non-goals (firm):** any change to the **sidecar** persistence, the **F4 +propose/accept model** (proposals stay pending-by-default — F10 surfaces and acts +on them, it does **not** change how Claude's edits enter the system), the +**cross-rung contract**, or `SCHEMA_VERSION`; a general-purpose markdown +previewer; a WYSIWYG editor; combining anything with the LLM/network/credential +surface (INV-8 untouched). + +### 1.8 Assumptions · Constraints · Dependencies + +- **Anchor:** Feature `#29` (F10). **Mostly assembly** of shipped features: + - **F6 baseline** (`#17`/`#19`) — kept as the **data layer** ("what changed + since when"; auto-advances at every machine landing, INV-18); F6's two-pane + *view* is deprecated as a user surface (command/keybinding hidden, code kept). + - **F7 render/diff engine** (`#21`/`#22`) — the rendered preview, block/word + diff, strikethrough deletions, atomic code/mermaid, and F7.1 intra-diagram + mermaid diff already exist and are reused. + - **F3 attribution** (`#6`) — kept as the **data layer** (`spansFor` — who + authored each span); its *in-editor decorations* are removed. + - **F4 propose/accept** (`#12`) — the accept/reject *logic* + (`ProposalController.accept`/`reject` → `applyAgentEdit`, the INV-9 + `WorkspaceEdit` seam) already exists; F10 moves the *controls* into the + preview and removes the in-editor proposal threads. + - **F9 authorship preview** (`#27`) — its two preview modes **collapse** into + the one annotations on/off toggle; its PUA-sentinel author-coloring technique + is **reused** inside the new combined render; its standalone + `renderAuthorship` / segmented control are **superseded** (removed). +- **Constraint:** the preview is a **sealed webview** (strict CSP, local assets, + no network — F7 INV-21). Interactive ✓/✗ controls post messages to the + extension host, which applies/discards proposals via the F4 path; the webview + **never** edits the document directly. +- **Constraint:** deletions aren't in the live buffer — they are reconstructed by + diffing the buffer against the F6 baseline (the F7 engine already does this). +- No LLM, no network, no new credential surface (INV-8 untouched); nothing + persisted changes (`.threads/`, the contract, `SCHEMA_VERSION` all untouched). + +### 1.9 Business Use Cases + +- **BUC-1 (unified review)** Mid-session the writer opens the preview and sees the + rendered chapter with their own additions in green, their deletions struck, and + Claude's pending proposals as blue blocks carrying ✓/✗ — one glance answers + "what changed, who, and do I keep Claude's edits?" +- **BUC-2 (act in place)** The writer clicks ✓ on a Claude proposal in the + preview; the proposal lands in the document, the baseline advances, and the + blue block becomes ordinary (unmarked) text — without leaving the preview or + touching the editor. +- **BUC-3 (clean writing)** The writer works in the editor, which shows **no** + coauthoring decorations at all — a plain markdown surface — and flips + annotations **off** in the preview to read the document clean (built-in-preview + parity) when they want to. + +--- + +## 2. Solution Proposal + +The editor is stripped of **all** coauthoring decorations (F3 attribution tint, +F4 proposal threads), and F6's two-pane diff command/keybinding are hidden +(`when:false`); the F6 baseline store and `onDidChangeBaseline` event remain as +the data layer. + +The preview's per-panel mode collapses from F9's `"changes" | "authorship"` to a +single **`"on" | "off"`** annotations toggle (default **on**): + +- **Off** — plain `markdown-it` render of the current buffer, no marks (parity + with VS Code's built-in markdown preview). +- **On** — a **combined annotated render** produced by one new **pure, + vscode-free** engine, `renderReview(baselineText, currentText, authorSpans, + proposals)`, that overlays **two axes in a single pass**: + 1. **Changes since the F6 baseline** — the F7 block + word diff of + `baselineText` vs `currentText`: additions and deletions, **author-colored** + via F3 `authorSpans` (human → green, agent → blue) using F9's PUA-sentinel + technique; deletions rendered **struck** (reconstructed from the baseline). + 2. **Pending Claude proposals** — each F4 pending proposal rendered **inline at + its resolved anchor** as a **blue block** (the replaced text struck, the + proposed text shown) carrying a `data-proposal-id` and **✓/✗ buttons**. + +The webview becomes **interactive**: clicking ✓/✗ posts `{type: "accept" | +"reject", proposalId}` to the host, which calls `ProposalController.accept` / +`reject` — accept runs the existing `applyAgentEdit` `WorkspaceEdit` seam and +advances the baseline (INV-18), reject discards the proposal; either way the host +re-renders. The webview never mutates the document (INV-20/21 hold). Everything +downstream of `(baseline, current, spans, proposals) → HTML` is a pure function +(INV-33), unit-testable with no vscode and no webview. + +Because proposals are now **preview-only**, a small **status-bar indicator** +("N Claude proposals — open review", clickable to open the preview) signals +pending proposals when no preview is open (§6.5 PUC-6). + +--- + +## 3. Product Personas + +- **PP-1 Inner-loop coauthor** — the human markdown writer/engineer (as F2–F9); + the only persona F10 serves. + +## 4. Product Use Cases + +- **PUC-1 (clean editor)** Opening / editing a markdown document shows a plain + editor with **no** coauthoring decorations (no F3 tint, no F4 threads, no F6 + diff). The writing surface is distraction-free. +- **PUC-2 (toggle annotations)** In the preview, a header switch flips + annotations **on** (combined annotated render) / **off** (clean rendered + markdown). Default on. Mode is remembered per panel. +- **PUC-3 (see who changed what)** With annotations on, the writer's additions + since the baseline render **green**, their deletions render **struck**, and + Claude's pending proposals render as **blue** blocks. One visual language. +- **PUC-4 (accept a proposal)** The writer clicks **✓** on a blue proposal → it + lands in the document via the F4 seam → the baseline advances → the block + re-renders as ordinary (unmarked) text. The editor reflects the new text, still + with no decorations. +- **PUC-5 (reject a proposal)** The writer clicks **✗** on a blue proposal → the + proposal is discarded → the block disappears on re-render → the document is + unchanged. +- **PUC-6 (proposal while no preview open)** Claude proposes while no preview is + open → a **status-bar item** ("N Claude proposals — open review") appears; + clicking it opens the preview where the proposals are reviewable. (No forced + auto-open — the writer stays in control of focus.) +- **PUC-7 (graceful edges)** Non-markdown active editor → the open command warns + and opens nothing (markdown-only). A malformed markdown/mermaid block → an + inline error chip for that block, the rest renders. No baseline yet → render + current doc with **no** change-marks (everything is "since opened") + a header + note; proposals (if any) still render with ✓/✗. + +## 5. UX Layout + +A webview in the editor column **beside** the source (`ViewColumn.Beside`). The +source editor stays fully editable but is now **visually clean** — no coauthoring +decorations. The preview is **read-only rendered output** except for the ✓/✗ +proposal controls, which post intent (never edit the doc directly). With +annotations **on**: + +- **Human additions** since baseline — soft green background (`cw-by-human` over + ``), themeable. +- **LLM-authored text** (already landed, attributed by F3) — soft blue + (`cw-by-claude`). +- **Deletions** — struck-through, muted (``), reconstructed + from the baseline. +- **Pending Claude proposal** — a **blue proposal block**: the text it would + replace shown struck, the proposed text shown, and a compact **✓ / ✗** control + pair in the block's corner (`data-proposal-id` on the block). +- **Atomic blocks** (code / mermaid) — diffed whole (INV-23) with F7.1 + intra-diagram mermaid styling for changed flowchart/sequence diagrams; an + overlapping author span yields a block-level author badge (F9 INV-27). +- A compact **header bar**: the annotations **on/off** switch; in the on state, a + `Since ` label (reusing F6's epoch text) + a `+N −M · P proposals` + summary and the legend (● You · ● Claude · ✓/✗ = accept/reject Claude). + +Colors derive from the active VS Code theme via webview CSS variables (light / +dark / high-contrast); proposal-block and ✓/✗ button styling are bespoke but +theme-variable-driven. + +--- + +## 6. Technical Design + +### 6.1 Invariants + +Parent invariants INV-1..INV-31 carry over **except where F10 supersedes them**. +F10 adds: + +- **INV-32 (single review surface; clean editor)** The rendered preview is the + **only** annotated review surface. The editor carries **zero** coauthoring + decorations: F3 attribution decorations, F4 in-editor proposal threads, and the + F6 two-pane diff are all removed from / hidden in the editor. (F3 `spansFor`, + F4 accept/reject logic, and the F6 baseline store remain as **data layers**.) +- **INV-33 (combined pure render)** The "on" render unifies two axes — + changes-since-baseline (author-colored additions + struck deletions) **and** + interactive pending proposals — in **one pass** of a pure, vscode-free, + DOM-free function `renderReview(baselineText, currentText, authorSpans, + proposals): string`: same inputs → identical HTML (extends INV-22). The "off" + render is the plain `markdown-it` of the current buffer. +- **INV-34 (✓/✗ act only on pending Claude proposals, via the F4 seam)** + Interactive controls appear **only** on pending Claude proposals (blocks + carrying `data-proposal-id`); human (green) changes carry no controls. ✓/✗ post + **intent** to the host; **all** document mutation goes through the existing F4 + `ProposalController.accept`/`reject` → `applyAgentEdit` (`WorkspaceEdit`) seam + (INV-9). The webview never mutates the document, sidecar, or baseline + (INV-20/21 hold). + +**Supersession (explicit):** F10 **reverses F9 INV-26** ("authorship mode is +baseline-independent and never combined with the diff"). F10 *combines* the +authorship axis with the change axis by design; F9's standalone authorship mode +and segmented control are removed. F9 INV-27 (atomic fences get a block-level +author badge) and INV-28 (pure authorship render) are **absorbed** into INV-33 +(the combined render keeps fences atomic and stays pure). + +### 6.2 High-level architecture + +Three reused data layers + one new pure render path + an interactive webview. + +```mermaid +flowchart LR + edit["source editor\n(clean — no decorations)"] -- onDidChangeTextDocument (debounced) --> ctl + base["F6 baseline\n(DiffViewController, INV-18)"] -- onDidChangeBaseline --> ctl["trackChangesPreview\n(vscode layer)"] + attr["F3 AttributionController\nspansFor(doc)"] -- author spans --> ctl + prop["F4 ProposalController\npending proposals"] -- onDidChangeProposals --> ctl + ctl -- "(baseline, current, spans, proposals)" --> model["renderReview\n(pure, vscode-free)\nblock+word diff · author sentinels · proposal blocks"] + model -- annotated HTML --> ctl + ctl -- postMessage{html, mode, summary} --> wv["webview\n(sealed CSP, local assets)\nmermaid.run() · ✓/✗ buttons"] + wv -- "postMessage{accept|reject, proposalId}" --> ctl + ctl -- "accept/reject" --> prop + prop -- "applyAgentEdit (WorkspaceEdit)" --> doc[(document)] + prop -- accept --> base +``` + +- **`trackChangesModel.ts`** (pure, vscode-free — INV-33) gains + **`renderReview(baselineText, currentText, authorSpans, proposals): string`**. + Internals: (1) the existing F7 `diffBlocks` over baseline/current → block ops; + (2) for changed/added **prose**, the existing word-level ``/``, + **then** author-color each `` (and live text) by intersecting `authorSpans` + via the F9 **PUA-sentinel** technique (refactored out of `renderAuthorship` + into a shared `colorByAuthor` helper); (3) **inject proposal blocks**: for each + proposal, resolve its anchor offset in `currentText`, emit a + `
` containing the struck + replaced-text and the proposed text + a `✓ ✗` + placeholder rendered by the webview; (4) atomic code/mermaid unchanged (F7.1 + reused). Off-mode calls a thin `renderPlain(currentText)` (markdown-it only). +- **`trackChangesPreview.ts`** (vscode layer) — `mode: Map` + (default `"on"`, replacing F9's `"changes"|"authorship"`); gains a + **`ProposalController` dependency** and subscribes to a F4 + `onDidChangeProposals` event (additive, mirroring F6's `onDidChangeBaseline`) + alongside the existing edit + baseline triggers; on the new + `{accept|reject, proposalId}` inbound message, calls + `ProposalController.accept`/`reject` then re-renders; owns the **status-bar + item** (PUC-6). +- **Webview asset** (`media/preview.ts` + `.css`) — header **on/off** switch + (replacing the segmented control); **inbound** click handler on `.cw-actions` + buttons → `postMessage({accept|reject, proposalId})` (nonce-gated inline + script, CSP-safe); proposal-block + ✓/✗ button CSS; reuses the existing + `mermaid.run()` swap. Stays sealed (INV-21). +- **F3 `AttributionController`** — remove the `render()` `setDecorations` calls + and dispose the decoration types; **keep** `spansFor`. Retire/hide + `cowriting.toggleAttribution`. +- **F4 `ProposalController`** — remove the in-editor proposal-thread decorations + and the in-editor accept/reject codelens/commands' UI; **keep** + `accept`/`reject`/`propose`. Add `listProposals(document): ProposalView[]` + (anchor offset + replacement + id) and `onDidChangeProposals` for the preview. +- **F6 `DiffViewController`** — unchanged code; `cowriting.toggleDiffView` + + `ctrl+alt+d` set `when:false` in `package.json`. + +### 6.3 Data model & ownership + +**No new persisted artifact** (INV-20). F10 holds only in-memory webview panels +keyed by document URI + the per-panel on/off mode + the status-bar item. The +baseline is owned by F6; proposals by F4 (sidecar); attribution by F3. The only +on-the-wire models are transient: + +```ts +// host → webview (transient HTML payload) +type RenderMsg = { + type: "render"; mode: "on" | "off"; html: string; + epoch?: string; summary?: { added: number; removed: number; proposals: number }; +}; +// webview → host (intent only — INV-34) +type ActionMsg = + | { type: "setMode"; mode: "on" | "off" } + | { type: "accept"; proposalId: string } + | { type: "reject"; proposalId: string }; + +// internal to renderReview (illustrative — not persisted) +type ProposalView = { id: string; anchorStart: number; anchorEnd: number; replacement: string }; +``` + +### 6.4 Interfaces & contracts + +- **`trackChangesModel`** (vscode-free): `renderReview(baselineText: string, + currentText: string, authorSpans: AuthorSpan[], proposals: ProposalView[]): + string` — annotated HTML for the on-state body; deterministic (INV-33). + `renderPlain(currentText: string): string` — off-state body. Exports + `colorByAuthor(html-or-blocks, spans)` (the salvaged F9 sentinel helper) for + unit tests. **Removes** the public `renderAuthorship` (superseded); + `renderTrackChanges` is retained internally / folded into `renderReview`. +- **`TrackChangesPreviewController`** (vscode layer): `show(document)`, + `refresh(document)` (debounced), `setMode(uriString, "on"|"off")` (test seam), + `getLastModel(uriString)` (test seam — the last computed block/proposal model, + so host E2E asserts marks without reading webview DOM). Constructor now takes + `AttributionController` **and** `ProposalController`. +- **`AttributionController`** (additive removal): `render()` no longer applies + editor decorations; `spansFor(document): AuthorSpan[]` unchanged. +- **`ProposalController`** (additive): `listProposals(document): ProposalView[]`; + `readonly onDidChangeProposals: vscode.Event<{ uri: string }>` fired on + propose/accept/reject. `accept`/`reject`/`applyAgentEdit` unchanged. +- **`DiffViewController`** (F6) — unchanged; only its `package.json` + command/keybinding `when` flips to `false`. +- **Commands / keybindings** (`package.json`): `cowriting.showTrackChangesPreview` + ("Cowriting: Open Review Preview", `ctrl+alt+r`, `when: editorLangId == + markdown`) — retitled, kept. `cowriting.toggleDiffView` + `ctrl+alt+d`, + `cowriting.toggleAttribution`, and the in-editor `cowriting.acceptProposal` / + `cowriting.rejectProposal` palette entries → `when:false` (hidden; the seams + stay for tests/programmatic use). Status-bar item registered in `extension.ts`. + +### 6.5 Per–Product-Use-Case design + +- **PUC-1 (clean editor):** delete `AttributionController.render`'s + `setDecorations` + decoration-type creation; remove F4's in-editor proposal + decorations. Editor shows plain text. +- **PUC-2 (toggle):** header switch → `{type:"setMode", mode}` → controller + updates the per-panel map → re-render. Off = `renderPlain`; on = `renderReview`. +- **PUC-3 (who changed what):** `renderReview` overlays the F7 baseline diff + (author-colored via F9 sentinels) + proposal blocks. +- **PUC-4 (accept):** `{type:"accept", id}` → `ProposalController.accept(state, + proposal)` → `applyAgentEdit` `WorkspaceEdit` (INV-9) → baseline advances + (INV-18) → `onDidChangeBaseline` + `onDidChangeProposals` → re-render (block now + ordinary text). +- **PUC-5 (reject):** `{type:"reject", id}` → `ProposalController.reject` → + proposal removed from sidecar → `onDidChangeProposals` → re-render (block gone); + document untouched. +- **PUC-6 (proposal while no preview):** on `onDidChangeProposals`, if no panel + is open for the doc, show/update a status-bar item with the pending count, + `command = cowriting.showTrackChangesPreview`. Hidden when count = 0 or a panel + is open. +- **PUC-7 (edges):** non-markdown → `showWarningMessage`, no panel. Per-block + `try/catch` → error chip (F7). No baseline → render with no change-marks + a + note; proposals still render. + +### 6.6 Non-functional requirements & cross-cutting concerns + +Render cost is O(document) on a debounced edit / proposal change — fine at +inner-loop scale (large-doc debounce/idle cap is the F7 tunable, not v1 +critical). The webview stays **sealed** (INV-21): local assets, strict CSP with a +per-load nonce, no network; mermaid lives only in the webview bundle. The ✓/✗ +buttons are host-rendered HTML with `data-proposal-id`; the nonce'd inline script +only reads those ids and posts intent (no eval, no remote). No telemetry, no LLM, +no credentials, nothing persisted. + +### 6.7 Key decisions & alternatives considered + +| Decision | Chosen | Alternatives rejected | +| --- | --- | --- | +| **Where Claude's reviewable changes come from** | **Pending F4 proposals surfaced in the preview** — keep propose-by-default; ✓ = F4 accept (apply + advance baseline), ✗ = F4 reject. | **Apply-then-review** (LLM edits land in the buffer; ✓/✗ pin/revert per span) — reverses F4 INV-10/11 and needs new per-span baseline machinery. **Both axes overlaid** (landed-LLM authorship + pending proposals + human diff) — richest but most rendering complexity; deferred. *(Operator decision, session 0029.)* | +| **Editor decoration scope** | **Fully clean** — strip F3 attribution **and** F4 proposal threads; proposals are **preview-only**. | **Strip F3 only, keep amber threads** — editor isn't truly "zero annotations"; two proposal surfaces to keep in sync. *(Operator decision, session 0029.)* | +| **F6 two-pane diff** | **Hide** command + `ctrl+alt+d` (`when:false`); keep the controller + baseline store. | **Delete the view code now** — larger, less reversible diff; a later cleanup PR can remove the dead view. *(Operator decision, session 0029.)* | +| **F9's two modes** | **Collapse** into one **on/off** toggle; on = combined render; off = plain markdown. Remove `renderAuthorship` + segmented control. | **Keep three modes** (changes / authorship / combined) — re-introduces the very surface-multiplicity F10 removes. | +| **Combined render approach** | **One pure `renderReview`** overlaying diff + author sentinels + proposal blocks (extends INV-22). | **Compose two HTML passes in the webview** — pushes logic into the sealed sandbox, loses unit-testability (INV-33). | +| **Proposal-while-no-preview** | **Status-bar indicator** (clickable to open). | **Force auto-open the preview** — hijacks focus, fights "write left, review right." *(Deferred decision — cheap to flip to auto-open.)* | +| **Deletion coloring** | **Neutral struck** (`cw-del`), per the issue's "strikethrough = deleted". | **Color the strikethrough by who deleted** — adds a fourth signal for marginal value. | +| **Non-markdown review** | **Dropped as a goal** (markdown-only product). | **Keep F6's any-file reach** — contradicts the locked product vision; rename to a markdown-specific identity is the eventual corollary (deferred). | + +### 6.8 Testing strategy + +- **Unit (vitest, vscode-free):** `renderReview` over fixtures — human addition + (green ``), human deletion (struck `cw-del`), a proposal **insert** (blue + `cw-proposal` block + `data-proposal-id` + ✓/✗ markup), a proposal **replace** + (struck old + proposed new in one block), mixed human+proposal in a paragraph, + an atomic code fence + a changed mermaid fence (F7.1 augmentation intact, no + inner sentinels), **off-mode** = plain markdown (`renderPlain` equals + markdown-it of source), determinism (same inputs → identical HTML). `colorByAuthor` + helper unit tests (salvaged F9 cases). Proposal-anchor resolution edge: an + unresolvable fingerprint → the proposal is rendered as a trailing block, never + dropped silently. +- **Host E2E (`@vscode/test-electron`, no LLM, extends the F7/F9 suite):** open a + markdown fixture → `cowriting.showTrackChangesPreview` → assert panel open + + `getLastModel` (opened baseline → no change-marks). Type → an `added` block, + author = human. `proposeAgentEdit` seam → `getLastModel` shows a proposal op + with id; **simulate `{type:"accept", id}`** → proposal applied, baseline + advanced, block now `unchanged`/ordinary; **`{type:"reject", id}`** → proposal + gone, document text unchanged. Assert the **editor has no decorations** (the F3 + decoration types are not created / applied). Assert `toggleDiffView` is not in + the active command set (or its `when` is false). Non-markdown doc → command + warns, no panel. Status-bar item appears on a proposal with no panel open. + Webview DOM / real button clicks are **not** E2E-asserted (sealed sandbox) — + manual smoke. +- **Live smoke (manual — `docs/MANUAL-SMOKE-F10.md`):** open a markdown doc; + confirm the editor is clean (no tint/threads); edit prose (green ins / struck + del); ask Claude to edit a selection → a blue proposal block with ✓/✗ appears + in the preview; click ✓ (lands, mark clears) and ✗ on another (block vanishes, + doc unchanged); toggle annotations off (clean render) / on; verify the + status-bar indicator with no preview open; verify light/dark theming and that + `git status` shows nothing. + +### 6.9 Failure modes, rollback & flags + +`markdown-it`/`mermaid` throwing on a block → inline error chip (F7), preview +still renders. A proposal whose fingerprint no longer resolves → rendered as a +trailing "unanchored proposal" block with ✓/✗ (never silently dropped — INV-34 +must remain actionable). Webview disposed → controller drops the panel + clears +the status-bar item; re-run the command to reopen. **No feature flag** — the +preview is a pure read-only view (INV-20); not opening it is the off state. +Rollback is reverting the PR with **zero** data migration (nothing persisted; the +F6 baseline, F4 sidecar, F3 attribution data are untouched, so the hidden/removed +UI can be restored). The clean-editor change is mostly deletion of decoration +calls — trivially reversible. + +--- + +## 7. Delivery Plan + +### 7.1 Approach / strategy + +One planning-and-executing session (F10 = `#29`), plan written just-in-time from +this spec — the F2–F9 precedent. Host-E2E tier (a VS Code extension has no +browser/deploy stage); no LLM in CI. The webview's *visual* rendering and real +button clicks are verified by the manual smoke; the automated seams are the pure +render **model** and the host's accept/reject wiring. + +### 7.2 Slicing plan + +- **SLICE-1 — Clean editor.** Strip F3 attribution decorations + (`AttributionController.render`) + F4 in-editor proposal threads; hide + `toggleDiffView`/`ctrl+alt+d` + `toggleAttribution` (`when:false`). Host E2E: + no editor decorations. *(Mostly deletion.)* +- **SLICE-2 — Combined render engine.** `renderReview` + `renderPlain` in + `trackChangesModel.ts`; salvage `colorByAuthor` from `renderAuthorship`; remove + the public `renderAuthorship`; proposal-block emission + anchor resolution; + vitest suite (§6.8). Pure, vscode-free (INV-33). +- **SLICE-3 — Interactive controller + webview.** `ProposalController.listProposals` + + `onDidChangeProposals`; wire `ProposalController` + `AttributionController` + into `TrackChangesPreviewController`; collapse mode to on/off; inbound + `accept`/`reject`/`setMode` handling + re-render; webview on/off switch + ✓/✗ + click→postMessage + proposal/button CSS; status-bar indicator (PUC-6). +- **SLICE-4 — Tests & docs.** Host E2E (open / toggle / propose→accept→reject / + clean-editor / hidden-F6 / status-bar) + `docs/MANUAL-SMOKE-F10.md` + README + F10 section (and a note that F6/F9 user surfaces are superseded). + +E2E are first-class plan tasks (handbook §9/§4); this app's required tier is host +E2E (the F2–F9 precedent). + +### 7.3 Rollout / launch plan + +Non-shippable (no marketplace publish). "Done" = `#29` acceptance: a clean +editor; the preview is the single review surface with an annotations on/off +toggle; on-state paints green human / blue LLM / struck deletions and renders +Claude's pending proposals with working ✓/✗ that land/discard via the F4 seam; +unit + host E2E green; live smoke performed once. The repo rename remains +deferred. + +### 7.4 Risks & mitigations + +| Risk | Mitigation | +| --- | --- | +| Proposals being preview-only hides pending work when no preview is open | Status-bar indicator (PUC-6); README documents "review happens in the preview"; auto-open is a cheap fallback if it proves insufficient | +| Overlaying author sentinels onto the diff's ``/`` perturbs markdown-it inline parsing (F9's known edge) | Per-block `try/catch` → error chip (no hard failure); fences atomic; covered by the common-case tests; intra-emphasis hardening stays deferred | +| Proposal anchor can't be resolved at render time | Render as a trailing "unanchored proposal" block (never dropped); unit-tested | +| Removing F4's in-editor threads loses a familiar affordance | The ✓/✗ move to the preview (the single review surface by design); status-bar signals presence | +| Two reversed decisions (F9 INV-26; F6 any-file `#19`) confuse future readers | This spec records the supersession explicitly (§6.1, §6.7); README updated | +| Webview interactivity widens the attack surface | Nonce-gated inline script reads only `data-proposal-id` and posts intent; CSP unchanged; host validates the id against live proposals before acting (INV-34) | + +--- + +## 8. Traceability matrix + +| Requirement (#29) | Use case | Design | Slice | +| --- | --- | --- | --- | +| Editor carries no coauthoring decorations | PUC-1 | INV-32, §6.2 | SLICE-1 | +| Preview annotations on/off toggle (off = clean render) | PUC-2 | §6.2, §6.4 | SLICE-3 | +| One language: green human / blue LLM / struck deleted | PUC-3 | INV-33, §6.2 (renderReview) | SLICE-2 | +| ✓/✗ on Claude's changes only; ✓ keeps, ✗ reverts | PUC-4/5 | INV-34, §6.5 (F4 seam) | SLICE-2/3 | +| Accept/reject updates doc + preview consistently | PUC-4/5 | §6.5, INV-18 reuse | SLICE-3 | +| Deletions reconstructed from the F6 baseline | PUC-3 | F7 reuse, §6.2 | SLICE-2 | +| Deprecate F6 as a view; keep baseline data layer | — | §6.2, §6.7 | SLICE-1 | +| Markdown-only; no sidecar/contract/SCHEMA change | — | §1.7, INV-20 | all | +| Sealed webview, no network/LLM | — | INV-21, §6.6 | SLICE-3 | +| Unit + host E2E, no LLM in CI | — | §6.8 | SLICE-1..4 | + +## 9. Open Questions & Decisions log + +- **RESOLVED (session 0029, operator):** Claude's reviewable changes = **pending + F4 proposals surfaced in the preview** (propose-by-default kept; ✓ = F4 accept + + baseline advance, ✗ = F4 reject); editor decoration scope = **fully clean** + (strip F3 attribution **and** F4 proposal threads — proposals preview-only); F6 + two-pane diff = **hidden** (`when:false`, code/baseline kept). +- **RESOLVED (this spec, autonomous — cheap to revisit):** proposal-while-no-preview + → **status-bar indicator** (not forced auto-open); F9's dead `renderAuthorship` + / segmented control → **removed** (the PUA-sentinel coloring salvaged into a + shared `colorByAuthor` helper); deletions → **neutral** strikethrough. +- **SUPERSEDED:** F9 INV-26 ("authorship never combined with the diff") — F10 + combines them by design; F9's authorship mode + segmented control are removed. + F6's any-file review reach (`#19`) is dropped as a user goal (markdown-only). +- **OPEN → later:** the repo rename to `vscode-markdown-cowriting-plugin` + (deliberate, deferred); preview→source scroll-sync; intra-emphasis + sentinel-safety hardening; whether to eventually overlay *landed*-LLM authorship + (F9's old axis) in the on-state too (the "both axes" option, deferred); deleting + F6's two-pane view code in a later cleanup PR. + +## 10. Glossary & References + +- **Review preview** — the rendered markdown preview as the single review + surface: clean (off) or annotated (on). **Annotations on/off** — the per-panel + toggle replacing F9's two modes. **Combined render** — `renderReview`'s + single-pass overlay of changes-since-baseline (author-colored + struck) and + pending proposals. **Pending proposal** — an F4 propose-by-default edit, not yet + in the buffer, rendered blue with ✓/✗. **Baseline / epoch / advance** — as F6 + (`coauthoring-diff-view.md`). **`spansFor`** — F3's author-span data layer + (`coauthoring-attribution.md`). **PUA sentinels / `colorByAuthor`** — F9's + author-coloring technique (`2026-06-11-authorship-preview-design.md`), salvaged + here. +- Feature `#29` (F10) · Epic `#1` · builds on F3 `#6`, F4 `#12`, F6 `#17`/`#19`, + F7 `#21`/`#22`, F9 `#27` · parent specs `coauthoring-inner-loop.md`, + `coauthoring-attribution.md`, `coauthoring-propose-accept.md`, + `coauthoring-diff-view.md`, `coauthoring-rendered-preview.md`, + `2026-06-11-authorship-preview-design.md` · capture session 0028 · lineage + `ben.stull/rfc-app#48`. diff --git a/specs/coauthoring-live-progress.md b/specs/coauthoring-live-progress.md new file mode 100644 index 0000000..fc87c91 --- /dev/null +++ b/specs/coauthoring-live-progress.md @@ -0,0 +1,398 @@ +--- +status: graduated +--- +# Solution Design: Live Turn Progress (#60) + +| | | +| --- | --- | +| **Author(s)** | Ben Stull (with Claude) | +| **Reviewers / approvers** | Ben Stull | +| **Status** | `graduated` | +| **Version** | v0.1.0 | +| **Source artifacts** | Feature `benstull/vscode-cowriting-plugin#60` (Show Claude's live output/progress during the "asking Claude…" status, `type/feature`, `priority/P1`) · Capture session `vscode-cowriting-plugin-0054` (2026-06-15) · Brainstorming session `vscode-cowriting-plugin-0055` · Builds on (all shipped): the LiveTurn machine-edit ingress (`liveTurn.ts`, INV-8/9), F4 `#12` (propose/accept seam), F10 `#29` (interactive review preview), F11 `#43` (preview toolbar + `editDocument`/`runEditAndPropose`) · Parent specs (graduated): `coauthoring-inner-loop.md`, `coauthoring-propose-accept.md`, `coauthoring-rendered-preview.md`, `coauthoring-document-edit-flow.md` · Lineage: `ben.stull/rfc-app#48` | + +**Change log** + +| Date | Version | Change | By | +| --- | --- | --- | --- | +| 2026-06-22 | v0.1.0 | Initial draft — brainstorming session 0055. Three forks locked with the operator: **(1) surface** = the existing "asking Claude…" notification carries a live **activity line** *plus* a host **OutputChannel** streaming the full assistant text; **(2) content** = activity (`thinking…`/`writing… (N chars)`/`running …`) **+ running token count**; **(3) cancellation** = reflect aborted/failed states **and** add a cancel button wired to `agent.abort()`. Two sub-decisions confirmed: OutputChannel auto-reveals (preserveFocus) on first text delta, gated by a `cowriting.liveProgress.revealOutput` setting (default on); the channel **appends** per-turn (with a header), doubling as a debug log. | Ben Stull + Claude | + +--- + +## 1. Business Context + +### 1.1 Executive Summary + +When the writer asks Claude to edit, the plugin shows one static notification — +*"Cowriting: asking Claude…"* — and then nothing until the whole turn resolves. +Both edit entry points (`editSelection` in `extension.ts:232` and the preview's +`askClaude` in `trackChangesPreview.ts:232`) `await` the turn as **one opaque +promise** (`liveTurn.ts` `runEditTurn` → `agent.run()`). For anything past a +trivial edit this is a black box: the writer can't tell whether Claude is +thinking, reading, partway through writing, or stuck — and a long edit feels like +the tool hung. + +The fix is small and almost entirely *additive observability*: `@cline/sdk` +**already streams** everything we need (`assistant-text-delta`, +`tool-started|finished`, `usage-updated`, lifecycle events) via +`agent.subscribe(listener)`, and exposes `agent.abort()` for cancellation — the +extension just never subscribes (`liveTurn.ts` constructs the Agent with no +hook). This design wires that stream to the existing notification (a live +**activity line** + running **token count**), mirrors the full assistant text +into a host **OutputChannel** so the writer can *read it forming*, and makes the +notification **cancellable**. The final result path — `runEditTurn` → +`runEditAndPropose` → F4 proposals — is **untouched**. + +### 1.2 Background + +The plugin's machine-edit ingress is `LiveTurn` (`liveTurn.ts`, spec +`coauthoring-inner-loop.md` §6.2): one `@cline/sdk` Agent turn on the built-in +`claude-code` provider (INV-8 — the extension holds no credentials; it rides the +operator's local Claude Code login). The turn's result feeds the F4 propose/accept +seam (`coauthoring-propose-accept.md`) and is surfaced in the F10/F11 interactive +review preview. `liveTurn.ts` is **deliberately `vscode`-free** (its module header +says so) so the SDK ingress stays independent of the editor host. + +Two gestures invoke a turn, both wrapping it in `vscode.window.withProgress(… +"asking Claude…")`: + +- **`editSelection`** (`extension.ts`) — selection → one single-range proposal. +- **preview `askClaude`** (`trackChangesPreview.ts`) — selection or whole + document → one proposal (range) or one-per-changed-block (document, INV-39). + +The preview path runs the turn through an **injectable** seam — `editTurn` +(`trackChangesPreview.ts:56`, type `EditTurn`) — so host E2E can stub the LLM (no +Claude in CI). This design widens that seam without breaking the stub. + +### 1.3 Who feels it & why it's P1 + +The **human coauthor** — anyone who asks Claude to edit and waits for the turn, +most acutely on longer/slower edits. It is **P1** because the opaque wait hurts +*every* Claude turn (the core gesture of the tool), the enabling stream already +exists (low cost), and visible progress materially raises trust ("the tool is +alive and on task"). WSJF (issue #60): Value 6 · TC 3 · OE 4 ÷ Size 4 ≈ 3.25. + +--- + +## 2. Product Design + +### 2.1 The experience + +During a turn, the writer sees **two coordinated surfaces**, both tied to the +existing "asking Claude…" status — no new window, no webview change: + +1. **The notification line (legible motion + scale).** Below the + "Cowriting: asking Claude…" title, a live activity line that updates as events + arrive: + + | State | Line | + | --- | --- | + | before first output | `thinking…` | + | streaming text | `writing… (412 chars)` | + | streaming text + usage seen | `writing… · 1.2k tokens` | + | a tool is running | `running read_file…` | + + The line shows the **current phase** and, once a `usage-updated` event has + arrived, the **running token total** (input + output). `chars` and `tokens` + render together when both are known (`writing… (412 chars) · 1.2k tokens`). + +2. **The OutputChannel (read it forming).** A single shared `"Cowriting"` output + channel receives the **full assistant text** as it streams, under a per-turn + header `── asking: ──`. On the **first** text delta the channel + reveals itself **without stealing focus** (`show(true)`), so the writer can + start reading the output before the turn finishes. Auto-reveal is gated by a + setting `cowriting.liveProgress.revealOutput` (default `true`) for writers who + prefer to open it manually. + +The notification is **cancellable** (`✕`): clicking it aborts the turn +(`agent.abort()`); the status resolves to **"cancelled"** (not a frozen spinner, +not a "failed" error), and **no proposal is created**. If the turn *fails*, the +existing error path shows the error (no longer a frozen "asking Claude…"). + +### 2.2 What this is *not* (non-goals — from #60) + +- **Not a chat/transcript panel.** This is *in-flight progress*, not a persistent + conversation history surface. (The OutputChannel's append-log is a debug + convenience, not a product history feature.) +- **Not streaming into the document.** The result still lands as F4 proposals + exactly as today; only *observation* is added. +- **Not a cancellation feature in its own right.** The `✕` button + abort wiring + are the cheap, in-scope *reflection* of cancel; rich cancel controls (partial + apply, resume, queueing) are out of scope. +- **Not surfacing reasoning text.** `assistant-reasoning-delta` is collapsed to a + generic `thinking…`; the chain-of-thought text is **not** shown (kept the + notification/OutputChannel uncluttered; operator fork). + +### 2.3 Surfaces considered & rejected + +| Surface | Why not (as the *primary*) | +| --- | --- | +| **Webview relay** (into the preview) | The `editSelection` path has **no** webview; can't cover both entry points uniformly (acceptance requires both). | +| **Status-bar item only** | One terse line; no room for streamed text; weaker "what is it doing" signal than the in-status notification line. | +| **OutputChannel only** (notification stays a plain spinner) | The "in/alongside the asking-Claude status" signal weakens — the writer must open the Output panel to learn anything. | + +Chosen: **notification activity-line (primary, in-status) + OutputChannel +(secondary, full text)** — covers both call sites, no new network/webview surface +(INV-21/INV-45), and delivers both "see motion" and "read output as it forms." + +--- + +## 3. Engineering Design + +### 3.1 Architecture — three units + +``` +@cline/sdk Agent events (raw AgentRuntimeEvent stream) + │ agent.subscribe(listener) + ▼ +┌─────────────────────────┐ pure, vscode-free, SDK-free +│ turnProgress.ts (NEW) │ reduce(state, event) → TurnProgressSnapshot +│ the reducer (INV-46) │ +└─────────────────────────┘ + │ TurnProgressSnapshot + ▼ +┌─────────────────────────┐ vscode-free (INV-43) +│ liveTurn.ts runEditTurn │ subscribe → reduce → onProgress(snapshot); +│ (EXTENDED) │ signal?.abort → agent.abort(); result UNCHANGED +└─────────────────────────┘ + │ onProgress(snapshot) + AbortSignal in + ▼ +┌─────────────────────────┐ the two UI call sites (relay only) +│ extension.ts:editSelection │ format notification line · append OutputChannel · +│ trackChangesPreview:askClaude │ withProgress({cancellable}) · token→AbortController +└─────────────────────────┘ +``` + +The decisive layering rule: **progress flows *out* of `liveTurn` as a domain +`TurnProgressSnapshot`; cancel flows *in* as a web-standard `AbortSignal`.** No +`vscode` import is added to `liveTurn.ts` or `turnProgress.ts` (INV-43). + +### 3.2 `src/turnProgress.ts` (new, pure) — INV-46 + +The reducer owns *all* event→state logic so the call sites stay dumb (format + +relay only) and the logic is unit-tested in isolation. + +```ts +export interface TurnProgressSnapshot { + phase: "thinking" | "writing" | "tool"; + tool?: string; // present iff phase === "tool" + chars: number; // accumulated assistant-text length so far + tokens?: number; // running total (input+output); undefined until first usage event + textDelta?: string; // the new assistant-text chunk since the last snapshot +} + +// Opaque carried state; start from createTurnProgressState(). +export interface TurnProgressState { /* phase, chars, tokens, toolDepth/name */ } +export function createTurnProgressState(): TurnProgressState; + +// Pure: fold one SDK event into the state, returning the next state and the +// snapshot to emit (or undefined for events that don't change the surface). +export function reduceTurnProgress( + state: TurnProgressState, + event: AgentRuntimeEvent, // imported as a TYPE only from @cline/shared +): { state: TurnProgressState; snapshot?: TurnProgressSnapshot }; +``` + +Event mapping (the relevant `AgentRuntimeEvent` members): + +| SDK event | Effect on snapshot | +| --- | --- | +| `run-started` / `turn-started` | `phase: "thinking"` | +| `assistant-text-delta` | `phase: "writing"`, `chars = accumulatedText.length`, `textDelta = text` | +| `assistant-reasoning-delta` | `phase: "thinking"` (text **not** surfaced) | +| `tool-started` / `tool-updated` | `phase: "tool"`, `tool = toolCall.name` | +| `tool-finished` | revert `phase` to `writing` if any text seen, else `thinking` | +| `usage-updated` | `tokens = usage` total (input+output) | +| `turn-finished` / `run-finished` / `run-failed` | no snapshot (resolution handled by the turn promise) | + +`AgentRuntimeEvent` is imported **type-only** (`import type`) — `turnProgress.ts` +has **no runtime dependency** on the SDK and no `vscode` import, so it is a pure, +fast unit (INV-43/46). Tool concurrency is tracked by name/depth so overlapping +`tool-started`/`tool-finished` pairs resolve the phase correctly. + +### 3.3 `liveTurn.ts` `runEditTurn` (extended) + +`opts` gains two optional fields; the signature and result type are otherwise +unchanged (INV-44): + +```ts +export interface RunEditTurnOptions { + modelId?: string; + onProgress?: (s: TurnProgressSnapshot) => void; // out: live progress + signal?: AbortSignal; // in: cancellation +} +export async function runEditTurn( + instruction: string, + selectedText: string, + opts?: RunEditTurnOptions, +): Promise; +``` + +Inside, around the existing `agent.run(...)`: + +```ts +const agent = new sdk.Agent({ providerId: "claude-code", modelId, systemPrompt }); +let state = createTurnProgressState(); +const unsubscribe = opts?.onProgress + ? agent.subscribe((ev) => { + const next = reduceTurnProgress(state, ev); + state = next.state; + if (next.snapshot) opts.onProgress!(next.snapshot); + }) + : undefined; +const onAbort = () => agent.abort(); +opts?.signal?.addEventListener("abort", onAbort); +try { + const result = await agent.run(``); + // existing non-"completed" guard throws (covers "aborted" and "failed") + … +} finally { + unsubscribe?.(); + opts?.signal?.removeEventListener("abort", onAbort); +} +``` + +`AbortSignal`/`addEventListener` are web standards (no `vscode`), preserving +INV-43. An aborted turn yields `status: "aborted"` → the existing guard throws → +the call site reflects "cancelled" and proposes nothing (INV-47). **`extractReplacement` and the return shape are untouched** (INV-44). + +### 3.4 The two call sites (UI relay) + +A small shared host module owns the relay so both sites are identical: + +```ts +// src/liveProgressUi.ts (host; vscode-only) +export function createLiveProgressUi(): { + channel: vscode.OutputChannel; + // Begin a turn: clears nothing, writes the header, returns the wiring for + // one withProgress run. + begin(instruction: string, progress: vscode.Progress<{message: string}>, token: vscode.CancellationToken): + { signal: AbortSignal; onProgress: (s: TurnProgressSnapshot) => void }; +}; +``` + +- **`onProgress(s)`** → `progress.report({ message: formatLine(s) })` where + `formatLine` renders `phase`/`chars`/`tokens`; and appends `s.textDelta` (if + any) to the shared `"Cowriting"` channel. On the **first** non-empty + `textDelta`, if `cowriting.liveProgress.revealOutput` is `true`, call + `channel.show(true)` once. +- **`signal`** comes from an `AbortController` whose `abort()` is fired by + `token.onCancellationRequested`. +- The `withProgress` options gain **`cancellable: true`**. +- On `catch`, the call site checks `token.isCancellationRequested` → show + **"cancelled"** (information) instead of the "failed" error. + +`extension.ts:editSelection` passes `{ onProgress, signal }` straight into +`runEditTurn`. The preview path threads the same through its seam: + +```ts +// trackChangesPreview.ts — widen the injectable seam (back-compat: opts optional) +type EditTurn = (instruction: string, text: string, opts?: RunEditTurnOptions) => Promise; +``` + +`runEditAndPropose` forwards `opts` to its single `editTurn(...)` call (both the +`range` and `document` branches invoke the turn exactly once per gesture, so there +is one progress stream per gesture). The **host-E2E stub** (the default-overriding +injection) keeps working — `opts` is optional and ignored by the stub; a focused +test may simulate progress by calling `opts?.onProgress` (§4). + +### 3.5 OutputChannel semantics + +- **One shared channel** (`createOutputChannel("Cowriting")`), created once at + activation and disposed with the extension. +- **Append, don't clear** — each turn writes a `── asking: ──` + header then streams text, so the channel doubles as a debug log of recent turns + (per #60 solution-notes). (Trimming an unbounded log is a possible later + increment, not v1.) +- **Reveal** on first text delta, `preserveFocus: true`, gated by + `cowriting.liveProgress.revealOutput` (boolean, default `true`). + +### 3.6 New configuration + +| Setting | Type | Default | Effect | +| --- | --- | --- | --- | +| `cowriting.liveProgress.revealOutput` | boolean | `true` | Auto-reveal the "Cowriting" OutputChannel (without focus) on the first streamed text of a turn. | + +Declared in `package.json` `contributes.configuration`. + +### 3.7 Invariants + +- **INV-43** — `liveTurn.ts` and `turnProgress.ts` stay **`vscode`-free**: + progress leaves as a domain `TurnProgressSnapshot`, cancel enters as a + web-standard `AbortSignal`; `AgentRuntimeEvent` is imported **type-only**. No + `vscode` import is added to either module. +- **INV-44** — Live progress is **purely additive**: it never changes the result + path (`runEditTurn` return shape, `extractReplacement`, `runEditAndPropose`, + the F4 proposals). A turn that emits **no** events (the E2E stub) produces the + identical proposals it does today. +- **INV-45** — **No new network/webview surface.** Progress rides the existing + `withProgress` notification + a host `OutputChannel` only; the sealed webview is + untouched (preserves INV-21). +- **INV-46** — The event→progress **reduction is a pure function** + (`reduceTurnProgress`), unit-tested in isolation; the UI call sites only format + and relay its snapshots. +- **INV-47** — **Cancellation reflects, doesn't mutate.** Abort surfaces + "cancelled" (not a frozen spinner, not a silent partial apply); an aborted turn + proposes **nothing** (parity with the empty / no-change outcomes). + +### 3.8 Error & edge handling + +- **Failure** — non-"completed" status throws (existing guard); call site shows + the error message, the notification resolves (no frozen spinner). +- **Cancel** — `token` → `agent.abort()` → `status: "aborted"` → guarded throw → + call site detects `token.isCancellationRequested` → "cancelled", no proposal. +- **No events** — if the provider emits nothing before completing, the line stays + `thinking…` and resolves normally; result identical to today (INV-44). +- **Overlapping tools** — phase tracked by tool name/depth so interleaved + `tool-started`/`tool-finished` resolve back to `writing`/`thinking` correctly. +- **Empty / no-change replacement** — unchanged from today (warning / info), + independent of progress. + +--- + +## 4. Testing & E2E + +Per the §9 pipeline and `coauthoring-*` precedent (unit + host E2E; this is a +VS Code extension — **no flotilla/PPE**, the pipeline's deploy tiers are N/A; the +gate is unit + host-E2E green). + +- **Unit (the core):** `turnProgress.test.ts` exercises `reduceTurnProgress` + across the full event table — phase transitions (thinking→writing→tool→writing), + `chars` accumulation, `tokens` appearing only after `usage-updated`, `textDelta` + carrying each chunk, reasoning-delta staying `thinking`, overlapping tools. Pure, + no vscode/SDK. (`formatLine` gets its own small unit test.) +- **Unit (relay):** `liveProgressUi` line formatting and first-delta reveal gating + (mock `OutputChannel`/`Progress`). +- **Host E2E:** the existing preview `editTurn` stub is extended in one test to + **emit synthetic progress** (call `opts.onProgress` with a couple of snapshots) + and assert the turn still yields the same proposals (INV-44) and that cancel + (firing the token) yields **zero** proposals (INV-47). Streamed UI itself (the + live notification text) isn't asserted — E2E can't read notification subtitles + reliably; that's covered by the unit tests on the mapping + a **manual smoke** + on a longer real edit (per #60 acceptance). +- **Manual smoke:** a longer real edit shows the activity line advancing, token + count rising, the OutputChannel streaming, and `✕` cancelling cleanly. + +--- + +## 5. Delivery Plan (rollout — not a task list) + +One increment (the feature is a single design-then-build, #60 decomposition). +Ships through the standard branch → PR → `main` flow; no migration, no persisted +state, no config beyond the one additive setting (default preserves prior +behavior except that progress is now visible). Reversible by reverting the PR. +The implementation plan (downstream `wgl-planning-and-executing` session) owns +the Task breakdown; a natural cut is: **(1)** pure `turnProgress.ts` + tests → +**(2)** `runEditTurn` subscribe/abort wiring → **(3)** `liveProgressUi` + +both call sites + setting → **(4)** host-E2E stub extension + manual smoke. + +--- + +## 6. Open Questions + +- **OQ-1** — Unbounded OutputChannel log: append-forever is fine for v1 (it's a + manual debug surface); if it becomes noisy, add per-turn trimming or a + "clear on new turn" mode later. *(Deferred — not blocking.)* +- **OQ-2 (inherited)** — The F11 design (`#43`) is still un-graduated (lives in + code + an issue draft). Unrelated to #60 but noted in the shared lineage; track + separately.