#284/#285: partner independence — and #286 is blocked upstream, with the byte question settled - #291
Conversation
…machinery (#284, #285) #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) <noreply@anthropic.com>
…view-diff 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) <noreply@anthropic.com>
…e-review high
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/<partner>.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) <noreply@anthropic.com>
Review round —
|
| form | before | now |
|---|---|---|
from views_postprocessing.unfao import product |
caught | caught |
import views_postprocessing.unfao.product |
caught | caught |
from ...unfao import product |
missed | caught |
from views_postprocessing import unfao |
missed | caught |
and not node.level skipped every relative import; and in the package form the sibling's name is on the alias, not the module. Lines 155–158 of this very file name from ..contract import gaul_schema as the miss that made the earlier regex insufficient — so this is the codebase's own demonstrated shape, not a hypothetical.
One correction to the review. It cited from ..unfao import product as a miss. Inside crafd/managers/ that resolves to views_postprocessing.crafd.unfao, which is not the sibling — ignoring it is correct. From crafd/product.py the same statement does reach the sibling, and is caught. Verified at both depths; the resolution is depth-correct.
And three smaller
- The scan read only
managers/<partner>.pywhile the subprocess half drops the wholemanagerspackage — somanagers/__init__.py, which carries a real import today, was covered by neither. Now globsmanagers/**/*.py. ".managers" not in mwas a substring test: a module namedmanagers_shared.pywould have been dropped from one half and invisible to the other — exempt from the guard with no signal. Matches on the package segment now.not m.endswith(".__init__")was dead code;_modules_on_diskalready filters those.- The fixture README insertion split the sentence it interrupted, stranding the pyarrow explanation after a views_frames digression. Restored, new material in its own paragraph.
Mutation proof
Against the real files, every form:
| mutation | result |
|---|---|
from ...unfao import product in the manager |
fails |
from views_postprocessing import unfao |
fails |
import views_postprocessing.unfao.product |
fails |
sibling import in managers/__init__.py |
fails |
| a comment naming the sibling | passes (no false alarm) |
an innocent contract import |
passes |
CI: 494 passed, 5 skipped, 37 xfailed, 0 failed.
Two issues, one branch, because #286 turned out to be a measurement rather than a change.
#286 — blocked upstream, but the risk is now settled
Every published
views-pipeline-corerelease, through the latest 3.1.1, pinsviews-frames <2.0.0. Poetry refuses the resolution outright. We cannot adopt views-frames 2.0.0 here until pipeline-core widens, so the pin is unchanged.But the question worth answering first is answered.
contract/wire/shard.pyemits throughviews_frames.io.arrow, so a major could have changed delivered bytes — which would be a three-repo fixture re-vendor, the shape of C-72. Measured in an isolated environment at the pinned toolchain (pyarrow 16.1.0, numpy 1.26.4):203650fd…12c54203650fd…12c54All 61 frames-dependent tests pass at 2.0.0 in that environment — including the three byte-parity modules that cannot even run in a drifted local venv.
The other three breaking changes were checked against the code rather than assumed:
contract/frames.pypasses a realSpatioTemporalIndexfrom_pgm_index; every.values[...]site in the package reads, none assigns;CONFORMANCE_FLOORis asserted nowhere.Recorded in
tests/fixtures/wire_contract/README.md, beside the existing claim that views_frames is not version-sensitive — which was written for 1.x and now has a MAJOR behind it. So nobody re-opens the byte question when the constraint widens.Adopting it needs views-pipeline-core to widen first. That is two repos changing together, so it is Simon's call whether to raise it there.
#284/#285 — the census got one fact wrong, and one real gap
#285 records this repo as "enforcement test: none". It has seven.
test_clone_readiness.pyproves both vertical arrows of ADR-002 by importing packages in a subprocess with the others blocked, and its docstring says a regex was tried first and rejected — it missedfrom ..contract import xandfrom 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
crafdandunfaoimporting each other — and that is the arrow that keeps a partner liftable. The two are deliberate clones (C-33), so the realistic violation is a copy-paste leaving a sibling's import behind, andtest_the_machinery_imports_without_any_partnercannot see it because that test imports the machinery.Added in two halves, matching the split this file already documents:
managers/, which the subprocess skips because importing a manager needs views-pipeline-core and a purity check should not depend on a heavy framework being installed (C-40 (a)).Mutation-proven both ways: a sibling import in
crafd/product.pyfails the first half; a sibling named incrafd/managers/crafd.pyfails the second.Why not import-linter
#284's contract would add a dev dependency, a CI step and a config block to assert three properties — two already covered here and covered more strongly, and the third being exactly this test. #285 itself points approvingly at views-datafactory doing the same thing in two assertions in an existing file, "no new dependency, no graph library, no new module". That argument applies here.
Nothing is foreclosed: if the platform standardises on import-linter later, it is a one-line
pyprojectblock and this test can stay or go.(Noted, not acted on:
frame_extraction.py::drop_unitsdefers an import inside the function to avoid a cycle that does not exist —contract/frames.pyimports nothing fromcontract. Harmless.)Verification
Suite 470 passed, 3 skipped, 37 xfailed; the 26 failures are C-104 venv drift, green in CI.
ruffclean. Your virtualenv was not touched — the 2.0.0 measurement ran in an isolated environment andpoetry lockwas reverted.