From f05118ea1f74005ef869f0b41c64d1f26af2f102 Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Sun, 15 Nov 2026 20:33:03 +0800 Subject: [PATCH] refactor: package shared technical contracts Agent-assisted-by: Claude Code (author) Agent-assisted-by: Grok Bot (author) Signed-off-by: GUOGUO --- AGENTS.md | 4 +- CHANGELOG.md | 16 + docs/contracts/current-design.md | 5 +- docs/module-layout.md | 11 +- src/inferrsi/apps/api.py | 2 +- src/inferrsi/apps/archive.py | 2 +- src/inferrsi/apps/artifact_reader.py | 2 +- src/inferrsi/apps/bridge.py | 2 +- src/inferrsi/apps/context_provider.py | 2 +- src/inferrsi/apps/legacy/evaluation.py | 2 +- src/inferrsi/apps/legacy/run.py | 2 +- src/inferrsi/apps/result.py | 2 +- src/inferrsi/apps/views/attempt.py | 2 +- src/inferrsi/arguments.py | 2 +- src/inferrsi/contracts.py | 5 - src/inferrsi/contracts/__init__.py | 10 + src/inferrsi/{ => contracts}/claim.py | 0 .../{_json_v1.py => contracts/json_v1.py} | 0 src/inferrsi/{ => contracts}/runspec.py | 0 src/inferrsi/execution.py | 2 +- src/inferrsi/harness/__init__.py | 2 +- src/inferrsi/harness/execution_service.py | 8 +- src/inferrsi/harness/execution_spec.py | 4 +- src/inferrsi/harness/run_store.py | 10 +- src/inferrsi/harness/runner.py | 2 +- tests/_gate_policy_support.py | 78 ++++- tests/architecture_real_ports.py | 2 +- tests/check_installed.py | 20 ++ tests/h01_run_store_child.py | 4 +- tests/test_apps_assess.py | 4 +- tests/test_apps_evidence.py | 2 +- tests/test_apps_execution_binding.py | 2 +- tests/test_apps_output.py | 2 +- tests/test_apps_prepare.py | 4 +- tests/test_claim.py | 2 +- tests/test_contracts_package.py | 317 ++++++++++++++++++ tests/test_dashboard_workflow.py | 2 +- tests/test_first_wave_isolation_policy.py | 165 ++++++++- tests/test_flare_adapter.py | 2 +- tests/test_harness_execute.py | 14 +- tests/test_harness_run_store.py | 57 ++-- tests/test_knowledge_model_policy.py | 3 +- tests/test_legacy_run_split.py | 4 +- tests/test_module_boundaries.py | 2 +- tests/test_package_root_inventory.py | 160 +++++++++ tests/test_smoke.py | 2 +- tests/test_target_module_import_gate.py | 19 +- tests/test_trial_check_reports.py | 2 +- tests/test_trial_checks.py | 2 +- 49 files changed, 875 insertions(+), 95 deletions(-) delete mode 100644 src/inferrsi/contracts.py create mode 100644 src/inferrsi/contracts/__init__.py rename src/inferrsi/{ => contracts}/claim.py (100%) rename src/inferrsi/{_json_v1.py => contracts/json_v1.py} (100%) rename src/inferrsi/{ => contracts}/runspec.py (100%) create mode 100644 tests/test_contracts_package.py create mode 100644 tests/test_package_root_inventory.py diff --git a/AGENTS.md b/AGENTS.md index 4a7caff70..1e4aca371 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,7 +64,7 @@ Parameterized or manual acceptance runs are issue-scoped; they are not the defau - **Resolve acceptance criteria** — preserve user-supplied criteria; when absent, use [performance-ceiling-analysis](src/inferrsi/knowledge/data/skill_library/methods/performance-ceiling-analysis/references/acceptance-criteria.md) to derive evidence-backed campaign targets. Retain partial requirements and derive only missing fields; unresolved evidence stays explicit. - **Freeze gates before search** — checklist and `validator/` locked in `plan/` before Phase 3 candidate experiments ([validation levels](docs/contributing.md#validation-levels)). - **One plane per campaign step** — change only the declared SIAS plane per step: `harness` | `model` | `infra` (under `src/inferrsi/`). -- **Emit RunSpec snapshots** — record repo × revision × backend × device × model × scenario (`runspec.py`, campaign `plan/`). +- **Emit RunSpec snapshots** — record repo × revision × backend × device × model × scenario (`contracts/runspec.py`, campaign `plan/`). - **Evaluator + evidence win** — never treat agent narrative as `PASS`; verdicts follow frozen gates and `evidence//` artifacts. ## Issues and claims (PM) @@ -81,7 +81,7 @@ Parameterized or manual acceptance runs are issue-scoped; they are not the defau ## Evidence and clearing the bar -Verdicts are **`PASS`**, **`FAIL`**, or **`INCONCLUSIVE`** only (`src/inferrsi/claim.py`). +Verdicts are **`PASS`**, **`FAIL`**, or **`INCONCLUSIVE`** only (`src/inferrsi/contracts/claim.py`). A public **`PASS`** requires: diff --git a/CHANGELOG.md b/CHANGELOG.md index 0878b3724..44acd319f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,22 @@ ### Changed +- 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`, + `contracts/runspec.py`, and `contracts/claim.py`; `contracts/__init__.py` + re-exports only `RunSpec` from its single definition. Public + `from inferrsi.contracts import RunSpec` remains supported, and Harness + re-exports the same `inferrsi.contracts.claim.Verdict` object. + - The old `inferrsi._json_v1`, `inferrsi.runspec`, and `inferrsi.claim` imports + and pickle module paths are retired, with no forwarding shells or + `sys.modules` aliases. Historical pickles require explicit migration or the + 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. - 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/contracts/current-design.md b/docs/contracts/current-design.md index c5fce4199..3d4fe0aae 100644 --- a/docs/contracts/current-design.md +++ b/docs/contracts/current-design.md @@ -25,11 +25,12 @@ 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. `contracts.py` remains a single-file `RunSpec` re-export. The legacy `claim.py` enum stays for compatibility but is not a new shared business model. +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. Permitted shared technical leaves are only: -- `inferrsi._json_v1` — the canonical encoder (D002); +- `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). The neutral leaf carries no target selection, task, scoring, scheduling, or public business DTO, and imports none of K/H/B/Apps. diff --git a/docs/module-layout.md b/docs/module-layout.md index acc9a9aac..36a975915 100644 --- a/docs/module-layout.md +++ b/docs/module-layout.md @@ -22,8 +22,10 @@ commands, including ordinary optimization, still load no Benchmark module. 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.py` stays a `RunSpec` -re-export. It must not become a shared business-contracts package. +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. 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 @@ -236,8 +238,9 @@ The leaf's own import rule and `REPO_TYPES` object identity govern it. annotations`, anywhere in the file. It defines only `REPO_TYPES` and `repository_pack`. Any `inferrsi.*` import or dynamic import in it fails. - **Closures.** A Knowledge module reaches nothing outside Knowledge except - the data leaf and `inferrsi._json_v1`, the canonical encoder in the P0a - design. Benchmark modules and every `inferrsi.harness.*` module reach no + the data leaf 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 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 diff --git a/src/inferrsi/apps/api.py b/src/inferrsi/apps/api.py index 26e91dd22..780662301 100644 --- a/src/inferrsi/apps/api.py +++ b/src/inferrsi/apps/api.py @@ -5,7 +5,7 @@ from pathlib import Path from typing import Protocol -from inferrsi._json_v1 import decode, encode +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.harness.errors import PublicError from inferrsi.harness.execution_spec import request_digest diff --git a/src/inferrsi/apps/archive.py b/src/inferrsi/apps/archive.py index 85df1e9af..6ca423a48 100644 --- a/src/inferrsi/apps/archive.py +++ b/src/inferrsi/apps/archive.py @@ -16,7 +16,7 @@ from typing import BinaryIO from uuid import uuid4 -from inferrsi._json_v1 import decode, encode +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.harness.errors import PublicError from .result import publication_error, validate_artifact_ref, validate_result diff --git a/src/inferrsi/apps/artifact_reader.py b/src/inferrsi/apps/artifact_reader.py index e844f222b..e07ac63a8 100644 --- a/src/inferrsi/apps/artifact_reader.py +++ b/src/inferrsi/apps/artifact_reader.py @@ -2,7 +2,7 @@ from io import BytesIO -from inferrsi._json_v1 import encode +from inferrsi.contracts.json_v1 import encode from inferrsi.harness.errors import PublicError from .archive import _logical_path, _read_directory_artifact, _storage_mounts diff --git a/src/inferrsi/apps/bridge.py b/src/inferrsi/apps/bridge.py index 9c757b961..dfa6b2eba 100644 --- a/src/inferrsi/apps/bridge.py +++ b/src/inferrsi/apps/bridge.py @@ -4,7 +4,7 @@ from copy import deepcopy from hashlib import sha256 -from inferrsi._json_v1 import decode, encode +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.harness.errors import PublicError from .archive import _logical_path diff --git a/src/inferrsi/apps/context_provider.py b/src/inferrsi/apps/context_provider.py index 756fcd59b..58fdc6824 100644 --- a/src/inferrsi/apps/context_provider.py +++ b/src/inferrsi/apps/context_provider.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from hashlib import sha256 -from inferrsi._json_v1 import MAX_SAFE_INTEGER +from inferrsi.contracts.json_v1 import MAX_SAFE_INTEGER from inferrsi.knowledge.knowledge_operations import KnowledgeReader _REQUEST_FIELDS = {"query", "ids", "pins", "selection_limits", "max_bytes", "round_id"} diff --git a/src/inferrsi/apps/legacy/evaluation.py b/src/inferrsi/apps/legacy/evaluation.py index 0d40a6d6d..90ba54c51 100644 --- a/src/inferrsi/apps/legacy/evaluation.py +++ b/src/inferrsi/apps/legacy/evaluation.py @@ -3,7 +3,7 @@ from inferrsi.benchmark.legacy_run_v0 import RunRules, _summary, read_metric from inferrsi.benchmark.legacy_run_v0 import evaluate as _evaluate -from inferrsi.runspec import RunSpec +from inferrsi.contracts import RunSpec __all__ = ["evaluate", "read_metric", "run_rules", "_summary"] diff --git a/src/inferrsi/apps/legacy/run.py b/src/inferrsi/apps/legacy/run.py index 2372f1e24..41b302f3f 100644 --- a/src/inferrsi/apps/legacy/run.py +++ b/src/inferrsi/apps/legacy/run.py @@ -7,10 +7,10 @@ from inferrsi.benchmark.legacy_run_v0 import evaluate as _evaluate from inferrsi.benchmark.legacy_run_v0 import read_metric, trial_metric +from inferrsi.contracts import RunSpec from inferrsi.harness.runner import run_experiment as _run_experiment from inferrsi.harness.scenarios.external import execute_trial as _execute_trial from inferrsi.infra.targets import repository_identity -from inferrsi.runspec import RunSpec from .evaluation import run_rules from .reporting import write_report diff --git a/src/inferrsi/apps/result.py b/src/inferrsi/apps/result.py index 79eb60bb0..23b3eebbf 100644 --- a/src/inferrsi/apps/result.py +++ b/src/inferrsi/apps/result.py @@ -4,7 +4,7 @@ from dataclasses import fields from hashlib import sha256 -from inferrsi._json_v1 import encode +from inferrsi.contracts.json_v1 import encode from inferrsi.harness.errors import PUBLIC_ERROR_CODES, PartialExecutionFacts _EXIT_CODES = { diff --git a/src/inferrsi/apps/views/attempt.py b/src/inferrsi/apps/views/attempt.py index 4c7a2467d..2b3d98653 100644 --- a/src/inferrsi/apps/views/attempt.py +++ b/src/inferrsi/apps/views/attempt.py @@ -2,7 +2,7 @@ from copy import deepcopy -from inferrsi._json_v1 import decode +from inferrsi.contracts.json_v1 import decode from inferrsi.harness.errors import PublicError from ..archive import read_assessment diff --git a/src/inferrsi/arguments.py b/src/inferrsi/arguments.py index 22a9be81f..eebe1cb42 100644 --- a/src/inferrsi/arguments.py +++ b/src/inferrsi/arguments.py @@ -10,7 +10,7 @@ from . import __version__ from ._skill_selection import RELEVANCE_METHODS from .catalog import BACKENDS, KNOWLEDGE_REPOS, KNOWLEDGE_SCOPES, REPO_TYPES -from .runspec import RunSpec +from .contracts import RunSpec def _common(parser: argparse.ArgumentParser) -> None: diff --git a/src/inferrsi/contracts.py b/src/inferrsi/contracts.py deleted file mode 100644 index 5733f22f5..000000000 --- a/src/inferrsi/contracts.py +++ /dev/null @@ -1,5 +0,0 @@ -"""Compatibility import for the original CLI RunSpec location.""" - -from .runspec import RunSpec - -__all__ = ["RunSpec"] diff --git a/src/inferrsi/contracts/__init__.py b/src/inferrsi/contracts/__init__.py new file mode 100644 index 000000000..16469adc6 --- /dev/null +++ b/src/inferrsi/contracts/__init__.py @@ -0,0 +1,10 @@ +"""Shared data and byte contracts: canonical JSON, RunSpec and claim types. + +``RunSpec`` is the only name this initializer re-exports. Import the canonical +encoder from ``contracts.json_v1`` and the verdict types from ``contracts.claim``. +No module here imports a core, Apps, Infra or Model module, or does I/O on import. +""" + +from .runspec import RunSpec + +__all__ = ["RunSpec"] diff --git a/src/inferrsi/claim.py b/src/inferrsi/contracts/claim.py similarity index 100% rename from src/inferrsi/claim.py rename to src/inferrsi/contracts/claim.py diff --git a/src/inferrsi/_json_v1.py b/src/inferrsi/contracts/json_v1.py similarity index 100% rename from src/inferrsi/_json_v1.py rename to src/inferrsi/contracts/json_v1.py diff --git a/src/inferrsi/runspec.py b/src/inferrsi/contracts/runspec.py similarity index 100% rename from src/inferrsi/runspec.py rename to src/inferrsi/contracts/runspec.py diff --git a/src/inferrsi/execution.py b/src/inferrsi/execution.py index bcda0c0b5..4ffd048f7 100644 --- a/src/inferrsi/execution.py +++ b/src/inferrsi/execution.py @@ -8,9 +8,9 @@ from .apps.legacy.run import execute_trial as _execute from .apps.legacy.run import run_experiment as _run_experiment +from .contracts import RunSpec from .harness.local_process import _stop_tree, _WindowsJob from .infra.targets import repository_identity -from .runspec import RunSpec __all__ = ["run_experiment", "repository_identity", "_execute", "_WindowsJob", "_stop_tree"] diff --git a/src/inferrsi/harness/__init__.py b/src/inferrsi/harness/__init__.py index 46185a94f..f166cc82a 100644 --- a/src/inferrsi/harness/__init__.py +++ b/src/inferrsi/harness/__init__.py @@ -1,5 +1,5 @@ """Harness plane: scenario composition, AB/BA orchestration, verdicts and reports.""" -from inferrsi.claim import Verdict, parse_verdict +from inferrsi.contracts.claim import Verdict, parse_verdict __all__ = ["Verdict", "parse_verdict"] diff --git a/src/inferrsi/harness/execution_service.py b/src/inferrsi/harness/execution_service.py index 70919f8a0..edc40a3fa 100644 --- a/src/inferrsi/harness/execution_service.py +++ b/src/inferrsi/harness/execution_service.py @@ -11,7 +11,7 @@ from datetime import datetime, timezone from pathlib import Path -from inferrsi import _json_v1 +from inferrsi.contracts import json_v1 from inferrsi.harness import input_binding, local_process, run_store from inferrsi.harness.errors import PublicError @@ -125,7 +125,7 @@ def _is_ids(value: object) -> bool: def _is_uint(value: object) -> bool: - return type(value) is int and 0 <= value <= _json_v1.MAX_SAFE_INTEGER + return type(value) is int and 0 <= value <= json_v1.MAX_SAFE_INTEGER def _is_definition(value: object) -> bool: @@ -328,8 +328,8 @@ def validate(spec: object, target: LocalCpuTarget) -> None: errors += _input_errors(spec["inputs"]) errors += _step_errors(spec["steps"], ordinary=not evaluation) try: - _json_v1.encode(spec) - except _json_v1.CanonicalError as exc: + json_v1.encode(spec) + except json_v1.CanonicalError as exc: errors.append(f"spec: not canonical inferrsi-json-v1 ({exc.stage})") if errors: raise PublicError( diff --git a/src/inferrsi/harness/execution_spec.py b/src/inferrsi/harness/execution_spec.py index 35c018cd0..380cba2a2 100644 --- a/src/inferrsi/harness/execution_spec.py +++ b/src/inferrsi/harness/execution_spec.py @@ -7,9 +7,9 @@ import hashlib from collections.abc import Mapping -from inferrsi import _json_v1 +from inferrsi.contracts import json_v1 def request_digest(spec: Mapping) -> str: """Return the D017 semantic identifier of a complete ExecutionSpec.""" - return "sha256:" + hashlib.sha256(_json_v1.encode(spec)).hexdigest() + return "sha256:" + hashlib.sha256(json_v1.encode(spec)).hexdigest() diff --git a/src/inferrsi/harness/run_store.py b/src/inferrsi/harness/run_store.py index a451788d2..11c4eb325 100644 --- a/src/inferrsi/harness/run_store.py +++ b/src/inferrsi/harness/run_store.py @@ -16,7 +16,7 @@ from datetime import datetime, timezone from pathlib import Path -from inferrsi import _json_v1 +from inferrsi.contracts import json_v1 from inferrsi.harness.errors import PartialExecutionFacts, PublicError from inferrsi.harness.execution_spec import request_digest as request_digest @@ -575,8 +575,8 @@ def _load_record( if digest != row["sha256"] or len(data) != row["size_bytes"]: raise _IntegrityFault(f"record version {version} bytes do not match their index row") try: - payload = _json_v1.decode(data) - except _json_v1.CanonicalError as exc: + payload = json_v1.decode(data) + except json_v1.CanonicalError as exc: raise _IntegrityFault(f"record version {version} is not canonical: {exc.stage}") from exc if ( not _has_fields(payload, PAYLOAD_FIELDS) @@ -689,7 +689,7 @@ def committed(self, attempt_id: str, record_version: int | None = None) -> Commi errors = [] if _is_id(attempt_id) else ["attempt_id: not an Id"] if record_version is not None and ( type(record_version) is not int - or not 1 <= record_version <= _json_v1.MAX_SAFE_INTEGER + or not 1 <= record_version <= json_v1.MAX_SAFE_INTEGER ): errors.append("record_version: must be null or a positive integer") if errors: @@ -1017,7 +1017,7 @@ def commit( errors.extend(_payload_artifact_errors(payload, artifacts, self._store.namespace)) if errors: raise ValueError(f"illegal record commit: {errors}") - data = _json_v1.encode(payload) + data = json_v1.encode(payload) try: for ref, source in artifacts: self._store._retain(ref, source) diff --git a/src/inferrsi/harness/runner.py b/src/inferrsi/harness/runner.py index 75be9729e..315e1b7b4 100644 --- a/src/inferrsi/harness/runner.py +++ b/src/inferrsi/harness/runner.py @@ -8,7 +8,7 @@ from datetime import datetime, timezone from pathlib import Path -from ..runspec import RunSpec +from ..contracts import RunSpec from .atomic_json import atomic_write_json as _write_json from .scenarios.external import collect_trial_checks diff --git a/tests/_gate_policy_support.py b/tests/_gate_policy_support.py index 84b84cf0b..7aa955430 100644 --- a/tests/_gate_policy_support.py +++ b/tests/_gate_policy_support.py @@ -50,8 +50,18 @@ "inferrsi.harness.local_process", "inferrsi.harness.run_store", ) +# Shared data/byte contracts (#451 C1). This exact module list is the whole package. A +# new module is a reviewed change to this list; an ``inferrsi.contracts.`` prefix never +# exempts anything. +CONTRACTS_PACKAGE = "inferrsi.contracts" # The P0a canonical encoder; the new K closure may use it and the data leaf. -CANONICAL_ENCODER = "inferrsi._json_v1" +CANONICAL_ENCODER = "inferrsi.contracts.json_v1" +# 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") +CONTRACTS_MODULES = frozenset( + {CONTRACTS_PACKAGE, CANONICAL_ENCODER, CONTRACTS_INIT_EXPORT[0], "inferrsi.contracts.claim"} +) # Names that load or build modules at run time. Any use fails closed. # P0b scans only these three spellings; getattr, exec, eval, runpy, and # pkgutil are intentionally out of scope here. @@ -207,8 +217,8 @@ def _package_ancestors(name: str, modules: dict[str, Path]) -> set[str]: return found -def _edges(tree: ast.Module, package: str, modules: dict[str, Path]) -> set[str]: - """Resolved ``inferrsi.*`` targets of every import in one module.""" +def _direct_targets(tree: ast.Module, package: str, modules: dict[str, Any]) -> set[str]: + """Resolved ``inferrsi.*`` targets written in one module, without parent inits.""" found: set[str] = set() for node in ast.walk(tree): if isinstance(node, ast.Import): @@ -218,7 +228,12 @@ def _edges(tree: ast.Module, package: str, modules: dict[str, Path]) -> set[str] for alias in node.names: candidate = f"{base}.{alias.name}" found.add(candidate if candidate in modules else base) - resolved = {name for name in found if name.startswith("inferrsi.")} + return {name for name in found if name.startswith("inferrsi.")} + + +def _edges(tree: ast.Module, package: str, modules: dict[str, Path]) -> set[str]: + """Resolved ``inferrsi.*`` targets of every import in one module.""" + resolved = _direct_targets(tree, package, modules) ancestors: set[str] = set() for name in resolved: ancestors.update(_package_ancestors(name, modules)) @@ -442,6 +457,46 @@ def _leaf_errors(tree: ast.Module, technical_leaf: dict[str, Any]) -> list[str]: return errors +def _contracts_errors(graph: _Graph) -> list[str]: + """The bounded contracts package: declared modules, one initializer export, no core. + + The initializer is a static shell that re-exports ``RunSpec`` from its own + ``runspec`` module and nothing else. Every other declared module imports no + ``inferrsi.*`` module at all, so loading the package cannot load a core, Apps, + Infra or Model module. An undeclared module under the package fails outright. + """ + export_module, export_name = CONTRACTS_INIT_EXPORT + errors: list[str] = [] + for name in sorted(name for name in graph.trees if _matches(name, CONTRACTS_PACKAGE)): + tree, package = graph.trees[name], graph.packages[name] + if name not in CONTRACTS_MODULES: + errors.append(f"{name} is not a declared contracts module") + elif name == CONTRACTS_PACKAGE: + errors += _shell_errors(name, tree, package, {export_module: {export_name}}) + imported = [ + alias.name + for node in tree.body + if isinstance(node, ast.ImportFrom) and _from_base(node, package) == export_module + for alias in node.names + ] + exported = [ + item.value + for node in tree.body + if "__all__" in _assigned_names(node) and _literal_all(node) + for item in node.value.elts + ] + if imported != [export_name] or exported != [export_name]: + errors.append(f"{name} must re-export exactly {export_name}") + else: + errors += [ + f"{name} imports {target}" + for target in sorted(_direct_targets(tree, package, graph.trees)) + ] + if _has_dynamic_import(tree): + errors.append(f"{name} has a dynamic import") + return errors + + def _model_or_infra(module: str) -> bool: return _matches(module, "inferrsi.model") or _matches(module, "inferrsi.infra") @@ -502,9 +557,22 @@ def first_wave_report( ) if leaf in graph.trees: violations += _leaf_errors(graph.trees[leaf], manifest["technical_leaf"]) + 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. + violations += [ + f"{name} imports {target}; Knowledge may import only {CANONICAL_ENCODER} there" + for name in modules + if _core_of(name) == "knowledge" + for target in sorted( + _direct_targets(graph.trees[name], graph.packages[name], graph.trees) + ) + if _matches(target, CONTRACTS_PACKAGE) and target != CANONICAL_ENCODER + ] + encoder_closure = (CANONICAL_ENCODER, CONTRACTS_PACKAGE, CONTRACTS_INIT_EXPORT[0]) def outside_knowledge(node: str) -> bool: - return _core_of(node) != "knowledge" and node not in (leaf, CANONICAL_ENCODER) + return _core_of(node) != "knowledge" and node != leaf 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/architecture_real_ports.py b/tests/architecture_real_ports.py index de46a34d1..9303eb345 100644 --- a/tests/architecture_real_ports.py +++ b/tests/architecture_real_ports.py @@ -16,10 +16,10 @@ from pathlib import Path from unittest.mock import patch +from inferrsi.contracts import RunSpec from inferrsi.evaluation import evaluate from inferrsi.harness.scenarios.external import execute_trial from inferrsi.knowledge_operations import KnowledgeReader -from inferrsi.runspec import RunSpec from tests.architecture_closed_loop import _canonical, _digest # Installed package card. Its bundle is record.json only, so this read does diff --git a/tests/check_installed.py b/tests/check_installed.py index 3a41b4d40..97dd2c8a5 100644 --- a/tests/check_installed.py +++ b/tests/check_installed.py @@ -5,12 +5,15 @@ import os import sys from importlib.resources import as_file, files +from importlib.util import find_spec from pathlib import Path import inferrsi +import inferrsi.harness 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.knowledge._source_snapshot import BUNDLE_LIBRARY, LIBRARY from inferrsi.knowledge_operations import KnowledgeReader @@ -566,3 +569,20 @@ def _inventory(root, prefix): f"{checkout}: only-installed={only_installed!r} " f"only-checkout={only_checkout!r} changed={changed!r}" ) + +# #451 C1: the shared data/byte contracts ship as one package. The four retired root +# modules are not in the wheel and have no alias. +assert RunSpec.__module__ == "inferrsi.contracts.runspec" +assert inferrsi.harness.Verdict is claim.Verdict +assert json_v1.decode(json_v1.encode({"b": [1, None], "a": "x"})) == {"a": "x", "b": [1, None]} +for retired in ("_json_v1", "runspec", "claim"): + assert find_spec(f"inferrsi.{retired}") is None, retired +installed_root = {child.name for child in files("inferrsi").iterdir() if child.is_file()} +assert not installed_root & {"_json_v1.py", "runspec.py", "claim.py", "contracts.py"} +if checkout: + # The wheel root holds the checkout's root files; the only file that may differ is + # the build-generated version metadata. + checkout_root = {child.name for child in Path(checkout).iterdir() if child.is_file()} + assert installed_root - {"_version.py"} == checkout_root - {"_version.py"}, ( + sorted(installed_root ^ checkout_root) + ) diff --git a/tests/h01_run_store_child.py b/tests/h01_run_store_child.py index cc55f7336..0a1a47ec3 100644 --- a/tests/h01_run_store_child.py +++ b/tests/h01_run_store_child.py @@ -9,7 +9,7 @@ from contextlib import contextmanager from pathlib import Path -from inferrsi import _json_v1 +from inferrsi.contracts import json_v1 from inferrsi.harness import run_store from inferrsi.harness.errors import PublicError @@ -49,7 +49,7 @@ def make_payload( attempt: dict, digest: str, version: int, phase: str, artifact_ref: dict | None = None ) -> dict: """The fixture not_started payload rebound to this attempt, or a started running one.""" - payload = _json_v1.decode((FIXTURES / "record-payload.json").read_bytes()) + payload = json_v1.decode((FIXTURES / "record-payload.json").read_bytes()) payload.update( request_id=attempt["request_id"], attempt_id=attempt["attempt_id"], diff --git a/tests/test_apps_assess.py b/tests/test_apps_assess.py index 1d529ed83..b506b9850 100644 --- a/tests/test_apps_assess.py +++ b/tests/test_apps_assess.py @@ -9,12 +9,12 @@ import pytest -from inferrsi._json_v1 import decode, encode from inferrsi.apps import archive from inferrsi.apps.api import assess as run_assess from inferrsi.apps.api import read_attempt_view from inferrsi.apps.artifact_reader import ArtifactReader from inferrsi.cli import main +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.harness.errors import PublicError from inferrsi.harness.run_store import RunStore @@ -337,7 +337,7 @@ def test_new_process_reassesses_the_requested_sqlite_revision(tmp_path, offline) subprocess.run( [sys.executable, "-c", """ from pathlib import Path -from inferrsi._json_v1 import decode, encode +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.apps.api import assess as run_assess, read_attempt_view workspace = Path.cwd() diff --git a/tests/test_apps_evidence.py b/tests/test_apps_evidence.py index 10bed6381..2cacd13b5 100644 --- a/tests/test_apps_evidence.py +++ b/tests/test_apps_evidence.py @@ -5,8 +5,8 @@ import pytest -from inferrsi._json_v1 import decode, encode from inferrsi.apps.bridge import to_evidence +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.harness.errors import PublicError _FIXTURES = Path(__file__).resolve().parents[1] / "docs/contracts/fixtures/t108-v4" diff --git a/tests/test_apps_execution_binding.py b/tests/test_apps_execution_binding.py index 2793eb871..ead7165a7 100644 --- a/tests/test_apps_execution_binding.py +++ b/tests/test_apps_execution_binding.py @@ -6,9 +6,9 @@ import pytest -from inferrsi._json_v1 import decode, encode from inferrsi.apps.api import execute from inferrsi.apps.bridge import bind_execution, verify_protocol +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.harness.errors import PublicError from inferrsi.harness.execution_spec import request_digest diff --git a/tests/test_apps_output.py b/tests/test_apps_output.py index 7d9077c90..034dc2c43 100644 --- a/tests/test_apps_output.py +++ b/tests/test_apps_output.py @@ -11,10 +11,10 @@ import pytest -from inferrsi._json_v1 import decode, encode from inferrsi.apps import archive from inferrsi.apps.api import execute from inferrsi.apps.archive import publish_result, read_result +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.harness.errors import PublicError from inferrsi.harness.run_store import Claim, RunStore, flatten, request_digest, utc_now diff --git a/tests/test_apps_prepare.py b/tests/test_apps_prepare.py index 75002a45f..1038d00a6 100644 --- a/tests/test_apps_prepare.py +++ b/tests/test_apps_prepare.py @@ -11,10 +11,10 @@ import pytest -from inferrsi._json_v1 import decode, encode from inferrsi.apps import archive from inferrsi.apps.api import prepare from inferrsi.apps.archive import read_archive_artifact, read_prepared, read_result +from inferrsi.contracts.json_v1 import decode, encode from inferrsi.harness.errors import PublicError _ROOT = Path(__file__).resolve().parents[1] @@ -165,7 +165,7 @@ def validate(value, data): script = """ import json import sys -from inferrsi._json_v1 import encode +from inferrsi.contracts.json_v1 import encode from inferrsi.apps.archive import read_archive_artifact, read_prepared config, ref = json.load(sys.stdin) diff --git a/tests/test_claim.py b/tests/test_claim.py index 8553326e7..9e65d460c 100644 --- a/tests/test_claim.py +++ b/tests/test_claim.py @@ -1,4 +1,4 @@ -from inferrsi.claim import Verdict, claim_requires_pinned_shas, parse_verdict +from inferrsi.contracts.claim import Verdict, claim_requires_pinned_shas, parse_verdict def test_parse_verdict() -> None: diff --git a/tests/test_contracts_package.py b/tests/test_contracts_package.py new file mode 100644 index 000000000..94dcbb23c --- /dev/null +++ b/tests/test_contracts_package.py @@ -0,0 +1,317 @@ +"""The shared data/byte contracts package (#451 C1). + +``inferrsi.contracts`` replaces root ``contracts.py``, ``_json_v1.py``, ``runspec.py`` +and ``claim.py``. The moved objects keep their behavior and identity. The old module +paths are retired without aliases, including for pickles that name them. +""" + +from __future__ import annotations + +import dataclasses +import importlib +import importlib.util +import json +import os +import pickle +import subprocess +import sys +from pathlib import Path + +import pytest + +import inferrsi.contracts as contracts +import inferrsi.harness as harness +from inferrsi import arguments, execution +from inferrsi.apps.legacy import evaluation as legacy_evaluation +from inferrsi.apps.legacy import run as legacy_run +from inferrsi.contracts import RunSpec, claim, json_v1, runspec +from inferrsi.harness import runner + +SRC = Path(__file__).resolve().parents[1] / "src" +ROOT = SRC / "inferrsi" +RETIRED = { + "inferrsi._json_v1": ("_json_v1.py", "CanonicalError"), + "inferrsi.runspec": ("runspec.py", "RunSpec"), + "inferrsi.claim": ("claim.py", "Verdict"), +} +MODULES = ( + "inferrsi.contracts", + "inferrsi.contracts.runspec", + "inferrsi.contracts.json_v1", + "inferrsi.contracts.claim", +) +COLD_IMPORT = """ +import importlib, json, sys +module = importlib.import_module(sys.argv[1]) +print(json.dumps({ + "loaded": sorted(name for name in sys.modules if name.split(".")[0] == "inferrsi"), + "public": sorted(name for name in vars(module) if not name.startswith("_")), +})) +""" +# Run each module body again, in a throwaway namespace, after its dependencies are +# loaded. Any filesystem, process or network audit event then comes from the body itself. +IO_PROBE = """ +import importlib, json, os, sys +IO_EVENTS = ("os.", "shutil.", "subprocess.", "socket.", "sqlite3.", "tempfile.", "glob.", + "pathlib.", "urllib.", "http.", "ctypes.", "winreg.", "mmap.", "fcntl.", "msvcrt.") +bodies = [] +for name in json.loads(sys.argv[1]): + module = importlib.import_module(name) + with open(module.__spec__.origin, encoding="utf-8") as handle: + code = compile(handle.read(), module.__spec__.origin, "exec") + bodies.append((name, module.__package__, code)) +bodies.append(("control", "", compile(sys.argv[2], "control", "exec"))) +seen = {name: [] for name, _, _ in bodies} +active = [] +def hook(event, args): + if active and (event == "open" or event.startswith(IO_EVENTS)): + seen[active[0]].append(event) +sys.addaudithook(hook) +defined = {} +for name, package, code in bodies: + namespace = {"__name__": name, "__package__": package} + active[:] = [name] + exec(code, namespace) + active.clear() + defined[name] = sorted(key for key in namespace if not key.startswith("_")) +print(json.dumps({"seen": seen, "defined": defined})) +""" +IO_CONTROL = "import os\nos.listdir(os.getcwd())\nopen(os.devnull).close()\n" +RUNSPEC_DEFAULTS = { + "repo": "example/repo", + "baseline_command": (), + "candidate_command": (), + "candidate_repo": None, + "repo_type": "vllm", + "backend": "cpu", + "scenario": "external_benchmark", + "mode": "config", + "validation_command": None, + "metric_key": "metrics.latency_ms", + "metric_unit": "ms", + "comparison_scope": "external_command", + "direction": "minimize", + "repetitions": 3, + "min_improvement_pct": 0.0, + "max_minutes": 120.0, + "workload_id": None, + "model": None, + "model_revision": None, + "agent": "manual", + "device_ids": (), + "revision": "", + "device": "", + "executor": "default", + "snapshot_id": "", + "output_dir": "runs", + "required_trial_checks": (), +} + + +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 test_initializer_reexports_only_runspec_and_there_is_one_class() -> None: + assert contracts.__all__ == ["RunSpec"] + assert contracts.RunSpec is runspec.RunSpec is RunSpec + assert (RunSpec.__module__, RunSpec.__qualname__) == ("inferrsi.contracts.runspec", "RunSpec") + # Every consumer in the tree holds that same class object. + for consumer in (arguments, execution, legacy_evaluation, legacy_run, runner): + assert consumer.RunSpec is RunSpec, consumer.__name__ + + +def test_harness_reexports_the_same_verdict_objects() -> None: + assert harness.__all__ == ["Verdict", "parse_verdict"] + assert harness.Verdict is claim.Verdict + assert harness.parse_verdict is claim.parse_verdict + assert claim.Verdict.__module__ == "inferrsi.contracts.claim" + assert [(verdict.name, verdict.value) for verdict in claim.Verdict] == [ + ("PASS", "PASS"), ("FAIL", "FAIL"), ("INCONCLUSIVE", "INCONCLUSIVE"), + ] + assert claim.parse_verdict("pass") is claim.Verdict.PASS + assert isinstance(claim.Verdict.PASS, str) and claim.Verdict.PASS == "PASS" + assert claim.claim_requires_pinned_shas(claim.Verdict.PASS) is True + assert claim.claim_requires_pinned_shas(claim.Verdict.FAIL) is False + with pytest.raises(ValueError) as caught: + claim.parse_verdict("maybe") + assert str(caught.value) == "Invalid verdict 'maybe'; expected one of PASS, FAIL, INCONCLUSIVE" + + +@pytest.mark.parametrize("module", MODULES) +def test_cold_import_loads_only_the_initializer_and_runspec(module: str) -> None: + # No core, Apps, Infra or Model module loads: the initializer's RunSpec import is all. + result = _fresh(COLD_IMPORT, module) + expected = {"inferrsi", "inferrsi.contracts", "inferrsi.contracts.runspec", module} + assert set(result["loaded"]) - {"inferrsi._version"} == expected + if module == "inferrsi.contracts": + assert result["public"] == ["RunSpec", "runspec"] + + +def test_module_bodies_do_no_io_on_import() -> None: + result = _fresh(IO_PROBE, json.dumps(MODULES), IO_CONTROL) + seen, defined = result["seen"], result["defined"] + # Positive control: the same probe reports a body that touches the filesystem. + assert {"os.listdir", "open"} <= set(seen.pop("control")) + assert seen == {name: [] for name in MODULES} + # Each body really ran under the probe. + assert "RunSpec" in defined["inferrsi.contracts"] + 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"]) + + +@pytest.mark.parametrize("name", sorted(RETIRED)) +def test_retired_root_module_paths_have_no_alias(name: str) -> None: + filename, attribute = RETIRED[name] + assert not (ROOT / filename).exists() + assert importlib.util.find_spec(name) is None + with pytest.raises(ModuleNotFoundError): + importlib.import_module(name) + assert name not in sys.modules + # A pickle that names a class by its retired module path no longer loads. + with pytest.raises(ModuleNotFoundError): + pickle.loads(f"c{name}\n{attribute}\n.".encode("ascii")) + + +def test_contracts_is_a_package_and_pickles_name_the_new_paths() -> None: + assert not (ROOT / "contracts.py").exists() + assert Path(contracts.__file__).name == "__init__.py" + spec = RunSpec(repo="example/repo", baseline_command=("run",), device_ids=("0", "1")) + assert b"inferrsi.contracts.runspec" in pickle.dumps(spec, protocol=0) + assert pickle.loads(pickle.dumps(spec)) == spec + assert b"inferrsi.contracts.claim" in pickle.dumps(claim.Verdict.PASS, protocol=0) + assert pickle.loads(pickle.dumps(claim.Verdict.PASS)) is claim.Verdict.PASS + + +def test_canonical_json_bytes_are_unchanged() -> None: + value = {"b": [1, -2, True, False, None], "a": "é\n
\x7f\"\\", "": {"z": 0, "A": []}} + data = json_v1.encode(value) + assert data == ( + b'{"":{"A":[],"z":0},"a":"\xc3\xa9\\n\xe2\x80\xa8\x7f\\"\\\\",' + b'"b":[1,-2,true,false,null]}' + ) + assert json_v1.decode(data) == value + assert json_v1.encode("\x00\x1f\b\t\f\r") == b'"\\u0000\\u001f\\b\\t\\f\\r"' + assert json_v1.MAX_SAFE_INTEGER == 9007199254740991 + for edge in (json_v1.MAX_SAFE_INTEGER, -json_v1.MAX_SAFE_INTEGER): + assert json_v1.decode(json_v1.encode(edge)) == edge + assert json_v1.CanonicalError.__module__ == "inferrsi.contracts.json_v1" + assert issubclass(json_v1.CanonicalError, ValueError) + + +@pytest.mark.parametrize( + "value,stage", + [ + (2**53, "integer_range"), + (-(2**53), "integer_range"), + (1.5, "float_token"), + (float("nan"), "nonfinite_token"), + (float("inf"), "nonfinite_token"), + ({1: "x"}, "ascii_key"), + ({"é": 1}, "ascii_key"), + ("\ud800", "unicode_scalar"), + ((1, 2), "unsupported_type"), + (b"bytes", "unsupported_type"), + ], + ids=repr, +) +def test_encode_error_stages_are_unchanged(value: object, stage: str) -> None: + with pytest.raises(json_v1.CanonicalError) as caught: + json_v1.encode(value) + assert caught.value.stage == stage + assert str(caught.value).startswith(stage + ": ") + + +@pytest.mark.parametrize( + "data,stage", + [ + (b"\xef\xbb\xbf{}", "no_bom"), + (b"\xff", "strict_utf8"), + (b"{", "json_syntax"), + (b"1.0", "float_token"), + (b"NaN", "nonfinite_token"), + (b"9007199254740992", "integer_range"), + (b"1" + b"0" * 4400, "integer_range"), + (b'{"a":1,"a":2}', "duplicate_key"), + (b'"\\ud800"', "unicode_scalar"), + (b'{"\\u00e9":1}', "ascii_key"), + (b'{"b":1,"a":2}', "canonical_bytes"), + (b" 1", "canonical_bytes"), + ], + ids=lambda item: item[:24].decode("latin-1") if isinstance(item, bytes) else item, +) +def test_decode_error_stages_are_unchanged(data: bytes, stage: str) -> None: + with pytest.raises(json_v1.CanonicalError) as caught: + json_v1.decode(data) + assert caught.value.stage == stage + assert str(caught.value).startswith(stage + ": ") + + +def _valid(tmp_path: Path, **changes: object) -> RunSpec: + repo = tmp_path / "repo" + repo.mkdir(exist_ok=True) + values: dict = { + "repo": str(repo), "baseline_command": ("base",), "candidate_command": ("candidate",), + "output_dir": str(tmp_path / "runs"), + } + return RunSpec(**{**values, **changes}) + + +def test_runspec_defaults_and_round_trip_are_unchanged(tmp_path: Path) -> None: + assert [field.name for field in dataclasses.fields(RunSpec)] == list(RUNSPEC_DEFAULTS) + assert RunSpec(repo="example/repo").to_dict() == RUNSPEC_DEFAULTS + spec = _valid( + tmp_path, validation_command=("check", "--strict"), device_ids=("0", "1"), + required_trial_checks=("evidence.closed",), min_improvement_pct=2.5, + ) + assert spec.validate() is None + # JSON turns the tuples into lists; from_dict restores the same frozen value. + restored = RunSpec.from_dict(json.loads(json.dumps(spec.to_dict()))) + assert restored == spec and hash(restored) == hash(spec) + assert isinstance(restored.baseline_command, tuple) and restored.candidate_repo is None + with pytest.raises(dataclasses.FrozenInstanceError): + spec.repo = "elsewhere" + with pytest.raises(ValueError, match="device_ids must be an argv/ID sequence"): + RunSpec.from_dict({"repo": "r", "device_ids": "0"}) + with pytest.raises(TypeError): + RunSpec.from_dict({"repo": "r", "unknown_field": 1}) + + +@pytest.mark.parametrize( + "changes,message", + [ + ({"repo": "missing-directory"}, "repo must be an existing directory"), + ({"baseline_command": ()}, "baseline_command must be a nonempty argv sequence"), + ({"candidate_command": ("",)}, "candidate_command executable cannot be empty"), + ({"validation_command": ("ok", 1)}, "validation_command must be a nonempty argv"), + ({"required_trial_checks": ("a", "a")}, "required_trial_checks must be a tuple"), + ({"required_trial_checks": ("a..b",)}, "required_trial_checks must be a tuple"), + ({"direction": "up"}, "direction must be minimize or maximize"), + ({"mode": "other"}, "mode must be config, code or semantic"), + ({"repetitions": 0}, "repetitions must be a positive integer"), + ({"repetitions": True}, "repetitions must be a positive integer"), + ({"max_minutes": float("nan")}, "max_minutes must be finite"), + ({"min_improvement_pct": -1.0}, "max_minutes must be positive and min_improvement_pct"), + ({"max_minutes": 7 * 24 * 60 + 1}, "max_minutes cannot exceed seven days"), + ({"metric_key": "a..b"}, "metric_key must be a nonempty dotted JSON key"), + ({"metric_unit": ""}, "metric_unit and comparison_scope must be declared"), + ], + ids=lambda item: next(iter(item)) if isinstance(item, dict) else None, +) +def test_runspec_validation_errors_are_unchanged( + tmp_path: Path, changes: dict, message: str +) -> None: + with pytest.raises(ValueError, match=message): + _valid(tmp_path, **changes).validate() + + +def test_runspec_output_dir_may_not_contain_a_source_repository(tmp_path: Path) -> None: + with pytest.raises(ValueError, match="output_dir cannot equal or contain a source"): + _valid(tmp_path, output_dir=str(tmp_path)).validate() diff --git a/tests/test_dashboard_workflow.py b/tests/test_dashboard_workflow.py index 2021bb740..b5362e8b9 100644 --- a/tests/test_dashboard_workflow.py +++ b/tests/test_dashboard_workflow.py @@ -7,7 +7,7 @@ from inferrsi.apps.legacy.run import execute_trial, run_experiment from inferrsi.apps.views.dashboard_data import load_dashboard -from inferrsi.runspec import RunSpec +from inferrsi.contracts import RunSpec def test_dashboard_tracks_real_native_abba_and_validation(tmp_path): diff --git a/tests/test_first_wave_isolation_policy.py b/tests/test_first_wave_isolation_policy.py index 084d98ca2..8a8e40469 100644 --- a/tests/test_first_wave_isolation_policy.py +++ b/tests/test_first_wave_isolation_policy.py @@ -47,6 +47,19 @@ "harness/__init__.py": "", "harness/api.py": "VALUE = 1\n", } +# The bounded contracts package (#451 C1): the initializer's one export and three +# standalone modules. +CONTRACTS_INIT = ( + '"""Shared contracts."""\n\n' + "from .runspec import RunSpec\n\n" + '__all__ = ["RunSpec"]\n' +) +CONTRACTS_TREE = { + "contracts/__init__.py": CONTRACTS_INIT, + "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", +} def _shell(importer: str) -> str: """A shape-valid static shell generated from the manifest export lists.""" @@ -65,11 +78,11 @@ def _k01_tree() -> dict[str, str]: files = { "__init__.py": "", "_repo_pack_data.py": LEAF_SOURCE, - "_json_v1.py": "import json\n", + **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/knowledge_operations.py": "from inferrsi import _json_v1\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", "harness/__init__.py": "", @@ -333,6 +346,154 @@ def test_undeclared_dynamic_edge_fails_closed(self) -> None: ) +K_OPERATIONS = "inferrsi.knowledge.knowledge_operations" +CONTRACTS_INIT_DEFECTS = ( + ("from .claim import Verdict\n", "unlisted import from inferrsi.contracts.claim"), + ("from . import json_v1\n", "unlisted import from inferrsi.contracts"), + ("from inferrsi.apps import main\n", "unlisted import from inferrsi.apps"), + ("from inferrsi.harness import api\n", "unlisted import from inferrsi.harness"), + ("from .runspec import helper\n", "unlisted name inferrsi.contracts.runspec.helper"), + ("def __getattr__(name):\n raise AttributeError(name)\n", "defines __getattr__"), + ( + "import sys\nsys.modules['inferrsi.runspec'] = sys.modules[__name__]\n", + "touches sys.modules", + ), + ("import importlib\nimportlib.import_module('inferrsi.claim')\n", "dynamic import"), + ("VALUE = 1\n", "module-level assignment"), +) + + +class ContractsPackageTest(unittest.TestCase): + """#451 C1: one bounded contracts package. Exact modules, one export, no core import.""" + + def test_knowledge_names_only_the_encoder_inside_the_package(self) -> None: + allowed = ( + "from inferrsi.contracts import json_v1\n", + "from inferrsi.contracts.json_v1 import encode\n", + "import inferrsi.contracts.json_v1\n", + "from ..contracts import json_v1\n", + ) + for source in allowed: + with self.subTest(source=source): + report = _report({"knowledge/knowledge_operations.py": source}) + self.assertEqual(report.violations, ()) + refused = ( + ("from inferrsi.contracts import RunSpec\n", "imports inferrsi.contracts;"), + ("from inferrsi.contracts import json_v1, RunSpec\n", "imports inferrsi.contracts;"), + ("import inferrsi.contracts\n", "imports inferrsi.contracts;"), + ( + "from inferrsi.contracts.runspec import RunSpec\n", + "imports inferrsi.contracts.runspec;", + ), + ("from ..contracts import claim\n", "imports inferrsi.contracts.claim;"), + ("from inferrsi.contracts.claim import Verdict\n", "reaches inferrsi.contracts.claim"), + ) + for source, expected in refused: + with self.subTest(source=source): + report = _report({"knowledge/knowledge_operations.py": source}) + self.assertTrue(_violated(report, K_OPERATIONS, expected), report.violations) + + def test_retired_root_encoder_is_outside_the_knowledge_closure(self) -> None: + report = _report( + { + "_json_v1.py": "import json\n", + "knowledge/knowledge_operations.py": "from inferrsi import _json_v1\n", + } + ) + self.assertTrue( + _violated(report, f"{K_OPERATIONS} reaches inferrsi._json_v1"), report.violations + ) + + def test_package_prefix_exempts_no_undeclared_module(self) -> None: + report = _report( + { + "contracts/extra.py": "VALUE = 1\n", + "knowledge/knowledge_operations.py": "from inferrsi.contracts import extra\n", + } + ) + self.assertTrue( + _violated(report, "inferrsi.contracts.extra is not a declared contracts module"), + report.violations, + ) + self.assertTrue( + _violated(report, f"{K_OPERATIONS} reaches inferrsi.contracts.extra"), + report.violations, + ) + + def test_initializer_is_limited_to_the_runspec_export(self) -> None: + for snippet, expected in CONTRACTS_INIT_DEFECTS: + with self.subTest(snippet=snippet): + report = _report({"contracts/__init__.py": CONTRACTS_INIT + snippet}) + self.assertTrue( + _violated(report, "inferrsi.contracts ", expected), report.violations + ) + shapes = ( + CONTRACTS_INIT.replace('["RunSpec"]', '["RunSpec", "runspec"]'), + CONTRACTS_INIT.replace('["RunSpec"]', "[]"), + CONTRACTS_INIT.replace("from .runspec import RunSpec\n", ""), + CONTRACTS_INIT.replace("import RunSpec", "import RunSpec, RunSpec"), + ) + for source in shapes: + with self.subTest(source=source): + report = _report({"contracts/__init__.py": source}) + self.assertTrue( + _violated(report, "inferrsi.contracts must re-export exactly RunSpec"), + report.violations, + ) + aliased = CONTRACTS_INIT.replace("import RunSpec", "import RunSpec as Spec") + report = _report({"contracts/__init__.py": aliased}) + self.assertTrue(_violated(report, "inferrsi.contracts aliases RunSpec as Spec")) + + def test_contract_modules_import_no_inferrsi_module(self) -> None: + dynamic = ( + "import importlib\n\n\n" + "def load(name):\n return importlib.import_module(name)\n" + ) + cases = ( + ( + "contracts/json_v1.py", + "from inferrsi.knowledge import catalog\n", + "inferrsi.contracts.json_v1 imports inferrsi.knowledge.catalog", + ), + ( + "contracts/json_v1.py", + "from inferrsi.harness import api\n", + "inferrsi.contracts.json_v1 imports inferrsi.harness.api", + ), + ( + "contracts/runspec.py", + "from inferrsi.infra import inspection\n", + "inferrsi.contracts.runspec imports inferrsi.infra.inspection", + ), + ( + "contracts/claim.py", + "from . import json_v1\n", + "inferrsi.contracts.claim imports inferrsi.contracts.json_v1", + ), + ("contracts/claim.py", dynamic, "inferrsi.contracts.claim has a dynamic import"), + ) + for path, source, expected in cases: + with self.subTest(expected=expected): + report = _report({path: source}) + self.assertTrue(_violated(report, expected), 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. + report = _report( + { + "contracts/json_v1.py": "from inferrsi.apps import main\n", + "harness/api.py": "from inferrsi.contracts import json_v1\n", + } + ) + for importer in ("inferrsi.harness.api", K_OPERATIONS): + with self.subTest(importer=importer): + self.assertTrue( + _violated(report, f"{importer} reaches inferrsi.apps"), report.violations + ) + self.assertFalse(report.new_core_closure_isolated) + + class HarnessEntryTest(unittest.TestCase): """A01-3: no harness module reaches Knowledge through an old root shell.""" diff --git a/tests/test_flare_adapter.py b/tests/test_flare_adapter.py index 4060b6580..b94e49241 100644 --- a/tests/test_flare_adapter.py +++ b/tests/test_flare_adapter.py @@ -8,8 +8,8 @@ from fixtures.flare_packet import build_packet, write_json from inferrsi.apps.legacy.run import run_experiment +from inferrsi.contracts import RunSpec from inferrsi.model.validators.flare import adapt, main, validate_pair -from inferrsi.runspec import RunSpec @pytest.fixture diff --git a/tests/test_harness_execute.py b/tests/test_harness_execute.py index 33a78c23f..be0819e44 100644 --- a/tests/test_harness_execute.py +++ b/tests/test_harness_execute.py @@ -11,7 +11,7 @@ import pytest -from inferrsi import _json_v1 +from inferrsi.contracts import json_v1 from inferrsi.harness import execution_service, input_binding, local_process, run_store from inferrsi.harness.api import Harness, LocalCpuTarget from inferrsi.harness.errors import PublicError @@ -27,15 +27,19 @@ } SCHEMA = REPO / "docs" / "contracts" / "schemas" / "contract-v1.schema.json" # The new H closure: the H01/H02 modules, the canonical encoder, and the package init -# residue (the mixed ``inferrsi.harness`` __init__ loads ``inferrsi.claim``). +# residue (the mixed ``inferrsi.harness`` __init__ loads ``inferrsi.contracts.claim``). # execution_spec landed on main with the #397 squash 8150ee04, which also # added it to this H02 closure. The set matches current main. +# #451 C1: the encoder and the claim types live in the contracts package, so importing +# either runs the ``inferrsi.contracts`` initializer, whose single import is the RunSpec +# module. The last two names are that exact residue; a ``contracts`` prefix is never allowed. NEW_H_CLOSURE = { "inferrsi.harness.api", "inferrsi.harness.execution_service", "inferrsi.harness.input_binding", "inferrsi.harness.local_process", "inferrsi.harness.run_store", "inferrsi.harness.errors", "inferrsi.harness.execution_spec", - "inferrsi._json_v1", - "inferrsi.harness", "inferrsi.claim", + "inferrsi.contracts.json_v1", + "inferrsi.harness", "inferrsi.contracts.claim", + "inferrsi.contracts", "inferrsi.contracts.runspec", } # Prints a grandchild's pid, then outlives any test deadline with it. TREE = ( @@ -533,7 +537,7 @@ def test_next_step_starts_only_after_process_domain_confirmed_empty( def test_record_bytes_are_canonical_and_store_fixed(h: Harness) -> None: record = h.execute(make_spec(h, [step("print('bytes')")])) committed = h._store.committed(record["attempt_id"]) - assert _json_v1.encode(committed.payload) == committed.payload_bytes + assert json_v1.encode(committed.payload) == committed.payload_bytes assert committed.record_ref == record["record_ref"] diff --git a/tests/test_harness_run_store.py b/tests/test_harness_run_store.py index 3110f915c..754077b4d 100644 --- a/tests/test_harness_run_store.py +++ b/tests/test_harness_run_store.py @@ -18,7 +18,7 @@ import pytest -from inferrsi import _json_v1 +from inferrsi.contracts import json_v1 from inferrsi.harness import run_store from inferrsi.harness.errors import PublicError from inferrsi.harness.run_store import Claim, CommittedRecord, RunStore, RunStoreReader @@ -40,15 +40,22 @@ SRC = REPO / "src" NEW_H_CLOSURE = { "inferrsi.harness.run_store", "inferrsi.harness.errors", - "inferrsi.harness.execution_spec", "inferrsi._json_v1", + "inferrsi.harness.execution_spec", "inferrsi.contracts.json_v1", } -# Package initializers run on import: the existing mixed harness __init__ loads inferrsi.claim -# (the Verdict enum). This is a known residue, not an edge added by the new H modules. -PACKAGE_INIT_RESIDUE = {"inferrsi", "inferrsi.harness", "inferrsi.claim"} +# #451 C1: the canonical encoder lives in the contracts package, so importing it runs the +# ``inferrsi.contracts`` initializer, whose single import is the RunSpec module. These are +# the two exact names that initializer adds; a ``contracts`` prefix is never allowed. +CONTRACTS_INIT = {"inferrsi.contracts", "inferrsi.contracts.runspec"} +# Package initializers run on import: the existing mixed harness __init__ loads +# inferrsi.contracts.claim (the Verdict enum). This is a known residue, not an edge added +# by the new H modules. +PACKAGE_INIT_RESIDUE = {"inferrsi", "inferrsi.harness", "inferrsi.contracts.claim"} FORBIDDEN_PREFIXES = ( "inferrsi.knowledge", "inferrsi.benchmark", "inferrsi.apps", "inferrsi.evaluation", "inferrsi.catalog", "inferrsi.research", "inferrsi.knowledge_store", "inferrsi.inspection", "inferrsi.skill_catalog", "inferrsi.runspec", "inferrsi.cli", "inferrsi.arguments", + # Root paths retired by #451 C1 with ``inferrsi.runspec``; none may return as an alias. + "inferrsi._json_v1", "inferrsi.claim", ) @@ -138,8 +145,8 @@ def test_h01_2_same_key_with_another_digest_conflicts(tmp_path: Path) -> None: def test_h01_3_new_process_reads_the_fixed_record_hash_and_version(tmp_path: Path) -> None: # Commit the frozen fixture payload; its golden RecordRef hash is the byte reference. payload_bytes = (FIXTURES / "record-payload.json").read_bytes() - golden = _json_v1.decode((FIXTURES / "fixed-refs.json").read_bytes())["record_ref"] - payload = _json_v1.decode(payload_bytes) + golden = json_v1.decode((FIXTURES / "fixed-refs.json").read_bytes())["record_ref"] + payload = json_v1.decode(payload_bytes) attempt = { "request_id": "att-1", "run_id": "run-1", "attempt_id": "att-1", "owner": "fixture", "target_request": payload["target_request"], "deadline_utc": "2099-01-01T00:00:00Z", @@ -219,7 +226,7 @@ def test_h01_4_crash_points_issue_no_fake_ref(tmp_path: Path, scenario: str) -> # Committed but the response was lost: the same key replays the original record. replay = store.claim(attempt, digest) assert isinstance(replay, CommittedRecord) - assert replay.payload_bytes == _json_v1.encode( + assert replay.payload_bytes == json_v1.encode( make_payload(attempt, digest, 1, "finished") ) assert _error(store.read, attempt_id, 2).code == "RecordNotFound" @@ -307,8 +314,9 @@ def _inferrsi_imports(module: str) -> set[str]: base = ".".join(parts + ([node.module] if node.module else [])) names.add(base) names.update(f"{base}.{alias.name}" for alias in node.names) - # Keep only names that resolve to a real module file; ``from inferrsi import x`` names the - # package container and an imported attribute such as ``_json_v1`` resolves to its file. + # Keep only names that resolve to a real module file; ``from inferrsi.contracts import x`` + # names the package initializer and an imported submodule such as ``json_v1`` resolves + # to its file. return { name for name in names if name.startswith("inferrsi.") and _module_file(name) is not None @@ -316,7 +324,8 @@ def _inferrsi_imports(module: str) -> set[str]: def test_h01_6_new_h_closure_reaches_no_legacy_k_b_or_apps_module() -> None: - # Static closure: explicit imports of the new H modules, followed transitively. + # Static closure: explicit imports of the new H modules, followed transitively. The + # encoder import passes through the contracts initializer and its RunSpec module. closure: set[str] = set() pending = ["inferrsi.harness.run_store"] while pending: @@ -324,11 +333,11 @@ def test_h01_6_new_h_closure_reaches_no_legacy_k_b_or_apps_module() -> None: if module not in closure: closure.add(module) pending.extend(_inferrsi_imports(module)) - assert closure == NEW_H_CLOSURE + assert closure == NEW_H_CLOSURE | CONTRACTS_INIT # Runtime: importing a submodule also runs each parent package's __init__. The mixed - # ``inferrsi.harness`` __init__ loads ``inferrsi.claim`` (the Verdict enum). That is a - # known residue of the existing package, not a legacy, K, B, or Apps module. + # ``inferrsi.harness`` __init__ loads ``inferrsi.contracts.claim`` (the Verdict enum). + # That is a known residue of the existing package, not a legacy, K, B, or Apps module. code = ( "import json, sys, inferrsi.harness.run_store; " "print(json.dumps(sorted(m for m in sys.modules if m.split('.')[0] == 'inferrsi')))" @@ -342,9 +351,9 @@ def test_h01_6_new_h_closure_reaches_no_legacy_k_b_or_apps_module() -> None: assert not [m for m in loaded if m.startswith(FORBIDDEN_PREFIXES)] # Keep the static graph exact. The runtime set may also include inferrsi._version # when setuptools_scm has written the gitignored file after `pip install -e`. - allowed = NEW_H_CLOSURE | PACKAGE_INIT_RESIDUE | {"inferrsi._version"} + allowed = NEW_H_CLOSURE | CONTRACTS_INIT | PACKAGE_INIT_RESIDUE | {"inferrsi._version"} assert loaded <= allowed - assert NEW_H_CLOSURE | PACKAGE_INIT_RESIDUE <= loaded + assert NEW_H_CLOSURE | CONTRACTS_INIT | PACKAGE_INIT_RESIDUE <= loaded def test_commit_rejects_unbacked_or_foreign_artifact_refs(tmp_path: Path) -> None: @@ -525,7 +534,7 @@ def test_reader_routes_only_its_own_mount_and_locations(tmp_path: Path) -> None: def test_request_digest_matches_the_d023_anchor() -> None: spec = (FIXTURES.parent / "execution-spec-d023.json").read_bytes() manifest = json.loads((FIXTURES.parent / "MANIFEST-d023.json").read_text(encoding="utf-8")) - assert run_store.request_digest(_json_v1.decode(spec)) == manifest["files"][0]["request_digest"] + assert run_store.request_digest(json_v1.decode(spec)) == manifest["files"][0]["request_digest"] def test_reentry_of_unknown_side_effect_is_recovery_required_even_with_owner_lock( @@ -591,8 +600,8 @@ def _strict_json_cases() -> list[dict]: @pytest.mark.parametrize("case", _strict_json_cases(), ids=lambda case: case["id"]) def test_json_v1_rejects_p0a_strict_byte_vectors(case: dict) -> None: - with pytest.raises(_json_v1.CanonicalError) as caught: - _json_v1.decode(bytes.fromhex(case["raw_hex"])) + with pytest.raises(json_v1.CanonicalError) as caught: + json_v1.decode(bytes.fromhex(case["raw_hex"])) assert caught.value.stage == case["expected_rejection_stage"] @@ -601,12 +610,12 @@ def test_json_v1_matches_p0a_codec_string_vectors() -> None: golden = (FIXTURES / "codec-strings.json").read_bytes() assert len(golden) == vectors["bytes"] assert hashlib.sha256(golden).hexdigest() == vectors["sha256"] - assert _json_v1.encode(vectors["value"]) == golden - assert _json_v1.decode(golden) == vectors["value"] - assert _json_v1.encode(_json_v1.decode(golden)) == golden + assert json_v1.encode(vectors["value"]) == golden + assert json_v1.decode(golden) == vectors["value"] + assert json_v1.encode(json_v1.decode(golden)) == golden for case in vectors["negative_fixed_payload_bytes"]: - with pytest.raises(_json_v1.CanonicalError): - _json_v1.decode(case["raw_utf8"].encode("utf-8")) + with pytest.raises(json_v1.CanonicalError): + json_v1.decode(case["raw_utf8"].encode("utf-8")) def _object_path(store: RunStore, ref: Mapping) -> Path: diff --git a/tests/test_knowledge_model_policy.py b/tests/test_knowledge_model_policy.py index 8a5380103..edb848cf2 100644 --- a/tests/test_knowledge_model_policy.py +++ b/tests/test_knowledge_model_policy.py @@ -184,7 +184,8 @@ def test_explicit_protected_paths_require_declarations(policy, path): assert assess(item, {path}, policy)["state"] == "failure" -@pytest.mark.parametrize("path", ["README.md", "src/inferrsi/cli.py", "src/inferrsi/contracts.py", +@pytest.mark.parametrize("path", ["README.md", "src/inferrsi/cli.py", + "src/inferrsi/contracts/__init__.py", "src/inferrsi/knowledge_store.py", "tests/test_claim.py"]) def test_no_blanket_source_scope_and_stable_not_applicable(policy, path): item = pr() diff --git a/tests/test_legacy_run_split.py b/tests/test_legacy_run_split.py index 60d72decf..6e0a41090 100644 --- a/tests/test_legacy_run_split.py +++ b/tests/test_legacy_run_split.py @@ -10,17 +10,17 @@ import pytest -from inferrsi import claim, evaluation, execution, reporting +from inferrsi import evaluation, execution, reporting from inferrsi.apps.legacy import evaluation as legacy_evaluation from inferrsi.apps.legacy import reporting as legacy_reporting from inferrsi.apps.legacy import run as legacy_run from inferrsi.benchmark import legacy_run_v0, sampling +from inferrsi.contracts import RunSpec, claim from inferrsi.harness import atomic_json from inferrsi.harness.runner import run_experiment from inferrsi.harness.scenarios.external import execute_trial from inferrsi.infra import artifacts from inferrsi.model.protocols import metrics -from inferrsi.runspec import RunSpec from tests._gate_policy_support import ROOT, _scan, _walk GRAPH = _scan(ROOT, frozenset()) diff --git a/tests/test_module_boundaries.py b/tests/test_module_boundaries.py index 05758fd91..4834beb2c 100644 --- a/tests/test_module_boundaries.py +++ b/tests/test_module_boundaries.py @@ -11,10 +11,10 @@ import pytest from inferrsi.apps.legacy.run import execute_trial, run_experiment +from inferrsi.contracts import RunSpec from inferrsi.infra.platforms import PLATFORM_PROBES, inspect_device from inferrsi.infra.platforms.cuda import qualify_devices from inferrsi.model.protocols.metrics import read_metric -from inferrsi.runspec import RunSpec def test_new_scenario_target_and_backend_share_abba_and_verdict(tmp_path): diff --git a/tests/test_package_root_inventory.py b/tests/test_package_root_inventory.py new file mode 100644 index 000000000..e22c35c26 --- /dev/null +++ b/tests/test_package_root_inventory.py @@ -0,0 +1,160 @@ +"""Exact inventory of the files directly under ``src/inferrsi/`` (#451 C1). + +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 +``__main__.py`` remain. It does not assert that final state early. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[1] +PACKAGE = "src/inferrsi" +# The 27 tracked root files before C1, minus the four that C1 moved into contracts/. +RETIRED_BY_C1 = frozenset({"_json_v1.py", "claim.py", "contracts.py", "runspec.py"}) +ROOT_FILES = frozenset( + { + "__init__.py", + "__main__.py", + "_mindie_read_worker.py", + "_repo_pack_data.py", + "_skill_selection.py", + "_source_snapshot.py", + "arguments.py", + "benchmark_operations.py", + "catalog.py", + "cli.py", + "evaluation.py", + "evidence_paths.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", + "summary_chart.py", + } +) +# setuptools-scm writes this file at build time. It is gitignored wheel metadata and is +# never a tracked root file. +GENERATED = "_version.py" + + +def inventory_errors(found: set[str], expected: frozenset[str] = ROOT_FILES) -> list[str]: + errors = [ + f"unexpected root file {PACKAGE}/{name}: a new module belongs in a subpackage" + for name in sorted(found - expected) + ] + errors += [ + f"missing root file {PACKAGE}/{name}: shrink ROOT_FILES in the PR that removes it" + for name in sorted(expected - found) + ] + return errors + + +def _listed(repo: Path, *options: str) -> set[str]: + output = subprocess.run( + ["git", "-C", str(repo), "ls-files", "-z", *options, "--", f":(glob){PACKAGE}/*"], + check=True, capture_output=True, + ).stdout + return {name.rsplit("/", 1)[1] for name in output.decode("utf-8").split("\0") if name} + + +def root_files(repo: Path) -> set[str]: + """Root files that the next commit of ``repo`` tracks. + + These are the tracked files still on disk plus the untracked files Git does not + ignore. In a clean checkout that is exactly the tracked set. A new root file is + therefore reported before and after it is staged, and the ignored generated + ``_version.py`` never counts. + """ + present = _listed(repo, "--cached", "--others", "--exclude-standard") + 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 + assert GENERATED not in ROOT_FILES + assert {"__init__.py", "__main__.py"} <= ROOT_FILES + + +def test_on_disk_root_matches_the_inventory() -> None: + on_disk = {path.name for path in (REPO / PACKAGE).iterdir() if path.is_file()} + assert inventory_errors(on_disk - {GENERATED}) == [] + + +def test_tracked_root_matches_the_inventory() -> None: + if not (REPO / ".git").exists(): + pytest.skip("Git metadata is required for the tracked-file inventory") + assert inventory_errors(root_files(REPO)) == [] + # The generated version file is ignored, so a build cannot add it to the inventory. + ignored = subprocess.run( + ["git", "-C", str(REPO), "check-ignore", "-q", f"{PACKAGE}/{GENERATED}"], check=False + ) + assert ignored.returncode == 0 + + +@pytest.mark.parametrize("name", ["helper.py", "py.typed", GENERATED, *sorted(RETIRED_BY_C1)]) +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" + ] + + +def test_a_removed_root_file_must_shrink_the_inventory() -> None: + assert inventory_errors(set(ROOT_FILES) - {"cli.py"}) == [ + 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"} + assert inventory_errors(set(smaller), smaller) == [] + + +def test_git_view_reports_root_changes_before_and_after_staging(tmp_path: Path) -> None: + def git(*args: str) -> None: + subprocess.run(["git", "-C", str(tmp_path), *args], check=True, capture_output=True) + + def unexpected(name: str) -> str: + return f"unexpected root file {PACKAGE}/{name}: a new module belongs in a subpackage" + + git("init", "-q") + (tmp_path / ".gitignore").write_bytes((REPO / ".gitignore").read_bytes()) + root = tmp_path / PACKAGE + (root / "contracts").mkdir(parents=True) + for name in ROOT_FILES: + (root / name).write_text("", encoding="utf-8") + # A subpackage file is not a root file, and the ignored build output never counts. + (root / "contracts" / "extra.py").write_text("", encoding="utf-8") + (root / GENERATED).write_text("", encoding="utf-8") + assert root_files(tmp_path) == ROOT_FILES + git("add", ".") + assert root_files(tmp_path) == ROOT_FILES + + (root / "helper.py").write_text("", encoding="utf-8") + assert inventory_errors(root_files(tmp_path)) == [unexpected("helper.py")] + git("add", ".") + assert inventory_errors(root_files(tmp_path)) == [unexpected("helper.py")] + (root / "helper.py").unlink() + assert inventory_errors(root_files(tmp_path)) == [] + + # Tracking the generated file by force is a root addition like any other. + git("add", "--force", f"{PACKAGE}/{GENERATED}") + assert inventory_errors(root_files(tmp_path)) == [unexpected(GENERATED)] + git("rm", "--cached", "-q", f"{PACKAGE}/{GENERATED}") + (root / "runspec.py").write_text("", encoding="utf-8") + assert inventory_errors(root_files(tmp_path)) == [unexpected("runspec.py")] + (root / "runspec.py").unlink() + (root / "cli.py").unlink() + assert inventory_errors(root_files(tmp_path)) == [ + f"missing root file {PACKAGE}/cli.py: shrink ROOT_FILES in the PR that removes it" + ] diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 1732edf2e..f572242f7 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -5,8 +5,8 @@ import pytest from inferrsi import __version__ +from inferrsi.contracts import RunSpec from inferrsi.evidence_paths import EVIDENCE_SEGMENTS, evidence_subpaths -from inferrsi.runspec import RunSpec 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 813c336b5..a174048c3 100644 --- a/tests/test_target_module_import_gate.py +++ b/tests/test_target_module_import_gate.py @@ -73,8 +73,23 @@ 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()) - self.assertTrue((ROOT / "contracts.py").is_file()) - self.assertFalse((ROOT / "contracts").is_dir()) + # #451 C1: 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"], + ) + for retired in ("contracts.py", "_json_v1.py", "runspec.py", "claim.py"): + self.assertFalse((ROOT / retired).exists(), retired) + self.assertEqual( + support.CONTRACTS_MODULES, + { + "inferrsi.contracts", + "inferrsi.contracts.claim", + "inferrsi.contracts.json_v1", + "inferrsi.contracts.runspec", + }, + ) # A01-4 staged model: packages may appear one at a time, but each # remaining trigger keeps overall false. report = first_wave_report() diff --git a/tests/test_trial_check_reports.py b/tests/test_trial_check_reports.py index 70a8b2e0e..34d8b2134 100644 --- a/tests/test_trial_check_reports.py +++ b/tests/test_trial_check_reports.py @@ -6,7 +6,7 @@ from inferrsi.apps.legacy.evaluation import evaluate from inferrsi.apps.legacy.reporting import write_report -from inferrsi.runspec import RunSpec +from inferrsi.contracts import RunSpec from inferrsi.summary_chart import SpecError, spec_from_evaluation diff --git a/tests/test_trial_checks.py b/tests/test_trial_checks.py index 6bc075234..8b96c9e28 100644 --- a/tests/test_trial_checks.py +++ b/tests/test_trial_checks.py @@ -10,8 +10,8 @@ from inferrsi.apps.legacy.evaluation import evaluate from inferrsi.apps.legacy.run import run_experiment from inferrsi.arguments import make_spec, parser +from inferrsi.contracts import RunSpec from inferrsi.harness.scenarios.external import collect_trial_checks -from inferrsi.runspec import RunSpec @pytest.mark.parametrize("value", [None, False, "true", 1, {}, []])