Emergent:Blu/Patches/sideman-fixes

From PickiPedia: A knowledge base of bluegrass, old time psychedelic jams, and other public domain music
Jump to navigationJump to search

Patch for pickipedia:blu/sideman-fixes: one commit, re's three follow-ups from her review of #165 (job titles anywhere in a role, medley sub-tracks, paging past 100 recordings) plus the 'track None' cosmetic. Made with git format-patch origin/production at block 26,343,078, on production at 48ed2cc. sha256 of the file: 565a566a0b935f23026685485b33755069debda03eba589b29f29e93d70c3dc8. 61 tests pass (recording_credits + rambler). tools/ only, nothing to redeploy. For magent to git am and open against production.

From b2b8e5c40b3702fbd381e6561edc231251225f8f Mon Sep 17 00:00:00 2001
From: blu <blu@cryptograss.live>
Date: Wed, 4 Nov 2026 17:57:12 +0000
Subject: [PATCH] Sideman: catch job titles anywhere in a role, cover medley
 sub-tracks, page past 100 recordings

Three follow-ups from re's review of #165:

- A Discogs role is judged by whole words anywhere in it, not by what it
  starts with. 'Assistant Engineer' and 'Guitar Technician' are jobs, not
  players; the first slipped past because it does not start with
  'engineer', the second because it starts with an instrument. 'technician',
  'tech', 'assistant' and 'roadie' join the list.
- A medley's sub-tracks ('2.1', '2.2') are searched for the title, and a
  credit on track '2' covers them. Before, a track-level credit on a
  sub-track was dropped without a word.
- find_recordings pages through MusicBrainz results by offset until the
  count is reached (ceiling 1000) instead of stopping at the first 100.
- render says 'track not found' instead of 'track None'.

14 new tests; 61 pass across Sideman and Rambler.
---
 tools/recording_credits.py      | 68 ++++++++++++++++++++++++---------
 tools/test_recording_credits.py | 60 +++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+), 17 deletions(-)

diff --git a/tools/recording_credits.py b/tools/recording_credits.py
index 730dd77..af19618 100644
--- a/tools/recording_credits.py
+++ b/tools/recording_credits.py
@@ -60,10 +60,15 @@ def _norm_role(text):
 # roles are free text, so this list is the dumb part: a role not on it is
 # shown as a performance. Extend it when a new one turns up; the entries are
 # normalized the same way a role is, so write them however reads best.
+#
+# An entry matches anywhere in a role, as whole words, not only at its start:
+# a job title usually ends with the job and starts with a qualifier, so
+# 'Assistant Engineer' and 'Guitar Technician' are both jobs, not players.
 DISCOGS_OTHER_ROLES = tuple(_norm_role(r) for r in (
     "producer", "executive producer", "co producer", "associate producer",
     "mixed by", "mixing", "mastered by", "mastering", "recorded by",
-    "engineer", "liner notes", "sleeve notes", "song notes", "notes",
+    "engineer", "technician", "tech", "assistant", "roadie",
+    "liner notes", "sleeve notes", "song notes", "notes",
     "design", "layout", "graphic design", "art direction", "artwork",
     "illustration", "cover", "photography", "photography by", "photographer",
     "written by", "songwriter", "composed by", "lyrics by", "music by",
@@ -124,16 +129,30 @@ def credit_line(artist_credit):
     return "".join(a["name"] + a.get("joinphrase", "") for a in artist_credit)
 
 
+MB_PAGE = 100
+MB_MAX_RESULTS = 1000  # "Shady Grove" territory: enough, and a ceiling on calls
+
+
 def find_recordings(artist, title):
-    """Every MusicBrainz recording of title whose artist credit mentions artist."""
+    """Every MusicBrainz recording of title whose artist credit mentions artist.
+
+    MusicBrainz answers at most 100 per call; a title with more recordings
+    than that is paged through by offset until its count is reached.
+    """
     query = f'recording:"{title}" AND artist:"{artist}"'
-    data = mb_get("recording/", query=query, limit=100)
     want = normalize_title(title)
-    return [
-        r for r in data.get("recordings", [])
-        if normalize_title(r["title"]) == want
-        and artist.lower() in credit_line(r["artist-credit"]).lower()
-    ]
+    found, offset = [], 0
+    while True:
+        data = mb_get("recording/", query=query, limit=MB_PAGE, offset=offset)
+        page = data.get("recordings", [])
+        found += [
+            r for r in page
+            if normalize_title(r["title"]) == want
+            and artist.lower() in credit_line(r["artist-credit"]).lower()
+        ]
+        offset += len(page)
+        if not page or offset >= min(data.get("count", 0), MB_MAX_RESULTS):
+            return found
 
 
 def pick_releases(recordings):
@@ -190,20 +209,34 @@ def discogs_id(release):
 
 
 def track_in_spec(position, spec):
-    """Whether a Discogs 'tracks' spec such as '1, 3 to 5' or 'A2' covers position."""
+    """Whether a Discogs 'tracks' spec such as '1, 3 to 5' or 'A2' covers position.
+
+    A sub-track of a medley is positioned '2.1'; a credit on track '2' covers it.
+    """
+    positions = [position]
+    if "." in position:
+        positions.append(position.split(".", 1)[0])
     for part in (p.strip() for p in spec.split(",")):
         if " to " in part:
             lo, hi = (x.strip() for x in part.split(" to ", 1))
-            if lo.isdigit() and hi.isdigit() and position.isdigit():
-                if int(lo) <= int(position) <= int(hi):
+            for pos in positions:
+                if lo.isdigit() and hi.isdigit() and pos.isdigit():
+                    if int(lo) <= int(pos) <= int(hi):
+                        return True
+                elif part == pos:
                     return True
-            elif part == position:
-                return True
-        elif part == position:
+        elif part in positions:
             return True
     return False
 
 
+def discogs_tracks(release):
+    """Every track on a Discogs release, sub-tracks of medleys included."""
+    for t in release.get("tracklist", []):
+        yield t
+        yield from t.get("sub_tracks", [])
+
+
 def performance_roles(role):
     """The parts of a Discogs role that mean performing.
 
@@ -215,7 +248,7 @@ def performance_roles(role):
     parts = []
     for part in re.split(r"[,/]", role):
         norm = _norm_role(part)
-        if norm and not any(norm.startswith(other) for other in DISCOGS_OTHER_ROLES):
+        if norm and not any(re.search(rf"\b{re.escape(other)}\b", norm) for other in DISCOGS_OTHER_ROLES):
             parts.append(part.strip())
     return parts
 
@@ -227,7 +260,7 @@ def is_performance(role):
 def discogs_credits(release, title):
     """Performance credits from a Discogs release for the track titled title."""
     want = normalize_title(title)
-    track = next((t for t in release.get("tracklist", []) if normalize_title(t["title"]) == want), None)
+    track = next((t for t in discogs_tracks(release) if normalize_title(t["title"]) == want), None)
     credits = []
     position = track["position"] if track else None
     for e in release.get("extraartists", []):
@@ -285,7 +318,8 @@ def render(artist, title, versions):
         lines.append("MusicBrainz has no recording with this title and artist.")
     for v in versions:
         lines.append(f'{v["artist"]}: {v["release"]} ({v["date"] or "undated"}), '
-                     f'{"; ".join(v["labels"]) or "no label"}, track {v["track"]}')
+                     f'{"; ".join(v["labels"]) or "no label"}, '
+                     f'{"track " + str(v["track"]) if v["track"] is not None else "track not found"}')
         for scope, heading in (("track", "Credited on this track"),
                                ("release", "Credited on the whole release (check liner notes)")):
             rows = [c for c in v["credits"] if c["scope"] == scope]
diff --git a/tools/test_recording_credits.py b/tools/test_recording_credits.py
index 05f2f34..871e2b6 100644
--- a/tools/test_recording_credits.py
+++ b/tools/test_recording_credits.py
@@ -174,11 +174,71 @@ def test_render_separates_track_from_release():
     ("Project Manager [Project Coordination]", []),
     ("Liner Notes [Song Notes]", []),
     ("Written-By", []),
+    # A job title ends with the job and starts with a qualifier. Matching only
+    # the start of the role let 'Assistant Engineer' through (it doesn't start
+    # with 'engineer') and 'Guitar Technician' (it starts with an instrument).
+    ("Assistant Engineer", []),
+    ("Production Assistant", []),
+    ("Guitar Technician", []),
+    ("Banjo Tech", []),
+    ("Guitar, Guitar Technician", ["Guitar"]),
+    ("Engineer [Assistant]", []),
 ])
 def test_composite_roles(role, performing):
     assert rc.performance_roles(role) == performing
 
 
+# ---------------------------------------------------------------------------
+# Sub-tracks and paging
+# ---------------------------------------------------------------------------
+
+def test_discogs_subtrack_is_covered_by_its_parent_track():
+    """A medley's parts are positioned '2.1', '2.2'; a credit on '2' covers them."""
+    release = {
+        "extraartists": [{"name": "Guest Player", "role": "Dobro", "tracks": "2"},
+                         {"name": "Other Guest", "role": "Fiddle", "tracks": "3"}],
+        "tracklist": [
+            {"position": "1", "title": "Old Train", "extraartists": []},
+            {"position": "2", "title": "Medley", "type_": "index", "extraartists": [],
+             "sub_tracks": [{"position": "2.1", "title": "House Of The Rising Sun", "extraartists": []},
+                            {"position": "2.2", "title": "Shady Grove", "extraartists": []}]},
+        ],
+    }
+    track, credits = rc.discogs_credits(release, "House of the Rising Sun")
+    assert track["position"] == "2.1"
+    assert [(c["name"], c["scope"]) for c in credits] == [("Guest Player", "track")]
+
+
+@pytest.mark.parametrize("position,spec,covered", [
+    ("2.1", "2", True), ("2.1", "1 to 2", True), ("2.1", "2.1", True), ("2.1", "3", False), ("2", "2.1", False),
+])
+def test_track_in_spec_subtracks(position, spec, covered):
+    assert rc.track_in_spec(position, spec) is covered
+
+
+def test_find_recordings_pages_past_100(monkeypatch):
+    calls = []
+
+    def fake_mb_get(path, **params):
+        calls.append(params["offset"])
+        start = params["offset"]
+        page = [{"id": f"r{i}", "title": "Shady Grove", "artist-credit": [{"name": "Jerry Garcia"}]}
+                for i in range(start, min(start + 100, 150))]
+        return {"count": 150, "offset": start, "recordings": page}
+
+    monkeypatch.setattr(rc, "mb_get", fake_mb_get)
+    found = rc.find_recordings("Jerry Garcia", "Shady Grove")
+    assert calls == [0, 100]
+    assert len(found) == 150 and found[-1]["id"] == "r149"
+
+
+def test_render_says_when_track_not_found():
+    out = rc.render("Tony Rice", "Nowhere", [{
+        "artist": "Tony Rice", "release": "A Compilation", "date": "", "labels": [], "track": None,
+        "sources": [], "credits": []}])
+    assert "track not found" in out and "None" not in out
+
+
 def test_producer_hidden_behind_a_performance_is_not_reported():
     _, credits = rc.discogs_credits(
         discogs_release([{"name": "Tony Rice", "role": "Guitar, Producer", "tracks": ""}],
-- 
2.53.0