Fix Architecture Violations reporting the inverse of reality#64
Merged
braedonsaunders merged 8 commits intoJul 13, 2026
Merged
Conversation
detectLayerViolations reads the documented {source: definition, target: caller}
connection convention backwards, reporting the exact inverse of reality —
healthy downward deps flagged, genuine upward violations missed. Verified
empirically against the real analyzer in both directions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task-by-task plan: direction inversion (primary), detectLayer leading-slash tolerance (secondary), coupling label correction (tertiary), regression pass. Every code change pre-verified against the real analyzer via card/lib/analyzer.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tasks 1+2 interact to introduce a new "test should not import from utils" false-positive class (dir-based test files classify as layer 'test'=6 and, with the direction corrected, get flagged for importing anything more foundational). Excluded via the existing isArchitectureTestFile classifier. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… violations Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (Python, test-folders) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@abk1969 is attempting to deploy a commit to the braedonsaunders' projects Team on Vercel. A member of the Team first needs to authorize it. |
braedonsaunders
approved these changes
Jul 13, 2026
braedonsaunders
left a comment
Owner
There was a problem hiding this comment.
Reviewed the graph convention and the implementation, including the root-level layer normalization and test-importer guard. The full branch suite passes locally (42/42). The only failing status is Vercel contributor authorization, not a code/test failure. Approved for merge.
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
CodeFlow's "Architecture Violations" detector (
Parser.detectLayerViolations) reported the exact inverse of reality: healthy downward dependencies (a service using a foundational util) were flagged as violations, while genuine upward violations (a util reaching into a service) were silently missed. On a real project this surfaced as a wall of false "utils should not import from services" entries — one per caller of each shared module.Root cause
The connection graph uses a documented convention (
index.html, theConnection formatcomment):{source: fileDefiningFn, target: fileCallingFn}—sourceis the file that defines the called function (depended-upon),targetis the file that calls it (the importer).detectLayerViolationsread this backwards, treatingsourceas the importer. Verified against the real analyzer in both directions: a healthyservice → utildependency produced a false positive, and a genuineutil → serviceviolation produced nothing.What changed
detectLayerViolationsnow treatstargetas the importer, matching the documented convention. Healthy downward deps no longer flag; genuine upward violations are correctly reported with accuratefrom/toattribution.isArchitectureTestFile(path) || importerLayer === 'test'. Both classifiers are needed:isArchitectureTestFilecovers*.test/*.specfiles outside atests/dir; thetestlayer covers Pythontest_*.py,conftest.py, andtest*-prefixed folders.detectLayerleading-slash tolerance (secondary): every folder pattern required a leading slash, so root-level layer folders (services/,ui/,lib/) never matched and fell through toutils. Normalized by prepending/to the path. No new folder vocabulary added.Each fix is backed by a test in the new
tests/layer-violation-direction.test.mjs(VM-harness pattern matchingtests/codeflow-golden.test.mjs).Design doc:
docs/superpowers/specs/2026-07-12-layer-violation-direction-design.mdPlan:
docs/superpowers/plans/2026-07-12-layer-violation-direction.mdKnown limitations (out of scope)
connsgraph is built or to the{source, target}convention — other consumers (graph rendering, circular detection) rely on it; only the misreading consumer was fixed.detectLayerfolder vocabulary is unchanged beyond leading-slash tolerance (e.g.next.config.mjswith a dot, or a baredb/folder, still fall through toutils).modules(level 5) importers reaching into a much higher layer remain flagged — niche, plausibly real smells, left as-is rather than expanding the exclusion speculatively.Test plan
node --testfrom repo root — 42/42 pass (all pre-existing tests unchanged + 6 new intests/layer-violation-direction.test.mjs)tests/codeflow-golden.test.mjsandtests/architecture-diagram.test.mjsstay green, unmodified (the repo has no root-level layer folders, so thedetectLayerchange can't shift its own self-scan)git worktree(not just the working directory) to rule out local-state false positives, including the direction fix in both directions and the Python/test-folder exclusion