fix(stats): native coupling hotspots exclude structural edges, matching map (#2388) - #2528
Conversation
…ng map (#2388) The native fetch_file_hotspots query counted every edge touching a file node -- including contains/parameter_of/receiver -- while the TypeScript path already excluded them. A file that merely declares many symbols ranked as a coupling hotspot ahead of genuinely central modules, and since native is the default engine, most users saw the inflated number. Hoisted the exclusion list into a shared NON_COUPLING_EDGE_KINDS constant in shared/kinds.ts (module-map.ts's four SQL fan-in/fan-out queries now derive from it instead of repeating the literal), and mirrored the identical filter in graph_read.rs's fetch_file_hotspots so the two engines can't drift again. docs check acknowledged: internal stats aggregation bugfix, no README/CLAUDE.md/ROADMAP surface area changed. Impact: 2 functions changed, 5 affected
Greptile SummaryThe PR aligns native coupling-hotspot calculations with the TypeScript module-map path by excluding structural edges from fan-in and fan-out.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "fix(stats): native coupling hotspots exc..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// outrank one with genuine cross-file dependencies. Mirrors | ||
| /// `NON_COUPLING_EDGE_KINDS` in `src/shared/kinds.ts` (#2388) — keep both | ||
| /// lists in sync. | ||
| const NON_COUPLING_EDGE_KINDS: &[&str] = &["contains", "parameter_of", "receiver"]; |
There was a problem hiding this comment.
When a call such as userService.getUser() resolves its receiver to a symbol in another file, this filter excludes the resulting receiver edge from both fan-in and fan-out, causing genuinely central modules to be under-reported or incorrectly ranked as coupling hotspots.
Knowledge Base Used:
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!
There was a problem hiding this comment.
This exclusion isn't new — it's pre-existing, intentional behavior this PR mirrors from the TypeScript side, not a regression introduced here.
The receiver edge doesn't represent the actual invocation coupling for userService.getUser(). Per resolveReceiverEdge's doc comment in src/domain/graph/builder/call-resolver.ts:737, it "resolve[s] the receiver-type edge" — i.e. it points to UserService (the class), not to getUser (the method actually being invoked). The real cross-file dependency — the caller depending on the target file's exported method — is captured separately by the primary calls edge, resolved via CHA/RTA dispatch (build-edges.ts Steps 1-4/6) before this receiver-type edge is emitted as a supplementary Step 5. That calls edge is never filtered out of coupling counts by NON_COUPLING_EDGE_KINDS (only contains/parameter_of/receiver are), so a resolved cross-file method call is already fully represented in fan-in/fan-out.
This also isn't specific to hotspots: classifyNodeRolesFull's own fan-in/fan-out computation (src/features/structure.ts) — which drives dead-code role classification, not just stats/map — already excludes receiver from both, via kind IN ('calls', 'imports-type') for fan-in and kind = 'calls' for fan-out. Three independent consumers (role classification, map, stats) all treat receiver as structural rather than coupling, consistent with how parameter_of is treated for the same reason (a type/structural relationship, not an invocation). Changing that classification for stats/map alone would make them inconsistent with role classification's own coupling model, and would be a design change affecting existing, shipped codegraph map behavior — out of scope for this issue, which asked specifically to make the native path match the existing TypeScript filter, not to redesign what counts as coupling.
If there's a concrete case where a receiver-type edge is the only signal for a resolved cross-file dependency (no accompanying calls edge), that would be a genuine edge case worth its own issue — but it wouldn't be fixed by including receiver broadly in coupling counts, since the vast majority of receiver edges are redundant with an already-counted calls edge to the specific method.
There was a problem hiding this comment.
You're right, and thank you for the detailed explanation. The receiver edge records the receiver-type relationship (class declaration), not the invocation — the actual cross-file dependency is already captured by the accompanying calls edge, which is never filtered. Since all three consumers (role classification, map, stats) consistently treat receiver as structural, this exclusion is coherent and intentional, and this PR is correctly mirroring the existing TS behavior rather than introducing a new design decision.
Shall I remember this as a rule: "In this repo, receiver edges are structural (receiver-type), not coupling — calls edges carry the invocation signal, so excluding receiver from fan-in/fan-out counts is intentional and consistent across all three consumers (role classification, map, stats)"?
Codegraph Impact Analysis2 functions changed → 5 callers affected across 3 files
|
1 participant
Summary
codegraph stats' "coupling hotspots" fan-in/fan-out counted every edge touching a file node in the native path, including structuralcontains/parameter_of/receiveredges. The TypeScript path that powerscodegraph mapalready excluded them. The two engines therefore reported different numbers for the same metric, and since native is the default, most users saw the inflated one — a file that merely declares many symbols (e.g. a pure type-declaration file) ranked as a coupling hotspot ahead of genuinely central modules.Root cause & fix
fetch_file_hotspotsincrates/codegraph-core/src/db/repository/graph_read.rshad no edge-kind filter at all.findHotspotsinsrc/domain/analysis/module-map.tswas already correctly filtered, but repeated the same literalkind NOT IN ('contains', 'parameter_of', 'receiver')four times across two functions (findHotspotsforstats, and themoduleMapDataquery formap) with no shared source of truth — exactly the kind of duplication the issue flagged as having already drifted once (bug(stats): roles map double-counts dead symbols — aggregate 'dead' key is a peer of its own sub-roles #2383's roles rollup).Fix:
NON_COUPLING_EDGE_KINDStosrc/shared/kinds.tsand rebuilt module-map.ts's SQL filter fragment from it, replacing all four literal occurrences.graph_read.rs'sfetch_file_hotspots, with a doc comment cross-referencing the TS constant so the two can't drift again.Verification
cargo fmt -- --check/cargo clippy --workspace --all-targets -- -D warnings: passtests/integration/queries.test.ts(statsData hotspots exclude structural edges from coupling, matching moduleMapData) — assertsstats' hotspot fanIn/fanOut for a file exactly equalmap's inEdges/outEdges for the same file, using the existing fixture'scontains/parameter_of/receiveredges. Verified this test fails against the pre-fix native addon (rebuilt vianapi buildwith the Rust fix reverted:expected 1 to be +0) before confirming it passes with the fix restored — not tautological.npx vitest run tests/integration/queries.test.ts tests/integration/roles.test.ts tests/presentation/queries-cli.test.ts tests/graph/: 397/397 passnapi build, not the stale prebuilt one):src/types.tsnow showsfan-in: 227 fan-out: 0instats, matchingmap'sinEdges: 227, outEdges: 0exactly — previouslyfan-out: 1322. Confirmed identical between the native-built and WASM-built graphs.npm run lint: passCloses #2388