Skip to content

Commit aec5d2b

Browse files
committed
fix: restrict entry-role classification to function/method-kind symbols (docs check acknowledged)
codegraph roles --role entry classified any exported, zero-fan-in node as `entry` regardless of kind, so non-exported interfaces/constants (e.g. ParsedUserConfig, WorkspaceEntry, _configCache in config.ts) were also sweeping into the entry bucket via a separate bug: the barrel-reexport-chain "exported" inference (#837) marks every symbol in a re-exported file as exported, including symbols that were never independently exportable (class/interface methods) and symbols without the `export` keyword. Two fixes, mirrored in the TS classifier and the Rust native engine: - classifyNodeRole / classify_node: the exported+zero-fan-in branch now requires kind IN ('function', 'method') to return `entry`. Every other exported kind (interface/type/constant/class) is classified `leaf` instead — a live, intentional part of the public surface, but not something invoked from outside the codebase. This also refines two #1583 cases (exported interface with same-file-only type usage) from `entry` to the more accurate `leaf`, while keeping their "never dead" guarantee intact. - classifyNodeRolesFull / classifyNodeRolesIncremental (structure.ts) and do_classify_full / do_classify_incremental (roles.rs): exclude `method` from the barrel-reexport-chain exported-inference query. A `reexports` edge only ever concerns top-level module bindings, so a class method can never be an independently re-exportable binding — without this exclusion, abstract base-class methods (e.g. Repository.findNodeById) were promoted to `entry` merely because a sibling symbol in the same file was re-exported through a barrel. Verified real entry points are unaffected: CLI command execute/validate methods, MCP tool handlers, ESM loader hooks, and event:/route:/command: framework-prefixed handlers all still classify as `entry`. Symptom 2 from the same issue (dead-unresolved for LanguageRules interface members in visitor-utils.ts) was already fully fixed by #1723's isTypeDeclarationMember exemption — verified via fresh build, no changes needed. Fixes #1780 Impact: 3 functions changed, 4 affected
1 parent d4e6212 commit aec5d2b

6 files changed

Lines changed: 251 additions & 12 deletions

File tree

crates/codegraph-core/src/graph/classifiers/roles.rs

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,20 @@ fn classify_node(
256256
}
257257

258258
if fan_in == 0 && is_exported {
259-
return "entry";
259+
// Exported, zero fan-in. A genuine entry point (CLI command handler,
260+
// exported API function called from outside the codebase, ESM loader
261+
// hook, MCP tool handler, etc.) is always a function or method. Every
262+
// other exported kind (interface/type/constant/class) is a live,
263+
// intentional part of the public surface — but a data shape or config
264+
// value, not something invoked from outside the codebase — so it's
265+
// `leaf`: never `dead-*` (#1583) and never `entry` (#1780), regardless
266+
// of whether the file has other active siblings. Mirrors JS
267+
// `classifyNodeRole`.
268+
return if kind == "function" || kind == "method" {
269+
"entry"
270+
} else {
271+
"leaf"
272+
};
260273
}
261274

262275
// Test-only: has callers but all are in test files
@@ -403,6 +416,15 @@ pub(crate) fn do_classify_full(conn: &Connection) -> rusqlite::Result<RoleSummar
403416
// (e.g. index.ts re-exports from internal.ts which re-exports from core.ts).
404417
// Then: any symbol whose file is a reexport target of a prod-reachable barrel
405418
// is considered exported (prevents false dead-code classification).
419+
//
420+
// `method` is excluded (#1780): a `reexports` edge only ever concerns
421+
// top-level module bindings (functions, classes, types, constants, ...) —
422+
// a class/interface method can never be an independently re-exportable
423+
// binding on its own, so inheriting "exported" status from a co-located
424+
// top-level re-export is a category error. Without this exclusion, e.g. an
425+
// abstract base class's zero-fan-in method declarations were promoted to
426+
// `entry` merely because some other symbol in the same file was re-exported
427+
// through a barrel.
406428
{
407429
let sql = format!(
408430
"WITH RECURSIVE prod_reachable(file_id) AS (
@@ -426,7 +448,7 @@ pub(crate) fn do_classify_full(conn: &Connection) -> rusqlite::Result<RoleSummar
426448
WHERE e.kind = 'reexports'
427449
AND e.source_id IN (SELECT file_id FROM prod_reachable)
428450
)
429-
AND n.kind NOT IN ('file', 'directory', 'parameter', 'property')",
451+
AND n.kind NOT IN ('file', 'directory', 'parameter', 'property', 'method')",
430452
test_file_filter_col("src.file")
431453
);
432454
let mut stmt = tx.prepare(&sql)?;
@@ -810,6 +832,7 @@ pub(crate) fn do_classify_incremental(
810832
// from production-reachable barrels (traces through multi-level chains) (#837).
811833
// Same recursive CTE logic as the full-classify path (step 3b), but scoped
812834
// to affected files only via the additional `AND n.file IN (...)` filter.
835+
// `method` is excluded (#1780) — see the full-classify path for rationale.
813836
{
814837
let reexport_sql = format!(
815838
"WITH RECURSIVE prod_reachable(file_id) AS (
@@ -833,7 +856,7 @@ pub(crate) fn do_classify_incremental(
833856
WHERE e.kind = 'reexports'
834857
AND e.source_id IN (SELECT file_id FROM prod_reachable)
835858
)
836-
AND n.kind NOT IN ('file', 'directory', 'parameter', 'property')
859+
AND n.kind NOT IN ('file', 'directory', 'parameter', 'property', 'method')
837860
AND n.file IN ({})",
838861
test_file_filter_col("src.file"),
839862
affected_ph

src/features/structure.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,15 @@ function classifyNodeRolesFull(db: BetterSqlite3Database, emptySummary: RoleSumm
816816

817817
// Mark symbols as exported when their files are targets of reexports edges
818818
// from production-reachable barrels (traces through multi-level chains) (#837)
819+
//
820+
// `method` is excluded (#1780): a `reexports` edge only ever concerns
821+
// top-level module bindings (functions, classes, types, constants, ...) — a
822+
// class/interface method can never be an independently re-exportable
823+
// binding on its own, so inheriting "exported" status from a co-located
824+
// top-level re-export is a category error. Without this exclusion, e.g. an
825+
// abstract base class's zero-fan-in method declarations were promoted to
826+
// `entry` merely because some other symbol in the same file was re-exported
827+
// through a barrel.
819828
const reexportExported = db
820829
.prepare(
821830
`WITH RECURSIVE prod_reachable(file_id) AS (
@@ -839,7 +848,7 @@ function classifyNodeRolesFull(db: BetterSqlite3Database, emptySummary: RoleSumm
839848
WHERE e.kind = 'reexports'
840849
AND e.source_id IN (SELECT file_id FROM prod_reachable)
841850
)
842-
AND n.kind NOT IN ('file', 'directory', 'parameter', 'property')`,
851+
AND n.kind NOT IN ('file', 'directory', 'parameter', 'property', 'method')`,
843852
)
844853
.all() as { id: number }[];
845854
for (const r of reexportExported) exportedIds.add(r.id);
@@ -1004,6 +1013,7 @@ function classifyNodeRolesIncremental(
10041013

10051014
// 3b. Mark symbols as exported when their files are targets of reexports edges
10061015
// from production-reachable barrels (traces through multi-level chains) (#837)
1016+
// `method` is excluded (#1780) — see classifyNodeRolesFull for rationale.
10071017
const reexportExported = db
10081018
.prepare(
10091019
`WITH RECURSIVE prod_reachable(file_id) AS (
@@ -1027,7 +1037,7 @@ function classifyNodeRolesIncremental(
10271037
WHERE e.kind = 'reexports'
10281038
AND e.source_id IN (SELECT file_id FROM prod_reachable)
10291039
)
1030-
AND n.kind NOT IN ('file', 'directory', 'parameter', 'property')
1040+
AND n.kind NOT IN ('file', 'directory', 'parameter', 'property', 'method')
10311041
AND n.file IN (${placeholders})`,
10321042
)
10331043
.all(...allAffectedFiles) as { id: number }[];

src/graph/classifiers/roles.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@
2121
* `isTypeDeclarationMember` and classified `leaf` unconditionally — they can
2222
* never gain inbound call edges by construction, so call-graph reachability
2323
* doesn't apply to them either (#1723).
24+
*
25+
* `entry` requires `kind IN ('function', 'method')` (plus the framework-prefix/
26+
* Commander-dispatch shortcuts, which are already kind-appropriate by
27+
* construction). An exported interface/type/constant/class with zero fan-in is
28+
* a data-shape declaration or config value — never invoked from outside the
29+
* codebase — so it can't be a real entry point; it's classified `leaf` instead
30+
* of inheriting `entry` merely from being exported (#1780).
2431
*/
2532

2633
import type { DeadSubRole, Role } from '../../types.js';
@@ -281,7 +288,14 @@ function classifyNodeRole(
281288
}
282289
return classifyUnreferencedNode(node);
283290
}
284-
return 'entry';
291+
// Exported, zero fan-in. A genuine entry point (CLI command handler, exported
292+
// API function called from outside the codebase, ESM loader hook, MCP tool
293+
// handler, etc.) is always a function or method. Every other exported kind
294+
// (interface/type/constant/class) is a live, intentional part of the public
295+
// surface — but a data shape or config value, not something invoked from
296+
// outside the codebase — so it's `leaf`: never `dead-*` (#1583) and never
297+
// `entry` (#1780), regardless of whether the file has other active siblings.
298+
return node.kind === 'function' || node.kind === 'method' ? 'entry' : 'leaf';
285299
}
286300

287301
const hasProdFanIn = typeof node.productionFanIn === 'number';

tests/graph/classifiers/roles.test.ts

Lines changed: 140 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ describe('classifyRoles', () => {
66
expect(classifyRoles([]).size).toBe(0);
77
});
88

9-
it('classifies entry nodes (no fan-in, exported)', () => {
10-
const nodes = [{ id: '1', name: 'init', fanIn: 0, fanOut: 3, isExported: true }];
9+
it('classifies entry nodes (no fan-in, exported, function kind)', () => {
10+
const nodes = [
11+
{ id: '1', name: 'init', kind: 'function', fanIn: 0, fanOut: 3, isExported: true },
12+
];
1113
const roles = classifyRoles(nodes);
1214
expect(roles.get('1')).toBe('entry');
1315
});
@@ -628,4 +630,140 @@ describe('classifyRoles', () => {
628630
const roles = classifyRoles(nodes);
629631
expect(roles.get('2')).toBe('dead-unresolved');
630632
});
633+
634+
// ── entry role requires function/method kind (#1780) ───────────────
635+
636+
it('classifies an exported interface with zero fan-in as leaf, not entry', () => {
637+
// Mirrors the #1780 repro: `interface ParsedUserConfig { ... }` in
638+
// src/infrastructure/config.ts. Even if the symbol were exported, an
639+
// interface is a data-shape declaration, never a callable entry point.
640+
const nodes = [
641+
{
642+
id: '1',
643+
name: 'ParsedUserConfig',
644+
kind: 'interface',
645+
file: 'src/infrastructure/config.ts',
646+
fanIn: 0,
647+
fanOut: 0,
648+
isExported: true,
649+
},
650+
];
651+
const roles = classifyRoles(nodes);
652+
expect(roles.get('1')).toBe('leaf');
653+
});
654+
655+
it('classifies an exported constant with zero fan-in as leaf, not entry', () => {
656+
// Mirrors the #1780 repro: module-level `const BUILD_HASH_KEYS = [...]`.
657+
// A constant can never be an invoked entry point regardless of export status.
658+
const nodes = [
659+
{
660+
id: '1',
661+
name: 'BUILD_HASH_KEYS',
662+
kind: 'constant',
663+
file: 'src/infrastructure/config.ts',
664+
fanIn: 0,
665+
fanOut: 0,
666+
isExported: true,
667+
},
668+
];
669+
const roles = classifyRoles(nodes);
670+
expect(roles.get('1')).toBe('leaf');
671+
});
672+
673+
it('classifies an exported class with zero fan-in as leaf, not entry', () => {
674+
// A class declaration itself is instantiated via `new`, not "invoked" the
675+
// way a CLI command handler or API function is — not a real entry point.
676+
const nodes = [
677+
{
678+
id: '1',
679+
name: 'Widget',
680+
kind: 'class',
681+
file: 'src/widget.ts',
682+
fanIn: 0,
683+
fanOut: 0,
684+
isExported: true,
685+
},
686+
];
687+
const roles = classifyRoles(nodes);
688+
expect(roles.get('1')).toBe('leaf');
689+
});
690+
691+
it('classifies an exported type alias with zero fan-in as leaf, not entry', () => {
692+
const nodes = [
693+
{
694+
id: '1',
695+
name: 'WorkspaceEntry',
696+
kind: 'type',
697+
file: 'src/infrastructure/config.ts',
698+
fanIn: 0,
699+
fanOut: 0,
700+
isExported: true,
701+
},
702+
];
703+
const roles = classifyRoles(nodes);
704+
expect(roles.get('1')).toBe('leaf');
705+
});
706+
707+
it('still classifies an exported function with zero fan-in as entry (no over-correction)', () => {
708+
// Genuine entry points (CLI handlers, MCP tool handlers, ESM loader hooks)
709+
// are, by definition, called from outside the codebase, so zero in-repo
710+
// fan-in is expected and correct for them — the fix must not lose this.
711+
const nodes = [
712+
{
713+
id: '1',
714+
name: 'handler',
715+
kind: 'function',
716+
file: 'src/mcp/tools/audit.ts',
717+
fanIn: 0,
718+
fanOut: 4,
719+
isExported: true,
720+
},
721+
];
722+
const roles = classifyRoles(nodes);
723+
expect(roles.get('1')).toBe('entry');
724+
});
725+
726+
it('still classifies an exported method with zero fan-in as entry (no over-correction)', () => {
727+
const nodes = [
728+
{
729+
id: '1',
730+
name: 'run',
731+
kind: 'method',
732+
file: 'src/cli/commands/custom.ts',
733+
fanIn: 0,
734+
fanOut: 2,
735+
isExported: true,
736+
},
737+
];
738+
const roles = classifyRoles(nodes);
739+
expect(roles.get('1')).toBe('entry');
740+
});
741+
742+
it('classifies a non-exported interface with zero fan-in the same as an exported one (leaf, active siblings)', () => {
743+
// Whether or not the interface itself is exported, it's still not a
744+
// callable entry point — both must land on the same non-entry path.
745+
const nodes = [
746+
{
747+
id: '1',
748+
name: 'ConsentResolutionResult',
749+
kind: 'interface',
750+
file: 'src/infrastructure/config.ts',
751+
fanIn: 0,
752+
fanOut: 0,
753+
isExported: false,
754+
hasActiveFileSiblings: true,
755+
},
756+
{
757+
id: '2',
758+
name: 'loadConfig',
759+
kind: 'function',
760+
file: 'src/infrastructure/config.ts',
761+
fanIn: 5,
762+
fanOut: 3,
763+
isExported: true,
764+
},
765+
];
766+
const roles = classifyRoles(nodes);
767+
expect(roles.get('1')).toBe('leaf');
768+
});
631769
});

tests/integration/roles.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ describe('barrel re-export role classification', () => {
115115
const _helperFn = insertNode(db, 'helperFn', 'function', 'src/inspect.ts', 30);
116116
const _appMain = insertNode(db, 'appMain', 'function', 'src/app.ts', 1);
117117
const testFn = insertNode(db, 'testQueryName', 'function', 'tests/inspect.test.ts', 1);
118+
// A class-method-kind member (e.g. an abstract base-class method) in the same
119+
// re-exported file, with no callers and no outgoing calls (#1780).
120+
const _abstractHelper = insertNode(db, 'abstractHelper', 'method', 'src/inspect.ts', 50);
118121

119122
// Barrel re-exports inspect.ts
120123
insertEdge(db, fBarrel, fInspect, 'reexports');
@@ -156,6 +159,20 @@ describe('barrel re-export role classification', () => {
156159
// With fanIn=0 and isExported=true → entry (exported but uncalled)
157160
expect(helperResult!.role).toBe('entry');
158161
});
162+
163+
test('method-kind member in a re-exported file does not inherit exported status (#1780)', () => {
164+
// A `reexports` edge only ever concerns top-level module bindings — a class
165+
// method can never be an independently re-exportable binding on its own, so
166+
// it must not inherit "exported" status merely because a sibling top-level
167+
// symbol (helperFn/queryName) in the same file is re-exported through the
168+
// barrel. Before the fix, this method landed on `entry` the same way
169+
// `helperFn` incorrectly did; it must now fall through to normal dead-code
170+
// classification instead (no callers, no outgoing calls → dead-unresolved).
171+
const data = rolesData(barrelDbPath);
172+
const methodResult = data.symbols.find((s) => s.name === 'abstractHelper');
173+
expect(methodResult).toBeDefined();
174+
expect(methodResult!.role).toBe('dead-unresolved');
175+
});
159176
});
160177

161178
// ─── Multi-level barrel re-export chain (#837) ───────────────────────

tests/unit/roles.test.ts

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,43 @@ describe('classifyNodeRoles', () => {
235235

236236
classifyNodeRoles(db);
237237
const role = db.prepare("SELECT role FROM nodes WHERE name = 'MyOpts'").get();
238-
// Should be entry (exported, fan-in 0), not dead-unresolved
238+
// Should be leaf (exported, fan-in 0), not dead-unresolved. An interface is a
239+
// data-shape declaration, never a callable entry point, so exported+zero-fanin
240+
// interfaces land on `leaf` rather than `entry` (#1780) — but #1583's original
241+
// guarantee (never `dead-*`) still holds.
242+
expect(role.role).toBe('leaf');
243+
});
244+
245+
it('classifies exported constant with no callers as leaf, not entry (#1780)', () => {
246+
// A module-level constant is a data value, never a callable entry point,
247+
// regardless of its exported=1 flag.
248+
db.prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)').run(
249+
'DEFAULT_OPTIONS',
250+
'constant',
251+
'src/helpers.ts',
252+
5,
253+
1,
254+
);
255+
256+
classifyNodeRoles(db);
257+
const role = db.prepare("SELECT role FROM nodes WHERE name = 'DEFAULT_OPTIONS'").get();
258+
expect(role.role).toBe('leaf');
259+
});
260+
261+
it('still classifies an exported function with no callers as entry (#1780 no over-correction)', () => {
262+
// A genuinely exported, callable function with zero fan-in is exactly what
263+
// `entry` is meant to capture — e.g. a public API function invoked only by
264+
// external consumers of the package. The kind-gate fix must not lose this.
265+
db.prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)').run(
266+
'publicApiFn',
267+
'function',
268+
'src/helpers.ts',
269+
40,
270+
1,
271+
);
272+
273+
classifyNodeRoles(db);
274+
const role = db.prepare("SELECT role FROM nodes WHERE name = 'publicApiFn'").get();
239275
expect(role.role).toBe('entry');
240276
});
241277

@@ -364,7 +400,7 @@ describe('classifyNodeRoles', () => {
364400

365401
it('incremental path: does not classify exported interface as dead when used only as same-file type annotation (#1583)', () => {
366402
// Exercises classifyNodeRolesIncremental (triggered by passing changedFiles).
367-
// An exported=1 interface with no cross-file edges must be promoted to entry,
403+
// An exported=1 interface with no cross-file edges must be promoted to leaf,
368404
// not dead-unresolved, on the incremental path just as on the full path.
369405
db.prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)').run(
370406
'IncrementalOpts',
@@ -377,8 +413,9 @@ describe('classifyNodeRoles', () => {
377413
// Pass the file as the changed-files list to trigger the incremental path.
378414
classifyNodeRoles(db, ['src/helpers.ts']);
379415
const role = db.prepare("SELECT role FROM nodes WHERE name = 'IncrementalOpts'").get();
380-
// Should be entry (exported, fan-in 0), not dead-unresolved
381-
expect(role.role).toBe('entry');
416+
// Should be leaf (exported, fan-in 0), not dead-unresolved or entry — an
417+
// interface is a data-shape declaration, never a callable entry point (#1780).
418+
expect(role.role).toBe('leaf');
382419
});
383420

384421
// ── Parameters and interface members are not dead-code targets (#1723) ──

0 commit comments

Comments
 (0)