From 6b14de9d8edca4a033d51b32b4da6fdaab68d8c6 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Thu, 11 Jun 2026 20:32:57 -0700 Subject: [PATCH 01/11] add plan ./plans/2026-06-11-f7-intra-diagram-mermaid-diff.md --- ...026-06-11-f7-intra-diagram-mermaid-diff.md | 1034 +++++++++++++++++ 1 file changed, 1034 insertions(+) create mode 100644 plans/2026-06-11-f7-intra-diagram-mermaid-diff.md 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. From 118bc293ad58d7876350ae3090f329acf185e24c Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Thu, 11 Jun 2026 21:52:37 -0700 Subject: [PATCH 02/11] add spec ./specs/coauthoring-interactive-review.md (status: graduated) --- specs/coauthoring-interactive-review.md | 609 ++++++++++++++++++++++++ 1 file changed, 609 insertions(+) create mode 100644 specs/coauthoring-interactive-review.md 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`. From 7a45af9abe62fdb01654e331362a75c6a25d9f06 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Fri, 12 Jun 2026 00:41:06 -0700 Subject: [PATCH 03/11] add plan ./plans/2026-06-11-f10-interactive-review.md --- plans/2026-06-11-f10-interactive-review.md | 995 +++++++++++++++++++++ 1 file changed, 995 insertions(+) create mode 100644 plans/2026-06-11-f10-interactive-review.md 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). From 855acec09b7a1fb82eac7d4d62d982622e102f7f Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Fri, 12 Jun 2026 12:17:35 -0700 Subject: [PATCH 04/11] add spec ./specs/2026-06-12-f11-preview-toolbar-interaction-surface.md (status: graduated) --- ...f11-preview-toolbar-interaction-surface.md | 602 ++++++++++++++++++ 1 file changed, 602 insertions(+) create mode 100644 specs/2026-06-12-f11-preview-toolbar-interaction-surface.md 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`. From 62706371a2c5e3bb540c9e501a5c848116f15395 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Fri, 12 Jun 2026 14:20:03 -0700 Subject: [PATCH 05/11] add plan ./plans/2026-06-12-f11-preview-toolbar.md --- plans/2026-06-12-f11-preview-toolbar.md | 123 ++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 plans/2026-06-12-f11-preview-toolbar.md 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. From 43913dcbb992a34627240f530696aac51c0fe4be Mon Sep 17 00:00:00 2001 From: BenStullsBets Date: Fri, 26 Jun 2026 08:30:29 -0700 Subject: [PATCH 11/11] update spec ./specs/coauthoring-inline-editor-diff.md (status: graduated) --- specs/coauthoring-inline-editor-diff.md | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/specs/coauthoring-inline-editor-diff.md b/specs/coauthoring-inline-editor-diff.md index bb3f90e..d45300c 100644 --- a/specs/coauthoring-inline-editor-diff.md +++ b/specs/coauthoring-inline-editor-diff.md @@ -8,13 +8,14 @@ status: graduated | **Author(s)** | Ben Stull (with Claude) | | **Reviewers / approvers** | Ben Stull | | **Status** | `graduated` | -| **Version** | v0.1.0 | +| **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 | --- @@ -318,12 +319,19 @@ all four to the controller methods of §3.3 (INV-35 preserved). ### 3.8 Invariants -- **INV-48 — Optimistic apply.** 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. **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-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