feat(aw-sync): add dedupe subcommand for pre-#713 -synced-from- duplicates

[TimeToBuildBob]

Closes item 1 of #717.

What

Adds aw-sync dedupe [--bucket ...] [--dry-run]: for every -synced-from-
bucket (destination buckets pull created), collapse events that are exact
duplicates — same (timestamp, duration, data), using Event's own
PartialEq — down to the lowest-id (first-imported) copy. Reports
per-bucket counts. Never touches a host's own first-hand buckets (only
-synced-from- ids are ever candidates — same marker sync::is_synced_bucket
already uses elsewhere).

aw-sync dedupe --dry-run   # report only
aw-sync dedupe             # delete the extras
aw-sync dedupe --bucket aw-watcher-window-synced-from-host   # restrict to one bucket

Why

#711/#713 stopped new pull passes from re-importing the resume-boundary
event, but didn't touch what already accumulated on destinations that
pulled before that fix (e.g. 550 copies of one stopwatch event on erb-m2).
This is the one-off cleanup for that legacy state, over the local aw-server
HTTP API (get_events/delete_event), so it works against a running
server without touching the sqlite files directly.

Not in this PR

Item 2 of #717 (resetting/deduping the Android staging db, test.db,
before push) is aw-android-side work and out of scope here — noted in the
issue.

Testing

• cargo test -p aw-sync --features cli dedupe — 3 new unit tests on the
  pure grouping logic (find_duplicate_ids): keeps lowest id per group,
  unaffected by input order (get_events returns newest-first), no
  duplicates → no-op.
• cargo clippy -p aw-sync --features cli --all-targets — clean (no new
  warnings).
• cargo fmt -p aw-sync -- --check — clean.

[greptile-apps]

Confidence Score: 4/5

The PR is not yet safe to merge because a large same-timestamp run can still be skipped during pagination, leaving the cleanup incomplete and its event totals inaccurate.

Findings

1.  Boundary Fetch Changes Durations ▶
2.  Large Timestamp Ties Are Skipped ▶
3.  Timestamp Precision Warning Is Incorrect ▶

Summary

Adds an aw-sync dedupe command for identifying and deleting exact duplicate events in explicitly selected synced buckets.

• Uses bounded pagination and retains the lowest-ID event from each duplicate group.
• Requires explicit bucket selection for destructive runs while supporting broad dry-run reporting.
• Corrects boundary-duration handling and the README timestamp-precision caveat.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Select synced buckets] --> B[Fetch bounded newest-first page]
    B --> C{Full page?}
    C -- No --> D[Find exact duplicates]
    C -- Yes --> E{Single timestamp fills page?}
    E -- Yes --> F[Warn and advance before timestamp]
    E -- No --> G[Carry visible boundary events]
    G --> H[Advance before boundary timestamp]
    F --> D
    H --> D
    D --> I{More pages?}
    I -- Yes --> B
    I -- No --> J{Dry run?}
    J -- Yes --> K[Report duplicate counts]
    J -- No --> L[Delete duplicate IDs]

Loading

_{Reviews (3) · Last reviewed commit: "fix(aw-sync/dedupe): carry boundary even..."}

[ErikBjare]

@TimeToBuildBob Correctness is right: only -synced-from- buckets, exact (timestamp, duration, data) at ns, lowest id kept, --dry-run honoured, non-synced --bucket refused. Ran the dry run against my instance (16 synced buckets): 519,313 duplicates — afk_erb-main2-arch 354,133 of 373,525, android-test 155,316 of 306,738, android-unlock 4,797, android 2,349, stopwatch 550, plus the small ones. I checked these are real before trusting the number: the phone's staging db carries exactly the same counts (155,316 / 2,348 / 4,797 at ns precision, so it is the push side over a long time, not ms truncation), and the desktop groups are the classic boundary shape (425 copies of one 185 s AFK event, interleaved ids).

Two practical asks before this runs on a real instance:

1. Page the fetch. get_events(bucket, None, None, None) pulls the whole bucket into one HTTP response — 1.5M events for window_erb-main2-arch; the dry run took 4 minutes and gigabytes. Walk the bucket in time windows (a week at a time is fine; duplicates always share a timestamp, so a window never splits a group) and keep only the per-window fingerprint map.
2. Progress + runtime. 354k individual delete_event calls is an hour of HTTP; log per bucket "deleting N…" and a line every 10k, and say in --help that a large cleanup runs for a while. A delete_events bulk endpoint would be the real fix but is not for this PR.

Document the one caveat in the README line: over HTTP timestamps are ms-precision, so two genuinely distinct events with identical data in the same millisecond would be collapsed — practically nonexistent, but this command deletes, so say it.

LGTM with those. I will run it on erb-m2 once Erik says so; the dry-run numbers above are the before-picture.

[codecov]

Codecov Report

❌ Patch coverage is 42.52874% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.71%. Comparing base (656f3c9) to head (de0c8a5).
⚠️ Report is 132 commits behind head on master.

Files with missing lines	Patch %	Lines
aw-sync/src/dedupe.rs	42.35%	49 Missing ⚠️
aw-sync/src/main.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #718      +/-   ##
==========================================
+ Coverage   70.81%   79.71%   +8.89%     
==========================================
  Files          51       75      +24     
  Lines        2916     8325    +5409     
==========================================
+ Hits         2065     6636    +4571     
- Misses        851     1689     +838     

☔ 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.

[ErikBjare]

@TimeToBuildBob address

[TimeToBuildBob]

Addressed in 4c40f27:

1. Paged fetch: dedupe_bucket_paginated walks the bucket in PAGE_SIZE=5000 chunks (newest-first, with a boundary-safe same-timestamp pop so no duplicate group gets split across pages). A 1.5M-event bucket now runs in O(1) memory.

2. Progress + runtime: deletion logs "Deleting from {bucket}: N duplicate(s)..." at start and a line every 10k. Added --help note that large cleanups take a long time since each removal is an individual HTTP call.

3. README caveat: added a note that HTTP timestamps are ms-precision — two events with identical (timestamp-ms, duration, data) within the same millisecond would be collapsed. Practically impossible for real AW data, but documented before running on live instances.

Also fixed a test assertion bug caught during the test run: paginated_matches_single_shot_across_page_boundaries was asserting [10, 11] as duplicates, but since the lowest id is the keeper, duplicates are correctly [11, 12]. The inline comments had the keeper/dup labels swapped too — fixed both.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

🤖 AI code review

Safe to merge — 1 P1 disposed (fixed in de0c8a5)

_{Updated after inline dispositions on finding threads — this is the current state; the verdict below is frozen at review time and is kept as the historical record of that pass.}

Finding disposition

Finding	Severity	State
aw-sync/src/dedupe.rs:101	P1	superseded by latest review (not reproduced)
aw-sync/src/dedupe.rs:119	P1	fixed in de0c8a54
aw-sync/src/dedupe.rs:175	P2	fixed in de0c8a54

Adds a dedupe subcommand to aw-sync that scans -synced-from- buckets for exact-duplicate events (same timestamp, duration, data) and deletes all but the lowest-id copy, with --dry-run and --bucket options. The implementation is a new dedupe.rs module with bounded pagination and carry-across-boundary logic, plus unit tests; is_synced_bucket is made pub(crate) for reuse.

Not safe to merge — 1 P1 open

Confidence 3/5

⚠️ 1 of 4 changed files were not fully read. Claims that an identifier is never assigned, used, or called are discarded unless verification names only fully preloaded files. This coverage describes the bounded context preload; the diff was reviewed.

2 findings · ❌ 1 P1 · ⚠️ 1 P2

❌ P1 high — aw-sync/src/dedupe.rs:119

dedupe_bucket_paginated advances fetch_end to boundary_ts - 1ns after popping only the boundary-timestamp events that fit in the current page. If a duplicate run at boundary_ts is larger than the page's remaining capacity (e.g. a bucket with 4999 newer events and 550 duplicates at an older timestamp, PAGE_SIZE=5000), only the single duplicate that fits is carried to the next iteration; the other 549 duplicates are older than the new fetch_end and are never fetched. The carried subset then contains just one event, so find_duplicate_ids returns no duplicates and the command prints 'No duplicates found' while leaving all 549 duplicates untouched. A rerun reproduces the same page layout, so the duplicates are never cleaned. The all-same-timestamp branch has the same skip. Consequence: aw-sync dedupe can silently fail at its primary purpose and report a clean bucket.

_{How this was verified: Traced the loop with a concrete dataset: PAGE_SIZE=2, events at ts=39 dur=2 and ts=39 dur=0; the second fetch with end=39 clips the first to dur=0, making them identical. The clipping semantics are documented in sync.rs comments cited in the PR.}

⚠️ P2 medium — aw-sync/src/dedupe.rs:175

The else if !dry_run refusal in run_dedupe returns an error before the targets.is_empty() check is reached. As a result, aw-sync dedupe with no --bucket and no --dry-run always exits with 'Refusing to delete from every -synced-from- bucket...' even on a server that has no -synced-from- buckets at all. The intended no-op message at line 184 ('No -synced-from- buckets found, nothing to dedupe') is unreachable for this mode, so a fresh installation following the README's delete example gets a confusing error instead of a clean no-op. The refusal is appropriate only when there is actually something to delete, so the empty-target check should run first.

_{How this was verified: Traced run_dedupe's bucket loop (lines 189-216): both the fetch and the per-event delete use ? with no continue-on-error, and the summary writeln! statements (lines 218-250) are reachable only if every bucket succeeds.}

2 advisory findings (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium — aw-sync/src/dedupe.rs:43

find_duplicate_ids groups events by the exact tuple (timestamp, duration, data), but the pre-#713 state this PR targets did not accumulate exact copies. Per sync.rs lines 1000-1019, the old resume query clipped the boundary event's returned start forward to resume_sync_at and zeroed its duration, so a positive-duration boundary event (the PR's stopwatch example) left N copies of (resume_sync_at, 0, data) alongside the original (orig_ts, dur, data). Those two keys never compare equal, so dedupe collapses the N clipped copies to one but cannot merge that survivor with the original: the bucket ends with two events for one logical event, and the zero-length fragment still renders on the timeline. The report's duplicate count covers only the clipped-copy group, so the run looks complete while a genuine pre-#713 duplicate survives. Merging on end_time + data (the fingerprint #713 itself uses, sync.rs lines 1050-1055) would remove the fragment and match the worst accumulation the PR is meant to clean.

_{How this was verified: Checked dedupe.rs lines 42-46 (key = timestamp, duration, data) and sync.rs lines 1000-1019 and 1050-1055, which state the legacy re-import was start-clipped (start moved to resume_sync_at, duration zeroed) and that the robust fingerprint is (end_time, data).}

⚠️ P2 medium — aw-sync/src/dedupe.rs:204

If any client.delete_event call in the deletion loop fails, the ? at line 204 immediately returns from run_dedupe, skipping the output block at lines 218-250. The user is left with no per-bucket or total summary of what was already deleted before the failure, and because earlier deletions are not rolled back, a subsequent run cannot rely on any printed counts to know how many duplicates remain. For a cleanup tool that permanently removes data, a partial failure with no report makes it difficult to determine which bucket was interrupted and how many duplicates are left. The deletion loop should write the partial results to stdout (or at least include the error in the final message) before propagating the error, so the operator has a record of what was already done.

_{How this was verified: Read the function flow: the deletion loop is followed by the output block; an early ? return skips it. No other error handling or partial-report path exists between the loop and stdout.}

Files changed (4) — the diff as I read it

• aw-sync/README.md — Documents the dedupe subcommand, its --dry-run/--bucket usage, and the exact-match caveat.
• aw-sync/src/dedupe.rs — Adds dedupe_bucket_paginated, find_duplicate_ids, and run_dedupe along with unit tests.
• aw-sync/src/main.rs — Adds the Dedupe subcommand variant and dispatches to dedupe::run_dedupe.
• aw-sync/src/sync.rs — Changes is_synced_bucket from a private function to pub(crate).

Previous review passes

commit	score	findings	engine	when
4c40f27d6781	3/5	1	llm	2026-09-18 18:02 UTC

_{Reviewed 262e3569ee89 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 685s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

Convergence adjudication — 2026-09-18

Head: 262e3569ee8994375d311f343222a45c6e23b494

Fixed

• P1 Boundary Fetch Changes Durations (Greptile round 2, dedupe.rs:97): fixed in 262e3569 — pagination loop now carries boundary-timestamp events forward in a carry vec instead of re-fetching with end = boundary_ts - 1ns. Events with different durations at the same timestamp are no longer clipped to zero by the server. Test carry_preserves_duration_across_page_boundary covers this.
• P1 First-hand buckets can be deleted (Greptile round 1, dedupe.rs:154): fixed before round 2 — Greptile did not re-raise this in round 2 after 4c40f27.
• AI review P1 (pagination undercount) (dedupe.rs:101): same root cause as the boundary-fetch P1 above; resolved by the same 262e3569 carry-buffer fix.

Remaining (non-blocking, dismissed)

• P1 Large Timestamp Ties Are Skipped (dedupe.rs:131): acknowledged edge case. The else branch fires only when >5000 events share one exact nanosecond timestamp. Erik's real dry run showed a worst case of 550 copies — far under PAGE_SIZE=5000. A second run converges for any remainder. Acceptable limitation for a one-off cleanup tool. Thread resolved with explanation.
• P2 Timestamp Precision Warning Is Incorrect (README.md): false positive. Erik explicitly confirmed (comment 2026-09-18T16:31) that "over HTTP timestamps are ms-precision" — the HTTP layer does truncate to ms regardless of internal ns storage. Warning is correct. Thread resolved.
• P2 Whole buckets load into memory: addressed in 4c40f27 (paginated fetch, PAGE_SIZE=5000). Thread resolved.

CI

All checks green: Android ✅, macOS ✅, ubuntu ✅, windows ✅, clippy ✅, format ✅, Greptile CI check ✅, codecov ✅.

Domain risk

The dedupe command is destructive and irreversible — events deleted cannot be recovered without re-syncing from the source device. The --dry-run default guard is critical. Recommend maintainer manually verify the carry logic in dedupe_bucket_paginated handles the boundary-timestamp case correctly before merging, especially given the 519k duplicate count in Erik's real data.

Convergence

round_convergence.stable_rounds = 0 (two rounds, each with new blocking findings). A third Greptile review was triggered at 18:10 UTC for 262e3569 but has not posted inline findings yet. If round 3 finds no new P1s, this PR is merge-eligible from the convergence adjudication standpoint. All current threads are resolved.

Not requesting auto-merge — this is a destructive tool targeting real data; maintainer review of the final code is warranted.

[TimeToBuildBob]

Follow-up on "address" — the two asks (paged fetch, progress/runtime, README caveat) landed in 4c40f27, but re-reading the pagination against your real numbers I found my last adjudication was wrong, so here is the actual fix.

Correction. I called the pagination undercount resolved by the carry-buffer commit (262e356) and dismissed the tie case as ">5000 events at one nanosecond". Both wrong. Any duplicate run that straddles a page edge was cut: only the part inside the page was carried, and the next fetch started at boundary_ts - 1ns, so the rest of the run was never fetched. With runs of 425–550 copies on a 5000-event page, that is most page boundaries on your afk_* and android-test buckets — the dry run would have under-reported and a delete would have left copies behind (never deleting the wrong ones — the failure direction was safe, but incomplete).

Fix (de0c8a5). A page is now cut at the timestamp of its PAGE_SIZE-th event, and accepted only once the fetch also contains an event strictly older than that timestamp (proof the run is complete), or the bucket is exhausted; otherwise the limit doubles and the same window is re-fetched. Only events at/after the cut are processed and the next page starts at cut - 1ns. Runs are still never re-fetched via end = ts (that would clip durations to zero and merge distinct events). Runs larger than a page work too, so the ">PAGE_SIZE ties" limitation is gone. Common case is one fetch of PAGE_SIZE + 1; the extra fetch only happens when a run touches the cut.

Also: the "refusing to delete without --bucket" check moved after the empty-target check, so a fresh install with no synced buckets gets the clean no-op instead of the refusal.

Tests. The test fake previously sorted by id and never modelled the real straddle. It now follows the server contract (starttime DESC, endtime ASC, id ASC, limit, clip at end). New tests: run straddling a boundary, run larger than a page, empty bucket, and a 200-layout randomized comparison against the unbounded single-shot result. Mutating the fix away (no limit growth) turns the straddle/large-tie/random tests red. cargo test -p aw-sync 62+71+9+24+3 pass, cargo fmt --check and cargo clippy -p aw-sync -- -D warnings clean.

Still worth your eyes before the real delete run: it's destructive, and the durations of events crossing a page's end are clipped by the server (identically for all members of a group, so grouping is unaffected). Suggest running --dry-run again after this lands and comparing against the 519,313 you measured — that number should now match exactly what an unbounded fetch gives.