diff --git a/src/anchorer.ts b/src/anchorer.ts index e8f9162..664f544 100644 --- a/src/anchorer.ts +++ b/src/anchorer.ts @@ -101,6 +101,30 @@ export function resolve(docText: string, fp: Fingerprint): OffsetRange | "orphan return "orphaned"; } +/** + * #70 (INV-5/INV-11): maintain a TRACKED applied span across an in-session edit. + * Unlike `shift`, which clamps every point into a best-effort range, a tracked + * span must stay *provably* meaningful — it is a revert target. So: + * - an edit fully OUTSIDE or fully INSIDE the span shifts it (the span is + * still "the applied block, as tweaked"); + * - an insertion exactly AT the span start lands BEFORE the span (both ends + * shift right) — matching exact-resolve semantics, where text typed just + * before the applied block is never part of it; + * - any edit STRADDLING a span boundary returns "distrusted": the shifted + * result would be a clamped guess, and stale text is never reverted by + * guess (INV-11). + */ +export function shiftTracked(range: OffsetRange, edit: TextEdit): OffsetRange | "distrusted" { + const delta = edit.newLength - (edit.end - edit.start); + if (edit.start === edit.end && edit.start === range.start) { + return { start: range.start + delta, end: range.end + delta }; + } + const outside = edit.end <= range.start || edit.start >= range.end; + const inside = edit.start >= range.start && edit.end <= range.end; + if (!outside && !inside) return "distrusted"; + return shift(range, edit); +} + /** Maintain a live range across an in-session edit (spec §6.4 `shift`). */ export function shift(range: OffsetRange, edit: TextEdit): OffsetRange { const delta = edit.newLength - (edit.end - edit.start); diff --git a/src/editorProposalController.ts b/src/editorProposalController.ts index 379f4d4..cdf4bf1 100644 --- a/src/editorProposalController.ts +++ b/src/editorProposalController.ts @@ -226,9 +226,16 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL if (document.languageId !== "markdown") return []; if (!this.registry.isCoediting(document.uri)) return []; const key = this.proposals.keyFor(document); - const applied = this.proposals - .listProposals(document) - .filter((v) => v.anchorStart !== null && this.proposals.isApplied(key, v.id)); + // #70: an interior tweak orphans the exact anchor (anchorStart null) but the + // proposal stays fully decidable — Keep by id, Reject via the tracked span — + // so the lens pair anchors at the tracked span instead of vanishing (which + // made the fixed reject path unreachable from the primary gesture surface). + const applied: { id: string; at: number }[] = []; + for (const v of this.proposals.listProposals(document)) { + if (!this.proposals.isApplied(key, v.id)) continue; + const at = v.anchorStart ?? this.proposals.trackedSpan(key, v.id)?.start; + if (at !== undefined) applied.push({ id: v.id, at }); + } const lenses: vscode.CodeLens[] = []; if (applied.length >= 2) { const top = new vscode.Range(0, 0, 0, 0); @@ -238,7 +245,7 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL ); } for (const v of applied) { - const pos = document.positionAt(v.anchorStart!); + const pos = document.positionAt(v.at); const line = new vscode.Range(pos.line, 0, pos.line, 0); lenses.push( new vscode.CodeLens(line, { title: "✓ Keep", command: "cowriting.proposalAcceptMenu", arguments: [v.id] }), @@ -261,10 +268,12 @@ export class EditorProposalController implements vscode.Disposable, vscode.CodeL if (!pick) return; const all = pick.includes("ALL"); if (kind === "accept") { - if (all) await this.proposals.acceptAllProposals(doc); + // The batch commands own the applied/skipped reporting (#70: a silent:true + // batch with a discarded tally re-creates the silent-skip failure mode). + if (all) await vscode.commands.executeCommand("cowriting.acceptAllProposals"); else await this.proposals.finalizeInPlace(key, id); } else { - if (all) await this.proposals.rejectAll(doc); + if (all) await vscode.commands.executeCommand("cowriting.rejectAllProposals"); else await this.proposals.revertInPlace(key, id); } } diff --git a/src/proposalController.ts b/src/proposalController.ts index f31e02b..1435729 100644 --- a/src/proposalController.ts +++ b/src/proposalController.ts @@ -13,7 +13,7 @@ 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, buildFingerprint, type OffsetRange } from "./anchorer"; +import { resolve, shift, shiftTracked, buildFingerprint, type OffsetRange } from "./anchorer"; import { addProposal, removeProposal, setProposalApplied } from "./proposalModel"; import type { AttributionController } from "./attributionController"; import type { VersionGuard } from "./versionGuard"; @@ -73,6 +73,13 @@ export class ProposalController implements vscode.Disposable { this.disposables.push(this.onDidChangeProposalsEmitter); this.disposables.push( vscode.workspace.onDidChangeTextDocument((e) => this.onDidChange(e)), + // #70: a tracked applied span is only meaningful while its buffer is live — + // a closed doc can change on disk with no change events, so a span kept + // across close/reopen could revert over unrelated text (INV-11: never by + // guess). Cleared here; a resolving anchor rebuilds it at the next renderAll. + vscode.workspace.onDidCloseTextDocument((doc) => { + this.docs.get(this.keyOf(doc))?.appliedSpans.clear(); + }), // PUC-7 restore-on-enter (mirrors ThreadController/AttributionController): // `renderAll` is the only path that recomputes state.live/unresolved AND // fires onDidChangeProposals — the event EditorProposalController listens @@ -335,6 +342,22 @@ export class ProposalController implements vscode.Disposable { return this.docs.get(docPath)?.applied.has(proposalId) ?? false; } + /** #70: the shift-tracked applied span (the revert-fallback bookkeeping) — + * exposed so the editor surface can keep the decision gestures reachable + * for an applied proposal whose exact anchor an interior tweak orphaned. */ + trackedSpan(docPath: string, proposalId: string): OffsetRange | undefined { + return this.docs.get(docPath)?.appliedSpans.get(proposalId); + } + + /** The one proposal-clear sequence (record + in-memory bookkeeping + re-render) + * shared by finalize/revert/reject so no path leaves a stale entry behind. */ + private clearProposal(state: DocState, proposalId: string, document?: vscode.TextDocument): void { + this.store.update(state.docPath, (a) => removeProposal(a, proposalId)); + state.applied.delete(proposalId); + state.appliedSpans.delete(proposalId); + if (document) this.renderAll(document); + } + /** * 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 @@ -427,15 +450,13 @@ export class ProposalController implements vscode.Disposable { * purposes anymore (the `onDidApplyAgentEdit → advance` wire was removed). */ async finalizeInPlace(docPath: string, proposalId: string): Promise { + if (this.guard.isReadOnly(docPath)) return false; 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); - hit.state.appliedSpans.delete(proposalId); - this.renderAll(document); + this.clearProposal(hit.state, proposalId, document); return true; } @@ -450,6 +471,7 @@ export class ProposalController implements vscode.Disposable { * leaving the proposed text in the buffer. Undo (or ✓ Keep) remains the way out. */ async revertInPlace(docPath: string, proposalId: string, opts?: { silent?: boolean }): Promise { + if (this.guard.isReadOnly(docPath)) return false; const hit = this.byId(docPath, proposalId); if (!hit) return false; const document = this.openDoc(hit.state); @@ -457,10 +479,7 @@ export class ProposalController implements vscode.Disposable { // Never optimistically applied → nothing of ours is in the buffer; clearing // the record IS the reject (the legacy pending-only path). if (hit.proposal.original === undefined) { - this.store.update(docPath, (a) => removeProposal(a, proposalId)); - hit.state.applied.delete(proposalId); - hit.state.appliedSpans.delete(proposalId); - this.renderAll(document); + this.clearProposal(hit.state, proposalId, document); return true; } const fp = hit.state.artifact.anchors[hit.proposal.anchorId]?.fingerprint; @@ -468,9 +487,18 @@ export class ProposalController implements vscode.Disposable { const span = resolved !== "orphaned" ? resolved : hit.state.appliedSpans.get(proposalId); if (!span) { if (!opts?.silent) { - void vscode.window.showWarningMessage( - "Cowriting: this proposal's applied text can't be located (it changed or moved) — undo your edits, or Keep it to leave the buffer as-is (it is never reverted by guess).", - ); + // The revert is refused, but the record must stay dismissable (a window + // reload empties both the tracked spans and the undo stack): offer a + // record-only discard that leaves the buffer exactly as it stands. + const DISCARD = "Discard proposal (leave text)"; + void vscode.window + .showWarningMessage( + "Cowriting: this proposal's applied text can't be located (it changed or moved) — it is never reverted by guess. Undo your edits, Keep it, or discard the record as-is.", + DISCARD, + ) + .then((choice) => { + if (choice === DISCARD) this.clearProposal(hit.state, proposalId, document); + }); } return false; } @@ -481,10 +509,7 @@ export class ProposalController implements vscode.Disposable { hit.proposal.original, ); if (!(await vscode.workspace.applyEdit(we))) return false; - this.store.update(docPath, (a) => removeProposal(a, proposalId)); - hit.state.applied.delete(proposalId); - hit.state.appliedSpans.delete(proposalId); - this.renderAll(document); + this.clearProposal(hit.state, proposalId, document); return true; } @@ -521,9 +546,7 @@ export class ProposalController implements vscode.Disposable { private reject(state: DocState, proposal: Proposal): void { if (this.guard.isReadOnly(state.docPath)) return; - this.store.update(state.docPath, (a) => removeProposal(a, proposal.id)); - const document = this.openDoc(state); - if (document) this.renderAll(document); + this.clearProposal(state, proposal.id, this.openDoc(state)); } // ---- PUC-4: load / external change / resolve-or-flag -------------------------------- @@ -553,9 +576,14 @@ export class ProposalController implements vscode.Disposable { this.recordProposal(state, proposal, { start: off, end: off }, false); } else { this.recordProposal(state, proposal, resolved, true); - // #70: an exact resolve is ground truth — (re)sync the applied-range - // bookkeeping (covers a prior-session apply, whose in-memory map is empty). - if (state.applied.has(proposal.id)) state.appliedSpans.set(proposal.id, resolved); + // #70: REBUILD the applied-range bookkeeping from an exact resolve when + // the map has no entry (a prior-session apply, or after a close/reopen + // clear). An entry already present has been shift-maintained continuously + // since apply and is ground truth — never clobbered by a resolve, which + // can land on an exact duplicate of the applied text elsewhere. + if (state.applied.has(proposal.id) && !state.appliedSpans.has(proposal.id)) { + state.appliedSpans.set(proposal.id, resolved); + } } } this.renderStatus(state); @@ -583,10 +611,9 @@ export class ProposalController implements vscode.Disposable { // external replace — makes the shifted range a clamped guess, so the span // is distrusted (deleted) rather than reverted-to by guess (INV-11). for (const [id, range] of state.appliedSpans) { - const outside = edit.end <= range.start || edit.start >= range.end; - const inside = edit.start >= range.start && edit.end <= range.end; - if (outside || inside) state.appliedSpans.set(id, shift(range, edit)); - else state.appliedSpans.delete(id); + const shifted = shiftTracked(range, edit); + if (shifted === "distrusted") state.appliedSpans.delete(id); + else state.appliedSpans.set(id, shifted); } } } diff --git a/test/anchorer.test.ts b/test/anchorer.test.ts index 852ddff..5dea86f 100644 --- a/test/anchorer.test.ts +++ b/test/anchorer.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { buildFingerprint, resolve, shift, type OffsetRange } from "../src/anchorer"; +import { buildFingerprint, resolve, shift, shiftTracked, type OffsetRange } from "../src/anchorer"; const DOC = "line0\nline1 needle here\nline2\nneedle again on line3\n"; @@ -67,3 +67,54 @@ describe("shift", () => { expect(shift(r, { start: 3, end: 7, newLength: 1 })).toEqual({ start: 3, end: 7 }); }); }); + +describe("shiftTracked (#70 — tracked applied spans)", () => { + const r: OffsetRange = { start: 10, end: 20 }; + + it("edit entirely before the span shifts both endpoints by the delta", () => { + expect(shiftTracked(r, { start: 0, end: 0, newLength: 3 })).toEqual({ start: 13, end: 23 }); + }); + + it("edit entirely after the span leaves it unchanged", () => { + expect(shiftTracked(r, { start: 25, end: 28, newLength: 0 })).toEqual({ start: 10, end: 20 }); + }); + + it("edit fully inside the span keeps the start and grows/shrinks the end", () => { + // replace [12,15) with 5 chars (delta = +2) + expect(shiftTracked(r, { start: 12, end: 15, newLength: 5 })).toEqual({ start: 10, end: 22 }); + }); + + it("replacing the span's exact full contents is an interior edit", () => { + expect(shiftTracked(r, { start: 10, end: 20, newLength: 4 })).toEqual({ start: 10, end: 14 }); + }); + + it("an insertion exactly at the span start lands BEFORE the span (never absorbed)", () => { + expect(shiftTracked(r, { start: 10, end: 10, newLength: 6 })).toEqual({ start: 16, end: 26 }); + }); + + it("an insertion exactly at the span end lands AFTER the span (never absorbed)", () => { + expect(shiftTracked(r, { start: 20, end: 20, newLength: 6 })).toEqual({ start: 10, end: 20 }); + }); + + it("an insertion at an EMPTY span's point lands before it (deletion-proposal span)", () => { + const empty: OffsetRange = { start: 10, end: 10 }; + expect(shiftTracked(empty, { start: 10, end: 10, newLength: 3 })).toEqual({ start: 13, end: 13 }); + }); + + it("an edit straddling the span's start boundary is distrusted (INV-11)", () => { + expect(shiftTracked(r, { start: 8, end: 12, newLength: 0 })).toBe("distrusted"); + }); + + it("an edit straddling the span's end boundary is distrusted (INV-11)", () => { + expect(shiftTracked(r, { start: 18, end: 22, newLength: 1 })).toBe("distrusted"); + }); + + it("an edit fully containing the span (e.g. a wholesale replace) is distrusted (INV-11)", () => { + expect(shiftTracked(r, { start: 0, end: 30, newLength: 12 })).toBe("distrusted"); + }); + + it("a deletion consuming an EMPTY span's neighborhood is distrusted (INV-11)", () => { + const empty: OffsetRange = { start: 10, end: 10 }; + expect(shiftTracked(empty, { start: 8, end: 12, newLength: 0 })).toBe("distrusted"); + }); +}); diff --git a/test/e2e/suite/f12InlineDiff.test.ts b/test/e2e/suite/f12InlineDiff.test.ts index 3be02c7..4b35097 100644 --- a/test/e2e/suite/f12InlineDiff.test.ts +++ b/test/e2e/suite/f12InlineDiff.test.ts @@ -320,6 +320,35 @@ suite("F12 inline diff — #70 reject after interior edit (INV-5)", () => { assert.strictEqual(doc.getText(), "# T\n\nOne aaa.\n\nTwo bbb.\n", "document fully restored (INV-5)"); assert.strictEqual(p.listProposals(doc).length, 0, "all cleared"); }); + + // The decision gestures must stay REACHABLE for a tweaked proposal: the ✓/✗ + // CodeLens pair anchors at the tracked span when the interior tweak orphans + // the exact anchor — otherwise the fixed reject path exists only at the API. + test("the ✓ Keep / ✗ Reject CodeLens pair survives an interior tweak (tracked-span anchor)", async () => { + const ORIGINAL = "Lens target sentence here."; + const { doc } = await freshDoc("docs/s70-lens-tweak.md", `# T\n\n${ORIGINAL}\n`); + const api = await getApi(); + const p = api.proposalController; + const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 }; + const id = await p.propose(doc, fp, "Lens TARGET sentence here.", + { kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" }); + await p.optimisticApply(doc, id!); + await settle(); + const at = doc.getText().indexOf("TARGET"); + const we = new vscode.WorkspaceEdit(); + we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + 6)), "TARGET-tweaked"); + assert.ok(await vscode.workspace.applyEdit(we), "interior tweak applied"); + await settle(); + assert.strictEqual(p.listProposals(doc).find((v) => v.id === id)!.anchorStart, null, "anchor orphaned"); + const lenses = api.editorProposalController.provideCodeLenses(doc); + const titles = lenses.map((l) => l.command?.title); + assert.deepStrictEqual(titles, ["✓ Keep", "✗ Reject"], "the pair survives the tweak"); + assert.deepStrictEqual(lenses[1].command!.arguments, [id], "reject lens still targets the tweaked proposal"); + // The reject it routes at still restores the original (the Task-1 contract). + assert.ok(await p.revertInPlace(p.keyFor(doc), id!), "reject via the surviving gesture succeeds"); + await settle(); + assert.strictEqual(doc.getText(), `# T\n\n${ORIGINAL}\n`, "original restored exactly (INV-5)"); + }); }); // Reload-safety: a proposal that was optimistically applied in a PRIOR session