perf: eliminate WASM re-parse for native complexity + build optimizations#140
perf: eliminate WASM re-parse for native complexity + build optimizations#140carlos-alm merged 3 commits intomainfrom
Conversation
|
Claude finished @carlos-alm's task —— View job PR Review CompleteReviewing the performance optimization PR that ports complexity calculations from WASM to Rust. Review Checklist
Root Cause AnalysisProblem Being Solved: Complexity calculation caused a 1.9→4.8 ms/file regression due to expensive WASM re-parsing during build process. The original implementation required a separate WASM tree-sitter parse just for complexity metrics after initial symbol extraction. Why This Approach: By porting complexity computation to Rust native engine, the expensive WASM tree-sitter re-parse is eliminated. The complexity metrics are computed during initial parsing and cached with symbols, then used as fallback when WASM is unavailable. Risk Assessment: Low risk - purely additive optimization that maintains backward compatibility through WASM fallback. Critical ConcernsNone identified - This is a well-structured performance optimization. Code Quality Analysis✅ Excellent Rust Implementation
✅ Smart Integration Strategy
✅ Performance Optimizations
✅ Definition Completeness Fix
Minor Issues
Final Recommendation
Recommended merge - this will significantly improve build performance for codebases with substantial JS/TS content. |
Greptile SummaryFixed a critical runtime crash where the native complexity engine path was only supplying 4 parameters to a database upsert statement that expects 18 columns (added by the Halstead/MI migration in PR #139). The native engine intentionally skips WASM AST parsing for performance, so Halstead/LOC/MI columns are filled with zeros. Additional improvements:
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 7c08fa0 |
| "function f(x) { if (x > 0) { return 1; } else if (x < 0) { return -1; } else { return 0; } }", | ||
| ); | ||
| assert_eq!(m.cognitive, 4); // +1 if, +1 else-if, +1 else (from else-if's else_clause), +1 else-if cognitive | ||
| // Wait, let me recalculate: |
There was a problem hiding this comment.
Duplicate assertion - this test has two assert_eq!(m.cognitive, ...) with different values (4 on line 327 and 3 on line 336). Remove the incorrect first assertion.
| // Wait, let me recalculate: | |
| // Cognitive complexity calculation: |
There was a problem hiding this comment.
Fixed in 8721016 — removed the duplicate assert_eq!(m.cognitive, 4) and the rambling recalculation comments. The correct value is 3 (if +1, else-if +1, else +1). Also added #[napi(js_name = "maxNesting")] to the Rust struct to eliminate the snake_case fallback shims in parser.js and complexity.js.
The native complexity shortcut was passing only 4 params to the upsert statement which now expects 18 (added by the Halstead/MI migration). This crashed at runtime for natively-parsed functions. Fill Halstead/LOC/MI columns with zeros since native engine intentionally skips WASM AST. Also adds #[napi(js_name = "maxNesting")] to ComplexityMetrics so Rust exports camelCase directly, removing the snake_case fallback in parser.js and complexity.js. Cleans up a duplicate assertion and fixes indentation in go.rs. Impact: 4 functions changed, 4 affected
8721016 to
7c08fa0
Compare
|
Claude finished @carlos-alm's task —— View job PR Review: Performance Optimization AnalysisReviewing the performance optimization PR that ports complexity calculations from WASM to Rust. Review Checklist
Root Cause AnalysisProblem Being Solved: Complexity calculation caused a 1.9→4.8 ms/file regression due to expensive WASM re-parsing during build process. The original implementation required a separate WASM tree-sitter parse just for complexity metrics after initial symbol extraction. Why This Approach: By porting complexity computation to Rust native engine, the expensive WASM tree-sitter re-parse is eliminated. The complexity metrics are computed during initial parsing and cached with symbols, then used as fallback when WASM is unavailable. Risk Assessment: Low risk - purely additive optimization that maintains backward compatibility through WASM fallback. Critical ConcernsNone identified - This is a well-structured performance optimization. Code Quality Analysis✅ Excellent Rust Implementation
✅ Smart Integration Strategy
✅ Performance Optimizations
✅ Definition Completeness Fix
Minor Issues
Final Recommendation
Recommended merge - this will significantly improve build performance for codebases with substantial JS/TS content. |
Complexity rules (cognitive, cyclomatic, maxNesting) now always report warnings but never cause process.exit(1). This keeps codegraph focused on dependency graph analysis rather than acting as a CI linter gate. - Add reportOnly flag to function-level complexity RULE_DEFS - Force fail: null in resolveRules() when reportOnly is set - Remove fail key from complexity defaults in config.js - Add integration test verifying user fail thresholds are ignored Impact: 1 functions changed, 2 affected
Impact: 1 functions changed, 2 affected
|
Claude finished @carlos-alm's task —— View job PR Review: Performance Optimization AnalysisReviewing the performance optimization PR that ports complexity calculations from WASM to Rust. Review Checklist
Root Cause AnalysisProblem Being Solved: Complexity calculation caused a 1.9→4.8 ms/file regression due to expensive WASM re-parsing during build process. The original implementation required a separate WASM tree-sitter parse just for complexity metrics after initial symbol extraction. Why This Approach: By porting complexity computation to Rust native engine, the expensive WASM tree-sitter re-parse is eliminated. The complexity metrics are computed during initial parsing and cached with symbols, then used as fallback when WASM is unavailable. Risk Assessment: Low risk - purely additive optimization that maintains backward compatibility through WASM fallback. Critical ConcernsNone identified - This is a well-structured performance optimization. Code Quality Analysis✅ Excellent Rust Implementation
✅ Smart Integration Strategy
✅ Performance Optimizations
✅ Definition Completeness Fix
Minor Issues
Final Recommendation
Recommended merge - this will significantly improve build performance for codebases with substantial JS/TS content. |
Summary
complexity.rs, 456 lines) — eliminates the expensive WASM re-parse that caused a 1.9→4.8 ms/file regressioncomplexity: Noneinitializers in Definition structs for non-function symbols (interfaces, traits, structs, type aliases)Files changed
complexity.rsmodule + extractor updatesbuilder.js,complexity.js,parser.js,structure.jsTest plan
cargo checkconfirms Rust compiles cleanly