feat(analyzer): architectural anti-pattern detection (#69)#101
Merged
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
zaebee
force-pushed
the
feat/issue-69-anti-pattern-detection
branch
2 times, most recently
from
aa40828 to
fd15f09
Compare
Adds AnalyzerEngine with three detectors:
- detect_cycles() — iterative Tarjan SCC on IMPORTS edges; each
cycle emitted as CIRCULAR_DEPENDENCY anomaly
with severity scaling by cycle length
- detect_zone_of_pain() — Uncle Bob D-distance metric (afferent/efferent
coupling + abstractness via EXTENDS/metadata);
flags stable-concrete classes (D >= 0.70)
- detect_god_objects() — classes with >= 10 methods AND >= 5 efferent
targets flagged as GOD_OBJECT
New CLI command: cgis analyze [--db graph.db] [--min-severity 0.5]
[--format text|json]
18 unit tests covering Tarjan algorithm correctness, external-node
filtering, threshold boundaries, and the full run() report.
Also adds missing packages to pre-commit mypy additional_dependencies
(rich, typer, tree-sitter*) so the hook has the same dependency set
as the local venv.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace chained startswith() with tuple argument - Extract _pop_scc() helper from _tarjan_step to reduce cognitive complexity from 16 to 9 (limit is 15) - Use pytest.approx() for floating-point equality assertion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
BLOCKER #1: sort adjacency keys in _tarjan_scc for deterministic output order across runs (dict insertion order isn't stable after DB queries). BLOCKER #2: remove EXTENDS-based abstractness heuristic from _get_abstract_fqns. Marking the child of an EXTENDS edge as abstract is wrong (e.g. UserService(Base) is concrete). Only node metadata is_abstract is authoritative. MAJOR #2: add tests for raw_import: filtering and empty adj in Tarjan. BLOCKER #3, MAJOR #1, MINORs: not applied — #3 is a misread (external calls are already counted via fallback), #1 is pedantic, MINORs are edge cases / out-of-scope feature requests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MINOR-1: add inline rationale comments to all threshold constants. MAJOR-2: add explicit empty-graph tests for detect_zone_of_pain and detect_god_objects (run() already covers this via test_run_empty_graph, but focused tests are clearer). MAJOR-4: add self-calls test — methods calling siblings in the same class must NOT inflate efferent coupling (correct behavior confirmed). MAJOR-1: add test documenting that raw_call: targets count as efferent coupling (external calls are real dependencies). Skipped: BLOCKER-1 (annotations already present — hallucination), BLOCKER-2 (FUNCTION→class coupling is out of CLASS-metric scope), BLOCKER-3 (ID collision is edge case, same issue as prev review MINOR), MAJOR-3 (DECLARES alongside CONTAINS is safe fallback), MINORs 2-4. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Clarify _ZONE_OF_PAIN_DISTANCE comment: D > 0.7 AND I <= 0.3 - Add boundary tests: 9-method class not flagged (one below threshold), high-instability class (I≈0.33) not in Zone of Pain Skipped: all three BLOCKERs (two hallucinations + one misclassified), two MAJORs (hallucination + scope creep), MINORs 2-4. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
zaebee
force-pushed
the
feat/issue-69-anti-pattern-detection
branch
from
d21f6c8 to
ccc6569
Compare
Owner
Author
|
/guardian review |
Multiple calls to the same unresolved target count as one efferent entry (set semantics). Validates the 1/4 valid finding from Gemini Flash Lite review. Also restrict pre-commit mypy hook to src/ only — tests/ imports cgis as a local package not available in the hook's isolated virtualenv, matching what make type-check already does. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Contributor
|
LGTM — no defects found in this diff. The implementation is highly robust, clean, and complies fully with both the engineering standards and the semantic ontology. Key areas verified:
|
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
src/cgis/query/anomaly.py— frozen Pydantic models:AnomalyType,ArchitecturalAnomaly,ArchitecturalReportsrc/cgis/query/analyzer.py—AnalyzerEnginewith three detectors:detect_cycles()— iterative Tarjan SCC onIMPORTSedges; severity scales with cycle lengthdetect_zone_of_pain()— Uncle Bob D-distance (I + A - 1); flags stable-concrete classes (D ≥ 0.70, Ca ≥ 3)detect_god_objects()— classes with ≥ 10 methods AND ≥ 5 efferent targetstests/unit/test_analyzer.py— 18 tests covering Tarjan correctness, external-node filtering, threshold boundaries, and fullrun()reportsrc/cgis/cli.py— newcgis analyzecommand with--db,--min-severity,--format text|json.pre-commit-config.yaml— added missing packages to mypyadditional_dependencies(rich, typer, tree-sitter*)Design notes
_ZONE_OF_PAIN_MIN_AFFERENT = 3threshold avoids false positives on low-usage classesNodeNamespace.EXTERNAL) are excluded from all detectorsTest plan
make format && make lint && make type-check && make pytest && make doc-coverage— all green (301 tests)cgis analyze --db graph.dbon a real codebase snapshot--min-severityfiltering works in text and JSON outputCloses #69
🤖 Generated with Claude Code