From 7b982492869085895d5c4689d1650a7edc2ac0ea Mon Sep 17 00:00:00 2001 From: benstull <1+benstull@noreply.localhost> Date: Fri, 26 Jun 2026 15:28:14 +0000 Subject: [PATCH] F12: inline editable proposed-change diff in the Markdown editor + Accept/Reject control parity (#64) (#66) --- .../plans/2026-06-26-inline-editor-diff.md | 1246 +++++++++++++++++ media/preview.css | 5 + media/preview.ts | 7 +- package.json | 27 + specs/coauthoring-inline-editor-diff.md | 445 ++++++ src/attributionController.ts | 20 +- src/editorProposalController.ts | 151 ++ src/extension.ts | 20 + src/model.ts | 10 + src/proposalController.ts | 195 ++- src/proposalModel.ts | 19 + src/trackChangesModel.ts | 109 +- src/trackChangesPreview.ts | 29 +- test/decorationPlan.test.ts | 27 + test/e2e/runTest.ts | 14 +- test/e2e/suite/f10Review.test.ts | 12 +- test/e2e/suite/f11Toolbar.test.ts | 9 +- test/e2e/suite/f12InlineDiff.test.ts | 251 ++++ test/e2e/suite/f12Reach.test.ts | 6 +- test/e2e/suite/liveProgress.test.ts | 5 +- test/e2e/suite/proposals.test.ts | 55 +- test/model.test.ts | 16 +- test/proposalModel.test.ts | 17 +- test/trackChangesModel.test.ts | 51 +- 24 files changed, 2689 insertions(+), 57 deletions(-) create mode 100644 docs/superpowers/plans/2026-06-26-inline-editor-diff.md create mode 100644 specs/coauthoring-inline-editor-diff.md create mode 100644 src/editorProposalController.ts create mode 100644 test/decorationPlan.test.ts create mode 100644 test/e2e/suite/f12InlineDiff.test.ts diff --git a/docs/superpowers/plans/2026-06-26-inline-editor-diff.md b/docs/superpowers/plans/2026-06-26-inline-editor-diff.md new file mode 100644 index 0000000..cbee0f4 --- /dev/null +++ b/docs/superpowers/plans/2026-06-26-inline-editor-diff.md @@ -0,0 +1,1246 @@ +# F12 Inline Editable Proposed-Change Diff — 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:** When Claude proposes an edit, show it inline in the Markdown editor — optimistically applied (editable, tinted insertions + struck-red non-editable deletion hints) — with `Accept ▾` / `Reject ▾` (and Accept-all / Reject-all) reachable from both the editor (CodeLens + QuickPick) and the review webview. + +**Architecture:** Reverses F4/INV-10 (propose no longer leaves the buffer untouched) and F10/INV-32 (editor no longer unconditionally clean). On propose, a new `EditorProposalController` optimistically applies each proposal through the existing attribution seam with the **baseline-advance suppressed**, **re-anchors** the proposal to the applied text, and stores the pre-apply original on the proposal. Accept becomes **finalize-in-place** (advance the baseline + clear), reject becomes **revert-in-place** (restore the original + clear). The webview render diffs against *current-minus-pending-proposals* so a proposed span renders exactly once (INV-50). One pure diff feeds both surfaces (INV-49). + +**Tech Stack:** TypeScript, VS Code extension API (`TextEditorDecorationType`, `CodeLensProvider`, `QuickPick`, `WorkspaceEdit`), `diff`, `markdown-it`, esbuild webview bundle, Mocha host-E2E (`@vscode/test-electron`) + Vitest unit. + +**Spec:** `specs/coauthoring-inline-editor-diff.md` (INV-48…54). **Issue:** `benstull/vscode-cowriting-plugin#64`. + +--- + +## Architecture decisions locked before coding + +1. **Re-anchor on optimistic apply (spec refinement).** F4's `fp.text` is the *original* target. Once the replacement is in the buffer, `resolve(buffer, fp_original)` orphans. So optimistic apply: (a) stores the original on `Proposal.original`, (b) re-fingerprints the proposal's anchor to the **applied** span. After that, `resolve(buffer, fp)` finds the applied text — finalize/revert/decorate all key off it. +2. **Optimistic apply = `acceptBlock`/`applyAgentEdit` with `landBaseline:false`.** Reuses the existing word-precise seam path (INV-40 keeps unchanged words' authorship), but does **not** fire the machine-landing `applyEmitter` (baseline stays at pre-proposal → the change reads as pending). +3. **Finalize = advance the baseline now** (`attribution.signalLanded(document)` fires the same emitter `extension.ts` already wires to `diffViewController.advance`). No text re-applied (it's already in the buffer). **Revert = replace the live applied span with `proposal.original`.** +4. **Render once (INV-50).** `renderReview` diffs `baseline` vs `currentMinusPending` (the buffer with each pending proposal's applied span swapped back to its `original`), then overlays the proposals. When the only changes are pending proposals, the landed diff is empty and each change renders once as its proposal block. +5. **An "applied" guard** prevents the optimistic-apply trigger from re-applying a proposal it already applied (propose → onDidChangeProposals → apply → sidecar re-anchor write → onDidChangeProposals again). + +--- + +## File structure + +| File | Responsibility | Change | +| --- | --- | --- | +| `src/model.ts` | Artifact/Proposal types + serialize | Add `Proposal.original?: string`; serialize it. | +| `src/proposalModel.ts` | pure `proposals[]` helpers | `addProposal` accepts `original`; new `setProposalApplied(a, id, fp, original)` re-anchor helper. | +| `src/attributionController.ts` | F3 seam | `applyAgentEdit` gains `opts.landBaseline` (default true); new `signalLanded(document)`. | +| `src/proposalController.ts` | F4 lifecycle | `optimisticApply`, `finalizeInPlace`, `revertInPlace`, `rejectAll`, `appliedRange`; accept/reject route to finalize/revert. | +| `src/editorProposalController.ts` | **NEW** editor surface | optimistic-apply trigger, decorations, deletion hints, CodeLens + QuickPick menus. | +| `src/trackChangesModel.ts` | pure render engine | `currentMinusPending` reconciliation in `renderReview` (INV-50); `proposalBlockHtml` → `Accept ▾`/`Reject ▾`; export `decorationPlan` (INV-49). | +| `src/trackChangesPreview.ts` | webview host | route `rejectAll`; pass `original` through `listProposals`/`ProposalView`. | +| `media/preview.ts` / `media/preview.css` | webview client | `Accept ▾`/`Reject ▾` + dropdown; route `rejectAll`. | +| `src/extension.ts` | activation/commands | register `EditorProposalController` + CodeLens + menu commands + `rejectAllProposals`. | +| `package.json` | contributes | new commands, CodeLens, `when` clauses. | + +--- + +## Task 1: `Proposal.original` field (model) + +**Files:** +- Modify: `src/model.ts` (the `Proposal` interface ~L79-100; `serializeArtifact` proposals map ~L245-258) +- Test: `test/model.test.ts` + +- [ ] **Step 1: Write the failing test** + +Add to `test/model.test.ts`: + +```ts +test("serializeArtifact round-trips Proposal.original", () => { + const a = emptyArtifact("doc.md"); + a.anchors["a_1"] = { fingerprint: { text: "new", before: "", after: "", lineHint: 0 } }; + a.proposals.push({ + id: "pr_1", anchorId: "a_1", replacement: "new", + author: { kind: "agent", id: "claude", agent: { sdk: "@cline/sdk", model: "sonnet", sessionId: "s" } }, + createdAt: "2026-06-26T00:00:00.000Z", original: "old", granularity: "block", + }); + const json = serializeArtifact(a); + assert.match(json, /"original": "old"/); + assert.match(json, /"granularity": "block"/); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/model.test.ts -t "round-trips Proposal.original"` +Expected: FAIL — `original` not emitted as a canonical key (it currently survives only as an unknown; this test pins it as explicit + ordered). + +- [ ] **Step 3: Add the field and serialize it** + +In `src/model.ts`, the `Proposal` interface — add after `granularity`: + +```ts + /** + * F12/#64 (INV-48): the pre-apply text, captured when a proposal is + * optimistically applied to the buffer. `replacement` is now in the buffer and + * `fp.text` re-anchors to it, so `original` is the only record of what to revert + * to (revert-in-place) and what to show struck in the `` half. Absent on a + * proposal created but not yet optimistically applied (or older sidecars). + */ + original?: string; +``` + +In `serializeArtifact`, the proposals `.map` canonical object — add after the `instruction` line: + +```ts + ...(p.original !== undefined ? { original: p.original } : {}), + ...(p.granularity !== undefined ? { granularity: p.granularity } : {}), +``` + +(Note: `granularity` was previously preserved only via `withUnknowns`; making both explicit keeps a deterministic order, INV-2.) + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/model.test.ts -t "round-trips Proposal.original"` +Expected: PASS + +- [ ] **Step 5: Run the full unit suite (no regressions)** + +Run: `npm test` +Expected: PASS (243+ tests) + +- [ ] **Step 6: Commit** + +```bash +git add src/model.ts test/model.test.ts +git commit -m "feat(model): add Proposal.original for F12 optimistic-apply revert (#64)" +``` + +--- + +## Task 2: `proposalModel` — carry `original` + re-anchor helper (pure) + +**Files:** +- Modify: `src/proposalModel.ts` +- Test: `test/proposalModel.test.ts` + +- [ ] **Step 1: Write the failing test** + +Add to `test/proposalModel.test.ts`: + +```ts +test("setProposalApplied stores original and re-anchors the fingerprint to the applied text", () => { + const a = emptyArtifact("doc.md"); + const { proposalId, anchorId } = addProposal( + a, + { text: "old", before: "", after: "", lineHint: 0 }, + "new", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, + { granularity: "block" }, + ); + setProposalApplied(a, proposalId, { text: "new", before: "", after: "", lineHint: 0 }, "old"); + const p = a.proposals.find((x) => x.id === proposalId)!; + assert.strictEqual(p.original, "old"); + assert.strictEqual(a.anchors[anchorId].fingerprint.text, "new"); +}); +``` + +(Add `setProposalApplied` to the existing import from `../src/proposalModel`.) + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/proposalModel.test.ts -t "setProposalApplied"` +Expected: FAIL — `setProposalApplied is not a function`. + +- [ ] **Step 3: Implement the helper** + +In `src/proposalModel.ts` add: + +```ts +import type { Fingerprint } from "./model"; + +/** + * F12/#64 (INV-48): record a proposal as optimistically applied — store the + * pre-apply `original` (for revert + the struck ``) and re-point its anchor + * fingerprint to the now-in-buffer applied text so `resolve()` finds it. Idempotent + * shape: a second call simply overwrites with the same values. + */ +export function setProposalApplied( + artifact: Artifact, + proposalId: string, + appliedFp: Fingerprint, + original: string, +): boolean { + const p = artifact.proposals.find((x) => x.id === proposalId); + if (!p) return false; + p.original = original; + artifact.anchors[p.anchorId] = { fingerprint: appliedFp }; + return true; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/proposalModel.test.ts -t "setProposalApplied"` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add src/proposalModel.ts test/proposalModel.test.ts +git commit -m "feat(proposalModel): setProposalApplied re-anchors + stores original (#64)" +``` + +--- + +## Task 3: seam `landBaseline` opt + `signalLanded` (attribution) + +**Files:** +- Modify: `src/attributionController.ts` (`applyAgentEdit` ~L247-300; add `signalLanded`) +- Test: `test/e2e/suite/f12InlineDiff.test.ts` (**new** host-E2E file — applyAgentEdit needs a live editor) + +> Rationale: `applyAgentEdit` is `vscode`-bound; it can only be exercised in the host-E2E harness. This task adds the capability and one focused E2E; the editor surface (Task 5) exercises it end-to-end. + +- [ ] **Step 1: Write the failing test** + +Create `test/e2e/suite/f12InlineDiff.test.ts` with the standard harness header (copy the top of `f12Accept.test.ts`: imports, `WS`, `settle`, `getApi`, `freshDoc`), then: + +```ts +suite("F12 inline diff — seam landBaseline (#64, INV-48)", () => { + test("applyAgentEdit with landBaseline:false applies text but does NOT advance the baseline", async () => { + const { doc, key } = await freshDoc("docs/f12-land.md", "# T\n\nalpha here.\n"); + const api = await getApi(); + await vscode.commands.executeCommand("cowriting.showTrackChangesPreview"); + await settle(); + const before = api.diffViewController.getBaseline(key)?.text ?? doc.getText(); + const start = doc.getText().indexOf("alpha"); + const ok = await api.attributionController.applyAgentEdit( + doc, + new vscode.Range(doc.positionAt(start), doc.positionAt(start + "alpha".length)), + "ALPHA", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, + { landBaseline: false }, + ); + await settle(); + assert.ok(ok, "edit applied"); + assert.ok(doc.getText().includes("ALPHA"), "text in buffer"); + assert.strictEqual(api.diffViewController.getBaseline(key)?.text ?? doc.getText(), before, "baseline unchanged"); + }); +}); +``` + +Register the new file in `test/e2e/suite/index.ts` (follow how the other `*.test.ts` files are globbed/added — it likely auto-globs `**/*.test.js`; confirm and add if explicit). + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npm run test:e2e -- --grep "landBaseline"` (or the suite's documented invocation) +Expected: FAIL — `landBaseline` ignored → baseline advances (machine-landing) so the assertion that it's unchanged fails. + +- [ ] **Step 3: Implement the opt + `signalLanded`** + +In `applyAgentEdit`, widen `opts` and gate the emitter: + +```ts + opts?: { expectedVersion?: number; turnId?: string; landBaseline?: boolean }, +``` + +Change the landing fire (~L293-298) to: + +```ts + if (ok && opts?.landBaseline !== false) { + // F6 (INV-18): a real machine landing — advance the baseline. F12 (INV-48) + // suppresses this for optimistic apply: the proposed text is in the buffer + // but the change stays PENDING (baseline at pre-proposal) until accept. + this.applyEmitter.fire({ document }); + } +``` + +Add the explicit landing signal (after `applyAgentEdit`): + +```ts + /** + * F12/#64 (INV-51): fire the machine-landing signal WITHOUT applying text — used + * by finalize-in-place, where the proposed text already landed in the buffer via + * optimistic apply (`landBaseline:false`) and accept only needs to advance the + * F6 baseline so the now-accepted change stops reading as pending. + */ + signalLanded(document: vscode.TextDocument): void { + if (this.isTracked(document)) this.applyEmitter.fire({ document }); + } +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npm run test:e2e -- --grep "landBaseline"` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add src/attributionController.ts test/e2e/suite/f12InlineDiff.test.ts test/e2e/suite/index.ts +git commit -m "feat(attribution): landBaseline opt + signalLanded for F12 optimistic apply (#64)" +``` + +--- + +## Task 4: ProposalController — optimistic apply / finalize / revert / rejectAll + +**Files:** +- Modify: `src/proposalController.ts` +- Test: `test/e2e/suite/f12InlineDiff.test.ts` + +> These methods are `vscode`-bound (buffer edits + seam). Test via host-E2E. The webview/editor surfaces call them in Tasks 5-6. + +- [ ] **Step 1: Write the failing tests** + +Append to `f12InlineDiff.test.ts`: + +```ts +suite("F12 inline diff — finalize / revert in place (#64, INV-51)", () => { + // Optimistic apply lands the text + re-anchors; the buffer becomes the accepted result. + test("optimisticApply puts the proposed text in the buffer and re-anchors", async () => { + const { doc } = await freshDoc("docs/f12-opt.md", "# T\n\nReplace alpha please.\n"); + const api = await getApi(); + const p = api.proposalController; + const start = doc.getText().indexOf("Replace alpha please."); + const fp = { text: "Replace alpha please.", before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Replace ALPHA please.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await api.proposalController.optimisticApply(doc, id!); + await settle(); + assert.ok(doc.getText().includes("Replace ALPHA please."), "applied to buffer"); + // re-anchored: the proposal still resolves against the mutated buffer + assert.strictEqual(p.listProposals(doc)[0].anchorStart !== null, true, "re-anchored, resolves"); + assert.strictEqual(p.listProposals(doc)[0].original, "Replace alpha please.", "original stored"); + }); + + test("finalizeInPlace clears the proposal and keeps the applied text (no double-apply)", async () => { + const { doc, key } = await freshDoc("docs/f12-fin.md", "# T\n\nKeep alpha now.\n"); + const api = await getApi(); + const p = api.proposalController; + const fp = { text: "Keep alpha now.", before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Keep ALPHA now.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + const ok = await p.finalizeInPlace(key, id!); + await settle(); + assert.ok(ok, "finalized"); + assert.strictEqual(doc.getText(), "# T\n\nKeep ALPHA now.\n", "applied text retained, no double-apply"); + assert.strictEqual(p.listProposals(doc).length, 0, "proposal cleared"); + }); + + test("revertInPlace restores the original and clears the proposal", async () => { + const { doc, key } = await freshDoc("docs/f12-rev.md", "# T\n\nUndo alpha here.\n"); + const api = await getApi(); + const p = api.proposalController; + const fp = { text: "Undo alpha here.", before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Undo ALPHA here.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + const ok = await p.revertInPlace(key, id!); + await settle(); + assert.ok(ok, "reverted"); + assert.strictEqual(doc.getText(), "# T\n\nUndo alpha here.\n", "original restored"); + assert.strictEqual(p.listProposals(doc).length, 0, "proposal cleared"); + }); + + test("rejectAll reverts every pending proposal", async () => { + const { doc, key } = await freshDoc("docs/f12-rejall.md", "# T\n\nOne aaa.\n\nTwo bbb.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + const p = api.proposalController; + ctl.setEditTurnForTest(async () => ({ replacement: "# T\n\nOne AAA.\n\nTwo BBB.\n", model: "m", sessionId: "s" })); + const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "up"); + await settle(); + for (const id of ids) await p.optimisticApply(doc, id); + await settle(); + assert.ok(doc.getText().includes("AAA") && doc.getText().includes("BBB"), "both applied"); + const { reverted } = await p.rejectAll(doc); + await settle(); + assert.strictEqual(reverted, ids.length, "all reverted"); + assert.strictEqual(doc.getText(), "# T\n\nOne aaa.\n\nTwo bbb.\n", "document restored"); + assert.strictEqual(p.listProposals(doc).length, 0, "all cleared"); + }); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `npm run test:e2e -- --grep "finalize / revert in place"` +Expected: FAIL — `optimisticApply` / `finalizeInPlace` / `revertInPlace` / `rejectAll` are not functions. + +- [ ] **Step 3: Implement the four methods** + +In `src/proposalController.ts`, add imports: + +```ts +import { addProposal, removeProposal, setProposalApplied } from "./proposalModel"; +import { buildFingerprint } from "./anchorer"; +``` + +Add a per-doc applied-set to `DocState`: + +```ts + /** ids already optimistically applied to the buffer (so the trigger doesn't re-apply). */ + applied: Set; +``` + +(initialize `applied: new Set()` in `ensureState`). + +Add the methods (after `acceptBlock`): + +```ts + /** True once this proposal's text is in the buffer (optimistic apply ran). */ + isApplied(docPath: string, proposalId: string): boolean { + return this.docs.get(docPath)?.applied.has(proposalId) ?? false; + } + + /** + * F12/#64 (INV-48): optimistically apply a pending proposal INTO the buffer so + * the editor shows the would-be-accepted result (editable). Reuses the F4 + * word-precise seam (block → per-word hunks, INV-40; single → whole range) but + * with `landBaseline:false` (the change stays pending). Then re-anchors the + * proposal to the applied text and stores the original (`setProposalApplied`), so + * `resolve()` finds it in the mutated buffer and revert/decorate key off it. + * Idempotent: a no-op if already applied. + */ + async optimisticApply(document: vscode.TextDocument, proposalId: string): Promise { + if (!this.isTracked(document) || this.guard.isReadOnly(this.keyOf(document))) return false; + const docPath = this.keyOf(document); + const state = this.ensureState(document); + if (state.applied.has(proposalId)) return true; + state.artifact = this.store.load(docPath) ?? emptyArtifact(docPath); + const proposal = state.artifact.proposals.find((p) => p.id === proposalId); + const fp = proposal ? state.artifact.anchors[proposal.anchorId]?.fingerprint : undefined; + if (!proposal || !fp) return false; + const resolved = resolve(document.getText(), fp); + if (resolved === "orphaned") return false; + const original = document.getText( + new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), + ); + const ok = + proposal.granularity === "block" + ? await this.applyBlockOptimistic(document, resolved, proposal) + : await this.attribution.applyAgentEdit( + document, + new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), + proposal.replacement, + proposal.author, + { expectedVersion: document.version, turnId: proposal.turnId, landBaseline: false }, + ); + if (!ok) return false; + state.applied.add(proposalId); + // Re-anchor to the applied text now in the buffer (its start is unchanged; its + // end shifts by the net length delta of the replacement). + const appliedStart = resolved.start; + const appliedEnd = appliedStart + proposal.replacement.length; + const appliedFp = buildFingerprint(document.getText(), { start: appliedStart, end: appliedEnd }); + this.store.update(docPath, (a) => setProposalApplied(a, proposalId, appliedFp, original)); + this.renderAll(document); + return true; + } + + /** Block optimistic apply: the INV-40 per-word hunks, but landBaseline:false. */ + private async applyBlockOptimistic( + document: vscode.TextDocument, + resolved: OffsetRange, + proposal: Proposal, + ): Promise { + const blockText = document.getText( + new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), + ); + const subHunks = wordEditHunks(blockText, proposal.replacement); + if (subHunks.length === 0) return true; + for (const h of [...subHunks].sort((a, b) => b.start - a.start)) { + const range = new vscode.Range( + document.positionAt(resolved.start + h.start), + document.positionAt(resolved.start + h.end), + ); + const ok = await this.attribution.applyAgentEdit(document, range, h.replacement, proposal.author, { + expectedVersion: document.version, turnId: proposal.turnId, landBaseline: false, + }); + if (!ok) return false; + } + return true; + } + + /** + * F12/#64 (INV-51): ACCEPT an optimistically-applied proposal — the text is + * already in the buffer, so this only advances the F6 baseline (machine-landing, + * via `attribution.signalLanded`) and clears the proposal. No re-application. + */ + async finalizeInPlace(docPath: string, proposalId: string): Promise { + const hit = this.byId(docPath, proposalId); + if (!hit) return false; + const document = this.openDoc(hit.state); + if (!document) return false; + this.attribution.signalLanded(document); + this.store.update(docPath, (a) => removeProposal(a, proposalId)); + hit.state.applied.delete(proposalId); + this.renderAll(document); + return true; + } + + /** + * F12/#64 (INV-51): REJECT an optimistically-applied proposal — replace its live + * applied span with the stored `original`, then clear it. Reverts the whole block + * regardless of any in-place edits the human made to the inserted text. + */ + async revertInPlace(docPath: string, proposalId: string): Promise { + const hit = this.byId(docPath, proposalId); + if (!hit) return false; + const document = this.openDoc(hit.state); + if (!document) return false; + const fp = hit.state.artifact.anchors[hit.proposal.anchorId]?.fingerprint; + const resolved = fp ? resolve(document.getText(), fp) : "orphaned"; + if (resolved !== "orphaned" && hit.proposal.original !== undefined) { + const we = new vscode.WorkspaceEdit(); + we.replace( + document.uri, + new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), + hit.proposal.original, + ); + if (!(await vscode.workspace.applyEdit(we))) return false; + } + this.store.update(docPath, (a) => removeProposal(a, proposalId)); + hit.state.applied.delete(proposalId); + this.renderAll(document); + return true; + } + + /** + * F12/#64 (INV-53): reject EVERY pending proposal on a document — revert each in + * DESCENDING anchor order (so an earlier revert never shifts a later one's + * offsets), symmetric with #46's accept-all. Returns the reverted count. + */ + async rejectAll(document: vscode.TextDocument): Promise<{ reverted: number }> { + if (!this.isTracked(document)) return { reverted: 0 }; + const docPath = this.keyOf(document); + const state = this.ensureState(document); + state.artifact = this.store.load(docPath) ?? emptyArtifact(docPath); + const text = document.getText(); + const ordered = state.artifact.proposals + .map((p) => { + const fp = state.artifact.anchors[p.anchorId]?.fingerprint; + const r = fp ? resolve(text, fp) : "orphaned"; + return { id: p.id, start: r === "orphaned" ? -1 : r.start }; + }) + .sort((a, b) => b.start - a.start); + let reverted = 0; + for (const it of ordered) if (await this.revertInPlace(docPath, it.id)) reverted++; + return { reverted }; + } +``` + +Then route the existing webview accept/reject to the new in-place semantics. Change `acceptById` and `rejectById`: + +```ts + /** Accept by id — F12: finalize the already-applied text in place (INV-51). */ + async acceptById(docPath: string, proposalId: string, opts?: { silent?: boolean }): Promise { + if (this.isApplied(docPath, proposalId)) return this.finalizeInPlace(docPath, proposalId); + // Fallback: a proposal that was never optimistically applied (e.g. orphaned at + // apply time) keeps the legacy seam-apply accept. + const hit = this.byId(docPath, proposalId); + return hit ? this.accept(hit.state, hit.proposal, opts) : false; + } + + /** Reject by id — F12: revert the applied text in place (INV-51). */ + async rejectByIdInPlace(docPath: string, proposalId: string): Promise { + if (this.isApplied(docPath, proposalId)) return this.revertInPlace(docPath, proposalId); + return this.rejectById(docPath, proposalId); + } +``` + +> Keep the existing synchronous `rejectById` (legacy / non-applied path); callers that want in-place revert use `rejectByIdInPlace`. `acceptAllProposals` already loops `acceptById`, which now finalizes-in-place for applied proposals — no change needed there. + +- [ ] **Step 4: Run to verify it passes** + +Run: `npm run test:e2e -- --grep "finalize / revert in place"` +Expected: PASS (all four tests) + +- [ ] **Step 5: Run the full unit suite (no regressions)** + +Run: `npm test` +Expected: PASS + +- [ ] **Step 6: Commit** + +```bash +git add src/proposalController.ts test/e2e/suite/f12InlineDiff.test.ts +git commit -m "feat(proposals): optimisticApply + finalize/revert in place + rejectAll (#64)" +``` + +--- + +## Task 5: `EditorProposalController` — decorations, deletion hints, CodeLens, QuickPick + +**Files:** +- Create: `src/editorProposalController.ts` +- Modify: `src/extension.ts` (construct + register), `src/trackChangesModel.ts` (export `decorationPlan`) +- Test: `test/decorationPlan.test.ts` (**new**, pure); `test/e2e/suite/f12InlineDiff.test.ts` + +### 5a — pure decoration plan (INV-49) + +- [ ] **Step 1: Write the failing test** + +Create `test/decorationPlan.test.ts`: + +```ts +import { describe, test, expect } from "vitest"; +import { decorationPlan } from "../src/trackChangesModel"; + +describe("decorationPlan (INV-49)", () => { + test("derives insertion ranges + deletion hints from original→replacement", () => { + // anchorStart 10; original 'the brown fox' → 'the red fox' + const plan = decorationPlan(10, "the brown fox", "the red fox"); + // one changed run: 'brown ' → 'red ' (word-level); insertion 'red ' tinted, + // deletion hint 'brown ' shown at the run start. + expect(plan.insertions.length).toBeGreaterThan(0); + expect(plan.deletions.length).toBeGreaterThan(0); + // insertion offsets are in buffer coords (>= anchorStart) and lie within the applied text + for (const ins of plan.insertions) { + expect(ins.start).toBeGreaterThanOrEqual(10); + expect(ins.end).toBeLessThanOrEqual(10 + "the red fox".length); + } + // a deletion hint carries the struck original text at a buffer offset + expect(plan.deletions[0].text).toContain("brown"); + }); + + test("pure insertion (no deletion) yields an insertion range and no deletion hint", () => { + const plan = decorationPlan(0, "ab", "axb"); + expect(plan.insertions.length).toBe(1); + expect(plan.deletions.length).toBe(0); + }); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `npx vitest run test/decorationPlan.test.ts` +Expected: FAIL — `decorationPlan` not exported. + +- [ ] **Step 3: Implement `decorationPlan`** + +In `src/trackChangesModel.ts` (after `wordEditHunks`), add: + +```ts +/** F12/#64 (INV-49/52): the editor render of one optimistically-applied proposal. */ +export interface DecorationPlan { + /** buffer ranges of inserted (proposed) text → tinted (INV-52). */ + insertions: { start: number; end: number }[]; + /** struck original text shown as a non-editable hint at a buffer offset (INV-52). */ + deletions: { at: number; text: string }[]; +} + +/** + * F12/#64 (INV-49): compute the editor decoration plan for a proposal whose applied + * text occupies `[anchorStart, anchorStart+replacement.length)` in the buffer. The + * SAME word diff the webview uses (`wordEditHunks`) drives it, so both surfaces show + * the identical diff. A changed run maps to (a) an insertion range over the run's + * applied text and (b) a deletion hint carrying the run's removed original text at + * the run start; a pure insertion has no deletion hint; a pure deletion has only a + * hint. Pure, vscode-free, deterministic. + */ +export function decorationPlan(anchorStart: number, original: string, replacement: string): DecorationPlan { + const insertions: { start: number; end: number }[] = []; + const deletions: { at: number; text: string }[] = []; + // Walk the word diff once, tracking the applied-side offset as we consume parts. + let appliedOffset = anchorStart; + for (const part of diffWordsWithSpace(original, replacement)) { + if (part.added) { + insertions.push({ start: appliedOffset, end: appliedOffset + part.value.length }); + appliedOffset += part.value.length; + } else if (part.removed) { + deletions.push({ at: appliedOffset, text: part.value }); + // removed text is NOT in the applied buffer → appliedOffset does not advance + } else { + appliedOffset += part.value.length; + } + } + return { insertions, deletions }; +} +``` + +(`diffWordsWithSpace` is already imported at the top of the file.) + +- [ ] **Step 4: Run to verify it passes** + +Run: `npx vitest run test/decorationPlan.test.ts` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add src/trackChangesModel.ts test/decorationPlan.test.ts +git commit -m "feat(render): pure decorationPlan feeds editor + webview from one diff (#64, INV-49)" +``` + +### 5b — the controller (editor surface) + +- [ ] **Step 6: Write the controller** + +Create `src/editorProposalController.ts`: + +```ts +/** + * EditorProposalController — F12/#64 editor surface (spec coauthoring-inline-editor-diff + * §3.5). Reverses INV-32 for pending proposals: on `onDidChangeProposals` it + * (1) optimistically applies any not-yet-applied proposal into the active editor's + * buffer (ProposalController.optimisticApply, INV-48), (2) decorates each applied + * proposal — insertion tint over the proposed text + a non-editable struck-red hint + * for deletions (INV-52, decorationPlan/INV-49), and (3) provides a CodeLens + * `Accept ▾ / Reject ▾` above each block whose ▾ opens a QuickPick (this / all). + * Owns no proposal STATE — it is a view over ProposalController (which stays the + * pure F4 owner). A document with no pending proposals shows nothing (INV-32's + * spirit when nothing is pending). + */ +import * as vscode from "vscode"; +import type { ProposalController } from "./proposalController"; +import { decorationPlan } from "./trackChangesModel"; +import { isAuthorable } from "./workspacePath"; + +export class EditorProposalController implements vscode.Disposable, vscode.CodeLensProvider { + private readonly disposables: vscode.Disposable[] = []; + private readonly insertionDeco = vscode.window.createTextEditorDecorationType({ + backgroundColor: new vscode.ThemeColor("diffEditor.insertedTextBackground"), + }); + private readonly deletionDeco = vscode.window.createTextEditorDecorationType({ + // a non-editable struck-red hint injected AFTER the insertion (INV-52) + after: { color: new vscode.ThemeColor("gitDecoration.deletedResourceForeground") }, + textDecoration: "none", + }); + private readonly lensEmitter = new vscode.EventEmitter(); + readonly onDidChangeCodeLenses = this.lensEmitter.event; + + constructor(private readonly proposals: ProposalController) { + this.disposables.push( + this.insertionDeco, this.deletionDeco, this.lensEmitter, + vscode.languages.registerCodeLensProvider({ language: "markdown" }, this), + this.proposals.onDidChangeProposals(({ uri }) => void this.onProposalsChanged(uri)), + vscode.window.onDidChangeActiveTextEditor((ed) => ed && this.renderEditor(ed)), + // the four QuickPick-backed menu commands + vscode.commands.registerCommand("cowriting.proposalAcceptMenu", (id?: string) => this.menu("accept", id)), + vscode.commands.registerCommand("cowriting.proposalRejectMenu", (id?: string) => this.menu("reject", id)), + ); + } + + /** Apply any not-yet-applied proposals on the doc, then re-decorate + refresh lenses. */ + private async onProposalsChanged(uri: string): Promise { + const doc = vscode.workspace.textDocuments.find((d) => d.uri.toString() === uri); + if (!doc || doc.languageId !== "markdown" || !isAuthorable(doc.uri.scheme)) return; + const key = this.proposals.keyFor(doc); + for (const v of this.proposals.listProposals(doc)) { + if (v.anchorStart !== null && !this.proposals.isApplied(key, v.id)) { + await this.proposals.optimisticApply(doc, v.id); // fires onDidChangeProposals again; guarded by isApplied + } + } + const ed = vscode.window.visibleTextEditors.find((e) => e.document.uri.toString() === uri); + if (ed) this.renderEditor(ed); + this.lensEmitter.fire(); + } + + /** Decorate the editor for every applied proposal on its document (INV-52). */ + private renderEditor(editor: vscode.TextEditor): void { + const doc = editor.document; + if (doc.languageId !== "markdown") { + editor.setDecorations(this.insertionDeco, []); + editor.setDecorations(this.deletionDeco, []); + return; + } + const key = this.proposals.keyFor(doc); + const insertions: vscode.Range[] = []; + const deletions: vscode.DecorationOptions[] = []; + for (const v of this.proposals.listProposals(doc)) { + if (v.anchorStart === null || v.original === undefined || !this.proposals.isApplied(key, v.id)) continue; + const plan = decorationPlan(v.anchorStart, v.original, v.replacement); + for (const ins of plan.insertions) { + insertions.push(new vscode.Range(doc.positionAt(ins.start), doc.positionAt(ins.end))); + } + for (const del of plan.deletions) { + deletions.push({ + range: new vscode.Range(doc.positionAt(del.at), doc.positionAt(del.at)), + renderOptions: { after: { contentText: ` ${del.text} `, textDecoration: "line-through" } }, + }); + } + } + editor.setDecorations(this.insertionDeco, insertions); + editor.setDecorations(this.deletionDeco, deletions); + } + + /** CodeLensProvider: a `Accept ▾` / `Reject ▾` pair above each applied block. */ + provideCodeLenses(document: vscode.TextDocument): vscode.CodeLens[] { + if (document.languageId !== "markdown") return []; + const key = this.proposals.keyFor(document); + const lenses: vscode.CodeLens[] = []; + for (const v of this.proposals.listProposals(document)) { + if (v.anchorStart === null || !this.proposals.isApplied(key, v.id)) continue; + const pos = document.positionAt(v.anchorStart); + const line = new vscode.Range(pos.line, 0, pos.line, 0); + lenses.push( + new vscode.CodeLens(line, { title: "Accept ▾", command: "cowriting.proposalAcceptMenu", arguments: [v.id] }), + new vscode.CodeLens(line, { title: "Reject ▾", command: "cowriting.proposalRejectMenu", arguments: [v.id] }), + ); + } + return lenses; + } + + /** The dropdown: this-proposal vs all-proposals, then dispatch. */ + private async menu(kind: "accept" | "reject", id?: string): Promise { + const doc = vscode.window.activeTextEditor?.document; + if (!doc || !id) return; + const key = this.proposals.keyFor(doc); + const verb = kind === "accept" ? "Accept" : "Reject"; + const pick = await vscode.window.showQuickPick( + [`${verb} this proposal`, `${verb} ALL proposals`], + { placeHolder: `${verb} Claude's proposal` }, + ); + if (!pick) return; + const all = pick.includes("ALL"); + if (kind === "accept") { + if (all) await this.proposals.acceptAllProposals(doc); + else await this.proposals.finalizeInPlace(key, id); + } else { + if (all) await this.proposals.rejectAll(doc); + else await this.proposals.revertInPlace(key, id); + } + } + + dispose(): void { + for (const d of this.disposables) d.dispose(); + } +} +``` + +> `ProposalView` must now carry `original`. In `src/trackChangesModel.ts` add `original?: string;` to `ProposalView`, and in `ProposalController.listProposals` set `original: p.original` on each returned view. + +- [ ] **Step 7: Wire it into `extension.ts`** + +After the `trackChangesPreviewController` block (~L124), add: + +```ts + // --- F12 (#64): the editor surface — optimistic-apply proposals into the buffer, + // decorate the diff, and provide Accept ▾/Reject ▾ CodeLens (INV-48/49/52/53). --- + const editorProposalController = new EditorProposalController(proposalController); + context.subscriptions.push(editorProposalController); +``` + +Add the import at the top and the field to `CowritingApi`: + +```ts +import { EditorProposalController } from "./editorProposalController"; +``` +```ts + editorProposalController: EditorProposalController; +``` +and return it in the API object. + +- [ ] **Step 8: Write the E2E for the editor surface** + +Append to `f12InlineDiff.test.ts`: + +```ts +suite("F12 inline diff — editor surface (#64, INV-48/52)", () => { + test("proposing optimistically applies into the editor and the buffer is the accepted result", async () => { + const { doc } = await freshDoc("docs/f12-editor.md", "# E\n\nThe brown fox runs.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + ctl.setEditTurnForTest(async () => ({ replacement: "# E\n\nThe red fox runs.\n", model: "m", sessionId: "s" })); + await ctl.runEditAndPropose(doc, { kind: "document" }, "recolor the fox"); + await settle(); await settle(); + assert.ok(doc.getText().includes("The red fox runs."), "optimistically applied into the buffer"); + const v = api.proposalController.listProposals(doc)[0]; + assert.strictEqual(v.original, "The brown fox runs.", "original captured for the deletion hint/revert"); + }); + + test("editing the inserted text then finalizing keeps the human edit", async () => { + const { doc, key } = await freshDoc("docs/f12-edit-keep.md", "# E\n\nalpha word here.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + ctl.setEditTurnForTest(async () => ({ replacement: "# E\n\nALPHA word here.\n", model: "m", sessionId: "s" })); + const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "up"); + await settle(); await settle(); + // human tweaks the inserted text + const at = doc.getText().indexOf("ALPHA"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + 5)), "ALPHA!"); + await vscode.workspace.applyEdit(we); + await settle(); + await api.proposalController.finalizeInPlace(key, ids[0]); + await settle(); + assert.ok(doc.getText().includes("ALPHA! word here."), "human edit preserved on accept"); + assert.strictEqual(api.proposalController.listProposals(doc).length, 0, "cleared"); + }); +}); +``` + +- [ ] **Step 9: Run to verify it passes** + +Run: `npm run typecheck && npm run test:e2e -- --grep "editor surface"` +Expected: PASS + +- [ ] **Step 10: Commit** + +```bash +git add src/editorProposalController.ts src/extension.ts src/trackChangesModel.ts src/proposalController.ts test/e2e/suite/f12InlineDiff.test.ts +git commit -m "feat(editor): EditorProposalController — optimistic apply + decorations + CodeLens (#64)" +``` + +--- + +## Task 6: render-once reconciliation (INV-50) + +**Files:** +- Modify: `src/trackChangesModel.ts` (`renderReview`), `src/trackChangesPreview.ts` (pass `original` to `ProposalView`) +- Test: `test/trackChangesModel.test.ts` + +- [ ] **Step 1: Write the failing test** + +Add to `test/trackChangesModel.test.ts`: + +```ts +test("renderReview renders an applied proposal ONCE, not also as a landed diff (INV-50)", () => { + const baseline = "# T\n\nThe brown fox.\n"; + const current = "# T\n\nThe red fox.\n"; // proposal already optimistically applied + const proposals = [{ + id: "pr_1", + anchorStart: current.indexOf("The red fox."), + anchorEnd: current.indexOf("The red fox.") + "The red fox.".length, + replaced: "The brown fox.", // original + replacement: "The red fox.", + }]; + const html = renderReview(baseline, current, [], proposals); + // exactly one proposal block + expect((html.match(/cw-proposal/g) ?? []).length).toBe(1); + // the applied paragraph is NOT also emitted as a word-merged changed block + expect(html).not.toContain("brown"); // no double-render of the change as a baseline diff +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `npx vitest run test/trackChangesModel.test.ts -t "renders an applied proposal ONCE"` +Expected: FAIL — the baseline→current diff renders the changed block AND the proposal block (double render). + +- [ ] **Step 3: Implement the reconciliation** + +In `renderReview` (`src/trackChangesModel.ts`), before computing `ops`, revert each *resolved* proposal's applied span back to its `replaced` (original) so the landed diff excludes pending proposals: + +```ts + // F12/#64 (INV-50): with optimistic apply the proposed text is already in + // `currentText`, so a naive baseline→current diff would render each proposed + // change BOTH as a landed diff and as its proposal block. Diff against + // `currentText` with every resolved pending proposal reverted to its original, + // so the landed diff excludes pending proposals; they render once, as proposals. + const pendingApplied = proposals + .filter((p) => p.anchorStart !== null) + .sort((a, b) => b.anchorStart! - a.anchorStart!); + let landedText = currentText; + for (const p of pendingApplied) { + landedText = landedText.slice(0, p.anchorStart!) + p.replaced + landedText.slice(p.anchorEnd!); + } +``` + +Then change the two derivations to diff/range against `landedText` for the baseline diff, while still mapping proposals by their `currentText` anchor. Concretely, replace: + +```ts + const ranges = splitBlocksWithRanges(currentText); + const ops = diffBlocks(baselineText, currentText); +``` + +with: + +```ts + const ranges = splitBlocksWithRanges(landedText); + const ops = diffBlocks(baselineText, landedText); +``` + +and map each proposal's `anchorStart` into `landedText` coordinates for the `blockOf` placement (the revert shifts later offsets). Add, before the `blockOf` loop, a coordinate mapper: + +```ts + // Map a currentText offset to its landedText offset (account for reverted spans + // that precede it; reverts were applied high→low so the cumulative delta is stable). + const toLanded = (curOff: number): number => { + let delta = 0; + for (const p of [...pendingApplied].sort((a, b) => a.anchorStart! - b.anchorStart!)) { + if (p.anchorStart! < curOff) delta += p.replaced.length - (p.anchorEnd! - p.anchorStart!); + } + return curOff + delta; + }; +``` + +and use `blockOf(toLanded(p.anchorStart))` when placing each resolved proposal. + +> The proposal blocks themselves still render from `replaced`→`replacement` (unchanged `proposalBlockHtml`), so the `originalapplied` is correct. + +- [ ] **Step 4: Run to verify it passes** + +Run: `npx vitest run test/trackChangesModel.test.ts -t "renders an applied proposal ONCE"` +Expected: PASS + +- [ ] **Step 5: Pass `original` through to the view** + +In `src/trackChangesModel.ts` `ProposalView`, `replaced` already holds the original — but with optimistic apply, `ProposalController.listProposals` currently sets `replaced: fp.text`, and `fp.text` is now the APPLIED text (re-anchored). Fix `listProposals` to use `p.original ?? fp.text` for `replaced`: + +```ts + replaced: p.original ?? fp?.text ?? "", +``` + +Add a unit/E2E assertion that an applied proposal's `replaced` is the original (in `f12InlineDiff.test.ts`): + +```ts + test("listProposals reports the original as `replaced` after optimistic apply", async () => { + const { doc } = await freshDoc("docs/f12-replaced.md", "# R\n\nbrown here.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + ctl.setEditTurnForTest(async () => ({ replacement: "# R\n\nred here.\n", model: "m", sessionId: "s" })); + await ctl.runEditAndPropose(doc, { kind: "document" }, "x"); + await settle(); await settle(); + assert.strictEqual(api.proposalController.listProposals(doc)[0].replaced, "brown here."); + }); +``` + +- [ ] **Step 6: Run to verify + full suite** + +Run: `npm test && npm run typecheck` +Expected: PASS + +- [ ] **Step 7: Commit** + +```bash +git add src/trackChangesModel.ts src/proposalController.ts test/trackChangesModel.test.ts test/e2e/suite/f12InlineDiff.test.ts +git commit -m "feat(render): INV-50 render-once — diff against current-minus-pending (#64)" +``` + +--- + +## Task 7: webview controls (`Accept ▾`/`Reject ▾` + dropdown) + reject-all command + package.json + +**Files:** +- Modify: `src/trackChangesModel.ts` (`proposalBlockHtml`), `media/preview.ts`, `media/preview.css`, `src/trackChangesPreview.ts` (route `rejectAll`), `src/extension.ts` (`cowriting.rejectAllProposals`), `package.json` +- Test: `test/trackChangesModel.test.ts`, `test/e2e/suite/f12InlineDiff.test.ts` + +- [ ] **Step 1: Write the failing test (webview HTML controls)** + +Add to `test/trackChangesModel.test.ts`: + +```ts +test("proposalBlockHtml renders Accept/Reject controls with dropdown carets (#64)", () => { + const html = renderReview("a\n", "b\n", [], [ + { id: "pr_1", anchorStart: 0, anchorEnd: 1, replaced: "a", replacement: "b" }, + ]); + assert.match(html, /data-action="accept"/); + assert.match(html, /data-action="reject"/); + assert.match(html, /data-action="acceptAll"/); + assert.match(html, /data-action="rejectAll"/); + assert.match(html, /Accept/); + assert.match(html, /Reject/); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `npx vitest run test/trackChangesModel.test.ts -t "Accept/Reject controls with dropdown"` +Expected: FAIL — current HTML emits ✓/✗ glyphs, no `acceptAll`/`rejectAll` actions. + +- [ ] **Step 3: Update `proposalBlockHtml`** + +Replace the `actions` block (`src/trackChangesModel.ts` ~L717-721): + +```ts + const actions = + `` + + `` + + `` + + `` + + `` + + `` + + `` + + `` + + `` + + ``; +``` + +- [ ] **Step 4: Run to verify it passes** + +Run: `npx vitest run test/trackChangesModel.test.ts -t "Accept/Reject controls with dropdown"` +Expected: PASS + +- [ ] **Step 5: Wire the webview client + CSS** + +In `media/preview.ts`, replace the delegated proposal-button handler (~L97-107): + +```ts +// F12/#64: Accept/Reject (+ caret → accept-all/reject-all) on a proposal block. +body.addEventListener("click", (e) => { + const btn = (e.target as HTMLElement)?.closest(".cw-actions button"); + if (!btn) return; + const id = btn.closest(".cw-proposal")?.dataset.proposalId; + const action = btn.dataset.action; + if (action === "acceptAll") return void vscodeApi.postMessage({ type: "acceptAll" }); + if (action === "rejectAll") return void vscodeApi.postMessage({ type: "rejectAll" }); + if (id && (action === "accept" || action === "reject")) { + vscodeApi.postMessage({ type: action, proposalId: id }); + } +}); +``` + +In `media/preview.css`, add under the F10 block: + +```css +.cw-btngroup { display: inline-flex; } +.cw-btngroup .cw-caret { border-left: none; padding: 0.1em 0.25em; } +.cw-actions .cw-accept { font-weight: 600; } +.cw-accept:hover, .cw-btngroup:has(.cw-accept) .cw-caret:hover { background: var(--vscode-testing-iconPassed, #2ea043); color: #fff; } +.cw-reject:hover, .cw-btngroup:has(.cw-reject) .cw-caret:hover { background: var(--vscode-errorForeground, #f14c4c); color: #fff; } +``` + +- [ ] **Step 6: Route `rejectAll` in the host + add the command** + +In `src/trackChangesPreview.ts`, add `rejectAll` to the `ToolbarMsg` union: + +```ts + | { type: "acceptAll" } + | { type: "rejectAll" }; +``` + +In `handleWebviewMessage`, after the `acceptAll` branch: + +```ts + } else if (m?.type === "rejectAll") { + void this.rejectAll(document); +``` + +And update the existing accept/reject branches to the in-place revert for reject: + +```ts + } else if (m?.type === "reject" && m.proposalId) { + void this.proposals.rejectByIdInPlace(this.proposals.keyFor(document), m.proposalId).then(() => this.refresh(document)); +``` + +Add a public `rejectAll` (mirrors `acceptAll`): + +```ts + /** #64 (INV-53): revert every pending proposal on the document; report the count. */ + async rejectAll(document: vscode.TextDocument): Promise { + const { reverted } = await this.proposals.rejectAll(document); + this.refresh(document); + if (reverted > 0) { + void vscode.window.showInformationMessage( + `Cowriting: rejected ${reverted} proposal${reverted === 1 ? "" : "s"}.`, + ); + } + } +``` + +In `src/extension.ts`, register the command (next to `acceptAllProposals` ~L129): + +```ts + context.subscriptions.push( + vscode.commands.registerCommand("cowriting.rejectAllProposals", async () => { + const doc = vscode.window.activeTextEditor?.document; + if (!doc || doc.languageId !== "markdown") { + void vscode.window.showWarningMessage("Cowriting: open a Markdown document to reject its proposals."); + return; + } + await trackChangesPreviewController.rejectAll(doc); + }), + ); +``` + +- [ ] **Step 7: Update `package.json` contributes** + +Add to `contributes.commands`: + +```json +{ "command": "cowriting.rejectAllProposals", "title": "Reject All Claude Proposals" }, +{ "command": "cowriting.proposalAcceptMenu", "title": "Accept Claude Proposal" }, +{ "command": "cowriting.proposalRejectMenu", "title": "Reject Claude Proposal" } +``` + +Add `commandPalette` `when` guards (mirror `acceptAllProposals`): + +```json +{ "command": "cowriting.rejectAllProposals", "when": "editorLangId == markdown" }, +{ "command": "cowriting.proposalAcceptMenu", "when": "false" }, +{ "command": "cowriting.proposalRejectMenu", "when": "false" } +``` + +- [ ] **Step 8: Write the E2E (both surfaces resolve identically)** + +Append to `f12InlineDiff.test.ts`: + +```ts +suite("F12 inline diff — control parity (#64, INV-53)", () => { + test("reject from the webview reverts in place; rejectAll clears every proposal", async () => { + const { doc, key } = await freshDoc("docs/f12-parity.md", "# P\n\nuno aaa.\n\ndos bbb.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + await vscode.commands.executeCommand("cowriting.showTrackChangesPreview"); + await settle(); + ctl.setEditTurnForTest(async () => ({ replacement: "# P\n\nuno AAA.\n\ndos BBB.\n", model: "m", sessionId: "s" })); + const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "up"); + await settle(); await settle(); + assert.ok(doc.getText().includes("AAA") && doc.getText().includes("BBB")); + // reject ONE via the webview intent → that block reverts, the other stays applied + ctl.receiveMessage(key, { type: "reject", proposalId: ids[0] }); + await settle(); await settle(); + assert.ok(doc.getText().includes("uno aaa.") && doc.getText().includes("BBB"), "one reverted, one applied"); + // rejectAll via the command → all gone, document restored + ctl.receiveMessage(key, { type: "rejectAll" }); + await settle(); await settle(); + assert.strictEqual(doc.getText(), "# P\n\nuno aaa.\n\ndos bbb.\n", "document restored"); + assert.strictEqual(api.proposalController.listProposals(doc).length, 0); + }); + + test("cowriting.rejectAllProposals is registered + markdown-guarded", async () => { + const all = await vscode.commands.getCommands(true); + assert.ok(all.includes("cowriting.rejectAllProposals")); + const pkg = JSON.parse(fs.readFileSync(path.join(__dirname, "../../../../package.json"), "utf8")); + const entry = (pkg.contributes.menus.commandPalette as Array<{ command: string; when?: string }>).find( + (m) => m.command === "cowriting.rejectAllProposals", + ); + assert.ok(entry && /editorLangId == markdown/.test(entry.when ?? ""), "palette-guarded on markdown"); + }); +}); +``` + +- [ ] **Step 9: Build the webview + run the full pipeline** + +Run: `npm run build && npm run typecheck && npm test && npm run test:e2e` +Expected: all PASS (unit suite + the full host-E2E including the new `f12InlineDiff` suite, F10/F11/F12 existing suites green). + +- [ ] **Step 10: Commit** + +```bash +git add src/trackChangesModel.ts media/preview.ts media/preview.css src/trackChangesPreview.ts src/extension.ts package.json test/trackChangesModel.test.ts test/e2e/suite/f12InlineDiff.test.ts +git commit -m "feat(webview): Accept/Reject + dropdown, rejectAll, control parity (#64, INV-53)" +``` + +--- + +## Task 8: regression sweep + manual smoke + +- [ ] **Step 1: Full green gate** + +Run: `npm run typecheck && npm test && npm run test:e2e` +Expected: typecheck clean; full unit suite PASS; host-E2E PASS (no regressions in F4 proposals, F10 review, F11 toolbar, F12 accept/reach/review suites). + +- [ ] **Step 2: Manual smoke (per spec §4)** + +Launch the extension (F5 / `code --extensionDevelopmentPath`), open a Markdown file, Ask Claude to edit a paragraph, and confirm: +- the editor shows the change applied, inserted text tinted, a struck-red deletion hint, and `Accept ▾`/`Reject ▾` CodeLens above the block; +- editing the inserted text then Accept keeps the edit; +- Reject restores the original; +- Accept-all / Reject-all work from the CodeLens dropdown AND the webview; +- the webview shows the same diff as the editor for the same proposal; +- saving while pending writes the proposed text (no decoration markup in the file); +- after all proposals are resolved, the editor is clean again. + +- [ ] **Step 3: Update the spec invariant note (optional, low-risk)** + +Note the re-anchor refinement (Task intro) in the spec's §3.2 if updating the content-repo copy; otherwise it rides the finalize deferred-decisions report. + +--- + +## Self-review notes (planner) + +- **Spec coverage:** INV-48 (Task 4/5), INV-49 (Task 5a), INV-50 (Task 6), INV-51 (Task 4), INV-52 (Task 5), INV-53 (Task 7), INV-54 (save persists — inherent to optimistic apply; covered by the editor-surface E2E asserting the buffer holds the proposed text + a smoke check; an explicit save-and-read E2E can be added if desired). §2.2 edit-then-accept (Task 5 E2E). §2.5 rejected alternatives — N/A to code. +- **Risk (INV-50 reconciliation):** Task 6's coordinate mapping (`toLanded`) is the subtlest piece; its unit test pins the no-double-render guarantee. If a multi-proposal layout edge appears, extend the unit test before touching the mapper. +- **Concurrency:** the optimistic-apply trigger re-enters via the sidecar watcher; the `isApplied` guard (Task 4) makes it idempotent. Verify in the editor-surface E2E that N document proposals each apply exactly once. +- **Out of scope (YAGNI):** live token-streaming into the editor; non-Markdown; intra-diagram mermaid editing — all per spec §2.4. diff --git a/media/preview.css b/media/preview.css index cdfdde1..937e63f 100644 --- a/media/preview.css +++ b/media/preview.css @@ -89,6 +89,11 @@ pre.mermaid[data-cw-error] { color: var(--vscode-errorForeground); } } .cw-accept:hover { background: var(--vscode-testing-iconPassed, #2ea043); color: #fff; } .cw-reject:hover { background: var(--vscode-errorForeground, #f14c4c); color: #fff; } +.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; } /* 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; } diff --git a/media/preview.ts b/media/preview.ts index b42e8ab..f4769e2 100644 --- a/media/preview.ts +++ b/media/preview.ts @@ -94,13 +94,14 @@ acceptAllEl?.addEventListener("click", () => { vscodeApi.postMessage({ type: "acceptAll" }); }); -// F10: delegated ✓/✗ accept/reject of pending proposals (routed back to the F4 seam). +// 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 block = btn.closest(".cw-proposal"); - const id = block?.dataset.proposalId; + 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 }); } diff --git a/package.json b/package.json index e8c2e90..913b6fa 100644 --- a/package.json +++ b/package.json @@ -103,6 +103,21 @@ "command": "cowriting.acceptAllProposals", "title": "Accept All Claude Proposals", "category": "Cowriting" + }, + { + "command": "cowriting.rejectAllProposals", + "title": "Reject All Claude Proposals", + "category": "Cowriting" + }, + { + "command": "cowriting.proposalAcceptMenu", + "title": "Accept Claude Proposal", + "category": "Cowriting" + }, + { + "command": "cowriting.proposalRejectMenu", + "title": "Reject Claude Proposal", + "category": "Cowriting" } ], "menus": { @@ -142,6 +157,18 @@ { "command": "cowriting.acceptAllProposals", "when": "editorLangId == markdown" + }, + { + "command": "cowriting.rejectAllProposals", + "when": "editorLangId == markdown" + }, + { + "command": "cowriting.proposalAcceptMenu", + "when": "false" + }, + { + "command": "cowriting.proposalRejectMenu", + "when": "false" } ], "editor/title": [ diff --git a/specs/coauthoring-inline-editor-diff.md b/specs/coauthoring-inline-editor-diff.md new file mode 100644 index 0000000..d45300c --- /dev/null +++ b/specs/coauthoring-inline-editor-diff.md @@ -0,0 +1,445 @@ +--- +status: graduated +--- +# Solution Design: Inline editable proposed-change diff in the Markdown editor (#64) + +| | | +| --- | --- | +| **Author(s)** | Ben Stull (with Claude) | +| **Reviewers / approvers** | Ben Stull | +| **Status** | `graduated` | +| **Version** | v0.1.1 | +| **Source artifacts** | Feature `benstull/vscode-cowriting-plugin#64` (Inline editable proposed-change diff in the Markdown editor + Accept/Reject controls in both surfaces, `type/feature`, `priority/P2`) · Brainstorming session `vscode-cowriting-plugin-0058` (2026-06-26) · Builds on (all shipped): F4 `#12` (propose/accept seam, INV-9/10/11/13), F6 `#17` (baseline / machine-landing), F7 `#21` (rendered preview, pure render engine INV-22), F10 `#29` (interactive review preview — the **clean-editor** decision INV-32; ✓/✗ accept-reject), F11 `#43` (preview toolbar, `data-src` block→source mapping INV-36), document-edit-flow `#42/#47/#46` (block proposals INV-39/40, accept-all INV-42) · Parent specs (graduated): `coauthoring-propose-accept.md`, `coauthoring-diff-view.md`, `coauthoring-rendered-preview.md`, `coauthoring-interactive-review.md`, `coauthoring-document-edit-flow.md` · Lineage: `ben.stull/rfc-app#48` | + +**Change log** + +| Date | Version | Change | By | +| --- | --- | --- | --- | +| 2026-06-26 | v0.1.1 | Implementation refinement (planning-and-executing session 0059): made the **re-anchor** explicit. Optimistic apply (§3.2) must store the pre-apply text on a new `Proposal.original` field **and** re-fingerprint the proposal's anchor to the *applied* text — F4's `fp.text` is the *original* target, which leaves the buffer once the replacement lands, so `resolve()` would orphan every applied proposal otherwise. `finalizeInPlace`/`revertInPlace`/decorate all key off the re-anchored fp; revert restores `original`. Reload-safety (INV-51/54): a proposal already carrying `original` is never re-captured (the in-memory applied-set is empty after a window reload), so the revert target survives save+reload. Shipped #64 (PR, session 0059). | +| 2026-06-26 | v0.1.0 | Initial draft — brainstorming session 0058. Four forks locked with the operator: **(1) editor model** = *optimistic apply + decorations* — on propose, the editor buffer becomes the would-be-accepted text (insertions real & editable & tinted; deletions shown as struck-red non-editable hints); accept = finalize-in-place, reject = revert. **(2) timing** = *on proposal* (turn complete), not a live token-stream into the editor (that stays in #60's notification/OutputChannel). **(3) editor affordance** = CodeLens `Accept ▾ / Reject ▾` above each block, `▾` → QuickPick (this / all); the webview keeps HTML buttons with the same dropdown. **(4) controls parity** = Accept / Reject / Accept-all / Reject-all reachable from **both** the editor and the webview. Two sub-decisions confirmed: a **dedicated `EditorProposalController`** owns optimistic-apply + decorations + CodeLens (keeps `ProposalController` the pure F4 state/seam owner); **saving while pending persists** the proposed (accepted-result) text. Reverses INV-32 and INV-10; supersedes the ✓/✗ glyph controls. | Ben Stull + Claude | + +--- + +## 1. Business Context + +### 1.1 Executive Summary + +Today, when Claude proposes an edit, the change is shown **only in the rendered +review webview** (`oldnew` with ✓/✗ controls); the Markdown +**editor stays deliberately clean** (F10 / INV-32 — "the preview is the single +review surface"). The writer who lives in the editor sees nothing: to review what +Claude proposed they must open the preview panel, and they cannot *edit* the +proposed text in place — they can only accept or reject it wholesale in the +webview. + +This design brings the proposed change **into the Markdown editor itself**: +editable, with a track-changes diff that matches the webview and the post-accept +result **exactly**. It also replaces the per-proposal `✓`/`✗` glyphs with labelled +**Accept** / **Reject** controls — each carrying a `▾` dropdown for **Accept all** +/ **Reject all** — and makes all four actions reachable from **both** the editor +and the webview. The writer can now read, tweak, and resolve a proposal without +leaving the document. + +### 1.2 Background + +The plugin's review model (graduated specs `coauthoring-propose-accept.md`, +`coauthoring-rendered-preview.md`, `coauthoring-interactive-review.md`): + +- A machine edit becomes an **F4 proposal** — a pending-only record + (`Proposal{ id, anchorId, replacement, granularity }` + a `Fingerprint` anchor + whose `text` is the exact target) that **never mutates the document** (INV-10), + re-resolved against the live text at accept (INV-11) and cleared when + accepted/rejected (INV-13). +- The **render engine** (`trackChangesModel.ts`) is a pure, deterministic, + `vscode`-free unit (INV-22): it diffs at **block** granularity (INV-39), word- + merges prose blocks, and emits webview HTML — `renderReview` (annotated) / + `renderPlain` (clean) — with proposal blocks (`proposalBlockHtml`) carrying the + ✓/✗ buttons and `data-src` offsets for selection mapping (INV-36). +- The **F10 decision (INV-32)** removed *all* in-editor decorations + (`grep` confirms zero `TextEditorDecorationType` / `setDecorations` in the tree) + so the webview is the sole review surface; `ProposalController` carries the note + *"no in-editor UI — INV-32 makes the rendered preview the single review surface."* +- The sealed webview is **intent-only** (INV-35): it posts `{accept|reject| + acceptAll|…}` to the host, which routes to `ProposalController.acceptById` / + `rejectById` / `acceptAllProposals` (#46 / INV-42). Accept **applies** the + replacement via the F4 seam (`applyAgentEdit`), which advances the F6 baseline + (machine-landing, INV-18). + +This feature **deliberately reverses two of those decisions** for the editor +surface, and supersedes the ✓/✗ glyph controls in both surfaces. + +### 1.3 Who feels it & why + +The **human coauthor working in the editor** — the writer who asked Claude to edit +and wants to see, adjust, and resolve the result without context-switching to the +preview panel. The pain: the proposed change is invisible in the place they are +actually writing, and it is not editable at all (only accept/reject-able). P2 +(issue #64): it materially deepens the inner-loop "edit-in-place" experience and +unifies the review controls, but the webview review path already works, so it is +an enhancement rather than a gap. + +--- + +## 2. Product Design + +### 2.1 The experience + +When Claude's turn lands a proposal (or, for a document edit, N block proposals), +the **editor** shows the change as track-changes, in place: + +``` + Accept ▾ Reject ▾ ← CodeLens, above each proposed block + The quick ~~brown~~ red fox jumps over the lazy dog. + └ struck red ┘└ green/blue, editable ┘ +``` + +- **Insertions** are **real, editable buffer text**, tinted by origin + (green = human-origin, blue = LLM) — the writer can click in and edit them like + any other text. +- **Deletions** appear as a **struck-red, non-editable hint** rendered adjacent to + the insertion (a decoration, not buffer text) — visually mirroring the webview's + `…`. +- The buffer content **is exactly the would-be-accepted result**, so what the + writer sees (and can edit) is precisely what accepting produces. + +Above each proposed block sit two **CodeLens** actions — **`Accept ▾`** and +**`Reject ▾`**. Clicking opens a small QuickPick: + +``` + Accept ▾ → ┌───────────────────────┐ Reject ▾ → ┌───────────────────────┐ + │ Accept this proposal │ │ Reject this proposal │ + │ Accept ALL proposals │ │ Reject ALL proposals │ + └───────────────────────┘ └───────────────────────┘ +``` + +The **webview** shows the same proposal with the same diff, and its controls are +the parallel HTML buttons — **`Accept ▾`** / **`Reject ▾`** — where the `▾` +reveals *Accept all* / *Reject all*. The legacy ✓/✗ glyphs are **replaced** by +these labelled controls in both surfaces. + +All four actions — **Accept**, **Reject**, **Accept all**, **Reject all** — are +reachable from **either** surface and route to the same controller logic +(INV-53). + +### 2.2 Accept / Reject semantics (editable-in-place) + +- **Accept** finalizes the change *that is already in the buffer*: it records the + span's attribution as landed, advances the F6 baseline (machine-landing), and + clears the proposal. It does **not** re-apply text (the text is already there) — + see §3.3. +- **Reject** reverts that block's region back to the stored original + (`Fingerprint.text`), and clears the proposal. +- **If the writer edited the inserted text before deciding:** **Accept** keeps + their edited text (their keystrokes layer on as human authorship over the + proposed span); **Reject** still reverts the whole block to the original. +- **Accept all / Reject all** operate on the current document's pending proposals, + in descending anchor order, skipping orphans (the #46 / INV-42 shape; **Reject + all is new**). + +### 2.3 Save semantics + +Because the proposed text is *in the buffer*, **saving while a proposal is pending +persists the proposed (accepted-result) text** to disk. The deletion hints are +decorations (never buffer text), so the **saved file is clean** — it contains the +accepted-result text, not yet "finalized" only in the sense of attribution / +baseline. "Pending" therefore means *provisional attribution, baseline not yet +advanced* — **not** "document unchanged" (INV-54). We deliberately **allow** the +save rather than block it or strip on save; once the editor shows the live +document, blocking saves would be the surprising behavior. + +### 2.4 What this is *not* (non-goals) + +- **Not live token-streaming into the editor.** The diff appears when the turn + produces a proposal; watching Claude type into the document is out of scope (the + live token stream is #60's notification + OutputChannel). +- **Not a new review *model*.** F4 proposals, anchoring (INV-11), block + granularity (INV-39/40), accept-all (INV-42) are reused; this adds an editor + *surface* and unifies the controls. +- **Not non-Markdown.** Same Markdown-gated scope as the rest of the review UI. +- **Not intra-diagram mermaid editing.** Atomic fences stay atomic (INV-23); a + mermaid block proposal is editable as its whole fence, not sub-diagram. +- **Not removing the webview.** The rendered preview remains a full review surface; + it gains label/dropdown parity, not a demotion. + +### 2.5 Surfaces considered & rejected + +| Editor approach | Why not | +| --- | --- | +| **Read-only decoration overlay** (doc unchanged; ghost insertions via decoration) | The inserted text would **not be editable** — fails the core "human-editable" requirement. | +| **Inline track-changes markup in the buffer** (`{~~old~~|++new++}`) | Pollutes the file on disk with markup until resolved; complex; conflicts with clean save. | +| **Native `vscode.diff` two-pane** | The two-pane diff was *removed* in #34 (F10) precisely to make one review surface; a separate diff editor is not "in the Markdown file." | + +Chosen: **optimistic apply + decorations** — the only model that yields an +editable diff that is *exactly* the accepted result, in the document itself. + +--- + +## 3. Engineering Design + +### 3.1 Architecture + +``` + ┌──────────────────────────────────────────────┐ + one F4 turn ───▶ │ ProposalController (F4 state/seam owner) │ + (#12 propose) │ propose() · finalizeInPlace() · revertInPlace│ + │ · rejectAll() · onDidChangeProposals │ + └───────────────┬──────────────────────────────┘ + │ proposals (pending records + anchors) + ┌─────────────────────┴───────────────────────┐ + ▼ ▼ + ┌───────────────────────────┐ ┌────────────────────────────┐ + │ EditorProposalController │ shared │ TrackChangesPreview- │ + │ (NEW, vscode host) │ pure diff │ Controller (webview) │ + │ · optimistic buffer write │◀──────────────▶ │ · renderReview HTML │ + │ · insertion tint deco │ trackChangesModel│ · Accept▾/Reject▾ buttons │ + │ · deletion-hint deco │ (diff → plan) │ · dropdown → acceptAll/ │ + │ · CodeLens Accept▾/Reject▾│ │ rejectAll │ + └───────────────────────────┘ └────────────────────────────┘ + ▲ ▲ + └──────────── both route actions to ───────────┘ + ProposalController.{finalizeInPlace,revertInPlace, + acceptAll,rejectAll} +``` + +The decisive rule: **one pure diff is the single source of truth** for both +surfaces (INV-49). The editor renders it as *buffer text + decorations + CodeLens*; +the webview renders it as *HTML*. They cannot diverge because they consume the +same hunks. + +### 3.2 Optimistic apply (on propose) — INV-48 + +`propose()` gains a buffer-write step, owned by `EditorProposalController` (so +`ProposalController` stays the pure state owner): + +1. Record the pending proposal(s) + anchor(s) **exactly as today** (the sidecar + record is unchanged: `replacement`, `Fingerprint{text,before,after,lineHint}`, + `granularity`). +2. Write the proposed text into the **live editor buffer**: replace the resolved + anchor span with `replacement` (for a document turn, this makes the buffer + Claude's full proposed document; for a selection turn, the single span). This + write **does not advance the F6 baseline** — the baseline stays at the + pre-proposal text, so the change reads as *pending*, not *landed*. +3. Compute the **decoration plan** from the same per-block word/line diff used by + `acceptBlock` (`wordEditHunks(fp.text, replacement)`): insertion segments → + tinted ranges (buffer coords, offset by the live anchor); deletion segments → + struck-red hint injections (decoration `before`/`after` content) at the segment + boundary. + +Critically, the optimistic write must **not** fire the F4 seam's +`onDidApplyAgentEdit` (which would advance the baseline / register a landed edit). +Optimistic apply is a *distinct* buffer-mutation path from accept; only **accept** +advances the baseline (§3.3). This is the load-bearing distinction that keeps the +change *pending* while *present*. + +### 3.3 Accept = finalize-in-place; Reject = revert-in-place — INV-51 + +Because the proposed text is already in the buffer, the old "accept ⇒ apply via +seam" path would **double-apply**. The accept/reject paths are reworked: + +- **`finalizeInPlace(id)`** — resolve the proposal's anchor against the live text; + record the span's attribution as **landed** (provenance from the proposal's + `author`, word-precise per INV-40 for block granularity); **advance the F6 + baseline** for that region (the machine-landing the seam used to do on apply); + `removeProposal(id)`. No text is re-applied. If the writer edited the inserted + span, the *current* buffer text is what gets finalized (their edits become + layered human authorship). +- **`revertInPlace(id)`** — resolve the proposal's live span; replace it with the + stored original `Fingerprint.text`; `removeProposal(id)`. Reverts the whole + block regardless of any in-place edits. +- **`acceptAll()` / `rejectAll()`** — iterate the current document's pending + proposals in **descending** anchor order (so earlier resolutions don't shift + later offsets), skipping orphans with a tally (INV-42 shape). `acceptAll` reuses + the #46 batch ordering; **`rejectAll` is new** and symmetric. + +Both surfaces (editor CodeLens/QuickPick and webview buttons/dropdown) call these +**same four methods** (INV-53). The webview's existing intent messages are renamed/ +extended (`accept`/`reject`/`acceptAll` + new `rejectAll`) but stay intent-only +(INV-35). + +### 3.4 Single diff source & render-once — INV-49 / INV-50 + +- **INV-49 (single diff source).** The per-proposal diff (block hunks → word/line + edit hunks) is produced by one pure function in `trackChangesModel.ts` (extends + the existing `diffToBlockHunks` / `wordEditHunks`, INV-39/40). The webview HTML + path and the editor decoration-plan path both consume it. Same proposal ⇒ + identical diff in both surfaces (this is the testable parity guarantee). +- **INV-50 (render once).** With optimistic apply, the baseline→buffer delta now + *contains* the proposed change. The renderer must attribute any baseline→buffer + delta that is covered by a **pending proposal** to that proposal (rendered as a + proposal block), and must **not** also render it as an already-landed + machine-diff. Mechanism: the webview render reconciles baseline-diff spans + against pending-proposal anchors; a span covered by a pending proposal is + rendered exactly once, as the proposal. *(This reconciliation is the primary + implementation risk; the implementation plan owns the exact span-mapping, but the + invariant — rendered once — is fixed here.)* + +### 3.5 Editor decorations & CodeLens (the editor surface) + +`EditorProposalController` (new, `vscode` host module) owns: + +- **Two `TextEditorDecorationType`s** — an *insertion tint* (green/blue background, + by provenance) over real buffer ranges, and a *deletion hint* (a + `before`/`after` render-option carrying the struck original text, red, + strikethrough, non-editable). Both are recomputed from the decoration plan on + every `onDidChangeProposals` and on active-editor change. +- **A `CodeLensProvider`** — for each pending proposal whose anchor resolves in the + active document, emit two lenses positioned above the block: `Accept ▾` + (command `cowriting.proposalAcceptMenu` with the proposal id) and `Reject ▾` + (`cowriting.proposalRejectMenu`). Each command opens a `vscode.window + .showQuickPick(["… this proposal","… ALL proposals"])` and dispatches to + `finalizeInPlace`/`acceptAll` or `revertInPlace`/`rejectAll`. +- **Lifecycle wiring** — subscribes to `proposals.onDidChangeProposals` (the same + event the preview uses) and `window.onDidChangeActiveTextEditor`; clears its + decorations + lenses for documents with no pending proposals (so a clean editor + stays clean — INV-32's spirit holds *when there is nothing pending*). + +Deletion hints are **decoration-only** (never buffer text, never saved) and +insertion tints decorate **real** buffer text (INV-52). + +### 3.6 Webview controls (label + dropdown parity) + +`proposalBlockHtml` (`trackChangesModel.ts`) swaps the ✓/✗ glyph buttons for an +**`Accept ▾`** / **`Reject ▾`** control pair. The `▾` opens a small CSS/JS dropdown +in the sealed webview (`preview.ts`/`preview.css`, no network — INV-21) offering +*Accept all* / *Reject all*; selecting posts the corresponding intent +(`acceptAll` / `rejectAll`) or the per-proposal `accept`/`reject`. The host routes +all four to the controller methods of §3.3 (INV-35 preserved). + +### 3.7 Components & files + +| File | Change | +| --- | --- | +| `src/editorProposalController.ts` | **NEW** — optimistic apply, decoration types, deletion-hint injection, `CodeLensProvider`, QuickPick menus; subscribes to `onDidChangeProposals` + active-editor change. | +| `src/proposalController.ts` | Add `finalizeInPlace(id)`, `revertInPlace(id)`, `rejectAll(docKey)`; `propose()` delegates the optimistic buffer write (so the controller stays state-pure); accept/reject routing now finalizes/reverts in place (no seam re-apply). | +| `src/trackChangesModel.ts` | Factor the shared per-proposal diff into one pure producer feeding both surfaces; reconcile pending-proposal spans in `renderReview` (INV-50); swap ✓/✗ → `Accept ▾`/`Reject ▾` in `proposalBlockHtml`. | +| `src/trackChangesPreview.ts` | Route new `rejectAll` intent; keep `accept`/`reject`/`acceptAll`; relabel toolbar/proposal controls. | +| `src/preview.ts` / `preview.css` | Dropdown control for `Accept ▾`/`Reject ▾`; same diff CSS reused. | +| `src/extension.ts` | Register `EditorProposalController`, its CodeLens provider, and the menu commands; wire it alongside `ProposalController`/`TrackChangesPreviewController`. | +| `package.json` | Add `cowriting.rejectAllProposals`, `cowriting.proposalAcceptMenu`, `cowriting.proposalRejectMenu`; CodeLens contribution; markdown-gated `when` clauses. | + +### 3.8 Invariants + +- **INV-48 — Optimistic apply (with re-anchor).** On propose, the proposed text is + written into the live editor buffer (the buffer becomes the would-be-accepted + result); this write does **not** advance the F6 baseline and does **not** fire the + F4 machine-landing seam. Because F4's `fp.text` is the *original* target — which + leaves the buffer once the replacement lands — optimistic apply **stores the + pre-apply text on `Proposal.original` and re-fingerprints the anchor to the applied + text**, so `resolve()` keeps finding the proposal in the mutated buffer; finalize / + revert / decorate all key off the re-anchored fp, and revert restores `original`. + `original` is captured **exactly once** (first apply): a proposal that survives a + save+reload already carries it, so a fresh session never re-captures it from the + already-applied buffer (reload-safety, INV-51/54). **Reverses INV-10** (propose no + longer leaves the document untouched) and **INV-32** (the editor is no longer kept + unconditionally clean — it shows pending proposals). +- **INV-49 — Single diff source.** Editor decorations and webview HTML both derive + from one pure, deterministic per-proposal diff (extends `diffToBlockHunks` / + `wordEditHunks`, INV-22/39/40). The same proposal yields the same diff in both + surfaces. +- **INV-50 — Rendered once.** A span covered by a pending proposal is rendered + exactly once — as that proposal — never additionally as an already-landed + baseline diff. +- **INV-51 — Finalize / revert in place.** Accept finalizes the span already in the + buffer (record attribution, advance baseline, clear proposal) without + re-applying text; Reject reverts the block region to the stored + `Fingerprint.text`. No double-apply. +- **INV-52 — Decoration roles.** Deletion hints are decoration-only (non-editable + `before`/`after` content, never buffer text, never saved); insertion tints + decorate real, editable buffer text. +- **INV-53 — Control parity.** Accept / Reject / Accept-all / Reject-all are + reachable from **both** the editor (CodeLens + QuickPick) and the webview + (buttons + dropdown) and route to the same `ProposalController` methods; the ✓/✗ + glyph controls are superseded by labelled `Accept ▾`/`Reject ▾`. +- **INV-54 — Save persists pending.** Saving while a proposal is pending persists + the proposed (accepted-result) text; "pending" denotes provisional attribution / + un-advanced baseline, not document divergence. (Decorations are not persisted, so + the saved file is clean text.) + +### 3.9 Error & edge handling + +- **Orphaned proposal** (anchor no longer resolves) — no editor decorations/lenses + for it; it still surfaces in the webview at end with a dashed border (INV-34); + accept-all/reject-all skip it with a tally (INV-42). +- **Writer edits the inserted span, then accepts** — current buffer text is + finalized (their edits layer as human authorship); **then rejects** — block + reverts to original regardless. +- **External / concurrent edit shifts geometry** — anchors re-resolve on + `onDidChangeProposals` (existing resolve-or-flag); decoration plan + lenses + recompute. +- **Multiple proposals from one turn** — all optimistically applied (buffer = full + proposed document); each block independently finalizable/revertible; descending + order on accept-all/reject-all keeps offsets valid. +- **Reject of an in-buffer span after partial accept of siblings** — each + proposal's revert uses its own live-resolved span, independent of siblings. +- **Non-authorable document** (read-only / not on disk) — controls disabled exactly + as today (`authorable` gating). + +--- + +## 4. Testing & E2E + +Per the §9 pipeline and `coauthoring-*` precedent (this is a VS Code extension — +**no flotilla/PPE**; the gate is unit + host-E2E green). + +- **Unit (the core):** + - The shared per-proposal diff producer: same hunks → insertion/deletion + segments; parity assertion that the decoration plan and the webview HTML derive + identical add/del spans for the same proposal (INV-49). + - `renderReview` render-once reconciliation: a pending-proposal span is emitted + as a proposal block and **not** as a landed baseline diff (INV-50). + - Decoration-plan computation from `wordEditHunks(fp.text, replacement)`: + insertion ranges in buffer coords; deletion-hint positions (INV-52). +- **Host E2E:** + - Propose (via the injectable `editTurn` stub) → editor shows insertion tint + + deletion hint + `Accept ▾`/`Reject ▾` CodeLens; buffer equals the accepted + result (INV-48). + - Edit the inserted text → **finalizeInPlace** keeps the edited text; baseline + advanced; proposal cleared (INV-51). + - **revertInPlace** restores `Fingerprint.text`; proposal cleared. + - Accept / Reject / Accept-all / Reject-all from **both** the editor command path + and the webview intent path produce the same end state (INV-53); `rejectAll` + clears all pending and reverts all blocks. + - Save while a proposal is pending → file on disk has the proposed text; no + decoration markup in the saved bytes (INV-54). + - Webview/editor parity: same proposal renders the same diff in both (INV-49). +- **Manual smoke:** ask Claude to edit a paragraph; confirm the editor diff is + editable, the deletion hint reads correctly, both surfaces' controls resolve it, + and a clean editor returns once nothing is pending. + +--- + +## 5. Delivery Plan (rollout — not a task list) + +One increment, single design-then-build (#64). Ships through branch → PR → `main`; +no migration and no new persisted sidecar shape (the `Proposal` record is +unchanged). The implementation plan (downstream `wgl-planning-and-executing` +session) owns the Task breakdown; a natural cut: + +1. **Shared diff + render-once** — factor the pure per-proposal diff producer; + reconcile pending spans in `renderReview` (INV-49/50) + unit tests. +2. **Accept/reject rework** — `finalizeInPlace` / `revertInPlace` / `rejectAll` on + `ProposalController` (no seam re-apply); unit tests. +3. **Editor surface** — `EditorProposalController`: optimistic apply, decorations, + deletion hints, CodeLens + QuickPick menus; wire in `extension.ts`. +4. **Control parity** — relabel ✓/✗ → `Accept ▾`/`Reject ▾` + dropdown in the + webview; route `rejectAll`; `package.json` commands/`when`. +5. **Host E2E + manual smoke** across both surfaces. + +Reversible by reverting the PR (restores the clean-editor / ✓-✗ webview behavior). + +--- + +## 6. Open Questions + +- **OQ-1 — Provenance tint exactness.** Insertion tint uses green=human-origin / + blue=LLM by the proposal `author`; whether to additionally word-tint *within* a + block by the F3 attribution of edited-in-place text (vs a single block tint) can + follow the existing F9/F10 coloring; v1 tints the proposed insertion by the + proposal author. *(Not blocking.)* +- **OQ-2 (inherited)** — The F11 design (`#43`) remains un-graduated (lives in code + + an issue draft); unrelated to #64 but noted in the shared lineage. Track + separately. +- **OQ-3 — Editor dropdown fidelity.** The editor uses a QuickPick to stand in for + the webview's true `▾` dropdown (CodeLens cannot render a caret menu inline); if + a more button-like affordance is wanted later, a webview-style overlay is a + possible increment. *(Accepted for v1.)* diff --git a/src/attributionController.ts b/src/attributionController.ts index a9f4947..c3e7066 100644 --- a/src/attributionController.ts +++ b/src/attributionController.ts @@ -249,7 +249,7 @@ export class AttributionController implements vscode.Disposable { range: vscode.Range, newText: string, provenance: Provenance, - opts?: { expectedVersion?: number; turnId?: string }, + opts?: { expectedVersion?: number; turnId?: string; landBaseline?: boolean }, ): Promise { if (!this.isTracked(document)) return false; if (opts?.expectedVersion !== undefined && document.version !== opts.expectedVersion) return false; @@ -290,15 +290,25 @@ export class AttributionController implements vscode.Disposable { "(host minimized differently?) — the edit may be mis-attributed (INV-9).", ); } - if (ok) { - // F6 (INV-18): a real machine landing — signal the baseline to advance so - // this text never shows as a change in the diff view. Fire regardless of - // attribution-match bookkeeping above; the landing happened either way. + if (ok && opts?.landBaseline !== false) { + // F6 (INV-18): a real machine landing — advance the baseline. F12 (INV-48) + // suppresses this for optimistic apply: the proposed text is in the buffer + // but the change stays PENDING (baseline at pre-proposal) until accept. this.applyEmitter.fire({ document }); } return ok; } + /** + * F12/#64 (INV-51): fire the machine-landing signal WITHOUT applying text — used + * by finalize-in-place, where the proposed text already landed in the buffer via + * optimistic apply (`landBaseline:false`) and accept only needs to advance the + * F6 baseline so the now-accepted change stops reading as pending. + */ + signalLanded(document: vscode.TextDocument): void { + if (this.isTracked(document)) this.applyEmitter.fire({ document }); + } + // ---- PUC-4: persistence on save ---------------------------------------------------- private onDidSave(document: vscode.TextDocument): void { diff --git a/src/editorProposalController.ts b/src/editorProposalController.ts new file mode 100644 index 0000000..0e2441d --- /dev/null +++ b/src/editorProposalController.ts @@ -0,0 +1,151 @@ +/** + * EditorProposalController — F12/#64 editor surface (spec coauthoring-inline-editor-diff + * §3.5). Reverses INV-32 for pending proposals: on `onDidChangeProposals` it + * (1) optimistically applies any not-yet-applied proposal into the active editor's + * buffer (ProposalController.optimisticApply, INV-48), (2) decorates each applied + * proposal — insertion tint over the proposed text + a non-editable struck-red hint + * for deletions (INV-52, decorationPlan/INV-49), and (3) provides a CodeLens + * `Accept ▾ / Reject ▾` above each block whose ▾ opens a QuickPick (this / all). + * Owns no proposal STATE — it is a view over ProposalController (which stays the + * pure F4 owner). A document with no pending proposals shows nothing (INV-32's + * spirit when nothing is pending). + */ +import * as vscode from "vscode"; +import type { ProposalController } from "./proposalController"; +import { decorationPlan } from "./trackChangesModel"; +import { isAuthorable } from "./workspacePath"; + +export class EditorProposalController implements vscode.Disposable, vscode.CodeLensProvider { + private readonly disposables: vscode.Disposable[] = []; + private readonly insertionDeco = vscode.window.createTextEditorDecorationType({ + backgroundColor: new vscode.ThemeColor("diffEditor.insertedTextBackground"), + }); + private readonly deletionDeco = vscode.window.createTextEditorDecorationType({ + // a non-editable struck-red hint injected AFTER the insertion (INV-52) + after: { color: new vscode.ThemeColor("gitDecoration.deletedResourceForeground") }, + textDecoration: "none", + }); + private readonly lensEmitter = new vscode.EventEmitter(); + readonly onDidChangeCodeLenses = this.lensEmitter.event; + /** Pending debounce timers — one per URI — coalesce rapid-fire propose events + * (e.g. runEditAndPropose's N sequential propose() calls) into a single + * optimistic-apply pass that runs after ALL proposals are created. Without this, + * each propose() fires onDidChangeProposals synchronously and the controller's + * optimisticApply runs concurrently with the still-in-progress propose loop, + * causing "file changed in the meantime" workspace-edit conflicts. */ + private readonly pendingApply = new Map>(); + + constructor(private readonly proposals: ProposalController) { + this.disposables.push( + this.insertionDeco, this.deletionDeco, this.lensEmitter, + vscode.languages.registerCodeLensProvider({ language: "markdown" }, this), + this.proposals.onDidChangeProposals(({ uri }) => this.scheduleApply(uri)), + vscode.window.onDidChangeActiveTextEditor((ed) => ed && this.renderEditor(ed)), + // the four QuickPick-backed menu commands + vscode.commands.registerCommand("cowriting.proposalAcceptMenu", (id?: string) => this.menu("accept", id)), + vscode.commands.registerCommand("cowriting.proposalRejectMenu", (id?: string) => this.menu("reject", id)), + ); + } + + /** Debounce-schedule an optimistic-apply pass for the given URI. Multiple rapid + * onDidChangeProposals events (from a single runEditAndPropose batch) collapse + * into one pass that runs after the batch completes. */ + private scheduleApply(uri: string): void { + const prev = this.pendingApply.get(uri); + if (prev !== undefined) clearTimeout(prev); + this.pendingApply.set( + uri, + setTimeout(() => { + this.pendingApply.delete(uri); + void this.onProposalsChanged(uri); + }, 0), + ); + } + + /** Apply any not-yet-applied proposals on the doc, then re-decorate + refresh lenses. */ + private async onProposalsChanged(uri: string): Promise { + const doc = vscode.workspace.textDocuments.find((d) => d.uri.toString() === uri); + if (!doc || doc.languageId !== "markdown" || !isAuthorable(doc.uri.scheme)) return; + const key = this.proposals.keyFor(doc); + for (const v of this.proposals.listProposals(doc)) { + if (v.anchorStart !== null && !this.proposals.isApplied(key, v.id)) { + await this.proposals.optimisticApply(doc, v.id); // fires onDidChangeProposals again; guarded by isApplied + } + } + const ed = vscode.window.visibleTextEditors.find((e) => e.document.uri.toString() === uri); + if (ed) this.renderEditor(ed); + this.lensEmitter.fire(); + } + + /** Decorate the editor for every applied proposal on its document (INV-52). */ + private renderEditor(editor: vscode.TextEditor): void { + const doc = editor.document; + if (doc.languageId !== "markdown") { + editor.setDecorations(this.insertionDeco, []); + editor.setDecorations(this.deletionDeco, []); + return; + } + const key = this.proposals.keyFor(doc); + const insertions: vscode.Range[] = []; + const deletions: vscode.DecorationOptions[] = []; + for (const v of this.proposals.listProposals(doc)) { + if (v.anchorStart === null || v.original === undefined || !this.proposals.isApplied(key, v.id)) continue; + const plan = decorationPlan(v.anchorStart, v.original, v.replacement); + for (const ins of plan.insertions) { + insertions.push(new vscode.Range(doc.positionAt(ins.start), doc.positionAt(ins.end))); + } + for (const del of plan.deletions) { + deletions.push({ + range: new vscode.Range(doc.positionAt(del.at), doc.positionAt(del.at)), + renderOptions: { after: { contentText: ` ${del.text} `, textDecoration: "line-through" } }, + }); + } + } + editor.setDecorations(this.insertionDeco, insertions); + editor.setDecorations(this.deletionDeco, deletions); + } + + /** CodeLensProvider: a `Accept ▾` / `Reject ▾` pair above each applied block. */ + provideCodeLenses(document: vscode.TextDocument): vscode.CodeLens[] { + if (document.languageId !== "markdown") return []; + const key = this.proposals.keyFor(document); + const lenses: vscode.CodeLens[] = []; + for (const v of this.proposals.listProposals(document)) { + if (v.anchorStart === null || !this.proposals.isApplied(key, v.id)) continue; + const pos = document.positionAt(v.anchorStart); + const line = new vscode.Range(pos.line, 0, pos.line, 0); + lenses.push( + new vscode.CodeLens(line, { title: "Accept ▾", command: "cowriting.proposalAcceptMenu", arguments: [v.id] }), + new vscode.CodeLens(line, { title: "Reject ▾", command: "cowriting.proposalRejectMenu", arguments: [v.id] }), + ); + } + return lenses; + } + + /** The dropdown: this-proposal vs all-proposals, then dispatch. */ + private async menu(kind: "accept" | "reject", id?: string): Promise { + const doc = vscode.window.activeTextEditor?.document; + if (!doc || !id) return; + const key = this.proposals.keyFor(doc); + const verb = kind === "accept" ? "Accept" : "Reject"; + const pick = await vscode.window.showQuickPick( + [`${verb} this proposal`, `${verb} ALL proposals`], + { placeHolder: `${verb} Claude's proposal` }, + ); + if (!pick) return; + const all = pick.includes("ALL"); + if (kind === "accept") { + if (all) await this.proposals.acceptAllProposals(doc); + else await this.proposals.finalizeInPlace(key, id); + } else { + if (all) await this.proposals.rejectAll(doc); + else await this.proposals.revertInPlace(key, id); + } + } + + dispose(): void { + for (const t of this.pendingApply.values()) clearTimeout(t); + this.pendingApply.clear(); + for (const d of this.disposables) d.dispose(); + } +} diff --git a/src/extension.ts b/src/extension.ts index ec36c22..4cc73cd 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -12,6 +12,7 @@ import { SidecarRouter } from "./sidecarRouter"; import { DiffViewController } from "./diffViewController"; import { TrackChangesPreviewController } from "./trackChangesPreview"; import { LiveProgressUi } from "./liveProgressUi"; +import { EditorProposalController } from "./editorProposalController"; import { isAuthorable, routeEdit, selectionRejection } from "./workspacePath"; const CHANNEL_NAME = "Cowriting (Cline SDK)"; @@ -25,6 +26,7 @@ export interface CowritingApi { trackChangesPreviewController: TrackChangesPreviewController; sidecarRouter: SidecarRouter; liveProgressUi: LiveProgressUi; + editorProposalController: EditorProposalController; } export function activate(context: vscode.ExtensionContext): CowritingApi | undefined { @@ -115,6 +117,11 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef ); context.subscriptions.push(trackChangesPreviewController); + // --- F12 (#64): the editor surface — optimistic-apply proposals into the buffer, + // decorate the diff, and provide Accept ▾/Reject ▾ CodeLens (INV-48/49/52/53). --- + const editorProposalController = new EditorProposalController(proposalController); + context.subscriptions.push(editorProposalController); + // #46 (INV-42): accept every pending proposal on the active doc in one gesture // (also reachable from the preview toolbar's "Accept all" button). Reuses the // batched F4 seam + reports applied-vs-skipped. @@ -129,6 +136,18 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef }), ); + // #64 (INV-53): reject every pending proposal on the active doc in one gesture. + 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); + }), + ); + // --- F6 machine-landing wiring — now for ANY authorable doc --- // The seam's single machine-landing signal advances the F6 baseline (INV-18); // the seam can now fire on out-of-folder files too, so wire it unconditionally. @@ -342,6 +361,7 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef trackChangesPreviewController, sidecarRouter, liveProgressUi, + editorProposalController, }; } diff --git a/src/model.ts b/src/model.ts index c24f133..c865d83 100644 --- a/src/model.ts +++ b/src/model.ts @@ -97,6 +97,14 @@ export interface Proposal { * back-compat with older sidecars) ⇒ a single-range proposal accepted whole. */ granularity?: "block" | "single"; + /** + * F12/#64 (INV-48): the pre-apply text, captured when a proposal is + * optimistically applied to the buffer. `replacement` is now in the buffer and + * `fp.text` re-anchors to it, so `original` is the only record of what to revert + * to (revert-in-place) and what to show struck in the `` half. Absent on a + * proposal created but not yet optimistically applied (or older sidecars). + */ + original?: string; } export interface Artifact { @@ -252,6 +260,8 @@ export function serializeArtifact(a: Artifact): string { createdAt: p.createdAt, ...(p.turnId !== undefined ? { turnId: p.turnId } : {}), ...(p.instruction !== undefined ? { instruction: p.instruction } : {}), + ...(p.original !== undefined ? { original: p.original } : {}), + ...(p.granularity !== undefined ? { granularity: p.granularity } : {}), }, p, ), diff --git a/src/proposalController.ts b/src/proposalController.ts index 5e712bf..ba9649c 100644 --- a/src/proposalController.ts +++ b/src/proposalController.ts @@ -13,8 +13,8 @@ import * as vscode from "vscode"; import { SidecarRouter, docIdentity } from "./sidecarRouter"; import { emptyArtifact, type Artifact, type Fingerprint, type Proposal, type Provenance } from "./model"; -import { resolve, shift, type OffsetRange } from "./anchorer"; -import { addProposal, removeProposal } from "./proposalModel"; +import { resolve, shift, buildFingerprint, type OffsetRange } from "./anchorer"; +import { addProposal, removeProposal, setProposalApplied } from "./proposalModel"; import type { AttributionController } from "./attributionController"; import type { VersionGuard } from "./versionGuard"; import { isAuthorable } from "./workspacePath"; @@ -39,6 +39,8 @@ interface DocState { live: Map; /** proposal ids whose anchor did not resolve at last render (stale/orphaned). */ unresolved: Set; + /** ids already optimistically applied to the buffer (so the trigger doesn't re-apply). */ + applied: Set; } export class ProposalController implements vscode.Disposable { @@ -89,8 +91,9 @@ export class ProposalController implements vscode.Disposable { id: p.id, anchorStart: resolved === "orphaned" ? null : resolved.start, anchorEnd: resolved === "orphaned" ? null : resolved.end, - replaced: fp?.text ?? "", + replaced: p.original ?? fp?.text ?? "", replacement: p.replacement, + original: p.original, }; }); } @@ -104,6 +107,7 @@ export class ProposalController implements vscode.Disposable { artifact: this.store.load(docPath) ?? emptyArtifact(docPath), live: new Map(), unresolved: new Set(), + applied: new Set(), }; this.docs.set(docPath, state); } @@ -137,8 +141,33 @@ export class ProposalController implements vscode.Disposable { // ---- PUC-2/PUC-3: accept / reject (INV-11/INV-12) ---------------------------------- - /** Accept by proposal id (test-facing twin of the thread-menu gesture). */ + /** Accept by id — F12: finalize the already-applied text in place (INV-51). */ async acceptById(docPath: string, proposalId: string, opts?: { silent?: boolean }): Promise { + if (this.isApplied(docPath, proposalId)) { + // INV-11 (applied path): if an external write mangled the optimistically-applied + // text so the fingerprint no longer resolves, refuse finalize — same guard as the + // legacy accept path. Direct finalizeInPlace calls (CodeLens Accept gesture where + // the user may have edited inside the applied span) bypass this check intentionally. + const hit = this.byId(docPath, proposalId); + if (hit) { + const document = this.openDoc(hit.state); + if (document) { + const fp = hit.state.artifact.anchors[hit.proposal.anchorId]?.fingerprint; + const resolved = fp ? resolve(document.getText(), fp) : "orphaned"; + if (resolved === "orphaned") { + if (!opts?.silent) { + void vscode.window.showWarningMessage( + "Cowriting: this proposal's target text changed or is missing — undo to restore it, or reject to discard (it is never applied by guess).", + ); + } + return false; + } + } + } + return this.finalizeInPlace(docPath, proposalId); + } + // Fallback: a proposal that was never optimistically applied (e.g. orphaned at + // apply time) keeps the legacy seam-apply accept. const hit = this.byId(docPath, proposalId); return hit ? this.accept(hit.state, hit.proposal, opts) : false; } @@ -181,6 +210,12 @@ export class ProposalController implements vscode.Disposable { return true; } + /** Reject by id — F12: revert the applied text in place (INV-51). */ + async rejectByIdInPlace(docPath: string, proposalId: string): Promise { + if (this.isApplied(docPath, proposalId)) return this.revertInPlace(docPath, proposalId); + return this.rejectById(docPath, proposalId); + } + private async accept(state: DocState, proposal: Proposal, opts?: { silent?: boolean }): Promise { if (this.guard.isReadOnly(state.docPath)) return false; const document = this.openDoc(state); @@ -257,6 +292,158 @@ export class ProposalController implements vscode.Disposable { return true; } + /** True once this proposal's text is in the buffer (optimistic apply ran). */ + isApplied(docPath: string, proposalId: string): boolean { + return this.docs.get(docPath)?.applied.has(proposalId) ?? false; + } + + /** + * F12/#64 (INV-48): optimistically apply a pending proposal INTO the buffer so + * the editor shows the would-be-accepted result (editable). Reuses the F4 + * word-precise seam (block → per-word hunks, INV-40; single → whole range) but + * with `landBaseline:false` (the change stays pending). Then re-anchors the + * proposal to the applied text and stores the original (`setProposalApplied`), so + * `resolve()` finds it in the mutated buffer and revert/decorate key off it. + * Idempotent: a no-op if already applied. + */ + async optimisticApply(document: vscode.TextDocument, proposalId: string): Promise { + if (!this.isTracked(document) || this.guard.isReadOnly(this.keyOf(document))) return false; + const docPath = this.keyOf(document); + const state = this.ensureState(document); + if (state.applied.has(proposalId)) return true; + state.artifact = this.store.load(docPath) ?? emptyArtifact(docPath); + const proposal = state.artifact.proposals.find((p) => p.id === proposalId); + const fp = proposal ? state.artifact.anchors[proposal.anchorId]?.fingerprint : undefined; + if (!proposal || !fp) return false; + // Reload-safety (INV-51/54): a proposal that already carries `original` was + // optimistically applied in a PRIOR session — the buffer holds the applied text + // and `fp` points at it, but this (fresh) controller's in-memory `applied` set is + // empty. Re-applying would recapture `original` from the already-applied buffer + // (= the replacement) and CLOBBER the true revert target, breaking Reject. Mark it + // applied in memory and stop — `original` is captured exactly once, on first apply. + if (proposal.original !== undefined) { + state.applied.add(proposalId); + this.renderAll(document); + return true; + } + const resolved = resolve(document.getText(), fp); + if (resolved === "orphaned") return false; + const original = document.getText( + new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), + ); + const ok = + proposal.granularity === "block" + ? await this.applyBlockOptimistic(document, resolved, proposal) + : await this.attribution.applyAgentEdit( + document, + new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), + proposal.replacement, + proposal.author, + { expectedVersion: document.version, turnId: proposal.turnId, landBaseline: false }, + ); + if (!ok) return false; + state.applied.add(proposalId); + // Re-anchor to the applied text now in the buffer (its start is unchanged; its + // end shifts by the net length delta of the replacement). + const appliedStart = resolved.start; + const appliedEnd = appliedStart + proposal.replacement.length; + const appliedFp = buildFingerprint(document.getText(), { start: appliedStart, end: appliedEnd }); + this.store.update(docPath, (a) => setProposalApplied(a, proposalId, appliedFp, original)); + this.renderAll(document); + return true; + } + + /** Block optimistic apply: the INV-40 per-word hunks, but landBaseline:false. */ + private async applyBlockOptimistic( + document: vscode.TextDocument, + resolved: OffsetRange, + proposal: Proposal, + ): Promise { + const blockText = document.getText( + new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), + ); + const subHunks = wordEditHunks(blockText, proposal.replacement); + if (subHunks.length === 0) return true; + for (const h of [...subHunks].sort((a, b) => b.start - a.start)) { + const range = new vscode.Range( + document.positionAt(resolved.start + h.start), + document.positionAt(resolved.start + h.end), + ); + const ok = await this.attribution.applyAgentEdit(document, range, h.replacement, proposal.author, { + expectedVersion: document.version, turnId: proposal.turnId, landBaseline: false, + }); + if (!ok) return false; + } + return true; + } + + /** + * F12/#64 (INV-51): ACCEPT an optimistically-applied proposal — the text is + * already in the buffer, so this only advances the F6 baseline (machine-landing, + * via `attribution.signalLanded`) and clears the proposal. No re-application. + */ + async finalizeInPlace(docPath: string, proposalId: string): Promise { + const hit = this.byId(docPath, proposalId); + if (!hit) return false; + const document = this.openDoc(hit.state); + if (!document) return false; + this.attribution.signalLanded(document); + this.store.update(docPath, (a) => removeProposal(a, proposalId)); + hit.state.applied.delete(proposalId); + this.renderAll(document); + return true; + } + + /** + * F12/#64 (INV-51): REJECT an optimistically-applied proposal — replace its live + * applied span with the stored `original`, then clear it. Reverts the whole block + * regardless of any in-place edits the human made to the inserted text. + */ + async revertInPlace(docPath: string, proposalId: string): Promise { + const hit = this.byId(docPath, proposalId); + if (!hit) return false; + const document = this.openDoc(hit.state); + if (!document) return false; + const fp = hit.state.artifact.anchors[hit.proposal.anchorId]?.fingerprint; + const resolved = fp ? resolve(document.getText(), fp) : "orphaned"; + if (resolved !== "orphaned" && hit.proposal.original !== undefined) { + const we = new vscode.WorkspaceEdit(); + we.replace( + document.uri, + new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)), + hit.proposal.original, + ); + if (!(await vscode.workspace.applyEdit(we))) return false; + } + this.store.update(docPath, (a) => removeProposal(a, proposalId)); + hit.state.applied.delete(proposalId); + this.renderAll(document); + return true; + } + + /** + * F12/#64 (INV-53): reject EVERY pending proposal on a document — revert each in + * DESCENDING anchor order (so an earlier revert never shifts a later one's + * offsets), symmetric with #46's accept-all. Returns the reverted count. + */ + async rejectAll(document: vscode.TextDocument): Promise<{ reverted: number }> { + if (!this.isTracked(document)) return { reverted: 0 }; + const docPath = this.keyOf(document); + const state = this.ensureState(document); + state.artifact = this.store.load(docPath) ?? emptyArtifact(docPath); + const text = document.getText(); + const ordered = state.artifact.proposals + .map((p) => { + const fp = state.artifact.anchors[p.anchorId]?.fingerprint; + const r = fp ? resolve(text, fp) : "orphaned"; + return { id: p.id, start: r === "orphaned" ? -1 : r.start }; + }) + .sort((a, b) => b.start - a.start); + let reverted = 0; + for (const it of ordered) if (await this.revertInPlace(docPath, it.id)) reverted++; + return { reverted }; + } + private reject(state: DocState, proposal: Proposal): void { if (this.guard.isReadOnly(state.docPath)) return; this.store.update(state.docPath, (a) => removeProposal(a, proposal.id)); diff --git a/src/proposalModel.ts b/src/proposalModel.ts index 8aa57cf..4aefbee 100644 --- a/src/proposalModel.ts +++ b/src/proposalModel.ts @@ -37,6 +37,25 @@ export function removeProposal(artifact: Artifact, proposalId: string): boolean return artifact.proposals.length < before; } +/** + * F12/#64 (INV-48): record a proposal as optimistically applied — store the + * pre-apply `original` (for revert + the struck ``) and re-point its anchor + * fingerprint to the now-in-buffer applied text so `resolve()` finds it. Idempotent + * shape: a second call simply overwrites with the same values. + */ +export function setProposalApplied( + artifact: Artifact, + proposalId: string, + appliedFp: Fingerprint, + original: string, +): boolean { + const p = artifact.proposals.find((x) => x.id === proposalId); + if (!p) return false; + p.original = original; + artifact.anchors[p.anchorId] = { fingerprint: appliedFp }; + return true; +} + /** Markdown comment body: instruction header + fenced whole-range diff. */ export function proposalBody(targetText: string, p: Proposal): string { const header = p.instruction ? `**Claude proposes** — _${p.instruction}_` : "**Claude proposes**"; diff --git a/src/trackChangesModel.ts b/src/trackChangesModel.ts index 4235e60..930772e 100644 --- a/src/trackChangesModel.ts +++ b/src/trackChangesModel.ts @@ -263,6 +263,42 @@ export function wordEditHunks(currentText: string, rewrittenText: string): EditH return hunks; } +/** F12/#64 (INV-49/52): the editor render of one optimistically-applied proposal. */ +export interface DecorationPlan { + /** buffer ranges of inserted (proposed) text → tinted (INV-52). */ + insertions: { start: number; end: number }[]; + /** struck original text shown as a non-editable hint at a buffer offset (INV-52). */ + deletions: { at: number; text: string }[]; +} + +/** + * F12/#64 (INV-49): compute the editor decoration plan for a proposal whose applied + * text occupies `[anchorStart, anchorStart+replacement.length)` in the buffer. The + * SAME word diff the webview uses (`wordEditHunks`) drives it, so both surfaces show + * the identical diff. A changed run maps to (a) an insertion range over the run's + * applied text and (b) a deletion hint carrying the run's removed original text at + * the run start; a pure insertion has no deletion hint; a pure deletion has only a + * hint. Pure, vscode-free, deterministic. + */ +export function decorationPlan(anchorStart: number, original: string, replacement: string): DecorationPlan { + const insertions: { start: number; end: number }[] = []; + const deletions: { at: number; text: string }[] = []; + // Walk the word diff once, tracking the applied-side offset as we consume parts. + let appliedOffset = anchorStart; + for (const part of diffWordsWithSpace(original, replacement)) { + if (part.added) { + insertions.push({ start: appliedOffset, end: appliedOffset + part.value.length }); + appliedOffset += part.value.length; + } else if (part.removed) { + deletions.push({ at: appliedOffset, text: part.value }); + // removed text is NOT in the applied buffer → appliedOffset does not advance + } else { + appliedOffset += part.value.length; + } + } + return { insertions, deletions }; +} + const isWs = (c: string): boolean => /\s/.test(c); /** @@ -701,6 +737,8 @@ export interface ProposalView { replaced: string; /** the proposed replacement text. */ replacement: string; + /** F12/#64 (INV-48): the pre-apply original, set after optimistic apply. */ + original?: string; } function proposalBlockHtml(p: ProposalView, render: (src: string) => string): string { @@ -716,8 +754,14 @@ function proposalBlockHtml(p: ProposalView, render: (src: string) => string): st const after = `${safe(p.replacement)}`; const actions = `` + - `` + - `` + + `` + + `` + + `` + + `` + + `` + + `` + + `` + + `` + ``; return `
${actions}${before}${after}
`; } @@ -747,6 +791,26 @@ function renderReviewOp( * INV-34). Deterministic: proposals in the same block are ordered by anchorStart * then id; trailing proposals keep input order. */ +/** + * F12/#64 (INV-50): `currentText` with every resolved pending proposal's applied + * span reverted to its original (`replaced`) — the "landed" text the baseline diff + * should run against, so a pending proposal renders ONCE (as a proposal), never also + * as a landed change. Reverts high→low so earlier offsets stay valid. Pure. The + * preview's summary tally diffs against this too, so the toolbar count matches the + * body (a pending change is not double-counted as both a landed add/remove and a + * proposal). + */ +export function landedTextOf(currentText: string, proposals: ProposalView[]): string { + const pendingApplied = proposals + .filter((p) => p.anchorStart !== null) + .sort((a, b) => b.anchorStart! - a.anchorStart!); + let landedText = currentText; + for (const p of pendingApplied) { + landedText = landedText.slice(0, p.anchorStart!) + p.replaced + landedText.slice(p.anchorEnd!); + } + return landedText; +} + export function renderReview( baselineText: string, currentText: string, @@ -755,8 +819,18 @@ export function renderReview( opts: RenderOptions = {}, ): string { const render = opts.render ?? defaultRender; - const ranges = splitBlocksWithRanges(currentText); - const ops = diffBlocks(baselineText, currentText); + + // F12/#64 (INV-50): with optimistic apply the proposed text is already in + // `currentText`, so a naive baseline→current diff would render each proposed + // change BOTH as a landed diff and as its proposal block. Diff against the + // "landed" text (current minus pending proposals) so they render once. + const pendingApplied = proposals + .filter((p) => p.anchorStart !== null) + .sort((a, b) => b.anchorStart! - a.anchorStart!); + const landedText = landedTextOf(currentText, proposals); + + const ranges = splitBlocksWithRanges(landedText); + const ops = diffBlocks(baselineText, landedText); // #48: right after a PIN (baseline reason "pinned") with no changes since, the // panel is fully clean: no change marks (already absent) AND no authorship // coloring, so the pin reads as "this is my clean starting point". Skip @@ -766,7 +840,28 @@ export function renderReview( // its authorship coloring (F10 INV-33), so this is gated on the pin specifically. const clean = opts.pinned === true && ops.every((o) => o.kind === "unchanged"); - // Associate each resolved proposal with the current-side block index whose range + // Map a currentText offset to its landedText offset (account for reverted spans + // that precede it; reverts were applied high→low so the cumulative delta is stable). + const toLanded = (curOff: number): number => { + let delta = 0; + for (const p of [...pendingApplied].sort((a, b) => a.anchorStart! - b.anchorStart!)) { + if (p.anchorStart! < curOff) delta += p.replaced.length - (p.anchorEnd! - p.anchorStart!); + } + return curOff + delta; + }; + + // F12/#64 (INV-49/50): blocks are split from `landedText`, so `blk.start` is a + // landedText offset — but `authorSpans` arrive in `currentText` coordinates. A + // colored block sitting AFTER a length-changing pending proposal would otherwise + // be mis-colored (the offsets diverge by the revert delta). Map the spans into + // landedText coordinates once so authorship coloring stays aligned. + const landedSpans: AuthorSpan[] = authorSpans.map((s) => ({ + ...s, + start: toLanded(s.start), + end: toLanded(s.end), + })); + + // Associate each resolved proposal with the landedText block index whose range // it anchors into: the largest block with start <= anchorStart (the containing // block, or the nearest preceding block when the anchor sits in a gap). A // resolved anchor before all blocks, and every unresolved proposal, trails. @@ -778,7 +873,7 @@ export function renderReview( const byBlock = new Map(); const trailing: ProposalView[] = []; for (const p of proposals) { - const j = p.anchorStart === null ? -1 : blockOf(p.anchorStart); + const j = p.anchorStart === null ? -1 : blockOf(toLanded(p.anchorStart)); if (j < 0) { trailing.push(p); continue; @@ -795,7 +890,7 @@ export function renderReview( const blockIndex = op.kind === "removed" ? -1 : ci; const blk = op.kind === "removed" ? undefined : ranges[ci++]; const colored = (raw: string): string => - blk && !clean ? colorByAuthor(raw, blk.start, authorSpans, render) : render(raw); + blk && !clean ? colorByAuthor(raw, blk.start, landedSpans, render) : render(raw); bodyParts.push(renderReviewOp(op, render, colored, srcAttr(blk))); const here = blockIndex >= 0 ? byBlock.get(blockIndex) : undefined; if (here) for (const p of here) bodyParts.push(proposalBlockHtml(p, render)); diff --git a/src/trackChangesPreview.ts b/src/trackChangesPreview.ts index 45c9244..d7b0ffc 100644 --- a/src/trackChangesPreview.ts +++ b/src/trackChangesPreview.ts @@ -13,7 +13,7 @@ import * as vscode from "vscode"; import type { DiffViewController } from "./diffViewController"; import type { AttributionController } from "./attributionController"; import type { ProposalController } from "./proposalController"; -import { renderReview, renderPlain, diffBlocks, diffToBlockHunks, type BlockOp } from "./trackChangesModel"; +import { renderReview, renderPlain, diffBlocks, diffToBlockHunks, landedTextOf, type BlockOp } from "./trackChangesModel"; import { buildFingerprint } from "./anchorer"; import { isAuthorable } from "./workspacePath"; import type { EditTurnResult, RunEditTurnOptions } from "./liveTurn"; @@ -44,7 +44,8 @@ type ToolbarMsg = | { type: "pinBaseline" } | { type: "askClaude"; scope: "document" } | { type: "askClaude"; scope: "selection"; start: number; end: number } - | { type: "acceptAll" }; + | { type: "acceptAll" } + | { type: "rejectAll" }; export class TrackChangesPreviewController implements vscode.Disposable { private readonly disposables: vscode.Disposable[] = []; @@ -196,8 +197,7 @@ export class TrackChangesPreviewController implements vscode.Disposable { .acceptById(this.proposals.keyFor(document), m.proposalId) .then(() => this.refresh(document)); } else if (m?.type === "reject" && m.proposalId) { - this.proposals.rejectById(this.proposals.keyFor(document), m.proposalId); - this.refresh(document); + void this.proposals.rejectByIdInPlace(this.proposals.keyFor(document), m.proposalId).then(() => this.refresh(document)); } else if (m?.type === "pinBaseline") { // F6 baseline store re-render arrives via the onDidChangeBaseline subscription. this.diffView.pin(document); @@ -208,6 +208,8 @@ export class TrackChangesPreviewController implements vscode.Disposable { } else if (m?.type === "acceptAll") { // #46 (INV-42): batch-accept every pending proposal on this doc, then report. void this.acceptAll(document); + } else if (m?.type === "rejectAll") { + void this.rejectAll(document); } } @@ -228,6 +230,17 @@ export class TrackChangesPreviewController implements vscode.Disposable { ); } + /** #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"}.`, + ); + } + } + /** * F11 (PUC-3/4): prompt host-side for the instruction (keeps the LLM/secret * surface out of the sealed webview, INV-8/35), run the edit turn, and surface @@ -359,9 +372,13 @@ export class TrackChangesPreviewController implements vscode.Disposable { } const spans = this.attribution.spansFor(document); const proposals = this.proposals.listProposals(document); + // F12/#64 (INV-50): count added/removed against the LANDED text (current minus + // pending proposals), matching the body — a pending change shows once, as a + // proposal, and is not also tallied as a landed add/remove. + const landedOps = diffBlocks(baselineText, landedTextOf(current, proposals)); const summary = { - added: ops.filter((o) => o.kind === "added").length, - removed: ops.filter((o) => o.kind === "removed").length, + added: landedOps.filter((o) => o.kind === "added").length, + removed: landedOps.filter((o) => o.kind === "removed").length, proposals: proposals.length, }; void panel.webview.postMessage({ diff --git a/test/decorationPlan.test.ts b/test/decorationPlan.test.ts new file mode 100644 index 0000000..c149df9 --- /dev/null +++ b/test/decorationPlan.test.ts @@ -0,0 +1,27 @@ +import { describe, test, expect } from "vitest"; +import { decorationPlan } from "../src/trackChangesModel"; + +describe("decorationPlan (INV-49)", () => { + test("derives insertion ranges + deletion hints from original→replacement", () => { + // anchorStart 10; original 'the brown fox' → 'the red fox' + const plan = decorationPlan(10, "the brown fox", "the red fox"); + // one changed run: 'brown ' → 'red ' (word-level); insertion 'red ' tinted, + // deletion hint 'brown ' shown at the run start. + expect(plan.insertions.length).toBeGreaterThan(0); + expect(plan.deletions.length).toBeGreaterThan(0); + // insertion offsets are in buffer coords (>= anchorStart) and lie within the applied text + for (const ins of plan.insertions) { + expect(ins.start).toBeGreaterThanOrEqual(10); + expect(ins.end).toBeLessThanOrEqual(10 + "the red fox".length); + } + // a deletion hint carries the struck original text at a buffer offset + expect(plan.deletions[0].text).toContain("brown"); + }); + + test("pure insertion (no deletion) yields an insertion range and no deletion hint", () => { + // 'hello' → 'hello world': ' world' is a pure word-level insertion (no removed tokens) + const plan = decorationPlan(0, "hello", "hello world"); + expect(plan.insertions.length).toBe(1); + expect(plan.deletions.length).toBe(0); + }); +}); diff --git a/test/e2e/runTest.ts b/test/e2e/runTest.ts index 543c383..383f9af 100644 --- a/test/e2e/runTest.ts +++ b/test/e2e/runTest.ts @@ -13,11 +13,20 @@ async function main(): Promise { const workspace = fs.mkdtempSync(path.join(os.tmpdir(), "cowriting-e2e-")); fs.cpSync(fixture, workspace, { recursive: true }); + // VS Code derives its IPC socket path from --user-data-dir. The default + // (`/.vscode-test/user-data`) plus `/-main.sock` can blow + // past macOS's ~103-char UNIX-socket limit (`listen EINVAL`) when the project + // lives at a long path (e.g. a git worktree). Pin the user-data dir to a SHORT + // root under /tmp so the socket path stays well under the limit. (os.tmpdir() on + // macOS is itself a long /var/folders/… path, so we use /tmp directly.) + const userDataRoot = process.platform === "win32" ? os.tmpdir() : "/tmp"; + const userDataDir = fs.mkdtempSync(path.join(userDataRoot, "cwud-")); + try { await runTests({ extensionDevelopmentPath, extensionTestsPath, - launchArgs: [workspace, "--disable-extensions"], + launchArgs: [workspace, "--disable-extensions", "--user-data-dir", userDataDir], extensionTestsEnv: { E2E_WORKSPACE: workspace }, }); @@ -26,10 +35,11 @@ async function main(): Promise { await runTests({ extensionDevelopmentPath, extensionTestsPath: path.resolve(__dirname, "./suite-no-workspace/index"), - launchArgs: ["--disable-extensions"], + launchArgs: ["--disable-extensions", "--user-data-dir", userDataDir], }); } finally { fs.rmSync(workspace, { recursive: true, force: true }); + fs.rmSync(userDataDir, { recursive: true, force: true }); } } diff --git a/test/e2e/suite/f10Review.test.ts b/test/e2e/suite/f10Review.test.ts index 7aec9ee..496d8c5 100644 --- a/test/e2e/suite/f10Review.test.ts +++ b/test/e2e/suite/f10Review.test.ts @@ -109,8 +109,9 @@ suite("F10 interactive review (host E2E — preview is the single review surface const pIdx = html.indexOf(`data-proposal-id="${id}"`); const t2Idx = html.indexOf("second claude target"); assert.ok(t2Idx >= 0 && pIdx < t2Idx, "the proposal renders in place, before the following block (#31)"); - // INV-10: proposing never touches the document. - assert.ok(doc.getText().includes(T1), "document unchanged by propose"); + // F12 (INV-48): EditorProposalController auto-applies the proposal into the buffer; + // the replacement text is now in the document, proposal is still pending. + assert.ok(doc.getText().includes("A FIRST claude REPLACEMENT sentence."), "F12 optimistic-apply: replacement in buffer"); }); test("accept → the proposal lands, clears from the preview, and the baseline advances (PUC-4)", async () => { @@ -131,17 +132,18 @@ suite("F10 interactive review (host E2E — preview is the single review surface assert.ok(!marked, "the just-landed Claude text renders unmarked (baseline advanced)"); }); - test("reject → the proposal vanishes and the document is untouched (PUC-5)", async () => { + test("reject → the proposal vanishes and the document is reverted (PUC-5)", async () => { const { doc, key } = await reopen(DOC_REL); const api = await getApi(); const before = doc.getText(); const id2 = await propose(doc, key, T2, "A SECOND would-be replacement.", "turn-f10-2"); await settle(); assert.ok(api.proposalController.listProposals(doc).some((v) => v.id === id2), "second proposal pending"); - assert.strictEqual(api.proposalController.rejectById(DOC_REL, id2), true, "reject"); + // F12: use rejectByIdInPlace to revert the optimistically-applied buffer edit. + assert.ok(await api.proposalController.rejectByIdInPlace(DOC_REL, id2), "rejectByIdInPlace reverts + removes"); await settle(); assert.ok(!api.proposalController.listProposals(doc).some((v) => v.id === id2), "rejected proposal gone"); - assert.strictEqual(doc.getText(), before, "document untouched by reject"); + assert.strictEqual(doc.getText(), before, "document reverted to pre-propose state"); const html = api.trackChangesPreviewController.renderHtmlFor(key); assert.ok(!html.includes(`data-proposal-id="${id2}"`), "no rejected block in the preview"); }); diff --git a/test/e2e/suite/f11Toolbar.test.ts b/test/e2e/suite/f11Toolbar.test.ts index 7d1034c..31e69d8 100644 --- a/test/e2e/suite/f11Toolbar.test.ts +++ b/test/e2e/suite/f11Toolbar.test.ts @@ -99,8 +99,9 @@ suite("F11 preview toolbar (host E2E — message → seam wiring, no LLM)", () = "The quick RED fox jumps over the lazy CAT.", "the block proposal carries the whole rewritten paragraph", ); - // INV-10: proposing never mutates the document. - assert.ok(doc.getText().includes("brown fox") && doc.getText().includes("lazy dog"), "document unchanged by propose"); + // F12 (INV-48): EditorProposalController optimistically applies the proposed text, + // so after settle the buffer has the replacement. The proposal is still pending. + assert.ok(doc.getText().includes("RED fox") && doc.getText().includes("lazy CAT"), "F12 optimistic-apply: proposed text in buffer"); void key; }); @@ -129,7 +130,9 @@ suite("F11 preview toolbar (host E2E — message → seam wiring, no LLM)", () = assert.ok(view, "the proposal is live"); assert.strictEqual(view!.replacement, "The REWRITTEN paragraph from Claude.", "carries the turn replacement"); assert.strictEqual(view!.replaced, target, "replaces exactly the selected range"); - assert.ok(doc.getText().includes(target), "document unchanged by propose (INV-10)"); + // F12 (INV-48): the EditorProposalController optimistically applies, so the buffer + // now has the replacement. `view.replaced` still records the original target text. + assert.ok(doc.getText().includes("The REWRITTEN paragraph from Claude."), "F12 optimistic-apply: proposed text in buffer"); void key; }); diff --git a/test/e2e/suite/f12InlineDiff.test.ts b/test/e2e/suite/f12InlineDiff.test.ts new file mode 100644 index 0000000..3fe8599 --- /dev/null +++ b/test/e2e/suite/f12InlineDiff.test.ts @@ -0,0 +1,251 @@ +import * as assert from "assert"; +import * as fs from "fs"; +import * as path from "path"; +import * as vscode from "vscode"; +import type { CowritingApi } from "../../../src/extension"; +import { addProposal, setProposalApplied } from "../../../src/proposalModel"; +import { buildFingerprint } from "../../../src/anchorer"; + +const WS = process.env.E2E_WORKSPACE!; +const settle = () => new Promise((r) => setTimeout(r, 400)); + +async function getApi(): Promise { + const ext = vscode.extensions.getExtension("benstull.vscode-cowriting-plugin")!; + const api = (await ext.activate()) as CowritingApi; + assert.ok(api?.trackChangesPreviewController && api?.proposalController, "exports controllers"); + return api; +} + +async function freshDoc(rel: string, body: string): Promise<{ doc: vscode.TextDocument; key: string }> { + const abs = path.join(WS, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, body, "utf8"); + const uri = vscode.Uri.file(abs); + const doc = await vscode.workspace.openTextDocument(uri); + await vscode.window.showTextDocument(doc); + await settle(); + return { doc, key: uri.toString() }; +} + +suite("F12 inline diff — seam landBaseline (#64, INV-48)", () => { + test("applyAgentEdit with landBaseline:false applies text but does NOT advance the baseline", async () => { + const { doc, key } = await freshDoc("docs/f12-land.md", "# T\n\nalpha here.\n"); + const api = await getApi(); + await vscode.commands.executeCommand("cowriting.showTrackChangesPreview"); + await settle(); + const before = api.diffViewController.getBaseline(key)?.text ?? doc.getText(); + const start = doc.getText().indexOf("alpha"); + const ok = await api.attributionController.applyAgentEdit( + doc, + new vscode.Range(doc.positionAt(start), doc.positionAt(start + "alpha".length)), + "ALPHA", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, + { landBaseline: false }, + ); + await settle(); + assert.ok(ok, "edit applied"); + assert.ok(doc.getText().includes("ALPHA"), "text in buffer"); + assert.strictEqual(api.diffViewController.getBaseline(key)?.text ?? doc.getText(), before, "baseline unchanged"); + }); +}); + +suite("F12 inline diff — finalize / revert in place (#64, INV-51)", () => { + // Optimistic apply lands the text + re-anchors; the buffer becomes the accepted result. + test("optimisticApply puts the proposed text in the buffer and re-anchors", async () => { + const { doc } = await freshDoc("docs/f12-opt.md", "# T\n\nReplace alpha please.\n"); + const api = await getApi(); + const p = api.proposalController; + const fp = { text: "Replace alpha please.", before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Replace ALPHA please.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await api.proposalController.optimisticApply(doc, id!); + await settle(); + assert.ok(doc.getText().includes("Replace ALPHA please."), "applied to buffer"); + // re-anchored: the proposal still resolves against the mutated buffer + assert.strictEqual(p.listProposals(doc)[0].anchorStart !== null, true, "re-anchored, resolves"); + assert.strictEqual(p.listProposals(doc)[0].original, "Replace alpha please.", "original stored"); + }); + + test("finalizeInPlace clears the proposal and keeps the applied text (no double-apply)", async () => { + const { doc } = await freshDoc("docs/f12-fin.md", "# T\n\nKeep alpha now.\n"); + const api = await getApi(); + const p = api.proposalController; + const docPath = p.keyFor(doc); + const fp = { text: "Keep alpha now.", before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Keep ALPHA now.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + const ok = await p.finalizeInPlace(docPath, id!); + await settle(); + assert.ok(ok, "finalized"); + assert.strictEqual(doc.getText(), "# T\n\nKeep ALPHA now.\n", "applied text retained, no double-apply"); + assert.strictEqual(p.listProposals(doc).length, 0, "proposal cleared"); + }); + + test("revertInPlace restores the original and clears the proposal", async () => { + const { doc } = await freshDoc("docs/f12-rev.md", "# T\n\nUndo alpha here.\n"); + const api = await getApi(); + const p = api.proposalController; + const docPath = p.keyFor(doc); + const fp = { text: "Undo alpha here.", before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Undo ALPHA here.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + const ok = await p.revertInPlace(docPath, id!); + await settle(); + assert.ok(ok, "reverted"); + assert.strictEqual(doc.getText(), "# T\n\nUndo alpha here.\n", "original restored"); + assert.strictEqual(p.listProposals(doc).length, 0, "proposal cleared"); + }); + + test("rejectAll reverts every pending proposal", async () => { + const { doc } = await freshDoc("docs/f12-rejall.md", "# T\n\nOne aaa.\n\nTwo bbb.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + const p = api.proposalController; + ctl.setEditTurnForTest(async () => ({ replacement: "# T\n\nOne AAA.\n\nTwo BBB.\n", model: "m", sessionId: "s" })); + const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "up"); + await settle(); + for (const id of ids) await p.optimisticApply(doc, id); + await settle(); + assert.ok(doc.getText().includes("AAA") && doc.getText().includes("BBB"), "both applied"); + const { reverted } = await p.rejectAll(doc); + await settle(); + assert.strictEqual(reverted, ids.length, "all reverted"); + assert.strictEqual(doc.getText(), "# T\n\nOne aaa.\n\nTwo bbb.\n", "document restored"); + assert.strictEqual(p.listProposals(doc).length, 0, "all cleared"); + }); +}); + +suite("F12 inline diff — INV-50 listProposals.replaced", () => { + test("listProposals reports the original as `replaced` after optimistic apply", async () => { + const { doc } = await freshDoc("docs/f12-replaced.md", "# R\n\nbrown here.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + ctl.setEditTurnForTest(async () => ({ replacement: "# R\n\nred here.\n", model: "m", sessionId: "s" })); + await ctl.runEditAndPropose(doc, { kind: "document" }, "x"); + await settle(); await settle(); + assert.strictEqual(api.proposalController.listProposals(doc)[0].replaced, "brown here."); + }); +}); + +suite("F12 inline diff — editor surface (#64, INV-48/52)", () => { + test("proposing optimistically applies into the editor and the buffer is the accepted result", async () => { + const { doc } = await freshDoc("docs/f12-editor.md", "# E\n\nThe brown fox runs.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + ctl.setEditTurnForTest(async () => ({ replacement: "# E\n\nThe red fox runs.\n", model: "m", sessionId: "s" })); + await ctl.runEditAndPropose(doc, { kind: "document" }, "recolor the fox"); + await settle(); await settle(); + assert.ok(doc.getText().includes("The red fox runs."), "optimistically applied into the buffer"); + const v = api.proposalController.listProposals(doc)[0]; + assert.strictEqual(v.original, "The brown fox runs.", "original captured for the deletion hint/revert"); + }); + + test("editing the inserted text then finalizing keeps the human edit", async () => { + const { doc } = await freshDoc("docs/f12-edit-keep.md", "# E\n\nalpha word here.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + ctl.setEditTurnForTest(async () => ({ replacement: "# E\n\nALPHA word here.\n", model: "m", sessionId: "s" })); + const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "up"); + await settle(); await settle(); + // human tweaks the inserted text + const at = doc.getText().indexOf("ALPHA"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + 5)), "ALPHA!"); + await vscode.workspace.applyEdit(we); + await settle(); + // keyFor(doc) gives the repo-relative path that finalizeInPlace uses as its key; + // the `key` from freshDoc is the URI string, which would not match for in-workspace docs. + const docKey = api.proposalController.keyFor(doc); + await api.proposalController.finalizeInPlace(docKey, ids[0]); + await settle(); + assert.ok(doc.getText().includes("ALPHA! word here."), "human edit preserved on accept"); + assert.strictEqual(api.proposalController.listProposals(doc).length, 0, "cleared"); + }); +}); + +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"); + }); +}); + +// Reload-safety: a proposal that was optimistically applied in a PRIOR session +// persists with `original` set and its fp re-anchored to the applied text. A fresh +// controller (empty in-memory `applied` set) must NOT re-capture `original` from the +// already-applied buffer — doing so would clobber the true revert target and break +// Reject. (Regression for the final-review CRITICAL; INV-51/54.) +suite("F12 inline diff — reload-safety (#64, INV-51/54)", () => { + test("optimisticApply does not clobber a previously-persisted original", async () => { + const TRUE_ORIGINAL = "The original sentence here."; + const APPLIED = "The APPLIED sentence here."; + const { doc } = await freshDoc("docs/f12-reload.md", `# R\n\n${TRUE_ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const key = p.keyFor(doc); + + // 1) The buffer holds the APPLIED text (as the saved-while-pending file would). + const at = doc.getText().indexOf(TRUE_ORIGINAL); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + TRUE_ORIGINAL.length)), APPLIED); + assert.ok(await vscode.workspace.applyEdit(we), "apply the prior-session applied text"); + await settle(); + + // 2) Record the proposal directly in the sidecar exactly as a prior session left + // it: fp anchored to the APPLIED text + `original` = the TRUE original. We do + // NOT call optimisticApply, so this controller's in-memory `applied` stays empty. + const appliedAt = doc.getText().indexOf(APPLIED); + const appliedFp = buildFingerprint(doc.getText(), { start: appliedAt, end: appliedAt + APPLIED.length }); + let id = ""; + api.sidecarRouter.update(key, (a) => { + id = addProposal(a, appliedFp, APPLIED, { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }).proposalId; + setProposalApplied(a, id, appliedFp, TRUE_ORIGINAL); + }); + p.renderAll(doc); + await settle(); + + // 3) Re-entry as a reload would trigger (EditorProposalController re-applies on + // onDidChangeProposals because `applied` is empty). The guard must preserve original. + await p.optimisticApply(doc, id); + await settle(); + const view = p.listProposals(doc).find((v) => v.id === id); + assert.ok(view, "proposal still present"); + assert.strictEqual(view!.original, TRUE_ORIGINAL, "true original preserved, NOT clobbered with the applied text"); + + // 4) Reject restores the TRUE original (not the applied text). + assert.ok(await p.revertInPlace(key, id), "revert"); + await settle(); + assert.ok(doc.getText().includes(TRUE_ORIGINAL), "reject restored the true original"); + assert.ok(!doc.getText().includes(APPLIED), "applied text removed on reject"); + }); +}); diff --git a/test/e2e/suite/f12Reach.test.ts b/test/e2e/suite/f12Reach.test.ts index 7b10bc6..e559073 100644 --- a/test/e2e/suite/f12Reach.test.ts +++ b/test/e2e/suite/f12Reach.test.ts @@ -98,8 +98,10 @@ suite("F12 SLICE-1 — Ask-Claude reach (#42, INV-38)", () => { 0, "active doc A was NOT edited — editDocument honored the tab URI", ); - // INV-10: proposing never mutates the document. - assert.ok(b.doc.getText().includes("tab target paragraph"), "tab doc unchanged by propose"); + // F12 (INV-48): EditorProposalController optimistically applies proposals into the + // buffer, so the proposed text is now in the tab doc B. Confirm one of the proposal + // texts is present (the tab doc was edited, not A). + assert.ok(b.doc.getText().includes("REWRITTEN"), "F12 optimistic-apply: proposed text in tab doc B"); }); // No URI arg (palette / keybinding) → fall back to the active editor. diff --git a/test/e2e/suite/liveProgress.test.ts b/test/e2e/suite/liveProgress.test.ts index a10bbe7..1ca5c7c 100644 --- a/test/e2e/suite/liveProgress.test.ts +++ b/test/e2e/suite/liveProgress.test.ts @@ -50,7 +50,10 @@ suite("#60 live turn progress (additive + cancel)", () => { assert.strictEqual(ids.length, 1, "one changed block → one proposal, regardless of progress events"); const view = api.proposalController.listProposals(doc).find((v) => v.id === ids[0]); assert.ok(view, "the proposal is live"); - assert.ok(doc.getText().includes("Old paragraph."), "document unchanged by propose (INV-10)"); + // F12 (INV-48): the EditorProposalController optimistically applies the proposed + // text into the buffer, so after settle the active-editor buffer shows the + // replacement. The proposal is still pending (not finalized) until Accept. + assert.ok(doc.getText().includes("New paragraph."), "F12 optimistic-apply: proposed text in buffer"); }); test("an aborted turn proposes nothing (INV-47)", async () => { diff --git a/test/e2e/suite/proposals.test.ts b/test/e2e/suite/proposals.test.ts index 19d4e61..fc6721c 100644 --- a/test/e2e/suite/proposals.test.ts +++ b/test/e2e/suite/proposals.test.ts @@ -63,13 +63,14 @@ suite("F4 propose/accept (host E2E — programmatic ingress, no LLM)", () => { const TARGET = "The propose target sentence lives here."; const REPLACEMENT = "PROPOSED-BY-CLAUDE replacement sentence."; - test("propose records + renders a pending proposal and does NOT touch the document (INV-10)", async () => { + test("propose records a pending proposal; F12 optimistically applies it (INV-48, reverses INV-10)", async () => { const doc = await openDoc(); const api = await getApi(); - const before = doc.getText(); await proposeViaCommand(doc, TARGET, REPLACEMENT, "turn-p1"); await settle(); - assert.strictEqual(doc.getText(), before, "document text unchanged (INV-10)"); + // F12 (INV-48): EditorProposalController auto-applies the proposed text into the + // buffer, so after settle the buffer has the replacement text (INV-10 is reversed). + assert.ok(doc.getText().includes(REPLACEMENT), "F12 optimistic-apply: replacement in buffer after propose"); const rendered = api.proposalController.getRendered(DOC_REL); assert.strictEqual(rendered.length, 1); assert.strictEqual(rendered[0].pending, true); @@ -77,7 +78,8 @@ suite("F4 propose/accept (host E2E — programmatic ingress, no LLM)", () => { assert.strictEqual(rendered[0].canReply, false, "proposals are decide-only — no dead reply input (INV-12)"); const art = readSidecar(); assert.strictEqual(art.proposals.length, 1, "proposal persisted at propose time"); - assert.strictEqual(art.anchors[art.proposals[0].anchorId].fingerprint.text, TARGET); + // After optimistic apply, the fingerprint re-anchors to the replacement text. + assert.strictEqual(art.anchors[art.proposals[0].anchorId].fingerprint.text, REPLACEMENT); }); test("accept applies via the seam: text replaced, Claude-attributed, proposal removed (INV-9/11/13)", async () => { @@ -97,15 +99,18 @@ suite("F4 propose/accept (host E2E — programmatic ingress, no LLM)", () => { assert.strictEqual(readSidecar().proposals.length, 0, "removed from the sidecar"); }); - test("reject leaves the document untouched and removes the proposal (PUC-3)", async () => { + test("reject reverts the optimistically-applied text and removes the proposal (PUC-3, F12-INV-48)", async () => { const doc = await openDoc(); const api = await getApi(); const before = doc.getText(); const id = await proposeViaCommand(doc, "A second target for coexistence checks.", "WOULD-BE replacement.", "turn-p2"); await settle(); - assert.strictEqual(api.proposalController.rejectById(DOC_REL, id), true); + // F12: optimistic apply has put "WOULD-BE replacement." in the buffer. + assert.ok(doc.getText().includes("WOULD-BE replacement."), "F12: optimistically applied"); + // rejectByIdInPlace reverts the buffer to the original (PUC-3, INV-51). + assert.ok(await api.proposalController.rejectByIdInPlace(DOC_REL, id), "rejectByIdInPlace removes + reverts"); await settle(); - assert.strictEqual(doc.getText(), before, "document untouched"); + assert.strictEqual(doc.getText(), before, "document restored to pre-propose state"); assert.strictEqual(api.proposalController.getRendered(DOC_REL).length, 0); assert.strictEqual(readSidecar().proposals.length, 0); }); @@ -114,25 +119,35 @@ suite("F4 propose/accept (host E2E — programmatic ingress, no LLM)", () => { let doc = await openDoc(); const api = await getApi(); const anchor = "A stable closing paragraph."; - await proposeViaCommand(doc, anchor, "A PROPOSED closing paragraph.", "turn-p3"); + const appliedText = "A PROPOSED closing paragraph."; + await proposeViaCommand(doc, anchor, appliedText, "turn-p3"); await settle(); + // F12: optimistic apply has put the proposed text in the buffer (anchor → appliedText). + assert.ok(doc.getText().includes(appliedText), "F12: optimistically applied before reload"); const uri = vscode.Uri.file(path.join(WS, DOC_REL)); + // doc.getText() now has appliedText; the reload prepends a line. doc = await externalWriteAndReload(uri, "PREPENDED LINE\n\n" + doc.getText()); await settle(); api.proposalController.renderAll(doc); const rendered = api.proposalController.getRendered(DOC_REL); assert.strictEqual(rendered.length, 1, "proposal survived reload"); assert.strictEqual(rendered[0].pending, true, "still decidable"); - const moved = doc.getText().indexOf(anchor); - assert.strictEqual(rendered[0].range.start, moved, "re-anchored after the move"); + // After F12 re-anchor, the fingerprint tracks the appliedText, not the original anchor. + const moved = doc.getText().indexOf(appliedText); + assert.ok(moved >= 0, "applied text present in the reloaded document"); + assert.strictEqual(rendered[0].range.start, moved, "re-anchored to appliedText after the move"); }); test("editing the target text makes the proposal stale: flagged, accept refused, doc untouched (INV-11)", async () => { let doc = await openDoc(); const api = await getApi(); const id = api.proposalController.getRendered(DOC_REL)[0].id; - const mangled = doc.getText().replace("A stable closing paragraph.", "A reworded closing paragraph."); - doc = await externalWriteAndReload(vscode.Uri.file(path.join(WS, DOC_REL)), mangled); + // After F12 optimistic apply the fingerprint tracks the APPLIED text ("A PROPOSED + // closing paragraph."), not the original. Mangle that text to make the proposal stale. + const preMangled = doc.getText(); + const mangled = preMangled.replace("A PROPOSED closing paragraph.", "A reworded closing paragraph."); + const uri = vscode.Uri.file(path.join(WS, DOC_REL)); + doc = await externalWriteAndReload(uri, mangled); await settle(); api.proposalController.renderAll(doc); assert.strictEqual(api.proposalController.getStaleCount(DOC_REL), 1, "flagged stale"); @@ -143,22 +158,30 @@ suite("F4 propose/accept (host E2E — programmatic ingress, no LLM)", () => { assert.strictEqual(readSidecar().proposals.length, 1, "proposal still pending (recoverable)"); // discard the husk so later tests see a clean sidecar assert.strictEqual(api.proposalController.rejectById(DOC_REL, id), true); + // Restore the file so subsequent tests can find their fixture targets: + // replace the optimistically-applied text back to the original fixture text. + const restored = preMangled.replace("A PROPOSED closing paragraph.", "A stable closing paragraph."); + doc = await externalWriteAndReload(uri, restored); + await settle(); }); test("multiple pending proposals coexist and are decidable out of order (PUC-5)", async () => { const doc = await openDoc(); const api = await getApi(); - const id1 = await proposeViaCommand(doc, "A stable opening paragraph.", "An ACCEPTED opening paragraph.", "turn-p4"); - const id2 = await proposeViaCommand(doc, "PROPOSED-BY-CLAUDE replacement sentence.", "A REPLACED-AGAIN sentence.", "turn-p5"); + // Clean up any proposals left over from prior tests (F12: proposals persist in state). + await api.proposalController.rejectAll(doc); await settle(); - assert.strictEqual(api.proposalController.getRendered(DOC_REL).length, 2); + const id1 = await proposeViaCommand(doc, "A stable opening paragraph.", "An ACCEPTED opening paragraph.", "turn-p4"); + const id2 = await proposeViaCommand(doc, "A stable closing paragraph.", "A REPLACED closing paragraph.", "turn-p5"); + await settle(); + assert.strictEqual(api.proposalController.getRendered(DOC_REL).length, 2, "two new proposals"); // decide the SECOND first, then the first — order independence assert.strictEqual(await api.proposalController.acceptById(DOC_REL, id2), true); await settle(); assert.strictEqual(await api.proposalController.acceptById(DOC_REL, id1), true); await settle(); assert.ok(doc.getText().includes("An ACCEPTED opening paragraph.")); - assert.ok(doc.getText().includes("A REPLACED-AGAIN sentence.")); + assert.ok(doc.getText().includes("A REPLACED closing paragraph.")); assert.strictEqual(api.proposalController.getRendered(DOC_REL).length, 0); }); }); diff --git a/test/model.test.ts b/test/model.test.ts index 7baf525..303c6be 100644 --- a/test/model.test.ts +++ b/test/model.test.ts @@ -1,4 +1,5 @@ -import { describe, it, expect } from "vitest"; +import assert from "node:assert"; +import { describe, it, test, expect } from "vitest"; import { SCHEMA_VERSION, emptyArtifact, @@ -179,6 +180,19 @@ describe("unknown-field preservation (F5 SLICE-2, INV-15)", () => { }); }); +test("serializeArtifact round-trips Proposal.original", () => { + const a = emptyArtifact("doc.md"); + a.anchors["a_1"] = { fingerprint: { text: "new", before: "", after: "", lineHint: 0 } }; + a.proposals.push({ + id: "pr_1", anchorId: "a_1", replacement: "new", + author: { kind: "agent", id: "claude", agent: { sdk: "@cline/sdk", model: "sonnet", sessionId: "s" } }, + createdAt: "2026-06-26T00:00:00.000Z", original: "old", granularity: "block", + }); + const json = serializeArtifact(a); + assert.match(json, /"original": "old"/); + assert.match(json, /"granularity": "block"/); +}); + describe("attributions[] (F3 SLICE-1)", () => { it("round-trips an attribution record through serialize → parse", () => { const a = emptyArtifact("docs/x.md"); diff --git a/test/proposalModel.test.ts b/test/proposalModel.test.ts index 9fce315..1075658 100644 --- a/test/proposalModel.test.ts +++ b/test/proposalModel.test.ts @@ -9,7 +9,7 @@ import { type Proposal, } from "../src/model"; import { CoauthorStore } from "../src/store"; -import { addProposal, proposalBody, removeProposal } from "../src/proposalModel"; +import { addProposal, proposalBody, removeProposal, setProposalApplied } from "../src/proposalModel"; const agent = { kind: "agent" as const, @@ -104,4 +104,19 @@ describe("proposalModel helpers (spec §6.4)", () => { expect(body).toContain("- old line two"); expect(body).toContain("+ new only line"); }); + + it("setProposalApplied stores original and re-anchors the fingerprint to the applied text", () => { + const a = emptyArtifact("docs/x.md"); + const { proposalId, anchorId } = addProposal( + a, + { text: "old", before: "", after: "", lineHint: 0 }, + "new", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, + { granularity: "block" }, + ); + setProposalApplied(a, proposalId, { text: "new", before: "", after: "", lineHint: 0 }, "old"); + const p = a.proposals.find((x) => x.id === proposalId)!; + expect(p.original).toBe("old"); + expect(a.anchors[anchorId].fingerprint.text).toBe("new"); + }); }); diff --git a/test/trackChangesModel.test.ts b/test/trackChangesModel.test.ts index 0b396fa..a6b42f4 100644 --- a/test/trackChangesModel.test.ts +++ b/test/trackChangesModel.test.ts @@ -231,7 +231,7 @@ describe("renderReview", () => { 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 ✓/✗ actions", () => { + test("renderReview: a pending proposal renders a blue block with data-proposal-id and Accept/Reject actions", () => { 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"'); @@ -375,6 +375,55 @@ describe("renderReview", () => { const b = renderReview(doc, doc, [], proposals); expect(a).toBe(b); }); + + test("renderReview renders an applied proposal ONCE, not also as a landed diff (INV-50)", () => { + const baseline = "# T\n\nThe brown fox.\n"; + const current = "# T\n\nThe red fox.\n"; // proposal already optimistically applied + const proposals: ProposalView[] = [{ + id: "pr_1", + anchorStart: current.indexOf("The red fox."), + anchorEnd: current.indexOf("The red fox.") + "The red fox.".length, + replaced: "The brown fox.", // original + replacement: "The red fox.", + }]; + const html = renderReview(baseline, current, [], proposals); + // exactly one proposal block + expect((html.match(/cw-proposal/g) ?? []).length).toBe(1); + // the applied paragraph is NOT also emitted as a word-merged changed block + expect(html).not.toContain("brown"); // no double-render of the change as a baseline diff + }); + + test("renderReview keeps author coloring aligned on a block AFTER a length-changing pending proposal (INV-49/50)", () => { + // block 1 has a pending proposal whose applied text is MUCH LONGER than its + // original; block 2 carries a Claude authorship span (in currentText coords). + const baseline = "one short.\n\ntwo stable here.\n"; + const current = "one MUCH LONGER REPLACED TEXT.\n\ntwo stable here.\n"; + const proposals: ProposalView[] = [{ + id: "pr_1", + anchorStart: current.indexOf("one MUCH LONGER REPLACED TEXT."), + anchorEnd: current.indexOf("one MUCH LONGER REPLACED TEXT.") + "one MUCH LONGER REPLACED TEXT.".length, + replaced: "one short.", + replacement: "one MUCH LONGER REPLACED TEXT.", + original: "one short.", + }]; + const sStart = current.indexOf("stable"); + const authorSpans = [{ start: sStart, end: sStart + "stable".length, author: "claude" as const }]; + const html = renderReview(baseline, current, authorSpans, proposals); + // the Claude coloring wraps "stable" exactly — not shifted by the proposal's length delta. + expect(html).toMatch(/stable<\/span>/); + }); + + test("proposalBlockHtml renders Accept/Reject controls with dropdown carets (#64)", () => { + const html = renderReview("a\n", "b\n", [], [ + { id: "pr_1", anchorStart: 0, anchorEnd: 1, replaced: "a", replacement: "b" }, + ]); + expect(html).toMatch(/data-action="accept"/); + expect(html).toMatch(/data-action="reject"/); + expect(html).toMatch(/data-action="acceptAll"/); + expect(html).toMatch(/data-action="rejectAll"/); + expect(html).toMatch(/Accept/); + expect(html).toMatch(/Reject/); + }); }); import { renderTrackChanges as rtc2 } from "../src/trackChangesModel";