Reject on an optimistically-applied proposal now restores the retained original even after interior tweaks: appliedSpans tracked-range fallback (pure shiftTracked, boundary-straddle distrust, close-clears, rebuild-only resync), honest hard failure with a Discard action, INV-16 read-only guards, rejectAll {reverted,skipped} reporting on all batch surfaces, CodeLens reachability at the tracked span. 312 unit + 94/5 host E2E.
Closes #70.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
27 KiB
#70 — INV-5 Reject-After-Interior-Edit Gap 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: ✗ Reject on an optimistically-applied proposal restores the retained original (INV-5) even after the writer typed inside the pending range — and when it genuinely cannot locate the applied span, it fails loudly instead of reporting success while leaving the proposed text behind.
Architecture: Issue #70's fix direction (a) with an honest (b) fallback. A new per-doc appliedSpans: Map<proposalId, OffsetRange> becomes the real F12 applied-range bookkeeping: written at optimistic-apply time, re-synced whenever the exact anchor resolves at renderAll, shifted through interior-safe buffer edits (shift()), and deleted (distrusted) when an edit straddles a span boundary (e.g. a whole-buffer external replace — a shifted range clamps to garbage there, and restoring at a guessed offset would corrupt the doc, INV-11's "never applied by guess"). revertInPlace then reverts via exact resolve → appliedSpans fallback → hard failure (warning, proposal kept, false returned). Direction (c) (fuzzy resolve) is rejected: it guesses. rejectAll orders by the same fallback and reports skips like acceptAllProposals does.
Tech Stack: TypeScript VS Code extension; npm test (vitest unit), npm run test:e2e (@vscode/test-electron host suite — ProposalController is vscode-coupled, so coverage is host E2E, matching every existing F12 test).
Global Constraints
- INV-5: "Reject restores the retained original exactly" — the contract this fix enforces;
revertInPlace's doc comment already promises "reverts the whole block regardless of any in-place edits the human made". - INV-11: anchors are immutable for a proposal's life; stale text is flagged, never applied by guess — the fallback must only use a span whose tracking is provably continuous (no fuzzy resolve, no boundary-straddled ranges).
- INV-12: accept/reject stay human-only gestures; no new mutation paths.
- Keep
✓ Keep(finalizeInPlace) behavior byte-identical — its lookup-by-id bypass of resolution is correct by design (issue "Contrast" note). - Existing test suites must stay green: 265+ unit, full host E2E (notably
proposals.test.tsINV-11 stale-accept,f12InlineDiff.test.ts,fullLoop.test.ts). - Wiggleverse hygiene: no inline trailing comments in CLI blocks; commits cite the issue and carry the
Co-Authored-Bytrailer.
Task 1: appliedSpans bookkeeping + revertInPlace tracked-range fallback (the INV-5 repro)
Files:
- Modify:
src/proposalController.ts(DocState ~line 35,ensureState~127,optimisticApply~336,finalizeInPlace~417,revertInPlace~434,renderAll~495,onDidChange~529) - Test:
test/e2e/suite/f12InlineDiff.test.ts(new suite appended)
Interfaces:
-
Consumes:
shift(range, edit)/resolve(text, fp)fromsrc/anchorer.ts(existing);DocState.applied: Set<string>(existing). -
Produces:
DocState.appliedSpans: Map<string, OffsetRange>(private bookkeeping; Tasks 2–3 rely on its distrust + ordering semantics);revertInPlace(docPath, proposalId, opts?: { silent?: boolean }): Promise<boolean>— signature gains the optionaloptsused by Task 3. -
Step 1: Write the failing E2E test — the issue's repro sketch
Append to test/e2e/suite/f12InlineDiff.test.ts:
// #70 (INV-5): rejecting a proposal AFTER typing inside its pending range must
// still restore the retained original. The exact-substring anchor is orphaned by
// the interior tweak (INV-1/INV-11) — the revert falls back to the F12
// applied-range bookkeeping (appliedSpans), which shift-tracks the span through
// interior edits.
suite("F12 inline diff — #70 reject after interior edit (INV-5)", () => {
test("revertInPlace restores the original after a tweak inside the pending range", async () => {
const ORIGINAL = "The quick brown fox jumps.";
const { doc } = await freshDoc("docs/s70-reject-tweaked.md", `# T\n\n${ORIGINAL}\n`);
const api = await getApi();
const p = api.proposalController;
const docKey = p.keyFor(doc);
const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 };
const id = await p.propose(doc, fp, "The rapid brown fox jumps.",
{ kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" });
await p.optimisticApply(doc, id!);
await settle();
assert.ok(doc.getText().includes("The rapid brown fox jumps."), "optimistically applied");
// Human types INSIDE the pending range → orphans the exact anchor (INV-11).
const at = doc.getText().indexOf("rapid");
const we = new vscode.WorkspaceEdit();
we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + "rapid".length)), "RAPID-ish");
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 by the tweak");
// ✗ Reject: must restore the retained original EXACTLY (INV-5), not skip.
assert.ok(await p.revertInPlace(docKey, id!), "reject reports success");
await settle();
assert.strictEqual(doc.getText(), `# T\n\n${ORIGINAL}\n`, "original restored exactly (INV-5)");
assert.strictEqual(p.listProposals(doc).length, 0, "proposal cleared");
});
test("rejectByIdInPlace routes the tweaked-applied case the same way", async () => {
const ORIGINAL = "A steady closing line.";
const { doc } = await freshDoc("docs/s70-reject-route.md", `# T\n\n${ORIGINAL}\n`);
const api = await getApi();
const p = api.proposalController;
const docKey = p.keyFor(doc);
const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 };
const id = await p.propose(doc, fp, "A wobbly closing line.",
{ 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("wobbly");
const we = new vscode.WorkspaceEdit();
we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at)), "very-");
assert.ok(await vscode.workspace.applyEdit(we), "interior insertion applied");
await settle();
assert.ok(await p.rejectByIdInPlace(docKey, id!), "gesture-level reject succeeds");
await settle();
assert.strictEqual(doc.getText(), `# T\n\n${ORIGINAL}\n`, "original restored exactly (INV-5)");
});
});
- Step 2: Run the new suite to verify it fails
npm run test:e2e 2>&1 | grep -A 3 "#70"
Expected: both tests FAIL — first on doc.getText() still holding the tweaked replacement (RAPID-ish) after a true return (the silent-skip bug), i.e. the "original restored exactly" assertion.
- Step 3: Implement
appliedSpans+ the fallback
In src/proposalController.ts:
(1) DocState gains the map (after applied):
/** ids already optimistically applied to the buffer (so the trigger doesn't re-apply). */
applied: Set<string>;
/**
* #70 (INV-5): the F12 applied-range bookkeeping — proposal id -> the applied
* span's CURRENT buffer range. Written at optimistic-apply, re-synced whenever
* the exact anchor resolves (renderAll), shifted through interior-safe edits,
* and DELETED (distrusted) when an edit straddles a span boundary — a clamped
* range is a guess, and stale text is never applied by guess (INV-11). Lets
* Reject restore the original after the human types INSIDE the pending range
* (which orphans the exact-substring anchor).
*/
appliedSpans: Map<string, OffsetRange>;
(2) ensureState initializes it: appliedSpans: new Map(), after applied: new Set(),.
(3) optimisticApply records the span where it already re-anchors (after state.applied.add(proposalId); and the appliedStart/appliedEnd computation — move the add down beside it or record right after the existing lines):
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;
state.appliedSpans.set(proposalId, { start: appliedStart, end: appliedEnd });
(4) renderAll's resolved branch re-syncs (covers the prior-session-applied reload, where the in-memory map starts empty):
} else {
this.recordProposal(state, proposal, resolved, true);
if (state.applied.has(proposal.id)) state.appliedSpans.set(proposal.id, resolved);
}
(5) onDidChange maintains it — interior-safe edits shift, boundary-straddling edits distrust. Replace the method body:
private onDidChange(e: vscode.TextDocumentChangeEvent): void {
const state = this.docs.get(this.keyOf(e.document));
if (!state || (state.live.size === 0 && state.appliedSpans.size === 0)) return;
for (const change of e.contentChanges) {
const edit = { start: change.rangeOffset, end: change.rangeOffset + change.rangeLength, newLength: change.text.length };
for (const [id, range] of state.live) state.live.set(id, shift(range, edit));
// #70: an edit fully inside a tracked applied span (or fully outside it)
// keeps the span meaningful; one straddling a boundary — e.g. a whole-buffer
// 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);
}
}
}
(6) revertInPlace — the fix itself. Replace the method (keep its position; doc comment updated to describe the layered lookup; opts is consumed by Task 3):
/**
* 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: when an
* interior tweak has orphaned the exact anchor (INV-11), the revert falls back to
* the shift-tracked applied span (#70, INV-5). When neither locates the span
* (e.g. a boundary-straddling external rewrite distrusted it), the reject FAILS
* loudly — warning shown, proposal kept, `false` returned — instead of silently
* leaving the proposed text in the buffer. Undo (or ✓ Keep) remains the way out.
*/
async revertInPlace(docPath: string, proposalId: string, opts?: { silent?: boolean }): Promise<boolean> {
const hit = this.byId(docPath, proposalId);
if (!hit) return false;
const document = this.openDoc(hit.state);
if (!document) return false;
// 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);
return true;
}
const fp = hit.state.artifact.anchors[hit.proposal.anchorId]?.fingerprint;
const resolved = fp ? resolve(document.getText(), fp) : "orphaned";
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).",
);
}
return false;
}
const we = new vscode.WorkspaceEdit();
we.replace(
document.uri,
new vscode.Range(document.positionAt(span.start), document.positionAt(span.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);
hit.state.appliedSpans.delete(proposalId);
this.renderAll(document);
return true;
}
(7) finalizeInPlace cleans the map beside its existing applied.delete:
hit.state.applied.delete(proposalId);
hit.state.appliedSpans.delete(proposalId);
- Step 4: Typecheck + unit tests + the new E2E
npm run typecheck && npm test
npm run test:e2e
Expected: typecheck clean; all unit tests PASS (no unit test touches ProposalController); host E2E PASS including the two new #70 tests and all pre-existing F12/proposals/fullLoop suites.
- Step 5: Commit
git add src/proposalController.ts test/e2e/suite/f12InlineDiff.test.ts
git commit -m "fix(#70): reject after an interior tweak restores the original via tracked applied span (INV-5)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
Task 2: The honest hard-failure path (fix direction (b) for the residual orphan)
Files:
- Modify:
src/proposalController.ts(only if Step 2 exposes a gap — the Task 1 code already implements the path) - Test:
test/e2e/suite/f12InlineDiff.test.ts(extend the #70 suite)
Interfaces:
-
Consumes: Task 1's
appliedSpansdistrust semantics andrevertInPlacefailure contract. -
Produces: nothing new — locks the contract with a regression test.
-
Step 1: Write the failing/locking E2E test
Append inside the #70 suite from Task 1:
Execution note: the first cut of this test used a whole-buffer rewrite as the distrusting edit and FAILED — VS Code minimizes workspace edits before change events fire, so a rewrite sharing a prefix/suffix with the old text decomposes into interior hunks the span legitimately survives (and the revert then correctly restores the original in place). The distrusting edit must straddle a span boundary even after minimization — a deletion from outside the span into its interior, as below.
// Fix direction (b): when the tracked span itself is distrusted (an edit
// straddled its boundary — here a deletion running from the heading into the
// span's interior), the reject FAILS honestly: warning path, proposal kept,
// buffer untouched. Silent-skip-and-report-success (the #70 bug) must not come
// back; reverting at a clamped guessed offset (INV-11) must not either.
// (A boundary-straddling DELETION is used because VS Code minimizes workspace
// edits before change events fire — a wholesale rewrite sharing a prefix/suffix
// decomposes into interior hunks the span legitimately survives.)
test("reject fails loudly — proposal kept, buffer untouched — when the span is distrusted", async () => {
const ORIGINAL = "Sentence one stands here.";
const { doc } = await freshDoc("docs/s70-reject-distrust.md", `# T\n\n${ORIGINAL}\n`);
const api = await getApi();
const p = api.proposalController;
const docKey = p.keyFor(doc);
const fp = { text: ORIGINAL, before: "", after: "", lineHint: 2 };
const id = await p.propose(doc, fp, "Sentence ONE stands here.",
{ kind: "agent", id: "claude", agent: { sdk: "x", model: "m", sessionId: "s" } }, { granularity: "block" });
await p.optimisticApply(doc, id!);
await settle();
// Delete from inside the heading through the middle of the applied span:
// the edit straddles the span's start boundary → the tracked span is
// distrusted AND the exact anchor no longer resolves.
const mid = doc.getText().indexOf("ONE");
const we = new vscode.WorkspaceEdit();
we.replace(doc.uri, new vscode.Range(doc.positionAt(2), doc.positionAt(mid + 1)), "");
assert.ok(await vscode.workspace.applyEdit(we), "boundary-straddling deletion applied");
await settle();
const before = doc.getText();
assert.strictEqual(await p.revertInPlace(docKey, id!), false, "reject reports failure (no silent success)");
await settle();
assert.strictEqual(doc.getText(), before, "buffer untouched — never reverted by guess (INV-11)");
assert.ok(p.listProposals(doc).some((v) => v.id === id), "proposal kept (not silently dropped)");
// Cleanup for later tests: the never-locatable husk is discarded via the
// plain record-only reject.
assert.strictEqual(p.rejectById(docKey, id!), true);
});
- Step 2: Run it
npm run test:e2e 2>&1 | grep -B 2 -A 5 "distrusted"
Expected: PASS if Task 1's implementation is complete (this is the locking regression); if it FAILS, the failure pinpoints which leg (distrust bookkeeping vs. failure return vs. proposal retention) is wrong — fix src/proposalController.ts accordingly, not the test.
- Step 3: Commit
git add test/e2e/suite/f12InlineDiff.test.ts src/proposalController.ts
git commit -m "test(#70): lock the honest hard-failure reject path (distrusted span → warn, keep, false)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
Task 3: rejectAll parity — tweaked proposals revert too, skips are reported
Files:
- Modify:
src/proposalController.ts(rejectAll~line 461),src/extension.ts(cowriting.rejectAllProposalshandler ~line 220) - Test:
test/e2e/suite/f12InlineDiff.test.ts(extend the #70 suite)
Interfaces:
-
Consumes: Task 1's
revertInPlace(docPath, id, opts?: { silent?: boolean })andappliedSpans. -
Produces:
rejectAll(document): Promise<{ reverted: number; skipped: number }>— additiveskippedfield (existing{ reverted }destructurings stay valid). -
Step 1: Write the failing E2E test
Append inside the #70 suite:
// rejectAll must revert a tweaked (orphaned-anchor) proposal via the same
// tracked-span fallback, ordered descending by that span so earlier reverts
// never shift later ones — and must count what it could not revert.
test("rejectAll reverts tweaked proposals via the tracked span and reports skips", async () => {
const { doc } = await freshDoc("docs/s70-rejall-tweak.md", "# T\n\nOne aaa.\n\nTwo bbb.\n");
const api = await getApi();
const p = api.proposalController;
const editFlow = api.editFlow;
editFlow.setEditTurnForTest(async () => ({ replacement: "# T\n\nOne AAA.\n\nTwo BBB.\n", model: "m", sessionId: "s" }));
const ids = await editFlow.runEditAndPropose(doc, { kind: "document" }, "up");
await settle();
for (const id of ids) await p.optimisticApply(doc, id);
await settle();
// Tweak INSIDE the first applied block → its exact anchor orphans.
const at = doc.getText().indexOf("AAA");
const we = new vscode.WorkspaceEdit();
we.replace(doc.uri, new vscode.Range(doc.positionAt(at), doc.positionAt(at + 3)), "AAAZ");
assert.ok(await vscode.workspace.applyEdit(we), "interior tweak applied");
await settle();
const { reverted, skipped } = await p.rejectAll(doc);
await settle();
assert.strictEqual(reverted, ids.length, "ALL proposals reverted, tweaked one included");
assert.strictEqual(skipped, 0, "nothing skipped");
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");
});
- Step 2: Run it to verify the current gap
npm run test:e2e 2>&1 | grep -B 2 -A 5 "rejectAll reverts tweaked"
Expected: FAIL — today the tweaked proposal sorts as an orphan (start: -1, reverted last), and after the first successful revert's renderAll the old code silently removes it without restoring (document fully restored assertion fails), and skipped does not exist on the return type (typecheck catches first — that is the same failure).
- Step 3: Implement — order by the fallback span, count skips, report them
Replace rejectAll in src/proposalController.ts:
/**
* F12/#64 (INV-53): reject EVERY pending proposal on a document — revert each in
* DESCENDING span order (so an earlier revert never shifts a later one's
* offsets), symmetric with #46's accept-all. #70: a tweaked proposal whose exact
* anchor is orphaned orders (and reverts) by its shift-tracked applied span;
* one with no locatable span is SKIPPED — counted, never guessed (INV-11) —
* and the per-proposal warning is suppressed in favor of the batch report.
*/
async rejectAll(document: vscode.TextDocument): Promise<{ reverted: number; skipped: number }> {
if (!this.isTracked(document)) return { reverted: 0, skipped: 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";
const start = r !== "orphaned" ? r.start : state.appliedSpans.get(p.id)?.start ?? -1;
return { id: p.id, start };
})
.sort((a, b) => b.start - a.start);
let reverted = 0;
let skipped = 0;
for (const it of ordered) {
if (await this.revertInPlace(docPath, it.id, { silent: true })) reverted++;
else skipped++;
}
return { reverted, skipped };
}
And in src/extension.ts, the cowriting.rejectAllProposals handler mirrors accept-all's skip note:
const { reverted, skipped } = await proposalController.rejectAll(doc);
if (reverted === 0 && skipped === 0) return;
const skipNote = skipped > 0 ? `, ${skipped} skipped (applied text not locatable — undo your edits or Keep)` : "";
void vscode.window.showInformationMessage(
`Cowriting: rejected ${reverted} proposal${reverted === 1 ? "" : "s"}${skipNote}.`,
);
- Step 4: Typecheck + full test run
npm run typecheck && npm test
npm run test:e2e
Expected: all PASS — including the pre-existing rejectAll E2E tests (f12InlineDiff "rejectAll reverts every pending proposal", "control parity", proposals.test.ts cleanup calls), whose { reverted } destructuring is unaffected by the additive field.
- Step 5: Commit
git add src/proposalController.ts src/extension.ts test/e2e/suite/f12InlineDiff.test.ts
git commit -m "fix(#70): rejectAll reverts tweaked proposals via the tracked span; skips are counted and reported
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
Task 4: Close the loop — fullLoop E2E rejects a tweaked proposal (the deliberate avoidance falls)
Files:
- Modify:
test/e2e/suite/fullLoop.test.ts(the PUC-2 reject section, ~line 144)
Interfaces:
-
Consumes: the shipped Task 1 behavior; existing fullLoop fixtures (
rejectId,REJECT_REPLACEMENT,ORIG_REJECT). -
Produces: nothing — upgrades the flagship E2E so the gap can't silently regress.
-
Step 1: Tweak inside the reject proposal's range before rejecting
In test/e2e/suite/fullLoop.test.ts, immediately before the existing line
assert.ok(await api.proposalController.revertInPlace(docKey, rejectId), "reject reverts in place");,
insert:
// #70 (INV-5): the reject leg now ALSO takes an interior tweak first — the
// original deliberately rejected only an un-tweaked proposal because the
// pre-fix revert silently skipped an orphaned anchor. The tweak orphans the
// exact anchor; the revert must restore ORIG_REJECT via the tracked span.
const rejAt = doc.getText().indexOf(REJECT_REPLACEMENT);
assert.ok(rejAt >= 0, "reject proposal's applied text present");
const rejTweak = new vscode.WorkspaceEdit();
rejTweak.replace(doc.uri, new vscode.Range(doc.positionAt(rejAt + 2), doc.positionAt(rejAt + 2)), "x");
assert.ok(await vscode.workspace.applyEdit(rejTweak), "human tweak inside the reject proposal's range");
await settle();
assert.strictEqual(
api.proposalController.listProposals(doc).find((v) => v.id === rejectId)!.anchorStart,
null,
"tweak orphans the reject proposal's exact anchor (INV-11)",
);
(The existing assertions right after — proposal cleared, ORIG_REJECT restored, REJECT_REPLACEMENT gone — now verify the #70 path. If REJECT_REPLACEMENT is a multi-word string whose slice at +2 splits a word the later !doc.getText().includes(REJECT_REPLACEMENT) assertion still holds; the includes(ORIG_REJECT) restore assertion is the INV-5 check.)
- Step 2: Run the fullLoop suite
npm run test:e2e 2>&1 | grep -B 2 -A 8 "fullLoop\|full-loop\|full loop"
Expected: PASS end-to-end (would FAIL on unfixed code: the buffer would keep the tweaked REJECT_REPLACEMENT, so includes(ORIG_REJECT) fails).
- Step 3: Full verification sweep
npm run typecheck && npm test && npm run test:e2e
Expected: everything green.
- Step 4: Commit
git add test/e2e/suite/fullLoop.test.ts
git commit -m "test(#70): fullLoop reject leg now tweaks inside the pending range first (INV-5 end-to-end)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
Post-execution review wave (not in the original tasks)
A high-effort multi-agent branch review after Task 4 confirmed 10 findings; all
but one were fixed in a follow-up commit on this branch ("harden the
tracked-span revert per branch review"): pure shiftTracked in anchorer.ts
(+11 unit tests; insertion-at-span-start lands before the span), tracked spans
cleared on document close, rebuild-only resync at renderAll, guard.isReadOnly
on revertInPlace/finalizeInPlace, a "Discard proposal (leave text)" action on
the hard-fail warning, CodeLens pair anchored at the tracked span (+1 E2E),
QuickPick batch menu routed through the reporting commands, and one
clearProposal helper (also fixes reject()'s stale applied/appliedSpans).
The remaining finding — resolve() accepts a single exact occurrence without a
context check, so a duplicated block can hijack any anchor consumer — pre-dates
this fix and is filed as its own issue.
Self-review notes
- Issue coverage: direction (a) → Task 1 (
appliedSpansfallback); direction (b) → Task 2 (hard failure, warn, keep); direction (c) explicitly rejected (guessing violates INV-11). The issue's repro sketch is Task 1's test verbatim; the "full-loop E2E deliberately rejects only an un-tweaked proposal" note is retired by Task 4;rejectAll(samerevertInPlaceseam) is covered by Task 3. - Type consistency:
revertInPlace(docPath, proposalId, opts?: { silent?: boolean })defined in Task 1, consumed in Task 3;rejectAllreturns{ reverted, skipped }(additive);appliedSpans: Map<string, OffsetRange>used in Tasks 1 and 3. - Contrast guard:
finalizeInPlaceuntouched except symmetricappliedSpans.deletecleanup — the Keep bypass stays.