Design Patterns Review Report¶

Project: zolletta-metaskill
Language: Python
Scan Date: 2026-08-06
Generated By: zolletta-metaskill patterns

Grade¶

Score: 92/100

Justification: The project demonstrates excellent design pattern compliance. All 15 top class-metrics candidates are cohesive single-domain classes (static helper collections, parsers, scanners, orchestrators) — none are God classes. No inheritance hierarchies exist (only a Protocol), confirming composition-over-inheritance. Scanners depend on the LanguageEngine protocol, not concretions (DIP satisfied). No LSP violations. All 7 OCP scanner hits are false positives (AST/tree-sitter node-type dispatch in parsers). The ISP hit is a false positive (6-method protocol with zero stubbed implementers). Test structure mirrors source perfectly (0 misnamed, 0 misplaced, 0 missing). Coverage is 99% (1532 tests pass). ADR directives ADR-0004 (stdlib-only scanners) and ADR-0007 (language-neutral engine protocol) are satisfied. Points lost: -4 for the confusingly named test_glob_pattern_returns_glob protocol method (KISS violation — test_ prefix implies a test method, _returns_glob leaks implementation detail), -4 for the ADROrchestrator.refresh() method being ~120 lines with inline cache-classification logic that could be extracted to a helper (minor SRP/SoC observation).

Scanning Script Results¶

Class Metrics (top 15 candidates)¶

Class	File	Lines	Methods	Pub	Attrs	Verdict
DocStalenessScorer	documentor/doc_staleness_scorer.py	978	22	14	0	Cohesive — static helpers, single domain (doc staleness scoring)
DriftAnalyzer	documentor/drift_analyzer.py	843	16	13	0	Cohesive — static helpers, single domain (doc drift analysis)
APIDocValidator	documentor/api_doc_validator.py	737	11	7	0	Cohesive — static helpers, single domain (API doc validation)
DocstringStreamliner	code_style/python/docstring_streamliner.py	725	26	11	0	Cohesive — static helpers, single domain (docstring streamlining)
PHPEngine	core/engine/php_engine.py	513	26	11	2	Cohesive — parser/adapter, single domain (PHP parsing)
LinkChecker	documentor/link_checker.py	508	12	10	0	Cohesive — static helpers, single domain (link checking)
TestStructureScanner	testing_style/general/test_structure_scanner.py	462	7	1	0	Cohesive — static helpers, single domain (test structure scanning)
DependencyInversionScanner	patterns/php/dependency_inversion_scanner.py	331	12	3	0	Cohesive — scanner, single domain (PHP DIP detection)
AcronymCasingScanner	code_style/php/acronym_casing_scanner.py	313	6	1	0	Cohesive — scanner, single domain (PHP acronym casing)
DependencyInversionScanner	patterns/python/dependency_inversion_scanner.py	308	9	1	0	Cohesive — scanner, single domain (Python DIP detection)
TestSplitter	patterns/python/test_splitter.py	303	11	1	0	Cohesive — utility, single domain (test class splitting)
LiskovSubstitutionScanner	patterns/general/liskov_substitution_scanner.py	295	6	3	0	Cohesive — scanner, single domain (LSP detection)
PythonEngine	core/engine/python_engine.py	280	12	6	0	Cohesive — parser/adapter, single domain (Python parsing)
NamingConventionsScanner	code_style/general/naming_conventions_scanner.py	264	8	1	0	Cohesive — scanner, single domain (naming conventions)
AcronymCasingScanner	code_style/python/acronym_casing_scanner.py	247	5	1	0	Cohesive — scanner, single domain (Python acronym casing)

"Reason to change" test applied to all 15 candidates. Every class has methods that serve a single domain:

DocStalenessScorer: all changes stem from the documentation staleness scoring domain (scoring dimensions, git helpers, report generation). Single reason to change: the scoring algorithm changes.
DriftAnalyzer: all changes stem from the documentation drift analysis domain (git history, doc-to-code mapping, drift detection, report generation). Single reason to change: the drift detection logic changes.
APIDocValidator: all changes stem from the API documentation validation domain (AST extraction, doc parsing, validation, classification, reporting). Single reason to change: the validation rules change.
DocstringStreamliner: all changes stem from the docstring streamlining domain (parsing, analysis, rendering, file processing). Single reason to change: the streamlining rules change.
PHPEngine: all changes stem from the PHP parsing domain (tree-sitter AST walking, node extraction). Single reason to change: the PHP grammar or ModuleInfo mapping changes.
LinkChecker: all changes stem from the link checking domain (extraction, validation, duplicate detection, reporting). Single reason to change: the link validation logic changes.
TestStructureScanner: all changes stem from the test structure scanning domain (source indexing, test matching, reporting). Single reason to change: the matching algorithm changes.
All scanner classes: single domain (their respective SOLID principle or convention check).

All 15 classes are suppressed per "What is NOT a God class": static helper collections serving one domain, parsers, and scanners.

Structural Conventions¶

Check	Status	Details
One class per file	PASS	0 files with 2+ classes. 10 "class name != filename" hits are false positives — the scanner converts snake_case to PascalCase without accounting for acronyms (ADR, PHP, API). Confirmed by `acronym_casing_scanner.py`: 0 violations. Class names `ADRDiscovery`, `PHPEngine`, `APIDocValidator` etc. are correct per the acronym casing convention.
Test structure mirrors source	PASS	0 misnamed tests, 0 misplaced tests, 0 orphaned tests (excluding `.pytest_cache/` cache dirs), 0 missing tests. 6 indirect references (informative only).

Test Structure Scanner Output¶

Source package: src/zolletta_metaskill Test package: tests

1. Misnamed tests (0)¶

None — all test file names match their source stem or class name.

2. Misplaced tests (0)¶

None — all test files are in the correct mirrored directory.

3. Orphaned tests (0)¶

None — all test files and directories match a source counterpart.

Note: the scanner initially reported 3 orphaned directories (.pytest_cache/, .pytest_cache/v/, .pytest_cache/v/cache/). These are pytest cache artifacts, not test files. Suppressed with --ignore-dirs .pytest_cache.

4. Missing tests (0)¶

None — all source files with classes have direct or indirect tests.

Coverage cross-check: pytest --cov reports 99% total coverage (14688 statements, 20 missed, 1532 tests pass). No files in the "Missing tests" table — no coverage cross-check needed.

5. Indirect references (6) — informative only¶

Test file	Primary source	Indirectly tested sources	Indirectly tested classes
`code_style/python/test_docstring_streamliner.py`	`code_style/python/docstring_streamliner.py`	`code_style/python/structs/file_report.py`	FileReport
`core/engine/test_engine_registry.py`	`core/engine/engine_registry.py`	`core/engine/language_engine.py`	LanguageEngine
`core/engine/test_php_engine.py`	`core/engine/php_engine.py`	`core/engine/language_engine.py`	LanguageEngine
`core/engine/test_python_engine.py`	`core/engine/python_engine.py`	`core/engine/language_engine.py`	LanguageEngine
`documentor/test_api_doc_validator.py`	`documentor/api_doc_validator.py`	`documentor/structs/source_signature.py`	SourceSignature
`documentor/test_link_checker.py`	`documentor/link_checker.py`	`documentor/structs/link_info.py`	LinkInfo

SOLID Scanner Results¶

Scanner	Findings	Verdict
DIP (dependency_inversion_scanner.py)	2	Both suppressed — PHPEngine lazy-initializes a tree-sitter `Parser` (low-level parsing primitive, not a business collaborator). This is an internal implementation detail of the engine adapter, not a DIP violation. Injecting the Parser would be over-engineering (KISS).
ISP (interface_segregation_scanner.py)	1	Suppressed — `LanguageEngine` protocol has 6 methods but zero implementers stub any method. Both `PythonEngine` and `PHPEngine` fully implement all 6 methods. The protocol is cohesive (all methods serve language parsing/identification). Not an ISP violation.
OCP (open_closed_scanner.py)	7	All 7 suppressed — every hit is `isinstance` dispatch on `ast` node types (`ast.ClassDef`, `ast.FunctionDef`, `ast.Name`, etc.) or tree-sitter node types (`namespace_name`, `qualified_name`) inside parser code. This is the standard and correct way to traverse heterogeneous AST nodes, not type-based business logic dispatch. Not OCP violations.
LSP (liskov_substitution_scanner.py)	0	All clear.

Test God Classes Scanner¶

15 large test classes identified. All test a single SUT (e.g., TestMain tests one scanner's main() function, TestScanFile tests one scanner's scan_file() method). Per "When NOT to split": all test methods target the same SUT — the classes are large but not God classes. No splitting needed.

Coverage¶

Metric	Value
Total statements	14688
Missed	20
Coverage	99%
Tests passed	1532

ADR Directive Alignment¶

Directive	Status	Notes
ADR-0004: Python stdlib only for scanners	PASS	All scanner imports are stdlib (`argparse`, `ast`, `sys`, `pathlib`, `typing`, `json`, `re`) plus internal project imports. No external dependencies.
ADR-0007: Language-neutral engine protocol	PASS	`LanguageEngine` Protocol implemented by `PythonEngine` and `PHPEngine`. Scanners depend only on the protocol and `ModuleInfo` data model, never importing `ast` or tree-sitter directly.

Findings¶

Critical¶

None.

High¶

None.

Medium¶

None.

Low¶

#	File	Class/Symbol	Issue	Principle	Suggested Fix
1	src/zolletta_metaskill/core/engine/language_engine.py:55	LanguageEngine.test_glob_pattern_returns_glob	Method name is confusing: `test_` prefix implies a test method, `_returns_glob` leaks implementation detail into the name. Documentation (scripts.md) refers to this as `test_file_pattern()`.	KISS	Rename to `test_file_pattern` or `test_glob_pattern`.

Observations (not findings)¶

#	File	Observation
1	src/zolletta_metaskill/adr/adr_orchestrator.py:48-166	`ADROrchestrator.refresh()` is ~120 lines with inline cache-classification and directive-merging logic. The method is cohesive (single domain: ADR refresh flow) and delegates to `ADRDiscovery`, `ADRCache`, `ADRDistiller`, but the inline classification/merge logic could be extracted to a private helper for readability. Not a God class method — it reads like a procedure.
2	src/zolletta_metaskill/adr/adr_orchestrator.py:69	`ADROrchestrator.refresh()` creates `ADRCache(self.cache_path)` internally. `ADRCache` is a simple file-based JSON cache (value object / data access helper), not an external service. Per DIP exceptions, value objects with no external service calls are not violations. Noted for completeness.
3	src/zolletta_metaskill/core/engine/php_engine.py:212-228	`PHPEngine._get_parser()` creates `Parser()` internally (lazy initialization). The DIP scanner flagged this, but `PHPEngine` is a leaf-level adapter that wraps tree-sitter. The `Parser` is a low-level parsing primitive, not a business collaborator. Injecting it would be over-engineering. Suppressed: internal implementation detail of an engine adapter.

Recommendations¶

Rename LanguageEngine.test_glob_pattern_returns_glob to test_file_pattern or test_glob_pattern — the current name is confusing and leaks implementation detail.
Consider extracting the cache-classification and directive-merge logic from ADROrchestrator.refresh() into a private helper method to improve readability (optional — the method is cohesive and correct as-is).

Generated by zolletta-metaskill patterns