CODAP-1118: implement plugin undo/redo

[kswenson]

Summary

• Implement the undoChangeNotice handler with all three operations: undoableActionPerformed, undoButtonPress, redoButtonPress
• Uses a single shared-stack code path for both standalone and external modes, matching V2's mode-agnostic design
• Plugin undo entries are created via withCustomUndoRedo — no MST state changes, just side-effect callbacks that send undoAction/redoAction messages to the originating plugin
• Add proactive clearUndo/clearRedo notifications broadcast to all plugins when undo/redo stacks are emptied
• Fix bug: remove undo strings from all-cases-handler delete, which was the only DI handler that incorrectly created undo entries for plugin-triggered mutations
• Add architecture documentation (v3/doc/plugin-undo-redo-architecture.md) covering the shared-stack design, the protocol, and the five requirements plugins must satisfy
• Analysis and implementation plan in v3/doc/plugin-undo-redo.md

Refs CODAP-1118

Test plan

• Unit tests for all three handler operations (error cases, success, round-trip)
• Verify undoAction/redoAction messages are sent to the correct plugin tile via broadcastMessage
• Verify proactive clearUndo/clearRedo notifications are broadcast when stacks are emptied
• Manual testing with Building Models / SageModeler to validate the shared-stack approach handles interleaving correctly

Generated with Claude Code

[cypress]

codap-v3    Run #10917

Run Properties:   Passed #10917  •  db32f3474f: Increment the build number

Project	codap-v3
Branch Review	main
Run status	Passed #10917
Run duration	02m 09s
Commit	db32f3474f: Increment the build number
Committer	github-actions[bot]
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	4
View all changes introduced in this branch ↗︎

[bfinzer]

👍🏻LGTM

[codecov]

Codecov Report

❌ Patch coverage is 89.39394% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.28%. Comparing base (67af473) to head (2dd000f).
⚠️ Report is 52 commits behind head on main.

Files with missing lines	Patch %	Lines
...interactive/handlers/undo-change-notice-handler.ts	90.74%	5 Missing ⚠️
v3/src/models/document/create-document-model.ts	83.33%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (67af473) and HEAD (2dd000f). Click for more details.

HEAD has 15 uploads less than BASE

Flag	BASE (67af473)	HEAD (2dd000f)
cypress	16	1

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2388       +/-   ##
===========================================
- Coverage   86.35%   70.28%   -16.08%     
===========================================
  Files         768      768               
  Lines       42980    43045       +65     
  Branches    10280    10709      +429     
===========================================
- Hits        37117    30254     -6863     
- Misses       5851    12766     +6915     
- Partials       12       25       +13     

Flag	Coverage Δ
cypress	39.33% <16.94%> (-30.42%)	⬇️
jest	58.68% <89.39%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[scytacki]

The part missing from this analysis is the potential issue with id or other state that probably won't be maintained properly when the plugin undoes or replays its actions.

For example if the plugin creates the graph and then the user modifies the graph. If the graph change is undone and then the plugin's creation of the graph is undone. If the plugin's change is then redone, it will very likely create a tile with a new id instead of using the previous id. Now when the graph change is applied it will fail.

Perhaps this kind of failure is OK, but it should be noted here. Since many of the actions plugins take are creating graphs and tables this seems like it will make the undo support not very useful.

[Copilot]

Pull request overview

Implements Data Interactive plugin undo/redo support in V3 by recording plugin-originated undoable actions as custom history entries on CODAP’s shared undo stack, and relaying undo/redo execution back to the originating plugin.

Changes:

• Implement undoChangeNotice notify handler for undoableActionPerformed, undoButtonPress, and redoButtonPress using a custom undo/redo patcher that sends undoAction/redoAction to the plugin tile.
• Add a document-level MobX reaction that proactively broadcasts clearUndo/clearRedo when stacks become empty.
• Align DI “delete all cases” behavior by removing undo/redo strings so plugin-triggered mutations don’t create undo entries; add unit tests and architecture docs.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
v3/src/models/history/undo-store.test.ts	Adds coverage for history entries consisting only of custom patches (pure side effects).
v3/src/models/document/create-document-model.ts	Adds MobX reaction broadcasting clearUndo/clearRedo notifications to plugins.
v3/src/data-interactive/handlers/undo-change-notice-handler.ts	Implements plugin undo/redo DI protocol and registers custom undo/redo patcher.
v3/src/data-interactive/handlers/undo-change-notice-handler.test.ts	Adds tests for handler operations, targeted undo/redo messaging, and clear notifications.
v3/src/data-interactive/handlers/all-cases-handler.ts	Removes undo/redo strings to prevent plugin-triggered “delete all cases” from creating undo entries.
v3/doc/plugin-undo-redo.md	Adds detailed analysis/plan documenting the shared-stack approach and rationale.
v3/doc/plugin-undo-redo-architecture.md	Adds architecture/protocol documentation and plugin requirements for correct behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kswenson]

The part missing from this analysis is the potential issue with id or other state that probably won't be maintained properly when the plugin undoes or replays its actions.

Good point. We investigated this and confirmed the issue exists in V2 as well — graph modification commands store the component ID in closures, and redo fails if the graph has been destroyed and recreated with a new ID.

It hasn't manifested in practice because SageModeler doesn't make tile creation undoable (tile creation is fire-and-forget, and SageModeler's undo/redo operates on its own internal model, not CODAP tiles).

We've documented this as Known Limitation #4 in the architecture doc (v3/doc/plugin-undo-redo-architecture.md) with two complementary future improvements:

1. ID remapping layer in the undo/redo patch system (general robustness)
2. Optional id parameter in the DI API component creation (for well-behaved plugins)

[dougmartin]

Looks good 👍 @emcelroy and I reviewed it over Zoom.