benstull/vscode-cowriting-plugin
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(#70): INV-5 — reject after an interior tweak restores the retained original (PR #73)

Browse Source 
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>
This commit was merged in pull request #73.
This commit is contained in:
benstull
2026-07-03 01:19:46 +00:00
parent 42740f7cc6
commit 7583165354
8 changed files with 897 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/anchorer.ts
+24
View File
@@ -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);
src/editorProposalController.ts
+15 -6
View File
@@ -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);
}
}
src/extension.ts
+6 -6
View File
@@ -223,12 +223,12 @@ export function activate(context: vscode.ExtensionContext): CowritingApi | undef
void vscode.window.showWarningMessage("Cowriting: open a Markdown document to reject its proposals.");
return;
}
const { reverted } = await proposalController.rejectAll(doc);
if (reverted > 0) {
void vscode.window.showInformationMessage(
`Cowriting: rejected ${reverted} proposal${reverted === 1 ? "" : "s"}.`,
);
}
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}.`,
);
}),
);
src/proposalController.ts
+111 -28
View File
@@ -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";
@@ -42,6 +42,16 @@ interface DocState {
unresolved: Set<string>;
/** 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>;
}
export class ProposalController implements vscode.Disposable {
@@ -63,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
@@ -135,6 +152,7 @@ export class ProposalController implements vscode.Disposable {
live: new Map(),
unresolved: new Set(),
applied: new Set(),
appliedSpans: new Map(),
};
this.docs.set(docPath, state);
}
@@ -324,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
@@ -374,6 +408,7 @@ export class ProposalController implements vscode.Disposable {
// 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 });
const appliedFp = buildFingerprint(document.getText(), { start: appliedStart, end: appliedEnd });
this.store.update(docPath, (a) => setProposalApplied(a, proposalId, appliedFp, original));
this.renderAll(document);
@@ -415,51 +450,79 @@ export class ProposalController implements vscode.Disposable {
* purposes anymore (the `onDidApplyAgentEdit → advance` wire was removed).
*/
async finalizeInPlace(docPath: string, proposalId: string): Promise<boolean> {
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);
this.renderAll(document);
this.clearProposal(hit.state, proposalId, 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.
* 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> {
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;
// 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.clearProposal(hit.state, proposalId, document);
return true;
}
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;
const span = resolved !== "orphaned" ? resolved : hit.state.appliedSpans.get(proposalId);
if (!span) {
if (!opts?.silent) {
// 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;
}
this.store.update(docPath, (a) => removeProposal(a, proposalId));
hit.state.applied.delete(proposalId);
this.renderAll(document);
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.clearProposal(hit.state, proposalId, 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.
* 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 }> {
if (!this.isTracked(document)) return { reverted: 0 };
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);
@@ -468,19 +531,22 @@ export class ProposalController implements vscode.Disposable {
.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 };
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;
for (const it of ordered) if (await this.revertInPlace(docPath, it.id)) reverted++;
return { reverted };
let skipped = 0;
for (const it of ordered) {
if (await this.revertInPlace(docPath, it.id, { silent: true })) reverted++;
else skipped++;
}
return { reverted, skipped };
}
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 --------------------------------
@@ -510,6 +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: 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);
@@ -528,10 +602,19 @@ export class ProposalController implements vscode.Disposable {
private onDidChange(e: vscode.TextDocumentChangeEvent): void {
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) {
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 shifted = shiftTracked(range, edit);
if (shifted === "distrusted") state.appliedSpans.delete(id);
else state.appliedSpans.set(id, shifted);
}
}
}