From fc47ac9f1fac6b0b3f8f4b44621ba7902fc0021c Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:48:29 +0200 Subject: [PATCH] refactor: pair truth and platform entries one way diff_truth matched by name with a rank, export_native by path, guarded name and hash: on current data each named files the other did not. Both now read one matcher, which also accepts aliases and sha256. --- scripts/exporter/baseline.py | 144 ++++++++++++++++++++--------------- scripts/truth.py | 139 ++++++++------------------------- tests/test_truth_diff.py | 98 ++++++++++-------------- 3 files changed, 156 insertions(+), 225 deletions(-) diff --git a/scripts/exporter/baseline.py b/scripts/exporter/baseline.py index 66295d03..15305c42 100644 --- a/scripts/exporter/baseline.py +++ b/scripts/exporter/baseline.py @@ -267,6 +267,81 @@ def _match_key(entry: dict) -> tuple[str, str]: return dest.casefold(), str(entry.get("name", "")).casefold() +def _names(entry: dict) -> set[str]: + """The names an entry answers to: its own and the aliases it declares.""" + return { + str(n).casefold() + for n in (entry.get("name", ""), *(entry.get("aliases") or [])) + if n + } + + +def by_destination(truth_entry: dict, platform_entry: dict) -> bool: + """The platform's destination ends in the path the truth declares. + + Dolphin writes dolphin-emu/Sys/GC/USA/IPL.bin for the truth's + GC/USA/IPL.bin. A bare name is no path: dc/boot.bin ends in /boot.bin, + and fbneo's 480-byte boot.bin is not that 2 MB Dreamcast BIOS. + """ + t_dest = _match_key(truth_entry)[0] + theirs = _match_key(platform_entry)[0] + if not t_dest: + return False + return theirs == t_dest or ("/" in t_dest and theirs.endswith("/" + t_dest)) + + +def by_name(truth_entry: dict, platform_entry: dict) -> bool: + """One of the truth's names, in the same directory, at the same size.""" + t_dest = _match_key(truth_entry)[0] + theirs_dest, theirs = _match_key(platform_entry) + if not theirs or theirs not in _names(truth_entry): + return False + # Two declared directories that differ name two files: the truth's + # BeebFile/BIOS.rom is not Recalbox's np2kai/bios.rom. + if "/" in t_dest and "/" in theirs_dest and not theirs_dest.endswith( + "/" + t_dest.rsplit("/", 1)[0].rsplit("/", 1)[-1] + "/" + theirs + ): + return False + # A shared name with two declared sizes is two files: fbneo's 480-byte + # boot.bin is not RomM's 2 MB Dreamcast boot.bin. + t_size = truth_entry.get("size") + p_size = platform_entry.get("size") + return not (isinstance(t_size, int) and isinstance(p_size, int) and t_size != p_size) + + +def by_hash(truth_entry: dict, platform_entry: dict) -> bool: + """A value on any of the four hash fields in common: a renamed file.""" + ours = {v for f in HASH_FIELDS for v in _hash_values(truth_entry, f)} + if not ours: + return False + return bool(ours & {v for f in HASH_FIELDS for v in _hash_values(platform_entry, f)}) + + +def pair_truth_entry( + truth_entry: dict, candidates: list[dict], claimed: set[int] +) -> int | None: + """Which platform entry a truth entry describes, by index, or None. + + The one rule export_native and diff_truth read. Each test is tried + across every candidate before the next: with three IPL.bin under one + system, separated only by their path, a first-match-wins scan attached + the truth to whichever came first. An entry never answers for two. + """ + for test in (by_destination, by_name, by_hash): + for index, candidate in enumerate(candidates): + if index not in claimed and test(truth_entry, candidate): + return index + return None + + +def platform_declares(truth_entry: dict, candidates: list[dict]) -> bool: + """A file another truth entry already claimed is still the platform's + file, not one it lacks: scph101.bin, declared by two PSX cores.""" + return any( + by_destination(truth_entry, c) or by_name(truth_entry, c) for c in candidates + ) + + def build_native_model( truth: dict, scraped: dict | None, @@ -363,13 +438,6 @@ def build_native_model( if not name or name.startswith("_") or _is_placeholder(name): continue - t_dest, t_name = _match_key(truth_entry) - t_hashes = { - value - for field_name in HASH_FIELDS - for value in _hash_values(truth_entry, field_name) - } - candidates = [ candidate for native_id in target_ids @@ -378,60 +446,10 @@ def build_native_model( ).files if candidate.truth is None ] - - def by_destination(candidate: NativeFile, t_dest: str = t_dest) -> bool: - # Dolphin writes dolphin-emu/Sys/GC/USA/IPL.bin for the truth's - # GC/USA/IPL.bin: a destination ending in the path is the file. - # A bare name is no path: dc/boot.bin ends in /boot.bin, and - # fbneo's 480-byte boot.bin is not that 2 MB Dreamcast BIOS. - # Names go through by_name and its size guard. - theirs = _match_key(candidate.platform or {})[0] - if not t_dest: - return False - return theirs == t_dest or ("/" in t_dest and theirs.endswith("/" + t_dest)) - - def by_name( - candidate: NativeFile, - t_dest: str = t_dest, - t_name: str = t_name, - truth_entry: dict = truth_entry, - ) -> bool: - theirs_dest, theirs = _match_key(candidate.platform or {}) - if not t_name or theirs != t_name: - return False - # Two declared directories that differ name two files: the - # truth's BeebFile/BIOS.rom is not Recalbox's np2kai/bios.rom. - if "/" in t_dest and "/" in theirs_dest and not theirs_dest.endswith( - "/" + t_dest.rsplit("/", 1)[0].rsplit("/", 1)[-1] + "/" + t_name - ): - return False - # A shared name with two declared sizes is two files: fbneo's - # 480-byte boot.bin is not RomM's 2 MB Dreamcast boot.bin. - t_size = truth_entry.get("size") - p_size = (candidate.platform or {}).get("size") - return not ( - isinstance(t_size, int) and isinstance(p_size, int) and t_size != p_size - ) - - def by_hash(candidate: NativeFile, t_hashes: set = t_hashes) -> bool: - if not t_hashes: - return False - theirs = { - value - for field_name in HASH_FIELDS - for value in _hash_values(candidate.platform or {}, field_name) - } - return bool(t_hashes & theirs) - - # Tried in order across every candidate, not per candidate: with - # three IPL.bin under one system, separated only by their path, a - # first-match-wins scan would attach the truth to whichever came - # first and correct the wrong region's file. - matched: NativeFile | None = None - for test in (by_destination, by_name, by_hash): - matched = next((c for c in candidates if test(c)), None) - if matched is not None: - break + index = pair_truth_entry( + truth_entry, [c.platform or {} for c in candidates], set() + ) + matched = candidates[index] if index is not None else None if matched is not None: matched.truth = truth_entry @@ -462,12 +480,12 @@ def build_native_model( # platform's file, not a new one: scph101.bin, declared by two # PSX cores, was written into RetroDECK's manifest a second time. declared = [ - candidate + candidate.platform for native_id in target_ids for candidate in systems.get(native_id, NativeSystem(native_id)).files if candidate.platform is not None ] - if any(by_destination(c) or by_name(c) for c in declared): + if platform_declares(truth_entry, declared): continue # The truth knows a file the platform does not declare. diff --git a/scripts/truth.py b/scripts/truth.py index 6152b8e6..5611d882 100644 --- a/scripts/truth.py +++ b/scripts/truth.py @@ -576,52 +576,6 @@ def _hash_values(entry: dict, field: str) -> set[str]: return set(parse_md5_list(raw)) if isinstance(raw, (str, list)) else set() -def _match_renames( - unmatched_truth: list[dict], unmatched_scraped: dict -) -> tuple[set[int], set]: - """Pair files a platform renamed with the truth entry they came from. - - A platform is free to call a file whatever it likes -- Batocera ships - the same ROM as ROM1 -- so a name that matches nothing is not yet a - gap. When an unmatched file on either side carries the same hash, it - is one file under two names, and counting it as both missing and extra - would report a gap that does not exist. - - Returns the positions in unmatched_truth and the scraped keys that pair - up: a name does not identify a truth entry, two can share it. - """ - # Hash-based fallback: detect platform renames (e.g. Batocera ROM → ROM1) - # If an unmatched scraped file shares a hash with an unmatched truth file, - # it's the same file under a different name: a platform rename, not a gap. - rename_matched_truth: set[int] = set() - rename_matched_scraped: set = set() - - if unmatched_truth and unmatched_scraped: - # Build hash → truth file index for unmatched truth files - truth_hash_index: dict[str, int] = {} - for position, fe in enumerate(unmatched_truth): - for h in ("sha1", "md5", "crc32"): - for val in _hash_values(fe, h): - truth_hash_index[val] = position - - for s_key, s_entry in unmatched_scraped.items(): - position = next( - ( - truth_hash_index[val] - for h in ("sha1", "md5", "crc32") - for val in sorted(_hash_values(s_entry, h)) - if val in truth_hash_index - ), - None, - ) - if position is not None: - # Rename detected. Count as matched - rename_matched_truth.add(position) - rename_matched_scraped.add(s_key) - - return rename_matched_truth, rename_matched_scraped - - def _hash_set(entry: dict) -> set[str]: return {value for h in ("sha1", "md5", "crc32") for value in _hash_values(entry, h)} @@ -630,22 +584,6 @@ def _path_tail(value: object) -> str: return str(value or "").replace("\\", "/").casefold() -def _pair_rank(truth_entry: dict, scraped_entry: dict) -> tuple[bool, bool, bool, bool]: - """How well a same-named truth entry describes a scraped one. - - Exact path suffix, then same directory, then primary name over alias, - then a shared hash. - """ - destination = _path_tail(scraped_entry.get("destination")) - t_path = _path_tail(truth_entry.get("path")) - return ( - bool(t_path) and destination.endswith(t_path), - "/" in t_path and t_path.rsplit("/", 1)[0] == destination.rpartition("/")[0], - truth_entry["name"].lower() == scraped_entry["name"].lower(), - bool(_hash_set(scraped_entry) & _hash_set(truth_entry)), - ) - - def _hash_mismatch(t_entry: dict, s_entry: dict) -> dict | None: """The first hash both sides declare without a value in common. @@ -672,17 +610,19 @@ def _hash_mismatch(t_entry: dict, s_entry: dict) -> dict | None: def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: """Compare files between truth and scraped for a single system. - A name can stand for several files of one system (Dolphin's IPL.bin - under GC/USA, GC/EUR and GC/JAP), so each scraped entry is paired with - the same-named truth entry at its destination first, and an entry - never answers for two. + Entries are paired by pair_truth_entry, the rule export_native applies: + destination, then name or alias in the same directory at the same size, + then any hash in common. Two rules had diverged, and diff_truth flagged + a hash the export did not correct while the export corrected hashes the + diff never named. An entry never answers for two. """ - truth_files = truth_sys.get("files", []) - truth_index: dict[str, list[int]] = {} - for position, fe in enumerate(truth_files): - for key in [fe["name"], *fe.get("aliases", [])]: - truth_index.setdefault(key.lower(), []).append(position) + from exporter.baseline import _is_placeholder, pair_truth_entry, platform_declares + truth_files = [ + fe for fe in truth_sys.get("files", []) + if fe.get("name") and not fe["name"].startswith("_") + and not _is_placeholder(fe["name"]) + ] scraped_files = scraped_sys.get("files", []) missing: list[dict] = [] @@ -691,22 +631,21 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: extra_phantom: list[dict] = [] extra_unprofiled: list[dict] = [] - matched: set[int] = set() - unmatched_scraped: dict[int, dict] = {} - for s_position, s_entry in enumerate(scraped_files): - candidates = [ - p for p in truth_index.get(s_entry["name"].lower(), []) if p not in matched - ] - if not candidates: - if s_entry["name"].lower() not in truth_index: - unmatched_scraped[s_position] = s_entry + claimed: set[int] = set() + for t_entry in truth_files: + s_position = pair_truth_entry(t_entry, scraped_files, claimed) + if s_position is None: + if not platform_declares(t_entry, scraped_files): + missing.append( + { + "name": t_entry["name"], + "cores": list(t_entry.get("_cores", [])), + "source_refs": list(t_entry.get("_source_refs", [])), + } + ) continue - # The first best candidate wins, as max() keeps the first maximum. - ranked = [(_pair_rank(truth_files[p], s_entry), p) for p in candidates] - t_position = max(ranked, key=lambda pair: pair[0])[1] - matched.add(t_position) - t_entry = truth_files[t_position] - + claimed.add(s_position) + s_entry = scraped_files[s_position] mismatch = _hash_mismatch(t_entry, s_entry) if mismatch: hash_mismatch.append(mismatch) @@ -721,25 +660,13 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: } ) - # Collect unmatched files from both sides - unmatched_truth = [ - fe for position, fe in enumerate(truth_files) if position not in matched - ] - - rename_matched_truth, rename_matched_scraped = _match_renames( - unmatched_truth, unmatched_scraped - ) - - # Truth files not matched (by name, alias, or hash) -> missing - for position, fe in enumerate(unmatched_truth): - if position not in rename_matched_truth: - missing.append( - { - "name": fe["name"], - "cores": list(fe.get("_cores", [])), - "source_refs": list(fe.get("_source_refs", [])), - } - ) + # A scraped entry no truth entry describes, by path, name or hash. + unmatched_scraped = { + position: s_entry + for position, s_entry in enumerate(scraped_files) + if position not in claimed + and not any(platform_declares(t, [s_entry]) for t in truth_files) + } # Scraped files not in truth -> extra coverage = truth_sys.get("_coverage", {}) @@ -749,7 +676,7 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: seen_extra: set[tuple[str, str]] = set() for s_key, s_entry in unmatched_scraped.items(): key = (s_entry["name"].lower(), _path_tail(s_entry.get("destination"))) - if s_key in rename_matched_scraped or key in seen_extra: + if key in seen_extra: continue seen_extra.add(key) entry = {"name": s_entry["name"]} diff --git a/tests/test_truth_diff.py b/tests/test_truth_diff.py index 28cd2011..c8661f5e 100644 --- a/tests/test_truth_diff.py +++ b/tests/test_truth_diff.py @@ -19,9 +19,9 @@ from pathlib import Path REPO_ROOT = Path(__file__).resolve().parent.parent sys.path.insert(0, str(REPO_ROOT / "scripts")) +from exporter.baseline import pair_truth_entry # noqa: E402 from truth import ( # noqa: E402 _diff_system, - _match_renames, _merge_file_into_system, generate_platform_truth, ) @@ -132,74 +132,60 @@ class DifferentContentsStayApart(unittest.TestCase): class RenameMatching(unittest.TestCase): - def test_a_shared_sha1_pairs_the_two_names(self): - truth = [_entry("bios_CD_U.bin", sha1="a" * 40)] - scraped = {"rom1.bin": _entry("ROM1.bin", sha1="a" * 40)} - matched_truth, matched_scraped = _match_renames(truth, scraped) - self.assertEqual(matched_truth, {0}) - self.assertEqual(matched_scraped, {"rom1.bin"}) + """A platform may call a file anything (Batocera ships a ROM as ROM1): + a shared hash pairs the two names, through the rule export_native uses.""" - def test_md5_and_crc32_pair_them_too(self): - for algorithm in ("md5", "crc32"): + @staticmethod + def _pair(truth, scraped, claimed=None): + return pair_truth_entry(truth, scraped, claimed or set()) + + def test_a_shared_sha1_pairs_the_two_names(self): + self.assertEqual( + self._pair(_entry("bios_CD_U.bin", sha1="a" * 40), + [_entry("ROM1.bin", sha1="a" * 40)]), + 0, + ) + + def test_md5_crc32_and_sha256_pair_them_too(self): + for algorithm, value in (("md5", "b" * 32), ("crc32", "b" * 8), ("sha256", "c" * 64)): with self.subTest(algorithm=algorithm): - value = "b" * 32 - truth = [_entry("a.bin", **{algorithm: value})] - scraped = {"b.bin": _entry("b.bin", **{algorithm: value})} - matched_truth, _ = _match_renames(truth, scraped) - self.assertEqual(matched_truth, {0}) + self.assertEqual( + self._pair(_entry("a.bin", **{algorithm: value}), + [_entry("b.bin", **{algorithm: value})]), + 0, + ) def test_hash_comparison_ignores_case(self): - truth = [_entry("a.bin", sha1="AbCdEf" + "0" * 34)] - scraped = {"b.bin": _entry("b.bin", sha1="abcdef" + "0" * 34)} - self.assertEqual(_match_renames(truth, scraped)[0], {0}) + self.assertEqual( + self._pair(_entry("a.bin", sha1="AbCdEf" + "0" * 34), + [_entry("b.bin", sha1="abcdef" + "0" * 34)]), + 0, + ) def test_different_content_is_not_a_rename(self): - truth = [_entry("a.bin", sha1="a" * 40)] - scraped = {"b.bin": _entry("b.bin", sha1="b" * 40)} - self.assertEqual(_match_renames(truth, scraped), (set(), set())) + self.assertIsNone( + self._pair(_entry("a.bin", sha1="a" * 40), [_entry("b.bin", sha1="b" * 40)]) + ) def test_files_with_no_hashes_never_pair(self): """Two unknowns are not evidence of the same file.""" - truth = [_entry("a.bin")] - scraped = {"b.bin": _entry("b.bin")} - self.assertEqual(_match_renames(truth, scraped), (set(), set())) + self.assertIsNone(self._pair(_entry("a.bin"), [_entry("b.bin")])) - def test_a_non_string_hash_is_ignored_rather_than_crashing(self): - truth = [_entry("a.bin", crc32=12345678)] - scraped = {"b.bin": _entry("b.bin", crc32=12345678)} - self.assertEqual(_match_renames(truth, scraped), (set(), set())) - - def test_empty_input_on_either_side_pairs_nothing(self): - self.assertEqual(_match_renames([], {}), (set(), set())) - self.assertEqual(_match_renames([_entry("a", sha1="a" * 40)], {}), (set(), set())) + def test_a_hash_yaml_made_a_number_reads_as_its_digits(self): + # A digit-only crc32 written unquoted: the export writes it back. self.assertEqual( - _match_renames([], {"b": _entry("b", sha1="a" * 40)}), (set(), set()) + self._pair(_entry("a.bin", crc32=12345678), [_entry("b.bin", crc32="12345678")]), + 0, ) - def test_each_scraped_file_pairs_once(self): - """A file matching on several digests must not be counted twice.""" - truth = [_entry("a.bin", sha1="a" * 40, md5="b" * 32)] - scraped = {"b.bin": _entry("b.bin", sha1="a" * 40, md5="b" * 32)} - matched_truth, matched_scraped = _match_renames(truth, scraped) - self.assertEqual(len(matched_truth), 1) - self.assertEqual(len(matched_scraped), 1) + def test_a_claimed_entry_never_answers_twice(self): + scraped = [_entry("b.bin", sha1="a" * 40)] + self.assertIsNone(self._pair(_entry("a.bin", sha1="a" * 40), scraped, {0})) - def test_several_renames_are_all_reported(self): - truth = [_entry("a.bin", sha1="a" * 40), _entry("c.bin", sha1="c" * 40)] - scraped = { - "b.bin": _entry("b.bin", sha1="a" * 40), - "d.bin": _entry("d.bin", sha1="c" * 40), - } - matched_truth, matched_scraped = _match_renames(truth, scraped) - self.assertEqual(matched_truth, {0, 1}) - self.assertEqual(matched_scraped, {"b.bin", "d.bin"}) - - def test_an_unrelated_file_beside_a_rename_stays_unmatched(self): - truth = [_entry("a.bin", sha1="a" * 40), _entry("lonely.bin", sha1="f" * 40)] - scraped = {"b.bin": _entry("b.bin", sha1="a" * 40)} - matched_truth, _ = _match_renames(truth, scraped) - self.assertEqual(matched_truth, {0}) - self.assertNotIn("lonely.bin", matched_truth) + def test_the_diff_reads_no_gap_for_a_rename(self): + truth = {"files": [_entry("a.bin", sha1="a" * 40)]} + scraped = {"files": [_entry("b.bin", sha1="a" * 40)]} + self.assertEqual(_diff_system(truth, scraped), {}) @@ -222,7 +208,7 @@ class PlatformMultiHash(unittest.TestCase): def test_a_rename_is_found_through_the_list(self): truth = [_entry("scph1001.bin", md5=self.TRUTH)] scraped = {"psx.bin": _entry("psx.bin", md5=self.SCRAPED)} - self.assertEqual(_match_renames(truth, scraped), ({0}, {"psx.bin"})) + self.assertEqual(pair_truth_entry(truth[0], list(scraped.values()), set()), 0) class OnlySystemDirectoryEntries(unittest.TestCase):