sneak/vaultik
1
0
Fork 0
Code Issues 7 Pull Requests 3 Actions Packages Projects Releases Wiki Activity

fix: replace O(n²) duplicate detection with map-based O(1) lookups #45

Open
clawbot wants to merge 1 commits from fix/dedup-snapshot-ids-on2-to-o1 into main
pull from: fix/dedup-snapshot-ids-on2-to-o1
merge into: sneak:main
Conversation 4 Commits 1 Files Changed 1 +3 -8
clawbot commented 2026-03-17 13:49:11 +01:00
Collaborator
Copy Link

Replace linear scan deduplication of snapshot IDs in RemoveAllSnapshots() and PruneBlobs() with map[string]bool for O(1) lookups.

Previously, each new snapshot ID was checked against the entire collected slice via a linear scan, resulting in O(n²) overall complexity. Now a seen map provides constant-time membership checks while preserving insertion order in the slice.

Changes:

  • internal/vaultik/snapshot.go (RemoveAllSnapshots): replaced linear for loop dedup with seen map
  • internal/vaultik/prune.go (PruneBlobs): replaced linear for loop dedup with seen map

closes #12

Replace linear scan deduplication of snapshot IDs in `RemoveAllSnapshots()` and `PruneBlobs()` with `map[string]bool` for O(1) lookups. Previously, each new snapshot ID was checked against the entire collected slice via a linear scan, resulting in O(n²) overall complexity. Now a `seen` map provides constant-time membership checks while preserving insertion order in the slice. **Changes:** - `internal/vaultik/snapshot.go` (`RemoveAllSnapshots`): replaced linear `for` loop dedup with `seen` map - `internal/vaultik/prune.go` (`PruneBlobs`): replaced linear `for` loop dedup with `seen` map closes https://git.eeqj.de/sneak/vaultik/issues/12
clawbot added 1 commit 2026-03-17 13:49:11 +01:00
fix: replace O(n²) duplicate detection with map-based O(1) lookups
All checks were successful
check / check (pull_request) Successful in 2m28s
Details
6522ccea75 
Replace linear scan deduplication of snapshot IDs in RemoveAllSnapshots()
and PruneBlobs() with map[string]bool for O(1) lookups.

Previously, each new snapshot ID was checked against the entire collected
slice via a linear scan, resulting in O(n²) overall complexity. Now a
'seen' map provides constant-time membership checks while preserving
insertion order in the slice.

closes #12
clawbot added the needs-reviewneeds-review labels 2026-03-17 13:49:17 +01:00
clawbot removed the needs-review label 2026-03-17 13:52:44 +01:00
clawbot commented 2026-03-17 13:54:41 +01:00
Author
Collaborator
Copy Link

Review: PASS ✅

What was reviewed: PR #45 — replace O(n²) duplicate detection with map-based O(1) lookups, fixing issue #12.

Changes verified:

  • internal/vaultik/snapshot.go (RemoveAllSnapshots): linear scan dedup replaced with seen map — correct
  • internal/vaultik/prune.go (PruneBlobs): same pattern applied — correct
  • Both functions preserve insertion order via the existing snapshotIDs slice; the seen map is only used for O(1) membership checks
  • seen[id] = true is set before the append, keeping the map and slice in sync
  • 2 files, +6/-17 lines — minimal, focused change with no behavioral differences beyond performance

Integrity checks:

  • No test files modified
  • No linter config, Makefile, or Dockerfile changes
  • Single commit, clean diff
  • docker build . passes (lint, fmt-check, tests all green)

No issues found.

**Review: PASS** ✅ **What was reviewed:** [PR #45](https://git.eeqj.de/sneak/vaultik/pulls/45) — replace O(n²) duplicate detection with map-based O(1) lookups, fixing [issue #12](https://git.eeqj.de/sneak/vaultik/issues/12). **Changes verified:** - `internal/vaultik/snapshot.go` (`RemoveAllSnapshots`): linear scan dedup replaced with `seen` map — correct - `internal/vaultik/prune.go` (`PruneBlobs`): same pattern applied — correct - Both functions preserve insertion order via the existing `snapshotIDs` slice; the `seen` map is only used for O(1) membership checks - `seen[id] = true` is set before the `append`, keeping the map and slice in sync - 2 files, +6/-17 lines — minimal, focused change with no behavioral differences beyond performance **Integrity checks:** - No test files modified - No linter config, Makefile, or Dockerfile changes - Single commit, clean diff - `docker build .` passes (lint, fmt-check, tests all green) No issues found.
clawbot added merge-ready and removed needs-review labels 2026-03-17 13:56:10 +01:00
sneak was assigned by clawbot 2026-03-17 13:56:10 +01:00
sneak commented 2026-03-19 14:02:57 +01:00
Owner
Copy Link

conflict

conflict
clawbot force-pushed fix/dedup-snapshot-ids-on2-to-o1 from 6522ccea75 to ea8edd653f 2026-03-19 14:08:42 +01:00 Compare
clawbot commented 2026-03-19 14:08:50 +01:00
Author
Collaborator
Copy Link

Rebased onto main, resolving merge conflict in internal/vaultik/prune.go.

The conflict arose because PR #41 refactored PruneBlobs() to extract snapshot listing into listUniqueSnapshotIDs(), which already uses the seen map pattern. So the prune.go change from this PR is no longer needed — it's already incorporated.

The remaining diff (1 file, +3/-8) applies the seen map fix to listAllRemoteSnapshotIDs() in snapshot.go, which still had the O(n²) linear scan.

docker build . passes (lint, fmt-check, all tests green).

Rebased onto main, resolving merge conflict in `internal/vaultik/prune.go`. The conflict arose because [PR #41](https://git.eeqj.de/sneak/vaultik/pulls/41) refactored `PruneBlobs()` to extract snapshot listing into `listUniqueSnapshotIDs()`, which already uses the `seen` map pattern. So the `prune.go` change from this PR is no longer needed — it's already incorporated. The remaining diff (1 file, +3/-8) applies the `seen` map fix to `listAllRemoteSnapshotIDs()` in `snapshot.go`, which still had the O(n²) linear scan. `docker build .` passes (lint, fmt-check, all tests green).
clawbot added needs-review and removed merge-ready labels 2026-03-19 14:09:50 +01:00
clawbot reviewed 2026-03-19 14:11:23 +01:00
clawbot left a comment
Author
Collaborator
Copy Link

REVIEW: PASS ✅

Post-rebase review of PR #45 addressing issue #12.

Diff: 1 file, +3/-8 in internal/vaultik/snapshot.go — clean and minimal.

What changed

listAllRemoteSnapshotIDs previously used an O(n²) linear scan to deduplicate snapshot IDs:

// OLD — O(n²)
found := false
for _, id := range snapshotIDs {
    if id == sid {
        found = true
        break
    }
}
if !found {
    snapshotIDs = append(snapshotIDs, sid)
}

Replaced with O(1) map-based dedup:

// NEW — O(1)
seen := make(map[string]bool)
// ...
if !seen[sid] {
    seen[sid] = true
    snapshotIDs = append(snapshotIDs, sid)
}

Verification

  1. seen map correctly applied — initialized before the loop, checked before append, set on insert. Preserves insertion order in the slice while deduplicating via the map. ✅
  2. No remaining O(n²) dedup patterns — searched the entire codebase for found := false patterns in production code. The only remaining instances are in internal/cli/entry_test.go (test assertions, not snapshot dedup). All other for _, snapshotID := range snapshotIDs loops in prune.go, info.go, and snapshot.go are processing loops over already-deduplicated IDs. ✅
  3. prune.go change from original PR — correctly already incorporated by PR #41's refactoring. No remaining diff needed. ✅
  4. docker build . — passes (lint, fmt-check, tests, build). ✅

Recommendation: merge-ready.

**REVIEW: PASS** ✅ Post-rebase review of [PR #45](https://git.eeqj.de/sneak/vaultik/pulls/45) addressing [issue #12](https://git.eeqj.de/sneak/vaultik/issues/12). **Diff: 1 file, +3/-8 in `internal/vaultik/snapshot.go`** — clean and minimal. ### What changed `listAllRemoteSnapshotIDs` previously used an O(n²) linear scan to deduplicate snapshot IDs: ```go // OLD — O(n²) found := false for _, id := range snapshotIDs { if id == sid { found = true break } } if !found { snapshotIDs = append(snapshotIDs, sid) } ``` Replaced with O(1) map-based dedup: ```go // NEW — O(1) seen := make(map[string]bool) // ... if !seen[sid] { seen[sid] = true snapshotIDs = append(snapshotIDs, sid) } ``` ### Verification 1. **`seen` map correctly applied** — initialized before the loop, checked before append, set on insert. Preserves insertion order in the slice while deduplicating via the map. ✅ 2. **No remaining O(n²) dedup patterns** — searched the entire codebase for `found := false` patterns in production code. The only remaining instances are in `internal/cli/entry_test.go` (test assertions, not snapshot dedup). All other `for _, snapshotID := range snapshotIDs` loops in `prune.go`, `info.go`, and `snapshot.go` are processing loops over already-deduplicated IDs. ✅ 3. **`prune.go` change from original PR** — correctly already incorporated by [PR #41](https://git.eeqj.de/sneak/vaultik/pulls/41)'s refactoring. No remaining diff needed. ✅ 4. **`docker build .`** — passes (lint, fmt-check, tests, build). ✅ **Recommendation: merge-ready.**
clawbot added merge-ready and removed needs-review labels 2026-03-19 14:12:48 +01:00
All checks were successful
check / check (pull_request) Successful in 2m27s
Details
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/dedup-snapshot-ids-on2-to-o1:fix/dedup-snapshot-ids-on2-to-o1
git checkout fix/dedup-snapshot-ids-on2-to-o1
Sign in to join this conversation.
Reviewers
No Reviewers
clawbot
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-review
needs-review
needs-review
needs-review
needs-rework
Code review found issues to fix
No Label merge-ready
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#45
Delete Branch "fix/dedup-snapshot-ids-on2-to-o1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?