From 651acb42e580fa1cf9ebc9ac9a72a474d588ee9d Mon Sep 17 00:00:00 2001 From: Alex Vanderveen Date: Sat, 22 Aug 2026 20:45:47 -0400 Subject: [PATCH] lockstep: say what a skip reason MEANS, not just how many MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The segment-level run came back with 10,619 checked and 74,111 skipped, and the weak-coverage guard correctly refused to call it a result. But "74,111" on its own does not distinguish the two situations that produce it, and they call for opposite responses: something is wrong, or this mode structurally cannot cover this game. Here it is the second. 99% of the skips are segments containing an interrupt, which cannot be replayed — so in an interrupt-heavy game segment granularity skips almost everything by construction. The tool now names the dominant reason when one accounts for 60% or more, explains it, and in this case says outright to use block granularity instead. That is not hypothetical: the block-level run over the same game covered 5,679,898 blocks with ZERO skips. Same instrument, same question, and the two modes differ by four orders of magnitude in what they actually compared. No single reason above 60% names no cause at all, since inventing one would be worse than staying quiet. Co-Authored-By: Claude Opus 5 (1M context) --- tools/lockstep_check.py | 40 +++++++++++++++++++++++++++++++---- tools/tests/test_lockstep.py | 41 ++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/tools/lockstep_check.py b/tools/lockstep_check.py index a74fa888..230709cd 100644 --- a/tools/lockstep_check.py +++ b/tools/lockstep_check.py @@ -60,6 +60,27 @@ MEANING = { SKIPS = ("skipped_irq", "skipped_overflow", "skipped_unhandled", "skipped_conflict", "skipped_disabled") +# What a dominant skip reason means for the operator. The distinction that +# matters is between "this mode cannot cover this game" and "something is +# wrong": a bare count of 74,111 does not say which, and the two call for +# opposite responses. +SKIP_MEANING = { + "skipped_irq": + "segments containing an interrupt cannot be replayed. In an " + "interrupt-heavy game that is most of them, and it is a property of " + "SEGMENT granularity rather than a fault. Use block granularity, which " + "does not have this limitation.", + "skipped_overflow": + "the recorded operation trace ran out of room. Shorten the window.", + "skipped_unhandled": + "these segments contain operations the replayer does not implement, so " + "they are outside what this comparison can check at all.", + "skipped_conflict": + "overlapping recordings; usually harmless in small numbers.", + "skipped_disabled": + "the comparator was switched off for these segments.", +} + def summarise(d, func_mode, out=sys.stdout): checked = d.get("segments_checked" if func_mode else "blocks_checked", 0) @@ -68,9 +89,15 @@ def summarise(d, func_mode, out=sys.stdout): print(f"window frames {lo}..{hi}, {checked} {unit}(s) checked", file=out) skipped = {k: d.get(k, 0) for k in SKIPS if d.get(k)} + total_skipped = sum(skipped.values()) if skipped: print(" skipped: " + ", ".join(f"{k[8:]}={v}" for k, v in skipped.items()), file=out) + top = max(skipped, key=skipped.get) + if total_skipped and skipped[top] >= total_skipped * 0.6: + print(f" mostly {top[8:]} ({100 * skipped[top] // total_skipped}%): " + f"{SKIP_MEANING.get(top, '')}", file=out) + d["dominant_skip"] = top if not d.get("found"): if not checked: @@ -79,10 +106,15 @@ def summarise(d, func_mode, out=sys.stdout): "the frame window, or confirm the comparator is enabled in " "this build.", file=out) return "inconclusive" - if skipped and sum(skipped.values()) >= checked: - print(f"\nWEAK: {sum(skipped.values())} {unit}(s) were skipped " - f"against {checked} checked. Coverage is thin enough that a " - f"divergence could easily sit in what was skipped.", file=out) + if skipped and total_skipped >= checked: + print(f"\nWEAK: {total_skipped} {unit}(s) were skipped against " + f"{checked} checked — {total_skipped / max(1, checked):.1f}x " + f"more skipped than compared. A divergence could easily sit " + f"in what was skipped, so this is not evidence of anything.", + file=out) + if d.get("dominant_skip") == "skipped_irq" and func_mode: + print("Run it again at BLOCK granularity: blocks do not skip " + "interrupts, and cover far more of the code.", file=out) return "weak" print(f"\nNo divergence across {checked} {unit}(s). The compiled code " f"matched the interpreter everywhere it was compared.\n" diff --git a/tools/tests/test_lockstep.py b/tools/tests/test_lockstep.py index 3754bfeb..8f62ae8b 100644 --- a/tools/tests/test_lockstep.py +++ b/tools/tests/test_lockstep.py @@ -97,3 +97,44 @@ class TestVerdicts(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class TestSkipGuidance(unittest.TestCase): + """A skip count is a number; what it implies is the useful part. + + Measured on a real run: 10,619 segments checked against 74,111 skipped, 99% + of them because the segment contained an interrupt. That means SEGMENT + granularity cannot cover this game — not that anything is broken. The two + call for opposite responses, and 74,111 on its own does not say which. + """ + + def test_irq_dominated_skips_recommend_block_granularity(self): + v, txt = run({"found": 0, "segments_checked": 10619, "window": [2572, 2692], + "skipped_irq": 73489, "skipped_conflict": 300, + "skipped_disabled": 322}, func=True) + self.assertEqual(v, "weak") + self.assertIn("BLOCK granularity", txt) + self.assertIn("99%", txt) + + def test_the_ratio_is_stated_not_just_the_counts(self): + _, txt = run({"found": 0, "segments_checked": 100, "window": [0, 10], + "skipped_irq": 700}, func=True) + self.assertIn("7.0x", txt) + + def test_block_mode_does_not_suggest_itself(self): + # The advice only makes sense when you are NOT already using blocks. + _, txt = run({"found": 0, "blocks_checked": 100, "window": [0, 10], + "skipped_irq": 700}, func=False) + self.assertNotIn("BLOCK granularity", txt) + + def test_a_mixed_skip_profile_names_no_single_cause(self): + # No reason above 60% means there is no one thing to point at, and + # inventing one would be worse than staying quiet. + _, txt = run({"found": 0, "segments_checked": 10, "window": [0, 10], + "skipped_irq": 30, "skipped_conflict": 30, + "skipped_disabled": 30}, func=True) + self.assertNotIn("mostly", txt) + + def test_a_clean_run_with_no_skips_says_nothing_about_skipping(self): + _, txt = run({"found": 0, "blocks_checked": 5679898, "window": [9275, 9395]}) + self.assertNotIn("skipped", txt)