From a03daa914a29cb89cc4620e60517c74568d3576b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Milicevic?= Date: Thu, 18 Jun 2026 00:50:24 -0500 Subject: [PATCH] fix(scanner): stop dead-code flagging public API; deepen detection (Codex #10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dead-code scanner flagged any export with no in-repo references as dead. But a package's public API lives in its entry point (index.* / __init__.py) and is consumed by code OUTSIDE the repo, where the usage grep is blind — so the whole API surface was reported as removable. That's the "shallow analysis" weakness Codex #10 called out. - Skip symbols declared in entry-point / barrel files. New optional `DeadCodeProfile.isEntryPoint(path)`; when a profile omits it the scanner applies a convention default (`index.ts|tsx|mjs|…` and `__init__.py`) via the exported `isEntryPointFile()` helper. - Make the uncertainty explicit: dead-code task descriptions now state the finding is grep-based reachability (not type-aware) and to confirm the symbol isn't reflectively used or part of an external API before removing. - Recall fix: the TS export regex now also matches `enum`, `abstract class`, and `async function` declarations (previously missed → silent gaps). Adds tests: entry-point exports are never flagged, non-entry unused exports are still flagged (now with the heuristic caveat), and an `isEntryPointFile` classification table. Co-Authored-By: Claude Opus 4.8 --- .../src/__tests__/scanner.test.ts | 55 +++++++++++++++++++ packages/asil-improvement-loop/src/index.ts | 1 + .../src/language-profile.ts | 8 +++ .../src/profiles/typescript.ts | 3 +- packages/asil-improvement-loop/src/scanner.ts | 27 ++++++++- 5 files changed, 92 insertions(+), 2 deletions(-) diff --git a/packages/asil-improvement-loop/src/__tests__/scanner.test.ts b/packages/asil-improvement-loop/src/__tests__/scanner.test.ts index b4b3ea3..6b32390 100644 --- a/packages/asil-improvement-loop/src/__tests__/scanner.test.ts +++ b/packages/asil-improvement-loop/src/__tests__/scanner.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { normalizePath, isTestFile, + isEntryPointFile, scanCodebase, scanTestFailures, scanTypeErrors, @@ -326,6 +327,60 @@ describe('scanner', () => { expect(args).toContain('--include=*.tsx'); } }); + + it('does NOT flag exports in a public entry point (index.ts) even with no in-repo uses (Codex #10)', async () => { + const exportsOut = './packages/foo/src/index.ts:1:export const publicApi = 1'; + const runner = mockRunner([ + { + match: (_, args) => args.some((a) => a.includes('^export')), + stdout: exportsOut, + }, + { + match: (_, args) => args.some((a) => a.includes('publicApi')), + // Only the entry point itself references it — out-of-repo consumers + // are invisible to grep, so this must NOT be reported as dead. + stdout: './packages/foo/src/index.ts', + }, + ]); + const tasks = await scanDeadCode('/repo', { + runner, + fs: mockFileReader(), + }); + expect(tasks).toEqual([]); + }); + + it('still flags an unused export in a non-entry file, with a heuristic caveat', async () => { + const exportsOut = './pkg/foo.ts:1:export const lonely = 1'; + const runner = mockRunner([ + { + match: (_, args) => args.some((a) => a.includes('^export')), + stdout: exportsOut, + }, + { + match: (_, args) => args.some((a) => a.includes('lonely')), + stdout: './pkg/foo.ts', + }, + ]); + const tasks = await scanDeadCode('/repo', { + runner, + fs: mockFileReader(), + }); + expect(tasks.length).toBe(1); + expect(tasks[0]?.description).toContain('lonely'); + expect(tasks[0]?.description.toLowerCase()).toContain('heuristic'); + }); + }); + + describe('isEntryPointFile', () => { + it('treats index.* barrels and __init__.py as entry points, not plain source', () => { + expect(isEntryPointFile('packages/foo/src/index.ts')).toBe(true); + expect(isEntryPointFile('index.tsx')).toBe(true); + expect(isEntryPointFile('src/index.mjs')).toBe(true); + expect(isEntryPointFile('pkg/__init__.py')).toBe(true); + expect(isEntryPointFile('src/foo.ts')).toBe(false); + expect(isEntryPointFile('src/indexer.ts')).toBe(false); + expect(isEntryPointFile('src/my_init.py')).toBe(false); + }); }); it('scanCodebase aggregates all scanners in parallel', async () => { diff --git a/packages/asil-improvement-loop/src/index.ts b/packages/asil-improvement-loop/src/index.ts index dc9b902..2d89465 100644 --- a/packages/asil-improvement-loop/src/index.ts +++ b/packages/asil-improvement-loop/src/index.ts @@ -27,6 +27,7 @@ export { scanDeadCode, normalizePath, isTestFile, + isEntryPointFile, stableTaskId, toRepoRelative, type ScanResult, diff --git a/packages/asil-improvement-loop/src/language-profile.ts b/packages/asil-improvement-loop/src/language-profile.ts index af5ac22..c84edac 100644 --- a/packages/asil-improvement-loop/src/language-profile.ts +++ b/packages/asil-improvement-loop/src/language-profile.ts @@ -59,6 +59,14 @@ export interface DeadCodeProfile { /** File extensions to scan (e.g. ['ts', 'tsx'] or ['py', 'pyi']). * Excludes the leading dot. */ fileExtensions: string[]; + /** + * Optional: is this file a public entry point (barrel / package root)? + * Symbols declared in an entry point are the package's API surface — their + * real consumers may live outside the repo, where grep can't see them, so + * the dead-code scanner must never flag them. When omitted, the scanner + * applies a convention default (`index.*` / `__init__.py`). + */ + isEntryPoint?(path: string): boolean; } export interface LanguageProfile { diff --git a/packages/asil-improvement-loop/src/profiles/typescript.ts b/packages/asil-improvement-loop/src/profiles/typescript.ts index fe24390..6108b17 100644 --- a/packages/asil-improvement-loop/src/profiles/typescript.ts +++ b/packages/asil-improvement-loop/src/profiles/typescript.ts @@ -111,7 +111,8 @@ export const typescriptProfile: LanguageProfile = { }, deadCode: { - exportRegex: /^export\s+(?:const|function|class|interface|type)\s+([A-Za-z_][A-Za-z0-9_]*)/, + exportRegex: + /^export\s+(?:abstract\s+)?(?:async\s+)?(?:const|function|class|interface|type|enum)\s+([A-Za-z_][A-Za-z0-9_]*)/, fileExtensions: ['ts', 'tsx'], usageGrep(symbol: string, excludeDirArgs: string[]) { return { diff --git a/packages/asil-improvement-loop/src/scanner.ts b/packages/asil-improvement-loop/src/scanner.ts index 12b04ef..9983739 100644 --- a/packages/asil-improvement-loop/src/scanner.ts +++ b/packages/asil-improvement-loop/src/scanner.ts @@ -289,8 +289,17 @@ export async function scanDeadCode( symbolByFile.set(file, set); } + const isEntryPoint = dc.isEntryPoint ?? isEntryPointFile; + const tasks: ImprovementTask[] = []; for (const [file, symbols] of symbolByFile) { + // Entry-point / barrel files (index.*, __init__.py) are the package's + // public API. Their consumers can live outside the repo, where the + // usage grep is blind — so a missing in-repo reference does NOT mean + // the symbol is dead. Skip them to avoid flagging the whole API surface. + // (Codex review #10: dead-code analysis was too shallow to tell public + // API from genuinely unreachable code.) + if (isEntryPoint(file)) continue; const unused: string[] = []; for (const sym of symbols) { const grep = dc.usageGrep(sym, GREP_EXCLUDE_DIRS); @@ -307,7 +316,11 @@ export async function scanDeadCode( category: 'dead-code', severity: 'low', title: `Remove ${unused.length} unused export(s) in ${shortPath(file)}`, - description: `Unused: ${unused.join(', ')}`, + description: + `Unused: ${unused.join(', ')}\n\n` + + 'Heuristic: these exports have no in-repo references (grep-based ' + + 'reachability, not type-aware). Confirm they are not reflectively ' + + 'used or part of an external/public API before removing.', filePaths: [file], estimatedTokens: 30_000, }), @@ -366,6 +379,18 @@ export function isTestFile(p: string): boolean { ); } +/** + * Convention default for "is this file a public entry point?" — used by the + * dead-code scanner when a language profile doesn't supply its own + * `isEntryPoint`. Matches JS/TS barrels (`index.ts`, `index.tsx`, + * `index.mjs`, …) and Python package roots (`__init__.py`). Symbols declared + * in these files are treated as public API and never flagged as dead. + */ +export function isEntryPointFile(p: string): boolean { + const base = shortPath(p); + return /^index\.[cm]?[jt]sx?$/.test(base) || base === '__init__.py'; +} + /** * Convert an absolute path under `repoRoot` to a repo-relative path. * Coverage tools (vitest's istanbul reporter, coverage.py) emit