fix #38: undo/redo no longer mis-attributes restored text in the review preview #39

Merged
benstull merged 1 commits from bug/38-undo-wrong-marks-in-preview into main 2026-06-12 10:37:49 +00:00
Owner

What

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. (The disk-sync guard masks undo-to-saved-state; the bug bites on mid-edit undo, where the buffer stays dirty.)

Fix

On e.reason === Undo | Redo, reconcile span geometry (shift/clip/remove existing spans) but do not attribute the re-inserted chars — an undo is history navigation, not authorship. 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).

Known limitation (follow-up): restored text becomes neutral/unattributed rather than recovering its exact prior provenance (undoing a deletion of Claude's text shows it unattributed, not blue). Perfect restoration would need an attribution history stack synced to the editor's undo stack, which is fragile due to VS Code edit coalescing. The neutral behavior removes the misleading marks — the reported defect.

Tests

  • E2E (undoMarks.test.ts): reproduces the mid-edit-undo case and asserts restored "bravo" is unattributed + not cw-by-human in the preview render.
  • +3 unit tests for the geometric-only applyChange path (no span for re-inserted chars; still shifts existing spans; still reconciles deletions).
  • 197 unit + 50 E2E green; tsc --noEmit clean. No F7/F10 forward-edit behavior change.

Closes #38

🤖 Generated with Claude Code

## What 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. (The disk-sync guard masks undo-*to-saved-state*; the bug bites on **mid-edit** undo, where the buffer stays dirty.) ## Fix On `e.reason === Undo | Redo`, reconcile span **geometry** (shift/clip/remove existing spans) but do **not** attribute the re-inserted chars — an undo is history navigation, not authorship. `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). **Known limitation (follow-up):** restored text becomes **neutral/unattributed** rather than recovering its *exact* prior provenance (undoing a deletion of Claude's text shows it unattributed, not blue). Perfect restoration would need an attribution history stack synced to the editor's undo stack, which is fragile due to VS Code edit coalescing. The neutral behavior removes the misleading marks — the reported defect. ## Tests - **E2E** (`undoMarks.test.ts`): reproduces the mid-edit-undo case and asserts restored "bravo" is unattributed + not `cw-by-human` in the preview render. - **+3 unit** tests for the geometric-only `applyChange` path (no span for re-inserted chars; still shifts existing spans; still reconciles deletions). - 197 unit + 50 E2E green; `tsc --noEmit` clean. No F7/F10 forward-edit behavior change. Closes #38 🤖 Generated with [Claude Code](https://claude.com/claude-code)
benstull added 1 commit 2026-06-12 10:37:42 +00:00
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>
benstull merged commit 3c8b3597dd into main 2026-06-12 10:37:49 +00:00
Sign in to join this conversation.