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>
This commit is contained in:
+59
-11
@@ -42,6 +42,16 @@ interface DocState {
|
|||||||
unresolved: Set<string>;
|
unresolved: Set<string>;
|
||||||
/** ids already optimistically applied to the buffer (so the trigger doesn't re-apply). */
|
/** ids already optimistically applied to the buffer (so the trigger doesn't re-apply). */
|
||||||
applied: Set<string>;
|
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>;
|
||||||
}
|
}
|
||||||
|
|
||||||
export class ProposalController implements vscode.Disposable {
|
export class ProposalController implements vscode.Disposable {
|
||||||
@@ -135,6 +145,7 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
live: new Map(),
|
live: new Map(),
|
||||||
unresolved: new Set(),
|
unresolved: new Set(),
|
||||||
applied: new Set(),
|
applied: new Set(),
|
||||||
|
appliedSpans: new Map(),
|
||||||
};
|
};
|
||||||
this.docs.set(docPath, state);
|
this.docs.set(docPath, state);
|
||||||
}
|
}
|
||||||
@@ -374,6 +385,7 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
// end shifts by the net length delta of the replacement).
|
// end shifts by the net length delta of the replacement).
|
||||||
const appliedStart = resolved.start;
|
const appliedStart = resolved.start;
|
||||||
const appliedEnd = appliedStart + proposal.replacement.length;
|
const appliedEnd = appliedStart + proposal.replacement.length;
|
||||||
|
state.appliedSpans.set(proposalId, { start: appliedStart, end: appliedEnd });
|
||||||
const appliedFp = buildFingerprint(document.getText(), { start: appliedStart, end: appliedEnd });
|
const appliedFp = buildFingerprint(document.getText(), { start: appliedStart, end: appliedEnd });
|
||||||
this.store.update(docPath, (a) => setProposalApplied(a, proposalId, appliedFp, original));
|
this.store.update(docPath, (a) => setProposalApplied(a, proposalId, appliedFp, original));
|
||||||
this.renderAll(document);
|
this.renderAll(document);
|
||||||
@@ -422,6 +434,7 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
this.attribution.signalLanded(document);
|
this.attribution.signalLanded(document);
|
||||||
this.store.update(docPath, (a) => removeProposal(a, proposalId));
|
this.store.update(docPath, (a) => removeProposal(a, proposalId));
|
||||||
hit.state.applied.delete(proposalId);
|
hit.state.applied.delete(proposalId);
|
||||||
|
hit.state.appliedSpans.delete(proposalId);
|
||||||
this.renderAll(document);
|
this.renderAll(document);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
@@ -429,26 +442,48 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
/**
|
/**
|
||||||
* F12/#64 (INV-51): REJECT an optimistically-applied proposal — replace its live
|
* 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
|
* 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.
|
* 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): Promise<boolean> {
|
async revertInPlace(docPath: string, proposalId: string, opts?: { silent?: boolean }): Promise<boolean> {
|
||||||
const hit = this.byId(docPath, proposalId);
|
const hit = this.byId(docPath, proposalId);
|
||||||
if (!hit) return false;
|
if (!hit) return false;
|
||||||
const document = this.openDoc(hit.state);
|
const document = this.openDoc(hit.state);
|
||||||
if (!document) return false;
|
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 fp = hit.state.artifact.anchors[hit.proposal.anchorId]?.fingerprint;
|
||||||
const resolved = fp ? resolve(document.getText(), fp) : "orphaned";
|
const resolved = fp ? resolve(document.getText(), fp) : "orphaned";
|
||||||
if (resolved !== "orphaned" && hit.proposal.original !== undefined) {
|
const span = resolved !== "orphaned" ? resolved : hit.state.appliedSpans.get(proposalId);
|
||||||
const we = new vscode.WorkspaceEdit();
|
if (!span) {
|
||||||
we.replace(
|
if (!opts?.silent) {
|
||||||
document.uri,
|
void vscode.window.showWarningMessage(
|
||||||
new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)),
|
"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).",
|
||||||
hit.proposal.original,
|
);
|
||||||
);
|
}
|
||||||
if (!(await vscode.workspace.applyEdit(we))) return false;
|
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));
|
this.store.update(docPath, (a) => removeProposal(a, proposalId));
|
||||||
hit.state.applied.delete(proposalId);
|
hit.state.applied.delete(proposalId);
|
||||||
|
hit.state.appliedSpans.delete(proposalId);
|
||||||
this.renderAll(document);
|
this.renderAll(document);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
@@ -510,6 +545,9 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
this.recordProposal(state, proposal, { start: off, end: off }, false);
|
this.recordProposal(state, proposal, { start: off, end: off }, false);
|
||||||
} else {
|
} else {
|
||||||
this.recordProposal(state, proposal, resolved, true);
|
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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
this.renderStatus(state);
|
this.renderStatus(state);
|
||||||
@@ -528,10 +566,20 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
|
|
||||||
private onDidChange(e: vscode.TextDocumentChangeEvent): void {
|
private onDidChange(e: vscode.TextDocumentChangeEvent): void {
|
||||||
const state = this.docs.get(this.keyOf(e.document));
|
const state = this.docs.get(this.keyOf(e.document));
|
||||||
if (!state || state.live.size === 0) return;
|
if (!state || (state.live.size === 0 && state.appliedSpans.size === 0)) return;
|
||||||
for (const change of e.contentChanges) {
|
for (const change of e.contentChanges) {
|
||||||
const edit = { start: change.rangeOffset, end: change.rangeOffset + change.rangeLength, newLength: change.text.length };
|
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));
|
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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -204,6 +204,60 @@ suite("F12 inline diff — control parity (#64, INV-53)", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// #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)");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// Reload-safety: a proposal that was optimistically applied in a PRIOR session
|
// Reload-safety: a proposal that was optimistically applied in a PRIOR session
|
||||||
// persists with `original` set and its fp re-anchored to the applied text. A fresh
|
// persists with `original` set and its fp re-anchored to the applied text. A fresh
|
||||||
// controller (empty in-memory `applied` set) must NOT re-capture `original` from the
|
// controller (empty in-memory `applied` set) must NOT re-capture `original` from the
|
||||||
|
|||||||
Reference in New Issue
Block a user