BenStullsBets
1247 lines
55 KiB
Markdown
1247 lines
55 KiB
Markdown
# 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 `<del>` 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 `<del>`) 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<string>;
|
||
```
|
||
|
||
(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<boolean> {
|
||
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<boolean> {
|
||
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<boolean> {
|
||
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<boolean> {
|
||
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<boolean> {
|
||
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<boolean> {
|
||
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<void>();
|
||
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<void> {
|
||
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<void> {
|
||
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("<del>brown</del>"); // 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 `<del>original</del><ins>applied</ins>` 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 =
|
||
`<span class="cw-actions">` +
|
||
`<span class="cw-btngroup">` +
|
||
`<button class="cw-accept" data-action="accept">Accept</button>` +
|
||
`<button class="cw-caret" data-action="acceptAll" title="Accept all pending proposals">▾</button>` +
|
||
`</span>` +
|
||
`<span class="cw-btngroup">` +
|
||
`<button class="cw-reject" data-action="reject">Reject</button>` +
|
||
`<button class="cw-caret" data-action="rejectAll" title="Reject all pending proposals">▾</button>` +
|
||
`</span>` +
|
||
`</span>`;
|
||
```
|
||
|
||
- [ ] **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<HTMLElement>(".cw-actions button");
|
||
if (!btn) return;
|
||
const id = btn.closest<HTMLElement>(".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<void> {
|
||
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.
|