From dbaaa86d5be8efa07153a95de483a8cd84d23022 Mon Sep 17 00:00:00 2001 From: Marius Ciepluch <11855163+norandom@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:03:23 +0200 Subject: [PATCH] feat(hunt): contamination check (4.5): runs that used a reported repository are marked and kept out of any gain contamination.py reads the repositories named by the held-out pilot manifest and by every benchmark report (accepted_repos.json and NOTES.md links under benchmarks/measurements), marks each run directory clean or contaminated with the report digest and time, and refuses to count an unchecked run toward a gain. ablation.analyse carries excluded_from_gain when its run is contaminated. Implemented by Codex (gpt-6-astra), gated here; the benchmark import is lazy so the package imports without the benchmarks tree. Co-Authored-By: Claude Fable 5.1 --- .kiro/specs/hunter-learning-loop/tasks.md | 2 +- src/openultrasast/hunt/ablation.py | 11 +- src/openultrasast/hunt/contamination.py | 149 ++++++++++++++++++++ tests/test_hunt_contamination.py | 159 ++++++++++++++++++++++ 4 files changed, 319 insertions(+), 2 deletions(-) create mode 100644 src/openultrasast/hunt/contamination.py create mode 100644 tests/test_hunt_contamination.py diff --git a/.kiro/specs/hunter-learning-loop/tasks.md b/.kiro/specs/hunter-learning-loop/tasks.md index 87c08a3..047e924 100644 --- a/.kiro/specs/hunter-learning-loop/tasks.md +++ b/.kiro/specs/hunter-learning-loop/tasks.md @@ -72,7 +72,7 @@ - For every annotation, its effect when removed from the full set and when added to the baseline, with intervals and counts; an annotation stays only on a held-out effect whose interval excludes zero or when a kept annotation needs it; one that raises suspicions without raising recall is dropped and recorded - Done when the table is written for a fixture ablation and the rule's decisions match the fixture's expectations - _Requirements: 4.4, 4.5, 4.6_ -- [ ] 4.5 Contamination check +- [x] 4.5 Contamination check - A training repository later found in the pilot or in a benchmark report marks every run that used it as contaminated and excludes those runs from any gain - Done when a fixture run is marked contaminated after its repository is added to a report list - _Requirements: 2.7_ diff --git a/src/openultrasast/hunt/ablation.py b/src/openultrasast/hunt/ablation.py index f7b0b3f..07e4fc6 100644 --- a/src/openultrasast/hunt/ablation.py +++ b/src/openultrasast/hunt/ablation.py @@ -394,10 +394,19 @@ def analyse(out_dir: Path | str, manifest: Manifest, changes: Sequence[Mapping[s for arm, mask in manifest.arms.items() } result["dry_run"] = sum(c["kind"] == "vulnerability" for c in changes) < 40 + contamination_path = out / "contamination.json" + if contamination_path.exists(): + contamination = json.loads(contamination_path.read_text(encoding="utf-8")) + result["contaminated"] = bool(contamination["contaminated"]) + result["contaminating_repositories"] = contamination["repositories"] + if result["contaminated"]: + result["excluded_from_gain"] = True _write(out / "analysis.json", result) table = importance(result, annotations=[name for name, _, _ in INPUTS]) _write(out / "importance.json", json.loads(table.to_json(orient="records"))) - decide(table, needs={name: set(deps) for name, deps in manifest.rule.get("needs", {}).items()}) + decisions = decide(table, needs={name: set(deps) for name, deps in manifest.rule.get("needs", {}).items()}) + if result.get("excluded_from_gain"): + _write(out / "decisions.json", {**decisions, "excluded_from_gain": True}) return result diff --git a/src/openultrasast/hunt/contamination.py b/src/openultrasast/hunt/contamination.py new file mode 100644 index 0000000..1c56e6f --- /dev/null +++ b/src/openultrasast/hunt/contamination.py @@ -0,0 +1,149 @@ +"""Mark runs that used a reported repository and keep them out of gain reports.""" + +from __future__ import annotations + +import hashlib +import json +import tomllib +from collections.abc import Iterable +from datetime import UTC, datetime +from pathlib import Path + + +def _eligibility(): + """The benchmark's name normaliser and GitHub URL pattern, imported late so the package imports without the benchmarks tree.""" + from benchmarks.unseen.eligibility import URL, normalize + + return URL, normalize + + +def report_repositories(root: Path) -> set[str]: + """Read repository identities from the pilot and benchmark reports under root. + + The pilot's repository entries contribute their id, repository and URL fields. + Each measurements/*/accepted_repos.json is a list of names. GitHub links in + measurements/*/NOTES.md also count. Missing sources contribute nothing, while + unreadable or malformed files raise. Advisory and label fields are not inspected. + """ + URL, normalize = _eligibility() + root = Path(root) + if not root.is_dir(): + raise FileNotFoundError(root) + names = set() + pilot = root / "benchmarks/valid/pilot/pool-valid.toml" + if pilot.exists(): + data = tomllib.loads(pilot.read_text(encoding="utf-8")) + for entry in data.get("repository", []): + for key in ("id", "repository", "url"): + if key in entry: + names.add(normalize(entry[key])) + measurements = root / "benchmarks/measurements" + for path in sorted(measurements.glob("*/accepted_repos.json")): + entries = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(entries, list) or any(not isinstance(entry, str) for entry in entries): + raise ValueError(f"repository report must be a list of names: {path}") + names.update(normalize(entry) for entry in entries) + for path in sorted(measurements.glob("*/NOTES.md")): + names.update(normalize(match[1].rstrip(".")) for match in URL.finditer(path.read_text(encoding="utf-8"))) + return names + + +def _records(run_dir: Path) -> list[dict]: + """Read JSON objects with a change id, skipping artifacts and other metadata.""" + records = [] + for path in sorted(run_dir.glob("*.json")): + value = json.loads(path.read_text(encoding="utf-8")) + if isinstance(value, dict) and "id" in value: + records.append(value) + return records + + +def run_repositories(run_dir: Path) -> set[str]: + """Read study.json changes' repo fields, or resolve each record's repository. + + A record's repo or repository field takes precedence. The current runner writes + neither, so its record id is joined to inputs.json item_id and change.repo is + read there. Inputs without a record do not count. No advisory or label fields + are inspected. Raise if any change lacks a repository or no repositories exist. + """ + _, normalize = _eligibility() + run_dir = Path(run_dir) + study = run_dir / "study.json" + names = set() + if study.exists(): + for change in json.loads(study.read_text(encoding="utf-8"))["changes"]: + if not change.get("repo"): + raise ValueError(f"missing repository in study: {run_dir}") + names.add(normalize(change["repo"])) + else: + inputs = None + for record in _records(run_dir): + repo = record.get("repo") or record.get("repository") + if not repo: + if inputs is None: + path = run_dir / "inputs.json" + entries = json.loads(path.read_text(encoding="utf-8")) if path.exists() else [] + inputs = {entry["item_id"]: entry["change"] for entry in entries} + repo = inputs.get(record["id"], {}).get("repo") + if not repo: + raise ValueError(f"missing repository for record {record['id']} in {run_dir}") + names.add(normalize(repo)) + if not names: + raise ValueError(f"no repository named by run: {run_dir}") + return names + + +def check(run_dir: Path, reports: set[str]) -> dict: + """Atomically save the overlap and check time, including when the run is clean. + + The digest is SHA256 of sorted, normalized report names joined by newlines, + with no trailing newline. Repository resolution must succeed before writing. + """ + _, normalize = _eligibility() + reports = {normalize(name) for name in reports} + overlap = sorted(run_repositories(run_dir) & reports) + result = { + "contaminated": bool(overlap), + "repositories": overlap, + "reports_digest": hashlib.sha256("\n".join(sorted(reports)).encode("utf-8")).hexdigest(), + "checked_at": datetime.now(UTC).isoformat(), + } + path = Path(run_dir) / "contamination.json" + temporary = path.with_suffix(path.suffix + ".partial") + temporary.write_text(json.dumps(result, sort_keys=True) + "\n", encoding="utf-8") + temporary.replace(path) + return result + + +def mark_all(runs_root: Path, reports: set[str]) -> dict[str, dict]: + """Check each immediate child directory containing study.json or change records. + + Return checks keyed by directory name. Unresolvable repositories raise instead + of producing a clean check. Directories containing only metadata are skipped. + """ + results = {} + for path in sorted(Path(runs_root).iterdir()): + if path.is_dir() and ((path / "study.json").is_file() or _records(path)): + results[path.name] = check(path, reports) + return results + + +def clean_runs(run_dirs: Iterable[Path]) -> list[Path]: + """Return checked, uncontaminated runs in input order. + + Raise ValueError listing all unchecked paths if any contamination.json is + absent. A missing or non-boolean contaminated value also raises; only an + explicit false value qualifies a run for reporting gain. + """ + run_dirs = [Path(path) for path in run_dirs] + unchecked = [str(path) for path in run_dirs if not (path / "contamination.json").is_file()] + if unchecked: + raise ValueError("unchecked runs: " + ", ".join(unchecked)) + clean = [] + for path in run_dirs: + result = json.loads((path / "contamination.json").read_text(encoding="utf-8")) + if not isinstance(result, dict) or type(result.get("contaminated")) is not bool: + raise ValueError(f"invalid contamination check: {path}") + if not result["contaminated"]: + clean.append(path) + return clean diff --git a/tests/test_hunt_contamination.py b/tests/test_hunt_contamination.py new file mode 100644 index 0000000..8d86652 --- /dev/null +++ b/tests/test_hunt_contamination.py @@ -0,0 +1,159 @@ +"""A report added later must exclude every run that used its repositories.""" + +from __future__ import annotations + +import hashlib +import json +from dataclasses import replace +from datetime import datetime + +import pytest + +pytest.importorskip("dspy") +pytest.importorskip("pandas") +from test_hunt_ablation import finding, write_records # noqa: E402 +from test_hunt_ablation import population as population # noqa: E402 + +from openultrasast.hunt import ablation as ab # noqa: E402 +from openultrasast.hunt import contamination as ct # noqa: E402 +from openultrasast.hunt import runner # noqa: E402 + + +def write_json(path, value): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(value), encoding="utf-8") + + +def test_report_repositories_reads_all_sources_and_normalizes(tmp_path): + pilot = tmp_path / "benchmarks/valid/pilot/pool-valid.toml" + pilot.parent.mkdir(parents=True) + pilot.write_text( + '[[repository]]\nid = "Owner/Pilot-ID"\nrepository = "Owner/Pilot-Name.git"\n' + 'url = "https://github.com/Owner/Pilot-URL.git"\n' + 'advisory = "https://github.com/ignore/advisory"\nlabel = "https://github.com/ignore/label"\n' + ) + measurements = tmp_path / "benchmarks/measurements" + write_json(measurements / "first/accepted_repos.json", ["Owner/Accepted.git", "https://github.com/Owner/Accepted/"]) + write_json(measurements / "second/accepted_repos.json", ["Other/Accepted"]) + (measurements / "first/NOTES.md").write_text("[source](https://github.com/Owner/Noted.git/tree/main) and github.com/Other/Plain.\n") + (measurements / "second/NOTES.md").write_text("See https://GITHUB.COM/Owner/Second#readme\n") + write_json(measurements / "first/labels.json", {"repo": "ignore/labels"}) + assert ct.report_repositories(tmp_path) == { + "owner/pilot-id", + "owner/pilot-name", + "owner/pilot-url", + "owner/accepted", + "other/accepted", + "owner/noted", + "other/plain", + "owner/second", + } + + +def record_run(path, repo="Owner/Training.git"): + for change_id in ("first", "second"): + write_json(runner.record_path(path, change_id), {"id": change_id, "done": True, "push": {}}) + write_json( + path / "inputs.json", + [ + {"item_id": change_id, "change": {"repo": repo, "label": "ignore/label", "advisory": "ignore/advisory"}} + for change_id in ("first", "second") + ] + + [{"item_id": "not-run", "change": {"repo": "unused/input"}}], + ) + return path + + +def test_run_becomes_contaminated_after_report_is_added(tmp_path): + run_dir = record_run(tmp_path / "run") + accepted = tmp_path / "benchmarks/measurements/new/accepted_repos.json" + write_json(accepted, ["Other/Repo", "Another/Repo"]) + assert ct.run_repositories(run_dir) == {"owner/training"} + clean = ct.check(run_dir, ct.report_repositories(tmp_path)) + assert clean["contaminated"] is False and clean["repositories"] == [] + assert json.loads((run_dir / "contamination.json").read_text()) == clean + assert clean["reports_digest"] == hashlib.sha256(b"another/repo\nother/repo").hexdigest() + assert datetime.fromisoformat(clean["checked_at"]).tzinfo is not None + assert ct.clean_runs([run_dir]) == [run_dir] + + write_json(accepted, ["Other/Repo", "https://github.com/OWNER/Training.git"]) + contaminated = ct.check(run_dir, ct.report_repositories(tmp_path)) + assert contaminated["contaminated"] is True + assert contaminated["repositories"] == ["owner/training"] + assert contaminated["reports_digest"] != clean["reports_digest"] + assert json.loads((run_dir / "contamination.json").read_text()) == contaminated + assert not (run_dir / "contamination.json.partial").exists() + assert ct.clean_runs([run_dir]) == [] + + +def test_run_repositories_uses_direct_fields_and_study_first(tmp_path): + write_json(tmp_path / "record.json", {"id": "change", "repo": "Owner/Direct.git"}) + write_json(tmp_path / "inputs.json", [{"item_id": "change", "change": {"repo": "unused/fallback"}}]) + assert ct.run_repositories(tmp_path) == {"owner/direct"} + write_json(tmp_path / "study.json", {"changes": [{"id": "change", "repo": "https://github.com/Owner/Study.git"}]}) + assert ct.run_repositories(tmp_path) == {"owner/study"} + + +@pytest.mark.parametrize("contents", [{}, {"id": "unresolved"}]) +def test_run_repositories_refuses_missing_repository(tmp_path, contents): + write_json(tmp_path / "record.json", contents) + with pytest.raises(ValueError, match="repository"): + ct.check(tmp_path, set()) + assert not (tmp_path / "contamination.json").exists() + + +def test_run_repositories_refuses_partly_resolved_run(tmp_path): + record_run(tmp_path) + write_json(tmp_path / "unresolved.json", {"id": "missing"}) + with pytest.raises(ValueError, match="missing"): + ct.run_repositories(tmp_path) + + +def test_mark_all_checks_records_and_studies_but_not_metadata(tmp_path): + record_run(tmp_path / "records") + write_json(tmp_path / "study/study.json", {"changes": [{"repo": "OWNER/Training"}]}) + write_json(tmp_path / "other/study.json", {"changes": [{"repo": "Other/Repo"}]}) + write_json(tmp_path / "metadata/artifact.json", {"artifact_id": "artifact"}) + result = ct.mark_all(tmp_path, {"owner/training"}) + assert set(result) == {"records", "study", "other"} + assert result["records"]["contaminated"] is result["study"]["contaminated"] is True + assert result["other"]["contaminated"] is False + assert not (tmp_path / "metadata/contamination.json").exists() + + +def test_clean_runs_refuses_all_unchecked_names(tmp_path): + clean = record_run(tmp_path / "clean") + ct.check(clean, set()) + with pytest.raises(ValueError, match="unchecked.*first.*second"): + ct.clean_runs([clean, tmp_path / "first", tmp_path / "second"]) + + +@pytest.mark.parametrize("value", [None, 0, "false"]) +def test_clean_runs_refuses_invalid_check(tmp_path, value): + write_json(tmp_path / "contamination.json", {"contaminated": value}) + with pytest.raises(ValueError, match="contamination"): + ct.clean_runs([tmp_path]) + + +def test_ablation_preserves_analysis_but_excludes_contaminated_gain(population, tmp_path): + _, _, changes, _, original = population + manifest = replace(original, sample={**original.sample, "repeats": 1}) + out = tmp_path / "ablation" + write_records(out, manifest, changes, {"full": [[finding()]] * len(changes), "without-roles": [[]] * len(changes)}) + write_json(out / "study.json", {"manifest_id": ab.manifest_id(manifest), "changes": changes}) + ct.check(out, set()) + clean = ab.analyse(out, manifest, changes) + assert clean["contaminated"] is False and clean["contaminating_repositories"] == [] + assert not clean.get("excluded_from_gain", False) + importance = json.loads((out / "importance.json").read_text()) + decisions = json.loads((out / "decisions.json").read_text()) + + write_json(tmp_path / "benchmarks/measurements/later/accepted_repos.json", [changes[0]["repo"]]) + ct.check(out, ct.report_repositories(tmp_path)) + result = ab.analyse(out, manifest, changes) + assert result["contaminated"] is True and result["excluded_from_gain"] is True + assert result["contaminating_repositories"] == [changes[0]["repo"]] + assert result["heldout"] == clean["heldout"] + assert json.loads((out / "analysis.json").read_text())["excluded_from_gain"] is True + assert json.loads((out / "importance.json").read_text()) == importance + assert json.loads((out / "decisions.json").read_text()) == {**decisions, "excluded_from_gain": True}