From 0d6393e712b216c9eeafc96d1c916bf16b64a374 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= <snejus@protonmail.com>
Date: Tue, 31 Dec 2024 00:08:10 +0000
Subject: [PATCH] Fix track matching

I had previously tested the `munkres` -> `lapjv` replacement
extensively, so I was today surprised to find that nothing gets matched
correctly when I tried importing some new tracks.

On the other hand I now remember making a small adjustment in the logic
to make autotagging tests pass which is when I introduced a bug: I did
not realize that `lapjv` returns index '-1' for each unmatched item.

This issue did not get caught by tests because this 'unmatched' item
index '-1' anecdotally ended up pointing to the last (expected) item in
the test making it pass.

This commit adjusts the aforementioned test to catch this issue and
fixes the logic to correctly identify unmatched tracks.
---
 beets/autotag/match.py | 16 +++++++++++-----
 test/test_autotag.py   |  6 +++---
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/beets/autotag/match.py b/beets/autotag/match.py
index db4a35b13..a7121fd34 100644
--- a/beets/autotag/match.py
+++ b/beets/autotag/match.py
@@ -127,15 +127,21 @@ def assign_items(
     objects. These "extra" objects occur when there is an unequal number
     of objects of the two types.
     """
+    log.debug("Computing track assignment...")
     # Construct the cost matrix.
     costs = [[float(track_distance(i, t)) for t in tracks] for i in items]
-    # Find a minimum-cost bipartite matching.
-    log.debug("Computing track assignment...")
-    cost, _, assigned_idxs = lap.lapjv(np.array(costs), extend_cost=True)
+    # Assign items to tracks
+    _, _, assigned_item_idxs = lap.lapjv(np.array(costs), extend_cost=True)
     log.debug("...done.")
 
-    # Produce the output matching.
-    mapping = {items[i]: tracks[t] for (t, i) in enumerate(assigned_idxs)}
+    # Each item in `assigned_item_idxs` list corresponds to a track in the
+    # `tracks` list. Each value is either an index into the assigned item in
+    # `items` list, or -1 if that track has no match.
+    mapping = {
+        items[iidx]: t
+        for iidx, t in zip(assigned_item_idxs, tracks)
+        if iidx != -1
+    }
     extra_items = list(set(items) - mapping.keys())
     extra_items.sort(key=lambda i: (i.disc, i.track, i.title))
     extra_tracks = list(set(tracks) - set(mapping.values()))
diff --git a/test/test_autotag.py b/test/test_autotag.py
index e131a6be1..d6f4606e1 100644
--- a/test/test_autotag.py
+++ b/test/test_autotag.py
@@ -551,7 +551,7 @@ class AssignmentTest(unittest.TestCase):
     def test_order_works_with_missing_tracks(self):
         items = []
         items.append(self.item("one", 1))
-        items.append(self.item("three", 3))
+        items.append(self.item("two", 2))
         trackinfo = []
         trackinfo.append(TrackInfo(title="one"))
         trackinfo.append(TrackInfo(title="two"))
@@ -560,8 +560,8 @@ class AssignmentTest(unittest.TestCase):
             items, trackinfo
         )
         assert extra_items == []
-        assert extra_tracks == [trackinfo[1]]
-        assert mapping == {items[0]: trackinfo[0], items[1]: trackinfo[2]}
+        assert extra_tracks == [trackinfo[2]]
+        assert mapping == {items[0]: trackinfo[0], items[1]: trackinfo[1]}
 
     def test_order_works_with_extra_tracks(self):
         items = []