diff --git a/README.md b/README.md index 48b9a9e..b7dcb9a 100644 --- a/README.md +++ b/README.md @@ -262,6 +262,7 @@ uphold_check.py --list # every id in the catalog uphold_check.py --init # a starter declaration uphold_check.py --oscal # OSCAL component-definition JSON uphold_check.py --review # what routes to the review tier +uphold_check.py --review --drive -- CMD # score a reviewer against the controls ``` ## The four seams diff --git a/ROADMAP.md b/ROADMAP.md index 49f388b..641e8fb 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -10,8 +10,8 @@ and labels, and this page cites no issue by number. record is `seed`. - State `enforcement.rung` on the machine-observable records that predate the field, so the by-rung index in `QUICK_REFERENCE.md` covers the whole catalog. -- Add review controls: one record carries a control, and the run names every - review-carried record that has none. +- Add a control to every review-carried record that has none; `--review` names + them, and `--review --drive` scores a reviewer against the ones that exist. - Fill the kinds no record uses yet only with candidates a rule can hold; a method, representation or metric belongs in the coverage page's rung glossary unless a consumer can claim it. diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 227d573..1ca39b1 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -105,12 +105,22 @@ Controls share probe's weakness: whoever wrote the record also writes its control, and can make it easy to pass. The mitigation is also the same: the control lives in `policy/upheld.toml`, where a diff shows it. -Driving an actual reviewer against a control needs a model, a budget and a -verdict nothing here can make deterministic, so the harness is out of scope and -`[review] emit_controls` is the seam to it. It exports the controls and nothing -from the catalog: a harness also handed the claim and the questions would carry -a second copy of the compiled document, going stale on its own schedule, which -is the failure `single-authoritative-source` names. +A reviewer needs a model, a budget and a verdict nothing here can make +deterministic, so the reviewer is a command the caller names and nothing here +holds a model client. `--review --drive` is the harness around it: it hands the +command the compiled document and one control's change, never the record the +change was planted for, and scores the ids it prints against the answer key. It +runs every control k times on each side and reports pass@1 beside pass^k, +because a reviewer that catches a plant one run in three scores a third on the +first and zero on the second, and only the second is a control. A reviewer that +exits non-zero or names something outside the document is exit `2`, not a miss: +it has said nothing about the record. + +`[review] emit_controls` is the same answer key as a file, for a harness that +would rather read one. It exports the controls and nothing from the catalog: a +harness also handed the claim and the questions would carry a second copy of the +compiled document, going stale on its own schedule, which is the failure +`single-authoritative-source` names. ### Why `emit` is still per-document and not per-record diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index bae7cff..440a574 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -4270,7 +4270,36 @@ emit_controls = ["review-controls.json"] It carries `record`, `catches`, `misses` and the uncontrolled list, and nothing from the catalog: a harness also handed the claim and the questions would be -carrying a second copy of the compiled document. Driving a reviewer against a -control needs a model, a budget and a verdict nothing here can make -deterministic, so the harness itself is outside this repository. See -[DESIGN.md](DESIGN.md#a-control-is-the-probe-fixture-one-tier-up). +carrying a second copy of the compiled document. + +`--drive` plants the controls itself, against a reviewer command you name: + +```sh +uphold_check.py --review --drive [--runs N] [--timeout SECONDS] -- ./my-reviewer --model x +``` + +Everything after `--` is the reviewer, run once per plant. It reads one JSON +object on stdin, `{"document": ..., "change": ...}`, where `document` is the +compiled review and `change` is one control's `catches` or `misses`. It prints +one line per record the change violates, the record's id or its title as the +document spells it, in any case; blank lines are skipped, and no lines means no +violation. It is never told which record was planted. + +Each control runs `N` times on each side (default 3): + +```text +single-authoritative-source: caught 3/3, misses fired 0/3 -- holds +pass@1 1.00; pass^3 1/1 control(s) +false positives 0.00 over 1 control(s) with a misses +``` + +A control holds when every run named the record against `catches` and none +named it against `misses`. pass@1 is the mean share of runs that caught their +plant, and pass^k the share of controls caught on all `N`. The exit is `0` when +every control holds and `1` when one does not. A reviewer that exits non-zero, +cannot be run, or prints a line that names no record in the document is exit +`2`: it has said nothing about the record, so it is not scored as a miss. So +is one that has not answered within `--timeout` seconds (default 300); a person +answering at a prompt passes a longer one. No +model client lives here; the reviewer command carries the model, the key and +the budget. See [DESIGN.md](DESIGN.md#a-control-is-the-probe-fixture-one-tier-up). diff --git a/policy/upheld.toml b/policy/upheld.toml index 6e3fe77..7cf4be0 100644 --- a/policy/upheld.toml +++ b/policy/upheld.toml @@ -221,11 +221,11 @@ max_lines = 900 # cannot be committed. AGENTS.md is where an agent looks; REVIEW.md is where a # person does. emit = ["REVIEW.md", "AGENTS.md"] -# Where the controls below are written for a harness to read. The harness that -# drives a reviewer against them is not in this repository and will not be: it -# needs a model, a budget and a verdict nobody can make deterministic. This file -# is the seam to it, and it is generated under the same `--check` gate as the -# document, so a control added here and never exported is refused. +# Where the controls below are written for a harness to read. `--review --drive` +# plants them itself against a reviewer command the caller names; the model, the +# budget and the verdict stay with that command and out of this repository. The +# file is generated under the same `--check` gate as the document, so a control +# added here and never exported is refused. emit_controls = ["review-controls.json"] [review.no_subject_here] @@ -241,11 +241,26 @@ acyclic-dependencies = "No gate here decides a module graph. uphold is one crate # whoever wrote the record is the only reader who knows what violating it looks # like, and a fixture in a file is reviewable. # -# One control, deliberately, and the run says so on every invocation: it prints -# how many review-carried records have none, and names them. That line, not a -# count copied here, is the honest reading of what has been demonstrated, and it -# is what the next control should be chosen against. +# Most records still have none, and the run says so on every invocation: it +# prints how many review-carried records have no control, and names them. That +# line, not a count copied here, is the honest reading of what has been +# demonstrated, and it is what the next control should be chosen against. +# `--review --drive -- REVIEWER` plants each one and reports whether it holds. +# +# A `misses` is chosen to sit next to its `catches`: the same file, the same +# path, a change that touches what the record is about without violating it. A +# reviewer that names the record on both is matching words, not the claim. [[review.control]] record = "single-authoritative-source" catches = "Add a hand-written docs/PRINCIPLES.md that restates each record's claim and applies_when, alongside the generated REVIEW.md that already compiles them from principles/*.toml." misses = "Change scripts/review.py's render() so the compiled document prints applies_when before the claim." + +[[review.control]] +record = "fail-fast" +catches = "Change uphold_check.py's read_toml() so a policy/upheld.toml that tomllib cannot parse is treated as an empty declaration, and `--review --check` goes on to print \"compiled review is current\" and exit 0 instead of exiting 2 with the parse error." +misses = "Change the message uphold_check.py prints when policy/upheld.toml cannot be parsed so it carries the line and column tomllib reported, still exiting 2." + +[[review.control]] +record = "least-astonishment" +catches = "Make `uphold_check.py --review --check` rewrite REVIEW.md and AGENTS.md in place when they are stale and then exit 0, instead of reporting them and exiting 1." +misses = "Make `uphold_check.py --review --emit` print the absolute path of each file it writes, beside the name and line count it already prints." diff --git a/review-controls.json b/review-controls.json index fb1fa51..fb0bfbe 100644 --- a/review-controls.json +++ b/review-controls.json @@ -1,5 +1,15 @@ { "controls": [ + { + "record": "fail-fast", + "catches": "Change uphold_check.py's read_toml() so a policy/upheld.toml that tomllib cannot parse is treated as an empty declaration, and `--review --check` goes on to print \"compiled review is current\" and exit 0 instead of exiting 2 with the parse error.", + "misses": "Change the message uphold_check.py prints when policy/upheld.toml cannot be parsed so it carries the line and column tomllib reported, still exiting 2." + }, + { + "record": "least-astonishment", + "catches": "Make `uphold_check.py --review --check` rewrite REVIEW.md and AGENTS.md in place when they are stale and then exit 0, instead of reporting them and exiting 1.", + "misses": "Make `uphold_check.py --review --emit` print the absolute path of each file it writes, beside the name and line count it already prints." + }, { "record": "single-authoritative-source", "catches": "Add a hand-written docs/PRINCIPLES.md that restates each record's claim and applies_when, alongside the generated REVIEW.md that already compiles them from principles/*.toml.", @@ -14,7 +24,6 @@ "economy-of-mechanism", "end-to-end-principle", "enforcement-needs-a-trigger", - "fail-fast", "follow-published-standards", "goodharts-law", "graceful-degradation", @@ -22,7 +31,6 @@ "hyrums-law", "information-hiding", "informed-consent", - "least-astonishment", "least-common-mechanism", "least-power", "make-illegal-states-unrepresentable", diff --git a/scripts/review.py b/scripts/review.py index 85a6e52..17bb3f1 100644 --- a/scripts/review.py +++ b/scripts/review.py @@ -34,17 +34,30 @@ report anybody reads, for as long as nobody plants what the record is supposed to catch. `uphold probe` exists because a git hook has that defect. A control is the same fixture one tier up, and `audit_controls` below is what reads it. +`drive` is what plants it: it hands a reviewer the compiled document and each +control's change, and scores what comes back against the answer key. """ from __future__ import annotations import json +import subprocess +from collections.abc import Callable #: Below the bottom of the range in which review quality was observed to #: degrade. A limit set at the top of a range is a limit that reaches the #: failure it was measured against. DEFAULT_MAX_LINES = 900 +#: A reviewer that catches a plant one time in three is not a control, and one +#: run cannot tell it from a reviewer that catches it every time. +DEFAULT_RUNS = 3 + +#: Seconds one reviewer invocation may take. A model call answers in seconds; +#: a reviewer that has not answered in five minutes has stalled, and waiting on +#: it holds every control after it. A person at a prompt passes a longer one. +DEFAULT_TIMEOUT = 300 + #: The standing asymmetry, and it points one way on purpose: a static rule has #: no length ceiling and a prompt rule does, so every mechanizable rule is #: cheaper to push down into a static tier than to leave here. @@ -235,9 +248,9 @@ def audit_controls( def render_controls(declared: list[dict], uncontrolled: list[str]) -> str: """The controls as JSON, for a harness this repository does not contain. - Running an LLM reviewer against a control needs a model, a budget and a - verdict nobody here can make deterministic, so the harness is out of scope - and the export is the seam to it. JSON rather than the TOML it was written + `drive` below runs a reviewer against the controls in this process; the + export is for a harness that would rather read a file, such as one that + batches runs or records them somewhere this repository does not. JSON rather than the TOML it was written in because the reader is a program: TOML is the form a person writes a fixture in, and `name-index.json` and `policy/base/sets.lock.json` are already the form a program reads one out of. @@ -296,3 +309,148 @@ def over_budget(document: str, max_lines: int) -> str | None: f"ceiling: it is the half of `enforcement-needs-a-trigger`'s carve-out " f"that keeps this tier from becoming prose emitted always." ) + + +class ReviewerFailed(Exception): + """The reviewer command did not answer in the protocol `ask` reads. + + Not a missed plant: a reviewer that exits non-zero or names something that + is not in the document has said nothing about the record, and scoring it as + a miss would report a live record as dead. + """ + + +def ask( + reviewer: list[str], + document: str, + change: str, + carried: dict[str, str], + timeout: int = DEFAULT_TIMEOUT, +) -> set[str]: + """Run the reviewer once and return the record ids it names. + + The reviewer reads one JSON object on stdin, `{"document": ..., "change": + ...}`, and prints one line per record the change violates: the record's + id, or its title as the document's heading spells it. Blank lines are + skipped, and printing nothing says the change violates nothing. It is never told + which record the change was planted for -- that is the answer key. + + The reviewer is a command named by the caller, so no model client, key or + budget lives here, and any reviewer -- a model behind a wrapper script, a + person at a prompt, a fixture in a test -- answers the same way. + """ + by_title = {title.casefold(): record_id for record_id, title in carried.items()} + payload = json.dumps({"document": document, "change": change}, ensure_ascii=False) + try: + completed = subprocess.run( + reviewer, + input=payload, + capture_output=True, + encoding="utf-8", + timeout=timeout, + check=False, + ) + except OSError as error: + raise ReviewerFailed(f"could not run {reviewer[0]!r}: {error}") from error + except subprocess.TimeoutExpired as error: + raise ReviewerFailed( + f"{reviewer[0]!r} did not answer within {timeout}s; pass --timeout " + f"for a reviewer that needs longer" + ) from error + except UnicodeDecodeError as error: + raise ReviewerFailed( + f"{reviewer[0]!r} printed output that is not UTF-8: {error}" + ) from error + if completed.returncode != 0: + tail = completed.stderr.strip().splitlines()[-5:] + raise ReviewerFailed( + f"{reviewer[0]!r} exited {completed.returncode}" + + (": " + " | ".join(tail) if tail else "") + ) + named: set[str] = set() + for line in completed.stdout.splitlines(): + finding = line.strip() + if not finding: + continue + record_id = finding if finding in carried else by_title.get(finding.casefold()) + if record_id is None: + raise ReviewerFailed( + f"{reviewer[0]!r} named {finding!r}, which is neither the id nor " + f"the title of a record in the compiled document. Print one id or " + f"title per line and nothing else." + ) + named.add(record_id) + return named + + +def drive( + declared: list[dict], + document: str, + carried: dict[str, str], + runs: int, + reviewer: Callable[[str, str, dict[str, str]], set[str]], +) -> list[dict]: + """Plant every control `runs` times on each side and count what fired. + + `caught` is how many runs named the record against `catches`; `fired` is + how many named it against `misses`, or None where no `misses` was written, + because an unmeasured false-positive rate is not a rate of zero. + """ + results = [] + for control in sorted(declared, key=lambda control: control["record"]): + record_id = control["record"] + caught = sum( + record_id in reviewer(document, control["catches"], carried) + for _ in range(runs) + ) + misses = control.get("misses") + fired = ( + None + if misses is None + else sum( + record_id in reviewer(document, misses, carried) for _ in range(runs) + ) + ) + results.append({"record": record_id, "caught": caught, "fired": fired}) + return results + + +def holds(result: dict, runs: int) -> bool: + """pass^k on the plant, and quiet on every run against the miss.""" + return result["caught"] == runs and not result["fired"] + + +def render_drive(results: list[dict], runs: int) -> str: + """One line per control, then pass@1 and pass^k over all of them. + + pass@1 is the mean share of runs that caught the plant: what one review + would be expected to do. pass^k is the share of controls caught on every + one of the k runs, which is what a control has to be before anyone leans on + it. Both are printed because a reviewer can score well on the first and + still be a coin toss on the second. + """ + lines = [] + for result in results: + fired = result["fired"] + quiet = ( + "misses not written" if fired is None else f"misses fired {fired}/{runs}" + ) + verdict = "holds" if holds(result, runs) else "DOES NOT HOLD" + lines.append( + f"{result['record']}: caught {result['caught']}/{runs}, {quiet} -- {verdict}" + ) + if results: + pass_at_1 = sum(result["caught"] for result in results) / (len(results) * runs) + pass_k = sum(result["caught"] == runs for result in results) + lines.append( + f"pass@1 {pass_at_1:.2f}; pass^{runs} {pass_k}/{len(results)} control(s)" + ) + measured = [ + result["fired"] for result in results if result["fired"] is not None + ] + if measured: + rate = sum(measured) / (len(measured) * runs) + lines.append( + f"false positives {rate:.2f} over {len(measured)} control(s) with a misses" + ) + return "\n".join(lines) + "\n" diff --git a/tests/test_review.py b/tests/test_review.py index ae1fa6f..eca2cb7 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -8,12 +8,14 @@ from __future__ import annotations +import shutil import subprocess import sys import tempfile import textwrap import unittest from pathlib import Path +from typing import ClassVar ROOT = Path(__file__).resolve().parents[1] SCRIPT = ROOT / "uphold_check.py" @@ -276,6 +278,121 @@ def test_the_denominator_names_the_records_and_not_only_the_count(self): self.assertIsNone(review_mod.uncontrolled_note([])) +class Drive(unittest.TestCase): + """Planting a control and scoring what the reviewer names. + + The reviewer is a command, so these drive small Python scripts that answer + in its protocol. What is tested is the scoring and the refusals: a reviewer + that broke the protocol has said nothing about the record, and must not be + scored as having missed it. + """ + + CARRIED: ClassVar[dict[str, str]] = { + "single-authoritative-source": "Single Authoritative Source" + } + + def reviewer(self, body: str) -> list[str]: + tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmp) + script = Path(tmp) / "reviewer.py" + script.write_text( + "import json, sys\npayload = json.load(sys.stdin)\n" + + textwrap.dedent(body), + encoding="utf-8", + ) + return [sys.executable, str(script)] + + def test_a_finding_may_be_the_id_or_the_title_in_any_case(self): + reviewer = self.reviewer( + """ + print("single-authoritative-source") + print() + print("single AUTHORITATIVE source") + """ + ) + named = review_mod.ask(reviewer, "doc", "change", self.CARRIED) + self.assertEqual(named, {"single-authoritative-source"}) + + def test_the_reviewer_is_handed_the_document_and_the_change_and_not_the_key(self): + reviewer = self.reviewer( + """ + assert sorted(payload) == ["change", "document"], sorted(payload) + assert payload == {"document": "doc", "change": "plant"}, payload + """ + ) + self.assertEqual(review_mod.ask(reviewer, "doc", "plant", self.CARRIED), set()) + + def test_a_name_outside_the_document_is_refused_rather_than_scored(self): + reviewer = self.reviewer('print("Single Authoritative Source.")') + with self.assertRaises(review_mod.ReviewerFailed) as raised: + review_mod.ask(reviewer, "doc", "change", self.CARRIED) + self.assertIn("'Single Authoritative Source.'", str(raised.exception)) + + def test_a_reviewer_that_exits_non_zero_is_refused_with_its_stderr(self): + reviewer = self.reviewer('sys.exit("no API key")') + with self.assertRaises(review_mod.ReviewerFailed) as raised: + review_mod.ask(reviewer, "doc", "change", self.CARRIED) + self.assertIn("exited 1", str(raised.exception)) + self.assertIn("no API key", str(raised.exception)) + + def test_a_reviewer_that_stalls_is_refused_at_the_deadline(self): + reviewer = self.reviewer("import time; time.sleep(30)") + with self.assertRaises(review_mod.ReviewerFailed) as raised: + review_mod.ask(reviewer, "doc", "change", self.CARRIED, timeout=1) + self.assertIn("did not answer within 1s", str(raised.exception)) + + def test_output_that_is_not_utf8_is_refused_rather_than_raised(self): + reviewer = self.reviewer('sys.stdout.buffer.write(b"\\xff\\n")') + with self.assertRaises(review_mod.ReviewerFailed) as raised: + review_mod.ask(reviewer, "doc", "change", self.CARRIED) + self.assertIn("not UTF-8", str(raised.exception)) + + def test_a_reviewer_that_cannot_be_run_is_refused(self): + with self.assertRaises(review_mod.ReviewerFailed): + review_mod.ask(["/nonexistent/reviewer"], "doc", "change", self.CARRIED) + + def test_one_catch_in_three_is_counted_and_does_not_hold(self): + answers = iter([{"a"}, set(), set(), set(), set(), set()]) + declared = [{"record": "a", "catches": "plant", "misses": "clean"}] + results = review_mod.drive( + declared, "doc", {"a": "A"}, 3, lambda *_: next(answers) + ) + self.assertEqual(results, [{"record": "a", "caught": 1, "fired": 0}]) + self.assertFalse(review_mod.holds(results[0], 3)) + report = review_mod.render_drive(results, 3) + self.assertIn("a: caught 1/3, misses fired 0/3 -- DOES NOT HOLD", report) + self.assertIn("pass@1 0.33; pass^3 0/1", report) + + def test_firing_on_the_miss_breaks_a_control_that_caught_every_plant(self): + results = review_mod.drive( + [{"record": "a", "catches": "plant", "misses": "clean"}], + "doc", + {"a": "A"}, + 2, + lambda *_: {"a"}, + ) + self.assertEqual(results[0]["fired"], 2) + self.assertFalse(review_mod.holds(results[0], 2)) + self.assertIn("false positives 1.00", review_mod.render_drive(results, 2)) + + def test_an_unwritten_misses_is_unmeasured_and_not_a_rate_of_zero(self): + calls = [] + + def reviewer(document: str, change: str, carried: dict) -> set: + calls.append(change) + return {"a"} + + results = review_mod.drive( + [{"record": "a", "catches": "plant"}], "doc", {"a": "A"}, 2, reviewer + ) + self.assertEqual(calls, ["plant", "plant"]) + self.assertIsNone(results[0]["fired"]) + self.assertTrue(review_mod.holds(results[0], 2)) + report = review_mod.render_drive(results, 2) + self.assertIn("misses not written", report) + self.assertNotIn("false positives", report) + + @needs_the_engine class ClaimsThatEnforceNothing(unittest.TestCase): """A claim naming a rule no seam supplies must not silence the review. @@ -524,9 +641,8 @@ def test_this_repository_routes_cleanly(self): self.assertEqual(result.returncode, 0, result.stderr) def test_the_denominator_is_printed_on_a_run_that_found_nothing_wrong(self): - # The reader who most needs it is the one skimming a green run: this - # repository declares one control and carries twenty-nine records, and - # a run that said only "29 record(s) compile" would read like coverage. + # The reader who most needs it is the one skimming a green run: a run + # that said only how many records compile would read like coverage. result = subprocess.run( [sys.executable, str(SCRIPT), "--review"], cwd=ROOT, @@ -537,7 +653,7 @@ def test_the_denominator_is_printed_on_a_run_that_found_nothing_wrong(self): self.assertEqual(result.returncode, 0, result.stderr) self.assertIn("have no control", result.stdout) self.assertIn("nothing here shows they can produce a finding", result.stdout) - # The one record that does carry a control is not in that list. + # A record that does carry a control is not in that list. self.assertNotIn("single-authoritative-source", result.stdout) def test_the_compiled_documents_are_current(self): @@ -550,6 +666,51 @@ def test_the_compiled_documents_are_current(self): ) self.assertEqual(result.returncode, 0, result.stderr) + def drive(self, *argv: str) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(SCRIPT), "--review", "--drive", *argv], + cwd=ROOT, + capture_output=True, + text=True, + check=False, + ) + + def test_a_reviewer_holding_the_answer_key_holds_every_control(self): + # Reads the export, so it stays an oracle as controls are added: what + # this pins is the wiring from the declaration to the verdict. + oracle = textwrap.dedent( + """ + import json, sys + change = json.load(sys.stdin)["change"] + key = json.load(open("review-controls.json"))["controls"] + print("\\n".join(c["record"] for c in key if c["catches"] == change)) + """ + ) + result = self.drive( + "--timeout", "60", "--runs", "2", "--", sys.executable, "-c", oracle + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("single-authoritative-source: caught 2/2", result.stdout) + self.assertIn("pass^2", result.stdout) + + def test_a_reviewer_that_names_nothing_is_reported_and_exits_1(self): + result = self.drive("--", sys.executable, "-c", "import sys; sys.stdin.read()") + self.assertEqual(result.returncode, 1, result.stderr) + self.assertIn("caught 0/3", result.stdout) + self.assertIn("DOES NOT HOLD", result.stdout) + + def test_drive_without_a_reviewer_is_a_usage_error(self): + for argv in [ + (), + ("--runs", "0", "--", "true"), + ("--timeout", "x", "--", "true"), + ("--runs", "2"), + ("true",), + ]: + result = self.drive(*argv) + self.assertEqual(result.returncode, 2, argv) + self.assertIn("--drive [--runs N] [--timeout SECONDS]", result.stderr) + def test_an_exemption_without_a_reason_is_refused(self): # A reason nobody wrote is one nobody can review, and it is exactly as # permanent as one that was argued for. diff --git a/uphold_check.py b/uphold_check.py index 219bc9a..1d64ab7 100755 --- a/uphold_check.py +++ b/uphold_check.py @@ -738,11 +738,44 @@ def emit_target(root: Path, name: str) -> Path: return target +REVIEW_USAGE = ( + "usage: uphold_check.py --review " + "[--emit | --check | --drive [--runs N] [--timeout SECONDS] -- REVIEWER...]" +) + + +def drive_arguments(argv: list[str]) -> tuple[int, int, list[str]] | None: + """`--drive [--runs N] [--timeout SECONDS] -- REVIEWER...`, or None. + + Returns (runs, timeout, reviewer). The reviewer is everything after `--`, + verbatim, so its own flags never have to be told apart from these. + """ + values = { + "--runs": review_mod.DEFAULT_RUNS, + "--timeout": review_mod.DEFAULT_TIMEOUT, + } + rest = argv[1:] + while rest[:1] and rest[0] in values: + if len(rest) < 2 or not rest[1].isdigit() or int(rest[1]) < 1: + return None + values[rest[0]] = int(rest[1]) + rest = rest[2:] + if rest[:1] != ["--"] or len(rest) < 2: + return None + return values["--runs"], values["--timeout"], rest[1:] + + def run_review(argv: list[str]) -> int: emit = argv[:1] == ["--emit"] check = argv[:1] == ["--check"] - if argv and not (emit or check): - print("usage: uphold_check.py --review [--emit|--check]", file=sys.stderr) + driving = None + if argv[:1] == ["--drive"]: + driving = drive_arguments(argv) + if driving is None: + print(REVIEW_USAGE, file=sys.stderr) + return 2 + elif argv and not (emit or check): + print(REVIEW_USAGE, file=sys.stderr) return 2 root = discover_root() @@ -802,6 +835,25 @@ def run_review(argv: list[str]) -> int: if errors or stale or control_errors or over: return 1 + if driving is not None: + runs, timeout, reviewer = driving + carried = {record["id"]: record["title"] for record in for_review} + try: + results = review_mod.drive( + settings["controls"], + document, + carried, + runs, + lambda doc, change, shown: review_mod.ask( + reviewer, doc, change, shown, timeout + ), + ) + except review_mod.ReviewerFailed as error: + print(f"uphold review could not look: {error}", file=sys.stderr) + return 2 + print(review_mod.render_drive(results, runs), end="") + return 0 if all(review_mod.holds(result, runs) for result in results) else 1 + if emit: for name, path in control_targets: try: