From 181ad0c63013e4fe9abe90be88d599a45a85836e Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:24:00 +0200 Subject: [PATCH] fix: pair truth entries hash-consistent first Each truth entry claimed its platform line in turn, so the first one to arrive won. dosbox-x's system.rom (alias bios.rom, no path) took Batocera's np2kai/BIOS.ROM by name before np2kai's own np2kai/bios.rom was tried, and the export wrote an IBM PCjr md5 on it. A path also outranked a contradicting hash: the truth's MSX.rom joins bluemsx's path and fmsx's md5, which replaced the blueMSX dump on Recalbox's and System.dat's Machines/Shared Roms/MSX.rom while fmsx's own line held it. pair_entries runs each test over every entry before the next, tests that contradict no hash first. export_native and diff_truth both read it. --- scripts/exporter/baseline.py | 98 +++++++++++++++++++++++++----------- scripts/truth.py | 19 ++++--- tests/test_exporters.py | 72 ++++++++++++++++++++++++++ tests/test_truth_diff.py | 11 ++-- 4 files changed, 155 insertions(+), 45 deletions(-) diff --git a/scripts/exporter/baseline.py b/scripts/exporter/baseline.py index 9044afb0..e3e3df6f 100644 --- a/scripts/exporter/baseline.py +++ b/scripts/exporter/baseline.py @@ -14,6 +14,7 @@ grouping is rebuilt from the per-file native_system the scrapers record. from __future__ import annotations import sys +from collections.abc import Callable from dataclasses import dataclass, field from pathlib import Path @@ -317,26 +318,64 @@ def by_hash(truth_entry: dict, platform_entry: dict) -> bool: 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. +def _contradicts(truth_entry: dict, platform_entry: dict) -> bool: + """A hash both entries declare, without a value in common.""" + return any( + (ours := set(_hash_values(truth_entry, f))) + and (theirs := set(_hash_values(platform_entry, f))) + and not ours & theirs + for f in HASH_FIELDS + ) - 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. + +def _agreeing(test: Callable[[dict, dict], bool]) -> Callable[[dict, dict], bool]: + def agrees(truth_entry: dict, platform_entry: dict) -> bool: + return test(truth_entry, platform_entry) and not _contradicts( + truth_entry, platform_entry + ) + return agrees + + +_PAIRING_TESTS = ( + _agreeing(by_destination), + _agreeing(by_name), + by_hash, + by_destination, + by_name, +) + + +def pair_entries(truth_entries: list[dict], candidates: list[dict]) -> dict[int, int]: + """Which platform entry each truth entry describes, as index -> index. + + The one rule export_native and diff_truth read. Each test runs over + every truth entry and every candidate before the next one does: + claimed entry by entry, dosbox-x's system.rom took np2kai/BIOS.ROM by + its alias before np2kai's own np2kai/bios.rom was tried, and wrote an + IBM PCjr md5 on it. A path or a name that contradicts a hash comes + after the hash: the truth's MSX.rom carries bluemsx's path and fmsx's + md5, and fmsx's own line holds that md5. 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 + pairs: dict[int, int] = {} + claimed: set[int] = set() + for test in _PAIRING_TESTS: + for t_index, truth_entry in enumerate(truth_entries): + if t_index in pairs: + continue + for index, candidate in enumerate(candidates): + if index not in claimed and test(truth_entry, candidate): + pairs[t_index] = index + claimed.add(index) + break + return pairs 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.""" + file, not one it lacks: scph101.bin, declared by two PSX cores. The + same content under another name is not: fuse opens fuse/128-1.rom by + name, and Recalbox's fuse/128p-1.rom holding that dump does not serve + it.""" return any( by_destination(truth_entry, c) or by_name(truth_entry, c) for c in candidates ) @@ -432,23 +471,22 @@ def build_native_model( continue target_ids = _target_native_ids(truth_sid) + truth_files = [ + entry for entry in truth_files + if (name := str(entry.get("name", ""))) + and not name.startswith("_") and not _is_placeholder(name) + ] + candidates = [ + candidate + for native_id in target_ids + for candidate in systems.get(native_id, NativeSystem(native_id)).files + if candidate.truth is None + ] + pairs = pair_entries(truth_files, [c.platform or {} for c in candidates]) - for truth_entry in truth_files: - name = str(truth_entry.get("name", "")) - if not name or name.startswith("_") or _is_placeholder(name): - continue - - candidates = [ - candidate - for native_id in target_ids - for candidate in systems.get( - native_id, NativeSystem(native_id) - ).files - if candidate.truth is None - ] - index = pair_truth_entry( - truth_entry, [c.platform or {} for c in candidates], set() - ) + for t_index, truth_entry in enumerate(truth_files): + name = str(truth_entry["name"]) + index = pairs.get(t_index) matched = candidates[index] if index is not None else None if matched is not None: diff --git a/scripts/truth.py b/scripts/truth.py index 2c559d03..13577ea8 100644 --- a/scripts/truth.py +++ b/scripts/truth.py @@ -618,13 +618,12 @@ 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. - 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. + Entries are paired by pair_entries, the rule export_native applies. 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. """ - from exporter.baseline import _is_placeholder, pair_truth_entry, platform_declares + from exporter.baseline import _is_placeholder, pair_entries, platform_declares truth_files = [ fe for fe in truth_sys.get("files", []) @@ -639,9 +638,10 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: extra_phantom: list[dict] = [] extra_unprofiled: list[dict] = [] - claimed: set[int] = set() - for t_entry in truth_files: - s_position = pair_truth_entry(t_entry, scraped_files, claimed) + pairs = pair_entries(truth_files, scraped_files) + claimed = set(pairs.values()) + for t_index, t_entry in enumerate(truth_files): + s_position = pairs.get(t_index) if s_position is None: if not platform_declares(t_entry, scraped_files): missing.append( @@ -652,7 +652,6 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: } ) continue - claimed.add(s_position) s_entry = scraped_files[s_position] mismatch = _hash_mismatch(t_entry, s_entry) if mismatch: diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 520f45bc..19502e5a 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -199,6 +199,78 @@ class Reconciliation(unittest.TestCase): names = {fe.name for fe in systems["psx"].files} self.assertNotIn(".bin", names) + def test_a_name_match_does_not_take_a_later_entrys_destination(self): + """dosbox-x's system.rom (alias bios.rom, no path) came before + np2kai's np2kai/bios.rom in the truth, took Batocera's + np2kai/BIOS.ROM by name and wrote an IBM PCjr md5 on it.""" + truth = {"systems": {"nec-pc-98": {"files": [ + {"name": "system.rom", "aliases": ["bios.rom"], "md5": "c" * 32}, + {"name": "bios.rom", "path": "np2kai/bios.rom"}, + ]}}} + scraped = {"systems": {"nec-pc-98": {"native_id": "pc98", "files": [ + {"name": "BIOS.ROM", "destination": "np2kai/BIOS.ROM", + "native_system": "pc98", "md5": "e" * 32}, + ]}}} + systems, report = build_native_model(truth, scraped) + entry = systems["pc98"].files[0] + self.assertEqual(entry.truth["path"], "np2kai/bios.rom") + self.assertEqual(entry.hash("md5"), "e" * 32) + self.assertEqual(report.hashes_corrected, []) + self.assertEqual(report.files_added, 0) + + def test_a_path_does_not_outrank_the_hash_another_line_carries(self): + """The truth's MSX.rom joins bluemsx's path and fmsx's md5. Paired by + path, fmsx's md5 replaced the blueMSX dump on Recalbox's + Machines/Shared Roms/MSX.rom line while fmsx's own line held it.""" + truth = {"systems": {"microsoft-msx": {"files": [ + {"name": "MSX.rom", "path": "Machines/Shared Roms/MSX.rom", + "md5": "a" * 32}, + ]}}} + scraped = {"systems": {"microsoft-msx": {"native_id": "msx", "files": [ + {"name": "MSX.rom", "destination": "Machines/Shared Roms/MSX.rom", + "native_system": "msx", "md5": "b" * 32}, + {"name": "MSX.ROM", "destination": "MSX.ROM", + "native_system": "msx", "md5": "a" * 32}, + ]}}} + systems, report = build_native_model(truth, scraped) + bluemsx, fmsx = systems["msx"].files + self.assertIsNone(bluemsx.truth) + self.assertEqual(bluemsx.hash("md5"), "b" * 32) + self.assertIs(fmsx.truth, truth["systems"]["microsoft-msx"]["files"][0]) + self.assertEqual(report.hashes_corrected, []) + + def test_a_name_the_core_opens_is_added_beside_its_content(self): + """fuse opens fuse/128-1.rom and fuse/128p-1.rom, one dump. Recalbox + declares only 128p-1.rom. Paired entry by entry, 128-1.rom took that + line by hash and was never added. A core that opens a name is not + served by the same bytes under another one.""" + truth = {"systems": {"sinclair-zx-spectrum": {"files": [ + {"name": "128p-1.rom", "path": "fuse/128p-1.rom", "md5": "d" * 32}, + {"name": "128-1.rom", "path": "fuse/128-1.rom", "md5": "d" * 32}, + ]}}} + scraped = {"systems": {"sinclair-zx-spectrum": {"native_id": "zx", "files": [ + {"name": "128p-1.rom", "destination": "fuse/128p-1.rom", + "native_system": "zx", "md5": "d" * 32}, + ]}}} + systems, report = build_native_model(truth, scraped) + own, added = systems["zx"].files + self.assertEqual(own.truth["name"], "128p-1.rom") + self.assertEqual(added.destination, "fuse/128-1.rom") + self.assertEqual(report.files_added, 1) + + def test_the_diff_pairs_by_the_same_rule(self): + """diff_truth reads the same pairing, or it names a mismatch the + export does not correct.""" + from truth import _diff_system + truth_sys = {"files": [ + {"name": "system.rom", "aliases": ["bios.rom"], "md5": "c" * 32}, + {"name": "bios.rom", "path": "np2kai/bios.rom"}, + ]} + scraped_sys = {"files": [ + {"name": "BIOS.ROM", "destination": "np2kai/BIOS.ROM", "md5": "e" * 32}, + ]} + self.assertNotIn("hash_mismatch", _diff_system(truth_sys, scraped_sys)) + def test_systems_are_keyed_by_the_name_the_platform_uses(self): systems, _ = model() self.assertEqual(sorted(systems), ["psx"]) diff --git a/tests/test_truth_diff.py b/tests/test_truth_diff.py index b2ffe176..dd1c677a 100644 --- a/tests/test_truth_diff.py +++ b/tests/test_truth_diff.py @@ -19,7 +19,7 @@ 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 exporter.baseline import pair_entries # noqa: E402 from truth import ( # noqa: E402 _diff_system, _merge_file_into_system, @@ -136,8 +136,8 @@ class RenameMatching(unittest.TestCase): a shared hash pairs the two names, through the rule export_native uses.""" @staticmethod - def _pair(truth, scraped, claimed=None): - return pair_truth_entry(truth, scraped, claimed or set()) + def _pair(truth, scraped): + return pair_entries([truth], scraped).get(0) def test_a_shared_sha1_pairs_the_two_names(self): self.assertEqual( @@ -180,7 +180,8 @@ class RenameMatching(unittest.TestCase): 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})) + truth = [_entry("a.bin", sha1="a" * 40), _entry("c.bin", sha1="a" * 40)] + self.assertEqual(pair_entries(truth, scraped), {0: 0}) def test_the_diff_reads_no_gap_for_a_rename(self): truth = {"files": [_entry("a.bin", sha1="a" * 40)]} @@ -208,7 +209,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(pair_truth_entry(truth[0], list(scraped.values()), set()), 0) + self.assertEqual(pair_entries(truth, list(scraped.values())), {0: 0}) class OnlySystemDirectoryEntries(unittest.TestCase):