feat(cli): codegraph cycles lists the files that import each other in a circle (#3099)

New `codegraph cycles [path] [--json]` prints the groups of files that
all reach each other through imports. It exits 1 when there are any and
0 when there are none, so `codegraph cycles || exit 1` works as a
pre-commit or CI check.

The command stays a thin wrapper over CodeGraph.findCircularDependencies(),
but the method itself had to change. It had no callers, and on real
repos its walk wasn't fit to print:
- It reported one list per back edge, so the number depends on the walk:
  1,490 overlapping lists on excalidraw, 87 on gin, 44 on flask.
- It left out files. A walk that meets a visited file stops there, so a
  file whose loop closes through one is never listed (now pinned by a
  test: a>b, b>a, a>c, c>b).
- It counted every edge kind, and most of the graph's edges aren't
  imports. An interface's call fans out to a test's fake, so
  src/extraction/parse-pool.ts "depended" on its own test. Two Next.js
  pages that link to each other were a cycle. gin's files were a 55-file
  knot, though Go forbids import cycles. With exit 1 those are failures
  nobody can fix.

The method now takes the strongly connected components of the file
import graph, one group per knot. The Tarjan pass moves from the
viewer's map into src/graph/file-cycles.ts and both surfaces use it, so
the viewer's "Circular imports between files" panel, which was reading
every edge too, now reports the same groups. Dependent counts on the map
keep their own edge list.

Validation: new cli-cycles and file-cycles suites, plus cycles in the
read-commands suite (mid-rebuild, and old schema indexes); 4 of the 10
findCircularDependencies and finder tests fail with main's method and
all 15 pass with this one. Against the TypeScript compiler's own module
resolver over the same files (kit oracle): excalidraw 3 groups, 369
files, all identical; codegraph's src/ 3 of 4 identical, and the fourth
needs src/resolution/name-matcher.ts, which at 1.6 MB is over the
default size limit and so isn't parsed. flask reports 2 groups and gin
none. It runs in under a second on each. Type-only imports count, as
they aren't told apart in the graph.

The MCP tool from PR #889 is left out on purpose: a new tool is rarely
picked up, and this needs no agent steering.

Reimplements PR #1012 by @jiangyq9, with the exit code from the CLI in
PR #889 by @goutham80808.

Fixes #888

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-30 19:53:53 +00:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 47526d2078
commit 9bfdd09238
13 changed files with 475 additions and 139 deletions
+55
View File
@@ -20,6 +20,7 @@
* codegraph callees <symbol> Find what a function/method calls
* codegraph impact <symbol> 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
*/
+24 -9
View File
@@ -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.
*
+89
View File
@@ -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<string, string[]>();
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<string, number>();
const low = new Map<string, number>();
const onStack = new Set<string>();
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;
}
+4 -39
View File
@@ -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<string>();
const recursionStack = new Set<string>();
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());
}
/**
+13 -4
View File
@@ -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();
+10 -85
View File
@@ -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<string, string>
): WireMapPayload['cycles'] {
const adjacency = new Map<string, string[]>();
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<string, number>();
const low = new Map<string, number>();
const onStack = new Set<string>();
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.