diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20b31958ce..4644872723 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,6 +113,13 @@ jobs: node --experimental-strip-types --test scripts/layering/model.test.ts node --experimental-strip-types scripts/layering/check.ts + - name: Check the depgraph report agrees with the gate + # scripts/depgraph reads the same model as the gate, so its inversion count must + # reproduce TYPE_INVERSION_BASELINE. Free two-sources check: if the tree changes + # and only one side is updated, this fails and names the difference. Runs here + # rather than in its own job so the two can never be green independently. + run: node --experimental-strip-types --test scripts/depgraph/model.test.ts + affected-selector: name: Affected-check Selector runs-on: ubuntu-latest diff --git a/package.json b/package.json index 2a894e10e8..157b562e07 100644 --- a/package.json +++ b/package.json @@ -118,6 +118,8 @@ "check:affected": "node --experimental-strip-types scripts/check-affected/run.ts", "check:affected:test": "node --experimental-strip-types --test scripts/check-affected/model.test.ts scripts/check-affected/run.test.ts", "check:layering": "node --experimental-strip-types --test scripts/layering/model.test.ts && node --experimental-strip-types scripts/layering/check.ts", + "depgraph": "node --experimental-strip-types scripts/depgraph/build.ts", + "depgraph:test": "node --experimental-strip-types --test scripts/depgraph/model.test.ts", "check:production-exports": "fallow dead-code --config fallow-production-exports.json --production --unused-exports --fail-on-issues", "check:bundle-owner-files": "node --experimental-strip-types scripts/check-bundle-owner-files.ts", "check:freerange": "fr", @@ -125,7 +127,7 @@ "sync:mcp-metadata": "node scripts/sync-mcp-metadata.mjs", "check:mcp-metadata": "node scripts/sync-mcp-metadata.mjs --check", "version": "node scripts/sync-mcp-metadata.mjs && git add server.json", - "check:tooling": "pnpm lint && pnpm typecheck && pnpm check:layering && pnpm check:production-exports && pnpm check:mcp-metadata && pnpm build && pnpm check:bundle-owner-files", + "check:tooling": "pnpm lint && pnpm typecheck && pnpm check:layering && pnpm depgraph:test && pnpm check:production-exports && pnpm check:mcp-metadata && pnpm build && pnpm check:bundle-owner-files", "check:unit": "pnpm test:unit && pnpm test:smoke", "check": "pnpm check:tooling && pnpm check:fallow && pnpm check:unit", "prepack": "pnpm check:mcp-metadata && pnpm build:all && pnpm package:apple-runner:npm && pnpm package:android-snapshot-helper:npm && pnpm package:android-ime-helper:npm", diff --git a/scripts/depgraph/README.md b/scripts/depgraph/README.md new file mode 100644 index 0000000000..b3f34f5f78 --- /dev/null +++ b/scripts/depgraph/README.md @@ -0,0 +1,120 @@ +# Dependency graph report + +```sh +pnpm depgraph # -> .tmp/depgraph/graph.json + a text summary +pnpm depgraph --out /tmp/graph.json +pnpm depgraph:test +``` + +Emits the dependency graph of every production file under `src/` (tests excluded) as JSON, +plus a short summary of what the layering gate does not enforce. There is no renderer: the +productive artifact is the JSON, queried directly. + +## When to reach for this + +It pays for itself on three questions, and misleads on a fourth. + +**"What am I about to break?"** `nodes[].in` is the dependent count — blast radius. Size the +nodes by dependents and the files you should touch carefully are the big ones. Faster than +grepping, and it counts type-only and dynamic edges that a grep for `from '...'` misses. + +**"Where is the debt actually concentrated?"** Zone-level counts (`zoneEdges`) answer "which +boundary carries the most traffic" in one query. The pass that produced ADR-adjacent findings +started here. + +**"What is wrong that the gate does not enforce?"** This is the part CI cannot give you. The +gate rejects value-import cycles (R4) and spine back-edges (R5); the graph additionally reports: + +- **value edges whose target is also reachable at distance >= 2** — static module reachability, + and *only* that. It is **not** a removability claim, and the obvious reading is wrong: + reachability does not carry bindings (if `a` imports `{ c }` while `b` only re-exports it as + `{ c as b }`, the path exists and deleting `a -> c` still breaks `a`), it does not preserve when + a module's side effects run, and a direct import is often deliberately clearer than reaching + through a barrel. Deciding whether any given edge can go needs symbol-level analysis this does + not attempt. ~1300 of them: a place to look, never a work list. +- **type-only and dynamic cycles** — 8 of them, all outside R4 by design (a type-only import is + free at runtime, a dynamic one is a deliberate cold-start seam). Worth reading when a module + feels hard to reason about. + +**Where it misleads: a cluster's size is not its difficulty.** This is worth stating plainly +because it already cost a day. The `commands -> client` cluster looked like the obvious win — 28 +type-only inversions, all pointing at one file. Moving that file down took the gate from **42 to +48**, because the vocabulary it holds *depends on* `commands/`, `metro/`, `core/` and `remote/`; +declaring it in `contracts/` made the foundation depend on the layers above it. The picture shows +you an edge's weight, not whether it can be reversed. + +So: use the render to find a candidate, then answer "can this move?" numerically before planning +anything. The question is always *what does the target itself import, and what rank is that?* + +```sh +pnpm depgraph +# Zone pairs that invert the ranked spine. Read `typeInversions` rather than deriving it from +# `zoneEdges`: those counts come from the COLLAPSED edge list, where one edge per file pair +# survives and `dynamic` outranks `type`, so a module imported both lazily and for its types +# would drop out. `typeInversions` is counted by the gate's own rule and is what CI compares +# against TYPE_INVERSION_BASELINE. +node -e "const j=require('./.tmp/depgraph/graph.json'); + Object.entries(j.typeInversions) + .sort((a, b) => b[1] - a[1]) + .forEach(([pair, n]) => console.log(String(n).padStart(4), pair));" +``` + + +Note `zoneEdges[].backEdge` flags **R5 value** back-edges only, and there are none — filtering on +it returns an empty list, which is the gate passing, not a broken query. + + +## What is authoritative + +`pnpm check:layering` is. The viewer reads the same model, so the numbers should agree — and that +agreement is now enforced rather than hoped for: the **Layering Guard job runs +`scripts/depgraph/model.test.ts`**, whose last test asserts this report's inversion count reproduces +`TYPE_INVERSION_BASELINE`. If the tree changes and only one side is updated, CI fails and names the +difference. The two cannot be green independently. + +What that check proves precisely: the report's graph build, over the real tree, agrees with the +gate's baseline. It is a cross-check of the extraction and the baseline against reality, not two +independent algorithms — `typeInversionsByPair` deliberately applies the gate's counting rule (once +per file pair, over raw edges) so the numbers cannot diverge for a reason unrelated to layering. In +particular it does NOT count from the collapsed edge list, where `dynamic` outranks `type` and a +module imported both lazily and for its types would drop out. + +If they ever disagree, the gate is right and the baseline or the tree is wrong. + +### Notes + + +`pnpm check:layering` is. This reads the same model, so the numbers should agree — its R6 +count matching `TYPE_INVERSION_BASELINE` is a useful self-check — but if they ever diverge, the +gate is right and the graph is stale. + +One thing here DOES run in CI, and only one: the Layering Guard job runs +`scripts/depgraph/model.test.ts`, whose parity test asserts this report's inversion count equals +`TYPE_INVERSION_BASELINE`. Nothing else here gates a merge — the report itself is an instrument, not +a rule, and no finding it produces is enforced. + +## Why it reuses the layering gate + +The graph is extracted with `scripts/layering/model.ts`, the same module +`scripts/layering/check.ts` uses in CI. File set, zone partition, edge kinds +(value / type-only / dynamic), and cycle definition are therefore identical to the rules +the gate enforces — a separate extractor with its own resolution behaviour would draw a +graph nobody is enforcing. Cross-checked once against `dependency-cruiser` 3.1.1 (at the commit it was written): same +modules and edges, plus 88 dynamic/type-only edges dependency-cruiser fails to resolve. + +## What the JSON carries + +- `zones[]` — id, spine `rank` (`null` when intentionally unranked), `classification`, file + count, LOC. +- `zoneEdges[]` — per zone pair: total `count`, `valueCount`, and `backEdge` (R5 value + back-edges only — see the note above). +- `nodes[]` — per file: zone index, LOC, `in`/`out` degree, `lvl` (longest path to a sink over + value edges; R4 guarantees that subgraph is a DAG), and `cyc` (index into `cycles`, or `-1`). +- `edges[]` — index-addressed `[from, to, kind, flags]`. Kind: `0` value, `1` type-only, `2` + dynamic. Flags bitfield: `1` spine back-edge, `2` target also reachable at distance >= 2, `4` type-only + inversion. +- `cycles[]` — each with `kind` (`value` / `type` / `dynamic`) and its node path. + +Bit `2` means the target is reachable from the source at distance >= 2 over value edges. That is +module reachability, not removability — see the caveats above. Treat it as a question ("why is +this imported directly as well?"), never as an instruction. diff --git a/scripts/depgraph/build.ts b/scripts/depgraph/build.ts new file mode 100644 index 0000000000..291c28848f --- /dev/null +++ b/scripts/depgraph/build.ts @@ -0,0 +1,151 @@ +// Dependency-graph report — the numbers the layering gate does not enforce. +// +// node --experimental-strip-types scripts/depgraph/build.ts [--out ] +// +// Emits a JSON graph plus a short text summary. It reuses scripts/layering/model.ts, the same +// module check.ts uses in CI, so the file set, zone partition, edge kinds and cycle definition +// are the ones actually enforced — a second extractor would describe a graph nobody gates. +// +// What it adds over `pnpm check:layering`: value edges whose target is also reachable at distance +// >= 2 (module reachability, NOT a removability claim), cycles that are +// deliberately outside R4 (type-only and dynamic), per-zone size, and per-file fan-in/fan-out. +// See README.md for which question each field answers. + +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { listSourceFiles } from '../layering/check.ts'; +import { resolveImportEdges, zoneRank } from '../layering/model.ts'; +import { buildGraph, computeLevels, type GraphData } from './model.ts'; + +const repoRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { + encoding: 'utf8', +}).trim(); + +/** Compact wire form. Nodes and edges are index-addressed to keep the payload small. */ +type Payload = { + generated: { commit: string; files: number; edges: number }; + zones: { id: string; rank: number | null; classification: string; files: number; loc: number }[]; + zoneEdges: GraphData['zoneEdges']; + nodes: { + id: string; + z: number; + loc: number; + in: number; + out: number; + lvl: number; + cyc: number; + }[]; +/** + * `[fromIndex, toIndex, kind, flags]`; kind 0=value 1=type 2=dynamic, + * flags bit0=R5 back-edge, bit1=target also reachable at distance >= 2, bit2=R6 type inversion. + */ + edges: [number, number, number, number][]; + cycles: { kind: string; path: number[] }[]; + /** Type-only spine inversions per zone pair, by the gate's counting rule. */ + typeInversions: Record; +}; + +function headCommit(): string { + try { + return execFileSync('git', ['rev-parse', '--short', 'HEAD'], { + cwd: repoRoot, + encoding: 'utf8', + }).trim(); + } catch { + return 'unknown'; + } +} + +const EDGE_KIND_CODES = { value: 0, type: 1, dynamic: 2 } as const; + +/** Wire code for an edge kind, so the payload carries a number rather than a string per edge. */ +function edgeKindCode(kind: GraphData['edges'][number]['kind']): number { + return EDGE_KIND_CODES[kind]; +} + +/** + * Bitfield: 1 = spine back-edge (R5), 2 = target also reachable at distance >= 2, 4 = type-only + * inversion (R6). Bit 2 is reachability, NOT removability — see markTransitivelyReachableEdges. + */ +function edgeFlags(edge: GraphData['edges'][number]): number { + return ( + (edge.backEdge ? 1 : 0) | + (edge.transitivelyReachable ? 2 : 0) | + (edge.typeInversion ? 4 : 0) + ); +} + +function buildPayload(): Payload { + const files = listSourceFiles(); + const sources = new Map( + files.map((file) => [file, fs.readFileSync(path.join(repoRoot, file), 'utf8')]), + ); + const resolved = resolveImportEdges(sources); + const graph = buildGraph(sources, resolved); + const levels = computeLevels(graph.nodes, graph.edges); + + const zoneIndex = new Map(graph.zones.map((zone, index) => [zone.id, index])); + const nodeIndex = new Map(graph.nodes.map((node, index) => [node.id, index])); + + return { + generated: { commit: headCommit(), files: graph.nodes.length, edges: graph.edges.length }, + zones: graph.zones.map((zone) => ({ ...zone, rank: zoneRank(zone.id) })), + zoneEdges: graph.zoneEdges, + nodes: graph.nodes.map((node) => ({ + id: node.id.replace(/^src\//, ''), + z: zoneIndex.get(node.zone)!, + loc: node.loc, + in: node.fanIn, + out: node.fanOut, + lvl: levels.get(node.id) ?? 0, + cyc: node.cycle, + })), + edges: graph.edges.map((edge) => [ + nodeIndex.get(edge.from)!, + nodeIndex.get(edge.to)!, + edgeKindCode(edge.kind), + edgeFlags(edge), + ]), + cycles: graph.cycles.map((cycle) => ({ + kind: cycle.kind, + path: cycle.path.map((file) => nodeIndex.get(file)!), + })), + typeInversions: graph.typeInversions, + }; +} + +function main(argv: readonly string[]): number { + const outFlag = argv.indexOf('--out'); + const jsonPath = + outFlag >= 0 && argv[outFlag + 1] + ? path.resolve(argv[outFlag + 1]!) + : path.join(repoRoot, '.tmp/depgraph/graph.json'); + + const payload = buildPayload(); + fs.mkdirSync(path.dirname(jsonPath), { recursive: true }); + fs.writeFileSync(jsonPath, `${JSON.stringify(payload, null, 2)}\n`); + + const valueCycles = payload.cycles.filter((cycle) => cycle.kind === 'value').length; + const otherCycles = payload.cycles.length - valueCycles; + const backEdges = payload.edges.filter(([, , , flags]) => flags & 1).length; + const transitivelyReachable = payload.edges.filter(([, , , flags]) => flags & 2).length; + const typeInversions = Object.values(payload.typeInversions).reduce((sum, n) => sum + n, 0); + process.stdout.write( + `Dependency graph: ${payload.generated.files} files, ${payload.generated.edges} edges, ` + + `${payload.zones.length} zones\n` + + ` value-import cycles (R4): ${valueCycles}\n` + + ` type-only/dynamic cycles (not gate-rejected): ${otherCycles}\n` + + ` spine back-edges (R5): ${backEdges}\n` + + ` type-only spine inversions (R6): ${typeInversions}\n` + + ` value edges whose target is also reachable at distance >= 2: ${transitivelyReachable}\n` + + ` (reachability only — not a removability claim, see scripts/depgraph/README.md)\n` + + ` wrote ${path.relative(repoRoot, jsonPath)}\n`, + ); + return 0; +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { + process.exit(main(process.argv.slice(2))); +} diff --git a/scripts/depgraph/model.test.ts b/scripts/depgraph/model.test.ts new file mode 100644 index 0000000000..f9c8d20567 --- /dev/null +++ b/scripts/depgraph/model.test.ts @@ -0,0 +1,267 @@ +import assert from 'node:assert/strict'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { existsSync, mkdtempSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { test } from 'node:test'; +import { listSourceFiles, TYPE_INVERSION_BASELINE } from '../layering/check.ts'; +import { resolveImportEdges } from '../layering/model.ts'; +import { + buildGraph, + collapseEdges, + collectCycles, + markTransitivelyReachableEdges, + typeInversionsByPair, +} from './model.ts'; + +function sources(entries: Record): Map { + return new Map(Object.entries(entries)); +} + +test('collapseEdges keeps one edge per pair at the strongest kind', () => { + const edges = resolveImportEdges( + sources({ + 'src/core/a.ts': [ + "import type { Shape } from './b.ts';", + "import { run } from './b.ts';", + "import type { Other } from './c.ts';", + "void import('./d.ts');", + ].join('\n'), + 'src/core/b.ts': 'export const run = 1;', + 'src/core/c.ts': 'export type Other = string;', + 'src/core/d.ts': 'export const lazy = 1;', + }), + ); + + assert.deepEqual( + collapseEdges(edges).map((edge) => ({ to: edge.to, kind: edge.kind })), + [ + { to: 'src/core/b.ts', kind: 'value' }, + { to: 'src/core/c.ts', kind: 'type' }, + { to: 'src/core/d.ts', kind: 'dynamic' }, + ], + ); +}); + +test('flags only value edges whose target is already reachable at distance >= 2', () => { + const edges = collapseEdges( + resolveImportEdges( + sources({ + // a -> b -> c means c is reachable from a at distance 2, so the direct a -> c edge is + // FLAGGED. Note it is not removable: `b` re-exports c's binding under a different name, + // so deleting a -> c would break a's `c` import. That gap is the point of the rename — + // this measures module reachability, not safe removal. a -> d is the only route to d. + 'src/core/a.ts': [ + "import { b } from './b.ts';", + "import { c } from './c.ts';", + "import { d } from './d.ts';", + ].join('\n'), + 'src/core/b.ts': "export { c as b } from './c.ts';", + 'src/core/c.ts': 'export const c = 1;', + 'src/core/d.ts': 'export const d = 1;', + }), + ), + ); + markTransitivelyReachableEdges(edges); + + const flagged = edges + .filter((edge) => edge.transitivelyReachable) + .map((edge) => `${edge.from} -> ${edge.to}`); + assert.deepEqual(flagged, ['src/core/a.ts -> src/core/c.ts']); +}); + +test('a type-only shortcut is never flagged against a value path', () => { + const edges = collapseEdges( + resolveImportEdges( + sources({ + 'src/core/a.ts': ["import { b } from './b.ts';", "import type { C } from './c.ts';"].join( + '\n', + ), + 'src/core/b.ts': "export { c as b } from './c.ts';", + 'src/core/c.ts': 'export type C = string;\nexport const c = 1;', + }), + ), + ); + markTransitivelyReachableEdges(edges); + + assert.deepEqual( + edges.filter((edge) => edge.transitivelyReachable), + [], + ); +}); + +test('collectCycles separates gate-rejected value cycles from type-only and dynamic loops', () => { + const valueCycle = collectCycles( + resolveImportEdges( + sources({ + 'src/core/a.ts': "import { b } from './b.ts';\nexport const a = 1;", + 'src/core/b.ts': "import { a } from './a.ts';\nexport const b = 1;", + }), + ), + ); + assert.deepEqual( + valueCycle.map((cycle) => cycle.kind), + ['value'], + ); + + const typeCycle = collectCycles( + resolveImportEdges( + sources({ + 'src/core/a.ts': "import type { B } from './b.ts';\nexport type A = B;", + 'src/core/b.ts': "import type { A } from './a.ts';\nexport type B = A | null;", + }), + ), + ); + assert.deepEqual( + typeCycle.map((cycle) => cycle.kind), + ['type'], + ); + + const dynamicCycle = collectCycles( + resolveImportEdges( + sources({ + 'src/core/a.ts': "export const a = () => import('./b.ts');", + 'src/core/b.ts': "export const b = () => import('./a.ts');", + }), + ), + ); + assert.deepEqual( + dynamicCycle.map((cycle) => cycle.kind), + ['dynamic'], + ); +}); + +test('buildGraph reports zone membership, degrees, and cross-zone edge counts', () => { + const files = sources({ + 'src/kernel/errors.ts': 'export const fail = 1;\n', + 'src/core/interactors/tap.ts': "import { fail } from '../../kernel/errors.ts';\n", + 'src/commands/tap.ts': [ + "import { fail } from '../kernel/errors.ts';", + "import '../core/interactors/tap.ts';", + ].join('\n'), + }); + const graph = buildGraph(files, resolveImportEdges(files)); + + const kernel = graph.nodes.find((node) => node.id === 'src/kernel/errors.ts')!; + assert.equal(kernel.zone, 'kernel'); + assert.equal(kernel.fanIn, 2); + assert.equal(kernel.fanOut, 0); + + // A non-root zone member resolves to its folder, not to `(root)`. + const interactor = graph.nodes.find((node) => node.id === 'src/core/interactors/tap.ts')!; + assert.equal(interactor.zone, 'core'); + + assert.deepEqual( + graph.zoneEdges.map((edge) => `${edge.from} -> ${edge.to} (${edge.count})`), + ['commands -> core (1)', 'commands -> kernel (1)', 'core -> kernel (1)'], + ); + assert.deepEqual( + graph.zones.map((zone) => zone.classification), + ['ranked', 'ranked', 'ranked'], + ); +}); + +// Two-sources-of-truth check, run by the Layering Guard job. +// +// The report and the gate read the same model, so their inversion counts must agree. This locks +// that: if the tree changes and only one side is updated, or if the report's extraction diverges +// from what the gate sees, this fails and names the difference. +// +// What it proves precisely: the report's own graph build, over the real tree, reproduces +// TYPE_INVERSION_BASELINE. It is a cross-check of the extraction and the baseline against reality, +// not two independent algorithms — `typeInversionsByPair` deliberately applies the gate's counting +// rule so the numbers cannot differ for a reason unrelated to layering. The gate stays the +// authority; if these disagree, the baseline or the tree is wrong, never this test. +test("the report's inversion count reproduces the gate's TYPE_INVERSION_BASELINE", () => { + const files = listSourceFiles(); + const sources = new Map(files.map((file) => [file, readFileSync(file, 'utf8')])); + const actual = typeInversionsByPair(resolveImportEdges(sources)); + + assert.deepEqual( + actual, + // Object key order differs between the two literals; compare as sorted entries. + Object.fromEntries( + Object.entries(TYPE_INVERSION_BASELINE).sort(([left], [right]) => left.localeCompare(right)), + ), + 'depgraph and scripts/layering/check.ts disagree about type-only spine inversions. ' + + 'Regenerate with `pnpm depgraph` and update TYPE_INVERSION_BASELINE, or fix the edge.', + ); +}); + +// A raw NUL byte in a source file makes Git classify it as binary, which hides the whole diff +// behind `- -` and leaves the file unreviewable. This module used a literal NUL as a map-key +// delimiter and shipped that way through a review; it is now the escape sequence, identical at +// runtime and textual on disk. Guarded repo-wide rather than for this one file, because nothing +// else would catch a recurrence and the failure mode is silent: the code works, the review does not. +test('no tracked TypeScript source contains a raw NUL byte', () => { + const tracked = execFileSync('git', ['ls-files', 'src/*.ts', 'src/**/*.ts', 'scripts/**/*.ts'], { + encoding: 'utf8', + }) + .split('\n') + .filter(Boolean); + + const binary = tracked.filter((file) => readFileSync(file).includes(0)); + assert.deepEqual( + binary, + [], + 'these files contain a raw NUL byte, so Git treats them as binary and hides their diff. ' + + 'Use a unicode escape instead of a literal control character.', + ); +}); + +// build.ts had no coverage at all: every test above exercises model.ts, so the CLI could break its +// output path, JSON shape or summary without anything failing. These run it as a subprocess, which +// is the only way to cover argument handling and the file it actually writes. + +function runBuild(args: readonly string[]): { status: number | null; stdout: string; stderr: string } { + const result = spawnSync( + process.execPath, + ['--experimental-strip-types', 'scripts/depgraph/build.ts', ...args], + { encoding: 'utf8' }, + ); + return { status: result.status, stdout: result.stdout ?? '', stderr: result.stderr ?? '' }; +} + +test('build.ts writes the default path and a summary consistent with the JSON', () => { + const { status, stdout } = runBuild([]); + assert.equal(status, 0, stdout); + + const payload = JSON.parse(readFileSync('.tmp/depgraph/graph.json', 'utf8')) as { + generated: { commit: string; files: number; edges: number }; + zones: { id: string; rank: number | null }[]; + nodes: unknown[]; + edges: [number, number, number, number][]; + typeInversions: Record; + }; + + // Wire shape: the fields a consumer queries. A rename here is a breaking change for any script + // following README.md, so it is pinned rather than assumed. + assert.equal(payload.nodes.length, payload.generated.files); + assert.equal(payload.edges.length, payload.generated.edges); + assert.ok(payload.zones.length > 0); + assert.ok(Object.keys(payload.typeInversions).length > 0); + + // The printed summary must agree with the payload it was derived from. + const inversions = Object.values(payload.typeInversions).reduce((sum, n) => sum + n, 0); + assert.match(stdout, new RegExp(`${payload.generated.files} files, ${payload.generated.edges} edges`)); + assert.match(stdout, new RegExp(`type-only spine inversions \\(R6\\): ${inversions}`)); + const reachable = payload.edges.filter(([, , , flags]) => (flags & 2) !== 0).length; + assert.match(stdout, new RegExp(`reachable at distance >= 2: ${reachable}`)); +}); + +test('build.ts honours --out and reports the path it wrote', () => { + const out = join(mkdtempSync(join(tmpdir(), 'depgraph-')), 'custom.json'); + const { status, stdout } = runBuild(['--out', out]); + assert.equal(status, 0, stdout); + assert.ok(existsSync(out), `expected ${out} to exist`); + JSON.parse(readFileSync(out, 'utf8')); + assert.ok(stdout.includes('custom.json'), stdout); +}); + +test('build.ts falls back to the default path when --out has no value', () => { + // Not an error path today: a trailing `--out` is ignored rather than rejected. Pinned so the + // behaviour is a decision rather than an accident, and so changing it is a visible diff. + const { status, stdout } = runBuild(['--out']); + assert.equal(status, 0, stdout); + assert.ok(stdout.includes('.tmp/depgraph/graph.json'), stdout); +}); diff --git a/scripts/depgraph/model.ts b/scripts/depgraph/model.ts new file mode 100644 index 0000000000..790fea2c20 --- /dev/null +++ b/scripts/depgraph/model.ts @@ -0,0 +1,386 @@ +// Dependency-graph analysis model — pure functions over the layering gate's edge model. +// +// The graph is deliberately derived from `scripts/layering/model.ts` rather than a +// third-party extractor: the gate's file set (production `src/**/*.ts`, tests excluded), +// zone partition, edge kinds (value/type-only/dynamic), and cycle definition are already +// the repo's source of truth. A second extractor with its own resolution rules would +// visualize a graph the gate does not enforce. + +import { + backEdgePair, + classifyZone, + findValueImportCycles, + targetDagZone, + typeInversionPair, + type ResolvedImportEdge, +} from '../layering/model.ts'; + +export type EdgeKind = 'value' | 'type' | 'dynamic'; + +export type GraphEdge = { + from: string; + to: string; + kind: EdgeKind; + line: number; + /** Set when this edge is a ranked-spine back-edge (`R5`), as `from-zone -> to-zone`. */ + backEdge: string | null; + /** Set when this edge is a type-only spine inversion (`R6`), as `from-zone -> to-zone`. */ + typeInversion: string | null; + /** True when the same pair is also reachable through a longer path of the same weight class. */ + /** Target also reachable at distance >= 2. Reachability only — see the marker function. */ + transitivelyReachable: boolean; +}; + +export type GraphNode = { + id: string; + zone: string; + /** First two path segments — a finer cluster than the zone, used for layout gravity. */ + loc: number; + fanIn: number; + fanOut: number; + /** Index into `GraphData.cycles`, or -1. */ + cycle: number; +}; + +export type ZoneEdge = { + from: string; + to: string; + count: number; + valueCount: number; + backEdge: boolean; +}; + +export type GraphCycle = { + path: string[]; + /** `value` cycles are gate-rejected (R4); the others are gate-invisible by design. */ + kind: EdgeKind; +}; + +export type GraphData = { + nodes: GraphNode[]; + edges: GraphEdge[]; + zones: { id: string; classification: string; files: number; loc: number }[]; + zoneEdges: ZoneEdge[]; + cycles: GraphCycle[]; + /** Type-only spine inversions per zone pair, counted by the gate's rule. */ + typeInversions: Record; +}; + + +function countLines(source: string): number { + let lines = 1; + for (let index = 0; index < source.length; index++) { + if (source[index] === '\n') lines++; + } + return lines; +} + +function edgeKind(edge: ResolvedImportEdge): EdgeKind { + if (edge.dynamic) return 'dynamic'; + if (edge.typeOnly) return 'type'; + return 'value'; +} + +/** + * Deduplicate parsed import edges down to one edge per (from, to) pair, keeping the + * strongest kind. A file that imports both a type and a value from the same module has one + * dependency on it, and the value import is what constrains layering and cold-start. + */ +export function collapseEdges(edges: readonly ResolvedImportEdge[]): GraphEdge[] { + const strength: Record = { type: 0, dynamic: 1, value: 2 }; + const byPair = new Map(); + for (const edge of edges) { + if (edge.file === edge.target) continue; + const key = `${edge.file}\u0000${edge.target}`; + const kind = edgeKind(edge); + const existing = byPair.get(key); + if (existing && strength[existing.kind] >= strength[kind]) continue; + byPair.set(key, { + from: edge.file, + to: edge.target, + kind, + line: edge.line, + backEdge: backEdgePair(edge), + typeInversion: typeInversionPair(edge), + transitivelyReachable: false, + }); + } + return [...byPair.values()].sort( + (left, right) => left.from.localeCompare(right.from) || left.to.localeCompare(right.to), + ); +} + +/** + * Value-edge adjacency. Both the redundancy pass and the level computation walk the same + * subgraph — the one R4 guarantees is a DAG — so they share its construction rather than each + * rebuilding it. + */ +function valueSuccessors(edges: readonly GraphEdge[]): Map { + const successors = new Map(); + for (const edge of edges) { + if (edge.kind !== 'value') continue; + const list = successors.get(edge.from) ?? []; + list.push(edge.to); + successors.set(edge.from, list); + } + return successors; +} + +/** + * Flag value edges whose target is ALSO reachable from the source at distance >= 2. + * + * This is static module reachability and nothing more. It is emphatically NOT a removability + * claim, and the difference matters because the obvious reading is wrong: + * + * - Reachability does not carry BINDINGS. If `a` does `import { c } from './c'` while `b` only + * re-exports it under another name (`export { c as b } from './c'`), the path `a -> b -> c` + * exists and deleting `a -> c` still breaks `a`. The fixture in model.test.ts is exactly this + * shape. + * - It does not preserve EVALUATION. A module's side effects run when it is first imported; + * dropping a direct edge can change when, or whether from `a`'s perspective, that happens. + * - It says nothing about re-export chains being intentional. A direct import is frequently + * clearer than reaching through a barrel. + * + * So the output is a place to look, not a work list — and at ~1300 edges, a large one. Deciding + * whether any given edge can go needs symbol-level analysis this does not attempt. + */ +export function markTransitivelyReachableEdges(edges: GraphEdge[]): void { + const successors = valueSuccessors(edges); + for (const edge of edges) { + if (edge.kind !== 'value') continue; + edge.transitivelyReachable = reachableBeyondDirectEdge(edge, successors); + } +} + +/** + * Whether `edge.to` is reachable from `edge.from` WITHOUT using the direct edge, i.e. at + * distance >= 2. The frontier is seeded with the one-hop neighbours other than `to`, which is + * what excludes the direct hop without having to track path lengths. + */ +function reachableBeyondDirectEdge( + edge: GraphEdge, + successors: ReadonlyMap, +): boolean { + const seen = new Set([edge.from]); + const queue = (successors.get(edge.from) ?? []).filter((next) => next !== edge.to); + for (const next of queue) seen.add(next); + + for (let index = 0; index < queue.length; index++) { + for (const next of successors.get(queue[index]!) ?? []) { + if (next === edge.to) return true; + if (seen.has(next)) continue; + seen.add(next); + queue.push(next); + } + } + return false; +} + +/** + * Cycles over an edge subset that includes weaker edge kinds. `findValueImportCycles` + * covers the gate's R4 scope (static value edges); passing type-only and dynamic edges + * through the same detector surfaces the cycles the gate deliberately does not reject — + * still design signal, because a type-only cycle means two modules co-define one contract. + */ +export function collectCycles(edges: readonly ResolvedImportEdge[]): GraphCycle[] { + const asValue = (subset: readonly ResolvedImportEdge[]): ResolvedImportEdge[] => + subset.map((edge) => ({ ...edge, dynamic: false, typeOnly: false })); + + const valuePaths = findValueImportCycles(edges); + const valueKeys = new Set(valuePaths.map(cycleKey)); + const cycles: GraphCycle[] = valuePaths.map((path) => ({ path, kind: 'value' })); + + const staticEdges = edges.filter((edge) => !edge.dynamic); + for (const path of findValueImportCycles(asValue(staticEdges))) { + if (valueKeys.has(cycleKey(path))) continue; + valueKeys.add(cycleKey(path)); + cycles.push({ path, kind: 'type' }); + } + + for (const path of findValueImportCycles(asValue(edges))) { + if (valueKeys.has(cycleKey(path))) continue; + valueKeys.add(cycleKey(path)); + cycles.push({ path, kind: 'dynamic' }); + } + + return cycles; +} + +/** Rotation-independent identity for a cycle path, so the same loop is not reported twice. */ +function cycleKey(path: readonly string[]): string { + const members = [...new Set(path)].sort(); + return members.join('\u0000'); +} + +/** First cycle each file belongs to, so a node can point at its loop in one lookup. */ +function indexCyclesByFile(cycles: readonly GraphCycle[]): Map { + const cycleByFile = new Map(); + for (let index = 0; index < cycles.length; index++) { + for (const file of cycles[index]!.path) { + if (!cycleByFile.has(file)) cycleByFile.set(file, index); + } + } + return cycleByFile; +} + +/** One node per production file, with degrees accumulated from the collapsed edge list. */ +function buildNodes( + sources: ReadonlyMap, + edges: readonly GraphEdge[], + cycleByFile: ReadonlyMap, +): Map { + const nodes = new Map(); + for (const [file, source] of sources) { + nodes.set(file, { + id: file, + zone: targetDagZone(file), + loc: countLines(source), + fanIn: 0, + fanOut: 0, + cycle: cycleByFile.get(file) ?? -1, + }); + } + for (const edge of edges) { + const from = nodes.get(edge.from); + const to = nodes.get(edge.to); + if (from) from.fanOut++; + if (to) to.fanIn++; + } + return nodes; +} + +/** Busiest pair first, then alphabetical, so the output is stable across runs. */ +function compareZoneEdges(left: ZoneEdge, right: ZoneEdge): number { + return ( + right.count - left.count || + left.from.localeCompare(right.from) || + left.to.localeCompare(right.to) + ); +} + +/** The zone pair an edge crosses, or `null` when it stays inside one zone. */ +function crossedZonePair( + nodes: ReadonlyMap, + edge: GraphEdge, +): { from: string; to: string } | null { + const from = nodes.get(edge.from)?.zone; + const to = nodes.get(edge.to)?.zone; + if (from === undefined || to === undefined || from === to) return null; + return { from, to }; +} + +/** Cross-zone traffic, one entry per ordered zone pair. Same-zone edges are not boundary edges. */ +function aggregateZoneEdges( + nodes: ReadonlyMap, + edges: readonly GraphEdge[], +): ZoneEdge[] { + const zoneEdges = new Map(); + for (const edge of edges) { + const pair = crossedZonePair(nodes, edge); + if (!pair) continue; + const key = `${pair.from} ${pair.to}`; + const entry = zoneEdges.get(key) ?? { ...pair, count: 0, valueCount: 0, backEdge: false }; + entry.count++; + if (edge.kind === 'value') entry.valueCount++; + if (edge.backEdge) entry.backEdge = true; + zoneEdges.set(key, entry); + } + return [...zoneEdges.values()].sort(compareZoneEdges); +} + +/** Per-zone size, largest first — the "which boundary is big" view. */ +function aggregateZones(nodes: ReadonlyMap): GraphData['zones'] { + const stats = new Map(); + for (const node of nodes.values()) { + const entry = stats.get(node.zone) ?? { files: 0, loc: 0 }; + entry.files++; + entry.loc += node.loc; + stats.set(node.zone, entry); + } + return [...stats] + .map(([id, entry]) => ({ + id, + classification: classifyZone(id), + files: entry.files, + loc: entry.loc, + })) + .sort((left, right) => right.loc - left.loc); +} + +/** + * Type-only spine inversions per zone pair, counted the way the gate counts them: once per FILE + * pair, over the raw resolved edges. + * + * Deliberately not derived from the collapsed edge list. `collapseEdges` keeps one edge per file + * pair, strongest kind wins, and `dynamic` outranks `type` — so a module imported both lazily and + * for its types would collapse to `dynamic` and drop out of the count. No such pair exists today, + * but the count feeding a CI equality check must not be able to drift for a reason unrelated to + * layering. + */ +export function typeInversionsByPair( + edges: readonly ResolvedImportEdge[], +): Record { + const seen = new Set(); + const byPair = new Map(); + for (const edge of edges) { + const pair = typeInversionPair(edge); + if (!pair) continue; + const identity = `${edge.file} -> ${edge.target}`; + if (seen.has(identity)) continue; + seen.add(identity); + byPair.set(pair, (byPair.get(pair) ?? 0) + 1); + } + return Object.fromEntries([...byPair].sort(([left], [right]) => left.localeCompare(right))); +} + +export function buildGraph( + sources: ReadonlyMap, + edges: readonly ResolvedImportEdge[], +): GraphData { + const collapsed = collapseEdges(edges); + markTransitivelyReachableEdges(collapsed); + const cycles = collectCycles(edges); + const nodes = buildNodes(sources, collapsed, indexCyclesByFile(cycles)); + + return { + nodes: [...nodes.values()].sort((left, right) => left.id.localeCompare(right.id)), + edges: collapsed, + zones: aggregateZones(nodes), + zoneEdges: aggregateZoneEdges(nodes, collapsed), + cycles, + typeInversions: typeInversionsByPair(edges), + }; +} + +/** + * Longest distance from each node to a sink over value edges. The layering gate rejects + * production value-import cycles (R4), so that subgraph is a DAG and the height is + * well-defined; the `visiting` guard only exists so a future cycle degrades instead of + * overflowing the stack. + */ +export function computeLevels( + nodes: readonly GraphNode[], + edges: readonly GraphEdge[], +): Map { + const successors = valueSuccessors(edges); + + const levels = new Map(); + const visiting = new Set(); + + const height = (id: string): number => { + const cached = levels.get(id); + if (cached !== undefined) return cached; + if (visiting.has(id)) return 0; + visiting.add(id); + let best = 0; + for (const next of successors.get(id) ?? []) { + best = Math.max(best, height(next) + 1); + } + visiting.delete(id); + levels.set(id, best); + return best; + }; + + for (const node of nodes) height(node.id); + return levels; +} diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index 862c94909d..e2171aff17 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -206,7 +206,9 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): Violation[] { // // The counts may only go DOWN. Fixing edges without lowering the number fails too, so the // baseline cannot quietly stop describing the tree. -const TYPE_INVERSION_BASELINE: Readonly> = { +// Exported so scripts/depgraph can assert its own graph build reproduces it — see the +// baseline-parity test there. The gate remains the authority; the report follows. +export const TYPE_INVERSION_BASELINE: Readonly> = { 'commands -> client': 28, 'commands -> daemon-server': 1, 'contracts -> client': 1, diff --git a/scripts/layering/model.ts b/scripts/layering/model.ts index e0fe1dc4f0..da5f380b5c 100644 --- a/scripts/layering/model.ts +++ b/scripts/layering/model.ts @@ -52,6 +52,15 @@ const TARGET_DAG_RANK = new Map([ export const RANKED_ZONES: ReadonlySet = new Set(TARGET_DAG_RANK.keys()); +/** + * Spine rank of a zone, or `null` when the zone is intentionally unranked. The gate compares + * ranks internally; this is exported for the dependency-graph report, which records the rank per + * zone so a consumer can tell an inversion from an ordinary edge without re-deriving the spine. + */ +export function zoneRank(zone: string): number | null { + return TARGET_DAG_RANK.get(zone) ?? null; +} + // The one zone deliberately left OUT of the ranked spine. It is NOT unenforced: every file // in it is still subject to the global production value-import cycle rejection (R4) and the // R1-R3 move rules. It opts out of spine back-edge ranking because `(root)` holds the @@ -144,7 +153,7 @@ export function topFolder(file: string): string { return match ? match[1]! : '(root)'; } -function targetDagZone(file: string): string { +export function targetDagZone(file: string): string { if (file.startsWith('src/daemon/client/')) return 'daemon-client'; if (file.startsWith('src/daemon/')) return 'daemon-server'; return topFolder(file);