fix(ci): run tests/architecture/ guards on every PR, and fix the four they were hiding

[dieterolson]

Why

tests/architecture/test_sidekick_external_imports_3316.py::test_production_code_uses_shared_python_imports has been failing against main and nobody noticed, because the tests job runs changed test files plus a pinned core_tests set. Architecture guards assert properties of the tree as a whole, so the file a guard protects is almost never the guard's own file — under changed-file selection they only ran when someone happened to edit the guard itself.

That is not theoretical. It blocked #4446, which was a cosmetic ruff format reformat that enlisted the file; #4446 worked around it by restoring main's bytes so the file dropped back out of the changed set — re-hiding the failure rather than fixing it. Sweeping the whole directory turned up four failing guards across two files, not one.

Verdict on the sidekick imports: they were wrong

src/shared/python/import_aliases.py documents the bare sidekick. / theme. / ai. spellings as legacy-only for the #3316 deprecation window; they resolve through the compat shims in src/<pkg>/__init__.py. The package already uses the intended idiom nearly everywhere — 388 relative intra-package imports, and canonical shared.python.* for cross-package. The 18 violations were stragglers the codemod missed, and two files prove it internally:

• standalone/session_store.py imported from sidekick.persistence.schema two lines above from shared.python.contracts.
• standalone/window.py:241 used bare ai. twenty lines from an existing shared.python.theme. import.

So: intra-package → relative, cross-package → canonical shared.python.*. This also makes the package importable without src/ on sys.path, which matters for the standalone wheel.

Two further defects the sweep exposed

The guard could not have passed even at zero violations. _shared_python_roots() was called from _root_matches() once per import node in every src/**/*.py, re-running an ~8 ms directory scan each time — 66.9 s against the repo-wide 60 s pytest timeout. Cached, it is 9.3 s.

test_gh1696_god_modules.py was path-rotted. Its pressure_drop_calculator constants pointed under src/shared/python/upstream_drift_tools/, which the rename to sidekick reduced to a lone __init__.py. All three guarded invariants still hold at the canonical path (160 lines ≤ 200, no inline classes, _legacy.py present), so this is purely stale paths.

Closing the hole

Replaced the single test_layer_boundaries.py pin with the whole tests/architecture/ directory, so guards added later are covered the day they land instead of waiting for someone to remember this list. pytest deduplicates a directory against a file inside it, so the three architecture entries in the codex/tools-3316-import-canonicalization branch block stay harmless. The empty_core guard (#3324) covers the new entry — it collects 64 tests, so a future directory rename cannot silently make it vacuous.

Ordering is deliberate: the four fixes are in the parent commit, the always-on switch in the child, so main never goes red.

Verification

Check	Result
tests/architecture/ (whole directory, -n 0)	64 passed in 56.96s (was 3 failed + 1 timeout)
Guard runtime	66.9s → 9.3s
_duplicate_import_violations()	18 → 0
empty_core collect simulation on new entry	exit 0, 64 collected
Every relative import target resolved via find_spec	16/16 OK, 0 unresolvable
ruff check + ruff format --check (320 files)	clean
mypy on all 8 changed files	clean
prettier --check SPEC.md + workflow	clean
Sidekick per-file coverage gate	not triggered — no changed file is in coverage_policy.json tracked_packages

Pre-existing failures confirmed unrelated by re-running them against a pristine origin/main tree in the same worktree: 6 in tests/unit/sidekick/test_standalone_* + test_standalone_wheel.py (local venv is missing platformdirs, which is declared in pyproject.toml), and 3 in tests/ops/test_ci_standard_web_dependencies.py. Identical failures before and after.

One thing deliberately not fixed

StandalonePreferences.apply_tokens() imports theme.sidekick_tokens, which does not exist anywhere in the repo under either spelling — neither COLOR_TOKEN_MAP nor DEFAULT_SIDEKICK_TOKENS is defined in src/. The method has no callers and no tests, and was already dead on main; both spellings raise ModuleNotFoundError identically, so this PR is behaviour-neutral there. Choosing between deleting it (it is on the public API surface, so it needs a baseline regen plus downstream coordination) and reimplementing it against the live ui/design_tokens.get_token_dict is a product call, so the defect is documented in place rather than silently "fixed", and tracked separately.

Note

--timeout=60 means any always-on guard has to stay comfortably under a minute. The directory is at ~57s serial for the whole set, with the slowest single test at 9.3s, so there is headroom — but it is worth knowing that whole-tree AST guards are the natural place for this to bite again.

🤖 Generated with Claude Code

---

Update — root cause fixed, not just the symptom

The original PR made the tests/architecture/ guards always-on. Follow-up work in this branch fixes the reason those guards were needed and closes the same hole one layer up.

Why dead imports survive here

Probing each gate with a deliberately bogus first-party import:

Gate	Result
repo mypy (mypy.ini)	passes
CI delta-mypy (--ignore-missing-imports --follow-imports=skip)	passes
ruff check	passes
mypy with ignore_missing_imports = False	error: Cannot find implementation or library stub … [import-not-found]

ignore_missing_imports = True is the sole suppressor — --follow-imports=skip does not hide it. Ruff never resolves imports, and F401 stays quiet because the names are used. All six fleet repos carry this setting, so the blindness is fleet-wide even though the changed-file selection blind spot is Tools-only (every other repo runs pytest tests/ wholesale).

Two more live defects, both silent

• signal_toolkit.polynomial_generator imported shared.python.logging_config inside try/except ImportError. The real module is shared.python.logging_pkg.logging_config, so it had been falling back to bare logging forever. That module name exists in no fleet repo.

• model_generation.humanoid and model_generation.mesh each wrapped all their re-exports in one try/except ImportError: pass. A single missing module aborted the whole block. Verified: 34 of 34 and 14 of 14 exported names were dead at runtime while __all__ advertised them — from …model_generation.humanoid import BodyParameters raised ImportError. Both files carried # mypy: ignore-errors.

  Every name existed; only the submodule paths were wrong (.appearance, .builder, .segments, .urdf, .mesh.lod, .mesh.mesh_inertia never existed). Repointed at the defining modules, both pragmas removed, both facades now 34/34 and 14/14 live.

The guard

tests/architecture/test_import_resolvability.py walks every Import/ImportFrom in src/ and resolves first-party targets by filesystem path walk against declared roots — deliberately not importlib.util.find_spec, which reports failure when a parent package raises on import, depends on ambient sys.path, and executes package side effects during a static check. Naive find_spec resolution reported 63 production violations, ~40 of them phantom sub-app path artifacts; the path walk reported the 10 real ones.

Its one exemption is not a defect: shared.python.launcher_embed is UpstreamDrift's embeddable-tool contract, and Tools' migrated data_explorer adapter registers itself only when that host is present — a genuine cross-repo soft dependency. _HOST_PROVIDED is kept separate from an empty _KNOWN_UNRESOLVED ratchet so the two never blur, and two further tests fail if an entry stops being imported or starts resolving locally, so an exemption can't quietly become permanent.

Verified by negative control: reintroducing shared.python.theme.sidekick_tokens (the import that started all this) fails the guard with file:line; removing it passes.

tests/ops/ — the same blind spot one layer up

Those tests assert on .github/workflows/*.yml, so editing a workflow never enlists them. Three had been red since quality-gate moved to ubuntu-24.04 and correctly left the self-hosted tool-cache steps behind — the tests still demanded them by job name.

Retargeted at the mechanism rather than a job list, so a future migration can't rot them again: the tool-cache contracts now key on jobs that opt into the shared fleet cache, additionally assert such a job is self-hosted, and require the cleaner's version argument to track that job's own setup-python request instead of a duplicated literal. quality-gate's pip-isolation contract now asserts the isolation actually in force ($RUNNER_TEMP/ci-venv + PYTHONNOUSERSITE). Scoping to self-hosted jobs generally was tried first and over-reached — rust-quality-gate provisions Python without the tool-cache dance.

Verified by mutation: renaming the tool-cache step still fails the guard. Then added to core_tests — 130 tests in ~2 s, no Python provisioning.

Verification

Check	Result
tests/architecture/ + tests/ops/ + model_generation + signal_toolkit	777 passed, 3 skipped, 50.9 s
Import-resolvability guard	3 passed in 6.8 s; fails correctly on negative control
empty_core collect simulation, tests/ops/	exit 0, 133 collected
Both facades at runtime	34/34 and 14/14 names live (were 0/34, 0/14)
CI-exact delta mypy on all changed files	clean
ruff check / ruff format --check / prettier	clean

One pre-existing no-any-return in model_generation/inertia/calculator.py:120 is now inherited because this PR touches that file; fixed with the repo's established # type: ignore[no-any-return] idiom and confirmed live under --warn-unused-ignores.

Fleet

Scanned every fleet repo's origin/main. UpstreamDrift has the identical cluster — it owns its own humanoid_character_builder and logging_pkg trees with the same missing submodules — filed as UpstreamDrift#8641. The patch is explicitly not transplantable: the trees have drifted (URDFGeneratorConfig and PrimitiveMeshGenerator live in different modules there), which the issue calls out.

Gasification_Model's raw count is inflated by its Tools symlink — it has no own shared/python/sidekick/ tree, so those imports are shared-library-provided rather than broken. A guard there needs the host-provided concept first.

Not done here

Narrowing ignore_missing_imports to per-module overrides is the durable fix, but it can't be flipped in one PR: legacy bare sidekick./theme./ai. spellings resolve only through the runtime alias finder and are invisible to mypy, so flipping today floods errors. It should follow the #3316 canonicalisation — at which point it also makes that deprecation statically enforceable.