fix(native): align Rust role classification with JS#918
Conversation
The native Rust role classifier was producing different (less accurate) results than the JS fallback path. This fixes all divergences: - Include 'imports-type' edges in fan-in queries (full, incremental, medians) - Add reexport-chain exported detection via recursive CTE (#837) - Include 'imports-type' in exported IDs and production fan-in queries - Add constant + hasActiveFileSiblings logic (classify as leaf, not dead) - Add !isExported guard to test-only classification - Include 'imports-type' and 'reexports' in neighbour file discovery - Add test_file_filter_col() for arbitrary column filtering
|
Claude finished @carlos-alm's task —— View job PR Review AnalysisReviewing PR #918: fix(native): align Rust role classification with JS Todo List:
Root Cause AnalysisProblem Being Solved: The native Rust role classification engine was producing different (less accurate) results than the JS fallback when using Why This Approach: The fix systematically adds missing functionality to match the JS implementation:
Risk Assessment: ✅ Low risk. This is a bug fix, not a workaround. The discrepancy between engines violates CLAUDE.md's core principle: "If two engines produce different results, that is a bug in the less-accurate engine — not an acceptable 'parity gap.'" The implementation correctly mirrors the proven JS logic. Configuration Change Detection✅ No HIGH-RISK configuration changes detected. This is pure Rust code implementing missing functionality. No linting rules, test thresholds, or build configs were relaxed. Test Integrity✅ Test integrity maintained. The PR description shows:
The existing test at Code Quality ReviewStrengths:
Technical verification:
Minor observations:
Backlog Compliance
Critical ConcernsNone. This is a well-implemented bug fix that restores the design contract between engines. Final Recommendation
Test validation recommended: Run |
Codegraph Impact Analysis10 functions changed → 4 callers affected across 1 files
|
Greptile SummaryAligns the Rust native role classifier ( Confidence Score: 5/5Safe to merge — all changes correctly mirror the JS classifier logic with no P0/P1 issues. All four alignment fixes (imports-type fan-in, reexport CTE, constant leaf heuristic, !isExported test-only guard) are logically correct and verified against the JS reference implementation. The only finding is a P2 note about reexports edges being dead code in the neighbour query — a pre-existing JS quirk faithfully ported, with no correctness impact on the critical classification paths. No files require special attention beyond the noted P2 in find_neighbour_files. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Node: fan_in=0, not exported] --> B{kind == constant\nAND hasActiveFileSiblings?}
B -- Yes --> C[leaf]
B -- No --> D[classify_dead_sub_role]
D --> E{LEAF_KINDS?}
E -- Yes --> F[dead-leaf]
E -- No --> G{FFI extension?}
G -- Yes --> H[dead-ffi]
G -- No --> I{Entry path pattern?}
I -- Yes --> J[dead-entry]
I -- No --> K[dead-unresolved]
A2[Node: fan_in=0, is_exported] --> L[entry]
A3[Node: fan_in>0, prod_fan_in=0, NOT exported] --> M[test-only]
A4[Node: fan_in>0, prod_fan_in>0 OR is_exported] --> N{high_in / high_out?}
N -- high_in only --> O[core]
N -- both high --> P[utility]
N -- high_out only --> Q[adapter]
N -- neither --> R[leaf]
FW[Framework entry prefix] --> S[entry]
Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/native-role..." | Re-trigger Greptile |
Expand comments on the recursive prod_reachable CTEs in both the full and incremental classify paths to explain the base-case/recursive-step mechanics and how barrel reexport chains are traced.
|
Addressed the two minor suggestions from the review:
|
Summary
rolesphase to classify nodes incorrectly when using--engine nativeimports-typeedge support to all fan-in, exported-ID, and production fan-in queriesconstant+hasActiveFileSiblingslogic so constants in active files classify asleafinstead ofdead-leaf!isExportedguard to test-only classificationimports-typeandreexportsin incremental neighbour file discoveryTest plan
cargo checkpasses with no new warningscodegraph build --engine nativeandcodegraph build --engine wasmon a real project, comparecodegraph rolesoutput — should now match