Skip to content

Store undo history as serialized patches - #359

Open
nabeya11 wants to merge 6 commits into
masterfrom
perf/patch-history
Open

Store undo history as serialized patches#359
nabeya11 wants to merge 6 commits into
masterfrom
perf/patch-history

Conversation

@nabeya11

Copy link
Copy Markdown
Member

Problem

Every edit clones the whole point cloud in the WASM heap and pushes another full copy to the JS heap as undo history. For large PCDs the history alone keeps several times the file size resident, and since the WASM linear memory never shrinks, the per-edit clone turns the peak into permanent residency.

Changes

Groundwork for patch-based undo history; only the history mechanism is replaced.

  • The history uniformly stores serialized bytes of inverse-edit patches.
    • Serialization keeps the history unaware of patch internals (more types to come).
    • The old split storage — a Go-side header list plus raw bytes on the JS heap — is unified into a single self-contained byte sequence.
  • This PR implements only replacePatch (a whole-cloud snapshot); every edit records it, so behavior and memory characteristics stay the same. Follow-up PRs implement per-operation patches and switch edits over.
  • pop() is replaced by squashLatest(), which merges the two newest entries into one undo step.
  • The non-js stub (historyDummy) is replaced by a real implementation (historyMem), making undo behavior testable with plain go test. Added tests that apply random edit sequences, undo them all, and assert byte-exact restoration.

Replace the snapshot undo history (raw copies of the whole cloud plus
a Go-side header list) with a patch-based history: each edit pushes a
patch reverting it, serialized and stored uniformly on the JS heap.
For now every edit type uses replacePatch, a whole-cloud snapshot, so
behavior and memory characteristics are unchanged while the pipeline
(push, serialized storage, undo by revert) is in place. Follow-ups
replace the snapshot fallback with cheap per-operation patches and
compress what remains.

The non-js history stub becomes a real implementation (historyMem),
making undo behavior testable with plain go test; randomized
round-trip tests assert byte-exact restoration. Undo depth semantics
of max_history are unchanged (N entries = N undos).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.51648% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.93%. Comparing base (836d903) to head (04661d1).

Files with missing lines Patch % Lines
patch.go 82.45% 11 Missing and 9 partials ⚠️
editor.go 86.48% 5 Missing ⚠️
undo.go 86.66% 3 Missing and 1 partial ⚠️
command.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #359      +/-   ##
==========================================
+ Coverage   39.34%   45.93%   +6.58%     
==========================================
  Files           8        9       +1     
  Lines        1426     1585     +159     
==========================================
+ Hits          561      728     +167     
+ Misses        829      808      -21     
- Partials       36       49      +13     

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the editor undo history mechanism to store serialized “inverse edit” patches (currently implemented as a whole-cloud replacePatch) instead of storing full point cloud copies, laying groundwork for future per-operation patches and improved memory behavior (especially in WASM/JS).

Changes:

  • Introduces a patch serialization format (patch.go) and uses it to store undo history as packed patch bytes.
  • Replaces the previous undo history API with push(patch), undo(current), and squashLatest() and updates call sites accordingly.
  • Adds Go (non-js) in-memory history implementation plus new tests validating undo round-trips and history constraints.

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
undo.go Replaces dummy history with historyMem storing packed patch chunks for non-js builds.
undo_js.go Refactors JS history to store packed patch bytes in JS Uint8Array chunks and adds squashLatest().
patch.go Adds patch interface plus replacePatch encoding/decoding and chunk reversion logic.
patch_test.go Adds unit tests for replacePatch revert and encode/decode round-trip.
history_test.go Adds randomized editor edit/undo round-trip tests, max history depth tests, and squash tests.
editor.go Updates history interface and editor operations to record replacePatch snapshots on edits.
command.go Replaces the previous pop() usage with squashLatest() for voxel-filter undo grouping.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread undo.go
Comment thread undo_js.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread undo.go
Comment thread undo_js.go
Comment thread patch.go
nabeya11 and others added 2 commits August 22, 2026 21:29
A failed revert used to discard the entry; a later undo would then
apply an older patch to a state it was not recorded against. Keep the
history intact and block undo at the broken entry instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
buf.Bytes() retains the grown capacity of the buffer, which can be
nearly twice the content size and is held long-term by historyMem.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
historyMem exists only in the non-js build; go vet for GOOS=js compiles
test files too and failed on the reference.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread patch.go
Comment thread undo_js.go
Comment thread patch.go
nabeya11 and others added 2 commits August 22, 2026 21:52
Bound nFields by the minimal encoded field size so corrupted counts
fail before allocating, and rewrite the viewpoint bound in the same
multiplication-free form as the other guards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pushing a replacePatch serialized the whole cloud into a Go buffer
before copying it to the JS heap, transiently holding extra full-size
copies in the WASM linear memory, which never shrinks. Split the patch
wire form into a head and a raw payload (encodeHead/payload) and copy
both straight into one Uint8Array, restoring the memory behavior of
the previous direct-copy implementation for snapshots.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@nabeya11
nabeya11 requested a review from at-wat August 24, 2026 01:28
@nabeya11
nabeya11 marked this pull request as ready for review August 24, 2026 01:28
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.

2 participants