From 56dfdb0655490682e0351afbf32ebb850b44ab39 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Thu, 27 Aug 2026 07:57:00 -0500 Subject: [PATCH] =?UTF-8?q?feat(ui):=20dead=20code=20and=20islands=20?= =?UTF-8?q?=E2=80=94=20what=20nothing=20reaches,=20and=20everything=20that?= =?UTF-8?q?=20could=20still=20reach=20it=20(CG-59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Dead code screen and a mark on the Map, both drawn from one derivation in src/graph/dead-code.ts so a second surface can never disagree with the first. The SQL half is four lines — no incoming edge but `contains`. It returns ~2 500 candidates on this repository and the shipped list is 20; everything in between is the feature. A candidate is dropped the moment there is any reason to believe something outside the graph reaches it: exported symbols and header declarations, test and generated files, abstract and interface members, anything carrying a `decorates` edge, overrides of an ancestor's member, names the language calls by itself, vendored directories, files nothing in the index reaches (those are islands, and the Map says so instead), names the resolver failed to resolve somewhere, and names shared with a symbol that IS referenced — the mis-resolution that leaves a used method with a self-edge and its twin with nothing. The last rule is the only one that is not a graph query: before a claim is made, the declaring file and every file that reaches it are read and the identifier counted, which is what catches the references the extractor never recorded (`this.handleMessage.bind(this)`, a call inside an object literal, a shorthand property). Every subtraction is counted and printed under the list with the scale it came from, and the caveat line above it never collapses: the claim is "no static reference in the index", not "unused". On the Map a module nothing depends on keeps its stroke and says so in its count line, and tool-generated files and modules recede to ink-4 there, in the map's file list, in search results and on the file screen. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 + CLAUDE.md | 3 +- __tests__/dead-code.test.ts | 485 ++++++++++++ __tests__/ui-package.test.ts | 15 + docs/design/codegraph-ui-design-spec.md | 40 + scripts/check-ui-package.mjs | 1 + src/db/queries.ts | 167 ++++ src/graph/dead-code.ts | 886 ++++++++++++++++++++++ src/graph/index.ts | 14 + src/index.ts | 41 + src/ui-server/api/deadcode.ts | 221 ++++++ src/ui-server/api/index.ts | 19 +- src/ui-server/api/map.ts | 48 +- src/ui-server/api/search.ts | 12 +- src/ui-server/api/source.ts | 35 + src/ui-server/api/wire.ts | 8 + ui/README.md | 5 +- ui/src/App.svelte | 17 +- ui/src/components/PaletteRows.svelte | 10 +- ui/src/components/TopBar.svelte | 3 +- ui/src/components/map/MapSidePanel.svelte | 38 +- ui/src/components/map/ModuleNode.svelte | 24 +- ui/src/index.ts | 15 + ui/src/lib/adapter.ts | 42 +- ui/src/lib/api.ts | 19 + ui/src/lib/deadcode-model.ts | 95 +++ ui/src/lib/map-model.ts | 38 +- ui/src/lib/navigation.ts | 16 + ui/src/lib/router.svelte.ts | 12 + ui/src/lib/wire.ts | 55 ++ ui/src/views/DeadCodeView.svelte | 440 +++++++++++ ui/src/views/FileView.svelte | 7 +- 32 files changed, 2800 insertions(+), 39 deletions(-) create mode 100644 __tests__/dead-code.test.ts create mode 100644 src/graph/dead-code.ts create mode 100644 src/ui-server/api/deadcode.ts create mode 100644 ui/src/lib/deadcode-model.ts create mode 100644 ui/src/views/DeadCodeView.svelte diff --git a/CHANGELOG.md b/CHANGELOG.md index ba875b7..cce5174 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,14 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Members that redeclare something from a type above are marked in the outline ("overrides Base", or "satisfies Clock" for an interface), so a 40-member class shows at a glance which parts are its own and which are a contract it is filling. The number of implementations shown here is the same number `codegraph_explore` reports to your agent when it announces an interface dispatch. +- **Find the code nothing reaches, in `codegraph ui`.** A new **Dead code** tab lists the symbols no import, call or reference anywhere in your project reaches — biggest first, grouped by the file they live in, with the number of lines each one would take with it. A class nobody uses brings its methods along as a single finding rather than eleven. Every row opens the code. + + The screen is built to be believed rather than to look impressive. A line above the list says, and keeps saying, that this means "no static reference in the index" and not "unused" — reflection, a framework registry and a template can all reach code a graph cannot follow. Under the list, every reason a candidate was left off is printed with its count, so you can see the list is twenty findings out of two and a half thousand candidates rather than twenty out of twenty-one. + + Those exclusions are the feature. Anything exported, or declared in a header, is off the list by default, because something outside your repository can import it — one switch adds them back, with a warning band. So are test and generated files, abstract and interface declarations, anything a decorator registers, members that override something further up, names the language calls for you (`constructor`, `__enter__`, `main`), vendored directories, and files nothing in your project reaches at all. Two more rules catch what the graph itself missed: a name CodeGraph failed to resolve somewhere is never called unreferenced, and neither is a name shared with a symbol that *is* used — the twin may simply have been picked instead. Last, before any row is shown, the files that could reach it are read and the identifier counted: written down twice, something uses it and we did not see it. + + On the **Map**, a module nothing depends on now says so in its own count line instead of counting itself — usually your entry points, sometimes something you forgot to delete. Tool-generated files and modules are dimmed wherever they appear: on the map, in its file list, in search results and on the file screen. + ### Fixes - Fixed a long-running `codegraph ui` session serving a symbol that a sync had already deleted. The viewer keeps one connection to your index open, and its in-memory lookup didn't notice when another process — your agent's sync, or `codegraph sync` — rewrote the file underneath it, so a symbol screen could keep showing a body with no callers while search correctly reported it had moved. Because a symbol's identity includes the line it starts on, this happened after almost any edit above it. diff --git a/CLAUDE.md b/CLAUDE.md index c1d97af..862e7d5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,7 +77,8 @@ The public API surface is `src/index.ts` — the `CodeGraph` class wires all the - `src/db/` — `DatabaseConnection`, `QueryBuilder` (prepared statements), `schema.sql`, `sqlite-adapter.ts`. Backed by Node's built-in **`node:sqlite`** (`DatabaseSync`) — real SQLite with WAL + FTS5, exposed through a thin better-sqlite3-shaped adapter. The bundled runtime always ships Node ≥22.5, so `node:sqlite` is always available: **no native build step and no wasm fallback**. (Running from source needs Node ≥22.5.) `codegraph status` reports the live backend (`node-sqlite`, the sole backend). - `src/extraction/` — `ExtractionOrchestrator`, tree-sitter wrappers, per-language extractors under `languages/` (one file per language), plus standalone extractors for non-tree-sitter formats (`svelte-extractor.ts`, `vue-extractor.ts`, `liquid-extractor.ts`, `dfm-extractor.ts` for Delphi). `parse-worker.ts` runs heavy parsing off the main thread. - `src/resolution/` — `ReferenceResolver` orchestrates `import-resolver.ts` (with `path-aliases.ts` for tsconfig path aliases + cargo workspace member globs), `name-matcher.ts`, and `frameworks/` (Express, Laravel, Rails, FastAPI, Django, Flask, Spring, Gin, Axum, ASP.NET, Vapor, React Router, SvelteKit, Vue/Nuxt, Cargo workspaces). Frameworks emit `route` nodes and `references` edges. -- `src/graph/` — `GraphTraverser` (BFS/DFS, impact radius, path finding) and `GraphQueryManager` (high-level queries), plus the shared query-time derivations more than one surface renders: `named-symbol-flow.ts` (the one path finder, behind `codegraph_explore`'s Flow section and the viewer's Flow strip), `dynamic-boundary-report.ts` (where the graph stops), `type-hierarchy.ts` (ancestors/subtypes and the implementation count explore prints and the viewer draws). A derivation that two callers render must live here, not in `ToolHandler` — two derivations eventually disagree. +- `src/graph/` — `GraphTraverser` (BFS/DFS, impact radius, path finding) and `GraphQueryManager` (high-level queries), plus the shared query-time derivations more than one surface renders: `named-symbol-flow.ts` (the one path finder, behind `codegraph_explore`'s Flow section and the viewer's Flow strip), `dynamic-boundary-report.ts` (where the graph stops), `type-hierarchy.ts` (ancestors/subtypes and the implementation count explore prints and the viewer draws), + `dead-code.ts` (unreferenced symbols, and every reason a candidate is NOT claimed). A derivation that two callers render must live here, not in `ToolHandler` — two derivations eventually disagree. - `src/context/` — `ContextBuilder` + formatter for markdown/JSON output. - `src/search/` — full-text query parser and helpers for FTS5. - `src/sync/` — `FileWatcher` (native FSEvents/inotify/RDCW) with debounce + filter, and git-hook helpers. diff --git a/__tests__/dead-code.test.ts b/__tests__/dead-code.test.ts new file mode 100644 index 0000000..d946c46 --- /dev/null +++ b/__tests__/dead-code.test.ts @@ -0,0 +1,485 @@ +/** + * Dead code and islands (CG-59). + * + * Two halves, both against a real indexed fixture: the derivation in + * `src/graph/dead-code.ts`, and the `/api/deadcode` endpoint that renders it + * over a real loopback server, like the rest of the viewer's API suite. + * + * The fixture is shaped to produce, deliberately, one of each thing the report + * has to get RIGHT BY NOT CLAIMING IT: + * + * - a genuinely unreferenced helper (the only row that should survive); + * - a same-name pair where the resolver attaches the call to the wrong one — + * the mis-resolution that makes a used method look unreached; + * - a method that overrides a base's, reached only through the base; + * - a decorated method, registered by a framework the graph cannot see; + * - a helper only a template mentions, so no edge records the use but the file + * text does; + * - an exported function nothing here calls, which an outside caller may. + * + * Every one of those must be OFF the list, and the reason must be counted. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as http from 'http'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { + buildDeadCodeReport, + isHeaderFile, + isImplicitEntryName, + isTestScope, + isVendoredPath, + mentionCount, + DEAD_CODE_KINDS, +} from '../src/graph/dead-code'; +import { createGraphApi, startUiServer, type GraphApi, type UiServerHandle } from '../src/ui-server'; + +let server: UiServerHandle; +let api: GraphApi; +let tempDir: string; +let projectRoot: string; +let cg: CodeGraph; + +function write(root: string, rel: string, body: string): void { + const full = path.join(root, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, body); +} + +function request(requestPath: string): Promise<{ status: number; body: string; type?: string }> { + return new Promise((resolve, reject) => { + const req = http.request( + { + host: '127.0.0.1', + port: server.port, + path: requestPath, + method: 'GET', + headers: { Host: `127.0.0.1:${server.port}` }, + setHost: false, + }, + (res) => { + const chunks: Buffer[] = []; + res.on('data', (c: Buffer) => chunks.push(c)); + res.on('end', () => + resolve({ + status: res.statusCode ?? 0, + body: Buffer.concat(chunks).toString('utf-8'), + type: res.headers['content-type'], + }) + ); + } + ); + req.on('error', reject); + req.end(); + }); +} + +async function getDeadCode(query = ''): Promise { + const res = await request(`/api/deadcode${query}`); + expect(res.type).toBe('application/json; charset=utf-8'); + expect(res.status).toBe(200); + return JSON.parse(res.body); +} + +const names = (report: { entries: Array<{ node: { name: string } }> }): string[] => + report.entries.map((entry) => entry.node.name); + +beforeAll(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-deadcode-')); + projectRoot = path.join(tempDir, 'project'); + + // The one genuinely dead symbol, plus a live one beside it so the file is + // reached and the island rule does not swallow the whole thing. + write( + projectRoot, + 'src/util.ts', + `export function used(value: string): string { + return value.trim(); +} + +function neverCalledAnywhere(value: string): string { + return value.toUpperCase(); +} + +function alsoDeadButSmaller(): number { + return 1; +} + +// Exported and never called here — an outside caller may import it, so the +// default list must not claim it. It lives in a REACHED file on purpose: an +// unreached file is an island, which is a different exclusion. +export function publicEntryPoint(): string { + return 'hello'; +} +` + ); + + // The mis-resolution: \`Facade.load\` calls \`this.inner.load()\`, and the + // resolver prefers a same-name definition in the call site's own file. One of + // the two ends up with no incoming edge and neither is unreferenced. + write( + projectRoot, + 'src/inner.ts', + `export class Inner { + load(): string { + return 'inner'; + } +} +` + ); + + // A base and an override: calls land on \`Base.run\`, never on \`Child.run\`. + write( + projectRoot, + 'src/base.ts', + `export class Base { + run(): string { + return 'base'; + } +} +` + ); + write( + projectRoot, + 'src/child.ts', + `import { Base } from './base'; + +export class Child extends Base { + run(): string { + return 'child'; + } +} +` + ); + + write( + projectRoot, + 'src/facade.ts', + `import { Inner } from './inner'; +import { Base } from './base'; +import { Child } from './child'; +import { used } from './util'; + +function register(target: unknown, key: string): void { + void target; + void key; +} + +export class Facade { + inner = new Inner(); + child = new Child(); + + load(): string { + return this.inner.load(); + } + + go(): string { + const base: Base = this.child; + return used(base.run()) + this.load(); + } + + @register + onEvent(): void { + void 0; + } +} +` + ); + + // Mentioned in a template but never called anywhere the graph can see: the + // corroboration pass has to find the second mention in this file's own text. + write( + projectRoot, + 'src/handlers.ts', + `export function mountHandlers(): string { + return TEMPLATE; +} + +function onSubmit(): void { + void 0; +} + +const TEMPLATE = '
'; +` + ); + + // Nothing imports this file at all: its symbols' zero fan-in describes the + // file, not the symbol. That is the island rule, and it is the Map's job. + write( + projectRoot, + 'src/orphan.ts', + `function strandedHelper(): string { + return 'nobody imports this file'; +} + +function alsoStranded(): number { + return strandedHelper().length; +} +` + ); + + write( + projectRoot, + 'src/index.ts', + `import { Facade } from './facade'; +import { mountHandlers } from './handlers'; + +export function start(): string { + return new Facade().go() + mountHandlers(); +} +` + ); + + // A test helper file with a dependent, so `includeTests` is what decides + // whether its dead symbol shows — not the island rule. + write( + projectRoot, + 'tests/helpers.ts', + `export function sharedHelper(): string { + return 'shared'; +} + +function helperNothingCalls(): void { + void 0; +} +` + ); + write( + projectRoot, + 'tests/facade.test.ts', + `import { Facade } from '../src/facade'; +import { sharedHelper } from './helpers'; + +export function testFacade(): string { + return new Facade().go() + sharedHelper(); +} +` + ); + + const init = CodeGraph.initSync(projectRoot, { + config: { include: ['src/**/*.ts', 'tests/**/*.ts'], exclude: [] }, + }); + await init.indexAll(); + init.resolveReferences(); + init.close(); + + cg = CodeGraph.openSync(projectRoot); + + const viewerDir = path.join(tempDir, 'viewer'); + fs.mkdirSync(viewerDir, { recursive: true }); + fs.writeFileSync(path.join(viewerDir, 'index.html'), '
'); + + api = createGraphApi({ projectRoot }); + server = await startUiServer({ projectRoot, viewerDir, port: 0, api: api.handler }); +}, 120_000); + +afterAll(async () => { + cg?.close(); + api?.close(); + await server?.close(); + if (tempDir) fs.rmSync(tempDir, { recursive: true, force: true }); +}); + +describe('buildDeadCodeReport', () => { + it('finds the symbol nothing references', () => { + const report = buildDeadCodeReport(cg); + expect(names(report)).toContain('neverCalledAnywhere'); + }); + + it('leaves nothing on the list that anything reaches', () => { + const report = buildDeadCodeReport(cg); + // `used`, `start`, `go` and `mountHandlers` are all called; `Inner.load` + // and `Facade.load` are the same-name pair; `Child.run` is an override. + for (const name of ['used', 'start', 'go', 'mountHandlers', 'load', 'run']) { + expect(names(report)).not.toContain(name); + } + }); + + it('excludes a symbol only its own file mentions, and counts it', () => { + const report = buildDeadCodeReport(cg); + expect(names(report)).not.toContain('onSubmit'); + expect(report.excluded.mentioned).toBeGreaterThan(0); + expect(report.corroborated).toBe(true); + }); + + it('makes the claim when corroboration is switched off', () => { + // The rule that catches `onSubmit` is the only one that reads a file, so + // turning it off has to be visible in BOTH the list and the flag. + const report = buildDeadCodeReport(cg, { readSource: null }); + expect(report.corroborated).toBe(false); + expect(report.excluded.mentioned).toBe(0); + expect(names(report)).toContain('onSubmit'); + }); + + it('excludes exported symbols by default and includes them on request', () => { + const strict = buildDeadCodeReport(cg); + expect(names(strict)).not.toContain('publicEntryPoint'); + expect(strict.excluded.exported).toBeGreaterThan(0); + expect(strict.includeExported).toBe(false); + + const wide = buildDeadCodeReport(cg, { includeExported: true }); + expect(names(wide)).toContain('publicEntryPoint'); + expect(wide.includeExported).toBe(true); + expect(wide.excluded.exported).toBe(0); + }); + + it('excludes test files by default and includes them on request', () => { + expect(names(buildDeadCodeReport(cg))).not.toContain('helperNothingCalls'); + expect(buildDeadCodeReport(cg).excluded.tests).toBeGreaterThan(0); + expect(names(buildDeadCodeReport(cg, { includeTests: true }))).toContain( + 'helperNothingCalls' + ); + }); + + it('says nothing about a file nothing in the index reaches', () => { + // An island's symbols have zero fan-in because the FILE is unreached, which + // is a fact about the file — the Map draws it, this list does not claim it. + const report = buildDeadCodeReport(cg, { includeExported: true }); + expect(names(report)).not.toContain('strandedHelper'); + expect(report.excluded.unreachableFile).toBeGreaterThan(0); + }); + + it('excludes a decorated member — a framework registers it', () => { + const report = buildDeadCodeReport(cg); + expect(names(report)).not.toContain('onEvent'); + expect(report.excluded.decorated).toBeGreaterThan(0); + }); + + it('ranks by size and reports the real total when capped', () => { + const full = buildDeadCodeReport(cg); + const sizes = full.entries.map((entry) => entry.lines); + expect([...sizes].sort((a, b) => b - a)).toEqual(sizes); + + const capped = buildDeadCodeReport(cg, { limit: 1 }); + expect(capped.entries).toHaveLength(1); + expect(capped.total).toBe(full.total); + // The cap trims the tail, not the head: the biggest finding survives. + expect(capped.entries[0]?.node.name).toBe(full.entries[0]?.node.name); + }); + + it('every exclusion count is a number of candidates, and they add up', () => { + const report = buildDeadCodeReport(cg); + const excluded = Object.values(report.excluded).reduce((sum, n) => sum + n, 0); + expect(report.candidates).toBeGreaterThan(0); + expect(excluded + report.entries.length).toBeLessThanOrEqual(report.candidates); + expect(report.bounded).toBe(false); + }); + + it('restricts to the kinds asked for, and ignores nonsense', () => { + const classesOnly = buildDeadCodeReport(cg, { kinds: ['class'] }); + expect(classesOnly.kinds).toEqual(['class']); + for (const entry of classesOnly.entries) expect(entry.node.kind).toBe('class'); + + // An unknown kind is not a 500 and not an empty list: it falls back to the + // default set, which is the answer the caller meant. + const nonsense = buildDeadCodeReport(cg, { kinds: ['banana' as never] }); + expect(nonsense.kinds).toEqual([...DEAD_CODE_KINDS]); + }); +}); + +describe('the rules that are pure', () => { + it('counts whole-identifier mentions only', () => { + expect(mentionCount('const load = 1; loader(); reload();', 'load')).toBe(1); + expect(mentionCount('a.load(); load();', 'load')).toBe(2); + expect(mentionCount('nothing here', 'load')).toBe(0); + // Stops early: the caller only ever needs to know "one, or more than one". + expect(mentionCount('x x x x x', 'x', 2)).toBe(2); + }); + + it('matches vendored directories as whole segments', () => { + expect(isVendoredPath('vendor/lib/a.go')).toBe(true); + expect(isVendoredPath('a/node_modules/b/c.js')).toBe(true); + expect(isVendoredPath('src/vendored-parser.ts')).toBe(false); + }); + + it('recognises headers as declaration surfaces', () => { + expect(isHeaderFile('src/tree_sitter/parser.h')).toBe(true); + expect(isHeaderFile('types/global.d.ts')).toBe(true); + expect(isHeaderFile('src/parser.c')).toBe(false); + }); + + it('recognises a test scope inside a file', () => { + expect(isTestScope('tests::row_sizes_match')).toBe(true); + expect(isTestScope('Fixtures.Tests.Helper')).toBe(true); + expect(isTestScope('Latest.value')).toBe(false); + }); + + it('recognises names the language calls by itself', () => { + expect(isImplicitEntryName('constructor')).toBe(true); + expect(isImplicitEntryName('__enter__')).toBe(true); + expect(isImplicitEntryName('ToString')).toBe(true); + expect(isImplicitEntryName('mainHandler')).toBe(false); + }); +}); + +describe('GET /api/deadcode', () => { + it('groups the rows by file and keeps the totals honest', async () => { + const payload = await getDeadCode(); + expect(payload.rows.total).toBe(payload.rows.items.length); + expect(payload.rows.shown).toBe(payload.rows.items.length); + + // Every count equals a list length in the same payload. + const grouped = payload.groups.reduce((sum: number, g: any) => sum + g.rows.length, 0); + expect(grouped).toBe(payload.rows.shown); + + const files = payload.groups.map((g: any) => g.file); + expect(new Set(files).size).toBe(files.length); + expect(files).toContain('src/util.ts'); + }); + + it('carries the exclusions with their own wording', async () => { + const payload = await getDeadCode(); + expect(payload.excluded.length).toBeGreaterThan(0); + for (const entry of payload.excluded) { + expect(entry.count).toBeGreaterThan(0); + expect(typeof entry.label).toBe('string'); + expect(entry.label.length).toBeGreaterThan(0); + } + const sum = payload.excluded.reduce((n: number, e: any) => n + e.count, 0); + expect(payload.excludedTotal).toBe(sum); + expect(payload.candidates).toBeGreaterThanOrEqual(payload.excludedTotal); + expect(payload.corroborated).toBe(true); + }); + + it('widens on ?exported=1 and says which list it answered', async () => { + const strict = await getDeadCode(); + const wide = await getDeadCode('?exported=1'); + expect(strict.includeExported).toBe(false); + expect(wide.includeExported).toBe(true); + expect(wide.rows.total).toBeGreaterThan(strict.rows.total); + expect(wide.rows.items.some((r: any) => r.name === 'publicEntryPoint')).toBe(true); + }); + + it('honours ?limit= without lying about the total', async () => { + const full = await getDeadCode(); + const capped = await getDeadCode('?limit=1'); + expect(capped.rows.items).toHaveLength(1); + expect(capped.rows.total).toBe(full.rows.total); + expect(capped.rows.truncated).toBe(full.rows.total > 1); + }); + + it('is listed on the API index', async () => { + const res = await request('/api'); + const body = JSON.parse(res.body); + expect(body.endpoints.some((e: any) => e.path === '/api/deadcode')).toBe(true); + }); +}); + +describe('GET /api/map — generated files and islands', () => { + it('reports how many of a module’s files are tool-generated', async () => { + const res = await request('/api/map'); + const payload = JSON.parse(res.body); + for (const module of payload.modules) { + expect(typeof module.generated).toBe('number'); + expect(module.generated).toBeLessThanOrEqual(module.files); + // The dimmed rows are drawn from `fileList.items`, so the generated + // subset has to be a subset of exactly that list. + for (const file of module.generatedFiles) { + expect(module.fileList.items).toContain(file); + } + } + }); +}); diff --git a/__tests__/ui-package.test.ts b/__tests__/ui-package.test.ts index 46042a0..abd90c5 100644 --- a/__tests__/ui-package.test.ts +++ b/__tests__/ui-package.test.ts @@ -391,6 +391,21 @@ function mockAdapter(): { adapter: GraphAdapter; calls: string[] } { index: { lastIndexedAt: null, files: 3 }, timing: { elapsedMs: 1, cached: false }, }), + deadCode: () => + seen('deadCode', { + rows: { total: 0, shown: 0, truncated: false, items: [] }, + groups: [], + candidates: 0, + excluded: [], + excludedTotal: 0, + kinds: ['function'], + includeExported: false, + includeTests: false, + includeGenerated: false, + bounded: false, + corroborated: true, + timing: { elapsedMs: 1 }, + }), // Deliberately no `events`: a host without a live channel is the normal // case, and nothing may poll in its absence. }; diff --git a/docs/design/codegraph-ui-design-spec.md b/docs/design/codegraph-ui-design-spec.md index c2ef3d6..5407609 100644 --- a/docs/design/codegraph-ui-design-spec.md +++ b/docs/design/codegraph-ui-design-spec.md @@ -337,6 +337,46 @@ Nothing in the engine emits an `overrides` edge, so this is a NAME match inside says so. It is deliberately blind to signatures — an overload set would need type resolution the graph does not have. Layout is arithmetic (row height × index): no `ResizeObserver`, no measurement, same payload → same picture. +### 3.11 Dead code and islands (CG-59) +A screen (`#/dead`, `?exported=1`) and a mark on the Map. + +**The list.** Symbols no import, call or reference in the index reaches, ranked largest first and grouped by file with the +Symbol view's `.filegroup` / `.row` shapes (design spec §3.2) — file path 11px mono `--ink-3` with the group's +"N symbols · M lines" opposite it, then rows of kind glyph + 12.5px mono name + 11px mono `file:line` + an 11px `--ink-3` meta +line ("method · 51 lines"). A dead container folds its unreachable members into a wrapped strip of 11px mono links under it +rather than listing them as siblings — one finding, not eleven. Column max-width **760px**, 40px gutters, exactly like the +entry-points panel. + +**The caveat is part of the screen, not a note on it.** A persistent 11.5px `--ink-3` line sits above the rows, between two +hairline rules, and never collapses or dismisses: *"No static reference in the index — dynamic use is possible."* Under the list, +every reason a candidate was left off is printed with its count ("1 677 in test files", "378 exported, or declared in a header", +"40 overriding a member declared further up"), preceded by the scale — *"2 494 symbols in this index carry no incoming reference +at all; 2 474 of them were left off this list."* Twenty rows drawn from twenty candidates and twenty drawn from two and a half +thousand are different screens and only that sentence tells them apart. + +**One switch**, an 11px mono chip on the right of the caveat bar: `Internal only` (default) ↔ `Including exported`, carried in +the URL. Turning it on adds symbols something outside the repository could import, and the screen grows an `--accent-soft` band +with an `--accent-line` border saying so; each such row also carries an `exported` chip. Exported rows are never on the default +list, because the index cannot check a caller it does not contain. + +**Islands, on the Map.** A module no link in the payload arrives at keeps its normal 1px `--ink` stroke — it is not a lesser +module, it is an unreached one — and its 11px count line reads **"nothing depends on this"** in `--ink-2` *instead of* the +symbol/file counts, which stay in the side panel. The island verdict is computed from the whole link set, so hiding test modules +cannot manufacture one. Selecting the module adds a sentence in the panel. Note the box is sized from whichever string it will +show, so the layout and the node must be given the same verdict. + +**Generated files recede everywhere** (`files.generated`, §2.6): a module whose files are *all* tool-generated draws in +`--ink-4` with a `--rule-soft` stroke; a generated file in the Map panel's file list, a generated group on the dead code list, +a generated result in the search palette and a generated file's title in the File view are all `--ink-4`. Partly-generated +modules are not dimmed — a module with one `.pb.go` in it is still one somebody writes by hand. + +**What the list refuses to claim** is the whole design. Behind it, `src/graph/dead-code.ts` starts from "no incoming edge +other than `contains`" and subtracts every candidate there is any reason to believe something reaches: exported symbols and +header declarations, test and generated files, abstract and interface members, anything carrying a `decorates` edge, overrides +of an ancestor's member, names the language calls by itself, vendored directories, files nothing in the index reaches (those are +islands — the Map's job, not this list's), names the resolver failed to resolve somewhere, names shared with a symbol that IS +referenced, and — the only rule that reads a file — names written more than once in a file that can reach them. + ## 4. Libraries and versions - Svelte 5 (≥ 5.25) + Vite (workspace `ui/`), Svelte Flow `@xyflow/svelte` ^1.6 for the Map and Flow canvases only (custom nodes/edges, hidden handles for port spreading, local selection state — the pattern in docker-app's `StackGraph.svelte`); `@dagrejs/dagre` only as a diff --git a/scripts/check-ui-package.mjs b/scripts/check-ui-package.mjs index 6a0a04f..399c4ea 100644 --- a/scripts/check-ui-package.mjs +++ b/scripts/check-ui-package.mjs @@ -141,6 +141,7 @@ for (const name of [ 'TypeHierarchy', 'FlowStrip', 'ArchitectureMap', + 'DeadCodeView', 'TrailBar', 'SearchPalette', 'CodegraphUi', diff --git a/src/db/queries.ts b/src/db/queries.ts index 2de4d9c..f462637 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -1979,6 +1979,173 @@ export class QueryBuilder { return out; } + /** + * Symbols nothing in the index points at — the candidate set behind the dead + * code list (`src/graph/dead-code.ts`). + * + * "Points at" is every edge kind EXCEPT `contains`: a class containing a + * method is structure, not use, and counting it would make every member look + * reached by its own container. A self-edge is excluded for the same reason + * a recursive function is not its own caller. + * + * One scan, one index probe per candidate. `NOT EXISTS` over + * `idx_edges_target_kind` is what keeps it that way — the alternative + * (`LEFT JOIN edges … GROUP BY`) builds a row per edge for the whole table + * before discarding all but the empty groups. Ordered by position so the + * answer is stable across runs and groups by file without a second sort. + * + * The result is deliberately NOT called dead code: an unreferenced symbol is + * a symbol with no STATIC reference, and the caller applies the exclusions + * (tests, generated files, overrides, unresolved names) that turn the + * candidate set into a claim worth making. + */ + getUnreferencedNodes( + kinds: readonly string[], + limit: number + ): Array<{ node: Node; generated: boolean }> { + if (kinds.length === 0 || limit <= 0) return []; + const placeholders = kinds.map(() => '?').join(','); + const rows = this.db + .prepare( + `SELECT n.*, COALESCE(f.generated, 0) AS file_generated + FROM nodes n + LEFT JOIN files f ON f.path = n.file_path + WHERE n.kind IN (${placeholders}) + AND NOT EXISTS ( + SELECT 1 FROM edges e + WHERE e.target = n.id + AND e.kind != 'contains' + AND e.source != n.id + ) + ORDER BY n.file_path, n.start_line, n.name + LIMIT ?` + ) + .all(...kinds, limit) as Array; + return rows.map((row) => ({ node: rowToNode(row), generated: row.file_generated === 1 })); + } + + /** + * Which of `names` the index holds an UNRESOLVED reference to. + * + * The point is honesty about our own blind spots. A `failed` row in + * `unresolved_refs` records that some file referenced a name and the resolver + * could not decide what it meant — so a symbol with that name cannot be + * called unreferenced, whatever the edge table says. It is deliberately + * matched loosely, on the reference name AND on its tail (`util.greet` → + * `greet`), because the question being asked is "could this name be the one + * we failed to follow", and a maybe has to count as a yes. + * + * Bounded-lookup like {@link getGeneratedPathsAmong}: the caller holds a + * candidate list, so this is a chunked probe over `idx_unresolved_name`, not + * a scan of the table. + */ + getUnresolvedNamesAmong(names: Iterable): Set { + const unique = [...new Set(names)].filter((name) => name.length > 0); + const found = new Set(); + if (unique.length === 0) return found; + + for (let i = 0; i < unique.length; i += SQLITE_PARAM_CHUNK_SIZE) { + const chunk = unique.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + const rows = this.db + .prepare( + `SELECT DISTINCT reference_name AS name FROM unresolved_refs + WHERE reference_name IN (${placeholders}) + UNION + SELECT DISTINCT name_tail AS name FROM unresolved_refs + WHERE name_tail IN (${placeholders})` + ) + .all(...chunk, ...chunk) as Array<{ name: string }>; + for (const row of rows) found.add(row.name); + } + return found; + } + + /** + * Which of `names` are carried by MORE THAN ONE symbol, at least one of which + * something points at. + * + * The false positive this exists to kill: `CodeGraph.getTopRouteFile` calls + * `this.queries.getTopRouteFile()`, and the resolver — which prefers a + * same-name definition in the call site's own file — attaches that edge to + * the *calling* method. One of the two ends up with a self-edge and the other + * with nothing at all, and neither is unreferenced. From the edge table the + * mis-resolution and a genuinely unused twin are the same picture, so the + * claim is not made about either. + * + * Both halves of the condition are load-bearing. **More than one symbol**: + * a uniquely-named function that only calls itself is genuinely dead, and + * excluding every recursive function would gut the list. **Self-edges + * counted**: the self-edge IS the fingerprint of the mis-resolution above, so + * it has to count as evidence that this name resolves somewhere. + * + * Chunked probe over `idx_nodes_name`, bounded by the caller's candidate list. + */ + getAmbiguousReferencedNames(names: Iterable): Set { + const unique = [...new Set(names)].filter((name) => name.length > 0); + const found = new Set(); + if (unique.length === 0) return found; + + for (let i = 0; i < unique.length; i += SQLITE_PARAM_CHUNK_SIZE) { + const chunk = unique.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + const rows = this.db + .prepare( + `SELECT name FROM ( + SELECT n.name AS name, + EXISTS ( + SELECT 1 FROM edges e + WHERE e.target = n.id AND e.kind != 'contains' + ) AS referenced + FROM nodes n + WHERE n.name IN (${placeholders}) + ) + GROUP BY name + HAVING COUNT(*) > 1 AND SUM(referenced) > 0` + ) + .all(...chunk) as Array<{ name: string }>; + for (const row of rows) found.add(row.name); + } + return found; + } + + /** + * Which of the given languages the index records an EXPORT marker for. + * + * A self-measurement, and the honest basis for a whole class of exclusion. + * The dead code report's strongest filter is "exported symbols may be reached + * from outside this repository" — and that filter silently does nothing for a + * language whose exports are not recorded, either because the extractor does + * not record them (Rust `pub`) or because the language has no such concept at + * all (Python, C, Ruby: the header or the module IS the surface). Rather than + * carry a table of which is which, ask the index: if nothing in this language + * is marked exported, the filter did not run, and no claim about outside + * reachability can be made for it. + * + * `idx_nodes_language` covers the grouping; the caller passes the handful of + * languages its candidates are actually in. + */ + getLanguagesWithExports(languages: Iterable): Set { + const unique = [...new Set(languages)].filter((language) => language.length > 0); + const found = new Set(); + if (unique.length === 0) return found; + + for (let i = 0; i < unique.length; i += SQLITE_PARAM_CHUNK_SIZE) { + const chunk = unique.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + const rows = this.db + .prepare( + `SELECT language, MAX(is_exported) AS any_exported + FROM nodes + WHERE language IN (${placeholders}) + GROUP BY language` + ) + .all(...chunk) as Array<{ language: string; any_exported: number }>; + for (const row of rows) if (row.any_exported === 1) found.add(row.language); + } + return found; + } + /** * The nodes with the most DISTINCT dependents, most first. * diff --git a/src/graph/dead-code.ts b/src/graph/dead-code.ts new file mode 100644 index 0000000..1d724c5 --- /dev/null +++ b/src/graph/dead-code.ts @@ -0,0 +1,886 @@ +/** + * Dead code and islands — one derivation of "nothing in this repository + * reaches here". + * + * The graph can answer that question exactly, and that is the problem: the + * exact answer is *no incoming edge*, and a symbol with no incoming edge is not + * the same thing as a symbol nobody uses. Reflection calls it. A framework + * registers it by name. A test file that was never indexed imports it. The + * resolver saw the name and could not follow it. So the honest product of this + * module is two things at once — a list, and everything the list could not see. + * + * ## The shape of the claim + * + * `unreferenced` is a fact: no edge in the index, other than the `contains` + * edge from whatever holds it, points at this symbol. `dead` is an inference on + * top of that fact, and every step of the inference is subtractive — a + * candidate is dropped from the list the moment there is any reason to believe + * something outside the graph reaches it: + * + * - it is **exported** (something outside this repository may import it); + * - it lives in a **test** or a **generated** file (not code anyone deletes by + * hand); + * - it is **abstract** or declared on an interface (a declaration is dispatched + * to, never called); + * - it is **decorated** (`@app.route`, `@Component`, `@EventHandler`) — a + * decorator is a registration, and the framework that reads it is not in the + * graph. Seen as the symbol's own outgoing `decorates` edge, which is where + * the engine records it; `node.decorators` is only populated by a couple of + * languages and is checked as well rather than instead; + * - it **overrides** a member an ancestor declares (calls land on the ancestor; + * see {@link overrideCandidates} for why an ancestor we cannot read counts + * the same way); + * - it has a name the language calls by itself (`constructor`, `__enter__`, + * `main`); + * - it sits in a **vendored** directory (`vendor/`, `third_party/`, + * `node_modules/` — code the repository carries but does not own); + * - it is in a **test scope** the path does not reveal — a Rust + * `#[cfg(test)] mod tests`, a nested `Tests` namespace; + * - it is in a **component file** whose markup the index reads for calls but + * not for references, so a handler passed as `{onkeydown}` is invisible; + * - it is in a file **nothing in the index reaches**. Then "nothing references + * this symbol" is a restatement of "we cannot see how this file is wired", + * not a finding about the symbol — and it is the map, not this list, that + * says so: a file no one reaches is an island, and islands are drawn there; + * - the index holds an **unresolved reference** to its name. A `failed` row in + * `unresolved_refs` is the resolver's own record of a reference it could not + * follow, and a symbol whose name we failed to follow cannot be called + * unreferenced. + * - **another symbol of the same name IS referenced.** This is the one that + * matters most and the one nothing else would catch. `CodeGraph.getTopRouteFile` + * calls `this.queries.getTopRouteFile()`; the resolver prefers a same-name + * definition in the call site's own file, so the edge lands on the caller + * itself and the real target is left with nothing. From the edge table, + * "nobody calls this" and "the resolver picked the twin" are the same + * picture — so the claim is not made about either; + * - it is declared in a **header** (`.h`, `.hpp`, `.d.ts`, `.pyi`): a header IS + * the export surface, and the reference to it is an `#include` the resolver + * does not follow to the declaration; + * - it is in a language this index records **no export marker** for. The + * exported filter is the strongest one here, and for Rust (`pub` is not + * recorded) or Python and C (no such concept at all) it silently does + * nothing — so the index is asked, per language, whether it ran; + * - **its own file writes the name more than once.** The last rule, and the + * only one that is not a graph query. Everything above assumes the edge + * table is complete; it is not, and the gaps do not announce themselves — + * `this.handleMessage.bind(this)` is a value reference the extractor does not + * record, and a call inside an object-literal initialiser is another. Both + * leave the name written twice in one file and no edge at all. So before the + * claim is made, the identifier is counted in the file itself AND in every + * file the index says depends on it — written once, in its own declaration, + * nothing that can reach it writes it down; written twice, we simply did not + * see the second one. + * + * Every subtraction is counted. {@link DeadCodeReport.excluded} is not + * diagnostics — it is the sentence under the list ("47 exported, 12 overriding + * an ancestor…"), because a list of eight rows drawn from four thousand + * candidates means something different from a list of eight drawn from nine. + * + * ## Islands + * + * The other half of the task, a *module* nothing depends on, is not computed + * here: it falls straight out of the map's own link set (a module with no + * incoming link), and the map's layout is already a pure function in the + * viewer. Computing it a second time on this side would be a second answer to + * a question the map has already answered. See `ui/src/lib/map-model.ts`. + * + * Everything here is query-time and read-only. + */ + +import fs from 'fs'; +import path from 'path'; +import type CodeGraph from '../index'; +import type { Node, NodeKind } from '../types'; +import { isTestFile } from '../search/query-utils'; + +// ============================================================================= +// Caps and defaults +// ============================================================================= + +/** + * Kinds asked about by default. + * + * Callables and types, and nothing else. `variable`/`constant`/`field` are out + * deliberately: a value's uses are recorded as `references` edges, and that + * coverage is the most language-dependent thing in the resolver — a default + * that included them would produce a list whose truthfulness varied by which + * language the reader happened to be looking at. + */ +export const DEAD_CODE_KINDS: readonly NodeKind[] = [ + 'function', + 'method', + 'class', + 'component', + 'interface', + 'struct', + 'trait', + 'protocol', + 'enum', + 'union', + 'type_alias', +]; + +/** Kinds a `kinds=` request may ask for. Anything else is a caller bug. */ +export const DEAD_CODE_ALLOWED_KINDS: ReadonlySet = new Set([ + ...DEAD_CODE_KINDS, + 'variable', + 'constant', + 'property', + 'field', + 'enum_member', + 'namespace', + 'module', +]); + +/** + * Candidates pulled out of SQL before any exclusion runs. + * + * High enough that no real repository reaches it with the default kinds (this + * index produces ~1 400), and bounded so that a half-indexed monorepo cannot + * turn one screen into a scan of a million rows. When it bites, + * {@link DeadCodeReport.bounded} says so. + */ +export const MAX_DEAD_CODE_CANDIDATES = 20000; + +/** Levels walked up looking for an ancestor that declares the same member. */ +export const MAX_OVERRIDE_ANCESTOR_DEPTH = 8; + +/** + * Files read for the corroboration pass, and the biggest one read. + * + * The pass runs over the survivors only — everything cheap has already fired — + * so on this index it reads a few dozen files. The caps are a backstop against + * a repository whose survivors span a thousand files or include a generated + * megabyte. A file skipped for either reason counts as NOT corroborated, which + * drops the row: the safe direction is always the one that says less. + */ +export const MAX_CORROBORATION_FILES = 600; +export const MAX_CORROBORATION_BYTES = 2_000_000; + +/** Kinds that can carry members, i.e. whose ancestors are worth walking. */ +const CONTAINER_KINDS: ReadonlySet = new Set([ + 'class', + 'interface', + 'struct', + 'trait', + 'protocol', + 'enum', + 'union', + 'type_alias', +]); + +/** Container kinds whose members are declarations, never call targets. */ +const DECLARATION_CONTAINER_KINDS: ReadonlySet = new Set([ + 'interface', + 'trait', + 'protocol', +]); + +/** Member kinds an override can be declared on. */ +const OVERRIDABLE_KINDS: ReadonlySet = new Set([ + 'method', + 'function', + 'property', + 'field', +]); + +/** + * Names a language or a runtime calls without anything in the source naming + * them. + * + * Kept short on purpose. The temptation is a per-language table of every + * lifecycle hook ever written, which would be wrong twice over — it would go + * stale, and it would hide real dead code behind a name coincidence. The + * entries below are the ones where the *language itself* does the calling, so + * no source file could name them even in principle. Framework hooks are caught + * by the decorator and override rules instead, which are structural. + */ +const IMPLICIT_ENTRY_NAMES: ReadonlySet = new Set([ + 'constructor', + 'main', + 'init', + 'deinit', + 'finalize', + 'destructor', + 'dispose', + 'drop', + 'default', + 'tostring', + 'equals', + 'gethashcode', + 'hashcode', +]); + +/** + * Qualified-name segments that mean "inside a test scope the file path does not + * reveal" — a Rust `#[cfg(test)] mod tests`, a nested `Tests` class in C#, a + * Go `TestMain` helper block. `isTestFile` only reads paths, and an in-file test + * module is invisible to it. + */ +const TEST_SCOPE_SEGMENTS: ReadonlySet = new Set([ + 'test', + 'tests', + '__tests__', + 'spec', + 'specs', + 'testing', +]); + +/** + * Languages whose files are markup with a script block inside them. + * + * The extractors for these read the ` + +
+
+

Dead code

+

+ Symbols no import, call or reference in this index reaches, largest first. Everything below + is what the graph can see; the notes under the list are what it cannot. +

+
+ +
+

{DEAD_CODE_CAVEAT}

+ { + event.preventDefault(); + navigate(deadHref({ exported: !exported })); + }}>{exported ? 'Including exported' : 'Internal only'} +
+ + {#if failure} +

Could not read the list — {failure}

+ {:else if loading && payload === null} +

Reading the graph…

+ {:else if payload} + {#if exported} +

+ Exported symbols are on this list. Nothing in this repository references them, but anything + outside it can — a published package, another service, a script. Read each one before you + believe it. +

+ {/if} + + {#if payload.groups.length === 0} +

{emptyMessage(payload)}

+ {:else} +

{headline}

+
+ {#each payload.groups as group (group.file)} +
+
+ {group.file} + {groupMeta(group)} +
+ {#each group.rows as row (row.id)} +
+ +
+
+ + {row.file}:{row.line} + {#if row.exported}exported{/if} +
+
{deadCodeRowMeta(row)}
+ {#if row.members.items.length > 0} +
+ {#each row.members.items as member (member.id)} + {member.name} + {/each} + {#if row.members.truncated} + +{row.members.total - row.members.shown} more + {/if} +
+ {/if} +
+
+ {/each} +
+ {/each} +
+ {/if} + +
+

{scale}

+ {#if phrases.length > 0} +
    + {#each phrases as phrase (phrase)} +
  • {phrase}
    • + {/each} +
+ {/if} + {#if !payload.corroborated} +

+ The rows were not checked against the text of the files that can reach them, so a + reference the extractor did not record would not have been caught. +

+ {/if} + {#if payload.bounded} +

+ The scan stopped at its cap — this index holds more unreferenced symbols than were + considered. +

+ {/if} + {#if payload.rows.truncated} +

+ Showing {payload.rows.shown} of {payload.rows.total} — the rest are in the index, not on + this list. +

+ {/if} +
+ {/if} +
+ + diff --git a/ui/src/views/FileView.svelte b/ui/src/views/FileView.svelte index 1344e3e..7fb365d 100644 --- a/ui/src/views/FileView.svelte +++ b/ui/src/views/FileView.svelte @@ -276,7 +276,8 @@
-

{basename(payload.file.path)}

+ +

{basename(payload.file.path)}

{fileMetaLine(payload)} {payload.file.path}
@@ -381,6 +382,10 @@ letter-spacing: -0.01em; } + .card-h h1.gen { + color: var(--ink-4); + } + .spacer { flex: 1 1 auto; }