Clean architecture audit fixes: align declared and actual architecture - #39
Merged
Merged
Conversation
… fixes) A clean-architecture audit found the runtime dependency rule held everywhere, but the type layer told a different story. This makes the two match, and adds the enforcement that keeps them matched. Dependency Rule: - CQRS query ports move from domain/*/interfaces to the application layer (application/query_handlers/interfaces.py): they serve views and return application read models, so the domain importing BookReadModel (the audit's only true inward-ring violation) disappears. Write-side ports (command repositories, units of work) stay in the domain. - All 17 handler annotations re-pointed from concrete adapters (LoanUnitOfWork, CacheAdapter, PatronQueryRepository) to ports (ILoanUnitOfWork, ICache, application query ports); 8 phantom TYPE_CHECKING imports of non-existent names fixed - sqlalchemy leaves the presentation layer: the loan unit of work translates IntegrityError on the unique-active-loan index into BookNotAvailableException at the boundary - Health routes stop importing infrastructure directly: PostgreSQL gains ping(), the circuit-breaker registry is container-provided Boundary contracts: - PatronReadModel/LoanReadModel were each defined twice; all read models consolidated in application/query_handlers/read_models.py - Patron and loan query repositories return typed read models instead of raw dicts, matching the book convention end to end - ICache protocol updated to the adapter's real (async) surface Main component: - api/main.py becomes a create_app() factory (uvicorn --factory); importing the module no longer builds the container or calls etcd, so the non-e2e test suite runs with zero infrastructure Dead architecture removed: - BookRepository, ICatalogQueryRepository, IPatronRepository, EmailTemplate, ITemplateRenderer (all zero-use) Enforcement: - mypy configured in pyproject (SQLAlchemy legacy-Column noise scoped to persistence adapters, documented) and clean on 134 files - GitHub Actions CI: mypy + the infrastructure-free test suites - mypy immediately caught two real defects: three patron routes dereferencing a possibly-None query result (500 on missing patron), and the reaper calling find_expired_reservations which existed on the concrete repository but not on the port 157 tests passing; verified live (factory app, saga end-to-end, health endpoints through the container-provided registry).
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.
Summary
An Uncle Bob-perspective audit found the runtime dependency rule held perfectly (zero framework imports inward of infrastructure, mechanically verified) — but the type layer told a different story: annotations violated the rule, most ports had no dependents, and dead machinery misled readers. This PR makes the declared architecture and the actual architecture the same thing, and adds the enforcement that keeps them that way.
Dependency Rule
application/query_handlers/interfaces.py) — they serve views and return application read models, so the audit's only true inward-ring violation (domain importingBookReadModel) disappears. Write-side ports stay in the domain.ILoanUnitOfWork,ICache, query ports); 8 phantom imports of non-existent names fixedIntegrityErroron the unique-active-loan index is translated toBookNotAvailableExceptioninside the loan unit of workPostgreSQL.ping(), circuit-breaker registry provided by the containerBoundary contracts
PatronReadModel/LoanReadModelwas defined twice); patron/loan query repositories now return typed read models instead of raw dictsICacheprotocol matches the adapter's real async surfaceMain component
api/main.pyis now acreate_app()factory (uvicorn --factory): importing the module performs no I/O, so the non-e2e suite runs with zero infrastructure (verified with etcd unreachable)Dead architecture removed
BookRepository,ICatalogQueryRepository,IPatronRepository,EmailTemplate,ITemplateRenderer— all zero-use.Enforcement (the part that keeps it fixed)
Columnnoise scoped to persistence adapters with a documented overridefind_expired_reservationswhich existed on the concrete repository but not on its portVerification