From 23fc2a2ad21d4d9600780aa20daed2ebc9fb7eba Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Sun, 15 Nov 2026 22:53:38 +0800 Subject: [PATCH 1/2] refactor: relocate resource facade and split evidence layout Agent-assisted-by: Claude Code (author) Signed-off-by: GUOGUO --- docs/legacy-knowledge-manifest.json | 49 ++- src/inferrsi/apps/evidence_layout.py | 19 ++ src/inferrsi/catalog.py | 4 +- .../evidence.py} | 14 +- src/inferrsi/infra/inspection.py | 2 +- src/inferrsi/knowledge/catalog.py | 2 +- src/inferrsi/resources/__init__.py | 5 + .../repository_packs.py} | 0 tests/_gate_policy_support.py | 62 +++- tests/check_installed.py | 46 ++- tests/test_contracts_package.py | 8 + tests/test_evidence_paths.py | 288 +++++++++++++++++- tests/test_first_wave_isolation_policy.py | 175 ++++++++++- tests/test_knowledge_compatibility_shells.py | 100 +++++- tests/test_package_root_inventory.py | 44 ++- tests/test_repository_packs_facade.py | 238 +++++++++++++++ tests/test_smoke.py | 2 +- tests/test_target_module_import_gate.py | 15 +- 18 files changed, 1004 insertions(+), 69 deletions(-) create mode 100644 src/inferrsi/apps/evidence_layout.py rename src/inferrsi/{evidence_paths.py => contracts/evidence.py} (59%) create mode 100644 src/inferrsi/resources/__init__.py rename src/inferrsi/{_repo_pack_data.py => resources/repository_packs.py} (100%) create mode 100644 tests/test_repository_packs_facade.py diff --git a/docs/legacy-knowledge-manifest.json b/docs/legacy-knowledge-manifest.json index 0f5646c22..7b213283c 100644 --- a/docs/legacy-knowledge-manifest.json +++ b/docs/legacy-knowledge-manifest.json @@ -1,7 +1,7 @@ { "$schema": "urn:inferrsi:docs:legacy-knowledge-manifest:v1", - "description": "First-wave compatibility exceptions for seven root Knowledge leaves plus the neutral technical data leaf. P0b froze the first revision at 0da7e2f960cd4ec994ae8eddea5b71023c5379ca and K01 re-recorded every leaf hash before the code move. This K02 revision changes only the neutral data leaf's locked file hash, because the pack.json resource moved to the Harness root. It is not a wildcard approval and changes only through formal review. D018/D020 chose these exact edges. file_sha256 is the old root leaf before the move, and line references are at the recorded base_commit.", - "base_commit": "d7de7f3a7a4c089da08cc17a0117a54d89466dee", + "description": "First-wave compatibility exceptions for seven root Knowledge leaves plus the neutral technical data leaf. P0b froze the first revision at 0da7e2f960cd4ec994ae8eddea5b71023c5379ca and K01 re-recorded every leaf hash before the code move. K02 changed only the neutral data leaf's locked file hash, because the pack.json resource moved to the Harness root. This C2 revision (#451) changes only where that leaf lives: it moves byte for byte from inferrsi._repo_pack_data to inferrsi.resources.repository_packs, and the two declared importers of the leaf point at the new module. The leaf's bytes and locked file hash, the seven root leaf hashes and the nine-pair count are unchanged. It is not a wildcard approval and changes only through formal review. D018/D020 chose these exact edges. file_sha256 is the old root leaf before the move, and line references are at the base_commit recorded by the revision that wrote them; this revision re-records none of them.", + "base_commit": "cd8fcb452bd2c7969a7a6e2544aa1cb5f8ad5332", "revision_history": [ { "base_commit": "0da7e2f960cd4ec994ae8eddea5b71023c5379ca", @@ -21,6 +21,16 @@ "changed_file_sha256": { "src/inferrsi/_repo_pack_data.py": "a14edac8283b1f7b6b5794381c44d7c2632dda5cc01f03c557dc293d05523afd" } + }, + { + "base_commit": "d7de7f3a7a4c089da08cc17a0117a54d89466dee", + "stage": "K02", + "superseded_by": "C2 module-path revision at cd8fcb452bd2c7969a7a6e2544aa1cb5f8ad5332", + "reason": "C2 (#451) moves the neutral data leaf from src/inferrsi/_repo_pack_data.py to src/inferrsi/resources/repository_packs.py and deletes the root file in the same change. The file bytes are unchanged, so the locked file hash keeps its value. The leaf's module and file, the neutral_leaf_export target and the targets of the catalog and infra.inspection pairs change to the new module. The seven root leaf hashes and the nine-pair count are unchanged.", + "changed_technical_leaf": { + "module": "inferrsi._repo_pack_data", + "file": "src/inferrsi/_repo_pack_data.py" + } } ], "compatibility_pairs": [ @@ -55,7 +65,7 @@ }, { "importer": "catalog", - "target": "inferrsi._repo_pack_data", + "target": "inferrsi.resources.repository_packs", "note": "Root catalog statically re-exports REPO_TYPES and repository_pack from the business-free neutral technical leaf" }, { @@ -216,8 +226,8 @@ }, { "importer": "infra.inspection", - "target": "inferrsi._repo_pack_data", - "note": "infra.inspection imports the neutral leaf directly, never via catalog or H. D020 requires the import rewrite; K01 implementation must change only the import line (currently L10: from ..catalog import repository_pack → from .._repo_pack_data import repository_pack) and keep original functions, injected parameters, and returns unchanged" + "target": "inferrsi.resources.repository_packs", + "note": "infra.inspection imports the neutral leaf directly, never via catalog or H. D020 required the import rewrite; K01 changed only the import line (L10: from ..catalog import repository_pack → from .._repo_pack_data import repository_pack) and kept original functions, injected parameters, and returns unchanged. C2 (#451) changes that same line to from ..resources.repository_packs import repository_pack and nothing else in the module." } ], "catalog_two_routes": { @@ -239,7 +249,7 @@ "note": "K.catalog does not export repository_pack; the named-symbol-only K export excludes repository_pack" }, "neutral_leaf_export": { - "target": "inferrsi._repo_pack_data", + "target": "inferrsi.resources.repository_packs", "symbols": [ "REPO_TYPES", "repository_pack" @@ -248,9 +258,9 @@ } }, "technical_leaf": { - "module": "inferrsi._repo_pack_data", - "note": "K01 created src/inferrsi/_repo_pack_data.py from the recorded original fragment plus a leading comment, the future import, and the three listed stdlib imports. K02 repointed the reader to the Harness resource root and extended that comment, so file_sha256 is relocked here. original_fragment keeps the P0b provenance text; current_fragment is the body in the file today. P0b recorded only the fragment.", - "file": "src/inferrsi/_repo_pack_data.py", + "module": "inferrsi.resources.repository_packs", + "note": "K01 created src/inferrsi/_repo_pack_data.py from the recorded original fragment plus a leading comment, the future import, and the three listed stdlib imports. K02 repointed the reader to the Harness resource root and extended that comment, so file_sha256 was relocked then. C2 (#451) moved the file byte for byte to src/inferrsi/resources/repository_packs.py and deleted the root file; file_sha256 is checked against the relocated file and keeps its K02 value. original_fragment keeps the P0b provenance text; current_fragment is the body in the file today. P0b recorded only the fragment.", + "file": "src/inferrsi/resources/repository_packs.py", "file_sha256": "751d7bf663318962a2e73cf9d75369e0f8f8c2bda27964c56e44021b9f86c992", "allowed_imports": [ "json", @@ -262,7 +272,7 @@ "REPO_TYPES", "repository_pack" ], - "top_level_shape": "module docstring, a future import, the two symbols only, and no other business definitions or core imports", + "top_level_shape": "a leading comment (a module docstring is also accepted), the future import, exactly the three listed stdlib imports, the two symbols only, and no other definition, inferrsi import or dynamic import. Its one parent initializer, src/inferrsi/resources/__init__.py, is a module docstring and no other statement, so it imports nothing", "original_fragment": { "description": "REPO_TYPES and repository_pack body from current catalog.py (lines 12, 171-175)", "hash_convention": "fragment_sha256 is the SHA-256 of source plus one trailing newline", @@ -285,13 +295,28 @@ "new_line": "path = files(\"inferrsi\").joinpath(\"harness\", \"data\", \"repo_packs\", repo_type, \"pack.json\")", "unchanged": "The importlib anchor stays files(\"inferrsi\") and joins the physical path, so the leaf still imports no inferrsi module. allowed_imports, the two symbols, the top-level shape, REPO_TYPES object identity and the two permitted importers are unchanged, and infra gains no edge into Harness.", "review": "This manifest revision requires formal architecture review (amy-why-3459 and zengchuang-hw). The implementer does not self-approve it." + }, + "module_path_revision": { + "stage": "C2", + "issue": "#451", + "reason": "Packaged-resource access leaves the flat package root. The leaf moves with its bytes unchanged, and the root file is deleted in the same change with no forwarding shell or sys.modules alias.", + "old_module": "inferrsi._repo_pack_data", + "new_module": "inferrsi.resources.repository_packs", + "old_file": "src/inferrsi/_repo_pack_data.py", + "new_file": "src/inferrsi/resources/repository_packs.py", + "old_file_sha256": "751d7bf663318962a2e73cf9d75369e0f8f8c2bda27964c56e44021b9f86c992", + "new_file_sha256": "751d7bf663318962a2e73cf9d75369e0f8f8c2bda27964c56e44021b9f86c992", + "parent_initializer": "src/inferrsi/resources/__init__.py", + "unchanged": "The file bytes, the importlib anchor files(\"inferrsi\") joined with harness/data/repo_packs//pack.json, allowed_imports, the two symbols, REPO_TYPES object identity and the two permitted importers outside Knowledge (root catalog and infra.inspection) are unchanged. The leaf still imports no inferrsi module, and its new parent initializer imports nothing, so infra gains no edge into Harness.", + "review": "This manifest revision requires formal architecture review, as the K02 resource_path_revision did. The implementer does not self-approve it, and passing tests are not that review." } }, "infra_inspection_rewrite": { "module": "src/inferrsi/infra/inspection.py", "current_import": "from ..catalog import repository_pack", "required_new_import": "from .._repo_pack_data import repository_pack", - "note": "K01 changes only this import line. Original functions, injected parameters, and returns stay the same. current_import records the line at base_commit." + "canonical_import": "from ..resources.repository_packs import repository_pack", + "note": "K01 changed only this import line, from current_import to required_new_import; both record that K01 rewrite. C2 (#451) changes the same line to canonical_import, which is the line in the module today. Original functions, injected parameters, and returns stay the same." }, - "intra_closure_note": "An import of inferrsi._repo_pack_data from inside inferrsi.knowledge (for example inferrsi/knowledge/catalog.py) is intra-closure, not a tenth compatibility exception; it is governed by the leaf's no-inferrsi.*-import rule and by REPO_TYPES object identity, not by this allowlist." + "intra_closure_note": "An import of inferrsi.resources.repository_packs from inside inferrsi.knowledge (for example inferrsi/knowledge/catalog.py) is intra-closure, not a tenth compatibility exception; it is governed by the leaf's no-inferrsi.*-import rule, its import-free parent initializer and REPO_TYPES object identity, not by this allowlist." } diff --git a/src/inferrsi/apps/evidence_layout.py b/src/inferrsi/apps/evidence_layout.py new file mode 100644 index 000000000..8510920e1 --- /dev/null +++ b/src/inferrsi/apps/evidence_layout.py @@ -0,0 +1,19 @@ +"""Create the standard evidence directories for one campaign root. + +This is the host I/O half of the evidence layout. The segment names and path rules +are ``inferrsi.contracts.evidence``, which never touches the filesystem. +""" + +from __future__ import annotations + +from pathlib import Path + +from inferrsi.contracts.evidence import evidence_subpaths + + +def ensure_evidence_layout(root: Path) -> dict[str, Path]: + """Create standard evidence subdirectories if missing (local/dev helper).""" + paths = evidence_subpaths(root) + for path in paths.values(): + path.mkdir(parents=True, exist_ok=True) + return paths diff --git a/src/inferrsi/catalog.py b/src/inferrsi/catalog.py index 3ab45f6fa..0f26b96f3 100644 --- a/src/inferrsi/catalog.py +++ b/src/inferrsi/catalog.py @@ -1,8 +1,7 @@ # Compatibility shell. Knowledge names live in inferrsi.knowledge.catalog; -# REPO_TYPES and repository_pack live in the neutral inferrsi._repo_pack_data. +# REPO_TYPES and repository_pack live in the neutral inferrsi.resources.repository_packs. from __future__ import annotations -from inferrsi._repo_pack_data import REPO_TYPES, repository_pack from inferrsi.knowledge.catalog import ( BACKENDS, KNOWLEDGE_REPOS, @@ -15,6 +14,7 @@ knowledge_records, search_knowledge, ) +from inferrsi.resources.repository_packs import REPO_TYPES, repository_pack __all__ = [ "REPO_TYPES", diff --git a/src/inferrsi/evidence_paths.py b/src/inferrsi/contracts/evidence.py similarity index 59% rename from src/inferrsi/evidence_paths.py rename to src/inferrsi/contracts/evidence.py index a5e8df928..32bf0ee56 100644 --- a/src/inferrsi/evidence_paths.py +++ b/src/inferrsi/contracts/evidence.py @@ -1,4 +1,8 @@ -"""First-class evidence directory layout (portable; no campaign pins).""" +"""Evidence segment names and per-campaign path rules (portable; no campaign pins). + +Path construction only. Nothing here reads or writes the filesystem; creating the +directories is ``inferrsi.apps.evidence_layout.ensure_evidence_layout``. +""" from __future__ import annotations @@ -15,11 +19,3 @@ def campaign_evidence_root(repo_root: Path, campaign_id: str) -> Path: def evidence_subpaths(root: Path) -> dict[str, Path]: """Map standard evidence segment names to paths under a campaign root.""" return {name: root / name for name in EVIDENCE_SEGMENTS} - - -def ensure_evidence_layout(root: Path) -> dict[str, Path]: - """Create standard evidence subdirectories if missing (local/dev helper).""" - paths = evidence_subpaths(root) - for path in paths.values(): - path.mkdir(parents=True, exist_ok=True) - return paths diff --git a/src/inferrsi/infra/inspection.py b/src/inferrsi/infra/inspection.py index c3251aef8..20b3d8473 100644 --- a/src/inferrsi/infra/inspection.py +++ b/src/inferrsi/infra/inspection.py @@ -7,7 +7,7 @@ from collections.abc import Callable from pathlib import Path -from .._repo_pack_data import repository_pack +from ..resources.repository_packs import repository_pack from .commands import probe_command from .platforms import inspect_device from .targets import repository_identity diff --git a/src/inferrsi/knowledge/catalog.py b/src/inferrsi/knowledge/catalog.py index 5ff3fdf77..fffd2d66b 100644 --- a/src/inferrsi/knowledge/catalog.py +++ b/src/inferrsi/knowledge/catalog.py @@ -8,7 +8,7 @@ from importlib.resources import files from typing import Any -from .._repo_pack_data import REPO_TYPES +from ..resources.repository_packs import REPO_TYPES from . import _source_snapshot # Learning scope does not imply that a repository has a runnable target adapter. diff --git a/src/inferrsi/resources/__init__.py b/src/inferrsi/resources/__init__.py new file mode 100644 index 000000000..05c29cff1 --- /dev/null +++ b/src/inferrsi/resources/__init__.py @@ -0,0 +1,5 @@ +"""Read-only access to packaged resources. + +This initializer is import-free. Python runs it on every import of +``inferrsi.resources.repository_packs``, so it holds this docstring and nothing else. +""" diff --git a/src/inferrsi/_repo_pack_data.py b/src/inferrsi/resources/repository_packs.py similarity index 100% rename from src/inferrsi/_repo_pack_data.py rename to src/inferrsi/resources/repository_packs.py diff --git a/tests/_gate_policy_support.py b/tests/_gate_policy_support.py index 7aa955430..570de349e 100644 --- a/tests/_gate_policy_support.py +++ b/tests/_gate_policy_support.py @@ -59,8 +59,17 @@ # The initializer's single import. Python runs the initializer for every contracts # import, so a module that may use the encoder also loads these two names, and no other. CONTRACTS_INIT_EXPORT = ("inferrsi.contracts.runspec", "RunSpec") +# #451 C2 adds exactly one module: the evidence segment and path rules. Directory +# creation is ``inferrsi.apps.evidence_layout`` and is not a contracts module. +CONTRACTS_EVIDENCE = "inferrsi.contracts.evidence" CONTRACTS_MODULES = frozenset( - {CONTRACTS_PACKAGE, CANONICAL_ENCODER, CONTRACTS_INIT_EXPORT[0], "inferrsi.contracts.claim"} + { + CONTRACTS_PACKAGE, + CANONICAL_ENCODER, + CONTRACTS_INIT_EXPORT[0], + "inferrsi.contracts.claim", + CONTRACTS_EVIDENCE, + } ) # Names that load or build modules at run time. Any use fails closed. # P0b scans only these three spellings; getattr, exec, eval, runpy, and @@ -423,8 +432,14 @@ def _allowed_key(entry: str) -> str: return f"{node.module}.{node.names[0].name}" -def _leaf_errors(tree: ast.Module, technical_leaf: dict[str, Any]) -> list[str]: - """The neutral data leaf: listed stdlib imports anywhere, two symbols only.""" +def _leaf_errors( + tree: ast.Module, technical_leaf: dict[str, Any], package: str = "inferrsi" +) -> list[str]: + """The neutral data leaf: listed stdlib imports anywhere, two symbols only. + + ``package`` is the leaf's own package, so a relative import is reported by the + module it really names. + """ module = technical_leaf["module"] allowed = {_allowed_key(entry) for entry in technical_leaf["allowed_imports"]} symbols = set(technical_leaf["two_symbols"]) @@ -433,7 +448,7 @@ def _leaf_errors(tree: ast.Module, technical_leaf: dict[str, Any]) -> list[str]: if isinstance(node, ast.Import): names = [alias.name for alias in node.names] elif isinstance(node, ast.ImportFrom): - base = _from_base(node, "inferrsi") + base = _from_base(node, package) names = [base if base in allowed else f"{base}.{alias.name}" for alias in node.names] else: continue @@ -457,6 +472,28 @@ def _leaf_errors(tree: ast.Module, technical_leaf: dict[str, Any]) -> list[str]: return errors +def _initializer_errors(module: str, tree: ast.Module, package: str) -> list[str]: + """A parent initializer of the data leaf: a docstring and no other statement. + + Python runs it on every import of the leaf, so it may import nothing at all, not + even a standard-library module, and may define nothing. + """ + errors: list[str] = [] + for node in ast.walk(tree): + if isinstance(node, ast.Import): + errors += [f"{module} imports {alias.name}" for alias in node.names] + elif isinstance(node, ast.ImportFrom): + base = _from_base(node, package) + errors += [f"{module} imports {base}.{alias.name}" for alias in node.names] + if _has_dynamic_import(tree): + errors.append(f"{module} has a dynamic import") + for index, node in enumerate(tree.body): + if (index == 0 and _is_docstring(node)) or isinstance(node, (ast.Import, ast.ImportFrom)): + continue + errors.append(f"{module} has a {type(node).__name__} statement") + return errors + + def _contracts_errors(graph: _Graph) -> list[str]: """The bounded contracts package: declared modules, one initializer export, no core. @@ -556,7 +593,15 @@ def first_wave_report( name, graph.trees[name], graph.packages[name], sources[name] ) if leaf in graph.trees: - violations += _leaf_errors(graph.trees[leaf], manifest["technical_leaf"]) + violations += _leaf_errors( + graph.trees[leaf], manifest["technical_leaf"], graph.packages[leaf] + ) + # #451 C2: the leaf sits in a package, so Python runs that package's initializer + # with it. Only those exact parent initializers join the leaf, and each must stay + # import-free. A sibling module under the same package gains nothing. + leaf_parents = _package_ancestors(leaf, graph.trees) + for parent in sorted(leaf_parents): + violations += _initializer_errors(parent, graph.trees[parent], graph.packages[parent]) violations += _contracts_errors(graph) # Inside the contracts package a Knowledge module may name only the encoder. The # initializer and its RunSpec module load with that import and are never named. @@ -572,7 +617,12 @@ def first_wave_report( encoder_closure = (CANONICAL_ENCODER, CONTRACTS_PACKAGE, CONTRACTS_INIT_EXPORT[0]) def outside_knowledge(node: str) -> bool: - return _core_of(node) != "knowledge" and node != leaf and node not in encoder_closure + return ( + _core_of(node) != "knowledge" + and node != leaf + and node not in leaf_parents + and node not in encoder_closure + ) def benchmark_forbidden(node: str) -> bool: return _core_of(node) in ("knowledge", "harness", "apps") or node in old_leaves diff --git a/tests/check_installed.py b/tests/check_installed.py index 97dd2c8a5..6982c085c 100644 --- a/tests/check_installed.py +++ b/tests/check_installed.py @@ -3,7 +3,9 @@ import io import json import os +import subprocess import sys +from importlib import import_module from importlib.resources import as_file, files from importlib.util import find_spec from pathlib import Path @@ -13,9 +15,10 @@ from inferrsi.benchmark.registry import builtin_tasks, describe_task, task_resources from inferrsi.catalog import REPO_TYPES, knowledge_records, repository_pack, search_knowledge from inferrsi.cli import main -from inferrsi.contracts import RunSpec, claim, json_v1 +from inferrsi.contracts import RunSpec, claim, evidence, json_v1 from inferrsi.knowledge._source_snapshot import BUNDLE_LIBRARY, LIBRARY from inferrsi.knowledge_operations import KnowledgeReader +from inferrsi.resources import repository_packs assert Path(inferrsi.__file__).resolve().parent.parent.name != "src", ( "This check requires an installed package" @@ -586,3 +589,44 @@ def _inventory(root, prefix): assert installed_root - {"_version.py"} == checkout_root - {"_version.py"}, ( sorted(installed_root ^ checkout_root) ) + +# #451 C2: the neutral repository-pack facade and the evidence path rules ship at their +# canonical paths. The two retired root modules are not in the wheel and have no alias. +assert repository_packs.REPO_TYPES is REPO_TYPES and len(REPO_TYPES) == 7 +for consumer in ("inferrsi.knowledge.catalog", "inferrsi.arguments"): + assert import_module(consumer).REPO_TYPES is REPO_TYPES, consumer +for repo_name in REPO_TYPES: + packed = files("inferrsi").joinpath("harness", "data", "repo_packs", repo_name, "pack.json") + data = packed.read_bytes() + # The inventory digest above is the one compared with the checkout byte for byte. + assert inventory[f"harness/data/repo_packs/{repo_name}/pack.json"] == ( + hashlib.sha256(data).hexdigest() + ), repo_name + assert repository_packs.repository_pack(repo_name) == json.loads(data.decode("utf-8")) + assert repository_pack(repo_name)["id"] == repo_name +try: + repository_packs.repository_pack("not-a-repository") +except ValueError as error: + assert str(error) == "Unknown repository type: not-a-repository" +else: + raise AssertionError("an unknown repository type must be refused") +assert evidence.EVIDENCE_SEGMENTS == ("plan", "results", "patch", "validator") +assert find_spec("inferrsi.apps.evidence_layout") is not None +for retired in ("_repo_pack_data", "evidence_paths"): + assert find_spec(f"inferrsi.{retired}") is None, retired +assert not installed_root & {"_repo_pack_data.py", "evidence_paths.py"} +# A fresh isolated interpreter reads all seven packs with only the facade and its +# import-free parent loaded: no Harness, Knowledge, Infra or Apps module. +facade_probe = ( + "import sys\n" + "from inferrsi.resources import repository_packs as packs\n" + "assert [packs.repository_pack(n)['id'] for n in packs.REPO_TYPES] == list(packs.REPO_TYPES)\n" + "loaded = {name for name in sys.modules if name.split('.')[0] == 'inferrsi'}\n" + "allowed = {'inferrsi', 'inferrsi._version', 'inferrsi.resources',\n" + " 'inferrsi.resources.repository_packs'}\n" + "assert loaded <= allowed, sorted(loaded - allowed)\n" +) +facade = subprocess.run( + [sys.executable, "-I", "-c", facade_probe], capture_output=True, text=True, check=False, +) +assert facade.returncode == 0, facade.stderr diff --git a/tests/test_contracts_package.py b/tests/test_contracts_package.py index 94dcbb23c..b2ea1e224 100644 --- a/tests/test_contracts_package.py +++ b/tests/test_contracts_package.py @@ -39,6 +39,8 @@ "inferrsi.contracts.runspec", "inferrsi.contracts.json_v1", "inferrsi.contracts.claim", + # #451 C2: evidence segment and path rules; no filesystem access. + "inferrsi.contracts.evidence", ) COLD_IMPORT = """ import importlib, json, sys @@ -165,6 +167,12 @@ def test_module_bodies_do_no_io_on_import() -> None: assert "RunSpec" in defined["inferrsi.contracts.runspec"] assert {"CanonicalError", "decode", "encode"} <= set(defined["inferrsi.contracts.json_v1"]) assert {"Verdict", "parse_verdict"} <= set(defined["inferrsi.contracts.claim"]) + assert {"EVIDENCE_SEGMENTS", "campaign_evidence_root", "evidence_subpaths"} <= set( + defined["inferrsi.contracts.evidence"] + ) + # The initializer still re-exports RunSpec alone; evidence is reached by its module. + assert not hasattr(contracts, "EVIDENCE_SEGMENTS") + assert not hasattr(contracts, "evidence_subpaths") @pytest.mark.parametrize("name", sorted(RETIRED)) diff --git a/tests/test_evidence_paths.py b/tests/test_evidence_paths.py index 1668ed47b..05c7ab6e4 100644 --- a/tests/test_evidence_paths.py +++ b/tests/test_evidence_paths.py @@ -1,6 +1,78 @@ +"""Evidence path rules and directory creation are separate modules (#451 C2). + +``inferrsi.contracts.evidence`` builds paths and never touches the filesystem. +``inferrsi.apps.evidence_layout`` is the one place that calls ``mkdir``. Root +``inferrsi.evidence_paths`` is retired without an alias. +""" + +from __future__ import annotations + +import ast +import importlib +import importlib.util +import json +import os +import pickle +import subprocess +import sys +from pathlib import Path, PurePosixPath, PureWindowsPath + +import pytest + +from inferrsi.apps import evidence_layout +from inferrsi.apps.evidence_layout import ensure_evidence_layout +from inferrsi.contracts import evidence +from inferrsi.contracts.evidence import ( + EVIDENCE_SEGMENTS, + campaign_evidence_root, + evidence_subpaths, +) + +SRC = Path(__file__).resolve().parents[1] / "src" +ROOT = SRC / "inferrsi" +RETIRED = "inferrsi.evidence_paths" +# Path rules run first and must emit nothing; directory creation runs second under the +# same hook and must be seen, so a silent hook cannot make the first half pass. +AUDIT = """ +import json, sys from pathlib import Path +from inferrsi.apps.evidence_layout import ensure_evidence_layout +from inferrsi.contracts.evidence import campaign_evidence_root, evidence_subpaths +IO_EVENTS = ("os.", "shutil.", "pathlib.", "tempfile.", "glob.", "subprocess.", "socket.") +seen = {"rules": [], "layout": []} +active = [] +def hook(event, args): + if active and (event == "open" or event.startswith(IO_EVENTS)): + seen[active[0]].append(event) +sys.addaudithook(hook) +base = Path(sys.argv[1]) +active[:] = ["rules"] +root = campaign_evidence_root(base, "demo") +paths = evidence_subpaths(root) +relative = evidence_subpaths(campaign_evidence_root(Path("."), "demo")) +active[:] = ["layout"] +made = ensure_evidence_layout(root) +active.clear() +print(json.dumps({ + "seen": seen, "same": made == paths, + "relative": [path.as_posix() for path in relative.values()], +})) +""" -from inferrsi.evidence_paths import campaign_evidence_root, ensure_evidence_layout + +def _reference(root: Path) -> dict[str, Path]: + """The body of the retired ``evidence_paths.ensure_evidence_layout``, verbatim.""" + paths = {name: root / name for name in EVIDENCE_SEGMENTS} + for path in paths.values(): + path.mkdir(parents=True, exist_ok=True) + return paths + + +def _tree(base: Path) -> list[str]: + return sorted( + path.relative_to(base).as_posix() + ("/" if path.is_dir() else "") + for path in base.rglob("*") + ) def test_campaign_root_under_evidence(tmp_path: Path) -> None: @@ -12,3 +84,217 @@ def test_ensure_layout_creates_dirs(tmp_path: Path) -> None: root = tmp_path / "evidence" / "x" paths = ensure_evidence_layout(root) assert all(p.is_dir() for p in paths.values()) + + +def test_segments_and_subpaths_are_unchanged(tmp_path: Path) -> None: + assert EVIDENCE_SEGMENTS == ("plan", "results", "patch", "validator") + assert type(EVIDENCE_SEGMENTS) is tuple + root = campaign_evidence_root(tmp_path, "demo-campaign") + paths = evidence_subpaths(root) + assert type(paths) is dict + assert list(paths) == list(EVIDENCE_SEGMENTS) + assert paths == {name: root / name for name in EVIDENCE_SEGMENTS} + # Each call builds a new mapping; nothing is cached or shared. + assert evidence_subpaths(root) is not paths + + +@pytest.mark.parametrize( + "repo_root,campaign_id", + [ + (Path("repo"), "demo"), + (Path("."), "demo"), + (Path("a/b"), "nested/campaign"), + (Path("repo"), PurePosixPath("as/pathlike")), + (PurePosixPath("/srv/repo"), "demo"), + (PureWindowsPath("C:/repo"), "demo"), + # No validation is added: an absolute campaign id still replaces the root. + (PurePosixPath("repo"), "/absolute"), + ], + ids=str, +) +def test_path_rules_keep_relative_pure_and_pathlike_inputs(repo_root, campaign_id) -> None: + expected = repo_root / "evidence" / campaign_id + root = campaign_evidence_root(repo_root, campaign_id) + assert root == expected and type(root) is type(expected) + # A relative root stays relative: nothing is resolved against the working directory. + assert root.is_absolute() == expected.is_absolute() + paths = evidence_subpaths(root) + assert paths == {name: expected / name for name in EVIDENCE_SEGMENTS} + assert {type(path) for path in paths.values()} == {type(expected)} + + +def test_path_rules_add_no_coercion() -> None: + # A string root was never coerced to a Path: the join itself raises, as before. + with pytest.raises(TypeError): + campaign_evidence_root("repo", "demo") + with pytest.raises(TypeError): + evidence_subpaths("evidence/demo") + with pytest.raises(TypeError): + campaign_evidence_root(Path("repo"), 7) + with pytest.raises(TypeError): + campaign_evidence_root(Path("repo"), None) + # Directory creation fails at the same stage, before any mkdir. + with pytest.raises(TypeError): + ensure_evidence_layout("evidence/demo") + + +def test_path_rules_create_nothing(tmp_path: Path) -> None: + root = campaign_evidence_root(tmp_path, "never-created") + paths = evidence_subpaths(root) + assert _tree(tmp_path) == [] + assert not root.exists() and not any(path.exists() for path in paths.values()) + + +def test_path_rules_emit_no_filesystem_event_and_directory_creation_does(tmp_path: Path) -> None: + environment = {**os.environ, "PYTHONPATH": str(SRC)} + result = subprocess.run( + [sys.executable, "-c", AUDIT, str(tmp_path)], capture_output=True, text=True, + encoding="utf-8", env=environment, cwd=tmp_path, timeout=120, check=False, + ) + assert result.returncode == 0, result.stderr + payload = json.loads(result.stdout) + assert payload["seen"]["rules"] == [] + # Positive control: the same hook saw the Apps half create directories. + assert "os.mkdir" in payload["seen"]["layout"] + assert payload["same"] is True + assert payload["relative"] == [f"evidence/demo/{name}" for name in EVIDENCE_SEGMENTS] + assert _tree(tmp_path) == ["evidence/", "evidence/demo/"] + sorted( + f"evidence/demo/{name}/" for name in EVIDENCE_SEGMENTS + ) + + +def test_contracts_evidence_source_holds_no_call_and_no_io_import() -> None: + tree = ast.parse((ROOT / "contracts" / "evidence.py").read_text(encoding="utf-8")) + # Path joins only: with no call expression the module cannot mkdir, stat or open. + assert [node for node in ast.walk(tree) if isinstance(node, ast.Call)] == [] + imports = [ + (node.module, [alias.name for alias in node.names]) + for node in ast.walk(tree) if isinstance(node, ast.ImportFrom) + ] + assert imports == [("__future__", ["annotations"]), ("pathlib", ["Path"])] + assert [node for node in ast.walk(tree) if isinstance(node, ast.Import)] == [] + assert sorted(name for name in vars(evidence) if not name.startswith("_")) == [ + "EVIDENCE_SEGMENTS", "Path", "annotations", "campaign_evidence_root", "evidence_subpaths", + ] + assert not hasattr(evidence, "ensure_evidence_layout") + + +def test_mkdir_lives_only_in_the_apps_module() -> None: + def mkdir_calls(path: Path) -> list[ast.Call]: + tree = ast.parse(path.read_text(encoding="utf-8")) + return [ + node for node in ast.walk(tree) + if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) + and node.func.attr in {"mkdir", "makedirs"} + ] + + for path in sorted((ROOT / "contracts").glob("*.py")): + assert mkdir_calls(path) == [], path.name + calls = mkdir_calls(ROOT / "apps" / "evidence_layout.py") + assert len(calls) == 1 and calls[0].args == [] + assert {key.arg: key.value.value for key in calls[0].keywords} == { + "parents": True, "exist_ok": True, + } + # The Apps module reuses the canonical rule; it holds no second copy of the segments. + assert evidence_layout.evidence_subpaths is evidence_subpaths + assert not hasattr(evidence_layout, "EVIDENCE_SEGMENTS") + + +def test_ensure_layout_creates_exactly_four_directories_and_returns_the_map( + tmp_path: Path, +) -> None: + root = tmp_path / "evidence" / "x" + paths = ensure_evidence_layout(root) + assert type(paths) is dict and list(paths) == list(EVIDENCE_SEGMENTS) + assert paths == evidence_subpaths(root) + assert all(path.is_dir() for path in paths.values()) + assert sorted(child.name for child in root.iterdir()) == sorted(EVIDENCE_SEGMENTS) + # Missing parents are created; nothing else appears. + assert _tree(tmp_path) == ["evidence/", "evidence/x/"] + sorted( + f"evidence/x/{name}/" for name in EVIDENCE_SEGMENTS + ) + + +def test_ensure_layout_is_idempotent_and_keeps_existing_content(tmp_path: Path) -> None: + root = tmp_path / "evidence" / "x" + first = ensure_evidence_layout(root) + (first["plan"] / "gates.md").write_text("frozen", encoding="utf-8") + before = _tree(tmp_path) + second = ensure_evidence_layout(root) + assert second == first and second is not first + assert _tree(tmp_path) == before + assert (first["plan"] / "gates.md").read_text(encoding="utf-8") == "frozen" + + +def test_ensure_layout_keeps_a_relative_root_relative(tmp_path: Path, monkeypatch) -> None: + monkeypatch.chdir(tmp_path) + paths = ensure_evidence_layout(Path("evidence") / "relative") + assert paths == {name: Path("evidence") / "relative" / name for name in EVIDENCE_SEGMENTS} + assert not any(path.is_absolute() for path in paths.values()) + assert all((tmp_path / path).is_dir() for path in paths.values()) + + +@pytest.mark.parametrize("blocker", ["plan", "patch", "validator"]) +def test_a_file_in_place_of_a_segment_fails_like_the_original_body( + tmp_path: Path, blocker: str +) -> None: + outcomes = [] + for label, function in (("reference", _reference), ("moved", ensure_evidence_layout)): + root = tmp_path / label / "evidence" / "x" + root.mkdir(parents=True) + (root / blocker).write_text("not a directory", encoding="utf-8") + with pytest.raises(OSError) as caught: + function(root) + outcomes.append((type(caught.value), caught.value.errno, _tree(tmp_path / label))) + assert (root / blocker).read_text(encoding="utf-8") == "not a directory" + assert outcomes[0] == outcomes[1] + assert outcomes[1][0] is FileExistsError + # Segments are created in order and a failure rolls nothing back. + moved = tmp_path / "moved" / "evidence" / "x" + created = [name for name in EVIDENCE_SEGMENTS if (moved / name).is_dir()] + assert created == list(EVIDENCE_SEGMENTS[: EVIDENCE_SEGMENTS.index(blocker)]) + + +def test_a_file_in_place_of_a_parent_fails_like_the_original_body(tmp_path: Path) -> None: + outcomes = [] + for label, function in (("reference", _reference), ("moved", ensure_evidence_layout)): + base = tmp_path / label + base.mkdir() + (base / "evidence").write_text("a file, not a directory", encoding="utf-8") + with pytest.raises(OSError) as caught: + function(base / "evidence" / "x") + outcomes.append((type(caught.value), caught.value.errno, _tree(base))) + # The exact OSError subclass is platform-specific; it is the same one as before. + assert outcomes[0] == outcomes[1] + assert outcomes[1][2] == ["evidence"] + + +def test_campaign_acceptance_template_lists_the_canonical_segments() -> None: + # The acceptance form consumes the segment names. Its ``evidence_paths`` key is a + # form field id, not the retired module path, and stays as it is. + text = (SRC.parent / ".github" / "ISSUE_TEMPLATE" / "campaign-acceptance.yml").read_text( + encoding="utf-8" + ) + lines = [f"{name}: evidence//{name}/" for name in EVIDENCE_SEGMENTS] + assert [line.strip() for line in text.splitlines() if line.strip() in lines] == lines + assert ", ".join(f"`{name}/`" for name in EVIDENCE_SEGMENTS) in text + assert "inferrsi." not in text + + +def test_retired_evidence_paths_module_has_no_alias() -> None: + assert not (ROOT / "evidence_paths.py").exists() + assert importlib.util.find_spec(RETIRED) is None + with pytest.raises(ModuleNotFoundError): + importlib.import_module(RETIRED) + assert RETIRED not in sys.modules + # A pickle that names a moved function by the retired module path no longer loads. + for name in ("campaign_evidence_root", "evidence_subpaths", "ensure_evidence_layout"): + with pytest.raises(ModuleNotFoundError): + pickle.loads(f"c{RETIRED}\n{name}\n.".encode("ascii")) + # New pickles name the canonical modules, and path values are plain data. + assert b"inferrsi.contracts.evidence" in pickle.dumps(evidence_subpaths, protocol=0) + assert pickle.loads(pickle.dumps(campaign_evidence_root)) is campaign_evidence_root + assert b"inferrsi.apps.evidence_layout" in pickle.dumps(ensure_evidence_layout, protocol=0) + assert pickle.loads(pickle.dumps(ensure_evidence_layout)) is ensure_evidence_layout + assert b"inferrsi" not in pickle.dumps(EVIDENCE_SEGMENTS, protocol=0) + assert b"inferrsi" not in pickle.dumps(evidence_subpaths(Path("root")), protocol=0) diff --git a/tests/test_first_wave_isolation_policy.py b/tests/test_first_wave_isolation_policy.py index 8a8e40469..dafe3a698 100644 --- a/tests/test_first_wave_isolation_policy.py +++ b/tests/test_first_wave_isolation_policy.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import Any +from tests import _gate_policy_support as support from tests._gate_policy_support import ( IsolationReport, blocker_kinds, @@ -35,19 +36,23 @@ "inferrsi.harness.execution_service", "inferrsi.harness.run_store", ) +# #451 C2: the leaf lives in ``inferrsi.resources``, whose initializer imports nothing. +LEAF_FILE = "resources/repository_packs.py" +RESOURCES_INIT = '"""Packaged resources."""\n' # Every first-wave trigger is cleared: no allowlist and no legacy mix. CLEARED_TREE = { "__init__.py": "", - "_repo_pack_data.py": LEAF_SOURCE, + "resources/__init__.py": RESOURCES_INIT, + LEAF_FILE: LEAF_SOURCE, "knowledge/__init__.py": "", - "knowledge/catalog.py": "from .._repo_pack_data import REPO_TYPES\n", + "knowledge/catalog.py": "from ..resources.repository_packs import REPO_TYPES\n", "benchmark/__init__.py": "", "apps/__init__.py": "", "apps/main.py": "from inferrsi.knowledge import catalog\nfrom inferrsi.harness import api\n", "harness/__init__.py": "", "harness/api.py": "VALUE = 1\n", } -# The bounded contracts package (#451 C1): the initializer's one export and three +# The bounded contracts package (#451 C1, C2): the initializer's one export and four # standalone modules. CONTRACTS_INIT = ( '"""Shared contracts."""\n\n' @@ -59,6 +64,7 @@ "contracts/json_v1.py": "import json\n", "contracts/runspec.py": "import math\n\n\nclass RunSpec:\n pass\n", "contracts/claim.py": "from enum import Enum\n", + "contracts/evidence.py": "from pathlib import Path\n", } def _shell(importer: str) -> str: @@ -77,14 +83,17 @@ def _k01_tree() -> dict[str, str]: """A K01-shaped tree that follows every first-wave rule.""" files = { "__init__.py": "", - "_repo_pack_data.py": LEAF_SOURCE, + "resources/__init__.py": RESOURCES_INIT, + LEAF_FILE: LEAF_SOURCE, **CONTRACTS_TREE, "knowledge/__init__.py": '"""Knowledge."""\n', # K.catalog -> the data leaf is intra-closure, not a tenth pair. - "knowledge/catalog.py": "from .. import _repo_pack_data\nfrom . import _source_snapshot\n", + "knowledge/catalog.py": ( + "from ..resources import repository_packs\nfrom . import _source_snapshot\n" + ), "knowledge/knowledge_operations.py": "from inferrsi.contracts import json_v1\n", "infra/__init__.py": "", - "infra/inspection.py": "from .._repo_pack_data import repository_pack\n", + "infra/inspection.py": "from ..resources.repository_packs import repository_pack\n", "harness/__init__.py": "", "harness/api.py": "VALUE = 1\n", "benchmark/__init__.py": "", @@ -136,6 +145,15 @@ def test_allowlist_is_exactly_nine_pairs(self) -> None: leaf_routes = {("inferrsi.catalog", LEAF), ("inferrsi.infra.inspection", LEAF)} self.assertEqual(len(MANIFEST["compatibility_pairs"]), 9) self.assertEqual(PAIRS, same_named | leaf_routes) + # #451 C2 redirected the two leaf routes to the canonical module and left + # nothing on the retired root path. + self.assertEqual(LEAF, "inferrsi.resources.repository_packs") + self.assertEqual( + MANIFEST["catalog_two_routes"]["neutral_leaf_export"]["target"], LEAF + ) + self.assertFalse( + [pair for pair in PAIRS if "inferrsi._repo_pack_data" in pair], PAIRS + ) def test_catalog_routes_keep_repository_pack_out_of_knowledge(self) -> None: catalog = SOURCES["inferrsi.catalog"] @@ -182,13 +200,21 @@ def test_second_importer_fails(self) -> None: ), ( "evaluation.py", - "from inferrsi._repo_pack_data import repository_pack\n", - "inferrsi.evaluation is a second importer of inferrsi._repo_pack_data", + "from inferrsi.resources.repository_packs import repository_pack\n", + "inferrsi.evaluation is a second importer of " + "inferrsi.resources.repository_packs", ), ( "harness/api.py", - "from inferrsi import _repo_pack_data\n", - "inferrsi.harness.api is a second importer of inferrsi._repo_pack_data", + "from inferrsi.resources import repository_packs\n", + "inferrsi.harness.api is a second importer of " + "inferrsi.resources.repository_packs", + ), + ( + "benchmark/probe.py", + "import inferrsi.resources.repository_packs\n", + "inferrsi.benchmark.probe is a second importer of " + "inferrsi.resources.repository_packs", ), ) for path, source, expected in cases: @@ -207,11 +233,14 @@ def test_only_declared_infra_importer_may_use_the_leaf(self) -> None: if importer.startswith("inferrsi.infra.") and target == LEAF } self.assertEqual(infra_leaf, {("inferrsi.infra.inspection", LEAF)}) - report = _report({"infra/other.py": "from .._repo_pack_data import REPO_TYPES\n"}) + report = _report( + {"infra/other.py": "from ..resources.repository_packs import REPO_TYPES\n"} + ) self.assertTrue( _violated( report, - "inferrsi.infra.other is a second importer of inferrsi._repo_pack_data", + "inferrsi.infra.other is a second importer of " + "inferrsi.resources.repository_packs", ), report.violations, ) @@ -303,17 +332,85 @@ def test_data_leaf_accepts_only_listed_stdlib_imports(self) -> None: (hidden, "imports inferrsi.catalog"), (LEAF_SOURCE.replace("import json\n", "import json\nimport inferrsi.infra\n"), "imports inferrsi.infra"), - (LEAF_SOURCE.replace("import json\n", "import json\nfrom . import catalog\n"), + # Relative forms resolve from the leaf's own package, inferrsi.resources. + (LEAF_SOURCE.replace("import json\n", "import json\nfrom .. import catalog\n"), "imports inferrsi.catalog"), + (LEAF_SOURCE.replace("import json\n", "import json\nfrom . import sibling\n"), + "imports inferrsi.resources.sibling"), + # The leaf never imports Harness to find the Harness-owned pack.json. + (LEAF_SOURCE.replace("import json\n", "import json\nfrom ..harness import api\n"), + "imports inferrsi.harness.api"), + (LEAF_SOURCE.replace("import json\n", "import json\nimport inferrsi.harness\n"), + "imports inferrsi.harness"), (dynamic, "has a dynamic import"), (LEAF_SOURCE.replace("import Any", "import Any, Dict"), "imports typing.Dict"), (LEAF_SOURCE + "\nEXTRA = 1\n", "defines EXTRA at top level"), ) for source, expected in cases: with self.subTest(expected=expected): - report = _report({"_repo_pack_data.py": source}) + report = _report({LEAF_FILE: source}) self.assertTrue(_violated(report, f"{LEAF} {expected}"), report.violations) + def test_leaf_parent_initializer_must_stay_import_free(self) -> None: + # #451 C2: Python runs inferrsi/resources/__init__.py with every leaf import. + self.assertEqual(_report({}).violations, ()) + dynamic = "import importlib\nimportlib.import_module('inferrsi.harness')\n" + cases = ( + ("from ..harness import api\n", "imports inferrsi.harness.api"), + ("from inferrsi.knowledge import catalog\n", "imports inferrsi.knowledge.catalog"), + ("import inferrsi.apps\n", "imports inferrsi.apps"), + ("from . import repository_packs\n", "imports inferrsi.resources.repository_packs"), + ("import json\n", "imports json"), + (dynamic, "has a dynamic import"), + ("__import__('inferrsi.harness')\n", "has a dynamic import"), + ("def __getattr__(name):\n raise AttributeError(name)\n", + "has a FunctionDef statement"), + ("VALUE = 1\n", "has a Assign statement"), + ("open('pack.json')\n", "has a Expr statement"), + ) + for snippet, expected in cases: + with self.subTest(snippet=snippet): + report = _report({"resources/__init__.py": RESOURCES_INIT + snippet}) + self.assertTrue( + _violated(report, "inferrsi.resources ", expected), report.violations + ) + + def test_core_import_in_the_leaf_parent_fails_every_closure_that_loads_it(self) -> None: + report = _report({"resources/__init__.py": "from ..harness import api\n"}) + for importer in ("inferrsi.infra.inspection", "inferrsi.knowledge.catalog"): + with self.subTest(importer=importer): + self.assertTrue( + _violated(report, f"{importer} reaches inferrsi.harness"), report.violations + ) + self.assertFalse(report.new_core_closure_isolated) + + def test_resources_prefix_exempts_no_adjacent_module(self) -> None: + # Knowledge may load the leaf and its initializer, not a sibling beside them. + report = _report( + { + "resources/extra.py": "VALUE = 1\n", + "knowledge/research.py": "from inferrsi.resources import extra\n", + } + ) + self.assertTrue( + _violated(report, "inferrsi.knowledge.research reaches inferrsi.resources.extra"), + report.violations, + ) + self.assertFalse(report.new_core_closure_isolated) + + def test_retired_root_leaf_is_outside_the_knowledge_closure(self) -> None: + # Adding the old root file back does not restore it as the leaf. + report = _report( + { + "_repo_pack_data.py": LEAF_SOURCE, + "knowledge/research.py": "from .. import _repo_pack_data\n", + } + ) + self.assertTrue( + _violated(report, "inferrsi.knowledge.research reaches inferrsi._repo_pack_data"), + report.violations, + ) + def test_infra_through_root_catalog_fails(self) -> None: report = _report({"infra/inspection.py": "from ..catalog import repository_pack\n"}) self.assertIn( @@ -387,6 +484,12 @@ def test_knowledge_names_only_the_encoder_inside_the_package(self) -> None: ), ("from ..contracts import claim\n", "imports inferrsi.contracts.claim;"), ("from inferrsi.contracts.claim import Verdict\n", "reaches inferrsi.contracts.claim"), + # #451 C2: the evidence path rules are a contracts module, not a Knowledge input. + ("from inferrsi.contracts import evidence\n", "imports inferrsi.contracts.evidence;"), + ( + "from ..contracts.evidence import EVIDENCE_SEGMENTS\n", + "reaches inferrsi.contracts.evidence", + ), ) for source, expected in refused: with self.subTest(source=source): @@ -471,12 +574,54 @@ def test_contract_modules_import_no_inferrsi_module(self) -> None: "inferrsi.contracts.claim imports inferrsi.contracts.json_v1", ), ("contracts/claim.py", dynamic, "inferrsi.contracts.claim has a dynamic import"), + # #451 C2: the path rules import no core, and never the Apps mkdir module. + ( + "contracts/evidence.py", + "from inferrsi.apps import evidence_layout\n", + "inferrsi.contracts.evidence imports inferrsi.apps.evidence_layout", + ), + ( + "contracts/evidence.py", + "from ..harness import api\n", + "inferrsi.contracts.evidence imports inferrsi.harness.api", + ), + ( + "contracts/evidence.py", + "from . import runspec\n", + "inferrsi.contracts.evidence imports inferrsi.contracts.runspec", + ), + ("contracts/evidence.py", dynamic, "inferrsi.contracts.evidence has a dynamic import"), ) for path, source, expected in cases: with self.subTest(expected=expected): - report = _report({path: source}) + report = _report( + {"apps/evidence_layout.py": "VALUE = 1\n", path: source} + ) self.assertTrue(_violated(report, expected), report.violations) + def test_apps_directory_creation_is_not_a_contracts_module(self) -> None: + # #451 C2: mkdir stays in Apps. The declared set gains evidence and nothing else. + self.assertIn("inferrsi.contracts.evidence", support.CONTRACTS_MODULES) + self.assertEqual(len(support.CONTRACTS_MODULES), 5) + self.assertFalse( + [name for name in support.CONTRACTS_MODULES if "evidence_layout" in name] + ) + clean = _report( + { + "apps/evidence_layout.py": ( + "from inferrsi.contracts.evidence import evidence_subpaths\n" + ) + } + ) + self.assertEqual(clean.violations, ()) + report = _report({"contracts/evidence_layout.py": "from pathlib import Path\n"}) + self.assertTrue( + _violated( + report, "inferrsi.contracts.evidence_layout is not a declared contracts module" + ), + report.violations, + ) + def test_core_reached_through_the_package_still_fails_its_importers(self) -> None: # The package grants no path: a core import inside it fails every closure that # loads it, here a new H entry and the Knowledge module that uses the encoder. diff --git a/tests/test_knowledge_compatibility_shells.py b/tests/test_knowledge_compatibility_shells.py index c6f14f547..3437563b5 100644 --- a/tests/test_knowledge_compatibility_shells.py +++ b/tests/test_knowledge_compatibility_shells.py @@ -2,6 +2,7 @@ from __future__ import annotations +import ast import hashlib import importlib import os @@ -33,19 +34,25 @@ def test_root_names_are_the_moved_objects(item): def test_catalog_routes_share_one_repo_types_tuple(): - from inferrsi import _repo_pack_data, catalog + from inferrsi import arguments, catalog from inferrsi.knowledge import catalog as knowledge_catalog + from inferrsi.resources import repository_packs - assert catalog.REPO_TYPES is _repo_pack_data.REPO_TYPES is knowledge_catalog.REPO_TYPES - assert catalog.repository_pack is _repo_pack_data.repository_pack + assert catalog.REPO_TYPES is repository_packs.REPO_TYPES is knowledge_catalog.REPO_TYPES + # The CLI argument surface takes the tuple through root catalog: still one object. + assert arguments.REPO_TYPES is repository_packs.REPO_TYPES + assert catalog.repository_pack is repository_packs.repository_pack assert not hasattr(knowledge_catalog, "repository_pack") assert knowledge_catalog.KNOWLEDGE_SCOPES["contribution"] == ( - *_repo_pack_data.REPO_TYPES, "vllm_ascend", "inferrsi" + *repository_packs.REPO_TYPES, "vllm_ascend", "inferrsi" ) def test_data_leaf_holds_the_recorded_fragment_and_locked_hash(): - data = (ROOT / "_repo_pack_data.py").read_bytes() + assert LEAF["module"] == "inferrsi.resources.repository_packs" + assert LEAF["file"] == "src/inferrsi/resources/repository_packs.py" + data = (REPO / LEAF["file"]).read_bytes() + assert data == (ROOT / "resources" / "repository_packs.py").read_bytes() assert hashlib.sha256(data).hexdigest() == LEAF["file_sha256"] # The body in the file today, and the P0b provenance text it came from. current = LEAF["current_fragment"] @@ -59,6 +66,82 @@ def test_data_leaf_holds_the_recorded_fragment_and_locked_hash(): assert revision["new_file_sha256"] == LEAF["file_sha256"] +def test_c2_relock_moves_the_leaf_without_changing_a_byte(): + # #451 C2: the K02 hash, the recorded old-source hash and the relocated file agree, + # so the move changed the path and nothing in the file. + moved = LEAF["module_path_revision"] + assert (moved["old_module"], moved["new_module"]) == ( + "inferrsi._repo_pack_data", LEAF["module"], + ) + assert (moved["old_file"], moved["new_file"]) == ( + "src/inferrsi/_repo_pack_data.py", LEAF["file"], + ) + assert moved["old_file_sha256"] == LEAF["resource_path_revision"]["new_file_sha256"] + assert moved["old_file_sha256"] == moved["new_file_sha256"] == LEAF["file_sha256"] + assert not (REPO / moved["old_file"]).exists() + assert hashlib.sha256((REPO / moved["new_file"]).read_bytes()).hexdigest() == ( + moved["new_file_sha256"] + ) + # The review requirement is recorded, never an approval. + assert "requires formal architecture review" in moved["review"] + assert "does not self-approve" in moved["review"] + superseded = MANIFEST["revision_history"][-1] + assert superseded["stage"] == "K02" + assert superseded["changed_technical_leaf"] == { + "module": moved["old_module"], "file": moved["old_file"], + } + assert MANIFEST["base_commit"] in superseded["superseded_by"] + + +def _leaf_imports(tree: ast.Module) -> set[str]: + found = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + found.update(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom): + assert node.level == 0, "the leaf has no relative import" + found.update(f"{node.module}.{alias.name}" for alias in node.names) + return found + + +def test_relocated_leaf_shape_matches_the_relocked_manifest_exactly(): + tree = ast.parse((REPO / LEAF["file"]).read_text(encoding="utf-8")) + # allowed_imports is the exact import set of the relocated file, not a superset. + assert _leaf_imports(tree) == { + "json", "importlib.resources.files", "typing.Any", "__future__.annotations", + } + assert LEAF["allowed_imports"] == [ + "json", "importlib.resources", "typing.Any", "from __future__ import annotations", + ] + # Top level: the future import, the three stdlib imports, then the two symbols. + kinds = [type(node).__name__ for node in tree.body] + assert kinds == ["ImportFrom", "Import", "ImportFrom", "ImportFrom", "Assign", "FunctionDef"] + assert tree.body[4].targets[0].id == "REPO_TYPES" + assert tree.body[5].name == "repository_pack" + assert LEAF["two_symbols"] == ["REPO_TYPES", "repository_pack"] + assert "three listed stdlib imports" in LEAF["top_level_shape"] + # The anchor is the root package joined with the physical Harness path. + anchor = 'files("inferrsi").joinpath("harness", "data", "repo_packs", repo_type, "pack.json")' + assert anchor in LEAF["current_fragment"]["source"] + # The parent initializer is a docstring and nothing else. + initializer = REPO / LEAF["module_path_revision"]["parent_initializer"] + assert initializer == ROOT / "resources" / "__init__.py" + body = ast.parse(initializer.read_text(encoding="utf-8")).body + assert len(body) == 1 and isinstance(body[0], ast.Expr) + assert isinstance(body[0].value, ast.Constant) and isinstance(body[0].value.value, str) + assert sorted(path.name for path in initializer.parent.iterdir() if path.is_file()) == [ + "__init__.py", "repository_packs.py", + ] + + +def test_infra_inspection_imports_the_leaf_by_the_recorded_line(): + rewrite = MANIFEST["infra_inspection_rewrite"] + lines = (REPO / rewrite["module"]).read_text(encoding="utf-8").splitlines() + assert lines[9] == rewrite["canonical_import"] + assert rewrite["canonical_import"] == "from ..resources.repository_packs import repository_pack" + assert not [line for line in lines if "_repo_pack_data" in line or "catalog" in line] + + def _fresh(script: str) -> None: environment = {**os.environ, "PYTHONPATH": str(REPO / "src")} result = subprocess.run( @@ -76,7 +159,12 @@ def test_package_init_and_infra_inspection_load_no_knowledge_body(): "import inferrsi.infra.inspection\n" "assert 'inferrsi.catalog' not in sys.modules\n" "assert not [m for m in sys.modules if m.startswith('inferrsi.knowledge.')]\n" - "assert 'inferrsi._repo_pack_data' in sys.modules\n" + "assert 'inferrsi.resources.repository_packs' in sys.modules\n" + "assert 'inferrsi._repo_pack_data' not in sys.modules\n" + # Reading a Harness-owned pack never loads Harness. + "from inferrsi.infra.inspection import repository_pack\n" + "assert repository_pack('vllm')['id'] == 'vllm'\n" + "assert not [m for m in sys.modules if m.startswith('inferrsi.harness')]\n" ) diff --git a/tests/test_package_root_inventory.py b/tests/test_package_root_inventory.py index e22c35c26..b78df8ab4 100644 --- a/tests/test_package_root_inventory.py +++ b/tests/test_package_root_inventory.py @@ -1,8 +1,8 @@ -"""Exact inventory of the files directly under ``src/inferrsi/`` (#451 C1). +"""Exact inventory of the files directly under ``src/inferrsi/`` (#451 C1, C2). The flat package root is being retired. This guard pins the root to the exact file -set left after C1 and rejects any other root file. Each later removal PR (C2, R1, -A1/A2, M1) shrinks ``ROOT_FILES`` in the same change, until only ``__init__.py`` and +set left after C2 and rejects any other root file. Each later removal PR (R1, A1/A2, +M1) shrinks ``ROOT_FILES`` in the same change, until only ``__init__.py`` and ``__main__.py`` remain. It does not assert that final state early. """ @@ -15,14 +15,16 @@ REPO = Path(__file__).resolve().parents[1] PACKAGE = "src/inferrsi" -# The 27 tracked root files before C1, minus the four that C1 moved into contracts/. +# The 27 tracked root files before C1, minus the four that C1 moved into contracts/ +# and the two that C2 moved into resources/, contracts/ and apps/. RETIRED_BY_C1 = frozenset({"_json_v1.py", "claim.py", "contracts.py", "runspec.py"}) +RETIRED_BY_C2 = frozenset({"_repo_pack_data.py", "evidence_paths.py"}) +RETIRED = RETIRED_BY_C1 | RETIRED_BY_C2 ROOT_FILES = frozenset( { "__init__.py", "__main__.py", "_mindie_read_worker.py", - "_repo_pack_data.py", "_skill_selection.py", "_source_snapshot.py", "arguments.py", @@ -30,7 +32,6 @@ "catalog.py", "cli.py", "evaluation.py", - "evidence_paths.py", "execution.py", "inspection.py", "knowledge_operations.py", @@ -81,11 +82,21 @@ def root_files(repo: Path) -> set[str]: return present - _listed(repo, "--deleted") -def test_inventory_is_the_exact_post_c1_set() -> None: - assert len(ROOT_FILES) == 23 - assert not ROOT_FILES & RETIRED_BY_C1 +def test_inventory_is_the_exact_post_c2_set() -> None: + assert len(ROOT_FILES) == 21 + assert len(RETIRED_BY_C1) == 4 and len(RETIRED_BY_C2) == 2 + assert not ROOT_FILES & RETIRED + # 27 before C1, 23 after C1, 21 after C2. + assert len(ROOT_FILES | RETIRED) == 27 + assert len(ROOT_FILES | RETIRED_BY_C2) == 23 assert GENERATED not in ROOT_FILES assert {"__init__.py", "__main__.py"} <= ROOT_FILES + # C2 moved its two files; it left nothing behind under either old name. + assert (REPO / PACKAGE / "resources" / "repository_packs.py").is_file() + assert (REPO / PACKAGE / "contracts" / "evidence.py").is_file() + assert (REPO / PACKAGE / "apps" / "evidence_layout.py").is_file() + for name in RETIRED: + assert not (REPO / PACKAGE / name).exists(), name def test_on_disk_root_matches_the_inventory() -> None: @@ -104,7 +115,7 @@ def test_tracked_root_matches_the_inventory() -> None: assert ignored.returncode == 0 -@pytest.mark.parametrize("name", ["helper.py", "py.typed", GENERATED, *sorted(RETIRED_BY_C1)]) +@pytest.mark.parametrize("name", ["helper.py", "py.typed", GENERATED, *sorted(RETIRED)]) def test_an_added_root_file_is_rejected(name: str) -> None: assert inventory_errors(set(ROOT_FILES) | {name}) == [ f"unexpected root file {PACKAGE}/{name}: a new module belongs in a subpackage" @@ -116,8 +127,19 @@ def test_a_removed_root_file_must_shrink_the_inventory() -> None: f"missing root file {PACKAGE}/cli.py: shrink ROOT_FILES in the PR that removes it" ] # A later removal PR passes once it shrinks the expected set with the files. - smaller = ROOT_FILES - {"_repo_pack_data.py", "evidence_paths.py"} + smaller = ROOT_FILES - {"catalog.py", "inspection.py"} assert inventory_errors(set(smaller), smaller) == [] + # C2 itself: the 23-file set it started from no longer matches the 21-file root. + before_c2 = ROOT_FILES | RETIRED_BY_C2 + assert inventory_errors(set(ROOT_FILES), before_c2) == [ + f"missing root file {PACKAGE}/{name}: shrink ROOT_FILES in the PR that removes it" + for name in sorted(RETIRED_BY_C2) + ] + # Keeping either old file beside its moved implementation is rejected. + assert inventory_errors(set(before_c2)) == [ + f"unexpected root file {PACKAGE}/{name}: a new module belongs in a subpackage" + for name in sorted(RETIRED_BY_C2) + ] def test_git_view_reports_root_changes_before_and_after_staging(tmp_path: Path) -> None: diff --git a/tests/test_repository_packs_facade.py b/tests/test_repository_packs_facade.py new file mode 100644 index 000000000..32a82a8ab --- /dev/null +++ b/tests/test_repository_packs_facade.py @@ -0,0 +1,238 @@ +"""The neutral repository-pack facade (#451 C2). + +``inferrsi.resources.repository_packs`` replaces root ``_repo_pack_data.py``. It holds +the one ``REPO_TYPES`` tuple and the ``pack.json`` reader, imports no ``inferrsi`` +module, and sits under an import-free initializer. The retired root path has no alias. +""" + +from __future__ import annotations + +import argparse +import ast +import hashlib +import importlib +import importlib.util +import json +import os +import pickle +import subprocess +import sys +from importlib.resources import files +from pathlib import Path + +import pytest + +import inferrsi.resources as resources +from inferrsi import arguments, catalog +from inferrsi.infra import inspection +from inferrsi.knowledge import catalog as knowledge_catalog +from inferrsi.resources import repository_packs +from tests.test_contracts_package import IO_CONTROL, IO_PROBE + +SRC = Path(__file__).resolve().parents[1] / "src" +ROOT = SRC / "inferrsi" +PACKS = ROOT / "harness" / "data" / "repo_packs" +SEVEN = ("vllm", "vllm_omni", "afd_plugin", "vllm_rlt", "strata", "sglang", "sglang_omni") +RETIRED = "inferrsi._repo_pack_data" +FACADE_CLOSURE = {"inferrsi", "inferrsi.resources", "inferrsi.resources.repository_packs"} +CORES = ("inferrsi.harness", "inferrsi.knowledge", "inferrsi.catalog", "inferrsi.infra", + "inferrsi.apps", "inferrsi.benchmark", "inferrsi.contracts") +# Every other inferrsi import is refused, so the facade works only if it needs none. +BLOCKED = """ +import importlib.abc, json, sys +ALLOWED = {"inferrsi", "inferrsi._version", "inferrsi.resources", + "inferrsi.resources.repository_packs"} +class Block(importlib.abc.MetaPathFinder): + def find_spec(self, name, path=None, target=None): + if name.split(".")[0] == "inferrsi" and name not in ALLOWED: + raise ImportError("blocked inferrsi import: " + name) + return None +sys.meta_path.insert(0, Block()) +def loaded(): + return sorted(name for name in sys.modules if name.split(".")[0] == "inferrsi") +import inferrsi.resources +parent_only = loaded() +parent_public = sorted(name for name in vars(inferrsi.resources) if not name.startswith("_")) +from inferrsi.resources import repository_packs +packs = {name: repository_packs.repository_pack(name) for name in repository_packs.REPO_TYPES} +after_reads = loaded() +refused = {} +for name in json.loads(sys.argv[1]): + try: + __import__(name) + except ImportError as error: + refused[name] = str(error) +print(json.dumps({ + "parent_only": parent_only, "parent_public": parent_public, "loaded": after_reads, + "types": list(repository_packs.REPO_TYPES), "packs": packs, "refused": refused, +})) +""" +# The resource reader is loaded before the hook, so the events are the read itself. +READ = """ +import importlib.readers, json, os, sys +from inferrsi.resources import repository_packs +IO_EVENTS = ("os.", "shutil.", "pathlib.", "tempfile.", "glob.", "subprocess.", "socket.") +seen = [] +active = [] +def hook(event, args): + if active and (event == "open" or event.startswith(IO_EVENTS)): + path = args[0] if args else None + path = os.fsdecode(path) if isinstance(path, (str, bytes, os.PathLike)) else None + mode = args[1] if event == "open" and len(args) > 1 else None + seen.append([event, path, mode if isinstance(mode, str) else None]) +sys.addaudithook(hook) +def loaded(): + return sorted(name for name in sys.modules if name.split(".")[0] == "inferrsi") +before = loaded() +active.append(True) +pack = repository_packs.repository_pack(sys.argv[1]) +active.clear() +print(json.dumps({"seen": seen, "before": before, "after": loaded(), "pack": pack})) +""" + + +def _fresh(script: str, *args: str) -> dict: + environment = {**os.environ, "PYTHONPATH": str(SRC)} + result = subprocess.run( + [sys.executable, "-c", script, *args], capture_output=True, text=True, + encoding="utf-8", env=environment, timeout=120, check=False, + ) + assert result.returncode == 0, result.stderr + return json.loads(result.stdout) + + +def _without_version(names) -> set[str]: + return set(names) - {"inferrsi._version"} + + +def test_one_repo_types_object_reaches_every_consumer() -> None: + canonical = repository_packs.REPO_TYPES + assert canonical == SEVEN and type(canonical) is tuple and len(canonical) == 7 + assert catalog.REPO_TYPES is canonical + assert knowledge_catalog.REPO_TYPES is canonical + assert arguments.REPO_TYPES is canonical + assert catalog.repository_pack is repository_packs.repository_pack + assert inspection.repository_pack is repository_packs.repository_pack + assert not hasattr(knowledge_catalog, "repository_pack") + assert knowledge_catalog.KNOWLEDGE_SCOPES["contribution"] == ( + *canonical, "vllm_ascend", "inferrsi" + ) + # The active argument surface offers that same object as its choices. + parser = argparse.ArgumentParser() + arguments._common(parser) + action = next(item for item in parser._actions if item.dest == "repo_type") + assert action.choices is canonical + + +def test_only_the_facade_defines_the_tuple_and_the_reader() -> None: + tuples, readers = [], [] + for path in sorted(ROOT.rglob("*.py")): + relative = path.relative_to(ROOT).as_posix() + for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))): + if isinstance(node, (ast.Assign, ast.AnnAssign, ast.AugAssign)): + targets = node.targets if isinstance(node, ast.Assign) else [node.target] + if any(isinstance(item, ast.Name) and item.id == "REPO_TYPES" for item in targets): + tuples.append(relative) + elif isinstance(node, ast.FunctionDef) and node.name == "repository_pack": + readers.append(relative) + assert tuples == ["resources/repository_packs.py"] + assert readers == ["resources/repository_packs.py"] + + +@pytest.mark.parametrize("repo_type", SEVEN) +def test_each_pack_parses_to_its_packaged_bytes(repo_type: str) -> None: + data = (PACKS / repo_type / "pack.json").read_bytes() + anchored = files("inferrsi").joinpath("harness", "data", "repo_packs", repo_type, "pack.json") + assert hashlib.sha256(anchored.read_bytes()).hexdigest() == hashlib.sha256(data).hexdigest() + pack = repository_packs.repository_pack(repo_type) + assert pack == json.loads(data.decode("utf-8")) + assert type(pack) is dict and pack["id"] == repo_type + # Each call parses again; callers never share one mutable dict. + assert repository_packs.repository_pack(repo_type) is not pack + assert catalog.repository_pack(repo_type) == pack + + +def test_the_seven_packs_are_the_whole_harness_resource_root() -> None: + found = sorted( + path.relative_to(PACKS).as_posix() for path in PACKS.rglob("*") if path.is_file() + ) + assert found == sorted(f"{name}/pack.json" for name in SEVEN) + + +@pytest.mark.parametrize( + "repo_type", ["not-a-repository", "xdit", "", "VLLM", "../vllm", "vllm/", None, 7] +) +def test_unknown_repository_type_fails_before_any_resource_access(repo_type, monkeypatch) -> None: + def no_access(*args, **kwargs): + raise AssertionError("an unknown repository type must not reach the resource") + + monkeypatch.setattr(repository_packs, "files", no_access) + with pytest.raises(ValueError) as caught: + repository_packs.repository_pack(repo_type) + assert str(caught.value) == f"Unknown repository type: {repo_type}" + + +def test_facade_and_parent_load_under_a_hook_that_blocks_every_other_inferrsi_import() -> None: + result = _fresh(BLOCKED, json.dumps(CORES)) + # The initializer alone loads nothing and binds no public name. + assert _without_version(result["parent_only"]) == {"inferrsi", "inferrsi.resources"} + assert result["parent_public"] == [] + # All seven packs were read with only the facade and its parent loaded. + assert _without_version(result["loaded"]) == FACADE_CLOSURE + assert tuple(result["types"]) == SEVEN + for name in SEVEN: + expected = json.loads((PACKS / name / "pack.json").read_bytes().decode("utf-8")) + assert result["packs"][name] == expected, name + # Positive control: the hook is live and refuses every core, Harness included. + assert result["refused"] == {name: f"blocked inferrsi import: {name}" for name in CORES} + + +def test_module_bodies_do_no_io_on_import() -> None: + modules = ("inferrsi.resources", "inferrsi.resources.repository_packs", + "inferrsi.apps.evidence_layout") + result = _fresh(IO_PROBE, json.dumps(modules), IO_CONTROL) + seen, defined = result["seen"], result["defined"] + assert {"os.listdir", "open"} <= set(seen.pop("control")) + assert seen == {name: [] for name in modules} + assert defined["inferrsi.resources"] == [] + assert {"REPO_TYPES", "repository_pack"} <= set(defined["inferrsi.resources.repository_packs"]) + assert "ensure_evidence_layout" in defined["inferrsi.apps.evidence_layout"] + assert sorted(name for name in vars(resources) if not name.startswith("_")) == [ + "repository_packs" + ] + + +@pytest.mark.parametrize("repo_type", SEVEN) +def test_a_read_opens_only_the_requested_pack_and_loads_no_module(repo_type: str) -> None: + result = _fresh(READ, repo_type) + package = os.path.normcase(str(ROOT)) + expected = os.path.normcase(str(PACKS / repo_type / "pack.json")) + # Inside the package tree the read is one open of the requested file: no listing, + # no second pack, no write. + inside = [ + (event, os.path.normcase(os.path.abspath(path)), mode) + for event, path, mode in result["seen"] + if path is not None and os.path.normcase(os.path.abspath(path)).startswith(package) + ] + assert inside == [("open", expected, "r")] + assert result["before"] == result["after"] + assert _without_version(result["after"]) == FACADE_CLOSURE + assert result["pack"]["id"] == repo_type + + +def test_retired_root_module_has_no_alias() -> None: + assert not (ROOT / "_repo_pack_data.py").exists() + assert importlib.util.find_spec(RETIRED) is None + with pytest.raises(ModuleNotFoundError): + importlib.import_module(RETIRED) + assert RETIRED not in sys.modules + assert not hasattr(sys.modules["inferrsi"], "_repo_pack_data") + # A pickle that names the reader by the retired module path no longer loads. + with pytest.raises(ModuleNotFoundError): + pickle.loads(f"c{RETIRED}\nrepository_pack\n.".encode("ascii")) + reader = repository_packs.repository_pack + assert reader.__module__ == "inferrsi.resources.repository_packs" + assert b"inferrsi.resources.repository_packs" in pickle.dumps(reader, protocol=0) + assert pickle.loads(pickle.dumps(reader)) is reader + # The tuple pickles by value, so stored REPO_TYPES values are unaffected by the move. + assert b"inferrsi" not in pickle.dumps(repository_packs.REPO_TYPES, protocol=0) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index f572242f7..011cf9697 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -6,7 +6,7 @@ from inferrsi import __version__ from inferrsi.contracts import RunSpec -from inferrsi.evidence_paths import EVIDENCE_SEGMENTS, evidence_subpaths +from inferrsi.contracts.evidence import EVIDENCE_SEGMENTS, evidence_subpaths def test_version_is_string() -> None: diff --git a/tests/test_target_module_import_gate.py b/tests/test_target_module_import_gate.py index a174048c3..c9ef0281f 100644 --- a/tests/test_target_module_import_gate.py +++ b/tests/test_target_module_import_gate.py @@ -73,23 +73,32 @@ def test_clean_scan_is_not_isolation(self) -> None: # Facts that hold at every stage. self.assertTrue((ROOT / "benchmark" / "registry.py").is_file()) self.assertTrue((ROOT / "harness").is_dir()) - # #451 C1: the shared data/byte contracts are one bounded package with exactly + # #451 C1, C2: the shared data/byte contracts are one bounded package with exactly # these files. The single-file re-export and its three root siblings are retired. self.assertEqual( sorted(path.name for path in (ROOT / "contracts").iterdir() if path.is_file()), - ["__init__.py", "claim.py", "json_v1.py", "runspec.py"], + ["__init__.py", "claim.py", "evidence.py", "json_v1.py", "runspec.py"], ) - for retired in ("contracts.py", "_json_v1.py", "runspec.py", "claim.py"): + for retired in ("contracts.py", "_json_v1.py", "runspec.py", "claim.py", + "_repo_pack_data.py", "evidence_paths.py"): self.assertFalse((ROOT / retired).exists(), retired) self.assertEqual( support.CONTRACTS_MODULES, { "inferrsi.contracts", "inferrsi.contracts.claim", + "inferrsi.contracts.evidence", "inferrsi.contracts.json_v1", "inferrsi.contracts.runspec", }, ) + # #451 C2: the neutral leaf package holds the import-free initializer and the + # leaf. Directory creation is an Apps module, outside the contracts set. + self.assertEqual( + sorted(path.name for path in (ROOT / "resources").iterdir() if path.is_file()), + ["__init__.py", "repository_packs.py"], + ) + self.assertTrue((ROOT / "apps" / "evidence_layout.py").is_file()) # A01-4 staged model: packages may appear one at a time, but each # remaining trigger keeps overall false. report = first_wave_report() From def0557ff99ddc804a7084710003eb5a80b7e04b Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Sun, 15 Nov 2026 23:33:14 +0800 Subject: [PATCH 2/2] docs: finish C2 contract and resource migration notes Agent-assisted-by: Claude Code (author) Signed-off-by: GUOGUO --- CHANGELOG.md | 45 +++++++++++++++++++-- docs/architecture.md | 12 ++++-- docs/contracts/current-design.md | 7 ++-- docs/module-layout.md | 68 ++++++++++++++++++++++++++------ 4 files changed, 111 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44acd319f..1bf42a69e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,44 @@ ### Changed +- Repository-pack access and the evidence path rules leave the package root + (Refs #451). Root `_repo_pack_data.py` and `evidence_paths.py` are removed in the + same change, with no forwarding shells or `sys.modules` aliases: + - `inferrsi._repo_pack_data` → `inferrsi.resources.repository_packs` + (`REPO_TYPES`, `repository_pack`). + - `inferrsi.evidence_paths` → `inferrsi.contracts.evidence` for + `EVIDENCE_SEGMENTS`, `campaign_evidence_root` and `evidence_subpaths`, and + `inferrsi.apps.evidence_layout` for `ensure_evidence_layout`. + - Importing either old module path fails, and a pickle that names one of these + functions by an old module path no longer loads. Such external pickles need + explicit migration or the prior release; new pickles name the canonical + modules above. `src/` has no pickle usage, and frozen evidence is not + rewritten. + - Signatures, return values and errors are unchanged. `REPO_TYPES` stays one + seven-entry tuple object for every remaining consumer: root `catalog`, + `knowledge.catalog` and the CLI arguments. The reader keeps the anchor + `files("inferrsi").joinpath("harness", "data", "repo_packs", ...)` and imports + no `inferrsi` module, and `resources/__init__.py` imports nothing. The seven + `pack.json` files keep their bytes and stay under `harness/data/`. + - `contracts.evidence` only builds paths and does no filesystem I/O. + `apps.evidence_layout` creates the four directories in the same order with + the same `mkdir(parents=True, exist_ok=True)` call. `contracts/__init__.py` + still re-exports only `RunSpec`. + - `docs/legacy-knowledge-manifest.json` relocks the neutral leaf's module and + file path and points the `catalog` and `infra.inspection` pairs at the new + module. The leaf's bytes and locked hash value are unchanged, and the nine + compatibility pairs remain. That relock requires formal architecture review: + passing tests do not supply it, and the implementer does not self-approve it. + - The exact root inventory in `tests/test_package_root_inventory.py` shrinks + from 23 to 21 tracked files: `__init__.py`, `__main__.py`, + `_mindie_read_worker.py`, `_skill_selection.py`, `_source_snapshot.py`, + `arguments.py`, `benchmark_operations.py`, `catalog.py`, `cli.py`, + `evaluation.py`, `execution.py`, `inspection.py`, `knowledge_operations.py`, + `knowledge_store.py`, `mindie_context.py`, `mindie_mcp.py`, + `mindie_project.py`, `reporting.py`, `research.py`, `skill_catalog.py` and + `summary_chart.py`. It is an exact set that each later removal shrinks, not + the final two-file root. The build-generated `_version.py` is wheel metadata, + not a tracked root file. - Shared technical contracts now live in `inferrsi.contracts` (Refs #451). The four root files `_json_v1.py`, `runspec.py`, `claim.py`, and `contracts.py` are removed in the same change. Canonical implementations are `contracts/json_v1.py`, @@ -17,9 +55,10 @@ prior release; new pickles name the canonical implementation modules. - Canonical JSON bytes, error stages, RunSpec serialization/validation, and Verdict values are unchanged. Frozen evidence is not rewritten. - - `tests/test_package_root_inventory.py` locks the exact current 23-file root - inventory and rejects unexpected additions; each later removal must shrink - that set in its own PR, rather than requiring the final two-file root now. + - `tests/test_package_root_inventory.py` locked the exact 23-file root + inventory that this change left and rejects unexpected additions; each later + removal must shrink that set in its own PR, rather than requiring the final + two-file root now. The entry above shrinks it to 21. - The `inferrsi.observability` package is dissolved into its owning packages. No forwarding modules remain, so imports must use the new paths: - `campaign_records`, `campaign_trace`, `optimization_log`, `optimization_trace`, diff --git a/docs/architecture.md b/docs/architecture.md index 7a784a213..67ca5fba5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -169,17 +169,23 @@ src/inferrsi/ run.py Old run composed from Harness, Benchmark and the report evaluation.py Old evaluate entry: RunSpec to the Benchmark gates reporting.py HTML evidence report - runspec.py, contracts.py Immutable experiment contract and compatibility export + contracts/ + runspec.py Immutable experiment contract; the initializer re-exports only RunSpec + json_v1.py Canonical inferrsi-json-v1 encoder and decoder + claim.py Campaign verdict labels PASS/FAIL/INCONCLUSIVE; rules only + evidence.py Evidence segments and per-campaign path rules; no filesystem I/O + resources/ + __init__.py Import-free initializer + repository_packs.py Neutral REPO_TYPES and pack.json reader; imports no inferrsi module catalog.py Knowledge search and cited context skill_catalog.py Packaged/default or custom skill metadata/body/resource resolution knowledge_operations.py Normalized reads and bounded Collect (KnowledgeReader) knowledge_store.py Authorized local store Create/Update (KnowledgeStore) research.py Read-only model-aware discovery joined over catalog queries _skill_selection.py Deterministic lexical skill ranking and context view - claim.py Campaign verdict labels PASS/FAIL/INCONCLUSIVE; rules only - evidence_paths.py Per-campaign evidence tree: plan/results/patch/validator summary_chart.py Stdlib SVG gain chart (chain/branch) and report figure spec apps/ + evidence_layout.py Create the plan/results/patch/validator evidence directories campaign.py Knowledge values connected to Harness campaign records views/ optimization_chart.py Managed optimization objective/token curves and final result diff --git a/docs/contracts/current-design.md b/docs/contracts/current-design.md index 3d4fe0aae..541fd47e3 100644 --- a/docs/contracts/current-design.md +++ b/docs/contracts/current-design.md @@ -25,15 +25,16 @@ Historical candidate text, review dispositions, and the earlier Proposed drafts ## D001 — One distribution, three cores plus Apps, document contracts rather than shared business DTOs -Decision: keep Knowledge, Harness, and Benchmark as sibling independent cores; Apps is the only assembler. Each core owns its types. Cross-boundary traffic uses versioned JSON envelopes with explicit conversion; there is no shared business `contracts` package. The technical-only `inferrsi.contracts` package re-exports only `RunSpec`, defined once in `contracts/runspec.py`; `contracts/claim.py` owns the same Verdict enum re-exported by Harness. These are not shared business DTOs. The old root `contracts.py`, `runspec.py`, `claim.py`, and `_json_v1.py` files are retired without forwarding shells. +Decision: keep Knowledge, Harness, and Benchmark as sibling independent cores; Apps is the only assembler. Each core owns its types. Cross-boundary traffic uses versioned JSON envelopes with explicit conversion; there is no shared business `contracts` package. The technical-only `inferrsi.contracts` package re-exports only `RunSpec`, defined once in `contracts/runspec.py`; `contracts/claim.py` owns the same Verdict enum re-exported by Harness. These are not shared business DTOs. The old root `contracts.py`, `runspec.py`, `claim.py`, and `_json_v1.py` files are retired without forwarding shells, and so are the old root `_repo_pack_data.py` and `evidence_paths.py`. Permitted shared technical leaves are only: - `inferrsi.contracts.json_v1` — the canonical encoder (D002); - `inferrsi.contracts.runspec` and `inferrsi.contracts.claim` — the existing RunSpec and Verdict technical types, with only RunSpec re-exported by the package initializer; -- `inferrsi._repo_pack_data` — the neutral raw pack-data reader and the existing static data enumeration (D020). +- `inferrsi.contracts.evidence` — the existing evidence segment names and pure path rules; it reads and writes no filesystem state and is not re-exported by the package initializer; +- `inferrsi.resources.repository_packs` — the neutral raw pack-data reader and the existing static data enumeration (D020), formerly root `inferrsi._repo_pack_data`. Its parent initializer `resources/__init__.py` imports nothing. -The neutral leaf carries no target selection, task, scoring, scheduling, or public business DTO, and imports none of K/H/B/Apps. +The neutral leaf carries no target selection, task, scoring, scheduling, or public business DTO, and imports none of K/H/B/Apps. It still finds the Harness-owned `pack.json` through `files("inferrsi").joinpath("harness", "data", "repo_packs", ...)`, so reading it never imports Harness. Creating the evidence directories is host I/O and stays in Apps, in `inferrsi.apps.evidence_layout`; that module is not a shared leaf. The list is exact: a `contracts.` or `resources.` prefix admits no other module. Why: this matches the accepted A–D direction and removes the reverse dependency that a shared DTO package creates. Rejected: a shared business DTO package; Apps owning scoring/sampling/budget algorithms; creating empty packages to declare migration done. diff --git a/docs/module-layout.md b/docs/module-layout.md index 36a975915..173630a6a 100644 --- a/docs/module-layout.md +++ b/docs/module-layout.md @@ -24,8 +24,25 @@ The four target modules are peers: `knowledge`, `harness`, `benchmark`, and `apps`. Knowledge, Harness, and Benchmark do not import one another. Apps is the only assembler. `src/inferrsi/contracts/__init__.py` re-exports only `RunSpec`, defined once in `contracts/runspec.py`. The technical-only package -also owns `json_v1.py` and `claim.py`; it contains no shared business DTOs -and performs no core or host initialization. +also owns `json_v1.py`, `claim.py` and `evidence.py`; it contains no shared +business DTOs and performs no core or host initialization. + +`contracts/evidence.py` holds the evidence segment names and the per-campaign +path rules. It only builds paths and does no filesystem I/O. Creating the +directories is `apps/evidence_layout.py`, which reuses those rules, so `mkdir` +stays out of the contracts package. `resources/repository_packs.py` is the +neutral repository-pack facade: the one `REPO_TYPES` tuple and the `pack.json` +reader. It imports no `inferrsi` module, and `resources/__init__.py` imports +nothing. Neither package is a fourth core. C2 (#451) moved these out of root +`_repo_pack_data.py` and `evidence_paths.py` and deleted both root files, with +no forwarding shell or alias. + +The flat package root is retired in stages. +`tests/test_package_root_inventory.py` pins the root to an exact file set: 23 +tracked files after C1 and 21 after C2. Each later removal PR shrinks that set +in the same change. The guard does not assert the final two-file root +(`__init__.py` and `__main__.py`) early, and the build-generated `_version.py` +is ignored wheel metadata, not a tracked root file. The current `src/inferrsi/harness/` tree is a mixed implementation. Its presence is not the target Harness boundary and is not evidence that the four @@ -95,10 +112,11 @@ Several flat modules remain shared seams until their slice starts. | `apps/legacy/` | Old run composition, old `evaluate` entry and HTML report | Supplies the Git source snapshot from `infra/targets`; path follows the confirmed split, #371 still proposed | | `evaluation.py`, `execution.py`, `reporting.py` | Root forwarders of the old run chain | Each forwards to one `apps.legacy` module; retired by R01 | | `model/validators/` | Benchmark-leaning checks | Not moved | -| `infra/`, `runspec.py`, `inspection.py` | Seams used by the current tree | Not a fourth core | -| `summary_chart.py`, `evidence_paths.py` | Current report helpers | Not moved | -| `claim.py` | Verdict vocabulary used by current checks | Not moved and not a fourth core | -| `contracts.py` | `RunSpec` compatibility re-export | Must remain one module file | +| `infra/`, `inspection.py` | Seams used by the current tree | Not a fourth core | +| `summary_chart.py` | Current report helper | Not moved | +| `contracts/` (`runspec.py`, `json_v1.py`, `claim.py`, `evidence.py`) | Technical-only contracts: RunSpec, canonical JSON, verdict vocabulary, evidence segment and path rules | Exact module set; the initializer re-exports only `RunSpec`; not a fourth core | +| `apps/evidence_layout.py` | Evidence directory creation over the `contracts/evidence.py` path rules | The `mkdir` half of former root `evidence_paths.py`; host I/O stays in Apps | +| `resources/repository_packs.py` | Neutral `REPO_TYPES` and `repository_pack` facade | Former root `_repo_pack_data.py`, bytes unchanged; import-free initializer; not a fourth core | | `knowledge/data/`, `harness/data/` | Packaged resources, placed by owner since K02 | Corpus under Knowledge; pack descriptions under Harness | ## Import gate @@ -206,18 +224,23 @@ permitted: | Importer | Exact target | Route | | --- | --- | --- | | `catalog` | `inferrsi.knowledge.catalog` | Named symbols only; not `repository_pack` | -| `catalog` | `inferrsi._repo_pack_data` | `REPO_TYPES`, `repository_pack` | +| `catalog` | `inferrsi.resources.repository_packs` | `REPO_TYPES`, `repository_pack` | | `_source_snapshot` | `inferrsi.knowledge._source_snapshot` | Same-named Knowledge module | | `skill_catalog` | `inferrsi.knowledge.skill_catalog` | Same-named Knowledge module | | `_skill_selection` | `inferrsi.knowledge._skill_selection` | Same-named Knowledge module | | `research` | `inferrsi.knowledge.research` | Same-named Knowledge module | | `knowledge_store` | `inferrsi.knowledge.knowledge_store` | Same-named Knowledge module | | `knowledge_operations` | `inferrsi.knowledge.knowledge_operations` | Same-named Knowledge module | -| `infra.inspection` | `inferrsi._repo_pack_data` | Direct, never through `catalog` or H | +| `infra.inspection` | `inferrsi.resources.repository_packs` | Direct, never through `catalog` or H | These nine pairs are the only compatibility exceptions. A module prefix does not authorize an adjacent module, and a second importer of an allowed -Knowledge target fails. `inferrsi._repo_pack_data` has exactly two permitted +Knowledge target fails. C2 (#451) redirected the two data-leaf targets from +`inferrsi._repo_pack_data` to `inferrsi.resources.repository_packs`; the count +stays nine. Retiring the eight root-shell pairs and replacing the +`infra.inspection` edge with a permanent exact rule outside this allowlist is +later R1 work (#412) and is not done by C2. +`inferrsi.resources.repository_packs` has exactly two permitted importers outside Knowledge: root `catalog` and `infra.inspection`. An import of that leaf from inside `inferrsi.knowledge`, for example from `inferrsi/knowledge/catalog.py`, is intra-closure and not a tenth exception. @@ -233,14 +256,19 @@ The leaf's own import rule and `REPO_TYPES` object identity govern it. module-level assignment, `__getattr__`, `sys.modules`, dynamic imports, and unlisted import lines or names fail. An old leaf that keeps a body after its Knowledge module exists also fails. -- **Data leaf.** `inferrsi._repo_pack_data` imports only `json`, +- **Data leaf.** `inferrsi.resources.repository_packs` imports only `json`, `importlib.resources`, `typing.Any`, and `from __future__ import annotations`, anywhere in the file. It defines only `REPO_TYPES` and `repository_pack`. Any `inferrsi.*` import or dynamic import in it fails. + Python runs its parent initializer `resources/__init__.py` with every import + of the leaf, so that initializer is a docstring only: any import, any other + statement or a dynamic import there fails. - **Closures.** A Knowledge module reaches nothing outside Knowledge except - the data leaf and `inferrsi.contracts.json_v1`, the canonical encoder in the P0a + the data leaf with its import-free `inferrsi.resources` initializer and + `inferrsi.contracts.json_v1`, the canonical encoder in the P0a design. The gate admits its exact RunSpec-only parent initialization closure, - not arbitrary `contracts.*` modules. Benchmark modules and every `inferrsi.harness.*` module reach no + not arbitrary `contracts.*` or `resources.*` modules, and `contracts.evidence` + is not a Knowledge input. Benchmark modules and every `inferrsi.harness.*` module reach no other core, no Apps module, and no old root leaf. Apps reaches no old root leaf. `inferrsi.infra` reaches no Knowledge, Harness, Benchmark, or Apps module, so `infra` → root @@ -283,6 +311,22 @@ are unchanged, so `infra` still gains no edge into Harness. That manifest revision goes through formal architecture review with the K02 move; the implementer does not self-approve it. +C2 (#451) moved the leaf from `src/inferrsi/_repo_pack_data.py` to +`src/inferrsi/resources/repository_packs.py` without changing a byte and +deleted the root file, with no forwarding shell or alias. The manifest relocks +`technical_leaf.module` and `technical_leaf.file` to the new location, records +the move as `technical_leaf.module_path_revision`, and points the +`neutral_leaf_export` route and the `catalog` and `infra.inspection` pairs at +the new module. `file_sha256` keeps its K02 value and is now checked against +the relocated file. The importlib anchor, allowed imports, two symbols, +top-level shape, `REPO_TYPES` object identity and the two permitted importers +are unchanged, so `infra` still gains no edge into Harness; the +`top_level_shape` text is restated for the relocated file and also names the +import-free parent initializer. The K01 and K02 records above stay as +provenance. The nine pairs remain, so the allowlist blocker remains. That +manifest revision requires formal architecture review; the implementer does +not self-approve it, and passing tests are not that review. + ## Old `run` exit codes Frozen from