fix: address code review findings

- Strip standalone reissue keywords (remaster/remastered/remix/deluxe/
  expanded/edition/reissue/anniversary/bonus) regardless of brackets so
  non-parenthesized remasters still match the plain local title above the
  0.85 threshold (was falsely reported missing).
- Fix TestAppRun_ScanLogsMissingReleases to verify run() performs the
  scan itself (capture its log output) instead of re-running ScanAll
  independently, which passed even if run() were a no-op.
- Fix TestSimilarity misleading cases that downgraded to a <0.85 range
  check with dead want/epsilon fields; assert actual computed scores.
This commit is contained in:
2026-07-19 21:33:52 +03:00
parent e2de91c5d3
commit 79a376a127
4 changed files with 44 additions and 27 deletions

View File

@@ -1,15 +1,17 @@
package main package main
import ( import (
"bytes"
"context" "context"
"log"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"time" "time"
"naviwatcher/internal/config" "naviwatcher/internal/config"
"naviwatcher/internal/database" "naviwatcher/internal/database"
"naviwatcher/internal/scanner"
) )
func TestAppRun_ScanLogsMissingReleases(t *testing.T) { func TestAppRun_ScanLogsMissingReleases(t *testing.T) {
@@ -54,6 +56,13 @@ func TestAppRun_ScanLogsMissingReleases(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
defer cancel() defer cancel()
// Capture run()'s log output so we assert that run() ITSELF performed
// the scan (not a separately re-run ScanAll). This guards against the
// hook silently becoming a no-op while still passing.
var buf bytes.Buffer
log.SetOutput(&buf)
defer log.SetOutput(os.Stderr)
// Run the (blocking) hook in a goroutine; cancel after it has had time to // Run the (blocking) hook in a goroutine; cancel after it has had time to
// perform the scan so run() returns nil via the ctx.Done() path. // perform the scan so run() returns nil via the ctx.Done() path.
done := make(chan error, 1) done := make(chan error, 1)
@@ -66,15 +75,10 @@ func TestAppRun_ScanLogsMissingReleases(t *testing.T) {
t.Fatalf("app.run() returned error: %v", err) t.Fatalf("app.run() returned error: %v", err)
} }
// The scan should have found the missing release (Animals) for artist-1. // run() must have logged the missing release (Animals) for artist-1.
// Use a fresh context for the verification scan since the run context was out := buf.String()
// cancelled above. if !strings.Contains(out, "missing: artist=artist-1") || !strings.Contains(out, "Animals") {
missing, err := scanner.ScanAll(context.Background(), db, 0.85) t.Fatalf("app.run() did not log the expected missing release; log output:\n%s", out)
if err != nil {
t.Fatalf("ScanAll() error: %v", err)
}
if len(missing) != 1 || missing[0].RGID != "rg2" {
t.Fatalf("expected 1 missing release (rg2/Animals), got %+v", missing)
} }
} }

View File

@@ -17,6 +17,12 @@ var (
bracketRe = regexp.MustCompile(`\[[^\]]*\]`) bracketRe = regexp.MustCompile(`\[[^\]]*\]`)
parenRe = regexp.MustCompile(`\([^)]*\)`) parenRe = regexp.MustCompile(`\([^)]*\)`)
yearRe = regexp.MustCompile(`\b[0-9]{4}\b`) yearRe = regexp.MustCompile(`\b[0-9]{4}\b`)
// keywordRe strips common reissue/edition keywords that appear WITHOUT
// brackets or parentheses (e.g. "The Wall 2011 Remaster", "Album 2020
// Remastered", "X Deluxe"). MusicBrainz release-group titles frequently
// carry these as free-standing words; they must be removed so a remaster
// still matches the plain local title above the fuzzy threshold.
keywordRe = regexp.MustCompile(`(?i)\b(remaster|remastered|remix|deluxe|expanded|edition|reissue|anniversary|bonus)\b`)
spaceRe = regexp.MustCompile(`\s+`) spaceRe = regexp.MustCompile(`\s+`)
// bareYearRe matches a title that is *only* a single year (with optional // bareYearRe matches a title that is *only* a single year (with optional
// surrounding whitespace), e.g. "1989" or "2112". Used to decide whether a // surrounding whitespace), e.g. "1989" or "2112". Used to decide whether a
@@ -46,6 +52,12 @@ func NormalizeString(s string) string {
// Remove parenthesized content (e.g., (Deluxe), (Remastered)) // Remove parenthesized content (e.g., (Deluxe), (Remastered))
s = parenRe.ReplaceAllString(s, "") s = parenRe.ReplaceAllString(s, "")
// Remove standalone reissue/edition keywords (e.g. "2011 Remaster",
// "2020 Remastered", "Deluxe"). These appear without brackets/parens
// in many MusicBrainz titles and must be stripped so a remaster still
// matches the plain local title above the fuzzy threshold.
s = keywordRe.ReplaceAllString(s, "")
// Remove years (any 4-digit number). If stripping the year // Remove years (any 4-digit number). If stripping the year
// empties the entire string, decide what to keep: // empties the entire string, decide what to keep:
// - A bare year title (e.g. "1989", "2112") has no other words, so keep // - A bare year title (e.g. "1989", "2112") has no other words, so keep

View File

@@ -22,7 +22,12 @@ func TestNormalizeString_Basic(t *testing.T) {
{"Album (Remastered)", "album"}, {"Album (Remastered)", "album"},
// Year removal // Year removal
{"Dark Side of the Moon 1973", "dark side of the moon"}, {"Dark Side of the Moon 1973", "dark side of the moon"},
{"Album 2020 Remastered", "album remastered"}, // Standalone reissue keywords (no brackets/parens) are stripped
{"Album 2020 Remastered", "album"},
{"The Wall 2011 Remaster", "the wall"},
{"X Deluxe", "x"},
{"Y Expanded Edition", "y"},
{"Z Remix", "z"},
// Space collapsing // Space collapsing
{"Dark Side of the Moon", "dark side of the moon"}, {"Dark Side of the Moon", "dark side of the moon"},
// Trim // Trim

View File

@@ -44,34 +44,30 @@ func TestSimilarity(t *testing.T) {
epsilon: 1e-9, epsilon: 1e-9,
}, },
{ {
name: "clearly different titles score below 0.85", name: "clearly different titles score low",
a: "The Wall", a: "The Wall",
b: "Completely Different Album", b: "Completely Different Album",
want: 0.0, want: 0.1538,
epsilon: 1e-9, epsilon: 1e-3,
}, },
{ {
name: "substring-ish title scores moderately", name: "substring-ish title scores moderately below threshold",
a: "Dark Side of the Moon", a: "Dark Side of the Moon",
b: "Dark Side of the Moon Part II", b: "Dark Side of the Moon Part II",
want: 0.0, // non-empty; value asserted only as below threshold want: 0.7241,
epsilon: 1e-9, epsilon: 1e-3,
}, },
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
got := Similarity(tt.a, tt.b) got := Similarity(tt.a, tt.b)
switch { if diff := got - tt.want; diff > tt.epsilon || diff < -tt.epsilon {
case tt.name == "clearly different titles score below 0.85" || t.Errorf("Similarity(%q, %q) = %v, want %v (+/- %v)", tt.a, tt.b, got, tt.want, tt.epsilon)
tt.name == "substring-ish title scores moderately": }
if got >= 0.85 { // Sanity: anything at/above the default threshold must be a match.
t.Errorf("Similarity(%q, %q) = %v, want < 0.85", tt.a, tt.b, got) if got >= 0.85 && !IsMatch(tt.a, tt.b, 0) {
} t.Errorf("Similarity(%q, %q) = %v >= 0.85 but IsMatch(...,0) is false", tt.a, tt.b, got)
default:
if diff := got - tt.want; diff > tt.epsilon || diff < -tt.epsilon {
t.Errorf("Similarity(%q, %q) = %v, want %v (+/- %v)", tt.a, tt.b, got, tt.want, tt.epsilon)
}
} }
}) })
} }