Record label edits as delta patches - #360
Conversation
Label, relabel and unlabel now mutate labels in place and record only the changed values, instead of cloning the whole cloud and keeping a snapshot. As a consequence the pre-edit PointCloud object is mutated after these edits; tests relying on it being untouched now create a fresh cloud per case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## perf/patch-history #360 +/- ##
======================================================
- Coverage 45.93% 45.86% -0.07%
======================================================
Files 9 9
Lines 1585 1598 +13
======================================================
+ Hits 728 733 +5
- Misses 808 813 +5
- Partials 49 52 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR changes label-edit operations (label/relabel/unlabel) to mutate the point cloud in place while recording undo history as compact “delta” patches containing only the indices and prior label values that actually changed.
Changes:
- Add a new
labelPatchpatch type with encode/decode support and revert logic. - Update label-edit flows to apply in-place updates and consolidate relabel/unlabel loops via
mutateLabels. - Update/add tests to account for in-place mutation and validate label patch behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| patch.go | Introduces labelPatch, plus uint32-slice encoding/decoding utilities and decode support for the new patch type. |
| editor.go | Switches label-related edits to in-place mutation and records undo as labelPatch deltas; consolidates relabel/unlabel via mutateLabels. |
| patch_test.go | Adds a labelPatch revert test and includes label patches in encode/decode roundtrip coverage. |
| command_test.go | Refactors tests to use fresh point clouds per case due to in-place mutation semantics. |
Suppressed comments (1)
editor.go:190
- mutateLabels pushes an empty labelPatch when no labels changed, creating a no-op undo step. Since this is now an in-place edit, the forced runtime.GC() is also likely unnecessary overhead.
e.push(p)
runtime.GC()
return nil
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Reject a labelPatch whose indices and oldLabels lengths differ, matching the integrity check deletePatch already has. Rewrite the length-field guards in a multiplication-free form so they hold on any int width. runtime.GC after label edits lost its purpose when the whole-cloud clone was removed; the in-place edit leaves no large garbage to collect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
patch.go:212
- writeUint32s allocates a temporary 4*len(vs) byte slice and then copies it into the bytes.Buffer. For large label patches this adds significant peak memory overhead (and extra copying) exactly when trying to keep undo deltas small. Writing each uint32 directly to the buffer avoids the extra allocation.
func writeUint32s(buf *bytes.Buffer, vs []uint32) {
b := make([]byte, 4*len(vs))
for i, v := range vs {
binary.LittleEndian.PutUint32(b[i*4:], v)
}
buf.Write(b)
}
Adapt labelPatch to the encodeHead/payload interface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Changes
Label edits (label / relabel / unlabel) now mutate labels in place and record only the delta.