Import hexagonal architecture enforcement - #8
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Hexagonal Architecture Enforcement ImplementationImports and adapts a repository-local AST-based hexagonal architecture checker (from Episodic) into BeatCue, enforcing package dependency boundaries through static analysis and package-barrel re-export expansion (explicit and star imports). The checker is policy-driven (domain, application, adapter, composition-root and infrastructure groups) and reports ARCH001 violations. Core Architecture Package (beatcue/architecture/)
Integration & Tooling
Design & Operational Documents
Tests & Fixtures
Notes & Follow-up
WalkthroughIntroduce a repository-local static architecture checker enforcing hexagonal import boundaries via AST analysis; expose it as python -m beatcue.architecture, add make check-architecture and run it from make lint; include fixture packages, tests (unit, parametrised and property), and documentation. ChangesHexagonal Architecture Enforcement
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
leynos
marked this pull request as ready for review
May 13, 2026 22:38
Reviewer's GuideAdds a local, AST-based hexagonal architecture checker under beatcue/architecture, wires it into make lint, and documents the enforced import boundaries and workflow, backed by fixture-based tests and an ADR/ExecPlan. Sequence diagram for make lint invoking the architecture checkersequenceDiagram
actor Developer
participant Make as make_lint
participant Ruff as ruff_check
participant ArchTarget as make_check_architecture
participant CLI as beatcue.architecture.cli.main
participant Checker as check_architecture
participant Reexp as build_reexport_index
participant Policy as default_policy
Developer->>Make: make lint
Make->>Ruff: ruff check
Ruff-->>Make: exit 0
Make->>ArchTarget: make check-architecture
ArchTarget->>CLI: python -m beatcue.architecture
CLI->>Checker: check_architecture(package_root, package, policy)
Checker->>Reexp: build_reexport_index(root, package)
Reexp-->>Checker: reexport_index
Checker->>Policy: default_policy()
Policy-->>Checker: ArchitecturePolicy
Checker-->>CLI: ArchitectureCheckResult
CLI->>CLI: print(violation.render()) [for each violation]
CLI-->>ArchTarget: exit code (0/1)
ArchTarget-->>Make: exit code (0/1)
Make-->>Developer: lint result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 313540a81d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Add a self-contained ExecPlan for importing Episodic's hexagonal architecture checker into BeatCue. Cover the planned implementation, validation gates, prior-art notes, and postmortem questions before any checker code is changed.
Add a repo-local architecture checker for BeatCue's planned hexagonal package boundaries. The checker parses imports with `ast`, expands package barrel re-exports, reports forbidden dependency directions, and exposes a small CLI entrypoint for local gates. Add fixture packages and tests for domain, application, adapter, re-export, star re-export, composition-root, and current-package cases. Keep the ExecPlan updated with validation evidence and review findings.
Add the `check-architecture` Makefile target and run it from `make lint` after Ruff so the local lint gate enforces BeatCue's package boundary. Document the accepted architecture fitness function in ADR 003, add contributor guidance for the new target, and complete the ExecPlan postmortem with prior-art and extraction notes.
Record the completed gate run, branch push, and draft pull request status in the ExecPlan so the living plan matches the published branch state.
Tighten architecture violation assertions so tests lock down both rendered messages and structured violation fields. Add CLI entrypoint coverage and valid relative-import cases for package and module imports. Split imported symbols from imported modules in the checker so policy classification only sees module names, and record the review follow-up in the ExecPlan.
lodyai
Bot
force-pushed
the
import-hex-architecture-enforcement
branch
from
May 14, 2026 09:14
25a8b0d to
9c25c94
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Give inbound adapters their own allowed-import set so CLI code can import the composition root without gaining permission to import outbound adapters directly. Add fixture coverage for the accepted CLI-to-config route and the forbidden CLI-to-outbound-adapter route, and record the policy decision in the ExecPlan.
Resolve the package-root source path directly through `__init__.py` and make literal `__all__` handling follow last-assignment semantics instead of stopping on dynamic assignments. Add regression coverage for package-root star re-exports and final resolvable `__all__` assignments. Also narrow `check-architecture` prerequisites to avoid redundant build work and align the ExecPlan status note with completion.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docs/execplans/import-hex-architecture-enforcement.md (1)
910-914:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the revision note match the final status.
Replace
IN PROGRESSwithCOMPLETEat Line 913 so the document has one authoritative state.Triage:
[type:docstyle]Patch
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/execplans/import-hex-architecture-enforcement.md` around lines 910 - 914, Update the revision note so the document reflects its final state by replacing the text "Status changed to `IN PROGRESS`" with "Status changed to `COMPLETE`" in the line containing that phrase (the line that currently reads "Status changed to `IN PROGRESS`, branch publication progress was recorded, and implementation may now proceed within the stated tolerances.").beatcue/architecture/policy.py (1)
131-164:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden outbound adapter import permissions.
Line 163 reuses
adapter_allowed, which still includesinbound_adapter. Remove that cross-adapter permission and give outbound adapters a dedicated allow-list soARCH001blocks outbound→inbound imports.Patch
inbound_adapter_allowed = frozenset({ "application", "composition_root", "domain", "inbound_adapter", }) + outbound_adapter_allowed = frozenset({ + "application", + "domain", + "infrastructure", + "outbound_adapter", + }) adapter_allowed = frozenset({ "adapter", "application", "domain", "infrastructure", "inbound_adapter", "outbound_adapter", @@ ModuleGroup( name="outbound_adapter", module_prefixes=(f"{package}.adapters.outbound",), - allowed_groups=adapter_allowed, + allowed_groups=outbound_adapter_allowed, ),Based on learnings: "Dependencies point inward: CLI and library adapters → application services → domain model and ports; outbound adapters → domain-owned ports".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@beatcue/architecture/policy.py` around lines 131 - 164, The current adapter_allowed frozenset includes "inbound_adapter", which lets outbound adapters import inbound ones; change this by removing "inbound_adapter" from adapter_allowed and create a new dedicated frozenset (e.g., outbound_adapter_allowed = frozenset({"adapter","application","domain","infrastructure","outbound_adapter"})) then update the ModuleGroup for outbound_adapter to use outbound_adapter_allowed instead of adapter_allowed; keep inbound_adapter_allowed as-is for the inbound ModuleGroup and ensure any other uses of adapter_allowed still match the intended inbound/adapters policy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@beatcue/architecture/policy.py`:
- Around line 131-164: The current adapter_allowed frozenset includes
"inbound_adapter", which lets outbound adapters import inbound ones; change this
by removing "inbound_adapter" from adapter_allowed and create a new dedicated
frozenset (e.g., outbound_adapter_allowed =
frozenset({"adapter","application","domain","infrastructure","outbound_adapter"}))
then update the ModuleGroup for outbound_adapter to use outbound_adapter_allowed
instead of adapter_allowed; keep inbound_adapter_allowed as-is for the inbound
ModuleGroup and ensure any other uses of adapter_allowed still match the
intended inbound/adapters policy.
In `@docs/execplans/import-hex-architecture-enforcement.md`:
- Around line 910-914: Update the revision note so the document reflects its
final state by replacing the text "Status changed to `IN PROGRESS`" with "Status
changed to `COMPLETE`" in the line containing that phrase (the line that
currently reads "Status changed to `IN PROGRESS`, branch publication progress
was recorded, and implementation may now proceed within the stated
tolerances.").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1700ba79-e647-4bce-8099-08f8520d0c29
📒 Files selected for processing (8)
beatcue/architecture/policy.pydocs/execplans/import-hex-architecture-enforcement.mdtests/fixtures/architecture/inbound_cli_imports_config/application.pytests/fixtures/architecture/inbound_cli_imports_config/cli.pytests/fixtures/architecture/inbound_cli_imports_config/config.pytests/fixtures/architecture/inbound_cli_imports_outbound_adapter/adapters/outbound.pytests/fixtures/architecture/inbound_cli_imports_outbound_adapter/cli.pytests/test_architecture_enforcement.py
Fail fast when the architecture checker is given a missing or non-directory package root. Tighten outbound adapter permissions so they do not inherit the fallback adapter policy. Document fixture noqa suppressions and add regression coverage for root validation, adapter permission splits, and final `__all__` assignment semantics.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_architecture_enforcement.py (1)
1-451: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this test module to satisfy the 400-line limit.
Break this file into focused test modules (for example checker, re-exports, and CLI) and keep each under the repository cap to preserve maintainability and policy compliance.
As per coding guidelines, "Files must not exceed 400 logical lines: Decompose large modules into subpackages".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_architecture_enforcement.py` around lines 1 - 451, This test module exceeds the 400-line policy; split it into multiple focused test files (e.g., test_architecture_checker.py, test_architecture_reexports.py, test_architecture_cli.py) so each file is under 400 logical lines. Move tests that exercise check_architecture and fixture_policy (including test_checker_reports_fixture_boundary_violations, test_checker_accepts_allowed_fixture_graphs, test_production_checker_accepts_current_beatcue_package, test_checker_rejects_missing_package_root, test_checker_rejects_file_package_root, test_fixture_policy_keeps_inbound_and_outbound_permissions_distinct) into the checker file; move reexport-related tests (test_reexport_index_resolves_star_imports_from_package_root, test_reexport_index_uses_last_resolvable_all_assignment, test_explicit_all_exports_uses_final_assignment, test_explicit_all_exports_returns_none_when_final_assignment_is_unresolved, and references to build_reexport_index and _explicit_all_exports) into the reexports file; and move CLI tests (test_cli_default_invocation_accepts_current_package, test_cli_none_argv_accepts_current_package, test_cli_fixture_policy_reports_fixture_violations, test_cli_fixture_policy_switches_from_default_policy) into the CLI file. Preserve the original imports (check_architecture, fixture_policy, relative_import_base, architecture_main, build_reexport_index, _explicit_all_exports) and FIXTURE_ROOT variable where needed, update module-level fixtures/parametrize decorators to their new files, and run pytest to confirm no import paths or name collisions remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/test_architecture_enforcement.py`:
- Around line 1-451: This test module exceeds the 400-line policy; split it into
multiple focused test files (e.g., test_architecture_checker.py,
test_architecture_reexports.py, test_architecture_cli.py) so each file is under
400 logical lines. Move tests that exercise check_architecture and
fixture_policy (including test_checker_reports_fixture_boundary_violations,
test_checker_accepts_allowed_fixture_graphs,
test_production_checker_accepts_current_beatcue_package,
test_checker_rejects_missing_package_root,
test_checker_rejects_file_package_root,
test_fixture_policy_keeps_inbound_and_outbound_permissions_distinct) into the
checker file; move reexport-related tests
(test_reexport_index_resolves_star_imports_from_package_root,
test_reexport_index_uses_last_resolvable_all_assignment,
test_explicit_all_exports_uses_final_assignment,
test_explicit_all_exports_returns_none_when_final_assignment_is_unresolved, and
references to build_reexport_index and _explicit_all_exports) into the reexports
file; and move CLI tests (test_cli_default_invocation_accepts_current_package,
test_cli_none_argv_accepts_current_package,
test_cli_fixture_policy_reports_fixture_violations,
test_cli_fixture_policy_switches_from_default_policy) into the CLI file.
Preserve the original imports (check_architecture, fixture_policy,
relative_import_base, architecture_main, build_reexport_index,
_explicit_all_exports) and FIXTURE_ROOT variable where needed, update
module-level fixtures/parametrize decorators to their new files, and run pytest
to confirm no import paths or name collisions remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca0dea17-7bea-4e57-a561-55e3d9c966c8
📒 Files selected for processing (5)
beatcue/architecture/checker.pybeatcue/architecture/policy.pybeatcue/architecture/reexports.pytests/fixtures/architecture/application_imports_star_reexported_adapter/adapters/__init__.pytests/test_architecture_enforcement.py
Separate the architecture enforcement tests into checker, CLI, and re-export modules so each file stays under the 400-line policy while preserving the same coverage. Keep the checker tests focused on policy and import-resolution behaviour, move CLI entrypoint tests into their own module, and isolate re-export resolver coverage in a dedicated file.
Add exact command-line diagnostics, public `python -m` entrypoint coverage, and invalid-root handling for the architecture checker CLI. Add Hypothesis-based invariant tests for module-name computation, relative-import bases, policy classification, and deterministic re-export indexing. Mark the completed roadmap fitness-function item so the roadmap matches the implemented gate.
Remove inbound adapters from the generic adapter permission set so shared adapter modules cannot depend on inbound adapters. Allow outbound adapters to import shared adapter utilities through their dedicated policy set.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/execplans/import-hex-architecture-enforcement.md`:
- Around line 601-603: Update the Stage C inbound permission list so that the
`inbound_adapter` allowed imports include `composition_root` in addition to
`domain`, `application`, and `inbound_adapter`; locate the rule block that
currently enumerates allowed imports for `inbound_adapter` (mentions `domain`,
`application`, and `inbound_adapter`) and add `composition_root` to the list to
match the later decision log and fixtures.
In `@docs/roadmap.md`:
- Around line 68-70: The roadmap shows task 1.2.2 checked while still declaring
"Requires 1.2.1" which is unchecked; update the document so dependency
sequencing is consistent by either marking task 1.2.1 as complete (check the box
for 1.2.1) if it is done, or remove/adjust the "Requires 1.2.1" line in task
1.2.2 to reflect the true prerequisite (e.g., change or remove the Requires
statement), making sure references to "1.2.2" and "1.2.1" in the roadmap remain
accurate and use dotted notation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7cc79347-0001-439e-9374-7b22e6590ffc
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
beatcue/architecture/cli.pybeatcue/architecture/policy.pydocs/execplans/import-hex-architecture-enforcement.mddocs/roadmap.mdpyproject.tomltests/test_architecture_checker.pytests/test_architecture_cli.pytests/test_architecture_reexports.py
Update the ExecPlan's inbound adapter rule to include composition-root imports, matching the implemented policy and fixture coverage. Clarify that the roadmap fitness-function task can precede the package skeleton because it uses fixture packages for the future boundaries.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/execplans/import-hex-architecture-enforcement.md`:
- Line 1: Change the level-1 heading from Title Case to sentence case: replace
"# Import Hexagonal Architecture Enforcement" with "# Import hexagonal
architecture enforcement" so it follows the documentation style guide; ensure
the new heading text appears exactly as "Import hexagonal architecture
enforcement" in the file and keep the rest of the document unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 564f5203-f714-48ce-95fd-0439cc613bcf
📒 Files selected for processing (2)
docs/execplans/import-hex-architecture-enforcement.mddocs/roadmap.md
Use sentence case for the architecture enforcement ExecPlan title to match the documentation style guide.
This was referenced May 18, 2026
This was referenced Jul 25, 2026
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
1 participant
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
This branch imports BeatCue's local hexagonal architecture enforcement trial from the Episodic mechanism and adapts it to BeatCue's package names, composition-root exception, and infrastructure boundaries.
Execplan: docs/execplans/import-hex-architecture-enforcement.md
It adds an
ast-based checker that expands package-barrel re-exports, verifies the currentbeatcue/skeleton, and uses fixture packages to prove the intended future domain, application, adapter, and config boundaries. It also wires the architecture gate intomake lint, records the accepted decision in ADR 003, and completes the ExecPlan retrospective with prior-art and extraction notes.Review walkthrough
Validation
make check-fmt 2>&1 | tee /tmp/check-fmt-beatcue-import-hex-architecture-enforcement.out: passed.make lint 2>&1 | tee /tmp/lint-beatcue-import-hex-architecture-enforcement.out: passed; Ruff ran first, thenmake check-architecture.make typecheck 2>&1 | tee /tmp/typecheck-beatcue-import-hex-architecture-enforcement.out: passed.make test 2>&1 | tee /tmp/test-beatcue-import-hex-architecture-enforcement.out: passed,11 passed.make markdownlint 2>&1 | tee /tmp/markdownlint-beatcue-import-hex-architecture-enforcement.out: passed.make nixie 2>&1 | tee /tmp/nixie-beatcue-import-hex-architecture-enforcement.out: passed.coderabbit review --agent: completed; valid Oxford-comma finding was fixed, and stale blank-line/spelling findings were verified against the current files and recorded in the ExecPlan.Notes
The implementation exceeded the original 700-line production tolerance during the checker import. The ExecPlan records the escalation and the later user direction to proceed. No runtime dependency was added.
The main follow-up is to extract the reusable checker core into a small df12/internal tool with a TOML or JSON policy schema before trying the mechanism in Prosidy Darn.
Summary by Sourcery
Introduce a repository-local hexagonal architecture checker, wire it into the lint workflow, and document the enforced import boundaries and workflow for contributors.
Enhancements:
Build:
Documentation:
Tests: