Follow-up from the approach review for #4673 (server/fleet folder decomposition).
Problem
Tests reach production seams through dotted strings — monkeypatch.setattr("autoskillit.server._lifespan.register_active_kitchen", ...), mock.patch("autoskillit.server._response_budget.logger.info"). The string is resolved at call time against sys.modules, so nothing verifies it until the test runs, and nothing connects it to the module it names.
When a module relocates, every such string silently becomes wrong. monkeypatch.setattr raises on a missing attribute, so most fail loudly — but a string pointing at a module that still exists under the old name, or one whose target was re-exported elsewhere, patches the wrong object and the test passes while asserting nothing.
Object form — monkeypatch.setattr(module, "attr", value) — moves the failure to import time and lets the type checker and the refactoring tools see the reference.
Scope
The fragile subset is patch targets whose dotted prefix names an internal submodule that can relocate. Not every dotted string qualifies, and converting the rest would be churn:
- In scope:
autoskillit.server._misc._prime_quota_cache, autoskillit.server._lifespan.register_active_kitchen, autoskillit.fleet._dispatch_reaper.kill_process_tree — the prefix is a module path that a decomposition ticket can move.
- Out of scope — gateway targets:
autoskillit.server._get_ctx (138 occurrences), autoskillit.server._ctx (105), autoskillit.server.logger (96). These name the package gateway, whose __all__ is deliberately stable. They do not break on module moves. Leave them.
- Out of scope — third-party symbols seen through a module:
autoskillit.execution.merge_queue.asyncio.sleep, autoskillit.cli.subprocess.Popen. String form is the clearer expression of "the asyncio this module sees"; object form obscures it.
Evidence
Measured at c3154b77:
Highest-density files: tests/fleet/test_dispatch_reaper.py (86), tests/server/test_tools_kitchen_envelope_failure.py (57), tests/server/test_lifespan_fleet_boot.py (56), tests/cli/test_terminal.py (47), tests/server/test_lifespan_skill_boot.py (38).
Sequencing
Land after #4673. That ticket rewrites the 191 affected strings in place to keep them working; converting the same lines to object form concurrently would conflict.
Notes
Some seams exist precisely to avoid an import — a module-object alias like _lifespan_pkg is bound in production to keep a late-bound lookup working. Where a test patches such a seam, object form requires importing the module the production code deliberately reaches through sys.modules. Check each case rather than converting mechanically; a conversion that forces a new import into a test module is a signal to leave that one alone.
Batch by directory so each batch is independently reviewable and independently passes task test-check.
Follow-up from the approach review for #4673 (server/fleet folder decomposition).
Problem
Tests reach production seams through dotted strings —
monkeypatch.setattr("autoskillit.server._lifespan.register_active_kitchen", ...),mock.patch("autoskillit.server._response_budget.logger.info"). The string is resolved at call time againstsys.modules, so nothing verifies it until the test runs, and nothing connects it to the module it names.When a module relocates, every such string silently becomes wrong.
monkeypatch.setattrraises on a missing attribute, so most fail loudly — but a string pointing at a module that still exists under the old name, or one whose target was re-exported elsewhere, patches the wrong object and the test passes while asserting nothing.Object form —
monkeypatch.setattr(module, "attr", value)— moves the failure to import time and lets the type checker and the refactoring tools see the reference.Scope
The fragile subset is patch targets whose dotted prefix names an internal submodule that can relocate. Not every dotted string qualifies, and converting the rest would be churn:
autoskillit.server._misc._prime_quota_cache,autoskillit.server._lifespan.register_active_kitchen,autoskillit.fleet._dispatch_reaper.kill_process_tree— the prefix is a module path that a decomposition ticket can move.autoskillit.server._get_ctx(138 occurrences),autoskillit.server._ctx(105),autoskillit.server.logger(96). These name the package gateway, whose__all__is deliberately stable. They do not break on module moves. Leave them.autoskillit.execution.merge_queue.asyncio.sleep,autoskillit.cli.subprocess.Popen. String form is the clearer expression of "theasynciothis module sees"; object form obscures it.Evidence
Measured at
c3154b77:Highest-density files:
tests/fleet/test_dispatch_reaper.py(86),tests/server/test_tools_kitchen_envelope_failure.py(57),tests/server/test_lifespan_fleet_boot.py(56),tests/cli/test_terminal.py(47),tests/server/test_lifespan_skill_boot.py(38).Sequencing
Land after #4673. That ticket rewrites the 191 affected strings in place to keep them working; converting the same lines to object form concurrently would conflict.
Notes
Some seams exist precisely to avoid an import — a module-object alias like
_lifespan_pkgis bound in production to keep a late-bound lookup working. Where a test patches such a seam, object form requires importing the module the production code deliberately reaches throughsys.modules. Check each case rather than converting mechanically; a conversion that forces a new import into a test module is a signal to leave that one alone.Batch by directory so each batch is independently reviewable and independently passes
task test-check.