From af365aac049d2b14a680deb38cc2469fc461766c Mon Sep 17 00:00:00 2001 From: Polichinl Date: Fri, 21 Aug 2026 12:33:58 +0200 Subject: [PATCH 1/3] test(clone): the partners are independent, not merely both below the machinery (#284, #285) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #285's census records this repo as "enforcement test: none". It has seven. `tests/test_clone_readiness.py` proves both VERTICAL arrows of ADR-002 by importing packages in a subprocess with the others blocked — and its own docstring says a regex was tried first and rejected, because it missed `from ..contract import x` and `from views_postprocessing import contract`. There is even a mutation proof of the detector. That is stronger than a static contract: it proves the modules import in isolation, not merely that no import statement mentions them. What nothing proved is the HORIZONTAL arrow. Nothing stopped `crafd` and `unfao` importing each other, and that is the arrow keeping a partner liftable: the two are deliberate clones (C-33), so the realistic violation is a copy-paste leaving a sibling's import behind. `test_the_machinery_imports_without_any_partner` cannot see it — that test imports the machinery, and this is partner-to-partner. Two halves, matching the split this file already documents: the subprocess is load-bearing and sees transitive arrivals; the source scan is the supplement and covers `managers/`, which the subprocess deliberately skips because importing a manager needs views-pipeline-core and a purity check should not be contingent on a heavy framework being installed (C-40 (a)). Mutation-proven in both halves: a sibling import added to `crafd/product.py` fails the subprocess half; a sibling named in `crafd/managers/crafd.py` fails the source half. WHY NOT import-linter, as #284 proposes. It would add a dev dependency, a CI step and a config block to assert three things — of which two are already covered here, and covered more strongly. The one it would add is this test. #285 itself points approvingly at views-datafactory doing the same thing in two assertions in an existing file, with no new dependency and no graph library; that is the argument, and it applies here. If the platform later standardises on import-linter, adopting it is a one-line pyproject block and this test can stay or go — nothing here forecloses it. Suite: 470 passed, 3 skipped, 37 xfailed. ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- tests/fixtures/wire_contract/README.md | 2 +- tests/test_clone_readiness.py | 47 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/tests/fixtures/wire_contract/README.md b/tests/fixtures/wire_contract/README.md index 83bb000..882c812 100644 --- a/tests/fixtures/wire_contract/README.md +++ b/tests/fixtures/wire_contract/README.md @@ -33,7 +33,7 @@ tool versions** (numpy per lockfile, `pyarrow 16.1.0`). The committed bytes + `S are canonical regardless. **A change to this fixture is a change to the contract (§10)** — do not regenerate casually. -**`pyarrow` is the version-sensitive one; `views_frames` is not.** Parquet bytes vary +**`pyarrow` is the version-sensitive one; `views_frames` is not — and that now includes across a MAJOR.** Measured 2026-08-21 in an isolated environment at the pinned toolchain (pyarrow 16.1.0, numpy 1.26.4): the shard emitted through `views_frames.io.arrow` under **1.10.2** and under **2.0.0** hash identically, and both equal the committed fixture (`203650fd…12c54`). All 61 frames-dependent tests pass at 2.0.0 there. So the views-frames 2.0.0 adoption (#286) is **not** a fixture re-vendor — it is blocked only by views-pipeline-core, every published release of which (through 3.1.1) pins `views-frames <2.0.0`. Recorded here so the byte question is not re-opened when that constraint widens. Parquet bytes vary across pyarrow versions — that is why the pin is `>=16.1.0,<17.0.0` and why a local pyarrow 23.x fails byte-parity while CI passes (register **C-72**). `views_frames` sits above it: the fixture was generated under `1.0.0`, CI has reproduced it continuously under diff --git a/tests/test_clone_readiness.py b/tests/test_clone_readiness.py index 54784d2..c79e1c7 100644 --- a/tests/test_clone_readiness.py +++ b/tests/test_clone_readiness.py @@ -274,6 +274,53 @@ def test_the_machinery_does_not_pull_in_pipeline_core(): ) +@pytest.mark.parametrize("partner", _PARTNER_PACKAGES) +def test_a_partner_does_not_import_its_sibling(partner): + """The partners are independent, not merely both below the machinery. + + Everything else in this file proves the *vertical* arrows of ADR-002 — machinery + imports no partner, invariants import no machinery. Nothing proved the horizontal + one, and it is the arrow that keeps a partner liftable: `crafd/` and `unfao/` are + deliberate clones (register **C-33**), so the realistic violation is a copy-paste + that leaves a sibling's import behind. `test_the_machinery_imports_without_any_partner` + cannot see it — that test imports the machinery, and this would be partner-to-partner. + + Two halves, for the reason the module docstring already gives about regexes: the + subprocess is load-bearing and sees transitive arrivals; the source scan is the + supplement, and covers `managers/` — which the subprocess deliberately skips because + importing a manager needs views-pipeline-core, and a purity check should not be + contingent on a heavy framework being installed (C-40 (a)). + """ + siblings = tuple(f"views_postprocessing.{p}" for p in _PARTNER_PACKAGES if p != partner) + if not siblings: + pytest.skip("independence needs a sibling; only one partner is declared") + + importable = sorted( + m for m in _modules_on_disk(partner) + if ".managers" not in m and not m.endswith(".__init__") + ) + assert importable, f"no importable modules found for {partner}" + + result = _import_in_subprocess(tuple(importable), siblings) + assert result.returncode == 0, ( + f"{partner}'s own modules failed to import:\n{result.stderr}" + ) + leaked = [m for m in result.stdout.split("LEAKED:")[-1].strip().split(",") if m] + assert not leaked, ( + f"{partner} pulled in a sibling partner: {leaked}. The two are deliberate " + "clones (C-33) and must stay liftable one at a time — an import between them " + "means neither can be taken without the other, and no other test here sees it." + ) + + manager = (_PKG / partner / "managers" / f"{partner}.py").read_text() + for sibling in siblings: + assert sibling not in manager, ( + f"{partner}'s manager names {sibling}. These files are copies of each other, " + "so this is the shape a careless clone leaves behind — and it is outside the " + "subprocess half above, which skips managers." + ) + + @pytest.mark.parametrize("partner", _PARTNER_PACKAGES) def test_the_guard_would_actually_catch_a_violation(partner): """A purity test that cannot fail is decoration. From 7648eeaba5e0c98955a857fee953548ed012fe7d Mon Sep 17 00:00:00 2001 From: Polichinl Date: Fri, 21 Aug 2026 12:37:45 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(tests):=20match=20imports,=20not=20pros?= =?UTF-8?q?e,=20in=20the=20independence=20check=20=E2=80=94=20/review-diff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The source half scanned the manager's whole text for the sibling's module path. This repository's comments cite module paths constantly — C-33's own text points at `unfao/product.py` — so a documentation comment naming the sibling would have failed the test for a prose reason. That is the false alarm ADR-014 §3 says gets a guard deleted, and it would have been deleted for being right about nothing. Now walks the AST and looks at `Import` / `ImportFrom` targets only. Re-mutation-proven, both directions: a real `from views_postprocessing.unfao import product` -> caught a comment naming views_postprocessing.unfao -> ignored Suite: 470 passed, 3 skipped, 37 xfailed. ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_clone_readiness.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/tests/test_clone_readiness.py b/tests/test_clone_readiness.py index c79e1c7..5134b0f 100644 --- a/tests/test_clone_readiness.py +++ b/tests/test_clone_readiness.py @@ -28,6 +28,7 @@ from __future__ import annotations +import ast import subprocess import sys import textwrap @@ -312,13 +313,27 @@ def test_a_partner_does_not_import_its_sibling(partner): "means neither can be taken without the other, and no other test here sees it." ) - manager = (_PKG / partner / "managers" / f"{partner}.py").read_text() - for sibling in siblings: - assert sibling not in manager, ( - f"{partner}'s manager names {sibling}. These files are copies of each other, " - "so this is the shape a careless clone leaves behind — and it is outside the " - "subprocess half above, which skips managers." - ) + # IMPORTS only, via the AST — not a substring scan of the file. This repository's + # comments cite module paths constantly (C-33's own text points at `unfao/product.py`), + # so a scan of the whole text would fail on documentation and get deleted for crying + # wolf, which is ADR-014 §3's whole point. + manager = _PKG / partner / "managers" / f"{partner}.py" + imported = set() + for node in ast.walk(ast.parse(manager.read_text())): + if isinstance(node, ast.Import): + imported.update(a.name for a in node.names) + elif isinstance(node, ast.ImportFrom) and node.module and not node.level: + imported.add(node.module) + + offending = sorted( + name for name in imported + if any(name == s or name.startswith(s + ".") for s in siblings) + ) + assert not offending, ( + f"{partner}'s manager imports {offending}. These files are copies of each other, " + "so this is the shape a careless clone leaves behind — and it is outside the " + "subprocess half above, which skips managers." + ) @pytest.mark.parametrize("partner", _PARTNER_PACKAGES) From b699ccbd3e7e05f8a0e49d7bb8ce2359b54130c6 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Fri, 21 Aug 2026 12:44:58 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(tests):=20resolve=20relative=20imports?= =?UTF-8?q?=20in=20the=20independence=20check=20=E2=80=94=20/code-review?= =?UTF-8?q?=20high?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six findings. The first two are the same class this file's own docstring says defeated the previous regex — reintroduced by me while "tightening" a substring scan into an AST one. 1. MEDIUM — `and not node.level` skipped every relative import, so `from ...unfao import product` in a manager passed. Lines 155-158 of this module name `from ..contract import gaul_schema` as precisely the miss that made the regex insufficient, and `from ..crafd import product` is the form once used to demonstrate a real gap in `contract/enrichment.py`. `level` is now resolved against the file's own package. Note on the review's example: `from ..unfao import product` inside `crafd/managers/` resolves to `views_postprocessing.crafd.unfao`, which is not the sibling — so ignoring it is correct. From `crafd/product.py` the same statement does reach the sibling, and is caught. The resolution is depth-correct, verified at both depths. 2. MEDIUM — `from views_postprocessing import unfao` was invisible: the sibling's name is on the alias, not the module. Each alias is now joined onto the resolved prefix. 3. MEDIUM — the scan read only `managers/.py`, while the subprocess half drops the whole `managers` package. `managers/__init__.py` carries a real import today and was covered by neither. Now globs `managers/**/*.py`, which also removes a FileNotFoundError traceback for a manager not named after its partner. 4. LOW — `".managers" not in m` is a substring test: a module named `managers_shared.py` would be dropped from the subprocess half while sitting outside the AST half, exempt from the guard with no signal. Matches on the package segment now. 5. LOW — `not m.endswith(".__init__")` was dead; `_modules_on_disk` already filters those. Removed, with a note saying where inits are covered instead. 6. LOW — the fixture README insertion split the sentence it interrupted, leaving the pyarrow explanation stranded after a views_frames digression. Restored, new material in its own paragraph. Mutation-proven against the real files, every form: 3-dot relative, the package form, the dotted import, and a sibling import in `managers/__init__.py` — each fails; an innocent `contract` import and a comment naming the sibling do not. Suite: 470 passed, 3 skipped, 37 xfailed. ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- tests/fixtures/wire_contract/README.md | 11 +++- tests/test_clone_readiness.py | 81 +++++++++++++++++++------- 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/tests/fixtures/wire_contract/README.md b/tests/fixtures/wire_contract/README.md index 882c812..bcab8c5 100644 --- a/tests/fixtures/wire_contract/README.md +++ b/tests/fixtures/wire_contract/README.md @@ -33,10 +33,19 @@ tool versions** (numpy per lockfile, `pyarrow 16.1.0`). The committed bytes + `S are canonical regardless. **A change to this fixture is a change to the contract (§10)** — do not regenerate casually. -**`pyarrow` is the version-sensitive one; `views_frames` is not — and that now includes across a MAJOR.** Measured 2026-08-21 in an isolated environment at the pinned toolchain (pyarrow 16.1.0, numpy 1.26.4): the shard emitted through `views_frames.io.arrow` under **1.10.2** and under **2.0.0** hash identically, and both equal the committed fixture (`203650fd…12c54`). All 61 frames-dependent tests pass at 2.0.0 there. So the views-frames 2.0.0 adoption (#286) is **not** a fixture re-vendor — it is blocked only by views-pipeline-core, every published release of which (through 3.1.1) pins `views-frames <2.0.0`. Recorded here so the byte question is not re-opened when that constraint widens. Parquet bytes vary +**`pyarrow` is the version-sensitive one; `views_frames` is not.** Parquet bytes vary across pyarrow versions — that is why the pin is `>=16.1.0,<17.0.0` and why a local pyarrow 23.x fails byte-parity while CI passes (register **C-72**). `views_frames` sits above it: the fixture was generated under `1.0.0`, CI has reproduced it continuously under `1.6.0`, and `1.10.2` was verified to regenerate **all five artifacts byte-identically** before the pin was raised (2026-08-02). Raising it again does not require regenerating the fixture — but it does require proving that, the same way. + +**Confirmed across a `views_frames` MAJOR (2026-08-21).** The shard emitted through +`views_frames.io.arrow` under **1.10.2** and under **2.0.0** hashes identically, and both +equal the committed fixture (`203650fd…12c54`) — measured in an isolated environment at the +pinned toolchain (pyarrow 16.1.0, numpy 1.26.4), where all 61 frames-dependent tests also +pass at 2.0.0. So the views-frames 2.0.0 adoption (#286) is **not** a fixture re-vendor. It +is blocked only by views-pipeline-core, every published release of which (through 3.1.1) +pins `views-frames <2.0.0`. Recorded so the byte question is not re-opened when that +constraint widens. \ No newline at end of file diff --git a/tests/test_clone_readiness.py b/tests/test_clone_readiness.py index 5134b0f..70be180 100644 --- a/tests/test_clone_readiness.py +++ b/tests/test_clone_readiness.py @@ -104,6 +104,40 @@ def _partner_prefixes() -> tuple[str, ...]: return tuple(f"views_postprocessing.{name}" for name in _PARTNER_PACKAGES) +def _imported_modules(path: Path, package: str) -> set[str]: + """Every absolute module name ``path`` imports, relative forms resolved. + + Three forms have to survive this, and the module docstring above names two of them + as the misses that made the earlier regex insufficient: + + import views_postprocessing.unfao.product + from views_postprocessing.unfao import product + from views_postprocessing import unfao <- the name is on the alias + from ..unfao import product <- the name is in `level` + + The last two are why this resolves `level` against the file's own package and joins + each alias onto the module. A first pass at this skipped both and would have passed + a manager importing its sibling relatively — the exact shape `contract/enrichment.py` + once used to demonstrate a real gap. + """ + parts = package.split(".") + found: set[str] = set() + for node in ast.walk(ast.parse(path.read_text())): + if isinstance(node, ast.Import): + found.update(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom): + if node.level: + base = parts[: len(parts) - (node.level - 1)] + prefix = ".".join(base + ([node.module] if node.module else [])) + else: + prefix = node.module or "" + if not prefix: + continue + found.add(prefix) + found.update(f"{prefix}.{alias.name}" for alias in node.names) + return found + + def _modules_on_disk(package: str) -> set[str]: return { "views_postprocessing." + f.relative_to(_PKG).with_suffix("").as_posix().replace("/", ".") @@ -296,9 +330,12 @@ def test_a_partner_does_not_import_its_sibling(partner): if not siblings: pytest.skip("independence needs a sibling; only one partner is declared") + # "managers" as a package SEGMENT, not a substring: a partner module named + # `managers_shared.py` would otherwise be dropped from this half while also sitting + # outside the AST half below, exempting it from the guard entirely with no signal. + # (`_modules_on_disk` already excludes `__init__.py`, so those arrive via the glob.) importable = sorted( - m for m in _modules_on_disk(partner) - if ".managers" not in m and not m.endswith(".__init__") + m for m in _modules_on_disk(partner) if "managers" not in m.split(".") ) assert importable, f"no importable modules found for {partner}" @@ -314,26 +351,26 @@ def test_a_partner_does_not_import_its_sibling(partner): ) # IMPORTS only, via the AST — not a substring scan of the file. This repository's - # comments cite module paths constantly (C-33's own text points at `unfao/product.py`), - # so a scan of the whole text would fail on documentation and get deleted for crying - # wolf, which is ADR-014 §3's whole point. - manager = _PKG / partner / "managers" / f"{partner}.py" - imported = set() - for node in ast.walk(ast.parse(manager.read_text())): - if isinstance(node, ast.Import): - imported.update(a.name for a in node.names) - elif isinstance(node, ast.ImportFrom) and node.module and not node.level: - imported.add(node.module) - - offending = sorted( - name for name in imported - if any(name == s or name.startswith(s + ".") for s in siblings) - ) - assert not offending, ( - f"{partner}'s manager imports {offending}. These files are copies of each other, " - "so this is the shape a careless clone leaves behind — and it is outside the " - "subprocess half above, which skips managers." - ) + # comments cite module paths constantly (C-33's own text points at + # `unfao/product.py`), so scanning the text would fail on documentation and get + # deleted for crying wolf, which is ADR-014 §3's whole point. + # + # EVERY file under `managers/`, not just `.py`: `managers/__init__.py` + # carries a real import today, and the subprocess half skips the whole package. + managers = sorted((_PKG / partner / "managers").rglob("*.py")) + assert managers, f"{partner} has no managers/ directory to scan" + for source in managers: + module = "views_postprocessing." + source.relative_to(_PKG).with_suffix("").as_posix().replace("/", ".") + package = module.rsplit(".", 1)[0] + offending = sorted( + name for name in _imported_modules(source, package) + if any(name == sib or name.startswith(sib + ".") for sib in siblings) + ) + assert not offending, ( + f"{source.relative_to(_REPO)} imports {offending}. These files are copies of " + "each other, so this is the shape a careless clone leaves behind — and it is " + "outside the subprocess half above, which skips managers/." + ) @pytest.mark.parametrize("partner", _PARTNER_PACKAGES)