fix #38: don't mis-attribute undo/redo as fresh human authorship in the preview
Undo in the editor rendered wrong marks in the F10 review preview: restored baseline text (and reverted Claude text) was colored as human-authored. Root cause — attributionController.onDidChange attributed every non-seam document change to currentAuthor() (always human) and ignored e.reason, so an undo/redo that re-inserts text created a fresh human span over it. renderReview is pure; the wrong marks came from these false author spans. Fix: on e.reason === Undo|Redo, reconcile span geometry but do NOT attribute the re-inserted chars — an undo is history navigation, not authorship, so restored text stays neutral (unattributed) rather than falsely claimed by the human. applyChange gains an `attributeInserted` flag (default true; false on undo/redo); seam matching is also skipped on undo/redo (the seam only applies forward edits). Restored text becomes neutral rather than recovering its exact prior provenance (e.g. undoing a deletion of Claude's text shows it unattributed, not blue) — perfect restoration would need an attribution history stack synced to the editor undo stack (fragile due to edit coalescing); filed mentally as a follow-up. The neutral behavior removes the misleading marks, which is the reported defect. Tests: E2E reproduces the mid-edit-undo case (buffer stays dirty so the disk-sync guard doesn't mask it) and asserts restored text is unattributed; +3 unit tests for the geometric-only applyChange path. 197 unit + 50 E2E green; typecheck clean. Closes #38 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -152,18 +152,28 @@ export class AttributionController implements vscode.Disposable {
|
||||
return;
|
||||
}
|
||||
const s = this.state(docPath);
|
||||
// An undo/redo is history navigation, NOT authorship (#38): reconcile span
|
||||
// geometry but never freshly attribute the re-inserted text to the current
|
||||
// author — otherwise restored baseline text (or reverted Claude text) is
|
||||
// falsely colored human in the preview. A seam edit is always a forward
|
||||
// apply, so undo/redo also bypasses seam matching.
|
||||
const isUndoRedo =
|
||||
e.reason === vscode.TextDocumentChangeReason.Undo ||
|
||||
e.reason === vscode.TextDocumentChangeReason.Redo;
|
||||
// One applyEdit = one change event, but the host may deliver a seam edit
|
||||
// as SEVERAL minimal hunks (word-level diffing). Match the EVENT's net
|
||||
// effect against the registry; on a hit the agent owns its FULL intended
|
||||
// replacement (INV-9) — apply it as ONE algebra edit, not per hunk.
|
||||
const hit = this.pending.matchEvent(
|
||||
docPath,
|
||||
e.contentChanges.map((c) => ({
|
||||
start: c.rangeOffset,
|
||||
end: c.rangeOffset + c.rangeLength,
|
||||
newLength: c.text.length,
|
||||
})),
|
||||
);
|
||||
const hit = isUndoRedo
|
||||
? null
|
||||
: this.pending.matchEvent(
|
||||
docPath,
|
||||
e.contentChanges.map((c) => ({
|
||||
start: c.rangeOffset,
|
||||
end: c.rangeOffset + c.rangeLength,
|
||||
newLength: c.text.length,
|
||||
})),
|
||||
);
|
||||
if (hit) {
|
||||
const full = hit.full ?? { start: hit.start, end: hit.end, newLength: hit.newText.length };
|
||||
s.spans = applyChange(s.spans, full, hit.provenance, {
|
||||
@@ -180,10 +190,16 @@ export class AttributionController implements vscode.Disposable {
|
||||
end: change.rangeOffset + change.rangeLength,
|
||||
newLength: change.text.length,
|
||||
};
|
||||
s.spans = applyChange(s.spans, edit, this.currentAuthor(), {
|
||||
newId: () => newId("at"),
|
||||
now: () => new Date().toISOString(),
|
||||
});
|
||||
s.spans = applyChange(
|
||||
s.spans,
|
||||
edit,
|
||||
this.currentAuthor(),
|
||||
{
|
||||
newId: () => newId("at"),
|
||||
now: () => new Date().toISOString(),
|
||||
},
|
||||
!isUndoRedo,
|
||||
);
|
||||
}
|
||||
}
|
||||
if (s.spans.length > 0) s.hadAttributions = true;
|
||||
|
||||
@@ -49,12 +49,19 @@ export function coalesce(spans: LiveSpan[]): LiveSpan[] {
|
||||
* Apply one document edit (the half-open range [start,end) replaced by
|
||||
* newLength chars) authored by `author`. Existing spans shift/split/clip;
|
||||
* inserted chars become a new span of `author` (INV-7).
|
||||
*
|
||||
* `attributeInserted` (default true) controls whether inserted chars get a new
|
||||
* span. Pass `false` for an UNDO/REDO change (#38): the geometry of existing
|
||||
* spans is still reconciled, but re-inserted text is NOT freshly attributed —
|
||||
* an undo is history navigation, not authorship, so restored text stays neutral
|
||||
* rather than being falsely claimed by the current author.
|
||||
*/
|
||||
export function applyChange(
|
||||
spans: LiveSpan[],
|
||||
edit: TextEdit,
|
||||
author: Provenance,
|
||||
ctx: TrackerCtx,
|
||||
attributeInserted = true,
|
||||
): LiveSpan[] {
|
||||
const delta = edit.newLength - (edit.end - edit.start);
|
||||
const out: LiveSpan[] = [];
|
||||
@@ -71,7 +78,7 @@ export function applyChange(
|
||||
if (right) out.push(left ? { ...right, id: ctx.newId() } : right);
|
||||
}
|
||||
}
|
||||
if (edit.newLength > 0) {
|
||||
if (attributeInserted && edit.newLength > 0) {
|
||||
out.push({
|
||||
id: ctx.newId(),
|
||||
start: edit.start,
|
||||
|
||||
Reference in New Issue
Block a user