Skip to content

Record label edits as delta patches - #360

Open
nabeya11 wants to merge 3 commits into
perf/patch-historyfrom
perf/patch-label
Open

Record label edits as delta patches#360
nabeya11 wants to merge 3 commits into
perf/patch-historyfrom
perf/patch-label

Conversation

@nabeya11

Copy link
Copy Markdown
Member

Changes

Label edits (label / relabel / unlabel) now mutate labels in place and record only the delta.

  • Only the indices and previous values of points whose label actually changed are recorded.
  • relabel / unlabel became nearly identical loops and are consolidated into mutateLabels.
  • With in-place edits, the pre-edit PointCloud object is mutated after an edit. Existing tests that relied on it being untouched now create a fresh cloud per test case.

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

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.86%. Comparing base (04661d1) to head (b964e52).

Files with missing lines Patch % Lines
patch.go 72.54% 8 Missing and 6 partials ⚠️
editor.go 93.10% 1 Missing and 1 partial ⚠️
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.
📢 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 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 labelPatch patch 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.

Comment thread patch.go
Comment thread patch.go
Comment thread editor.go
Comment thread patch_test.go
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>

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