diff --git a/crates/codegraph-core/src/graph/classifiers/roles.rs b/crates/codegraph-core/src/graph/classifiers/roles.rs index c45184650..1b45f03bc 100644 --- a/crates/codegraph-core/src/graph/classifiers/roles.rs +++ b/crates/codegraph-core/src/graph/classifiers/roles.rs @@ -106,6 +106,55 @@ fn median(sorted: &[u32]) -> f64 { } } +/// Compute, per file, the set of symbol names that are `TYPE_DEF_KINDS`-kind +/// declarations (interface/type/struct/enum/trait/record). Mirrors JS +/// `computeTypeDefNamesByFile` in `graph/classifiers/roles.ts`. +fn compute_type_def_names_by_file( + rows: &[(i64, String, String, String, u32, u32)], +) -> HashMap> { + let mut by_file: HashMap> = HashMap::new(); + for (_id, name, kind, file, _fan_in, _fan_out) in rows { + if TYPE_DEF_KINDS.iter().any(|k| *k == kind.as_str()) { + by_file + .entry(file.clone()) + .or_default() + .insert(name.clone()); + } + } + by_file +} + +/// True when `name`/`kind` is a `method`/`property` member of an interface/type +/// declared in the same file — e.g. TS `interface Foo { bar: string }` extracts +/// `bar` as a top-level `method`-kind definition named `Foo.bar` (#1723). Every +/// language extractor qualifies interface/type members as `Owner.member` +/// (mirroring class method qualification), so the owner name is recovered from +/// the prefix before the first `.` and looked up against same-file +/// `TYPE_DEF_KINDS` declarations. Class methods use the identical `Owner.member` +/// convention but are unaffected here because `class` is not in `TYPE_DEF_KINDS` +/// — they remain subject to normal dead-code detection. +/// +/// These members can never gain inbound call edges by construction, so a +/// `fan_in == 0` reading carries zero dead-code signal for them, unlike a real +/// function/method where it does. Mirrors JS `isTypeDeclarationMember`. +fn is_type_declaration_member( + name: &str, + kind: &str, + file: &str, + type_def_names_by_file: &HashMap>, +) -> bool { + if kind != "method" && kind != "property" { + return false; + } + let Some(dot_idx) = name.find('.') else { + return false; + }; + let owner_name = &name[..dot_idx]; + type_def_names_by_file + .get(file) + .is_some_and(|names| names.contains(owner_name)) +} + /// Dead sub-role classification matching JS `classifyDeadSubRole`. fn classify_dead_sub_role(_name: &str, kind: &str, file: &str) -> &'static str { // Leaf kinds @@ -126,6 +175,7 @@ fn classify_dead_sub_role(_name: &str, kind: &str, file: &str) -> &'static str { } /// Classify a single node into a role. +#[allow(clippy::too_many_arguments)] fn classify_node( name: &str, kind: &str, @@ -135,9 +185,16 @@ fn classify_node( is_exported: bool, production_fan_in: u32, has_active_file_siblings: bool, + is_type_member: bool, median_fan_in: f64, median_fan_out: f64, ) -> &'static str { + // Interface/type members (#1723) — never subject to call-graph dead-code + // detection, regardless of fan-in/fan-out/export status. + if is_type_member { + return "leaf"; + } + // Framework entry if FRAMEWORK_ENTRY_PREFIXES.iter().any(|p| name.starts_with(p)) { return "entry"; @@ -267,10 +324,14 @@ pub(crate) fn do_classify_full(conn: &Connection) -> rusqlite::Result dead-leaf + // 1. Property kind (class/struct fields) -> dead-leaf. + // `parameter` is deliberately NOT included here (#1723): a parameter's liveness + // is a local dataflow question, not a call-graph reachability question, so + // "no incoming call edges" carries zero dead-code signal for it. Parameters are + // also excluded from the main rows query below, so they never receive a role + // at all — the same treatment as `file`/`directory` nodes. let leaf_ids: Vec = { - let mut stmt = - tx.prepare("SELECT id FROM nodes WHERE kind IN ('parameter', 'property')")?; + let mut stmt = tx.prepare("SELECT id FROM nodes WHERE kind = 'property'")?; let rows = stmt.query_map([], |row| row.get::<_, i64>(0))?; rows.filter_map(|r| r.ok()).collect() }; @@ -408,6 +469,11 @@ pub(crate) fn do_classify_full(conn: &Connection) -> rusqlite::Result> = HashMap::new(); @@ -423,6 +489,7 @@ pub(crate) fn do_classify_full(conn: &Connection) -> rusqlite::Result, prod_fan_in: &HashMap, active_files: &std::collections::HashSet, called_active_files: &std::collections::HashSet, + type_def_names_by_file: &HashMap>, median_fan_in: f64, median_fan_out: f64, ids_by_role: &mut HashMap<&'static str, Vec>, @@ -573,6 +642,7 @@ fn classify_rows( } else { false }; + let is_type_member = is_type_declaration_member(name, kind, file, type_def_names_by_file); let role = classify_node( name, kind, @@ -582,6 +652,7 @@ fn classify_rows( is_exported, prod_fi, has_active_siblings, + is_type_member, median_fan_in, median_fan_out, ); @@ -629,16 +700,15 @@ fn find_neighbour_files( } /// Query leaf kind node IDs and callable node rows for a set of files. +/// `parameter` is intentionally excluded from the leaf query (#1723) — see +/// `do_classify_full`'s leaf_ids comment for the rationale. fn query_nodes_for_files( tx: &rusqlite::Transaction, files: &[&str], ) -> rusqlite::Result<(Vec, Vec<(i64, String, String, String, u32, u32)>)> { let ph: String = files.iter().map(|_| "?").collect::>().join(","); - let leaf_sql = format!( - "SELECT id FROM nodes WHERE kind IN ('parameter', 'property') AND file IN ({})", - ph - ); + let leaf_sql = format!("SELECT id FROM nodes WHERE kind = 'property' AND file IN ({})", ph); let leaf_ids: Vec = { let mut stmt = tx.prepare(&leaf_sql)?; for (i, f) in files.iter().enumerate() { @@ -804,6 +874,9 @@ pub(crate) fn do_classify_incremental( let (active_files, called_active_files) = compute_active_files(&rows); + // Compute interface/type owner names per file (#1723) — see do_classify_full. + let type_def_names_by_file = compute_type_def_names_by_file(&rows); + let mut ids_by_role: HashMap<&str, Vec> = HashMap::new(); if !leaf_ids.is_empty() { @@ -818,6 +891,7 @@ pub(crate) fn do_classify_incremental( &prod_fan_in, &active_files, &called_active_files, + &type_def_names_by_file, median_fan_in, median_fan_out, &mut ids_by_role, diff --git a/src/features/structure.ts b/src/features/structure.ts index 74b88ef4a..85f3afff9 100644 --- a/src/features/structure.ts +++ b/src/features/structure.ts @@ -777,13 +777,20 @@ function writeMedianCache( } function classifyNodeRolesFull(db: BetterSqlite3Database, emptySummary: RoleSummary): RoleSummary { - // Leaf kinds (parameter, property) can never have callers/callees. + // Property kind (class/struct fields) can never have callers/callees. // Classify them directly as dead-leaf without the expensive fan-in/fan-out JOINs. + // + // `parameter` is deliberately NOT included here (#1723): a parameter's liveness + // is a local dataflow question (is it referenced within its own function body), + // not a call-graph reachability question, so "no incoming call edges" carries + // zero dead-code signal for it. Parameters are also excluded from the main + // query below, so they never receive a role at all — the same treatment as + // `file`/`directory` nodes. const leafRows = db .prepare( `SELECT n.id FROM nodes n - WHERE n.kind IN ('parameter', 'property')`, + WHERE n.kind = 'property'`, ) .all() as { id: number }[]; @@ -967,11 +974,13 @@ function classifyNodeRolesIncremental( globalMedians = computed; } - // 2a. Leaf kinds (parameter, property) in affected files — always dead-leaf + // 2a. Property kind (class/struct fields) in affected files — always dead-leaf. + // `parameter` is intentionally excluded (#1723) — see classifyNodeRolesFull for + // the rationale. Parameters stay unclassified (role = NULL) after the reset below. const leafRows = db .prepare( `SELECT n.id FROM nodes n - WHERE n.kind IN ('parameter', 'property') + WHERE n.kind = 'property' AND n.file IN (${placeholders})`, ) .all(...allAffectedFiles) as { id: number }[]; diff --git a/src/graph/classifiers/roles.ts b/src/graph/classifiers/roles.ts index 2b17c33ba..fe278ff31 100644 --- a/src/graph/classifiers/roles.ts +++ b/src/graph/classifiers/roles.ts @@ -4,10 +4,23 @@ * Roles: entry, core, utility, adapter, leaf, dead-*, test-only * * Dead sub-categories refine the coarse "dead" bucket: - * dead-leaf — parameters, properties, constants (leaf nodes by definition) + * dead-leaf — properties, constants (leaf nodes by definition) * dead-entry — framework dispatch: CLI commands, MCP tools, event handlers * dead-ffi — cross-language FFI boundaries (e.g. Rust napi-rs bindings) * dead-unresolved — genuinely unreferenced callables (the real dead code) + * + * `parameter`-kind nodes never reach this module in production — callers + * (`features/structure.ts`, native `graph/classifiers/roles.rs`) exclude them + * entirely, leaving `role` unset, the same treatment as `file`/`directory` + * nodes. A parameter's liveness is a local dataflow question (is it referenced + * within its own function body), not a call-graph reachability question, so + * "no incoming call edges" carries zero dead-code signal for it (#1723). + * + * `method`/`property`-kind members of an interface/type declaration (e.g. + * `interface Foo { bar: string }`) DO reach this module, but are recognized by + * `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). */ import type { DeadSubRole, Role } from '../../types.js'; @@ -53,6 +66,56 @@ export interface ClassifiableNode { file?: string; } +/** + * Compute, per file, the set of symbol names that are `TYPE_DEF_KINDS`-kind + * declarations (interface/type/struct/enum/trait/record). Used by + * `isTypeDeclarationMember` to recognize `Owner.member`-qualified nodes whose + * owner is a type-level declaration rather than a class. + */ +function computeTypeDefNamesByFile(nodes: RoleClassificationNode[]): Map> { + const byFile = new Map>(); + for (const n of nodes) { + if (n.file && n.kind && TYPE_DEF_KINDS.has(n.kind)) { + let names = byFile.get(n.file); + if (!names) { + names = new Set(); + byFile.set(n.file, names); + } + names.add(n.name); + } + } + return byFile; +} + +/** + * True when `node` is a `method`/`property`-kind member of an interface/type + * declared in the same file — e.g. TS `interface Foo { bar: string }` extracts + * `bar` as a top-level `method`-kind definition named `Foo.bar` (#1723). Every + * language extractor qualifies interface/type members as `Owner.member` + * (mirroring class method qualification), so the owner name is recovered from + * the prefix before the first `.` and looked up against same-file + * `TYPE_DEF_KINDS` declarations. Class methods use the identical `Owner.member` + * convention but are unaffected here because `class` is not in `TYPE_DEF_KINDS` + * — they remain subject to normal dead-code detection. + * + * These members can never gain inbound call edges by construction — they are + * consumed via type annotations and structural typing, never calls — so a + * `fanIn === 0` reading carries zero dead-code signal for them, unlike a real + * function/method where it does. Call-edge-based reachability just doesn't + * apply to type-level declarations, so they must never be judged dead by it. + */ +function isTypeDeclarationMember( + node: RoleClassificationNode, + typeDefNamesByFile: Map>, +): boolean { + if (node.kind !== 'method' && node.kind !== 'property') return false; + if (!node.file) return false; + const dotIdx = node.name.indexOf('.'); + if (dotIdx === -1) return false; + const ownerName = node.name.slice(0, dotIdx); + return typeDefNamesByFile.get(node.file)?.has(ownerName) ?? false; +} + /** * Refine a "dead" classification into a sub-category. */ @@ -176,10 +239,21 @@ function classifyByFanShape(highIn: boolean, highOut: boolean): Role { /** * Apply role-classification rules to a single node. - * Order matters — framework entries are tagged first, then dead/test cases, - * then the fan-in/fan-out shape decides among the structural roles. + * Order matters — type-level members are ruled out first (they can never be + * judged by call-graph reachability at all), then framework entries, then + * dead/test cases, then the fan-in/fan-out shape decides among the structural + * roles. */ -function classifyNodeRole(node: RoleClassificationNode, medFanIn: number, medFanOut: number): Role { +function classifyNodeRole( + node: RoleClassificationNode, + medFanIn: number, + medFanOut: number, + typeDefNamesByFile: Map>, +): Role { + // Interface/type members (#1723) — never subject to call-graph dead-code + // detection, regardless of fan-in/fan-out/export status. + if (isTypeDeclarationMember(node, typeDefNamesByFile)) return 'leaf'; + if (FRAMEWORK_ENTRY_PREFIXES.some((p) => node.name.startsWith(p))) return 'entry'; if (node.fanIn === 0) { @@ -217,10 +291,11 @@ export function classifyRoles( if (nodes.length === 0) return new Map(); const { fanIn: medFanIn, fanOut: medFanOut } = medianOverrides ?? computeFanMedians(nodes); + const typeDefNamesByFile = computeTypeDefNamesByFile(nodes); const result = new Map(); for (const node of nodes) { - result.set(node.id, classifyNodeRole(node, medFanIn, medFanOut)); + result.set(node.id, classifyNodeRole(node, medFanIn, medFanOut, typeDefNamesByFile)); } return result; } diff --git a/tests/graph/classifiers/roles.test.ts b/tests/graph/classifiers/roles.test.ts index 03ae894c7..b1de630ee 100644 --- a/tests/graph/classifiers/roles.test.ts +++ b/tests/graph/classifiers/roles.test.ts @@ -515,4 +515,117 @@ describe('classifyRoles', () => { const roles = classifyRoles(nodes); expect(roles.get('1')).toBe('dead-unresolved'); }); + + // ── Interface/type member exemption (#1723) ───────────────────────── + + it('classifies interface method-signature member as leaf, not dead', () => { + // TS `interface Foo { bar(): void }` extracts `bar` as a top-level + // `method`-kind definition named `Foo.bar`. It can never gain an inbound + // call edge (nothing "calls" a type-level declaration), so fanIn === 0 + // carries zero dead-code signal here — unlike a real function/method. + const nodes = [ + { + id: '1', + name: 'Foo', + kind: 'interface', + file: 'src/a.ts', + fanIn: 0, + fanOut: 0, + isExported: false, + }, + { + id: '2', + name: 'Foo.bar', + kind: 'method', + file: 'src/a.ts', + fanIn: 0, + fanOut: 0, + isExported: false, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('2')).toBe('leaf'); + }); + + it('classifies type-alias property-signature member as leaf, not dead', () => { + // TS `type Foo = { bar: string }` — a property-kind member of a `type` owner. + const nodes = [ + { + id: '1', + name: 'Foo', + kind: 'type', + file: 'src/a.ts', + fanIn: 0, + fanOut: 0, + isExported: false, + }, + { + id: '2', + name: 'Foo.bar', + kind: 'property', + file: 'src/a.ts', + fanIn: 0, + fanOut: 0, + isExported: false, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('2')).toBe('leaf'); + }); + + it('classifies interface member as leaf even when the interface is isolated (no active siblings)', () => { + // Unlike bare TYPE_DEF_KINDS nodes (which need hasActiveFileSiblings to + // avoid being marked dead), members are exempt unconditionally — "no call + // edges" is a structural certainty for them, not merely a likelihood. + const nodes = [ + { + id: '1', + name: 'Isolated', + kind: 'interface', + file: 'src/isolated.ts', + fanIn: 0, + fanOut: 0, + isExported: false, + }, + { + id: '2', + name: 'Isolated.onlyMember', + kind: 'method', + file: 'src/isolated.ts', + fanIn: 0, + fanOut: 0, + isExported: false, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('2')).toBe('leaf'); + }); + + it('does not exempt class methods sharing the Owner.member naming convention', () => { + // Class methods are qualified identically to interface members + // (`ClassName.method`), but `class` is not in TYPE_DEF_KINDS — they must + // remain subject to normal dead-code detection. + const nodes = [ + { + id: '1', + name: 'Foo', + kind: 'class', + file: 'src/a.ts', + fanIn: 5, + fanOut: 0, + isExported: true, + }, + { + id: '2', + name: 'Foo.deadMethod', + kind: 'method', + file: 'src/a.ts', + fanIn: 0, + fanOut: 0, + isExported: false, + }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('2')).toBe('dead-unresolved'); + }); }); diff --git a/tests/unit/roles.test.ts b/tests/unit/roles.test.ts index 110e1c7bd..88d5ee1e0 100644 --- a/tests/unit/roles.test.ts +++ b/tests/unit/roles.test.ts @@ -380,4 +380,73 @@ describe('classifyNodeRoles', () => { // Should be entry (exported, fan-in 0), not dead-unresolved expect(role.role).toBe('entry'); }); + + // ── 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'); + }); });