diff --git a/AGENTS.md b/AGENTS.md index 01752bd63..3c1a7663e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -80,13 +80,13 @@ The public API surface is `src/index.ts` — the `CodeGraph` class wires all the - `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, Next.js — `nextjs.ts`: pages and `route.ts` handlers from files, `router.push` / `redirect` / `NextResponse.redirect` as `navigates` edges, with `next-router-synthesizer.ts` for `` — Expo Router, SvelteKit, Vue/Nuxt, Cargo workspaces). Frameworks emit `route` nodes and `references` edges. `callback-synthesizer.ts` holds the whole-graph synthesis passes (`SYNTH_PASSES`, merged in registry order — first-seen wins a duplicate pair) with the language gates; `tier-synthesizer.ts` is the cross-tier pass (a client's literal `fetch`/`axios` path onto its own route, a queue job onto its consumer, a bus / socket event onto its handler — `channel`, `tier`, `registeredAt` on every edge; registered before the in-process emitter pass so its more specific edge wins); `synth-utils.ts` has the helpers they share (`enclosingFn`, `enclosingValue`, `makeLineAt`). Express's `postExtract` composes `app.use('/prefix', router)` mounts onto a mounted file's route names, idempotently (the original path stays in `qualifiedName`). - `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), `synthesized-hop.ts` (how a synthesized hop reads, in explore's Flow, the `codegraph_node` trail and ContextBuilder's call paths — a new synthesizer's wording goes here). A derivation that two callers render must live here, not in `ToolHandler` — two derivations eventually disagree. + `dead-code.ts` (unreferenced symbols, and every reason a candidate is NOT claimed), `synthesized-hop.ts` (how a synthesized hop reads, in explore's Flow, the `codegraph_node` trail and ContextBuilder's call paths — a new synthesizer's wording goes here), `file-cycles.ts` (circular imports, in `codegraph cycles` and the viewer's map). 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. - `src/mcp/` — MCP server (`MCPServer`, `tools.ts`, `transport.ts`). `server-instructions.ts` is what the server returns in the MCP `initialize` response — keep it in sync with the user-facing tool guidance. - `src/installer/` — see below. -- `src/bin/codegraph.ts` — CLI (commander). Subcommands: `install`, `uninstall`, `init`, `uninit`, `index`, `sync`, `status`, `unlock`, `query`, `explore`, `context`, `node`, `files`, `callers`, `callees`, `impact`, `affected`, `daemon`, `telemetry`, `upgrade`, `version`. Hidden from `--help`: `serve --mcp`, `prompt-hook`, and `ui`/`web` unless `CODEGRAPH_UI=1`. +- `src/bin/codegraph.ts` — CLI (commander). Subcommands: `install`, `uninstall`, `init`, `uninit`, `index`, `sync`, `status`, `unlock`, `query`, `explore`, `context`, `node`, `files`, `callers`, `callees`, `impact`, `affected`, `cycles`, `daemon`, `telemetry`, `upgrade`, `version`. Hidden from `--help`: `serve --mcp`, `prompt-hook`, and `ui`/`web` unless `CODEGRAPH_UI=1`. - `src/ui/` — terminal UI (shimmer progress, worker). - `src/ui-server/` -- read-only JSON API for the `codegraph ui` browser viewer (`api/`: `node`, `flow`, `map`, `screens`, `steps`, `deadcode`, `trails`, `program`, ...) plus static server; Svelte viewer lives in `ui/` (see `docs/design/codegraph-ui-design-spec.md`). `screens`/`steps`/`program` share one fold (`via`/`when` via `graph/branch-guards.ts`); `api/effects.ts` curates calls that leave the index; `api/route-roots.ts` names where a route's code starts. Derivations rendered by more than one surface belong in `src/graph/`, not `ToolHandler`. **Not released yet:** `codegraph ui` / `web` are refused (and hidden from `--help`) unless `CODEGRAPH_UI=1` is set — `src/bin/viewer-gate.ts`. Viewer changelog entries wait in `docs/viewer-launch-changelog.md`, not under `[Unreleased]`. At launch: delete the gate, move those entries back, drop the "not released yet" notes from `site/`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c8b00e91..1f8a5584d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph status` now shows how many of a language's files yielded symbols when that isn't all of them, and warns when none did, so a parser that isn't working no longer looks healthy, and `codegraph status --strict` exits non-zero on that or on an index that's missing, unfinished or out of date, so packaging and CI can gate on it (#1658). Thanks @ferrine. - `codegraph serve --mcp --no-telemetry` turns telemetry off for one MCP server entry, so a shared MCP config can opt out without an environment variable; a background daemon that's already running for the project keeps its own setting until it exits. Thanks @danusha2345. (#1908) +- New `codegraph cycles` command lists the groups of files that import each other in a circle, as text or `--json`, and exits 1 when it finds any, so it works as a pre-commit or CI check (#888). Thanks @jiangyq9 and @goutham80808. ### Fixes diff --git a/README.md b/README.md index 54ce5ba02..93735ce09 100644 --- a/README.md +++ b/README.md @@ -543,6 +543,7 @@ codegraph callers # Find what calls a function/method (--limit, codegraph callees # Find what a function/method calls (--limit, --json) codegraph impact # Analyze what code is affected by changing a symbol (--depth, --json) codegraph affected [files...] # Find test files affected by changes (see below) +codegraph cycles [path] # List circular imports between files (--json); exits 1 when there are any codegraph daemon # Manage background daemons — pick one to stop (alias: daemons) codegraph telemetry [on|off] # Show or change anonymous usage telemetry codegraph upgrade [version] # Update to the latest release (--check, --force) diff --git a/__tests__/cli-cycles.test.ts b/__tests__/cli-cycles.test.ts new file mode 100644 index 000000000..c83ec7fa0 --- /dev/null +++ b/__tests__/cli-cycles.test.ts @@ -0,0 +1,116 @@ +/** + * `codegraph cycles`: circular imports between files, end to end against the + * built binary. Exit 1 when there are any is the command's contract: it is what + * makes `codegraph cycles || exit 1` a pre-commit or CI check. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +function run(cwd: string, ...args: string[]) { + const res = spawnSync(process.execPath, [BIN, 'cycles', cwd, ...args], { + encoding: 'utf-8', + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1' }, + }); + return { status: res.status, stdout: res.stdout, stderr: res.stderr }; +} + +async function indexProject(dir: string, files: Record): Promise { + for (const [rel, content] of Object.entries(files)) { + const abs = path.join(dir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } + const cg = CodeGraph.initSync(dir); + await cg.indexAll(); + cg.close(); +} + +/** `count` files in one import ring: each imports the next, the last the first. */ +function ring(count: number): Record { + const files: Record = {}; + for (let i = 0; i < count; i++) { + const next = (i + 1) % count; + files[`src/f${String(i).padStart(2, '0')}.ts`] = + `import { f${next} } from './f${String(next).padStart(2, '0')}';\nexport function f${i}() { return f${next}(); }\n`; + } + return files; +} + +describe('codegraph cycles', () => { + let dir: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cycles-cli-')); + }); + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('lists the files of a circular import and exits 1', async () => { + await indexProject(dir, { + 'src/a.ts': "import { b } from './b';\nexport function a() { return b(); }\n", + 'src/b.ts': "import { a } from './a';\nexport function b() { return a(); }\n", + 'src/plain.ts': "import { a } from './a';\nexport const plain = a;\n", + }); + + const res = run(dir); + expect(res.status).toBe(1); + expect(res.stdout).toContain('Circular imports: 1 group, 2 files'); + expect(res.stdout).toContain('src/a.ts'); + expect(res.stdout).toContain('src/b.ts'); + expect(res.stdout).not.toContain('src/plain.ts'); + }); + + it('prints the whole report as JSON and still exits 1', async () => { + await indexProject(dir, { + 'src/a.ts': "import { b } from './b';\nexport function a() { return b(); }\n", + 'src/b.ts': "import { a } from './a';\nexport function b() { return a(); }\n", + }); + + const res = run(dir, '--json'); + expect(res.status).toBe(1); + expect(JSON.parse(res.stdout)).toEqual({ count: 1, cycles: [['src/a.ts', 'src/b.ts']] }); + }); + + it('says so and exits 0 when the project has none', async () => { + await indexProject(dir, { + 'src/a.ts': 'export function a() { return 1; }\n', + 'src/b.ts': "import { a } from './a';\nexport function b() { return a(); }\n", + }); + + const res = run(dir); + expect(res.status).toBe(0); + expect(res.stdout).toContain('No circular imports found'); + + const json = run(dir, '--json'); + expect(json.status).toBe(0); + expect(JSON.parse(json.stdout)).toEqual({ count: 0, cycles: [] }); + }); + + it('shows the first files of a long group, and --json all of them', async () => { + await indexProject(dir, ring(14)); + + const res = run(dir); + expect(res.status).toBe(1); + expect(res.stdout).toContain('1. 14 files'); + expect(res.stdout).toContain('src/f11.ts'); + expect(res.stdout).not.toContain('src/f12.ts'); + expect(res.stdout).toContain('... and 2 more (--json lists them all)'); + + const parsed = JSON.parse(run(dir, '--json').stdout) as { cycles: string[][] }; + expect(parsed.cycles).toHaveLength(1); + expect(parsed.cycles[0]).toHaveLength(14); + }); + + it('fails plainly on a project with no index', () => { + const res = run(dir); + expect(res.status).toBe(1); + expect(res.stderr + res.stdout).toContain('not initialized'); + }); +}); diff --git a/__tests__/file-cycles.test.ts b/__tests__/file-cycles.test.ts new file mode 100644 index 000000000..7c23d228a --- /dev/null +++ b/__tests__/file-cycles.test.ts @@ -0,0 +1,146 @@ +/** + * What counts as a circular import: the strongly connected components of the + * file import graph, and nothing else the graph knows about two files. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { findFileCycles } from '../src/graph/file-cycles'; + +const pairs = (...edges: string[]) => + edges.map((edge) => { + const [source, target] = edge.split('>') as [string, string]; + return { source, target }; + }); + +describe('findFileCycles', () => { + it('finds nothing in a graph with no loop, a diamond included', () => { + expect(findFileCycles(pairs('a>b', 'a>c', 'b>d', 'c>d'))).toEqual([]); + expect(findFileCycles([])).toEqual([]); + }); + + it('reports a knot once, with every file in it', () => { + // a>b>a is a loop and so is a>c>b>a. A walk meets b first and has nothing + // left to do at c>b, so it reported a>b and left c out. + expect(findFileCycles(pairs('a>b', 'b>a', 'a>c', 'c>b'))).toEqual([['a', 'b', 'c']]); + // Two loops through one file are one group, however many ways round there are. + expect(findFileCycles(pairs('a>b', 'b>a', 'a>c', 'c>a'))).toEqual([['a', 'b', 'c']]); + }); + + it('keeps separate knots apart, smallest first, and leaves out the files between them', () => { + expect(findFileCycles(pairs('p>q', 'q>r', 'r>p', 'q>x', 'x>a', 'a>b', 'b>a'))).toEqual([ + ['a', 'b'], + ['p', 'q', 'r'], + ]); + }); + + it('gives the same report for the pairs in any order', () => { + const edges = pairs('a>b', 'b>c', 'c>a', 'd>e', 'e>d', 'c>d'); + expect(findFileCycles([...edges].reverse())).toEqual(findFileCycles(edges)); + }); + + it('walks a chain deeper than the call stack', () => { + const length = 200_000; + const chain = Array.from({ length }, (_, i) => ({ source: `f${i}`, target: `f${i + 1}` })); + chain.push({ source: `f${length}`, target: 'f0' }); + const cycles = findFileCycles(chain); + expect(cycles).toHaveLength(1); + expect(cycles[0]).toHaveLength(length + 1); + }); +}); + +describe('findCircularDependencies', () => { + let dir: string; + let cg: CodeGraph | null = null; + + async function index(files: Record): Promise { + for (const [rel, content] of Object.entries(files)) { + const abs = path.join(dir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } + cg = CodeGraph.initSync(dir); + await cg.indexAll(); + return cg; + } + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-file-cycles-')); + }); + afterEach(() => { + cg?.close(); + cg = null; + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('finds a loop that runs through a barrel', async () => { + const graph = await index({ + 'src/index.ts': "export * from './a';\n", + 'src/a.ts': "import { helper } from './helper';\nexport function a() { return helper(); }\n", + 'src/helper.ts': "import { a } from './index';\nexport function helper() { return a; }\n", + }); + expect(graph.findCircularDependencies()).toEqual([['src/a.ts', 'src/helper.ts', 'src/index.ts']]); + }); + + it('finds a loop in Python', async () => { + const graph = await index({ + 'a.py': 'from b import bb\n\ndef aa():\n return bb()\n', + 'b.py': 'from a import aa\n\ndef bb():\n return aa()\n', + }); + expect(graph.findCircularDependencies()).toEqual([['a.py', 'b.py']]); + }); + + it('does not take an interface call reaching a test fake for an import', async () => { + // Production code imports nothing from the test. The interface's call fans + // out to every implementer, the fake included, so the file edge list holds + // the loop src>test>src that no import makes. + const graph = await index({ + 'src/pool.ts': [ + 'export interface Worker { post(task: string): void }', + 'export function run(worker: Worker) { worker.post("x"); }', + '', + ].join('\n'), + 'src/pool.test.ts': [ + "import { run, Worker } from './pool';", + 'class FakeWorker implements Worker { post(task: string): void { void task; } }', + 'export function check() { run(new FakeWorker()); }', + '', + ].join('\n'), + }); + const edgeList = graph.getFileDependencyPairs(); + expect(edgeList).toContainEqual({ source: 'src/pool.ts', target: 'src/pool.test.ts' }); + expect(edgeList).toContainEqual({ source: 'src/pool.test.ts', target: 'src/pool.ts' }); + expect(graph.findCircularDependencies()).toEqual([]); + }); + + it('does not take two pages that link to each other for an import', async () => { + const graph = await index({ + 'package.json': JSON.stringify({ name: 'site', dependencies: { next: '15.0.0', react: '19.0.0' } }), + 'app/page.tsx': [ + "import Link from 'next/link';", + 'export default function Home() { return About; }', + '', + ].join('\n'), + 'app/about/page.tsx': [ + "import Link from 'next/link';", + 'export default function About() { return Home; }', + '', + ].join('\n'), + }); + const edgeList = graph.getFileDependencyPairs(); + expect(edgeList).toContainEqual({ source: 'app/page.tsx', target: 'app/about/page.tsx' }); + expect(edgeList).toContainEqual({ source: 'app/about/page.tsx', target: 'app/page.tsx' }); + expect(graph.findCircularDependencies()).toEqual([]); + }); + + it('does not take the files of one Go package calling each other for an import', async () => { + const graph = await index({ + 'go.mod': 'module example.com/m\n\ngo 1.21\n', + 'one.go': 'package m\n\nfunc One() { Two() }\n', + 'two.go': 'package m\n\nfunc Two() { One() }\n', + }); + expect(graph.findCircularDependencies()).toEqual([]); + }); +}); diff --git a/__tests__/read-commands-during-rebuild.test.ts b/__tests__/read-commands-during-rebuild.test.ts index 29eba2cb8..75bf3cdcf 100644 --- a/__tests__/read-commands-during-rebuild.test.ts +++ b/__tests__/read-commands-during-rebuild.test.ts @@ -57,6 +57,7 @@ const READ_COMMANDS: ReadCommand[] = [ { name: 'context', args: ['context', 'getUserProfile', 'loadOrderState'], answer: /loadOrderState/ }, { name: 'explore', args: ['explore', 'getUserProfile', 'loadOrderState'], answer: /src\/order\.ts/ }, { name: 'affected', args: ['affected', 'src/order.ts'], answer: /src\/user\.test\.ts/ }, + { name: 'cycles', args: ['cycles'], answer: /No circular imports found/ }, { name: 'prompt-hook', args: ['prompt-hook'], prompt: 'how does getUserProfile reach loadOrderState?', answer: / # Find what calls a function/method (--limit, codegraph callees # Find what a function/method calls (--limit, --json) codegraph impact # Analyze what code is affected by changing a symbol (--depth, --json) codegraph affected [files...] # Find test files affected by changes (see below) +codegraph cycles [path] # List circular imports between files (--json); exits 1 when there are any codegraph daemon # Manage background daemons — pick one to stop (alias: daemons) codegraph telemetry [on|off] # Show or change anonymous usage telemetry codegraph upgrade [version] # Update to the latest release (--check, --force) @@ -67,6 +68,18 @@ codegraph impact AuthMiddleware --depth 3 Traces import dependencies transitively to find which test files are affected by changed source files. See [Affected Tests in CI](/codegraph/guides/affected-tests/) for options and a CI example. +## cycles + +Lists circular imports: groups of files that all import each other, directly or through others. Each group is reported once with every file in it, smallest group first, and `--json` prints the whole list. + +```bash +codegraph cycles # the project you're standing in +codegraph cycles --json # { "count": 2, "cycles": [["src/a.ts", "src/b.ts"], ...] } +codegraph cycles || exit 1 # as a pre-commit or CI check +``` + +It exits 1 when there are any and 0 when there are none. Only import statements count, type-only imports included. Two files that merely call each other, like the files of one Go package, are not a circular import. + ## ui :::note[Not released yet] diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 869b31fec..fd574bacc 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -20,6 +20,7 @@ * codegraph callees Find what a function/method calls * codegraph impact Analyze what code is affected by changing a symbol * codegraph affected [files] Find test files affected by changes + * codegraph cycles [path] List circular imports between files * codegraph ui [path] Open the browser viewer (alias: web; not released yet — needs CODEGRAPH_UI=1) * codegraph upgrade [version] Update CodeGraph to the latest release */ @@ -2878,6 +2879,60 @@ program } }); +/** Files `codegraph cycles` prints per group; `--json` lists them all. */ +const CLI_CYCLE_FILES_SHOWN = 12; + +/** + * codegraph cycles [path] + * + * List groups of files that all import each other, directly or through others. + * Exits 1 when there are any, so it works as a pre-commit or CI check: + * + * codegraph cycles || exit 1 + */ +program + .command('cycles [path]') + .description('List circular imports between files (exits 1 when there are any)') + .option('-j, --json', 'Output as JSON') + .action(async (pathArg: string | undefined, options: { json?: boolean }) => { + const projectPath = resolveProjectPath(pathArg); + + try { + if (!isInitialized(projectPath)) { + error(`CodeGraph not initialized in ${projectPath}`); + process.exit(1); + } + + const cg = await openIndexToRead(projectPath); + try { + const cycles = cg.findCircularDependencies(); + if (options.json) { + console.log(JSON.stringify({ count: cycles.length, cycles }, null, 2)); + } else if (cycles.length === 0) { + success('No circular imports found'); + } else { + const files = cycles.reduce((sum, group) => sum + group.length, 0); + console.log(chalk.bold(`\nCircular imports: ${cycles.length} group${cycles.length === 1 ? '' : 's'}, ${files} files\n`)); + cycles.forEach((group, i) => { + console.log(chalk.cyan(`${i + 1}. ${group.length} files`)); + for (const file of group.slice(0, CLI_CYCLE_FILES_SHOWN)) console.log(` ${file}`); + if (group.length > CLI_CYCLE_FILES_SHOWN) { + console.log(chalk.dim(` ... and ${group.length - CLI_CYCLE_FILES_SHOWN} more (--json lists them all)`)); + } + console.log(); + }); + } + // exitCode, not exit(): a piped --json has to reach its reader whole. + if (cycles.length > 0) process.exitCode = 1; + } finally { + cg.destroy(); + } + } catch (err) { + error(`cycles failed: ${err instanceof Error ? err.message : String(err)}`); + process.exit(1); + } + }); + /** * codegraph install */ diff --git a/src/db/queries.ts b/src/db/queries.ts index 2e242d7ca..5435cf998 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -3360,16 +3360,8 @@ export class QueryBuilder { /** * Every ordered pair of files where one reaches into the other, once each. * - * The input a cycle finder wants: file-level circular dependencies are the - * strongly connected components of this graph. One query instead of the - * dependency lookup per file that {@link GraphQueryManager.findCircularDependencies} - * does — which matters because a cycle report is only interesting on a large - * repo, and that is exactly where a query per file stops being affordable. - * * `contains` is excluded (a file "contains" its own symbols, which is not a - * dependency), and so are same-file edges and low-confidence name matches: - * a cycle conjured by a common method name is a false alarm a reader cannot - * check. + * dependency), and so are same-file edges and low-confidence name matches. */ getCrossFileDependencyPairs(minConfidence: number): Array<{ source: string; target: string }> { return this.db @@ -3385,6 +3377,29 @@ export class QueryBuilder { .all(minConfidence) as Array<{ source: string; target: string }>; } + /** + * Every ordered pair of files where one imports the other, once each: the + * graph circular imports are read from. See `src/graph/file-cycles.ts`. + * + * Only `imports` edges, which the import resolver made from a statement in + * the file. Every other cross-file edge says something ran or was mentioned + * there, and a loop of those is not a circular import: an interface's call + * fans out to a test's fake that implements it, two pages link to each other, + * and the files of one Go package call each other freely. + */ + getFileImportPairs(): Array<{ source: string; target: string }> { + return this.db + .prepare( + `SELECT DISTINCT sn.file_path AS source, tn.file_path AS target + FROM edges e + JOIN nodes sn ON sn.id = e.source + JOIN nodes tn ON tn.id = e.target + WHERE e.kind = 'imports' + AND sn.file_path <> tn.file_path` + ) + .all() as Array<{ source: string; target: string }>; + } + /** * Every unresolved reference recorded in one FILE, ordered by line. * diff --git a/src/graph/file-cycles.ts b/src/graph/file-cycles.ts new file mode 100644 index 000000000..de1abc88f --- /dev/null +++ b/src/graph/file-cycles.ts @@ -0,0 +1,89 @@ +/** + * Circular imports between files, read by more than one surface: the + * `codegraph cycles` command (through `CodeGraph.findCircularDependencies`) and + * the viewer's map. + */ + +/** + * The strongly connected components of a file import graph, each with more than + * one file: groups of files that all reach each other through imports. A group + * is not a path. Two files that import each other are a group of two, and a + * 400-file knot is one group of 400, whatever the number of distinct loops that + * run through it, which depends on the order they are walked in. + * + * Files are sorted inside a group and groups by size, then first file, so two + * runs over one index print the same report. + */ +export function findFileCycles(pairs: ReadonlyArray<{ source: string; target: string }>): string[][] { + const adjacency = new Map(); + for (const pair of pairs) { + let out = adjacency.get(pair.source); + if (!out) adjacency.set(pair.source, (out = [])); + out.push(pair.target); + } + // Deterministic iteration: SQLite's DISTINCT ordering is not a contract. + const nodes = [...new Set([...adjacency.keys(), ...[...adjacency.values()].flat()])].sort(); + for (const list of adjacency.values()) list.sort(); + + return tarjan(nodes, (id) => adjacency.get(id) ?? []) + .filter((component) => component.length > 1) + .map((component) => component.slice().sort()) + .sort((a, b) => a.length - b.length || (a[0] ?? '').localeCompare(b[0] ?? '')); +} + +/** Tarjan's strongly connected components, iterative so a deep graph cannot blow the stack. */ +function tarjan(nodes: readonly string[], edgesOf: (id: string) => readonly string[]): string[][] { + const index = new Map(); + const low = new Map(); + const onStack = new Set(); + const stack: string[] = []; + const out: string[][] = []; + let counter = 0; + + for (const start of nodes) { + if (index.has(start)) continue; + const work: Array<{ id: string; edges: readonly string[]; at: number }> = [ + { id: start, edges: edgesOf(start), at: 0 }, + ]; + index.set(start, counter); + low.set(start, counter); + counter += 1; + stack.push(start); + onStack.add(start); + + while (work.length > 0) { + const frame = work[work.length - 1]; + if (frame === undefined) break; + if (frame.at < frame.edges.length) { + const next = frame.edges[frame.at]!; + frame.at += 1; + if (!index.has(next)) { + index.set(next, counter); + low.set(next, counter); + counter += 1; + stack.push(next); + onStack.add(next); + work.push({ id: next, edges: edgesOf(next), at: 0 }); + } else if (onStack.has(next)) { + low.set(frame.id, Math.min(low.get(frame.id) ?? 0, index.get(next) ?? 0)); + } + continue; + } + work.pop(); + if (low.get(frame.id) === index.get(frame.id)) { + const component: string[] = []; + for (;;) { + const popped = stack.pop(); + if (popped === undefined) break; + onStack.delete(popped); + component.push(popped); + if (popped === frame.id) break; + } + out.push(component); + } + const parent = work[work.length - 1]; + if (parent) low.set(parent.id, Math.min(low.get(parent.id) ?? 0, low.get(frame.id) ?? 0)); + } + } + return out; +} diff --git a/src/graph/queries.ts b/src/graph/queries.ts index e2af59335..717f4d662 100644 --- a/src/graph/queries.ts +++ b/src/graph/queries.ts @@ -7,6 +7,7 @@ import { Node, Edge, Context, Subgraph, EdgeKind } from '../types'; import { QueryBuilder } from '../db/queries'; import { GraphTraverser } from './traversal'; +import { findFileCycles } from './file-cycles'; /** * Graph query manager for complex queries @@ -219,48 +220,12 @@ export class GraphQueryManager { } /** - * Find circular dependencies in the graph + * Find circular imports between files * - * @returns Array of cycles, each cycle is an array of node IDs + * @returns Groups of files that all reach each other through imports, smallest first */ findCircularDependencies(): string[][] { - const files = this.queries.getAllFiles(); - const cycles: string[][] = []; - const visited = new Set(); - const recursionStack = new Set(); - - const dfs = (filePath: string, path: string[]): void => { - if (recursionStack.has(filePath)) { - // Found a cycle - const cycleStart = path.indexOf(filePath); - if (cycleStart !== -1) { - cycles.push(path.slice(cycleStart)); - } - return; - } - - if (visited.has(filePath)) { - return; - } - - visited.add(filePath); - recursionStack.add(filePath); - - const dependencies = this.getFileDependencies(filePath); - for (const dep of dependencies) { - dfs(dep, [...path, filePath]); - } - - recursionStack.delete(filePath); - }; - - for (const file of files) { - if (!visited.has(file.path)) { - dfs(file.path, []); - } - } - - return cycles; + return findFileCycles(this.queries.getFileImportPairs()); } /** diff --git a/src/index.ts b/src/index.ts index 777f5efd9..8cff94d95 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2341,13 +2341,21 @@ export class CodeGraph { } /** - * Every ordered pair of files where one reaches into the other — the edge - * list a cycle finder runs on. See {@link QueryBuilder.getCrossFileDependencyPairs}. + * Every ordered pair of files where one reaches into the other. See + * {@link QueryBuilder.getCrossFileDependencyPairs}. */ getFileDependencyPairs(minConfidence = 0): Array<{ source: string; target: string }> { return this.queries.getCrossFileDependencyPairs(minConfidence); } + /** + * Every ordered pair of files where one imports the other — the edge list + * circular imports are found on. See {@link QueryBuilder.getFileImportPairs}. + */ + getFileImportPairs(): Array<{ source: string; target: string }> { + return this.queries.getFileImportPairs(); + } + /** * References from a symbol that never resolved to an indexed node — the * calls and type mentions that leave the index. Lets a reader account for @@ -2962,9 +2970,10 @@ export class CodeGraph { } /** - * Find circular dependencies in the codebase + * Find circular imports between files * - * @returns Array of cycles, each cycle is an array of file paths + * @returns Groups of file paths that all reach each other through imports, + * smallest group first */ findCircularDependencies(): string[][] { return this.graphManager.findCircularDependencies(); diff --git a/src/ui-server/api/map.ts b/src/ui-server/api/map.ts index 5f48bfd9e..c2181b134 100644 --- a/src/ui-server/api/map.ts +++ b/src/ui-server/api/map.ts @@ -36,6 +36,7 @@ import type { CodeGraph } from '../../index'; import type { EdgeKind, Language } from '../../types'; +import { findFileCycles } from '../../graph/file-cycles'; import { isTestFile } from '../../search/query-utils'; import { badRequest } from './respond'; import { UNCERTAIN_BELOW, toPosixPath, wireList, type WireList } from './wire'; @@ -719,11 +720,7 @@ export function buildMap(cg: CodeGraph, projectRoot: string, query: URLSearchPar } } - // ONE fetch of the file edge list, read twice: the cycle finder and the - // dependent counts are both questions about it, and it is the expensive query - // on this screen. - const filePairs = cg.getFileDependencyPairs(UNCERTAIN_BELOW); - const dependents = countDependents(filePairs, moduleOfFile); + const dependents = countDependents(cg.getFileDependencyPairs(UNCERTAIN_BELOW), moduleOfFile); const payload: WireMapPayload = { root, @@ -756,7 +753,7 @@ export function buildMap(cg: CodeGraph, projectRoot: string, query: URLSearchPar links: [...links.values()].sort( (a, b) => a.source.localeCompare(b.source) || a.target.localeCompare(b.target) ), - cycles: fileCycles(filePairs, moduleOfFile), + cycles: fileCycles(cg.getFileImportPairs(), moduleOfFile), excluded: { uncertainEdges, confidenceBelow: UNCERTAIN_BELOW }, index: { lastIndexedAt: cg.getLastIndexedAt(), @@ -807,34 +804,19 @@ function countDependents( } /** - * File-level circular dependencies, as strongly connected components. + * The circular imports inside the chosen root (see {@link findFileCycles}). * - * Tarjan over the one-query file edge list. Components of size 1 are not - * cycles (a file depending on itself is a same-file edge, already excluded), - * and a component longer than {@link MAX_CYCLE_LENGTH} is reported truncated - * rather than printed — a 200-file knot is a fact about the repository, not a - * list anybody reads. + * A component longer than {@link MAX_CYCLE_LENGTH} is reported truncated rather + * than printed — a 200-file knot is a fact about the repository, not a list + * anybody reads. */ function fileCycles( pairs: ReadonlyArray<{ source: string; target: string }>, moduleOfFile: Map ): WireMapPayload['cycles'] { - const adjacency = new Map(); - for (const pair of pairs) { - if (!moduleOfFile.has(pair.source) || !moduleOfFile.has(pair.target)) continue; - let out = adjacency.get(pair.source); - if (!out) adjacency.set(pair.source, (out = [])); - out.push(pair.target); - } - // Deterministic iteration: SQLite's DISTINCT ordering is not a contract. - const nodes = [...new Set([...adjacency.keys(), ...[...adjacency.values()].flat()])].sort(); - for (const list of adjacency.values()) list.sort(); - - const components = tarjan(nodes, (id) => adjacency.get(id) ?? []); - const cycles = components - .filter((component) => component.length > 1) - .map((component) => component.slice().sort()) - .sort((a, b) => a.length - b.length || (a[0] ?? '').localeCompare(b[0] ?? '')); + const cycles = findFileCycles( + pairs.filter((pair) => moduleOfFile.has(pair.source) && moduleOfFile.has(pair.target)) + ); const items = cycles.slice(0, MAX_FILE_CYCLES).map((files) => ({ size: files.length, @@ -849,63 +831,6 @@ function fileCycles( }; } -/** Tarjan's strongly connected components, iterative so a deep graph cannot blow the stack. */ -function tarjan(nodes: readonly string[], edgesOf: (id: string) => readonly string[]): string[][] { - const index = new Map(); - const low = new Map(); - const onStack = new Set(); - const stack: string[] = []; - const out: string[][] = []; - let counter = 0; - - for (const start of nodes) { - if (index.has(start)) continue; - const work: Array<{ id: string; edges: readonly string[]; at: number }> = [ - { id: start, edges: edgesOf(start), at: 0 }, - ]; - index.set(start, counter); - low.set(start, counter); - counter += 1; - stack.push(start); - onStack.add(start); - - while (work.length > 0) { - const frame = work[work.length - 1]; - if (frame === undefined) break; - if (frame.at < frame.edges.length) { - const next = frame.edges[frame.at]!; - frame.at += 1; - if (!index.has(next)) { - index.set(next, counter); - low.set(next, counter); - counter += 1; - stack.push(next); - onStack.add(next); - work.push({ id: next, edges: edgesOf(next), at: 0 }); - } else if (onStack.has(next)) { - low.set(frame.id, Math.min(low.get(frame.id) ?? 0, index.get(next) ?? 0)); - } - continue; - } - work.pop(); - if (low.get(frame.id) === index.get(frame.id)) { - const component: string[] = []; - for (;;) { - const popped = stack.pop(); - if (popped === undefined) break; - onStack.delete(popped); - component.push(popped); - if (popped === frame.id) break; - } - out.push(component); - } - const parent = work[work.length - 1]; - if (parent) low.set(parent.id, Math.min(low.get(parent.id) ?? 0, low.get(frame.id) ?? 0)); - } - } - return out; -} - /** * The roots the selector offers: the repository root plus every top-level * directory that holds indexed files, biggest first.