From e6c29452f9cea48a3dc62d94ccca8bb74977c313 Mon Sep 17 00:00:00 2001
From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com>
Date: Sat, 17 Oct 2026 18:31:56 +0200
Subject: [PATCH] fix: never drop a held file for a missing region

Region filtering ranked every declared file, held or not, so a request
answered only by a missing file dropped the held fallback and emptied
the slot. Only held files now decide what gives way; the missing better
match stays reported.
---
 scripts/generate_pack.py |  8 ++++++--
 scripts/packextras.py    | 11 +++++++++++
 scripts/packverify.py    |  4 +++-
 scripts/region.py        | 24 +++++++++++++++++++-----
 scripts/verify.py        |  3 +++
 tests/test_region.py     | 27 +++++++++++++++++++++++++++
 6 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py
index b4846267..5fb8a0cd 100644
--- a/scripts/generate_pack.py
+++ b/scripts/generate_pack.py
@@ -476,6 +476,7 @@ def _select_variants(
     # One grouping for both passes: a file judged in its own system by one
     # pass must not land in every system of its profile for the other.
     region_groups: dict[str, list[tuple[str, str]]] = {}
+    held: set[str] = set()
     if regions or one_per_slot:
         region_groups, _extra_dests = platform_region_groups(
             config,
@@ -488,13 +489,14 @@ def _select_variants(
             include_extras=(source != "platform"),
             include_all=(source == "truth"),
             required_only=required_only,
+            held_out=held,
         )
     if regions:
         region_index = region_mod.platform_region_index(
             config, emu_profiles or {}, target_cores
         )
         region_drops = region_mod.resolve_region_drops(
-            region_groups, region_index, regions
+            region_groups, region_index, regions, held
         )
         region_fallbacks = region_mod.fallback_groups(
             region_groups, region_index, regions
@@ -3528,6 +3530,7 @@ def _manifest_selection(
     the full manifest withdraws what `--region` withdraws from the pack.
     """
     extras: list[dict] = []
+    held: set[str] = set()
     region_groups, _extra_dests = platform_region_groups(
         config,
         pack_systems,
@@ -3540,12 +3543,13 @@ def _manifest_selection(
         include_all=(source == "truth"),
         required_only=required_only,
         extras_out=extras,
+        held_out=held,
     )
     region_index = region_mod.platform_region_index(
         config, emu_profiles, target_cores
     )
     drops = (
-        region_mod.resolve_region_drops(region_groups, region_index, regions)
+        region_mod.resolve_region_drops(region_groups, region_index, regions, held)
         if regions
         else set()
     )
diff --git a/scripts/packextras.py b/scripts/packextras.py
index 33d5d1bf..6c65f70d 100644
--- a/scripts/packextras.py
+++ b/scripts/packextras.py
@@ -877,9 +877,14 @@ def platform_region_groups(
     include_all: bool = False,
     required_only: bool = False,
     extras_out: list[dict] | None = None,
+    held_out: set[str] | None = None,
 ) -> tuple[dict[str, list[tuple[str, str]]], dict[tuple[str, str, str], str]]:
     """Group a platform's pack candidates the way region filtering reads them.
 
+    ``held_out`` receives the destinations whose file the collection holds:
+    a missing file is grouped so the report can withdraw it, but it never
+    takes the slot of a held one (resolve_region_drops).
+
     Returns the groups and, for every core extra, the destination it was
     grouped under, keyed by (emulator, name).  verify.py needs that mapping to
     withdraw from its report exactly what the builder withdraws from the pack:
@@ -898,6 +903,10 @@ def platform_region_groups(
             )
             if dest:
                 members.append((dest, file_entry.get("name", "")))
+                if held_out is not None and db is not None and resolve_local_file(
+                    file_entry, db, dest_hint=dest
+                )[0]:
+                    held_out.add(dest)
 
     extra_dests: dict[tuple[str, str, str], str] = {}
     if not include_extras or db is None:
@@ -938,6 +947,8 @@ def platform_region_groups(
             groups.setdefault(group_id, []).append((dest, name))
         if extras_out is not None and id(extra) in held:
             extras_out.append(extra)
+        if held_out is not None and id(extra) in held:
+            held_out.add(dest)
     return groups, extra_dests
 
 
diff --git a/scripts/packverify.py b/scripts/packverify.py
index fe450dee..e09f38b6 100644
--- a/scripts/packverify.py
+++ b/scripts/packverify.py
@@ -582,6 +582,7 @@ def verify_pack_against_platform(
         region_index = region_mod.platform_region_index(
             config, emu_profiles, target_cores
         )
+        held: set[str] = set()
         region_groups, _extra_dests = platform_region_groups(
             config,
             expected_systems,
@@ -590,9 +591,10 @@ def verify_pack_against_platform(
             base_dest,
             emu_profiles,
             target_cores=target_cores,
+            held_out=held,
         )
         region_drops = region_mod.resolve_region_drops(
-            region_groups, region_index, regions
+            region_groups, region_index, regions, held
         )
 
     with zipfile.ZipFile(zip_path, "r") as zf:
diff --git a/scripts/region.py b/scripts/region.py
index 06cc0800..d70d1979 100644
--- a/scripts/region.py
+++ b/scripts/region.py
@@ -254,6 +254,7 @@ def resolve_region_drops(
     groups: dict[str, list[tuple[str, str]]],
     index: dict[str, dict],
     requested: list[str],
+    held: set[str] | None = None,
 ) -> set[str]:
     """Destinations to skip for a requested region priority list.
 
@@ -261,6 +262,10 @@ def resolve_region_drops(
     A world candidate beats unmatched regional fallbacks the code does not
     search before it, while untagged files always survive.  If neither a requested nor a world candidate exists, all
     regional candidates survive so filtering can never empty a group.
+
+    ``held`` names the destinations the collection holds. A match it lacks
+    never displaces a held file: the American BIOS stays in a Japanese pack
+    when the Japanese one is missing, and the missing one stays reported.
     """
     if not requested:
         return set()
@@ -284,11 +289,20 @@ def resolve_region_drops(
         if not regional:
             continue
         matched = [(r, destination) for r, destination in regional if r < len(requested)]
-        if matched:
-            best = min(r for r, _destination in matched)
-            keep |= {destination for r, destination in matched if r == best}
+        winners, rivals = matched, world
+        if held is not None:
+            # A missing file answers the request but takes no held file's
+            # place: only held files decide what gives way.
+            winners = [(r, d) for r, d in matched if d in held]
+            rivals = world & held
+        if winners:
+            best = min(r for r, _destination in winners)
+            keep |= {
+                destination for r, destination in matched
+                if r == best or (r < best and destination not in (held or ()))
+            }
             drop |= {destination for _r, destination in regional if destination not in keep}
-        elif world:
+        elif rivals:
             # A world file beats an unmatched regional one, unless the code
             # provably looks for the regional one first: DuckStation ranks its
             # PS1 images 5 to 50 and its world PS2 images 100, so a request
@@ -296,7 +310,7 @@ def resolve_region_drops(
             world_ranks = [
                 lookup_priority(index, destination, name)
                 for destination, name in members
-                if destination in world
+                if destination in rivals
             ]
             known = None not in world_ranks
             best_world = min(world_ranks) if known and world_ranks else None
diff --git a/scripts/verify.py b/scripts/verify.py
index b6f79e23..74151d22 100644
--- a/scripts/verify.py
+++ b/scripts/verify.py
@@ -967,6 +967,7 @@ def verify_platform(
         # every core extra a region run withdraws from the pack.
         from packextras import platform_region_groups
 
+        held: set[str] = set()
         region_groups, region_extra_dests = platform_region_groups(
             config,
             verify_systems,
@@ -975,11 +976,13 @@ def verify_platform(
             config.get("base_destination", ""),
             profiles,
             target_cores=target_cores,
+            held_out=held,
         )
         region_drops = region_mod.resolve_region_drops(
             region_groups,
             region_mod.platform_region_index(config, profiles, target_cores),
             regions,
+            held,
         )
 
     # A destination two systems declare with two hashes ships one file, the
diff --git a/tests/test_region.py b/tests/test_region.py
index 79736067..124a3972 100644
--- a/tests/test_region.py
+++ b/tests/test_region.py
@@ -265,6 +265,33 @@ class TestResolveRegionDrops(unittest.TestCase):
     def _drops(self, groups, requested):
         return region.resolve_region_drops(groups, self.index, requested)
 
+    def test_a_missing_match_never_displaces_a_held_file(self):
+        """Grouped so the report can withdraw it, a missing file must not
+        take a held one's place: a Japanese request with only the American
+        BIOS held kept nothing for the PlayStation."""
+        groups = {"psx": [("scph5501.bin", "scph5501.bin"),
+                          ("scph5500.bin", "scph5500.bin")]}
+        held = {"scph5501.bin"}
+        drop = region.resolve_region_drops
+        self.assertEqual(drop(groups, self.index, ["japan"], held), set())
+        self.assertEqual(
+            drop(groups, self.index, ["japan", "north-america"], held), set()
+        )
+        self.assertEqual(
+            drop(groups, self.index, ["north-america"], held), {"scph5500.bin"}
+        )
+        self.assertEqual(drop(groups, self.index, ["japan"]), {"scph5501.bin"})
+
+    def test_a_missing_world_file_never_displaces_a_held_one(self):
+        groups = {"psx": [("scph5500.bin", "scph5500.bin"),
+                          ("psxonpsp660.bin", "psxonpsp660.bin")]}
+        self.assertEqual(
+            region.resolve_region_drops(
+                groups, self.index, ["europe"], {"scph5500.bin"}
+            ),
+            set(),
+        )
+
     def test_a_world_file_the_code_ranks_lower_keeps_the_regional_ones(self):
         """DuckStation looks for its PS1 images (5) before the world PS2 ones
         (100): asked for Brazil, it kept only the PS2 fallbacks."""