From 9409b91ed10245974a858b207b8c6cc2728db597 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 9 Nov 2026 14:42:51 +0000 Subject: [PATCH 1/2] docs: record the target module layout and import gate State the four-module directory rule without moving files, and fail the build if a new Knowledge, Benchmark, or Apps import appears outside Apps. A clean scan does not mean those modules are isolated. Agent-assisted-by: Cursor Grok 4.7 (author) Signed-off-by: Cursor Agent Co-authored-by: Dong1017 --- docs/architecture.md | 4 + docs/module-layout.md | 84 +++++++++++++++++ tests/test_target_module_import_gate.py | 115 ++++++++++++++++++++++++ 3 files changed, 203 insertions(+) create mode 100644 docs/module-layout.md create mode 100644 tests/test_target_module_import_gate.py diff --git a/docs/architecture.md b/docs/architecture.md index 6589edcc9..dd2798a03 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 000000000..92a528e4d --- /dev/null +++ b/docs/module-layout.md @@ -0,0 +1,84 @@ +# 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 | +| `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 if any module outside a future +`inferrsi.apps` package imports `inferrsi.knowledge`, `inferrsi.benchmark`, or +`inferrsi.apps`, and if code under `knowledge/` or `benchmark/` imports +Harness, Apps, or the other core. The allowed direction is Apps importing the +cores. 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` 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 000000000..12bcbd0e4 --- /dev/null +++ b/tests/test_target_module_import_gate.py @@ -0,0 +1,115 @@ +"""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 unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] / "src" / "inferrsi" +NEW_CORES = ("knowledge", "benchmark", "apps") +FORBIDDEN_FROM_LEGACY = tuple(f"inferrsi.{name}" for name in NEW_CORES) + + +def _imported_modules(path: Path) -> list[str]: + return _modules_in_source( + path.read_text(encoding="utf-8"), + ["inferrsi", *path.relative_to(ROOT).parts[:-1]], + ) + + +def _modules_in_source(source: str, package_parts: list[str]) -> list[str]: + 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 "" + names.append(base) + return names + + +def _matches(module: str, prefix: str) -> bool: + return module == prefix or module.startswith(prefix + ".") + + +def cross_core_violations(root: Path = ROOT) -> list[str]: + """Imports that would connect the target cores outside Apps.""" + violations = [] + for path in sorted(root.rglob("*.py")): + importer = path.relative_to(root).parts[0] if path.parent != root else "" + for module in _imported_modules(path): + if importer == "apps": + continue + for prefix in FORBIDDEN_FROM_LEGACY: + if _matches(module, prefix): + violations.append(f"{path.relative_to(root)} imports {module}") + if importer in ("knowledge", "benchmark"): + for other in ("inferrsi.harness", "inferrsi.apps", "inferrsi.knowledge", "inferrsi.benchmark"): + own = f"inferrsi.{importer}" + if other != own and _matches(module, other): + violations.append(f"{path.relative_to(root)} imports {module}") + return violations + + +def isolation_complete(root: Path = ROOT) -> bool: + """True only when the target packages exist and the legacy mix is gone. + + This slice never sets that condition. Callers must not treat an empty + violation list as isolation. + """ + 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): + 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_detector_flags_a_new_core_import_outside_apps(self) -> None: + def forbidden(importer: str, source: str) -> bool: + if importer == "apps": + return False + return any( + _matches(module, prefix) + for module in _modules_in_source(source, ["inferrsi", importer]) + for prefix in FORBIDDEN_FROM_LEGACY + ) + + self.assertTrue(forbidden("harness", "import inferrsi.knowledge\n")) + self.assertFalse(forbidden("apps", "import inferrsi.knowledge\n")) + + def test_layout_doc_states_the_same_limit(self) -> None: + text = Path(__file__).resolve().parents[1].joinpath("docs/module-layout.md").read_text( + encoding="utf-8" + ) + self.assertIn("Isolation stays false", text) + + +if __name__ == "__main__": + unittest.main() From f827acec05672a61d99798fdaa57ef286175c26a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 9 Nov 2026 23:14:14 +0000 Subject: [PATCH 2/2] test: exercise the real import gate on a temporary tree Resolve from-import aliases, allow same-core imports, and pass the scan root into the parser. A temporary package tree now checks the forbidden, Apps, and same-core cases without claiming isolation. Agent-assisted-by: Cursor Grok 4.7 (author) Signed-off-by: Cursor Agent Co-authored-by: Dong1017 --- docs/module-layout.md | 22 +++-- tests/test_target_module_import_gate.py | 119 +++++++++++++++--------- 2 files changed, 88 insertions(+), 53 deletions(-) diff --git a/docs/module-layout.md b/docs/module-layout.md index 92a528e4d..2eba84c47 100644 --- a/docs/module-layout.md +++ b/docs/module-layout.md @@ -44,17 +44,21 @@ Several flat modules remain shared seams until their slice starts. | `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 if any module outside a future -`inferrsi.apps` package imports `inferrsi.knowledge`, `inferrsi.benchmark`, or -`inferrsi.apps`, and if code under `knowledge/` or `benchmark/` imports -Harness, Apps, or the other core. The allowed direction is Apps importing the -cores. There is no exception list of current edges: those three package names -are unused on this tree, so the violation list is empty. +`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/`, @@ -63,8 +67,10 @@ or Infra. ## Old `run` exit codes -Frozen from `src/inferrsi/cli.py` as the compatibility text for a later -integration check. This page does not change that code. +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 | | --- | --- | diff --git a/tests/test_target_module_import_gate.py b/tests/test_target_module_import_gate.py index 12bcbd0e4..1677d73d5 100644 --- a/tests/test_target_module_import_gate.py +++ b/tests/test_target_module_import_gate.py @@ -7,22 +7,17 @@ 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") -FORBIDDEN_FROM_LEGACY = tuple(f"inferrsi.{name}" for name in NEW_CORES) - - -def _imported_modules(path: Path) -> list[str]: - return _modules_in_source( - path.read_text(encoding="utf-8"), - ["inferrsi", *path.relative_to(ROOT).parts[:-1]], - ) +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)): @@ -30,11 +25,19 @@ def _modules_in_source(source: str, package_parts: list[str]) -> list[str]: 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] + keep = package.split(".")[ + : len(package.split(".")) - node.level + 1 + ] base = ".".join(keep + ([node.module] if node.module else [])) else: base = node.module or "" - names.append(base) + 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 @@ -42,38 +45,52 @@ 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]: - """Imports that would connect the target cores outside Apps.""" - violations = [] + """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")): - importer = path.relative_to(root).parts[0] if path.parent != root else "" - for module in _imported_modules(path): - if importer == "apps": + 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 - for prefix in FORBIDDEN_FROM_LEGACY: - if _matches(module, prefix): - violations.append(f"{path.relative_to(root)} imports {module}") - if importer in ("knowledge", "benchmark"): - for other in ("inferrsi.harness", "inferrsi.apps", "inferrsi.knowledge", "inferrsi.benchmark"): - own = f"inferrsi.{importer}" - if other != own and _matches(module, other): - violations.append(f"{path.relative_to(root)} imports {module}") - return violations + 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 when the target packages exist and the legacy mix is gone. - - This slice never sets that condition. Callers must not treat an empty - violation list as isolation. - """ + """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): + 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) @@ -91,24 +108,36 @@ def test_clean_scan_is_not_isolation(self) -> None: self.assertFalse((ROOT / "contracts").is_dir()) self.assertFalse(isolation_complete()) - def test_detector_flags_a_new_core_import_outside_apps(self) -> None: - def forbidden(importer: str, source: str) -> bool: - if importer == "apps": - return False - return any( - _matches(module, prefix) - for module in _modules_in_source(source, ["inferrsi", importer]) - for prefix in FORBIDDEN_FROM_LEGACY - ) - - self.assertTrue(forbidden("harness", "import inferrsi.knowledge\n")) - self.assertFalse(forbidden("apps", "import inferrsi.knowledge\n")) + 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].joinpath("docs/module-layout.md").read_text( - encoding="utf-8" - ) + 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__":