Skip to content

Add apply with inverse capture and per-session undo - #486

Merged
alex-rawlings-yyc merged 2 commits into
mainfrom
441-apply-and-undo
Oct 23, 2026
Merged

alex-rawlings-yyc merged 2 commits into
mainfrom
441-apply-and-undo

Conversation

@alex-rawlings-yyc

@alex-rawlings-yyc alex-rawlings-yyc commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Part of #426; closes #441. Stacked on #485.

Summary

The repository's apply, with inverse capture, and a linear undo history per session.

AnalysisRepository.apply(changes: ChangeSet) -> { inverse: ChangeSet }
ChangeSet = { intent, scope?, tokenAnalyses?, tokenAnalysisLinks?, phraseAnalyses?, ... }   # { puts, deletes } per kind
interlinearizer.undoEdit(sessionId) / redoEdit(sessionId) -> ChangeSet applied, or nothing
  • ChangeSet follows myieye's proposal on [1b] Route getDraft and saveDraft through an async repository #431: one typed list of whole-record puts and deletes per record kind, and the scope's whole record, which carries the boundary delta.
  • DraftRepository.apply builds on the draft its last apply left and rewrites only the books whose content changes, so a payload edit rewrites every book linking it. Every apply marks the draft unsaved.
  • EditHistory keeps each session's undo and redo stacks, capped at main's MAX_UNDO_STEPS. A new edit drops what was undone.

Evidence

  • Conformance tests cover apply, undo (the inverse), redo (the inverse's inverse) with boundary changes, a payload linked from two books edited once, a book whose stored analysis couldn't be read, and failed cleanup.
  • Unit suite at 100% coverage; lint and tsc clean. Eight mutations of inverse capture and history each fail a test.

Decisions with a strong alternative

  • Boundaries ride in the scope's whole record. Alternative: a field-level boundary change. Why not: the contract rules ask for whole records.
  • dirty stays out of the scope's record, and every apply sets it, so an undo after a Save leaves the draft unsaved, even when it returns to the saved content. Alternative: track clean steps in EditHistory, as main does. Why not: Save doesn't reach the session until 4a ([4a] Wire the WebView to the host behind a feature flag #453).
  • An apply touching a book whose stored analysis couldn't be read is refused, and records no undo step (the question on [3b] Add apply with inverse capture and per-session undo #441). Alternative: leave that book out, as replaceAll does. Why not: that is a partial write, which apply's all-or-nothing rule bars.
  • All-or-nothing holds for the repository's draft. A failed write can leave storage partly written until the next save, as replaceAll can.
  • An apply counts as a draft write for Save, so a Save that copied the draft just before doesn't then mark it clean.
  • undoEdit and redoEdit, since undo and redo are the WebView's menu commands.

Deferred

Re-anchoring what undo restores (3j), pool buckets in replies (3k), retry ids on change sets (3k, #450), and each undo step's focus location and summary (3l, #451). Only opening a session clears the history so far; host New and Open, once built, must clear it or arrive as openSession.

Merge Danger

Door: two-way

Nothing calls apply or the undo commands yet.

Blast Radius: DraftRepository's save path, now shared by replaceAll and apply

🤖 Generated with Claude Code


This change is Reviewable

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dd130651-05ae-45d5-8a20-6789c856be47
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@imnasnainaec imnasnainaec left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deviations from #441 and the plan are inline. Future issues that need a comment about this PR:

  • #450 (3k): retry ids on change sets are deferred here with no home.
  • #451 (3l): focus locations on undo steps are deferred here with no home.
  • #453 (4a): it asks to hear what 3b decides for unreadable books. apply refuses, while the flag-off path keeps the edit on screen and leaves it out of the write, so the flag-on and flag-off e2e tests must cover or exclude such a book. If New and Open stay out of the history clearing, 4a must clear it.
  • #448 (3i): a host wipe reaching an unreadable book will be refused by apply unless 3i decides otherwise.
  • #455 (4c): it expects #475's discardUnreadable handling in 3a/3b's whole-scope change (opening a session, New, Open); neither has it yet.
  • #449 (3j): its risk mitigation relies on a whole-scope change clearing the history, which holds only for opening a session.

Drafted by Claude Opus 5.5; slightly reviewed by me.

Comment thread src/services/editHistory.ts Outdated
Comment thread src/types/analysis-repository.ts
Comment thread src/services/editHistory.ts
Comment thread src/__tests__/analysis-repository-conformance.ts
Comment thread src/__tests__/analysis-repository-conformance.ts
Comment thread src/services/draftRepository.ts
@alex-rawlings-yyc
alex-rawlings-yyc marked this pull request as ready for review October 23, 2026 18:27

@imnasnainaec imnasnainaec left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imnasnainaec reviewed 18 files and all commit messages, and resolved 6 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

@alex-rawlings-yyc
alex-rawlings-yyc merged commit 3e41b87 into main Oct 23, 2026
10 checks passed
@alex-rawlings-yyc
alex-rawlings-yyc deleted the 441-apply-and-undo branch October 23, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[3b] Add apply with inverse capture and per-session undo

2 participants