feat: close campaign findings by code review

campaign.py review applies or rejects an open finding with the reason read in the code. Applied fields go through the profile editor with the finding's records as evidence. The rule table reads on_mismatch: warn and counts a signature as a content check.

schemas/campaign-finding.schema.json

@@ -25,8 +25,13 @@
     "file": { "type": "string", "minLength": 1 },
     "field": { "type": "string", "minLength": 1 },
     "status": {
-      "enum": ["applied", "open", "flagged", "resolved"],
-      "description": "applied: the rule table settled the field and the profile was changed. open: no rule settles it, it waits for review. flagged: a declared file the runs never opened. resolved: a later run found the profile in agreement."
+      "enum": ["applied", "open", "flagged", "resolved", "rejected"],
+      "description": "applied: the rule table, or a review of the code, settled the field and the profile was changed. open: no rule settles it, it waits for review. flagged: a declared file the runs never opened. resolved: a later run found the profile in agreement. rejected: a review of the code kept the profile as it is."
+    },
+    "reason": {
+      "type": "string",
+      "minLength": 1,
+      "description": "Written by a review (campaign.py review): what the code does, with its file:line."
     },
     "rule": { "type": "string", "minLength": 1 },
     "profile_says": {},

scripts/campaign.py

@@ -27,6 +27,12 @@ as `evidence:`. Any other goes to measurements/findings.jsonl, where the
 site lists it as open. Each pass adds one line to
 measurements/campaign-runs.jsonl.
 
+`review` closes an open finding once a maintainer has read the core's code:
+`applied` writes the fields it names through the same profile editor, with
+the finding's record ids as `evidence:`, and `rejected` keeps the profile.
+The reason, with its file:line, goes on the finding's line, and a rejected
+finding stays closed while the runs keep showing the same thing.
+
 `references` runs the full set once per core and content with its frames
 saved as PNG, and lists the screens that stay on for several samples: the
 candidates a reviewer looks at before writing them into oracle.yml.
@@ -45,6 +51,7 @@ Usage:
     python scripts/campaign.py references --only CORE,... [--system ID]
     python scripts/campaign.py status [--json]
     python scripts/campaign.py findings [--all] [--json]
+    python scripts/campaign.py review KEY --status applied|rejected --reason TEXT [--set FIELD=YAML]
     python scripts/campaign.py gate
     python scripts/campaign.py install-timer [--enable]
 
@@ -1424,6 +1431,12 @@ class Campaign:
                 and previous["run_shows"] == verdict.value and previous["profile_says"] == says \
                 and previous["evidence"] == list(verdict.evidence):
             return
+        # A review that read the code and kept the profile stands while the
+        # runs keep showing the same thing.
+        if previous and previous["status"] == "rejected" and status == "open" \
+                and previous["rule"] == verdict.rule and previous["run_shows"] == verdict.value \
+                and previous["profile_says"] == says:
+            return
         line = mr.seal_line({
             "schema_version": 1, "found_at": now_iso(), "key": key, "emulator": emulator,
             "system": content.system, "file": entry["name"], "field": verdict.field,
@@ -1824,6 +1837,52 @@ def open_findings(root: Path, everything: bool = False) -> list[dict]:
             if everything or line["status"] in ("open", "flagged")]
 
 
+REVIEWED = ("applied", "rejected")
+
+
+def review_finding(root: Path, key: str, status: str, reason: str,
+                   updates: dict[str, object] | None = None, index: int | None = None) -> dict:
+    """Close an open finding after reading the core's code.
+
+    `applied` writes `updates` into the entry the finding names, through the
+    same editor as the rule table, with the finding's record ids added as
+    `evidence:` under its field. An applied finding with no update records a
+    profile changed by hand. `rejected` keeps the profile as it is. Either
+    way a line with the reason is appended to the findings."""
+    if status not in REVIEWED:
+        raise ValueError(f"status {status!r}: one of {', '.join(REVIEWED)}")
+    if not reason.strip():
+        raise ValueError("a review names its reason")
+    latest = {line["key"]: line for line in mr.read_campaign(root, mr.FINDINGS)}
+    finding = latest.get(key)
+    if finding is None:
+        raise ValueError(f"no finding {key}")
+    if finding["status"] not in ("open", "flagged"):
+        raise ValueError(f"{key} is {finding['status']}, not open")
+    if updates and status != "applied":
+        raise ValueError("only an applied review changes the profile")
+    if updates:
+        path = root / "emulators" / f"{finding['emulator']}.yml"
+        entries = yaml.safe_load(path.read_text(encoding="utf-8")).get("files") or []
+        matches = [i for i, entry in enumerate(entries) if entry.get("name") == finding["file"]]
+        if index is not None:
+            matches = [i for i in matches if i == index]
+        if len(matches) != 1:
+            raise ValueError(f"{finding['emulator']}: {len(matches)} entries named {finding['file']}"
+                             + ("" if index is not None else ", pass --index"))
+        entry = entries[matches[0]]
+        verdict = rules.Verdict(finding["rule"], finding["field"], finding["run_shows"],
+                                finding["statement"], tuple(finding["evidence"]))
+        evidence = rules.merge_evidence(entry.get("evidence"), verdict)
+        campaign_profiles.write_fields(path, matches[0], {**updates, "evidence": evidence})
+    body = {key_: value for key_, value in finding.items() if key_ != "id"}
+    line = mr.seal_line({**body, "found_at": now_iso(), "status": status, "reason": reason},
+                        "found_at")
+    with (root / mr.records_path(mr.FINDINGS)).open("a", encoding="utf-8") as handle:
+        handle.write(mr.line_text(line) + "\n")
+    return line
+
+
 # --------------------------------------------------------------------------
 # Command line
 # --------------------------------------------------------------------------
@@ -1866,6 +1925,14 @@ def main(argv: Sequence[str] | None = None) -> int:
     findings_p = sub.add_parser("findings", help="profile claims the runs contradict, open for review")
     findings_p.add_argument("--all", action="store_true", help="applied and resolved too")
     findings_p.add_argument("--json", action="store_true", help="the findings as JSON")
+    review_p = sub.add_parser("review", help="close an open finding after reading the code")
+    review_p.add_argument("key", help="emulator/system/file/field, as `findings` prints it")
+    review_p.add_argument("--status", required=True, choices=REVIEWED)
+    review_p.add_argument("--reason", required=True,
+                          help="what the code does, with its file:line at the profile's pin")
+    review_p.add_argument("--set", action="append", default=[], metavar="FIELD=YAML",
+                          help="with --status applied, a field to write into the entry")
+    review_p.add_argument("--index", type=int, help="the entry's position when several share its name")
     sub.add_parser("gate", help="tests, schemas, verify and pack of the changed profiles")
     timer_p = sub.add_parser("install-timer",
                              help="opt-in: write the autorun units, the campaign job's timer among them")
@@ -1879,6 +1946,21 @@ def main(argv: Sequence[str] | None = None) -> int:
     if args.command == "install-timer":
         command = [sys.executable, str(ROOT / "scripts" / "autorun.py"), "install-timers"]
         return subprocess.run(command + (["--enable"] if args.enable else []), check=False).returncode
+    if args.command == "review":
+        updates: dict[str, object] = {}
+        for item in args.set:
+            name, sep, value = item.partition("=")
+            if not sep or not name:
+                parser.error(f"--set {item!r}: FIELD=YAML")
+            updates[name] = yaml.safe_load(value)
+        try:
+            line = review_finding(paths.root, args.key, args.status, args.reason, updates,
+                                  args.index)
+        except (ValueError, campaign_profiles.ProfileEditError) as exc:
+            print(f"campaign: {exc}", file=sys.stderr)
+            return 1
+        print(f"{line['status']} {line['key']} [{line['id']}]")
+        return 0
     if args.command == "findings":
         lines = open_findings(paths.root, args.all)
         if args.json:

scripts/campaign_profiles.py

@@ -24,6 +24,8 @@ ANCHORS = {
     "path": ("name",),
     "aliases": ("path", "name"),
     "validation": ("sha1", "md5", "crc32", "size", "name"),
+    "on_mismatch": ("validation", "sha1", "md5", "crc32", "size", "name"),
+    "found_by": ("path", "name"),
 }
 
 

scripts/campaign_rules.py

@@ -28,7 +28,9 @@ name (zeros of its size, its first half, another region's dump).
 core reports a BIOS error, or when it boots without the BIOS screens the
 right file shows (it fell back to a built-in BIOS). It is used when it
 turns the run into a hang or a crash that the run without any file does not
-show. Anything else settles nothing. Of validation, the table writes only a
+show. Anything else settles nothing. A check the entry declares with
+`on_mismatch: warn` is one the code only logs: a wrong file that runs
+agrees with it. Of validation, the table writes only a
 missing `size`. A contradicted content check goes to review: which hash a
 core computes is read in its code.
 
@@ -48,7 +50,7 @@ UNKNOWN = "unknown"
 FAILS = (BIOS_ERROR, CRASH, HANG)
 REFUSED = "refused"
 USED = "used"
-CONTENT_CHECKS = ("md5", "sha1", "sha256", "crc32", "adler32")
+CONTENT_CHECKS = ("md5", "sha1", "sha256", "crc32", "adler32", "signature")
 
 # A field an entry leaves out reads as this value.
 FIELD_DEFAULTS: dict[str, object] = {"hle_fallback": False, "path": None, "aliases": [],
@@ -185,6 +187,17 @@ def declared_checks(entry: dict) -> tuple[set[str], bool]:
     return {str(v) for v in value}, True
 
 
+def warned_checks(entry: dict, checks: set[str]) -> set[str]:
+    """The declared checks whose failure the code only logs (`on_mismatch:
+    warn`, for all of them or check by check): a wrong file still runs."""
+    level = entry.get("on_mismatch")
+    if level == "warn":
+        return set(checks)
+    if isinstance(level, dict):
+        return {check for check, value in level.items() if value == "warn"} & checks
+    return set()
+
+
 def rule_validation(zeros: Run | None, half: Run | None, none: Run, present: Run,
                     entry: dict) -> Verdict | None:
     """Same-size and short wrong files against the right one: validation."""
@@ -199,19 +212,20 @@ def rule_validation(zeros: Run | None, half: Run | None, none: Run, present: Run
     if content is None and size is None:
         return None
     declared, editable = declared_checks(entry)
+    enforced = declared - warned_checks(entry, declared)
     evidence = ids(*(run for run in (none, present, zeros, half) if run))
     said = sorted(declared)
     shown = []
     problems = []
     if content is not None:
         shown.append("checks the content" if content else "does not check the content")
-        if content != bool(declared & set(CONTENT_CHECKS)):
+        if content != bool(enforced & set(CONTENT_CHECKS)):
             problems.append("a same-size file of zeros is " + ("refused" if content else "run")
                             + f", the profile declares validation {said}")
     missing_size = size is True and "size" not in declared
     if size is not None:
         shown.append("checks the size" if size else "does not check the size")
-        if size is False and "size" in declared:
+        if size is False and "size" in enforced:
             problems.append(f"the first half of {name} is run, the profile declares a size check")
     statement = f"the core {' and '.join(shown)} of {name}"
     # What the runs show, for a finding beside what the profile says.

tests/test_campaign.py

@@ -222,6 +222,25 @@ class RuleTable(unittest.TestCase):
         self.assertIsNone(rules.rule_validation(run(rules.UNKNOWN), None, NONE_BOOTS, PRESENT,
                                                 {"name": "b"}))
 
+    def test_a_warned_check_lets_a_wrong_file_run(self) -> None:
+        """Beetle PSX logs a wrong SHA1 and boots the file (libretro.c:853-860),
+        PCSX ReARMed refuses an image without the kernel's first instruction
+        or "PS compatible" (frontend/libretro.c:3646-3651)."""
+        refused = run(rules.BIOS_ERROR)
+        used = run(rules.HANG)
+        warned = {"name": "b", "validation": ["sha1"], "on_mismatch": "warn"}
+        self.assertEqual(rules.rule_validation(used, used, NONE_BOOTS, PRESENT, warned).rule,
+                         "validation_agrees")
+        by_check = {"name": "b", "validation": ["size", "md5"], "on_mismatch": {"md5": "warn"}}
+        self.assertEqual(rules.rule_validation(used, refused, NONE_BOOTS, PRESENT, by_check).rule,
+                         "validation_agrees")
+        self.assertEqual(rules.rule_validation(used, None, NONE_BOOTS, PRESENT,
+                                               {"name": "b", "validation": ["sha1"]}).rule,
+                         "validation_contradicted")
+        signature = {"name": "b", "validation": ["size", "signature"]}
+        self.assertEqual(rules.rule_validation(refused, None, NONE_BOOTS, PRESENT, signature).rule,
+                         "validation_agrees")
+
     def test_path_and_alias(self) -> None:
         self.assertIsNone(rules.rule_path(self.ENTRY, "bios.bin", rules.Opened("bios.bin", self.RID)))
         moved = rules.rule_path(self.ENTRY, "bios.bin", rules.Opened("sys/bios.bin", self.RID))
@@ -900,6 +919,53 @@ class Findings(Workspace):
             for line in mr.read_campaign(self.root, name):
                 self.assertEqual(list(Draft202012Validator(schema).iter_errors(line)), [], name)
 
+    def contradicted(self) -> rules.Verdict:
+        return rules.rule_validation(rules.Run(rules.HANG, self.RID, "zeros"), None,
+                                     self.none(rules.BOOTS), self.present(),
+                                     {**self.entry(), "validation": ["md5"]})
+
+    def test_a_review_applies_through_the_profile_editor(self) -> None:
+        self.c.settle("fake", self.content, self.core, 0, self.entry(), [self.contradicted()])
+        key = self.lines()[-1]["key"]
+        with self.assertRaises(ValueError):
+            campaign.review_finding(self.root, key, "applied", " ")
+        line = campaign.review_finding(self.root, key, "applied", "the core logs the md5 (x.c:1)",
+                                       {"on_mismatch": "warn"})
+        self.assertEqual((line["status"], line["reason"]), ("applied", "the core logs the md5 (x.c:1)"))
+        entry = self.entry()
+        self.assertEqual(entry["on_mismatch"], "warn")
+        self.assertEqual(entry["evidence"], {"validation": [self.RID]})
+        text = self.profile.read_text()
+        self.assertLess(text.index("validation: [size]"), text.index("on_mismatch"))
+        self.assertEqual(campaign.open_findings(self.root), [])
+        with self.assertRaises(ValueError):
+            campaign.review_finding(self.root, key, "rejected", "again")
+        try:
+            from jsonschema import Draft202012Validator
+        except ImportError:
+            self.skipTest("jsonschema is a tooling dependency")
+        schema = json.loads((REPO_ROOT / "schemas" / "campaign-finding.schema.json").read_text())
+        for found in self.lines():
+            self.assertEqual(list(Draft202012Validator(schema).iter_errors(found)), [])
+
+    def test_a_rejected_finding_stays_closed_on_the_same_run(self) -> None:
+        verdict = self.contradicted()
+        self.c.settle("fake", self.content, self.core, 0, self.entry(), [verdict])
+        key = self.lines()[-1]["key"]
+        with self.assertRaises(ValueError):
+            campaign.review_finding(self.root, key, "rejected", "kept", {"required": False})
+        out = io.StringIO()
+        with contextlib.redirect_stdout(out), unittest.mock.patch.object(campaign.Paths, "default",
+                                                                return_value=self.paths):
+            self.assertEqual(campaign.main(["review", key, "--status", "rejected",
+                                            "--reason", "the md5 is checked (x.c:2)"]), 0)
+        before = self.profile.read_text()
+        later = rules.Verdict(verdict.rule, verdict.field, verdict.value, verdict.statement,
+                              ("2026-10-28-000000000000",))
+        self.c.settle("fake", self.content, self.core, 0, self.entry(), [later])
+        self.assertEqual([line["status"] for line in self.lines()], ["open", "rejected"])
+        self.assertEqual(self.profile.read_text(), before)
+
     def test_the_batch_commit_plan(self) -> None:
         verdicts = rules.rule_need(self.none(rules.BOOTS), self.present(), self.entry())
         self.c.settle("fake", self.content, self.core, 0, self.entry(), verdicts)