diff --git a/docs/architecture.md b/docs/architecture.md index 6589edcc..dd2798a0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -59,6 +59,10 @@ Dependencies flow from entrypoints to Harness, from scenarios to Model/Infra, and from Model to Infra utilities. Infra does not import Model or Harness. None of these shared packages imports a campaign from `examples/`. +The implemented table above is not the target four-module layout. The +suggested rule, including what the import gate does not prove, is +[module-layout.md](module-layout.md). That page does not move code. + The caller still prepares the benchmark runtime and baseline/candidate source checkouts. Separate working directories alone do not isolate Python imports, resources or an existing server. The local executor can run directly on a diff --git a/docs/module-layout.md b/docs/module-layout.md new file mode 100644 index 00000000..2eba84c4 --- /dev/null +++ b/docs/module-layout.md @@ -0,0 +1,90 @@ +# Target module layout + +This page records the directory rule for the integration slice. It is not an +implemented migration. No package has been created or moved to satisfy it. +#371 remains the proposal for the Apps package path and is not accepted here. + +## Target modules + +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 current `src/inferrsi/harness/` tree is a mixed implementation. Its +presence is not the target Harness boundary and is not evidence that the four +modules are isolated. + +Do not keep a sibling `data/` resource tier. Knowledge corpus later moves +under the Knowledge module. The internal corpus split is left to a future +Knowledge issue. `repo_packs/*/pack.json` is a target description and follows +the Harness slice. `data/dashboard/template.html` is a view template and +follows the Apps slice. Neither belongs in the corpus only because it sits +under `data/` today. + +`src/` stays the import root. `tests/` and `tools/` stay at the repository +root. Also keep these root files where they are: `AGENTS.md` (entry contract), +`CHANGELOG.md`, `MANIFEST.in` (it grafts `src/inferrsi`, so a later resource +move must update packaging in that same slice), and `.agents/` (review skill). + +Do not create empty `knowledge/`, `benchmark/`, or `apps/` directories in the +contract slice. Do not treat draft #338 as a reason to add a top-level +`benchmarks/` tree. + +## Tentative current ownership + +This table is a reading aid for the import gate. It does not authorize moves. +Several flat modules remain shared seams until their slice starts. + +| Current path | Tentative reading | This slice | +| --- | --- | --- | +| `cli.py`, `arguments.py`, `__main__.py` | Future Apps entry, after #371 | Not moved | +| `knowledge_operations.py`, `knowledge_store.py`, `catalog.py`, `research.py`, `skill_catalog.py`, `_skill_selection.py` | Knowledge behavior and corpus readers | Interfaces unchanged | +| `harness/` | Mixed legacy tree, not target isolation | Not redefined | +| `evaluation.py`, `model/validators/` | Benchmark-leaning checks | Not moved | +| `infra/`, `execution.py`, `runspec.py`, `inspection.py` | Seams used by the current tree | Not a fourth core | +| `reporting.py`, `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 | +| `data/` | Packaged resources, mixed kinds | Not relocated here | + +## Import gate + +`tests/test_target_module_import_gate.py` fails on a new import of +`inferrsi.knowledge`, `inferrsi.benchmark`, or `inferrsi.apps` from anywhere +except `inferrsi.apps`, and on Knowledge or Benchmark importing Harness. +`from inferrsi import knowledge` counts as an import of that core. +Same-core imports are allowed, including a relative import rewritten to an +absolute name such as `from .bar import VALUE` inside `knowledge/`. Each forbidden +importer and target pair is reported once. There is no exception list of +current edges: those three package names are unused on this tree, so the +violation list is empty. + +A clean result means no new cross-core import of those names. It does not mean +the modules are isolated. Isolation stays false while `knowledge/`, +`benchmark/`, or `apps/` is absent, and while `harness/` still imports Model +or Infra. + +## Old `run` exit codes + +Frozen from +[`src/inferrsi/cli.py`](https://github.com/InferMatrixLab/InferRSI/blob/9f61a83f030bfa2ffe3b54eac1781593309d6380/src/inferrsi/cli.py#L412-L417) +as the compatibility text for a later integration check. This page does not +change that code. + +| Condition | Exit | +| --- | --- | +| `status["verdict"]` is `PASS` | 0 | +| `status["verdict"]` is `FAIL` | 1 | +| Any other verdict, including missing | 2 | +| `OSError`, `ValueError`, `JSONDecodeError`, or `sqlite3.Error` via `parser.error` | 2 | + +## Later fixture move + +Not this slice. `tests/fixtures/` already contains helpers such as +`skill_eval_pr_support.py` and `skill_eval_compare_support.py`. A later +cleanup that moves `docs/examples/skill-comparison/` or +`docs/examples/skill-eval-pr/` should use new directories +`tests/fixtures/skill-comparison/` and `tests/fixtures/skill-eval-pr/`, and +must update the tests and workflow that cite the docs paths in the same +change. Do not overwrite the existing helper modules. diff --git a/tests/test_target_module_import_gate.py b/tests/test_target_module_import_gate.py new file mode 100644 index 00000000..1677d73d --- /dev/null +++ b/tests/test_target_module_import_gate.py @@ -0,0 +1,144 @@ +"""Guard new Knowledge / Harness / Benchmark / Apps import edges. + +A passing scan is not module isolation. The target packages are not all +present, and the existing ``inferrsi.harness`` tree still mixes other layers. +""" + +from __future__ import annotations + +import ast +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] / "src" / "inferrsi" +NEW_CORES = ("knowledge", "benchmark", "apps") +CORE_NAMES = ("knowledge", "harness", "benchmark", "apps") + + +def _modules_in_source(source: str, package_parts: list[str]) -> list[str]: + """Resolve import targets, including ``from package import name`` forms.""" + package = ".".join(package_parts) + names: list[str] = [] + for node in ast.walk(ast.parse(source)): + if isinstance(node, ast.Import): + names.extend(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom): + if node.level: + keep = package.split(".")[ + : len(package.split(".")) - node.level + 1 + ] + base = ".".join(keep + ([node.module] if node.module else [])) + else: + base = node.module or "" + if base: + names.append(base) + for alias in node.names: + if alias.name == "*": + continue + candidate = f"{base}.{alias.name}" if base else alias.name + names.append(candidate) + return names + + +def _matches(module: str, prefix: str) -> bool: + return module == prefix or module.startswith(prefix + ".") + + +def _core_of(module: str) -> str | None: + for name in CORE_NAMES: + if _matches(module, f"inferrsi.{name}"): + return name + return None + + +def _imported_modules(path: Path, root: Path) -> list[str]: + relative = path.relative_to(root) + return _modules_in_source( + path.read_text(encoding="utf-8"), + ["inferrsi", *relative.parts[:-1]], + ) + + +def cross_core_violations(root: Path = ROOT) -> list[str]: + """One message per forbidden importer and target core. + + Same-core imports, including relative imports rewritten to absolute + names, are allowed. Apps may import the other cores. Every other import + of ``inferrsi.knowledge``, ``inferrsi.benchmark``, or ``inferrsi.apps`` + is forbidden, as is Knowledge or Benchmark importing Harness. + """ + found: set[tuple[str, str]] = set() + for path in sorted(root.rglob("*.py")): + relative = path.relative_to(root) + importer = relative.parts[0] if len(relative.parts) > 1 else "" + for module in _imported_modules(path, root): + target = _core_of(module) + if target is None or importer == "apps" or importer == target: + continue + if target == "harness" and importer not in ("knowledge", "benchmark"): + continue + found.add((relative.as_posix(), target)) + return [f"{path} imports inferrsi.{target}" for path, target in sorted(found)] + + +def isolation_complete(root: Path = ROOT) -> bool: + """True only after the target packages exist and the legacy mix is gone.""" + if any(not (root / name).is_dir() for name in NEW_CORES): + return False + harness = root / "harness" + if not harness.is_dir(): + return False + for path in harness.rglob("*.py"): + for module in _imported_modules(path, root): + if _matches(module, "inferrsi.model") or _matches(module, "inferrsi.infra"): + return False + return not cross_core_violations(root) + + +class TargetModuleImportGateTest(unittest.TestCase): + def test_no_new_cross_core_imports(self) -> None: + self.assertEqual(cross_core_violations(), []) + + def test_clean_scan_is_not_isolation(self) -> None: + absent = [name for name in NEW_CORES if not (ROOT / name).is_dir()] + self.assertEqual(absent, ["knowledge", "benchmark", "apps"]) + self.assertTrue((ROOT / "harness").is_dir()) + self.assertTrue((ROOT / "contracts.py").is_file()) + self.assertFalse((ROOT / "contracts").is_dir()) + self.assertFalse(isolation_complete()) + + def test_real_tree_allows_apps_and_same_core_only(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + samples = { + "harness/probe.py": "from inferrsi import knowledge\n", + "apps/probe.py": "import inferrsi.knowledge\n", + "knowledge/bar.py": "VALUE = 1\n", + "knowledge/foo.py": "from .bar import VALUE\n", + "knowledge/out.py": "import inferrsi.harness\n", + "benchmark/edge.py": "from inferrsi.benchmark import edge\n", + } + for relative, source in samples.items(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(source, encoding="utf-8") + violations = cross_core_violations(root) + self.assertEqual( + violations, + [ + "harness/probe.py imports inferrsi.knowledge", + "knowledge/out.py imports inferrsi.harness", + ], + ) + + def test_layout_doc_states_the_same_limit(self) -> None: + text = ( + Path(__file__).resolve().parents[1] / "docs" / "module-layout.md" + ).read_text(encoding="utf-8") + self.assertIn("Isolation stays false", text) + self.assertIn("Same-core imports are allowed", text) + + +if __name__ == "__main__": + unittest.main()