Skip to content

refactor(core): eliminate three circular import edges (DIP violations) (#1937)#2013

Open
mvillmow wants to merge 2 commits into
mainfrom
1937-fix-circular-imports
Open

refactor(core): eliminate three circular import edges (DIP violations) (#1937)#2013
mvillmow wants to merge 2 commits into
mainfrom
1937-fix-circular-imports

Conversation

@mvillmow

Copy link
Copy Markdown
Collaborator

Summary

Eliminates the two live circular import cycles identified in #1937 and adds import-linter enforcement to prevent regression.

  • Edge 2 (cli↔e2e): Promoted shared progress types (RunStatus, RunProgress, TierProgress, EvalProgress, ProgressDisplay, format_duration, format_progress_bar) from scylla.cli.progress to new scylla.core.progress. Repointed scylla.e2e.progress to import from scylla.core.progress (atomic commit — no intermediate state with the violation). scylla.cli.progress is now a back-compat re-export shim.
  • Edge 3 (adapters↔e2e): Promoted TokenStats from scylla.e2e.models_internals.token_stats to new scylla.core.token_stats. Repointed all consumers: adapters/base.py, analysis/loader_internals/models.py, analysis/loader_internals/run_loader.py, e2e/models.py, e2e/models_internals/results.py. The original file becomes a one-line back-compat shim.
  • Edge 1 (config↔metrics): Confirmed NOT a live cycle — config/models.py already imports from scylla.core.thresholds. No code move required; regression locked by import-linter contracts.
  • import-linter: Added 3 forbidden contracts in pyproject.toml: e2e must not import cli, adapters must not import e2e, core has no scylla layer dependencies. Added lint-imports task to pixi and CI step in _required.yml. Added tests/test_import_layering.py pytest wrapper.
  • Mock retargeting: Both tests/unit/cli/test_progress.py and tests/unit/e2e/test_progress.py mock patch retargeted to scylla.core.progress.datetime.

Divergence from plan

  • cli/progress.py re-exports consolidated into a single import block (plan had separate blocks per symbol — functionally identical, cleaner style).
  • adapters.claude_code → e2e.rate_limit is a pre-existing intentional dependency narrowed via ignore_imports in the adapters must not import e2e contract, documented inline. Tracked as a follow-up refactor.

Verification

  • grep -rn "from scylla.cli" src/scylla/e2e/ → 0 matches
  • grep -rn "from scylla.e2e.*TokenStats" src/scylla/adapters/ src/scylla/executor/ src/scylla/metrics/ → 0 matches
  • pixi run --environment default lint-imports → Contracts: 3 kept, 0 broken
  • pixi run --environment lint mypy src/scylla/ → Success: no issues found in 180 source files
  • pixi run lint → All checks passed

Closes #1937

🤖 Generated with Claude Code

@mvillmow mvillmow enabled auto-merge (squash) May 29, 2026 23:45
@mvillmow mvillmow force-pushed the 1937-fix-circular-imports branch from c7681bd to 27ca9c6 Compare June 1, 2026 05:41
mvillmow and others added 2 commits June 28, 2026 08:47
#1937)

- Promote TokenStats from scylla.e2e.models_internals to scylla.core.token_stats
- Promote progress types from scylla.cli.progress to scylla.core.progress
- Repoint scylla.e2e.progress to scylla.core.progress (breaks cli↔e2e cycle)
- Repoint scylla.adapters.base to scylla.core.token_stats (breaks adapters↔e2e cycle)
- Retarget analysis/loader_internals TokenStats imports to scylla.core.token_stats
- Add back-compat re-export shims in cli/progress.py and e2e/models_internals/token_stats.py
- Retarget test mock patch sites to scylla.core.progress.datetime
- Add import-linter contracts (3 forbidden) in pyproject.toml with pixi lint feature
- Add pixi lint-imports task and CI step in _required.yml
- Add tests/test_import_layering.py pytest wrapper for import-linter contracts

Edge 1 (config↔metrics): not a live cycle; locked by import-linter contracts only.
Edge 2 (cli↔e2e): eliminated — e2e.progress no longer imports from scylla.cli.
Edge 3 (adapters↔e2e): eliminated — adapters.base no longer imports from scylla.e2e.
Note: adapters.claude_code→e2e.rate_limit pre-existing dep narrowed in contract.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
- pixi.lock was v7 (incompatible with CI pixi v0.67.2, max v6); regenerated
  from main + pixi.toml import-linter dep using pixi 0.67.2 -> v6
- Fix I001 import ordering in cli/progress.py

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
@mvillmow mvillmow force-pushed the 1937-fix-circular-imports branch from 27ca9c6 to e2a4536 Compare June 28, 2026 15:51
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.

[Audit] §3 CRITICAL: Three confirmed circular import edges (DIP violations)

1 participant