refactor(native): decompose core Rust algorithms and pipeline#845
refactor(native): decompose core Rust algorithms and pipeline#845carlos-alm merged 11 commits intomainfrom
Conversation
Extract hardcoded magic numbers to named constants in constants.rs: - Louvain: MAX_LEVELS=50, MAX_PASSES=20, MIN_GAIN=1e-12, DEFAULT_SEED=42 - Dataflow: TRUNCATION_LIMIT=120 - Build pipeline: FAST_PATH_MAX_CHANGED_FILES=5, FAST_PATH_MIN_EXISTING_FILES=20 Also extract DEFAULT_RANDOM_SEED=42 in TS louvain.ts.
|
Claude finished @carlos-alm's task —— View job Root Cause AnalysisProblem Being Solved: Reducing cognitive complexity in high-complexity native Rust functions that were identified as hard to maintain (louvain_impl cog=85→8, extract_param_names_strategy cog=83→1, run_pipeline cog=110→29) Why This Approach: Function decomposition is the standard solution for cognitive overload. The metrics improvements (bugs 7.42→4.39, MI 22.9→34.1) demonstrate real maintainability gains. This follows established "Titan" patterns for decomposing complex algorithms. Risk Assessment: ✅ LOW - Pure refactoring with preserved test coverage and functionality. The existing Louvain and dataflow tests validate behavioral compatibility. Backlog Compliance
Code Quality Analysis✅ Excellent Decomposition PatternThe refactoring follows clean separation principles: crates/codegraph-core/src/graph_algorithms.rs:
crates/codegraph-core/src/dataflow.rs:
crates/codegraph-core/src/build_pipeline.rs:
✅ Constants Extractioncrates/codegraph-core/src/constants.rs:
✅ Test Integrity MaintainedAll existing tests in
Final Recommendation
Minor note: This represents excellent engineering practice for managing technical debt in complex algorithms. |
Greptile SummaryThis PR decomposes three high-complexity Rust functions in the native engine —
Confidence Score: 5/5Safe to merge — pure decomposition refactor with no behavioral changes and no P0/P1 issues All three decompositions are mechanically correct and semantics-preserving. The Louvain phase split correctly retains the original edge map outside LouvainState for final modularity computation. The dataflow dispatch is a clean table-driven refactor. The build pipeline stage extraction preserves all control flow and variable scoping. The only finding is a P2 redundant field initializer. The prior concern about the No files require special attention Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[run_pipeline] --> B[check_version_mismatch]
A --> C[collect_source_files\nStage 2]
C --> D{Scoped build?}
D -- Yes --> D1[Use scope files directly]
D -- No --> D2{Incremental + journal?}
D2 -- Yes --> D3[try_fast_collect]
D2 -- No --> D4[collect_files]
A --> E[detect_changes\nStage 3]
E --> F{No changes?}
F -- Yes --> G[write_journal_header\nEarly return]
F -- No --> H[parse_files_parallel\nStage 4]
H --> I[do_insert_nodes\nStage 5]
I --> J[resolve_imports_batch\nStage 6]
J --> K{Incremental?}
K -- Yes --> L[reparse_barrel_candidates\nStage 6b]
K -- No --> M
L --> M[build_import_edges\nStage 7]
M --> N[build_and_insert_call_edges]
N --> O[structure metrics\nStage 8]
O --> P[role_classification]
P --> Q[finalize_build\nStage 9]
Reviews (4): Last reviewed commit: "fix: resolve merge conflicts with main" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (_rel_path, symbols) in file_symbols.iter() { | ||
| for imp in &symbols.imports { | ||
| let abs_file = Path::new(root_dir).join(_rel_path); |
There was a problem hiding this comment.
_rel_path is used — misleading underscore prefix
In Rust, the _ prefix on a variable name suppresses the "unused variable" compiler warning and signals to readers that the variable is intentionally not used. However, _rel_path is actively referenced on line 509 (Path::new(root_dir).join(_rel_path)), so the prefix is misleading and could cause confusion for future contributors.
| for (_rel_path, symbols) in file_symbols.iter() { | |
| for imp in &symbols.imports { | |
| let abs_file = Path::new(root_dir).join(_rel_path); | |
| for (rel_path, symbols) in file_symbols.iter() { | |
| for imp in &symbols.imports { | |
| let abs_file = Path::new(root_dir).join(rel_path); |
There was a problem hiding this comment.
Fixed in 457d85a. Removed the underscore prefix — the variable is now rel_path since it is actively used on the next line.
There was a problem hiding this comment.
Fixed in 457d85a. Removed the underscore prefix — the variable is now rel_path since it is actively used on the next line.
There was a problem hiding this comment.
This was already fixed in 457d85a by renaming _rel_path to rel_path. Confirmed the fix is preserved through the merge with main.
Codegraph Impact Analysis20 functions changed → 18 callers affected across 5 files
|
- Fix type mismatch in build_pipeline.rs: change FAST_PATH_MIN_EXISTING_FILES from usize to i64 to match get_existing_file_count return type - Remove misleading underscore prefix from _rel_path variable that is actually used (build_pipeline.rs:507) - Replace missed hardcoded 120 literal with DATAFLOW_TRUNCATION_LIMIT constant at dataflow.rs:1071
|
Addressed both Greptile findings in 457d85a:
Also fixed the Rust compile error that was failing CI: |
|
Addressed the two remaining Greptile findings from the re-review in b01db3f:
Also merged |
|
Resolved merge conflicts with main in bcf6ef6. Changes from PR #865 (scoped build paths, barrel resolution Windows normalization, build_meta table name) are incorporated into the decomposed helper functions. All three conflicts in |
1 participant
Summary
louvain_impl(cog=85, bugs=2.72) intoinit_communities(),move_nodes_phase(), andaggregate_phase()extract_param_names_strategy(cog=83, bugs=1.36) into per-language handler functionsrun_pipeline(cog=110, bugs=7.42) into stage functions: setup, collect, parse, resolve, edges, finalizeTitan Audit Context
Changes
crates/codegraph-core/src/graph_algorithms.rs-- decompose louvain_impl into 3 phase functionscrates/codegraph-core/src/dataflow.rs-- split extract_param_names_strategy into per-language handlerscrates/codegraph-core/src/build_pipeline.rs-- decompose run_pipeline into stage functionsMetrics Impact
Test plan