From a196f3c3387c0971e0029e5dab38f08eea4c02e3 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 17 Mar 2026 05:02:02 -0600 Subject: [PATCH 1/6] fix: classify test-only-called symbols as 'test-only' instead of 'dead' (#488) Impact: 4 functions changed, 4 affected --- src/cli/commands/roles.js | 3 +- src/domain/analysis/roles.js | 72 ++++++++++++++++++++++++++- src/features/structure.js | 4 +- src/graph/classifiers/roles.js | 6 +-- src/mcp/tool-registry.js | 2 +- src/shared/kinds.js | 2 +- tests/graph/classifiers/roles.test.js | 25 ++++++++++ tests/integration/roles.test.js | 34 +++++++++++++ tests/unit/roles.test.js | 10 +++- 9 files changed, 147 insertions(+), 11 deletions(-) diff --git a/src/cli/commands/roles.js b/src/cli/commands/roles.js index df756333..cb5a66c1 100644 --- a/src/cli/commands/roles.js +++ b/src/cli/commands/roles.js @@ -3,7 +3,8 @@ import { roles } from '../../presentation/queries-cli.js'; export const command = { name: 'roles', - description: 'Show node role classification: entry, core, utility, adapter, dead, leaf', + description: + 'Show node role classification: entry, core, utility, adapter, dead, test-only, leaf', options: [ ['-d, --db ', 'Path to graph.db'], ['--role ', `Filter by role (${VALID_ROLES.join(', ')})`], diff --git a/src/domain/analysis/roles.js b/src/domain/analysis/roles.js index a54362fd..9aa27656 100644 --- a/src/domain/analysis/roles.js +++ b/src/domain/analysis/roles.js @@ -13,9 +13,16 @@ export function rolesData(customDbPath, opts = {}) { const conditions = ['role IS NOT NULL']; const params = []; - if (filterRole) { + // When noTests + filtering for 'dead', also include 'test-only' candidates + // (they are stored as non-dead roles but may need reclassification) + if (filterRole && filterRole !== 'test-only') { conditions.push('role = ?'); params.push(filterRole); + } else if (filterRole === 'test-only') { + // test-only is not stored in DB; we need all symbols to reclassify + // Fetch everything and filter after reclassification + conditions.length = 1; // keep only 'role IS NOT NULL' + params.length = 0; } if (filterFile) { conditions.push('file LIKE ?'); @@ -28,7 +35,25 @@ export function rolesData(customDbPath, opts = {}) { ) .all(...params); - if (noTests) rows = rows.filter((r) => !isTestFile(r.file)); + if (noTests) { + rows = rows.filter((r) => !isTestFile(r.file)); + + // Reclassify symbols whose only callers are in test files as 'test-only'. + // A symbol that has fanIn > 0 at build time (all edges) but fanIn === 0 + // when test-file callers are excluded should be 'test-only' instead of + // whatever role it was assigned with the full graph. + const testOnlyIds = _findTestOnlyCalledIds(db); + for (const r of rows) { + if (testOnlyIds.has(`${r.name}:${r.file}:${r.line}`)) { + r.role = 'test-only'; + } + } + } + + // If we were asked for a specific role, filter now (after reclassification) + if (filterRole) { + rows = rows.filter((r) => r.role === filterRole); + } const summary = {}; for (const r of rows) { @@ -43,3 +68,46 @@ export function rolesData(customDbPath, opts = {}) { db.close(); } } + +/** + * Find node keys (name:file:line) for symbols whose callers are ALL in test files. + * These symbols have fanIn > 0 in the full graph but would have fanIn === 0 + * if test-file edges were excluded. + */ +function _findTestOnlyCalledIds(db) { + // Get all non-test symbols that have at least one caller + const rows = db + .prepare( + `SELECT target.name, target.file, target.line, + caller.file AS caller_file + FROM edges e + JOIN nodes target ON e.target_id = target.id + JOIN nodes caller ON e.source_id = caller.id + WHERE e.kind = 'calls' + AND target.kind NOT IN ('file', 'directory')`, + ) + .all(); + + // Group callers by target symbol + const callersByTarget = new Map(); + for (const r of rows) { + const key = `${r.name}:${r.file}:${r.line}`; + if (!callersByTarget.has(key)) + callersByTarget.set(key, { hasTestCaller: false, hasNonTestCaller: false }); + const entry = callersByTarget.get(key); + if (isTestFile(r.caller_file)) { + entry.hasTestCaller = true; + } else { + entry.hasNonTestCaller = true; + } + } + + // Return keys where ALL callers are in test files + const result = new Set(); + for (const [key, entry] of callersByTarget) { + if (entry.hasTestCaller && !entry.hasNonTestCaller) { + result.add(key); + } + } + return result; +} diff --git a/src/features/structure.js b/src/features/structure.js index 4ba9ee0a..5c91a2d8 100644 --- a/src/features/structure.js +++ b/src/features/structure.js @@ -335,7 +335,7 @@ export function classifyNodeRoles(db) { .all(); if (rows.length === 0) { - return { entry: 0, core: 0, utility: 0, adapter: 0, dead: 0, leaf: 0 }; + return { entry: 0, core: 0, utility: 0, adapter: 0, dead: 0, 'test-only': 0, leaf: 0 }; } const exportedIds = new Set( @@ -363,7 +363,7 @@ export function classifyNodeRoles(db) { const roleMap = classifyRoles(classifierInput); // Build summary and updates - const summary = { entry: 0, core: 0, utility: 0, adapter: 0, dead: 0, leaf: 0 }; + const summary = { entry: 0, core: 0, utility: 0, adapter: 0, dead: 0, 'test-only': 0, leaf: 0 }; const updates = []; for (const row of rows) { const role = roleMap.get(String(row.id)) || 'leaf'; diff --git a/src/graph/classifiers/roles.js b/src/graph/classifiers/roles.js index 394197d3..6c30860a 100644 --- a/src/graph/classifiers/roles.js +++ b/src/graph/classifiers/roles.js @@ -1,7 +1,7 @@ /** * Node role classification — pure logic, no DB. * - * Roles: entry, core, utility, adapter, leaf, dead + * Roles: entry, core, utility, adapter, leaf, dead, test-only */ export const FRAMEWORK_ENTRY_PREFIXES = ['route:', 'event:', 'command:']; @@ -15,7 +15,7 @@ function median(sorted) { /** * Classify nodes into architectural roles based on fan-in/fan-out metrics. * - * @param {{ id: string, name: string, fanIn: number, fanOut: number, isExported: boolean }[]} nodes + * @param {{ id: string, name: string, fanIn: number, fanOut: number, isExported: boolean, testOnlyFanIn?: number }[]} nodes * @returns {Map} nodeId → role */ export function classifyRoles(nodes) { @@ -44,7 +44,7 @@ export function classifyRoles(nodes) { if (isFrameworkEntry) { role = 'entry'; } else if (node.fanIn === 0 && !node.isExported) { - role = 'dead'; + role = node.testOnlyFanIn > 0 ? 'test-only' : 'dead'; } else if (node.fanIn === 0 && node.isExported) { role = 'entry'; } else if (highIn && !highOut) { diff --git a/src/mcp/tool-registry.js b/src/mcp/tool-registry.js index c81baee8..08a2d260 100644 --- a/src/mcp/tool-registry.js +++ b/src/mcp/tool-registry.js @@ -362,7 +362,7 @@ const BASE_TOOLS = [ { name: 'node_roles', description: - 'Show node role classification (entry, core, utility, adapter, dead, leaf) based on connectivity patterns', + 'Show node role classification (entry, core, utility, adapter, dead, test-only, leaf) based on connectivity patterns', inputSchema: { type: 'object', properties: { diff --git a/src/shared/kinds.js b/src/shared/kinds.js index 3f469c43..498ad210 100644 --- a/src/shared/kinds.js +++ b/src/shared/kinds.js @@ -47,4 +47,4 @@ export const STRUCTURAL_EDGE_KINDS = ['parameter_of', 'receiver']; // Full set for MCP enum and validation export const EVERY_EDGE_KIND = [...CORE_EDGE_KINDS, ...STRUCTURAL_EDGE_KINDS]; -export const VALID_ROLES = ['entry', 'core', 'utility', 'adapter', 'dead', 'leaf']; +export const VALID_ROLES = ['entry', 'core', 'utility', 'adapter', 'dead', 'test-only', 'leaf']; diff --git a/tests/graph/classifiers/roles.test.js b/tests/graph/classifiers/roles.test.js index b790996c..e76cc539 100644 --- a/tests/graph/classifiers/roles.test.js +++ b/tests/graph/classifiers/roles.test.js @@ -60,4 +60,29 @@ describe('classifyRoles', () => { const roles = classifyRoles(nodes); expect(roles.get('1')).toBe('leaf'); }); + + it('classifies test-only when fanIn is 0 but testOnlyFanIn > 0', () => { + const nodes = [ + { id: '1', name: 'helperForTests', fanIn: 0, fanOut: 0, isExported: false, testOnlyFanIn: 3 }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('test-only'); + }); + + it('classifies dead when fanIn is 0 and testOnlyFanIn is 0', () => { + const nodes = [ + { id: '1', name: 'reallyDead', fanIn: 0, fanOut: 0, isExported: false, testOnlyFanIn: 0 }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('dead'); + }); + + it('ignores testOnlyFanIn when fanIn > 0', () => { + const nodes = [ + { id: '1', name: 'normalLeaf', fanIn: 1, fanOut: 0, isExported: false, testOnlyFanIn: 2 }, + { id: '2', name: 'hub', fanIn: 10, fanOut: 10, isExported: true }, + ]; + const roles = classifyRoles(nodes); + expect(roles.get('1')).toBe('leaf'); + }); }); diff --git a/tests/integration/roles.test.js b/tests/integration/roles.test.js index 2a92b16a..04dca9b1 100644 --- a/tests/integration/roles.test.js +++ b/tests/integration/roles.test.js @@ -60,6 +60,7 @@ beforeAll(() => { const helper = insertNode(db, 'helper', 'function', 'lib.js', 1); const format = insertNode(db, 'format', 'function', 'lib.js', 10); insertNode(db, 'unused', 'function', 'lib.js', 20); + const testHelper = insertNode(db, 'testHelper', 'function', 'lib.js', 30); const testFn = insertNode(db, 'testMain', 'function', 'app.test.js', 1); // Import edges @@ -72,11 +73,13 @@ beforeAll(() => { // processData → format (cross-file) → makes format exported // helper → format (same file) // testFn → main (cross-file) → makes main exported + // testFn → testHelper (cross-file) → testHelper only called from test insertEdge(db, main, process_, 'calls'); insertEdge(db, main, helper, 'calls'); insertEdge(db, process_, format, 'calls'); insertEdge(db, helper, format, 'calls'); insertEdge(db, testFn, main, 'calls'); + insertEdge(db, testFn, testHelper, 'calls'); // unused has no callers and no cross-file callers → dead @@ -133,6 +136,37 @@ describe('rolesData', () => { expect(s.file).not.toMatch(/\.test\./); } }); + + test('reclassifies test-only-called symbols when noTests is true', () => { + const data = rolesData(dbPath, { noTests: true }); + const th = data.symbols.find((s) => s.name === 'testHelper'); + expect(th).toBeDefined(); + expect(th.role).toBe('test-only'); + }); + + test('does not reclassify test-only-called symbols when noTests is false', () => { + const data = rolesData(dbPath); + const th = data.symbols.find((s) => s.name === 'testHelper'); + expect(th).toBeDefined(); + expect(th.role).not.toBe('test-only'); + }); + + test('filters by role=test-only with noTests', () => { + const data = rolesData(dbPath, { noTests: true, role: 'test-only' }); + expect(data.count).toBeGreaterThan(0); + for (const s of data.symbols) { + expect(s.role).toBe('test-only'); + } + const names = data.symbols.map((s) => s.name); + expect(names).toContain('testHelper'); + }); + + test('unused symbol stays dead with noTests', () => { + const data = rolesData(dbPath, { noTests: true }); + const unused = data.symbols.find((s) => s.name === 'unused'); + expect(unused).toBeDefined(); + expect(unused.role).toBe('dead'); + }); }); // ─── statsData includes roles ─────────────────────────────────────────── diff --git a/tests/unit/roles.test.js b/tests/unit/roles.test.js index b703f899..1e8702a2 100644 --- a/tests/unit/roles.test.js +++ b/tests/unit/roles.test.js @@ -151,7 +151,15 @@ describe('classifyNodeRoles', () => { it('handles empty graph without crashing', () => { const summary = classifyNodeRoles(db); - expect(summary).toEqual({ entry: 0, core: 0, utility: 0, adapter: 0, dead: 0, leaf: 0 }); + expect(summary).toEqual({ + entry: 0, + core: 0, + utility: 0, + adapter: 0, + dead: 0, + 'test-only': 0, + leaf: 0, + }); }); it('adapts median thresholds to data', () => { From f4c817692fbfae15a69e832860a143f3c079913b Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 17 Mar 2026 05:21:59 -0600 Subject: [PATCH 2/6] fix: run test-only reclassification when filterRole is test-only Previously, `--role test-only` without `-T` silently returned empty because the reclassification block only ran when `noTests` was true. Now it also runs when the user explicitly requests the test-only role. Impact: 1 functions changed, 0 affected --- src/domain/analysis/roles.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/domain/analysis/roles.js b/src/domain/analysis/roles.js index 9aa27656..a4de5bd7 100644 --- a/src/domain/analysis/roles.js +++ b/src/domain/analysis/roles.js @@ -37,7 +37,9 @@ export function rolesData(customDbPath, opts = {}) { if (noTests) { rows = rows.filter((r) => !isTestFile(r.file)); + } + if (noTests || filterRole === 'test-only') { // Reclassify symbols whose only callers are in test files as 'test-only'. // A symbol that has fanIn > 0 at build time (all edges) but fanIn === 0 // when test-file callers are excluded should be 'test-only' instead of From 0f430abdb16af41663a8b42bf89459eb2467cf98 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 17 Mar 2026 05:22:14 -0600 Subject: [PATCH 3/6] fix: remove redundant array reset in rolesData The `conditions.length = 1; params.length = 0;` lines were no-ops since nothing had modified these arrays at that point in the code. Impact: 1 functions changed, 0 affected --- src/domain/analysis/roles.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/domain/analysis/roles.js b/src/domain/analysis/roles.js index a4de5bd7..7af6b5fb 100644 --- a/src/domain/analysis/roles.js +++ b/src/domain/analysis/roles.js @@ -21,8 +21,6 @@ export function rolesData(customDbPath, opts = {}) { } else if (filterRole === 'test-only') { // test-only is not stored in DB; we need all symbols to reclassify // Fetch everything and filter after reclassification - conditions.length = 1; // keep only 'role IS NOT NULL' - params.length = 0; } if (filterFile) { conditions.push('file LIKE ?'); From f199a687b80c6179accc9c1ec0c5d13c780ad0f3 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 17 Mar 2026 05:22:38 -0600 Subject: [PATCH 4/6] fix: use null separator in dedup key to avoid prefix collision The composite key `name:file:line` used `:` as separator which could collide with framework entry prefixes like `route:` or `event:`. Switch to `\0` (null character) which cannot appear in names or paths. Impact: 2 functions changed, 1 affected --- src/domain/analysis/roles.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/domain/analysis/roles.js b/src/domain/analysis/roles.js index 7af6b5fb..aea8fe66 100644 --- a/src/domain/analysis/roles.js +++ b/src/domain/analysis/roles.js @@ -44,7 +44,7 @@ export function rolesData(customDbPath, opts = {}) { // whatever role it was assigned with the full graph. const testOnlyIds = _findTestOnlyCalledIds(db); for (const r of rows) { - if (testOnlyIds.has(`${r.name}:${r.file}:${r.line}`)) { + if (testOnlyIds.has(`${r.name}\0${r.file}\0${r.line}`)) { r.role = 'test-only'; } } @@ -70,7 +70,7 @@ export function rolesData(customDbPath, opts = {}) { } /** - * Find node keys (name:file:line) for symbols whose callers are ALL in test files. + * Find node keys (name\0file\0line) for symbols whose callers are ALL in test files. * These symbols have fanIn > 0 in the full graph but would have fanIn === 0 * if test-file edges were excluded. */ @@ -91,7 +91,7 @@ function _findTestOnlyCalledIds(db) { // Group callers by target symbol const callersByTarget = new Map(); for (const r of rows) { - const key = `${r.name}:${r.file}:${r.line}`; + const key = `${r.name}\0${r.file}\0${r.line}`; if (!callersByTarget.has(key)) callersByTarget.set(key, { hasTestCaller: false, hasNonTestCaller: false }); const entry = callersByTarget.get(key); From e0afd0fa0833e8b214e544eb699c40522ce38422 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 17 Mar 2026 05:38:07 -0600 Subject: [PATCH 5/6] fix: warn when graph lacks test-file nodes for test-only role detection Add a guard in _findTestOnlyCalledIds that checks whether any test-file nodes exist in the DB before querying. When absent (e.g. graph built with -T), emits a warning instead of silently returning empty results. Impact: 1 functions changed, 3 affected --- src/domain/analysis/roles.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/domain/analysis/roles.js b/src/domain/analysis/roles.js index aea8fe66..67df3413 100644 --- a/src/domain/analysis/roles.js +++ b/src/domain/analysis/roles.js @@ -1,4 +1,5 @@ import { openReadonlyOrFail } from '../../db/index.js'; +import { warn } from '../../infrastructure/logger.js'; import { isTestFile } from '../../infrastructure/test-filter.js'; import { normalizeSymbol } from '../../shared/normalize.js'; import { paginateResult } from '../../shared/paginate.js'; @@ -75,6 +76,18 @@ export function rolesData(customDbPath, opts = {}) { * if test-file edges were excluded. */ function _findTestOnlyCalledIds(db) { + const hasTestNodes = db + .prepare( + `SELECT 1 FROM nodes WHERE file LIKE '%.test.%' OR file LIKE '%.spec.%' OR file LIKE '%__tests__%' LIMIT 1`, + ) + .get(); + if (!hasTestNodes) { + warn( + 'No test-file nodes in the graph — cannot determine test-only callers. Rebuild without -T to include test files.', + ); + return new Set(); + } + // Get all non-test symbols that have at least one caller const rows = db .prepare( From 5d6d64c77930984fa47eba226d8afb3e88d91c49 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 17 Mar 2026 05:37:59 -0600 Subject: [PATCH 6/6] fix: warn when test-only detection finds no test-file nodes in graph Impact: 1 functions changed, 1 affected --- src/domain/analysis/roles.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/domain/analysis/roles.js b/src/domain/analysis/roles.js index aea8fe66..ec271f4a 100644 --- a/src/domain/analysis/roles.js +++ b/src/domain/analysis/roles.js @@ -1,4 +1,5 @@ import { openReadonlyOrFail } from '../../db/index.js'; +import { warn } from '../../infrastructure/logger.js'; import { isTestFile } from '../../infrastructure/test-filter.js'; import { normalizeSymbol } from '../../shared/normalize.js'; import { paginateResult } from '../../shared/paginate.js'; @@ -75,6 +76,18 @@ export function rolesData(customDbPath, opts = {}) { * if test-file edges were excluded. */ function _findTestOnlyCalledIds(db) { + // Check whether any test-file nodes exist in the graph at all. + // If the graph was built with -T (excluding test files), there will be + // no test callers and the query below would silently return nothing. + const files = db.prepare(`SELECT DISTINCT file FROM nodes WHERE kind = 'file'`).all(); + const hasTestNodes = files.some((r) => isTestFile(r.file)); + if (!hasTestNodes) { + warn( + 'No test file nodes found in the graph — run "codegraph build" without -T to enable test-only detection', + ); + return new Set(); + } + // Get all non-test symbols that have at least one caller const rows = db .prepare(