feat(core): flag near-duplicate phrases via novelty (#76, part 3) - #85
Conversation
Failing tests for flag_phrase_duplicates: a later phrase that verbatim-quotes (transposition-aware) an earlier one past a share threshold is flagged a near-duplicate of it — the playtest's chorus/verse repeats. Includes a track-isolation case (compare the detected track on both sides). The API does not exist yet, so the suite fails to compile until green.
flag_phrase_duplicates compares each phrase's detected-track line against the earlier ones via measure_novelty (transposition-/resolution-aware) and flags a later phrase as a near-duplicate of phrase N when its longest verbatim quote covers >= PHRASE_DUPLICATE_SHARE (0.8) of its notes. Both sides are reduced to the detected track so references resolve to the same line. The web split emits duplicate_of / duplicate_share per phrase; the pager shows "≈ phrase N (X% quote)" in splitInfo so the curator can compare (#78) and decide — non-destructive (the issue's "flag", not "drop"). On the DGD split (11 phrases) it flags 6 repeats: p6≈p2, p7≈p3, p8≈p4, p9≈p5 (100%), p10≈p1 (93%), p11≈p2 (89%) — the song's verse/chorus repeats. core 19/19 novelty (3 new, incl. a track-isolation case); web split 5/5; JS 23/23; clippy clean; e2e confirms the flags. CLI/metadata flagging is an optional follow-up.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds phrase-level near-duplicate detection to the core novelty module via ChangesPhrase Near-Duplicate Detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/tests/novelty.rs`:
- Around line 427-440: The test
flag_phrase_duplicates_compares_the_detected_track_on_both_sides uses the same
decoy track in both phrases at position 0, so it will pass even if the duplicate
detection code incorrectly compares track 0 instead of track 1. To properly
validate track isolation, replace the decoy track in the second phrase with a
different decoy track (having different pitches than the first phrase's decoy)
while keeping the same REF_PITCHES at track 1. This ensures the test will fail
if the code mistakenly compares track 0 instead of the intended track 1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 31137ee7-40d1-47b1-b209-849ec3f3ee7c
📒 Files selected for processing (4)
core/src/novelty.rscore/tests/novelty.rsweb/src/lib.rsweb/static/app.js
Per review: the two phrases shared one track-0 decoy, so a consistently wrong-track comparison would still see identical track-0 lines and flag — the test passed for the wrong reason. Give each phrase a different track-0 decoy so a wrong-track comparison no longer matches; only the repeated track-1 line does. Also assert the verbatim quote_share. Behavior unchanged; test now actually guards single_track_line.
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Part 3 (final) of #76. After the over-long cap (#83) and trivial-drop (#84), this flags near-duplicate phrases — the playtest's "chorus/verse/filler repeats (p8≈p4, p9≈p5…)" — using the existing transposition-aware
measure_novelty. Per the design call: flag, don't drop (non-destructive; the curator decides, aided by the #78 compare overlay).What
core/src/novelty.rs: newflag_phrase_duplicates(phrases, track_index, min_quote_share) -> Vec<Option<PhraseDuplicate>>. For phrase i it compares against phrases0..iviameasure_noveltyand flags it as≈ phrase Nwhen the longest verbatim (transposition-/resolution-aware) quote covers ≥min_quote_shareof its notes. First occurrence is canonical; only later repeats are flagged. Both sides are reduced to the detected track (single_track_line) so a reference resolves to the same musical line even if an earlier track also sounds.PHRASE_DUPLICATE_SHARE = 0.8default (tunable).web/src/lib.rs: the split emitsduplicate_of/duplicate_shareper phrase.app.js: the pager shows· ≈ phrase N (X% quote)in splitInfo, pointing the curator at which phrase to compare against.Approach
Red → green (core algorithm):
de8eaf8adds failing tests (compile-fail, the repo's TDD idiom for a new API);1840d89implements it and wires the web path.Verification
corenovelty 19/19 (3 new: a later repeat flagged≈the first at ~100%; distinct phrases unflagged; a track-isolation case proving the detected track is compared on both sides — it fails withoutsingle_track_line). Fullcoresuite green; web split 5/5; JS 23/23;clippy -D warningsclean.This completes #76's three parts (over-long / trivial / dedup). Flagging in the CLI chunk metadata (for the corpus pipeline) is a clean optional follow-up.
🤖 Generated with Claude Code
https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Generated by Claude Code
Summary by CodeRabbit