From b1e3b39f31178ec07a0a99f9a98ed04eeebda06d Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Sat, 24 Oct 2026 22:32:09 -0400 Subject: [PATCH 1/2] Done plan phases gate every PR Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013ejYrMLnLWah8wCDDcb57V From eae92918164b2296de6c37df45f9f3ee2f4eec21 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Sat, 24 Oct 2026 22:50:10 -0400 Subject: [PATCH 2/2] Fail the change that breaks a done plan phase DonePhaseBroken reads every Plan family phase through PlanPhase.done at the base and the head, one process per revision, and fails the change for each phase done at the base and not at the head, keyed by phase and file:line. Evidence classes answer their failing sites through a concrete sites() (PatternAbsent, OwnClassChecksAbsent, RestatedMembersAbsent, ClassRostersAbsent, AllEvidence); the rest describe what is missing. MainTree keeps each path's last parsed blob, so one revision parses src once for all 73 phases instead of once per evidence. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013ejYrMLnLWah8wCDDcb57V --- src/agent_comms/phase_evidence.py | 110 ++++++++++++++++++++---------- tests/guards/test_debt_ratchet.py | 26 +++++++ tests/test_phase_evidence.py | 2 +- tools/debt_ratchet.py | 53 ++++++++++++++ 4 files changed, 155 insertions(+), 36 deletions(-) diff --git a/src/agent_comms/phase_evidence.py b/src/agent_comms/phase_evidence.py index 7b9e8e5248..092f9b7223 100644 --- a/src/agent_comms/phase_evidence.py +++ b/src/agent_comms/phase_evidence.py @@ -36,6 +36,7 @@ class MainTree: commit: str branch: ClassVar[str] = "main" source: ClassVar[str] = ".py" + parsed: ClassVar[dict[str, tuple[str, ParsedModule]]] = {} @classmethod def of(cls, repo: Repo) -> MainTree: @@ -97,18 +98,25 @@ def present_modules(self, paths: tuple[str, ...]) -> Iterator[ParsedModule]: yield self.module(path) def modules(self, paths: tuple[str, ...]) -> Iterator[ParsedModule]: - """Every Python module under ``paths``, read in one ``git cat-file --batch``.""" - listed = Repo.git(self.repo.checkout, "ls-tree", "-r", "--name-only", self.commit, "--", - *paths) - names = [name for name in listed.splitlines() if name.endswith(self.source)] + """Every Python module under ``paths``. Each path keeps its last parsed blob, so every + evidence reading one commit parses it once, and the rest are read in one + ``git cat-file --batch``.""" + listed = Repo.git(self.repo.checkout, "ls-tree", "-r", self.commit, "--", *paths) + blobs = [(entry.split()[2], name) for entry, _, name in + (line.partition("\t") for line in listed.splitlines()) + if name.endswith(self.source)] + fresh = [(blob, name) for blob, name in blobs + if self.parsed.get(name, ("", None))[0] != blob] batch = Repo.git_bytes(self.repo.checkout, "cat-file", "--batch", - input="".join(f"{self.commit}:{name}\n" for name in names)) - for name in names: + input="".join(f"{blob}\n" for blob, _ in fresh)) if fresh else b"" + for blob, name in fresh: header, _, batch = batch.partition(b"\n") size = int(header.rsplit(b" ", 1)[1]) source = batch[:size].decode() - yield ParsedModule(name, source, ast.parse(source, filename=name), name) + self.parsed[name] = blob, ParsedModule(name, source, + ast.parse(source, filename=name), name) batch = batch[size + 1 :] + yield from (self.parsed[name][1] for _, name in blobs) def contains(self, sha: str) -> bool: return self.repo.contains(sha, self.commit) @@ -186,6 +194,11 @@ def holds(self, reading: PlanReading) -> bool: ... @abstractmethod def describe(self) -> str: ... + def sites(self, reading: PlanReading) -> tuple[str, ...]: + """Each ``PATH:LINE`` that keeps this evidence from holding; evidence naming no line + has none, and its description says what is missing.""" + return () + class MainEvidence(PhaseEvidence): """Evidence that reads only main: one answer per main commit.""" @@ -206,6 +219,12 @@ def holds(self, reading: PlanReading) -> bool: @abstractmethod def read(self, main: MainTree) -> bool: ... + def sites(self, reading: PlanReading) -> tuple[str, ...]: + return self.located(reading.main) + + def located(self, main: MainTree) -> tuple[str, ...]: + return () + @dataclass(frozen=True) class CommitMerged(MainEvidence): @@ -237,10 +256,14 @@ class PatternEvidence(MainEvidence): paths: tuple[str, ...] = ("src",) def read(self, main: MainTree) -> bool: + return self.satisfied(main.count(self.pattern, self.searched(main))) + + def searched(self, main: MainTree) -> tuple[str, ...]: + """``paths`` without the phase declarations, which spell every pattern they ban.""" from .plan_phases import PlanPhase declaration = main.repo.module_file(PlanPhase.__module__) - return self.satisfied(main.count(self.pattern, (*self.paths, f":!{declaration.as_posix()}"))) + return (*self.paths, f":!{declaration.as_posix()}") @abstractmethod def satisfied(self, count: int) -> bool: ... @@ -248,6 +271,10 @@ def satisfied(self, count: int) -> bool: ... @dataclass(frozen=True) class PatternAbsent(PatternEvidence): + def located(self, main: MainTree) -> tuple[str, ...]: + return tuple(f"{path}:{line}" for path, line in + sorted(main.matching_lines(self.pattern, self.searched(main)))) + def satisfied(self, count: int) -> bool: return count == 0 @@ -333,6 +360,10 @@ class AllEvidence(PhaseEvidence): def holds(self, reading: PlanReading) -> bool: return all(reading.holds(part) for part in self.parts) + def sites(self, reading: PlanReading) -> tuple[str, ...]: + return tuple(site for part in self.parts if not reading.holds(part) + for site in part.sites(reading)) + def describe(self) -> str: return "; ".join(part.describe() for part in self.parts) @@ -448,22 +479,12 @@ def of( return functions.found -class Classes(ast.NodeVisitor): - """Every class statement in the visited trees, nested ones included.""" - - def __init__(self) -> None: - self.found: list[ast.ClassDef] = [] +class Classes: + """Every class statement in the given trees, nested ones included.""" - def visit_ClassDef(self, node: ast.ClassDef) -> None: - self.found.append(node) - self.generic_visit(node) - - @classmethod - def of(cls, trees: Iterable[ast.AST]) -> list[ast.ClassDef]: - classes = cls() - for tree in trees: - classes.visit(tree) - return classes.found + @staticmethod + def of(trees: Iterable[ast.AST]) -> list[ast.ClassDef]: + return [node for tree in trees for node in ast.walk(tree) if isinstance(node, ast.ClassDef)] @dataclass(frozen=True) @@ -654,7 +675,8 @@ class ClassChecks(ast.NodeVisitor): def __init__(self) -> None: self.declared: set[str] = set() - self.checked: set[str] = set() + self.path = "" + self.checked: list[tuple[str, int, frozenset[str]]] = [] self._scopes: list[set[str]] = [] self._methods: set[ast.FunctionDef | ast.AsyncFunctionDef] = set() self._comparison_guards: set[ast.Call] = set() @@ -701,9 +723,13 @@ def visit_Call(self, node: ast.Call) -> None: match node: case ast.Call(func=ast.Name(id=isinstance.__name__), args=[_, target]) \ if node not in self._comparison_guards: - self.checked |= NamedClasses.of((target,)) + self.checked.append((self.path, node.lineno, frozenset(NamedClasses.of((target,))))) self.generic_visit(node) + def owned(self) -> tuple[tuple[str, int], ...]: + """The path and line of each check naming a class the visited modules declare.""" + return tuple((path, line) for path, line, named in self.checked if named & self.declared) + @dataclass(frozen=True) class OwnClassChecksAbsent(MainEvidence): @@ -711,10 +737,18 @@ class OwnClassChecksAbsent(MainEvidence): typing and SDK types stay.""" def read(self, main: MainTree) -> bool: + return not self.located(main) + + def located(self, main: MainTree) -> tuple[str, ...]: + return tuple(f"{path}:{line}" for path, line in self.checks(main).owned()) + + @staticmethod + def checks(main: MainTree) -> ClassChecks: checks = ClassChecks() for module in main.modules(("src",)): + checks.path = module.path checks.visit(module.tree) - return not checks.declared & checks.checked + return checks def describe(self) -> str: return "no isinstance against a class declared in src on main" @@ -1013,16 +1047,16 @@ def visit_Call(self, node: ast.Call) -> None: class ClassAnswers(ast.NodeVisitor): - """Each member a class body declares by name, with its line and its whole declaration. A - field counts only on a dataclass, and a name ``type`` reads per class never counts.""" + """Each member a class body declares by name, with its declaration. A field counts only on + a dataclass, and a name ``type`` reads per class never counts.""" def __init__(self, declared: ast.ClassDef, per_class: frozenset[str]) -> None: self.fields = dataclass.__name__ in DecoratorNames.of(declared.decorator_list) self.per_class = per_class - self.found: dict[str, tuple[int, str]] = {} + self.found: dict[str, ast.stmt] = {} @classmethod - def of(cls, declared: ast.ClassDef, per_class: frozenset[str]) -> dict[str, tuple[int, str]]: + def of(cls, declared: ast.ClassDef, per_class: frozenset[str]) -> dict[str, ast.stmt]: answers = cls(declared, per_class) for member in declared.body: answers.visit(member) @@ -1030,7 +1064,7 @@ def of(cls, declared: ast.ClassDef, per_class: frozenset[str]) -> dict[str, tupl def record(self, name: str, member: ast.stmt) -> None: if name not in self.per_class: - self.found[name] = (member.lineno, ast.dump(member)) + self.found[name] = member def generic_visit(self, node: ast.AST) -> None: """Only the body's own statements declare members.""" @@ -1066,16 +1100,19 @@ class RestatedMembersAbsent(MainEvidence): def read(self, main: MainTree) -> bool: return not any(self.restatements(main)) + def located(self, main: MainTree) -> tuple[str, ...]: + return tuple(self.restatements(main)) + def restatements(self, main: MainTree) -> Iterator[str]: """Each restated member, as ``PATH:LINE Class.member``.""" scopes = DeclaredScopes.of(main, self.paths) for declared in scopes.classes.values(): for parent in scopes.parents(declared): inherited = ClassAnswers.of(parent.node, self.per_class) - for name, (line, answer) in ClassAnswers.of(declared.node, self.per_class).items(): - if name in inherited and inherited[name][1] == answer: - member = QualifiedName((declared.node.name, name)) - yield f"{declared.path}:{line} {member}" + for name, member in ClassAnswers.of(declared.node, self.per_class).items(): + if name in inherited and ast.dump(inherited[name]) == ast.dump(member): + restated = QualifiedName((declared.node.name, name)) + yield f"{declared.path}:{member.lineno} {restated}" def describe(self) -> str: return f"no member restating its base's in {' '.join(self.paths)} on main" @@ -1095,6 +1132,9 @@ class ClassRostersAbsent(MainEvidence): def read(self, main: MainTree) -> bool: return not any(self.rosters(main)) + def located(self, main: MainTree) -> tuple[str, ...]: + return tuple(self.rosters(main)) + def relations(self, scopes: DeclaredScopes) -> dict[DeclaredClass, frozenset[str]]: """Each family's declaration under the roots, found by its qualified name, with the class attributes it declares as a tuple of its own classes.""" diff --git a/tests/guards/test_debt_ratchet.py b/tests/guards/test_debt_ratchet.py index 72bd3c7605..fed130d99f 100644 --- a/tests/guards/test_debt_ratchet.py +++ b/tests/guards/test_debt_ratchet.py @@ -23,6 +23,7 @@ BroadExceptDefault, BroadSwallow, ContentCache, + DonePhaseBroken, ForeignFieldSpellings, FrozenFieldWrite, FunctionLocalImport, @@ -328,6 +329,31 @@ def test_added_type_check_fails(repo: Repository) -> None: } +def test_a_done_phase_fails_the_change_that_breaks_it(tmp_path: Path) -> None: + """OwnTypeChecksPhase holds at the base, so re-adding an isinstance on our own class + fails at its line; the phases not done at the base (every merged-commit one, here) + block nothing.""" + repo = Repository(tmp_path, "src") + # The merged-commit phases name this repository's commits; borrowing its objects lets + # them read "not an ancestor" of the fixture instead of fetching. + objects = subprocess.check_output(["git", "-C", str(SOURCE), "rev-parse", "--path-format=absolute", + "--git-common-dir"], text=True).strip() + (tmp_path / ".git/objects/info/alternates").write_text(f"{objects}/objects\n") + repo.git("remote", "add", "origin", str(SOURCE)) + owned = "class Owned:\n pass\n" + base = repo.commit({"agent_comms/owned.py": owned}) + head = repo.commit({"agent_comms/owned.py": owned + "\n\ndef owns(value):\n" + " return isinstance(value, Owned)\n"}) + status, report = repo.compare(base, head, DonePhaseBroken) + assert status == 1 + assert report["delta"] == { + "DonePhaseBroken:OwnTypeChecksPhase::src/agent_comms/owned.py:6": 1, + } + clean = repo.commit({"agent_comms/owned.py": owned + "\n\ndef owns(value):\n" + " return value.owned()\n"}) + assert repo.compare(base, clean, DonePhaseBroken)[0] == 0 + + def test_moves_between_files_pass(repo: Repository) -> None: source = 'x = row = {"key": 1}\nvalue = type(x) is not int\nread = row["key"]\n' base = repo.commit({"from.py": source, "other.py": "a = 1\n"}) diff --git a/tests/test_phase_evidence.py b/tests/test_phase_evidence.py index e45896edb0..aca9dd1f92 100644 --- a/tests/test_phase_evidence.py +++ b/tests/test_phase_evidence.py @@ -260,7 +260,7 @@ def test_source_shape_evidence_reads_declarations_on_main(tmp_path): def test_only_foreign_comparison_refusal_exempts_an_owned_type_check(source, allowed): checks = ClassChecks() checks.visit(ast.parse(source)) - assert (not checks.declared & checks.checked) is allowed + assert (not checks.owned()) is allowed @pytest.mark.parametrize(("required", "unskipped", "expected"), ( diff --git a/tools/debt_ratchet.py b/tools/debt_ratchet.py index e8aea938a9..c87b0ae314 100644 --- a/tools/debt_ratchet.py +++ b/tools/debt_ratchet.py @@ -53,6 +53,9 @@ from stored_formats import StoredForm from agent_comms.field_codec import FieldCodec, boundary_decoder, projected from agent_comms.mro_dispatch import MroDispatch, handles +from agent_comms.phase_evidence import MainTree, PlanReading +from agent_comms.plan_phases import PlanPhase +from agent_comms.repo import Repo AUDIT_SCRIPTS = Path(audit.__path__[0]).parent REPOSITORY = Path(__file__).resolve().parents[1] @@ -2492,6 +2495,56 @@ def counted(cls, repo: Path, base: str, head: str, changed: set[str], root: str) return {cls.__name__: 0}, {cls.__name__: uncarried} +class DonePhaseBroken(Measure): + """A plan phase whose evidence holds at the base and fails at the head: a done phase is a + standing guard, so a change re-adding what the phase deleted fails here, one key per site. + + The phases are the Plan family's registry and ``PlanPhase.done`` is their only reader; + a phase not yet done at the base is plan work in progress and blocks nothing. Evidence + that cannot be read here (GitHub's rules, with no token) holds at neither revision. Each + revision is read in one process, so its modules are parsed once for every phase, and kept + by the commit and the ratchet's code, which holds the phases' declarations.""" + + @classmethod + def parts(cls, repo: Path, base: str, head: str, changed: set[str], + root: str) -> tuple[Callable[[], object], ...]: + if root != f"{Repo.of(repo).source}/": + return () + return tuple(partial(cls.failing, repo, revision(repo, ref)) for ref in (base, head)) + + @classmethod + def combine(cls, results: list, repo: Path, base: str, head: str, changed: set[str], + root: str) -> tuple[dict[str, int], dict[str, int]]: + """A failing pull request pays for reading its sites; a passing one never reads them.""" + if not results: + return {}, {} + before, after = results + reading = PlanReading((), MainTree(Repo.of(repo), revision(repo, head))) + broken = { + cls.key(f"{phase.__name__}::{site}"): 1 + for phase in PlanPhase.members_with(PlanPhase) + if phase.__name__ in after - before + for site in phase.evidence.sites(reading) or (reading.describe(phase.evidence),) + } + return dict.fromkeys(broken, 0), broken + + @classmethod + def grew(cls, key: str, grown: int) -> str: + phase, _, site = key.partition(":")[2].partition("::") + return f"done plan phase {phase} no longer holds: {site}" + + @classmethod + def failing(cls, repo: Path, commit: str) -> frozenset[str]: + """The name of each phase not done at a commit.""" + return ContentCache.memo(partial(cls.read, repo, commit), cls.__name__, commit) + + @staticmethod + def read(repo: Path, commit: str) -> frozenset[str]: + reading = PlanReading((), MainTree(Repo.of(repo), commit)) + return frozenset(phase.__name__ for phase in PlanPhase.members_with(PlanPhase) + if not phase.done(reading)) + + def git(repo: Path, *args: str, input: bytes = b"") -> bytes: return subprocess.check_output(["git", "-C", str(repo), *args], input=input, timeout=20)