feat: manifesto-driven pass/fail rule engine#138
Conversation
Generic rule engine that evaluates function-level, file-level, and graph-level rules against the DB and returns pass/fail verdicts. - 9 rules: cognitive, cyclomatic, maxNesting (function), importCount, exportCount, lineCount, fanIn, fanOut (file), noCycles (graph) - Exits with code 1 on any fail-level breach (CI gate) - Exposed via CLI (codegraph manifesto), programmatic API, and MCP tool Impact: 9 functions changed, 7 affected
|
Claude finished @carlos-alm's task —— View job PR Review: Manifesto-Driven Rule EngineI'm conducting a comprehensive review of this PR. Here's my progress: Review Checklist:
Phase 1: Root Cause Analysis ✅Problem Being Solved: This PR introduces a rule engine to provide automated code quality gates for CI/CD pipelines. It addresses the need for configurable, automated checks of code complexity metrics, file metrics, and architectural constraints (like circular dependencies) with pass/fail thresholds. Why This Approach: The approach is well-justified - building on top of the existing complexity metrics and cycle detection features to create a unified rule evaluation system. The design follows good patterns:
Phase 2: Configuration Change Detection ✅No HIGH-RISK changes detected:
Phase 3: Test Integrity Assessment ✅Test quality is EXCELLENT:
No red flags detected - no weakened assertions, no removed validations. Phase 4: Code Quality Review ✅Implementation Quality - VERY GOOD: ✅ Proper SQL Parameter Binding: All queries use proper parameterized statements with ✅ Error Handling: Appropriate try-catch blocks around database operations with graceful fallbacks to empty arrays ✅ Code Organization: Clean separation of concerns with dedicated evaluator functions for each rule level ✅ Configuration Merging: Proper deep-merge logic for rule configuration (lines 64-74) ✅ CLI Integration: Well-integrated CLI command with proper option validation (lines 746-766 in cli.js) ✅ API Design: Clean public API with both data ( Minor Code Quality Notes:
Phase 5: Security and Performance Review ✅Security - EXCELLENT:
Performance - GOOD:
Integration Quality - EXCELLENT:
Minor Suggestions (Non-blocking)
Root Cause AnalysisProblem Being Solved: Adding automated code quality gates with configurable pass/fail thresholds for CI/CD integration. Critical ConcernsNone. This is a well-implemented feature with excellent test coverage and no security or architectural concerns. Final Recommendation
Strengths: Comprehensive testing, proper SQL practices, clean architecture, good error handling, appropriate defaults. This PR adds significant value to the codebase and is ready for merge. |
Greptile SummaryThis PR introduces a comprehensive manifesto-driven rule engine for code health checks, enabling CI/CD gates based on complexity metrics. The implementation includes native Rust complexity computation for JavaScript/TypeScript with WASM fallback, 9 configurable rules spanning function (cognitive/cyclomatic/nesting), file (imports/exports/lines/fan-in/fan-out), and graph levels (cycle detection), plus significant performance optimizations. Key additions:
The implementation is well-architected with clear separation between rule evaluation logic, threshold checking, and output formatting. All native extractors updated to populate complexity field (JS/TS/TSX compute it, others set to None for future implementation). Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
CLI[CLI: codegraph manifesto] --> manifestoFn[manifesto function]
MCP[MCP Tool: manifesto] --> manifestoData[manifestoData function]
API[Programmatic API] --> manifestoData
manifestoFn --> manifestoData
manifestoData --> loadConfig[Load .codegraphrc.json]
loadConfig --> resolveRules[Resolve rule thresholds]
resolveRules --> evalFunc[evaluateFunctionRules]
resolveRules --> evalFile[evaluateFileRules]
resolveRules --> evalGraph[evaluateGraphRules]
evalFunc --> queryDB[(Query function_complexity table)]
evalFile --> queryDB2[(Query node_metrics table)]
evalGraph --> findCycles[findCycles function]
queryDB --> checkThreshold[checkThreshold for each metric]
queryDB2 --> checkThreshold
findCycles --> checkCycles{Cycles detected?}
checkThreshold --> violations[Collect violations array]
checkCycles -->|Yes| violations
checkCycles -->|No| pass[Mark as pass]
violations --> summary[Generate summary stats]
pass --> summary
summary --> output{Output format}
output -->|JSON| jsonOut[JSON to stdout]
output -->|CLI| prettyOut[Pretty table + details]
prettyOut --> exitCode{Any fail violations?}
jsonOut --> exitCode
exitCode -->|Yes| exit1[Exit code 1]
exitCode -->|No| exit0[Exit code 0]
style manifestoData fill:#e1f5ff
style evalFunc fill:#fff4e1
style evalFile fill:#fff4e1
style evalGraph fill:#fff4e1
style violations fill:#ffe1e1
style exit1 fill:#ffcccc
style exit0 fill:#ccffcc
Last reviewed commit: b2e1fc1 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/manifesto.js
Outdated
| if (opts.noTests) { | ||
| rows = rows.filter((r) => !isTestFile(r.file)); | ||
| } |
There was a problem hiding this comment.
redundant test filtering - already filtered in SQL via NO_TEST_SQL on line 130, the isTestFile() filter here is unnecessary
| try { | ||
| rows = db | ||
| .prepare( | ||
| `SELECT n.name, n.kind, n.file, n.line, | ||
| fc.cognitive, fc.cyclomatic, fc.max_nesting | ||
| FROM function_complexity fc | ||
| JOIN nodes n ON fc.node_id = n.id | ||
| ${where}`, | ||
| ) | ||
| .all(...params); | ||
| } catch { | ||
| rows = []; | ||
| } |
There was a problem hiding this comment.
silent error handling may hide schema issues or SQL errors - consider logging the error for debugging
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…ions Port complexity algorithm to Rust (complexity.rs) so the native engine computes cognitive/cyclomatic/maxNesting during extraction, eliminating the expensive WASM re-parse fallback that caused the 1.9→4.8 ms/file regression. Additional optimizations: - Cache line counts during parse (avoids re-reading every file from disk) - Use pre-loaded nodesByNameAndFile maps for extends/implements edges (replaces inline DB queries in the edge-building loop) - Optimize structure cohesion from O(dirs×edges) to O(edges+dirs) via reverse file→dirs index and single-pass edge aggregation Impact: 44 functions changed, 46 affected
|
Claude finished @carlos-alm's task —— View job PR Review: Manifesto-Driven Rule EngineI'm conducting a comprehensive review of this PR following the specified review phases. Here's my progress: Review Checklist:
Phase 1: Root Cause Analysis ✅Problem Being Solved: This PR introduces a rule engine to provide automated code quality gates for CI/CD pipelines. It addresses the need for configurable, automated checks of code complexity metrics, file metrics, and architectural constraints (like circular dependencies) with pass/fail thresholds. Why This Approach: The approach is well-justified - building on top of the existing complexity metrics and cycle detection features to create a unified rule evaluation system. The design follows good patterns:
Phase 2: Configuration Change Detection ✅No HIGH-RISK changes detected:
Phase 3: Test Integrity Assessment ✅Test quality is EXCELLENT:
No red flags detected - no weakened assertions, no removed validations. Phase 4: Code Quality Review ✅Implementation Quality - VERY GOOD: ✅ Proper SQL Parameter Binding: All queries use proper parameterized statements with ✅ Error Handling: Appropriate try-catch blocks around database operations with graceful fallbacks to empty arrays ✅ Code Organization: Clean separation of concerns with dedicated evaluator functions for each rule level ✅ Configuration Merging: Proper deep-merge logic for rule configuration (lines 64-74) ✅ CLI Integration: Well-integrated CLI command with proper option validation (lines 746-766 in cli.js) ✅ API Design: Clean public API with both data ( Minor Code Quality Notes:
Phase 5: Security and Performance Review ✅Security - EXCELLENT:
Performance - GOOD:
Integration Quality - EXCELLENT:
Minor Suggestions (Non-blocking)
Root Cause AnalysisProblem Being Solved: Adding automated code quality gates with configurable pass/fail thresholds for CI/CD integration. Critical ConcernsNone. This is a well-implemented feature with excellent test coverage and no security or architectural concerns. Final Recommendation
Strengths: Comprehensive testing, proper SQL practices, clean architecture, good error handling, appropriate defaults. This PR adds significant value to the codebase and is ready for merge. |
…tool to MCP tests - Add complexity: None to Definition initializers in go, rust, java, csharp, and php extractors (fixes Rust compile errors) - Add 'manifesto' to MCP test ALL_TOOL_NAMES (fixes tool count mismatch) - Log errors in manifesto query catch blocks instead of silencing them - Remove redundant isTestFile filtering already handled by SQL WHERE Impact: 7 functions changed, 7 affected
|
Claude finished @carlos-alm's task —— View job PR Review: Manifesto-Driven Rule Engine I'm conducting a comprehensive review of this PR following the specified review phases. Here's my progress: Review Checklist:
Phase 1: Root Cause Analysis ✅Problem Being Solved: This PR introduces a rule engine to provide automated code quality gates for CI/CD pipelines. It addresses the need for configurable, automated checks of code complexity metrics, file metrics, and architectural constraints (like circular dependencies) with pass/fail thresholds. Why This Approach: The approach is well-justified - building on top of the existing complexity metrics and cycle detection features to create a unified rule evaluation system. The design follows good patterns:
Phase 2: Configuration Change Detection ✅No HIGH-RISK changes detected:
Phase 3: Test Integrity Assessment ✅Test quality is EXCELLENT:
No red flags detected - no weakened assertions, no removed validations. Phase 4: Code Quality Review ✅Implementation Quality - VERY GOOD: ✅ Proper SQL Parameter Binding: All queries use proper parameterized statements with ✅ Error Handling: Appropriate try-catch blocks around database operations with graceful fallbacks to empty arrays ✅ Code Organization: Clean separation of concerns with dedicated evaluator functions for each rule level ✅ Configuration Merging: Proper deep-merge logic for rule configuration (lines 64-74) ✅ CLI Integration: Well-integrated CLI command with proper option validation (lines 746-766 in cli.js) ✅ API Design: Clean public API with both data ( Minor Code Quality Notes:
Phase 5: Security and Performance Review ✅Security - EXCELLENT:
Performance - GOOD:
Integration Quality - EXCELLENT:
Minor Suggestions (Non-blocking)
Root Cause AnalysisProblem Being Solved: Adding automated code quality gates with configurable pass/fail thresholds for CI/CD integration. Critical ConcernsNone. This is a well-implemented feature with excellent test coverage and no security or architectural concerns. Final Recommendation
Strengths: Comprehensive testing, proper SQL practices, clean architecture, good error handling, appropriate defaults. This PR adds significant value to the codebase and is ready for merge. |
Update README, CLAUDE.md, BACKLOG, titan-paradigm, recommended-practices, and CLI/MCP examples to reflect today's merged PRs: complexity metrics (#130/#139), Louvain community detection (#133/#134), and manifesto rule engine (#138). Updates MCP tool count from 21 to 24 (25 in multi-repo), marks backlog items 6/11/21/22 as done, and adds real CLI output examples.
Update README, CLAUDE.md, BACKLOG, titan-paradigm, recommended-practices, and CLI/MCP examples to reflect today's merged PRs: complexity metrics (#130/#139), Louvain community detection (#133/#134), and manifesto rule engine (#138). Updates MCP tool count from 21 to 24 (25 in multi-repo), marks backlog items 6/11/21/22 as done, and adds real CLI output examples.
* fix: strict type validation for threshold values in complexity queries
Replace loose `!= null` checks with `typeof === 'number' && Number.isFinite()`
to prevent `Number("")`, `Number(null)`, and `Number(true)` from silently
coercing into valid SQL values. Add integration test verifying exceeds
arrays and summary.aboveWarn are correctly computed.
Addresses Greptile review feedback on #136.
Impact: 2 functions changed, 3 affected
* docs: add complexity, communities, and manifesto to all docs
Update README, CLAUDE.md, BACKLOG, titan-paradigm, recommended-practices,
and CLI/MCP examples to reflect today's merged PRs: complexity metrics
(#130/#139), Louvain community detection (#133/#134), and manifesto rule
engine (#138). Updates MCP tool count from 21 to 24 (25 in multi-repo),
marks backlog items 6/11/21/22 as done, and adds real CLI output examples.
* fix: remove redundant condition in paginate guard clauses
When limit === undefined, limit !== 0 is always true — the && check
was dead code. Simplified to just check limit === undefined.
Impact: 2 functions changed, 18 affected
* docs: update dogfood report with fix statuses
All 4 bugs now fixed (PR #117 merged, #116 closed via reverse-dep
cascade). 3 of 4 suggestions addressed. MCP tool counts updated
18→23 / 19→24. Rating upgraded 7/10 → 9/10 post-fix.
* fix: rename misleading test to match actual behavior
Test was named "handles non-numeric thresholds gracefully" but only
validated baseline exceeds/aboveWarn with valid thresholds. Actual
non-numeric threshold tests exist separately. Renamed to "produces
correct exceeds and aboveWarn with valid thresholds".
* fix: update stale MCP tool count in dogfood skill (21→24)
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix: strict type validation for threshold values in complexity queries
Replace loose `!= null` checks with `typeof === 'number' && Number.isFinite()`
to prevent `Number("")`, `Number(null)`, and `Number(true)` from silently
coercing into valid SQL values. Add integration test verifying exceeds
arrays and summary.aboveWarn are correctly computed.
Addresses Greptile review feedback on #136.
Impact: 2 functions changed, 3 affected
* docs: add complexity, communities, and manifesto to all docs
Update README, CLAUDE.md, BACKLOG, titan-paradigm, recommended-practices,
and CLI/MCP examples to reflect today's merged PRs: complexity metrics
(#130/#139), Louvain community detection (#133/#134), and manifesto rule
engine (#138). Updates MCP tool count from 21 to 24 (25 in multi-repo),
marks backlog items 6/11/21/22 as done, and adds real CLI output examples.
* fix: remove redundant condition in paginate guard clauses
When limit === undefined, limit !== 0 is always true — the && check
was dead code. Simplified to just check limit === undefined.
Impact: 2 functions changed, 18 affected
* docs: update dogfood report with fix statuses
All 4 bugs now fixed (PR #117 merged, #116 closed via reverse-dep
cascade). 3 of 4 suggestions addressed. MCP tool counts updated
18→23 / 19→24. Rating upgraded 7/10 → 9/10 post-fix.
* fix: rename misleading test to match actual behavior
Test was named "handles non-numeric thresholds gracefully" but only
validated baseline exceeds/aboveWarn with valid thresholds. Actual
non-numeric threshold tests exist separately. Renamed to "produces
correct exceeds and aboveWarn with valid thresholds".
* fix: update stale MCP tool count in dogfood skill (21→24)
* feat: add complexity analysis for Python, Go, Rust, Java, C#, Ruby, PHP
Parameterize the complexity algorithm to support all 10 languages instead
of just JS/TS/TSX. Add per-language COMPLEXITY_RULES, HALSTEAD_RULES, and
COMMENT_PREFIXES with three else-if detection patterns (else-wraps-if,
explicit elif, alternative field). Guard against tree-sitter keyword leaf
tokens that share node type names with their parent constructs.
Impact: 4 functions changed, 4 affected
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 participant
Summary
src/manifesto.js) that evaluates function-level, file-level, and graph-level rules against the DB and returns pass/fail verdictscognitive,cyclomatic,maxNesting(function),importCount,exportCount,lineCount,fanIn,fanOut(file),noCycles(graph)fail-level breach (CI gate)codegraph manifesto), programmatic API (manifestoData/manifesto/RULE_DEFS), and MCP toolTest plan
codegraph manifesto -Tverified against codegraph's own DB-j) and file-scoped (-f) modes verified