Skip to content

Lint the python and hold the package's layering in CI - #41

Merged
CrypticSwarm merged 4 commits into
masterfrom
refactor-guardrails
Aug 6, 2026
Merged

Lint the python and hold the package's layering in CI#41
CrypticSwarm merged 4 commits into
masterfrom
refactor-guardrails

Conversation

@CrypticSwarm

Copy link
Copy Markdown
Owner

Three checks that nothing was doing, plus the one real defect the first of them found.

Ruff, as a linter only

make lint runs ruff check over every Python file in the repo, with a pinned version in its own CI job. The rule set is ruff's defaults — pyflakes plus the pycodestyle checks about statement shape — and stops there deliberately: line length, import order, and whitespace are the author's, so switching the linter on does not reflow files a change never touched. ruff format is not part of this repo.

The launcher entry points had to be named explicitly in the config. Ruff finds files by extension and bin/run-anvil, bin/tongs, and bin/git-guard carry none, being commands rather than modules — which would have left the only python doing sys.path surgery as the only python nobody checks. Their existing # noqa: E402 comments were dead until now.

Turning it on surfaced exactly one thing: swarmforge/agents/translate.py imported strip_inline_comment without calling it. The name was reachable only because the parser tests still went through the translator's namespace, from before the parser moved to yamlite. Those assertions now call yamlite directly and the import is gone. It is the only source-file change in this PR.

The package's imports stay acyclic

The package serves both sides of the container boundary, and it only does that because its modules are layered: yamlite is a leaf either side can import, the tongs modules build on each other in one direction, and the anvil modules sit on top of tongs. Nothing held that shape.

A cycle is not a build failure — python resolves most of them, and the ones it cannot surface as a half-initialised module at startup, far from the import that closed the ring. So the graph is read straight out of the source with ast, and the whole ring is reported rather than the edge that closed it.

Two decisions worth knowing when it fires:

  • Reaching into a package counts as depending on it, because python runs the __init__ on the way in. An importer already inside that package does not count, since an __init__ importing its own children is how both subpackages are arranged. Without this the one shape that actually strands a half-initialised module — a ring closing through an __init__ — goes unreported.
  • Imports that do not run at import time are not edges: inside a function body, or behind if TYPE_CHECKING:. Those are the two standard ways to break a cycle, and a check that failed the fix it just demanded would be worse than no check.

Loading a module by file path stays in bin/

That is what the flat layout did before there was a package, and every consumer carried its own copy of the block deriving the path. What it loads is a second module under a name nothing imports — invisible to a linter, unmockable through the usual seams, and silently a different object from the one the rest of the tree holds.

The entry-point shims stay exempt, because resolving the checkout's path is the only thing they do. So does the status-line seeder's test: the seeder ships into the image as a standalone script rather than inside the package, so there is no module path to import it by. That exemption is itself asserted on — an entry whose file stops loading by path fails the build rather than quietly widening the rule.

Both scans say what they covered

A guardrail that silently scans nothing reports everything clean. The import graph refuses to skip a file it cannot parse, because everything it reads is ours. The path-loading scan walks the whole checkout, so it has to tolerate whatever a developer's tree holds — a fifo, a root-owned file a container wrote, a script for another interpreter — and therefore carries a floor: it must keep finding a shim, a package module, and a test. Directories it skips are matched by place rather than by name, so a package directory sharing a name with a container store is still read.

Checks

  • Unit suite by discovery: 495 → 503, nothing dropped, skipped, or weakened. The 8 added tests are the two guardrails plus the meta-tests that keep them from passing vacuously.
  • make lint clean, at the version CI pins.
  • Passthrough, launch-path, and image-layout tests green. The launch path and the image build are untouched; the single image-resident change is the dead import above, covered by the tests that run the container-side modules from a staged import root.

The translator imported `strip_inline_comment` without calling it: the
name was reachable only because the parser tests still went through the
translator's namespace, from before the parser moved to `yamlite`. Point
those assertions at `yamlite` and drop the import.
Nothing has been checking the Python for unused imports, undefined
names, or shadowed builtins -- the kinds of mistake that survive a green
test run because no test happens to reach the line.

Ruff's default rules only: pyflakes plus the pycodestyle checks about
statement shape, and none of the whitespace, line-length, or import-order
ones. A linter that reflows untouched files buries the diff that is
actually under review, so `ruff format` stays out of the repo entirely.
Test modules are exempt from the import-placement rule; each one puts the
repo root on sys.path first so it runs standalone as well as under
discovery.

The launcher entry points have to be named explicitly. Ruff finds files
by extension and those carry none, being commands rather than modules --
which would have left the only python doing sys.path surgery as the only
python nobody checks.

CI pins the ruff version: an unpinned linter turns a green branch red on
someone else's release day.
Two properties kept the package usable on both sides of the container
boundary, and nothing was checking either.

The first is that the modules are layered: `yamlite` is a leaf both sides
import, the tongs modules build on each other in one direction, and the
anvil modules sit on top of tongs. A cycle is not a build failure --
python resolves most of them, and the ones it cannot surface as a
half-initialised module at startup, far from the import that closed the
ring. So the graph is read out of the source with `ast` and the whole ring
is reported, not just the edge that closed it.

Reaching into a package counts as depending on it, since python runs the
`__init__` on the way in; an importer already inside the package does not,
because an `__init__` importing its own children is the arrangement here.
Imports that do not run at import time -- inside a function, or behind
`if TYPE_CHECKING:` -- are not edges at all. Those are the two standard
ways to break a cycle, and a check that failed the fix it asked for would
be worse than none.

The second property is that only the entry-point shims load python from a
file path. That is what the flat layout did, and it costs what a package
buys: a module loaded that way is a second copy under a name nothing
imports, invisible to a linter and unmockable through the usual seams. The
status-line seeder's test is the one standing exception -- the seeder ships
into the image as a standalone script, so there is no module path to reach
it by -- and the exemption is itself asserted on, so it cannot quietly
outlive its reason.

Both scans have to say what they covered. The import graph refuses to skip
a file it cannot parse, because everything it reads is ours. The
path-loading scan walks the whole checkout and so must tolerate whatever a
developer's tree holds, which means it needs a floor: it is required to
keep finding a shim, a package module, and a test. The directories it
skips are matched by place, not by name, so a package directory sharing a
name with a container store is still read.
The rules were only in the tests. Say what the levels are, so a
contributor learns the shape from the prose rather than from a failure.
@CrypticSwarm
CrypticSwarm merged commit e52ec31 into master Aug 6, 2026
4 checks passed
@CrypticSwarm
CrypticSwarm deleted the refactor-guardrails branch August 6, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant