feat: detect within-source duplicate likes - #8
Merged
Conversation
ytmusic likes accumulate within-source duplicates over time (same
video_id appears multiple times in the same snapshot). Confirmed real
example: F6J7-hVK-o8 duplicated in every ytmusic snapshot since 0.2.
YouTube side dedupes on its own; ytmusic does not. Until now these
leaked into ytmusic_only / possibly_missing_from_ytmusic buckets,
inflating those counts and (worse) making sync act on the surplus as
"missing from ytmusic" rows.
Scope is detection + isolation. Actual dedupe ships as a separate
0.5 command (`dedupe ytmusic`).
Design:
- New `duplicate_in_source` issue_type with `confidence=1.0` (count is a
fact, not a probability). One DiagnosisItem per duplicated video_id;
reason renders "appears N times in {source} snapshot (positions: ...)"
with positions sorted ascending and source_track_id deterministically
taken from the lowest-position row.
- New `dedupe_by_video_id` in compare.py, typed against a small Protocol
so the module stays decoupled from SQLAlchemy ORM. Sort by position,
group by video_id, keep first per group; rows with video_id IS NULL
preserved as-is. Idempotent.
- `_compare_and_persist` runs detection in Stage A0 over raw items
(returns groups), canonicalizes via dedupe_by_video_id for downstream
use, then persists duplicate_in_source rows AFTER create_diagnosis()
returns (because the builder needs diagnosis.id).
- Drift loop applies dedupe_by_video_id to both prev and curr items
inside the loop — drift re-fetches from DB so rebinding outer vars
doesn't propagate.
- compare-likes summary semantics: raw totals stay raw (user's actual
like count, dups included); matched / missing / ytmusic_only buckets
reflect canonicalized comparison. The new "Duplicate likes
(within-source)" row tells the user how many were collapsed.
- doctor health rate: denominator uses canonicalized music-candidate
count to match the canonicalized numerator. Otherwise a duplicated
unmatched YouTube row would falsely show 50% match (Codex P2 catch).
- sync.plan(): explicit informational branch for {ytmusic_only,
metadata_drift, duplicate_in_source} replaces today's silent
fallthrough. Unknown future types still fall through silently
(no new WARN channel — sync.plan stays pure).
Tests: +15 (4 builder, 3 dedupe helper, 5 CLI integration covering
both ytmusic-side and youtube-side duplicates plus raw-count
semantics, 1 sync ignore, 1 doctor denominator regression). All
221 → 222 passing; ruff format-check + lint clean.
Plan, plan-review (2 rounds), implementation, and code-review all via
hyperclaude. Plan at .hyperclaude/plans/duplicate-in-source.md.
There was a problem hiding this comment.
Pull request overview
This PR adds detection for within-source duplicate likes (same video_id appearing multiple times in a single snapshot), and updates the compare/sync/doctor pipeline to canonicalize items by video_id so duplicates don’t leak into cross-source mismatch buckets or mislead health metrics.
Changes:
- Introduces a new finding type
duplicate_in_sourceand persists it asDiagnosisItemrows. - Adds
dedupe_by_video_id()and uses it to canonicalize snapshot items before cross-source matching (and in drift/doctor denominators). - Updates
sync.plan()to explicitly treatduplicate_in_sourceas informational (no action/skip), and expands CLI/docs/tests accordingly.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Bumps editable package version metadata to 0.4.0. |
| tests/test_sync.py | Adds planner regression test ensuring duplicate_in_source is ignored while actionable items still plan. |
| tests/test_doctor.py | Verifies duplicate_in_source aggregation and fixes match-rate denominator regression via canonicalization. |
| tests/test_diagnosis.py | Adds unit tests for build_duplicate_in_source_items behavior and determinism. |
| tests/test_compare.py | Adds tests for dedupe_by_video_id() contract and idempotency. |
| tests/test_cli.py | Adds end-to-end CLI persistence tests covering duplicate detection + canonicalization semantics. |
| src/likesurgeon/sync.py | Explicitly ignores informational issue types including duplicate_in_source. |
| src/likesurgeon/models.py | Documents the new duplicate_in_source issue type in DiagnosisItem. |
| src/likesurgeon/doctor.py | Includes duplicate_in_source in summary and canonicalizes YouTube denominator for match-rate. |
| src/likesurgeon/diagnosis.py | Adds ISSUE_DUPLICATE_IN_SOURCE and builder to generate duplicate findings from raw snapshot rows. |
| src/likesurgeon/compare.py | Introduces dedupe_by_video_id() helper with a small Protocol contract. |
| src/likesurgeon/cli.py | Updates compare pipeline to (a) detect duplicates on raw rows, (b) match on canonicalized rows, (c) display raw totals and a new duplicate bucket. |
| README.md | Documents duplicate_in_source as informational and non-actionable in 0.4. |
| docs/ARCHITECTURE.md | Updates schema documentation to include the new issue type. |
Comments suppressed due to low confidence (1)
src/likesurgeon/cli.py:489
_compare_and_persistdocstring still refers to the "0.3 compare-likes pipeline", but this PR introduces new 0.4 pipeline stages (within-source duplicate detection + canonicalization) and bumps the package to 0.4. Consider updating the version reference in the docstring so it doesn't become stale/misleading.
def _compare_and_persist(session: Session) -> _PipelineResult:
"""Run the full 0.3 compare-likes pipeline against the current session
and return the persisted Diagnosis id plus the finding counts the CLI
summary needs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+195
to
+196
| ascending and ``source_track_id`` resolves to the position-1 row's | ||
| ``track_id`` so output is deterministic regardless of caller input order. |
Comment on lines
14
to
16
| from . import __version__ | ||
| from .compare import CompareInput, CompareResult, compare_likes | ||
| from .compare import CompareInput, CompareResult, compare_likes, dedupe_by_video_id | ||
| from .config import Config, InvalidRegionError, _validate_region |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
duplicate_in_sourcefinding for within-source duplicate likes (samevideo_idappearing multiple times in one snapshot — common on the ytmusic side).video_idbefore cross-source matching, so duplicates no longer leak intoytmusic_only/possibly_missing_from_ytmusicbuckets. This also prevents sync from acting on the surplus as "missing".dedupe ytmusiccommand in 0.5.What changed
duplicate_in_source(new)compare-likesbucket,issues --type,doctorsummaryPipeline
_compare_and_persistnow:dedupe_by_video_idto rebindyt_items/ytm_itemsfor downstream stages.prev_items/curr_itemssince it doesn't see the outer rebind).create_diagnosis()returns, callbuild_duplicate_in_source_items(diagnosis.id, raw_items, source)and persist.Notable design notes
video_idis counted once.compare.pystays decoupled from SQLAlchemy:dedupe_by_video_idis typed against a small Protocol (video_id: str | None,position: int) so existing fake-item tests stay natural.sync.plan(): explicitelif item.issue_type in {ISSUE_YTMUSIC_ONLY, ISSUE_METADATA_DRIFT, ISSUE_DUPLICATE_IN_SOURCE}: continuebranch replaces silent fallthrough for known informational types. Unknown future types still fall through silently.Process
Followed hyperclaude flow end-to-end on this change:
hyperclaude:planner→/hyperclaude:hyper-plan-review(2 rounds — v1 "send back to design", v2 "ship after fixes") →hyperclaude:implementer→/hyperclaude:hyper-code-review(caught the doctor P2). Plan saved at.hyperclaude/plans/duplicate-in-source.md(gitignored).Test plan
uv run pytest tests/— 221 → 222 passed (+15 new tests acrosstest_diagnosis.py,test_compare.py,test_cli.py,test_sync.py,test_doctor.py)uv run ruff format --check .cleanuv run ruff checkcleanlikesurgeon compare-likesagainst the real account; confirm the new bucket shows the expected count (≥1 from F6J7-hVK-o8 dup) and the dup no longer double-counts inytmusic_only