MusicBrainz manual search: field-scope the artist in non-strict mode (#754)

The user-facing Search-for-Match / Fix popup runs non-strict MB searches.
That path built a bare "track artist" query with no field scoping, so the
artist was just a free fuzzy term — covers and karaoke whose TITLES contained
the artist name outranked the canonical recording. Reproduced live: searching
"Say You Will" / Foreigner returned cover artists with Foreigner absent, and
"Sweet Child O Mine" / Guns N Roses returned only covers (Presnyakov, PMJ…),
never the Guns N' Roses original.

Keep the track/album side loose (no phrase quotes → diacritic + bracketed-
suffix recall, the reason non-strict exists) but field-scope the artist as
artist:(...) so it constrains. The artist value is Lucene-escaped via
_escape_lucene() — without it, names like "Sunn O)))" or "Anthony Green
(Saosin)" would close the artist:( group early (returning unrelated artists)
or break the query (zero results). Same fix applied to search_release.

Verified against the live MB API: both reporter queries now return the real
artist top-to-bottom; diacritic recall is preserved (artist:(Bjork) folds to
Björk); and paren/?/!-laden artist names produce valid, balanced queries.

Tests pin the constructed query string (no network): non-strict scopes and
escapes the artist while keeping the track loose/unquoted; strict path
unchanged; plus _escape_lucene unit coverage.

core/musicbrainz_client.py

@@ -7,6 +7,19 @@ from utils.logging_config import get_logger
 
 logger = get_logger("musicbrainz_client")
 
+# Lucene query-syntax characters that must be backslash-escaped when a
+# user-supplied value is interpolated into a query (e.g. artist names like
+# "Sunn O)))", "Anthony Green (Saosin)", "Therapy?", "!!!"). Without this an
+# unbalanced paren or a stray ?/* either breaks the field group (returning
+# unrelated results) or yields zero hits.
+_LUCENE_SPECIAL = set('+-&|!(){}[]^"~*?:\\/')
+
+
+def _escape_lucene(text: str) -> str:
+    """Backslash-escape Lucene special characters in a user-supplied term."""
+    return ''.join('\\' + ch if ch in _LUCENE_SPECIAL else ch for ch in text)
+
+
 # Global rate limiting variables
 _last_api_call_time = 0
 _api_call_lock = threading.Lock()
@@ -158,14 +171,14 @@ class MusicBrainzClient:
                     safe_artist = artist_name.replace('\\', '\\\\').replace('"', '\\"')
                     query += f' AND artist:"{safe_artist}"'
             else:
-                # Bare query — MB tokenizes against title + artist credit +
-                # alias + sortname indexes together with diacritic folding.
-                # Recovers cases like "Bjork" → "Björk" that strict phrase
-                # queries miss.
-                parts = [album_name]
-                if artist_name:
-                    parts.append(artist_name)
-                query = ' '.join(p for p in parts if p)
+                # Loose title terms (no phrase quotes → diacritic folding and
+                # alias/sortname recall, e.g. "Bjork" → "Björk"), but
+                # FIELD-SCOPE the artist so it constrains rather than floating
+                # as a free fuzzy term that lets unrelated releases whose titles
+                # echo the artist name rank first (same root cause as #754).
+                query = album_name
+                if artist_name and artist_name.strip():
+                    query += f' AND artist:({_escape_lucene(artist_name.strip())})'
 
             params = {
                 'query': query,
@@ -223,18 +236,24 @@ class MusicBrainzClient:
                     safe_artist = artist_name.replace('\\', '\\\\').replace('"', '\\"')
                     query += f' AND artist:"{safe_artist}"'
             else:
-                # Bare query — see search_release for rationale.
-                parts = [track_name]
-                if artist_name:
-                    parts.append(artist_name)
-                query = ' '.join(p for p in parts if p)
+                # Loose track terms (no phrase quotes → tolerant of bracketed
+                # suffixes and diacritics, hitting MB's alias/sortname indexes),
+                # but FIELD-SCOPE the artist so it actually constrains results.
+                # A bare "track artist" blob let the artist float as a free
+                # fuzzy term, so covers/karaoke whose TITLES contain the artist
+                # name outranked the real recording (#754: "Sweet Child O Mine"
+                # / "Guns N Roses" returned only covers). Scoping still folds
+                # diacritics — artist:(Bjork) matches "Björk".
+                query = track_name
+                if artist_name and artist_name.strip():
+                    query += f' AND artist:({_escape_lucene(artist_name.strip())})'
 
             params = {
                 'query': query,
                 'fmt': 'json',
                 'limit': limit
             }
-            
+
             response = self.session.get(
                 f"{self.BASE_URL}/recording",
                 params=params,

tests/metadata/test_musicbrainz_client_query.py

@@ -0,0 +1,122 @@
+"""Query-construction tests for core/musicbrainz_client.py.
+
+Regression guard for #754: the user-facing "Search for Match" / Fix popup
+runs non-strict (strict=False) MusicBrainz searches. The old non-strict path
+built a bare "track artist" blob with NO field scoping, so the artist was just
+a free fuzzy term — covers/karaoke whose TITLES contained the artist name
+outranked the real recording (e.g. "Sweet Child O Mine" / "Guns N Roses"
+returned only covers, never the Guns N' Roses original).
+
+The fix keeps the track/album side loose (diacritic + bracket recall) but
+field-scopes the artist (artist:(...)) so it actually constrains. These tests
+pin the query STRING the client sends — no network — by capturing the params
+passed to session.get.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from core.musicbrainz_client import MusicBrainzClient, _escape_lucene
+
+
+@pytest.fixture
+def client():
+    c = MusicBrainzClient("SoulSync", "2")
+    # Replace the HTTP session with a mock returning an empty result set, so
+    # we can inspect the query string without touching the network.
+    resp = MagicMock()
+    resp.raise_for_status = MagicMock()
+    resp.json = MagicMock(return_value={"recordings": [], "releases": []})
+    c.session = MagicMock()
+    c.session.get = MagicMock(return_value=resp)
+    return c
+
+
+def _query_of(client) -> str:
+    """The 'query' param of the last session.get call."""
+    _, kwargs = client.session.get.call_args
+    return kwargs["params"]["query"]
+
+
+# ── recording: non-strict must field-scope the artist ──────────────────────
+
+def test_recording_nonstrict_scopes_artist(client):
+    client.search_recording("Sweet Child O Mine", artist_name="Guns N Roses", strict=False)
+    q = _query_of(client)
+    assert "artist:(Guns N Roses)" in q          # artist is a CONSTRAINT, not a loose term
+    assert q.startswith("Sweet Child O Mine")    # track side stays loose (no phrase quotes)
+    assert '"' not in q                          # no phrase-quoting that kills bracket/diacritic recall
+
+
+def test_recording_nonstrict_without_artist_is_bare_track(client):
+    client.search_recording("Hyperballad", strict=False)
+    assert _query_of(client) == "Hyperballad"    # no artist → no AND clause
+
+
+def test_recording_nonstrict_whitespace_artist_is_bare_track(client):
+    # A whitespace-only artist must not produce a malformed artist:(   ) group.
+    client.search_recording("Hyperballad", artist_name="   ", strict=False)
+    assert _query_of(client) == "Hyperballad"
+
+
+def test_recording_nonstrict_strips_artist_padding(client):
+    client.search_recording("Money", artist_name="  Pink Floyd  ", strict=False)
+    assert _query_of(client) == "Money AND artist:(Pink Floyd)"
+
+
+def test_recording_strict_unchanged(client):
+    client.search_recording("Say You Will", artist_name="Foreigner", strict=True)
+    q = _query_of(client)
+    assert q == 'recording:"Say You Will" AND artist:"Foreigner"'  # strict path untouched
+
+
+def test_recording_nonstrict_escapes_lucene_specials_in_artist(client):
+    # Artist names with parens/?/! must NOT break the artist:(...) group.
+    # Without escaping, "Sunn O)))" closes the group early and returns
+    # unrelated artists; "Anthony Green (Saosin)" returns nothing.
+    client.search_recording("Hunting Season", artist_name="Sunn O)))", strict=False)
+    q = _query_of(client)
+    assert "artist:(Sunn O\\)\\)\\))" in q        # every ) escaped, group stays balanced
+    # The clause is well-formed: opening "artist:(" is closed by exactly one
+    # unescaped ")", so paren depth returns to zero.
+    depth = 0
+    after = q.split("artist:(", 1)[1]
+    i = 0
+    while i < len(after):
+        ch = after[i]
+        if ch == "\\":
+            i += 2
+            continue
+        if ch == "(":
+            depth += 1
+        elif ch == ")":
+            if depth == 0:
+                break          # this is the closing paren of artist:(
+            depth -= 1
+        i += 1
+    assert depth == 0
+
+
+def test_escape_lucene_helper():
+    assert _escape_lucene("Sunn O)))") == "Sunn O\\)\\)\\)"
+    assert _escape_lucene("Therapy?") == "Therapy\\?"
+    assert _escape_lucene("AC/DC") == "AC\\/DC"
+    assert _escape_lucene("Foreigner") == "Foreigner"   # plain text untouched
+
+
+# ── release: same fix, same guarantees ─────────────────────────────────────
+
+def test_release_nonstrict_scopes_artist(client):
+    client.search_release("Nevermind", artist_name="Nirvana", strict=False)
+    q = _query_of(client)
+    assert "artist:(Nirvana)" in q
+    assert q.startswith("Nevermind")
+    assert '"' not in q
+
+
+def test_release_strict_unchanged(client):
+    client.search_release("Nevermind", artist_name="Nirvana", strict=True)
+    assert _query_of(client) == 'release:"Nevermind" AND artist:"Nirvana"'