009 constitution compliance#18
Merged
Merged
Conversation
…itution to v1.1.0 Sets the design baseline before implementation. Full spec/plan/tasks for the constitution-compliance refactor (108 tasks across Setup, Foundational, US1-US5, and Polish), plus an amendment to the constitution that tightens handler limits (CLI handler functions ≤ 50 effective lines, MCP tool handler functions ≤ 30, per-function rather than per-file) and adds file-size budgets for the core domain (use-case files ≤ 200, entity files ≤ 300). The KNOWN_VIOLATIONS per-file allowlist is retired in favour of categorical exemptions encoded in specs/009-constitution-compliance/contracts/structural-rules.yaml.
Sets up the empty `tools/archcheck/` directory, ADR-0009 explaining the toolchain decisions, Makefile targets `audit-constitution` and `audit-constitution-watch`, and a `depguard`-enabled `.golangci.yml`. The audit binary itself lands in the next phase (T007-T023); for now `make audit-constitution` errors with "no Go files in tools/archcheck" which is the expected pre-implementation state. Build still green.
…T007-T023) Implements the structural-compliance audit binary at tools/archcheck/. Three check engines share one declarative YAML rule set: - file-size (per-file effective-line budget) - function-size (per-function effective-line budget via go/ast walk) - layer-import (allow/forbid lists with first-match-wins pathPattern) The effective-line counter mirrors scripts/audit-constitution.sh exactly so whole-file counts agree during the migration window. Output is contract- compliant per ci-step.contract.md: text + JSON + GitHub annotation modes, deterministic ordering, exit codes 0/1/2. Includes 7 unit-test files plus an end-to-end test that runs the built binary against testdata/fixture/ (3 intentional violations + 1 clean file). All tests pass; go vet clean. structural-rules.yaml fix: each layer's allowedImports now includes itself (intra-layer imports are unconditionally allowed). The constitution prose already implied this; the YAML rule encoding needed to be explicit. This eliminates 5 false-positive layer violations on the live codebase. Live baseline captured to specs/009-constitution-compliance/baseline- violations.json: 37 violations across 214 files (6 file-size, 27 function- size, 4 layer). The 4 layer violations are exactly the cmd files importing entities directly (cmd/build.go, cmd/init.go, cmd/new.go, cmd/validate.go); the file-size set matches the plan's named oversized files (graph.go, build_docs.go, query_architecture.go, build_architecture_graph.go, scaffold_entity.go, validate_architecture.go). Note: ports.go (426 raw lines) does NOT violate the 200 effective-line budget — comment density brings it under, so US4 task T077-T084 is not needed.
…0 (US1, T024-T037)
Split internal/core/usecases/scaffold_entity.go (364 lines, file-size violation)
into one file per entity type:
- scaffold.go — ScaffoldEntity struct, options, NewScaffoldEntity, Execute
dispatcher, renderTemplates, plus a new
SelectComponentTemplateName helper that lets cmd/new.go
drop its direct entity-package import
- scaffold_project.go — placeholder; documents that project-scaffolding is
handled inline by Execute (no dedicated method exists)
- scaffold_system.go — scaffoldSystem method
- scaffold_container.go — scaffoldContainer method
- scaffold_component.go — scaffoldComponent method
Tests migrated 1:1 from scaffold_entity_test.go to per-file partners
(scaffold_test.go, scaffold_system_test.go, etc.). No assertions weakened,
no tests removed.
cmd/new.go (was: Execute@83 52>50 effective lines, plus direct entity import):
- Removed `internal/core/entities` import (FR-008).
- Extracted `resolveTemplateName()` and `printResult()` helpers; `Execute` is
now under the 50-line budget.
- componentTemplateName() delegates to usecases.SelectComponentTemplateName.
cmd/build.go (was: direct entity import):
- Removed `internal/core/entities` import; setupTemplateEngine now takes
templateName string. renderMarkdown was inlined into Execute (small enough
not to warrant its own method).
internal/core/usecases/build_docs.go: top-of-file comment added (T034)
documenting the public BuildDocs entry point. Logic unchanged. Full file
split deferred to US4 (T062-T066).
External behaviour preserved: `loko new` and `loko build` produce identical
files, console output, and exit codes (smoke evidence in
specs/009-constitution-compliance/manual-smoke.md).
Audit: 37 → 33 violations.
- Resolved: cmd/new.go function-size, cmd/new.go layer, cmd/build.go layer,
scaffold_entity.go file-size (it's gone).
- Remaining for later phases: cmd/init.go + cmd/validate.go layer (US2),
cmd/watch.go function-size (out of MVP), 25 MCP function-size (US3),
5 core file-size (US4).
`go build ./...` clean; `go test ./...` all pass; `go vet ./...` clean.
- tools/archcheck/reporter.go: ignore fmt.Fprint{ln,f} return values explicitly
(errcheck) — these writes go to stderr/file where a partial-write error has
no recovery path
- tools/archcheck/main.go: wrap deferred f.Close() in a func that discards
the error (errcheck) — go.mod read is fail-closed; close error on a
read-only file is uninteresting
- 7 source files: replace `sb.WriteString(fmt.Sprintf(...))` with
`fmt.Fprintf(&sb, ...)` per staticcheck QF1012. Mechanical sweep across:
internal/core/entities/errors.go
internal/adapters/html/markdown_renderer.go
internal/adapters/d2/generator.go
internal/adapters/markdown/builder.go
internal/adapters/filesystem/project_repo.go
internal/adapters/filesystem/config.go
internal/core/usecases/build_docs.go
internal/core/usecases/enhance_component_diagram.go
internal/core/usecases/query_architecture.go
`task lint` now reports 0 issues. All 22 test packages still green;
go vet clean. No behavioural change.
…38-T045)
T038/T040: .golangci.yml gains a single focused depguard rule (cmd/ MUST NOT
import internal/core/entities) — the highest-leverage layer regression for
IDE-time and `task lint` feedback. Full layer enforcement remains
authoritative in tools/archcheck. A sync smoke test in
tools/archcheck/scenarios_test.go fails if the depguard rule disappears or
stops referencing the entities package path.
T039: --emit-golangci-config flag was already implemented in Phase 2.
T041-T043: cmd/init.go and cmd/validate.go no longer import
internal/core/entities directly (FR-008 cmd-only narrowing under v1.1.0).
- cmd/init.go: replaced inline entities.ValidateName + entities.NewProject
+ os.MkdirAll + repo.SaveProject orchestration with a call to a new
InitProject use case (internal/core/usecases/init_project.go). The
handler is now 11 effective lines.
- cmd/validate.go: dropped the []*entities.System parameter on
executeDriftCheck (the use case loads systems via the repository when
Systems is left nil in DetectDriftRequest). Replaced
entities.DriftError/DriftWarning severity comparisons with two new
pre-filtered slices on DetectDriftResult: Errors []DriftIssueView and
Warnings []DriftIssueView. The view type carries only ComponentID,
Message, Context — outer layers no longer need the entities package or
the DriftSeverity constants.
- internal/core/usecases/init_project.go: new InitProject use case with a
full table-driven test partner (init_project_test.go) covering nil
request, empty name, invalid name, default-path-from-name, and
happy-path SaveProject calling.
- internal/core/usecases/detect_drift.go: filtered Errors/Warnings
slices populated alongside the existing HasErrors/HasWarnings booleans.
T044: `task lint` reports 0 issues; `make audit-constitution` reports
31 violations (was 33; the 2 cmd-layer violations are gone).
T045: tools/archcheck/scenarios_test.go drives every spec US2 acceptance
scenario (AC#1-4) through the layer-import checker as table-driven cases.
Verifies that under v1.1.0 mcp and api MAY still import entities directly
while cmd MUST NOT.
External behaviour preserved: cmd/init.go and cmd/validate.go produce the
same files, console output, and exit codes as before.
Splits internal/mcp/tools/graph_tools.go (495 lines, 4 oversized handlers) into three single-responsibility files: - query_dependencies.go (QueryDependenciesTool + helpers) - query_related_components.go (QueryRelatedComponentsTool + helpers) - analyze_coupling.go (AnalyzeCouplingTool + helpers) graph_tools.go is deleted; tests migrated alongside. Inline InputSchema declarations (5 files: create_system, update_system, create_component, create_container, update_container) are moved to package-level vars in helpers.go, following the schemas.go-as-data-file pattern. Tool handlers' InputSchema() methods now return the var directly (~3 effective lines). build_docs.go, update_diagram.go, validate.go gain entries in the existing Schemas map for schema centralization. Every Call() method that exceeded the 30-line budget is split into smaller helpers — input parsing, domain-step extraction, output shaping. Helpers live in the same tool file or, for shared concerns (e.g., diagramMessageFor), in helpers.go (which is exempt from function-size budgets). Updated structural-rules.yaml: helpers.go, schemas.go, registry.go, constants.go, and *_cobra.go are now exempt from function-size budgets in addition to the existing file-size exemption. Rationale: data and shared- helper files are not protocol/CLI handlers, so FR-002's per-handler budget does not apply by intent. This drops 2 helpers.go violations (validateDiagramStructure 34>30, scaffoldOneComponent 32>30) that the agent's Call-extraction strategy would otherwise force into the wrong file. Audit progression: 31 → 29 (after exemption) → 6 violations. The remaining 6 are exactly US4's scope: - 5 core file-size (graph.go 377, build_architecture_graph.go 322, build_docs.go 334, query_architecture.go 397, validate_architecture.go 278) - 1 cmd function-size (cmd/watch.go Execute 75>50, deferred from US1 since it's not in cmd/new.go or cmd/build.go) External behaviour preserved: every MCP tool's Name(), Description(), InputSchema() return value, and Call() JSON response shape are byte-equivalent to before. All existing MCP tool tests still pass with no assertions weakened or tests removed. `task lint` 0 issues. `make audit-constitution` 6 violations.
- Improved regex patterns for handling backtick strings, package lines, imports, and comments in lines.go - Added test cases for: - Backtick string not treated as comment (mirrors shell awk) - Package line with qualifier dropped - Renamed struct fields in types.go: - PathPattern → pathPattern - MaxEffectiveLines → maxEffectiveLines - Updated exemption rules and line counting logic in lines_test.go
There was a problem hiding this comment.
Pull request overview
This PR introduces a build-time “constitution compliance” audit and refactors several CLI/MCP/usecase components to better align with Clean Architecture boundaries (particularly avoiding direct entities imports from outer layers). It also adds feature-009 documentation and contracts describing CI behavior and rule definitions.
Changes:
- Added
tools/archcheck(Go AST audit tool) + fixture-based end-to-end tests and YAML rule schema to enforce layer-import, file-size, and function-size budgets. - Added feature-009 contracts/docs (structural rules YAML, CI step contract, quickstart + smoke notes) and Makefile targets for running the audit locally.
- Refactored CLI/MCP tooling and core usecases/entities to reduce cross-layer coupling (new scaffolding/init use cases, moved graph helpers, schema centralization).
Reviewed changes
Copilot reviewed 102 out of 106 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/archcheck/types.go | Defines the YAML/JSON data model used by the archcheck tool. |
| tools/archcheck/rules_loader.go | Loads + validates the structural-rules YAML file. |
| tools/archcheck/reporter.go | Text/JSON reporting and deterministic sorting for violations. |
| tools/archcheck/reporter_test.go | Unit tests for report ordering, JSON output, annotations. |
| tools/archcheck/lines.go | Effective-line counting to match legacy shell audit rules. |
| tools/archcheck/lines_test.go | Test coverage for effective-line counting behavior. |
| tools/archcheck/layer.go | Layer-import rule enforcement based on YAML patterns. |
| tools/archcheck/glob.go | Glob-to-regexp matcher for rule path patterns. |
| tools/archcheck/glob_test.go | Tests for glob matching across key patterns. |
| tools/archcheck/funcsize.go | Function-size enforcement using effective-line counting. |
| tools/archcheck/filesize.go | File-size enforcement using effective-line counting. |
| tools/archcheck/filesize_test.go | Tests for file-size rule enforcement and exemptions. |
| tools/archcheck/archcheck_test.go | End-to-end fixture build/run test asserting violations/reporting. |
| tools/archcheck/testdata/fixture/rules.yaml | Fixture rules used for end-to-end tool validation. |
| tools/archcheck/testdata/fixture/go.mod | Fixture module definition for end-to-end tool runs. |
| tools/archcheck/testdata/fixture/clean.go | Fixture “clean” file with no violations. |
| tools/archcheck/testdata/fixture/cmd/oversized.go | Fixture function-size violation case. |
| tools/archcheck/testdata/fixture/internal/core/entities/big.go | Fixture file-size violation case. |
| tools/archcheck/testdata/fixture/internal/core/usecases/bad_import.go | Fixture layer-import violation case. |
| specs/009-constitution-compliance/contracts/structural-rules.yaml | Canonical YAML rule set for layer/file/function budgets and exemptions. |
| specs/009-constitution-compliance/contracts/ci-step.contract.md | Contract for CI gating behavior/output and determinism guarantees. |
| specs/009-constitution-compliance/quickstart.md | Developer quickstart for running/understanding the audit. |
| specs/009-constitution-compliance/mcp-smoke.md | Notes on MCP smoke testing evidence and gaps. |
| specs/009-constitution-compliance/manual-smoke.md | Manual CLI smoke test notes for feature 009 tasks. |
| specs/009-constitution-compliance/checklists/requirements.md | Requirements/spec quality checklist for feature 009. |
| docs/adr/0009-constitution-compliance-tooling.md | ADR documenting rationale/decisions for the audit tooling approach. |
| Makefile | Adds audit-constitution and watch targets. |
| .golangci.yml | Enables depguard and adds rule text for cmd/entities boundary. |
| .gitignore | Adds additional ignored agent/docs paths. |
| go.mod | Adds direct dependencies for YAML parsing and D2. |
| cmd/init.go | Refactors project init to use a core use case rather than entities directly. |
| cmd/new.go | Refactors template selection + output formatting; routes tech-template selection through usecases. |
| cmd/build.go | Adjusts template engine setup signature and markdown rendering invocation. |
| cmd/validate.go | Uses drift-result view types to avoid cmd importing entities directly. |
| internal/mcp/tools/helpers.go | Centralizes tool schemas and adds diagram messaging helper. |
| internal/mcp/tools/validate.go | Moves schema to centralized Schemas and refactors graph-building helper. |
| internal/mcp/tools/validate_diagram.go | Refactors validation flow; extracts syntax-check helper. |
| internal/mcp/tools/build_docs.go | Moves schema to centralized Schemas and factors project/system loading/build steps. |
| internal/mcp/tools/update_system.go | Factors out load/apply/save operations; uses shared schema. |
| internal/mcp/tools/update_container.go | Factors out load/apply/save operations; uses shared schema. |
| internal/mcp/tools/update_component.go | Factors out load/apply/save operations; uses shared schema. |
| internal/mcp/tools/update_diagram.go | Uses shared schema and factors diagram-path resolution/response building. |
| internal/mcp/tools/create_container.go | Uses shared schema and factors scaffolding execution. |
| internal/mcp/tools/create_components.go | Refactors multi-component scaffolding into helper. |
| internal/mcp/tools/create_relationship.go | Refactors argument validation/system validation and cache invalidation. |
| internal/mcp/tools/delete_relationship.go | Refactors system resolution and response building. |
| internal/mcp/tools/list_relationships.go | Refactors system resolution and response building. |
| internal/mcp/tools/query_related_components.go | Adds new MCP tool to query dependencies/dependents for a component. |
| internal/mcp/tools/analyze_coupling.go | Adds new MCP tool to compute coupling metrics via graph analysis. |
| internal/core/usecases/init_project.go | Introduces a dedicated init-project use case. |
| internal/core/usecases/init_project_test.go | Adds unit tests for init-project use case behavior. |
| internal/core/usecases/scaffold.go | Introduces ScaffoldEntity use case and SelectComponentTemplateName. |
| internal/core/usecases/scaffold_project.go | Documentation-only file clarifying scaffold scope (no project entity). |
| internal/core/usecases/scaffold_system.go | Implements system scaffolding behavior inside ScaffoldEntity. |
| internal/core/usecases/scaffold_system_test.go | Tests system scaffolding via the ScaffoldEntity dispatcher. |
| internal/core/usecases/scaffold_container.go | Implements container scaffolding behavior inside ScaffoldEntity. |
| internal/core/usecases/scaffold_container_test.go | Tests container scaffolding via the ScaffoldEntity dispatcher. |
| internal/core/usecases/scaffold_component.go | Implements component scaffolding behavior inside ScaffoldEntity. |
| internal/core/usecases/scaffold_component_test.go | Tests component scaffolding via the ScaffoldEntity dispatcher. |
| internal/core/usecases/validate_structural.go | Splits structural validation checks into dedicated file. |
| internal/core/usecases/validate_circular.go | Splits circular dependency detection into dedicated file. |
| internal/core/usecases/validate_architecture.go | Removes inlined helper implementations after splitting to new files. |
| internal/core/usecases/query_architecture_responses.go | Adds response-building variants for QueryArchitecture by detail level. |
| internal/core/usecases/detect_drift.go | Adds view types + pre-filtered Errors/Warnings to avoid outer-layer entities usage. |
| internal/core/usecases/build_docs_tables.go | Adds markdown table generators for docs building. |
| internal/core/usecases/build_docs_diagrams.go | Adds parallel diagram rendering pipeline for docs building. |
| internal/core/usecases/build_architecture_graph.go | Extracts relationship parsing/query helpers into dedicated files. |
| internal/core/usecases/architecture_graph_relationships.go | Adds relationship-edge construction helpers for graph building. |
| internal/core/usecases/architecture_graph_queries.go | Adds query helpers (system subgraph, dependency analysis) for the graph builder. |
| internal/core/usecases/enhance_component_diagram.go | Refactors string building to use fmt.Fprintf for efficiency/readability. |
| internal/core/entities/graph.go | Moves traversal/id helpers out to dedicated files (smaller core file). |
| internal/core/entities/graph_traversal.go | Adds traversal/dependency/path helpers for ArchitectureGraph. |
| internal/core/entities/graph_traversal_test.go | Adds tests covering traversal/dependency/path APIs. |
| internal/core/entities/graph_id.go | Adds qualified-ID helpers (build/parse) for graph nodes. |
| internal/core/entities/errors.go | Refactors to fmt.Fprintf for string building. |
| internal/adapters/filesystem/config.go | Refactors to fmt.Fprintf for TOML generation. |
| internal/adapters/markdown/builder.go | Refactors to fmt.Fprintf for markdown generation. |
| internal/adapters/html/markdown_renderer.go | Refactors to fmt.Fprintf for HTML building. |
| internal/api/handlers/dist/js/main.js | Adds client-side JS for docs UI behaviors (search/scroll/navigation). |
| AGENTS.md | Removes agent guidance file (now gitignored). |
Agent-Logs-Url: https://github.com/madstone-tech/loko/sessions/93a688b2-0713-4235-bf95-3d8e011bd622 Co-authored-by: andhijeannot <13513173+andhijeannot@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The commit adds a new function `graphNodesToMapList` to the `helpers.go` file, which converts a slice of graph nodes into a JSON-friendly map structure. This function is specifically designed to support the MCP tool handlers, which require efficient node conversion for processing graph data. The change maintains consistency with the existing logic in `query_dependencies.go` and aligns with the project's code structure.
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.
Pull Request
Summary
Related Issues
Closes #
Type of Change
Changes Made
Testing
go test ./...)Checklist
go fmt ./...)golangci-lint run)Additional Notes