diff --git a/crates/codegraph-core/src/graph/classifiers/roles.rs b/crates/codegraph-core/src/graph/classifiers/roles.rs index 9de3a07a..35fbe68a 100644 --- a/crates/codegraph-core/src/graph/classifiers/roles.rs +++ b/crates/codegraph-core/src/graph/classifiers/roles.rs @@ -256,7 +256,20 @@ fn classify_node( } if fan_in == 0 && is_exported { - return "entry"; + // Exported, zero fan-in. A genuine entry point (CLI command handler, + // exported API function called from outside the codebase, ESM loader + // hook, MCP tool handler, etc.) is always a function or method. Every + // other exported kind (interface/type/constant/class) is a live, + // intentional part of the public surface — but a data shape or config + // value, not something invoked from outside the codebase — so it's + // `leaf`: never `dead-*` (#1583) and never `entry` (#1780), regardless + // of whether the file has other active siblings. Mirrors JS + // `classifyNodeRole`. + return if kind == "function" || kind == "method" { + "entry" + } else { + "leaf" + }; } // Test-only: has callers but all are in test files @@ -403,6 +416,15 @@ pub(crate) fn do_classify_full(conn: &Connection) -> rusqlite::Result rusqlite::Result = Vec::with_capacity(barrels.len()); @@ -969,7 +993,7 @@ pub(crate) fn do_classify_incremental( JOIN edges e ON e.target_id = f.id JOIN nodes b ON e.source_id = b.id WHERE e.kind = 'reexports' AND b.file IN ({barrel_ph}) - AND n.kind NOT IN ('file', 'directory', 'parameter', 'property') + AND n.kind NOT IN ('file', 'directory', 'parameter', 'property', 'method') AND n.file IN ({affected_ph})" ); let mut stmt = tx.prepare(&sql)?; diff --git a/src/features/structure.ts b/src/features/structure.ts index 81a4838f..8c8b2002 100644 --- a/src/features/structure.ts +++ b/src/features/structure.ts @@ -829,6 +829,15 @@ function classifyNodeRolesFull(db: BetterSqlite3Database, emptySummary: RoleSumm // Mark symbols as exported when their files are targets of reexports edges // from production-reachable barrels (traces through multi-level chains) (#837) + // + // `method` is excluded (#1780): a `reexports` edge only ever concerns + // top-level module bindings (functions, classes, types, constants, ...) — a + // class/interface method can never be an independently re-exportable + // binding on its own, so inheriting "exported" status from a co-located + // top-level re-export is a category error. Without this exclusion, e.g. an + // abstract base class's zero-fan-in method declarations were promoted to + // `entry` merely because some other symbol in the same file was re-exported + // through a barrel. const reexportExported = db .prepare( `WITH RECURSIVE prod_reachable(file_id) AS ( @@ -852,7 +861,7 @@ function classifyNodeRolesFull(db: BetterSqlite3Database, emptySummary: RoleSumm WHERE e.kind = 'reexports' AND e.source_id IN (SELECT file_id FROM prod_reachable) ) - AND n.kind NOT IN ('file', 'directory', 'parameter', 'property')`, + AND n.kind NOT IN ('file', 'directory', 'parameter', 'property', 'method')`, ) .all() as { id: number }[]; for (const r of reexportExported) exportedIds.add(r.id); @@ -1127,6 +1136,8 @@ function classifyNodeRolesIncremental( // 1-3 files), then answer "is this specific barrel production-reachable" // with a backward-scoped check instead of the whole-graph forward closure // (see `isBarrelProdReachable`). + // + // `method` is excluded (#1780) — see classifyNodeRolesFull for rationale. const reexportBarrels = findDirectReexportBarrels(db, allAffectedFiles); const reachableBarrels = reexportBarrels.filter((b) => isBarrelProdReachable(db, b)); if (reachableBarrels.length > 0) { @@ -1139,7 +1150,7 @@ function classifyNodeRolesIncremental( JOIN edges e ON e.target_id = f.id JOIN nodes b ON e.source_id = b.id WHERE e.kind = 'reexports' AND b.file IN (${barrelPlaceholders}) - AND n.kind NOT IN ('file', 'directory', 'parameter', 'property') + AND n.kind NOT IN ('file', 'directory', 'parameter', 'property', 'method') AND n.file IN (${placeholders})`, ) .all(...reachableBarrels, ...allAffectedFiles) as { id: number }[]; diff --git a/src/graph/classifiers/roles.ts b/src/graph/classifiers/roles.ts index 632037aa..9511ab73 100644 --- a/src/graph/classifiers/roles.ts +++ b/src/graph/classifiers/roles.ts @@ -21,6 +21,13 @@ * `isTypeDeclarationMember` and classified `leaf` unconditionally — they can * never gain inbound call edges by construction, so call-graph reachability * doesn't apply to them either (#1723). + * + * `entry` requires `kind IN ('function', 'method')` (plus the framework-prefix/ + * Commander-dispatch shortcuts, which are already kind-appropriate by + * construction). An exported interface/type/constant/class with zero fan-in is + * a data-shape declaration or config value — never invoked from outside the + * codebase — so it can't be a real entry point; it's classified `leaf` instead + * of inheriting `entry` merely from being exported (#1780). */ import type { DeadSubRole, Role } from '../../types.js'; @@ -149,7 +156,7 @@ export function median(sorted: number[]): number { export interface RoleClassificationNode { id: string; name: string; - kind?: string; + kind: string; file?: string; fanIn: number; fanOut: number; @@ -281,7 +288,14 @@ function classifyNodeRole( } return classifyUnreferencedNode(node); } - return 'entry'; + // Exported, zero fan-in. A genuine entry point (CLI command handler, exported + // API function called from outside the codebase, ESM loader hook, MCP tool + // handler, etc.) is always a function or method. Every other exported kind + // (interface/type/constant/class) is a live, intentional part of the public + // surface — but a data shape or config value, not something invoked from + // outside the codebase — so it's `leaf`: never `dead-*` (#1583) and never + // `entry` (#1780), regardless of whether the file has other active siblings. + return node.kind === 'function' || node.kind === 'method' ? 'entry' : 'leaf'; } const hasProdFanIn = typeof node.productionFanIn === 'number'; diff --git a/tests/graph/classifiers/roles.test.ts b/tests/graph/classifiers/roles.test.ts index b1de630e..9a036ec3 100644 --- a/tests/graph/classifiers/roles.test.ts +++ b/tests/graph/classifiers/roles.test.ts @@ -6,8 +6,10 @@ describe('classifyRoles', () => { expect(classifyRoles([]).size).toBe(0); }); - it('classifies entry nodes (no fan-in, exported)', () => { - const nodes = [{ id: '1', name: 'init', fanIn: 0, fanOut: 3, isExported: true }]; + it('classifies entry nodes (no fan-in, exported, function kind)', () => { + const nodes = [ + { id: '1', name: 'init', kind: 'function', fanIn: 0, fanOut: 3, isExported: true }, + ]; const roles = classifyRoles(nodes); expect(roles.get('1')).toBe('entry'); }); @@ -628,4 +630,140 @@ describe('classifyRoles', () => { const roles = classifyRoles(nodes); expect(roles.get('2')).toBe('dead-unresolved'); }); + + // ── entry role requires function/method kind (#1780) ─────────────── + + it('classifies an exported interface with zero fan-in as leaf, not entry', () => { + // Mirrors the #1780 repro: `interface ParsedUserConfig { ... }` in + // src/infrastructure/config.ts. Even if the symbol were exported, an + // interface is a data-shape declaration, never a callable entry point. + const nodes = [ + { + id: '1', + name: 'ParsedUserConfig', + kind: 'interface', + file: 'src/infrastructure/config.ts', + fanIn: 0, + fanOut: 0, + isExported: true, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('leaf'); + }); + + it('classifies an exported constant with zero fan-in as leaf, not entry', () => { + // Mirrors the #1780 repro: module-level `const BUILD_HASH_KEYS = [...]`. + // A constant can never be an invoked entry point regardless of export status. + const nodes = [ + { + id: '1', + name: 'BUILD_HASH_KEYS', + kind: 'constant', + file: 'src/infrastructure/config.ts', + fanIn: 0, + fanOut: 0, + isExported: true, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('leaf'); + }); + + it('classifies an exported class with zero fan-in as leaf, not entry', () => { + // A class declaration itself is instantiated via `new`, not "invoked" the + // way a CLI command handler or API function is — not a real entry point. + const nodes = [ + { + id: '1', + name: 'Widget', + kind: 'class', + file: 'src/widget.ts', + fanIn: 0, + fanOut: 0, + isExported: true, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('leaf'); + }); + + it('classifies an exported type alias with zero fan-in as leaf, not entry', () => { + const nodes = [ + { + id: '1', + name: 'WorkspaceEntry', + kind: 'type', + file: 'src/infrastructure/config.ts', + fanIn: 0, + fanOut: 0, + isExported: true, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('leaf'); + }); + + it('still classifies an exported function with zero fan-in as entry (no over-correction)', () => { + // Genuine entry points (CLI handlers, MCP tool handlers, ESM loader hooks) + // are, by definition, called from outside the codebase, so zero in-repo + // fan-in is expected and correct for them — the fix must not lose this. + const nodes = [ + { + id: '1', + name: 'handler', + kind: 'function', + file: 'src/mcp/tools/audit.ts', + fanIn: 0, + fanOut: 4, + isExported: true, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('entry'); + }); + + it('still classifies an exported method with zero fan-in as entry (no over-correction)', () => { + const nodes = [ + { + id: '1', + name: 'run', + kind: 'method', + file: 'src/cli/commands/custom.ts', + fanIn: 0, + fanOut: 2, + isExported: true, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('entry'); + }); + + it('classifies a non-exported interface with zero fan-in the same as an exported one (leaf, active siblings)', () => { + // Whether or not the interface itself is exported, it's still not a + // callable entry point — both must land on the same non-entry path. + const nodes = [ + { + id: '1', + name: 'ConsentResolutionResult', + kind: 'interface', + file: 'src/infrastructure/config.ts', + fanIn: 0, + fanOut: 0, + isExported: false, + hasActiveFileSiblings: true, + }, + { + id: '2', + name: 'loadConfig', + kind: 'function', + file: 'src/infrastructure/config.ts', + fanIn: 5, + fanOut: 3, + isExported: true, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('leaf'); + }); }); diff --git a/tests/integration/roles.test.ts b/tests/integration/roles.test.ts index b5f3a8a5..5d715c5d 100644 --- a/tests/integration/roles.test.ts +++ b/tests/integration/roles.test.ts @@ -115,6 +115,9 @@ describe('barrel re-export role classification', () => { const _helperFn = insertNode(db, 'helperFn', 'function', 'src/inspect.ts', 30); const _appMain = insertNode(db, 'appMain', 'function', 'src/app.ts', 1); const testFn = insertNode(db, 'testQueryName', 'function', 'tests/inspect.test.ts', 1); + // A class-method-kind member (e.g. an abstract base-class method) in the same + // re-exported file, with no callers and no outgoing calls (#1780). + const _abstractHelper = insertNode(db, 'abstractHelper', 'method', 'src/inspect.ts', 50); // Barrel re-exports inspect.ts insertEdge(db, fBarrel, fInspect, 'reexports'); @@ -156,6 +159,20 @@ describe('barrel re-export role classification', () => { // With fanIn=0 and isExported=true → entry (exported but uncalled) expect(helperResult!.role).toBe('entry'); }); + + test('method-kind member in a re-exported file does not inherit exported status (#1780)', () => { + // A `reexports` edge only ever concerns top-level module bindings — a class + // method can never be an independently re-exportable binding on its own, so + // it must not inherit "exported" status merely because a sibling top-level + // symbol (helperFn/queryName) in the same file is re-exported through the + // barrel. Before the fix, this method landed on `entry` the same way + // `helperFn` incorrectly did; it must now fall through to normal dead-code + // classification instead (no callers, no outgoing calls → dead-unresolved). + const data = rolesData(barrelDbPath); + const methodResult = data.symbols.find((s) => s.name === 'abstractHelper'); + expect(methodResult).toBeDefined(); + expect(methodResult!.role).toBe('dead-unresolved'); + }); }); // ─── Multi-level barrel re-export chain (#837) ─────────────────────── diff --git a/tests/unit/roles.test.ts b/tests/unit/roles.test.ts index 88d5ee1e..9b24cb5f 100644 --- a/tests/unit/roles.test.ts +++ b/tests/unit/roles.test.ts @@ -235,7 +235,43 @@ describe('classifyNodeRoles', () => { classifyNodeRoles(db); const role = db.prepare("SELECT role FROM nodes WHERE name = 'MyOpts'").get(); - // Should be entry (exported, fan-in 0), not dead-unresolved + // Should be leaf (exported, fan-in 0), not dead-unresolved. An interface is a + // data-shape declaration, never a callable entry point, so exported+zero-fanin + // interfaces land on `leaf` rather than `entry` (#1780) — but #1583's original + // guarantee (never `dead-*`) still holds. + expect(role.role).toBe('leaf'); + }); + + it('classifies exported constant with no callers as leaf, not entry (#1780)', () => { + // A module-level constant is a data value, never a callable entry point, + // regardless of its exported=1 flag. + db.prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)').run( + 'DEFAULT_OPTIONS', + 'constant', + 'src/helpers.ts', + 5, + 1, + ); + + classifyNodeRoles(db); + const role = db.prepare("SELECT role FROM nodes WHERE name = 'DEFAULT_OPTIONS'").get(); + expect(role.role).toBe('leaf'); + }); + + it('still classifies an exported function with no callers as entry (#1780 no over-correction)', () => { + // A genuinely exported, callable function with zero fan-in is exactly what + // `entry` is meant to capture — e.g. a public API function invoked only by + // external consumers of the package. The kind-gate fix must not lose this. + db.prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)').run( + 'publicApiFn', + 'function', + 'src/helpers.ts', + 40, + 1, + ); + + classifyNodeRoles(db); + const role = db.prepare("SELECT role FROM nodes WHERE name = 'publicApiFn'").get(); expect(role.role).toBe('entry'); }); @@ -364,7 +400,7 @@ describe('classifyNodeRoles', () => { it('incremental path: does not classify exported interface as dead when used only as same-file type annotation (#1583)', () => { // Exercises classifyNodeRolesIncremental (triggered by passing changedFiles). - // An exported=1 interface with no cross-file edges must be promoted to entry, + // An exported=1 interface with no cross-file edges must be promoted to leaf, // not dead-unresolved, on the incremental path just as on the full path. db.prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)').run( 'IncrementalOpts', @@ -377,8 +413,78 @@ describe('classifyNodeRoles', () => { // Pass the file as the changed-files list to trigger the incremental path. classifyNodeRoles(db, ['src/helpers.ts']); const role = db.prepare("SELECT role FROM nodes WHERE name = 'IncrementalOpts'").get(); - // Should be entry (exported, fan-in 0), not dead-unresolved - expect(role.role).toBe('entry'); + // Should be leaf (exported, fan-in 0), not dead-unresolved or entry — an + // interface is a data-shape declaration, never a callable entry point (#1780). + expect(role.role).toBe('leaf'); + }); + + // ── Parameters and interface members are not dead-code targets (#1723) ── + + it('excludes parameter-kind nodes from role classification entirely', () => { + // A parameter's liveness is a local dataflow question (is it referenced + // within its own function body), not a call-graph reachability question. + // "No incoming call edges" is guaranteed for every parameter regardless of + // usage, so parameters must never receive a `dead-*` role — or any role at + // all, the same treatment as file/directory nodes. + insertNode('src/helpers.ts', 'file', 'src/helpers.ts', 0); + const fn = insertNode('findChild', 'function', 'src/helpers.ts', 26); + insertNode('node', 'parameter', 'src/helpers.ts', 26); + insertNode('type', 'parameter', 'src/helpers.ts', 26); + // An external caller so findChild itself is not incidentally dead too — + // isolates the assertion to parameter handling specifically. + const caller = insertNode('caller', 'function', 'other.ts', 1); + insertEdge(caller, fn, 'calls'); + + classifyNodeRoles(db); + + const paramRoles = db.prepare("SELECT role FROM nodes WHERE kind = 'parameter'").all() as { + role: string | null; + }[]; + expect(paramRoles).toHaveLength(2); + for (const row of paramRoles) { + expect(row.role).toBeNull(); + } + }); + + it('does not classify interface members as dead', () => { + // TS `interface ExtractParametersOptions { typeMap?: Map<...> }` extracts + // `typeMap` as a top-level `method`-kind definition named + // `ExtractParametersOptions.typeMap` (property-signature members are + // currently mislabeled `method` by the extractor — tracked separately). + // It has no callers by construction; it must not be flagged dead. + insertNode('src/helpers.ts', 'file', 'src/helpers.ts', 0); + insertNode('ExtractParametersOptions', 'interface', 'src/helpers.ts', 359); + insertNode('ExtractParametersOptions.typeMap', 'method', 'src/helpers.ts', 361); + + classifyNodeRoles(db); + + const role = db + .prepare("SELECT role FROM nodes WHERE name = 'ExtractParametersOptions.typeMap'") + .get() as { role: string | null }; + expect(role.role).toBe('leaf'); + }); + + it('regression: used parameters, interface members, and a genuinely dead function are classified correctly together', () => { + // Mirrors the exact issue #1723 repro: `findChild(node, type)` in + // src/extractors/helpers.ts, alongside an interface and a truly dead function. + insertNode('src/helpers.ts', 'file', 'src/helpers.ts', 0); + const findChildFn = insertNode('findChild', 'function', 'src/helpers.ts', 26); + insertNode('node', 'parameter', 'src/helpers.ts', 26); + insertNode('type', 'parameter', 'src/helpers.ts', 26); + insertNode('ChaContext', 'interface', 'src/helpers.ts', 18); + insertNode('ChaContext.implementors', 'method', 'src/helpers.ts', 20); + insertNode('trulyDeadHelper', 'function', 'src/helpers.ts', 400); + + const caller = insertNode('caller', 'function', 'other.ts', 1); + insertEdge(caller, findChildFn, 'calls'); + + classifyNodeRoles(db); + const getRole = (name) => db.prepare('SELECT role FROM nodes WHERE name = ?').get(name)?.role; + + expect(getRole('node')).toBeNull(); + expect(getRole('type')).toBeNull(); + expect(getRole('ChaContext.implementors')).toBe('leaf'); + expect(getRole('trulyDeadHelper')).toBe('dead-unresolved'); }); // ── Parameters and interface members are not dead-code targets (#1723) ──