refactor: enforce layer boundaries so adapters stop reaching into each other - #99
Merged
Conversation
This was referenced Aug 4, 2026
…h other
The package already had protocols/, slicer/ and download/ as separate
directories, but nothing enforced the separation and it had drifted:
- slicer/output.py imported a *private FTPS helper* to delete a partial
file, so a change to Bambu transport code silently changed slicer
behavior — exactly the coupling the split was meant to prevent
- slicer/options.py reached into download/naming for a basename helper
- download/ pulled three private filesystem helpers out of protocols/ftps
- protocols/ftps and protocols/mqtt reached *up* to bambu_cli.printer for
an ambient get_printer(), making them untestable in isolation
- job/steps.py late-bound to bambu_cli.commands for its default handlers
- interactive/ imported cli.build_parser, dragging the entrypoint (and its
sys.exit) into the domain layer
Rather than move files and hope, scripts/check_layers.py now assigns every
module a rank and fails CI on any upward import or any import between the
three sibling adapters. Deferred (function-local) imports count: they break
the import cycle, not the dependency.
Changes:
- new fsutil.py (rank 10) holds the four pure path/file helpers that were
stranded in protocols/ftps and download/naming
- new cliparse.py holds the argparse tree; cli.py drops 728 -> 308 lines
and keeps main()/dispatch, so it remains the only module with sys.exit
- camera.py -> protocols/camera.py (it is a TLS transport sharing tlspin)
- get_ftp(printer) and monitor_status(args, printer) now take the printer
instead of looking one up; the two callers that omitted it pass one
- JobSteps no longer defaults to bambu_cli.commands handlers. commands.cmd_job
is the composition root and wires them; a missing step raises MissingJobStep
instead of silently importing across a layer
- DIRECT_CAMERA_MODELS moved to constants.py so doctor and preflight can
both read it without an upward import
Behavior is unchanged. Tests that patched bambu_cli.printer.get_printer to
inject a printer now pass one directly.
One edge stays allowlisted with a reason: context -> printer (RuntimeContext
lazily builds a BambuPrinter). Fixing it needs a composition root that installs
a printer factory, which would have made this diff unreviewable.
Gates: 1146 passed (baseline 1145; +1 new test), coverage 86.2% (was 84.6%),
ruff/ruff-format/mypy/bandit/syntax/help/workflow smokes all green.
DLANSAMA
force-pushed
the
refactor/layer-boundaries
branch
from
August 5, 2026 12:49
7084611 to
a066582
Compare
DLANSAMA
marked this pull request as ready for review
August 5, 2026 14:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 1 of 4 in a structural refactor. Draft on purpose — see sequencing below.
Why
protocols/,slicer/anddownload/were already separate directories. Nothing enforced the separation, and it had drifted:The OrcaSlicer runner imported a private Bambu FTPS helper to delete a partial file, so a change to printer transport code silently changed slicer behavior. Five more of the same shape, four of them hidden behind function-local imports.
The load-bearing change
scripts/check_layers.py, now blocking in the lint job. It ranks every module and rejects any upward import, plus any import between the three sibling adapters. Deferred (function-local) imports count — they break the import cycle, not the dependency.Directories were never the mechanism. This is.
Changes
fsutil.py(rank 10)protocols/ftpsanddownload/namingcliparse.pycli.pydrops 728 -> 308 lines and stays the only module withsys.exitcamera.py->protocols/camera.pytlspinget_ftp(printer),monitor_status(args, printer)JobStepscommands.cmd_jobis the composition root; a missing step raisesMissingJobSteprather than importing across a layerDIRECT_CAMERA_MODELS->constants.pydoctorandpreflightboth read it without an upward importBehavior is unchanged. Two tests were injecting printers by patching
bambu_cli.printer.get_printer— module-global patching the repo guidance forbids — and now inject directly.Gates
Deliberately not fixed
context -> printer—RuntimeContextlazily builds aBambuPrinter. Allowlisted in the checker with a written reason; the real fix is a composition root installing a printer factory, which would have made this 39-file diff unreviewable.protocols/mqtt.py(~880 LOC) — same reasoning: not splitting a hotspot inside a boundary refactor.Both are now recorded in AGENTS.md under known architecture debt instead of being quietly dropped.
Sequencing
Draft because #97 (feat/tui) should merge first. This branch moves
camera.py, splitscli.py, and inverts thecommands <-> interactiveedge;tui/importsinteractive,commandsandprotocols, so whichever lands second resolves conflicts. Once #97 is in, I will rebase this ontomainand mark it ready.