F10: interactive track-changes review in the markdown preview (#29) #30
@@ -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<string, vscode.CommentThread>` (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) => `<p>${src}</p>`;
|
||||
const html = colorByAuthor(raw, 0, spans, render);
|
||||
expect(html).toContain('<span class="cw-by-human">hello</span>');
|
||||
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("<h1>Title</h1>");
|
||||
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 <ins>", () => {
|
||||
const html = renderReview("hello", "hello world",
|
||||
[{ start: 6, end: 11, author: "human" }], []);
|
||||
expect(html).toMatch(/<ins>[^<]*world[^<]*<\/ins>|cw-by-human/);
|
||||
});
|
||||
|
||||
test("renderReview: deletion since baseline renders struck cw-del/<del>", () => {
|
||||
const html = renderReview("hello world", "hello", [], []);
|
||||
expect(html).toMatch(/<del>|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(/<del>[^<]*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 `<ins>` 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 ? `<del class="cw-del">${safe(p.replaced)}</del>` : "";
|
||||
const after = `<ins class="cw-add">${safe(p.replacement)}</ins>`;
|
||||
const actions =
|
||||
`<span class="cw-actions">` +
|
||||
`<button class="cw-accept" data-action="accept">✓</button>` +
|
||||
`<button class="cw-reject" data-action="reject">✗</button>` +
|
||||
`</span>`;
|
||||
return `<div class="cw-proposal${unanchored}" data-proposal-id="${p.id}">${actions}${before}${after}</div>`;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 <ins>/<del>
|
||||
// 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 <ins>/<del>
|
||||
// unchanged / added prose → author-colored.
|
||||
return `<div class="cw-blk ${op.kind === "added" ? "cw-added" : "cw-unchanged"}">${colored(op.block.raw)}</div>`;
|
||||
}
|
||||
```
|
||||
|
||||
> 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 `<ins>` 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<string, "on" | "off">();
|
||||
```
|
||||
|
||||
- [ ] **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
|
||||
<div id="cw-header">
|
||||
<label id="cw-toggle"><input type="checkbox" id="cw-annotations" checked /> Annotations</label>
|
||||
<span id="cw-epoch">Review</span>
|
||||
<span id="cw-summary"></span>
|
||||
<span id="cw-legend"></span>
|
||||
</div>
|
||||
```
|
||||
|
||||
- [ ] **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<HTMLElement>(".cw-actions button");
|
||||
if (!btn) return;
|
||||
const block = btn.closest<HTMLElement>(".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 `<ins>` / struck `<del>` 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).
|
||||
Reference in New Issue
Block a user