Skip to content

fix(stats): native coupling hotspots exclude structural edges, matching map (#2388) #2528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlos-alm merged 1 commit into from
Aug 15, 2026
+61 −9
Merged

fix(stats): native coupling hotspots exclude structural edges, matching map (#2388) #2528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions crates/codegraph-core/src/db/repository/graph_read.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,17 +597,31 @@ fn fetch_quality_metrics(
})
}

/// Edge kinds representing structural containment rather than cross-symbol
/// coupling (a parent declaring a child, a function declaring its own
/// parameter, a method's implicit receiver) — excluded from fan-in/fan-out
/// hotspot counts so a file that merely declares many symbols doesn't
/// 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"];

@greptile-apps greptile-apps Bot Aug 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Receiver coupling is discarded

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!

Fix in Claude Code

@carlos-alm carlos-alm Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@greptile-apps greptile-apps Bot Aug 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)"?


fn fetch_file_hotspots(
conn: &rusqlite::Connection,
tf_n_file: &str,
) -> napi::Result<Vec<FileHotspot>> {
let edge_filter = NON_COUPLING_EDGE_KINDS
.iter()
.map(|k| format!("'{k}'"))
.collect::<Vec<_>>()
.join(", ");
let sql = format!(
"SELECT n.file, \
(SELECT COUNT(*) FROM edges WHERE target_id = n.id) as fan_in, \
(SELECT COUNT(*) FROM edges WHERE source_id = n.id) as fan_out \
(SELECT COUNT(*) FROM edges WHERE target_id = n.id AND kind NOT IN ({edge_filter})) as fan_in, \
(SELECT COUNT(*) FROM edges WHERE source_id = n.id AND kind NOT IN ({edge_filter})) as fan_out \
FROM nodes n WHERE n.kind = 'file' {tf_n_file} \
ORDER BY (SELECT COUNT(*) FROM edges WHERE target_id = n.id) \
+ (SELECT COUNT(*) FROM edges WHERE source_id = n.id) DESC \
ORDER BY (SELECT COUNT(*) FROM edges WHERE target_id = n.id AND kind NOT IN ({edge_filter})) \
+ (SELECT COUNT(*) FROM edges WHERE source_id = n.id AND kind NOT IN ({edge_filter})) DESC \
LIMIT 5",
);
let mut stmt = conn
Expand Down
17 changes: 12 additions & 5 deletions src/domain/analysis/module-map.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@ import {
} from '../../db/index.js';
import { debug } from '../../infrastructure/logger.js';
import { isTestFile } from '../../infrastructure/test-filter.js';
import { DEAD_ROLE_PREFIX } from '../../shared/kinds.js';
import { DEAD_ROLE_PREFIX, NON_COUPLING_EDGE_KINDS } from '../../shared/kinds.js';
import type { BetterSqlite3Database, NativeDatabase } from '../../types.js';
import { findCycles } from '../graph/cycles.js';
import { LANGUAGE_REGISTRY } from '../parser.js';

// SQL fragment excluding structural-containment edges from coupling counts —
// built once from the shared NON_COUPLING_EDGE_KINDS constant so this file's
// four fan-in/fan-out queries (findHotspots, moduleMapData) and the native
// path (crates/codegraph-core/src/db/repository/graph_read.rs) cannot drift
// out of sync again (#2388).
const NON_COUPLING_EDGE_FILTER = `kind NOT IN (${NON_COUPLING_EDGE_KINDS.map((k) => `'${k}'`).join(', ')})`;

export const FALSE_POSITIVE_NAMES = new Set([
'run',
'get',
Expand Down Expand Up @@ -123,12 +130,12 @@ function findHotspots(db: BetterSqlite3Database, noTests: boolean, limit: number
FROM nodes n
LEFT JOIN (
SELECT target_id, COUNT(*) AS cnt FROM edges
WHERE kind NOT IN ('contains', 'parameter_of', 'receiver')
WHERE ${NON_COUPLING_EDGE_FILTER}
GROUP BY target_id
) fi ON fi.target_id = n.id
LEFT JOIN (
SELECT source_id, COUNT(*) AS cnt FROM edges
WHERE kind NOT IN ('contains', 'parameter_of', 'receiver')
WHERE ${NON_COUPLING_EDGE_FILTER}
GROUP BY source_id
) fo ON fo.source_id = n.id
WHERE n.kind = 'file' ${testFilter}
Expand Down Expand Up @@ -325,12 +332,12 @@ export function moduleMapData(customDbPath: string, limit = 20, opts: { noTests?
FROM nodes n
LEFT JOIN (
SELECT source_id, COUNT(*) AS cnt FROM edges
WHERE kind NOT IN ('contains', 'parameter_of', 'receiver')
WHERE ${NON_COUPLING_EDGE_FILTER}
GROUP BY source_id
) fo ON fo.source_id = n.id
LEFT JOIN (
SELECT target_id, COUNT(*) AS cnt FROM edges
WHERE kind NOT IN ('contains', 'parameter_of', 'receiver')
WHERE ${NON_COUPLING_EDGE_FILTER}
GROUP BY target_id
) fi ON fi.target_id = n.id
WHERE n.kind = 'file'
Expand Down
9 changes: 9 additions & 0 deletions src/shared/kinds.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ export const STRUCTURAL_EDGE_KINDS: readonly StructuralEdgeKind[] = [
// Full set for MCP enum and validation
export const EVERY_EDGE_KIND: readonly EdgeKind[] = [...CORE_EDGE_KINDS, ...STRUCTURAL_EDGE_KINDS];

// Edge kinds that represent structural containment rather than cross-symbol
// coupling (a parent declaring a child, a function declaring its own
// parameter, a method's implicit receiver) — excluded from fan-in/fan-out
// coupling metrics (`codegraph map`, `stats`' coupling hotspots) so a file
// that merely declares many symbols doesn't outrank one with genuine
// cross-file dependencies (#2388). Single source of truth for both the
// native and TypeScript aggregation paths in module-map.ts.
export const NON_COUPLING_EDGE_KINDS: readonly EdgeKind[] = ['contains', ...STRUCTURAL_EDGE_KINDS];

// Dead sub-categories — refine the coarse "dead" bucket
export const DEAD_ROLE_PREFIX = 'dead';
export const DEAD_SUB_ROLES: readonly DeadSubRole[] = [
Expand Down
22 changes: 22 additions & 0 deletions tests/integration/queries.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,28 @@ describe('expanded edge types', () => {
expect(authNode.inEdges).toBe(2);
});

test('statsData hotspots exclude structural edges from coupling, matching moduleMapData (#2388)', () => {
// Regression test for #2388: the native `stats` path counted every edge
// touching a file node (including contains/parameter_of/receiver), while
// moduleMapData's JS path correctly excluded them — same graph, two
// different fan-in/fan-out numbers for the same file. auth.js has a
// `contains` edge as its SOURCE (auth.js -> UserService) that must not
// inflate fanOut, exactly mirroring the moduleMapData test above.
const mapData = moduleMapData(dbPath);
const authMapNode = mapData.topNodes.find((n) => n.file === 'auth.js');
expect(authMapNode).toBeDefined();

const stats = statsData(dbPath);
const authHotspot = stats.hotspots.find((h) => h.file === 'auth.js');
expect(authHotspot).toBeDefined();
expect(authHotspot.fanIn).toBe(authMapNode.inEdges);
expect(authHotspot.fanOut).toBe(authMapNode.outEdges);
// auth.js is imported by middleware.js and auth.test.js → fanIn = 2;
// its only outgoing edge is the structural contains edge → fanOut = 0.
expect(authHotspot.fanIn).toBe(2);
expect(authHotspot.fanOut).toBe(0);
});

test('queryNameData returns new edge kinds in callers/callees', () => {
// authenticate has a parameter_of edge from userId
const authData = queryNameData('authenticate', dbPath);
Expand Down
Loading