Skip to content

Commit 6a24dc1

Browse files
authored
chore(depgraph): stop re-deriving the layering inversion baseline (#2241)
* chore(depgraph): stop re-deriving the layering inversion baseline The report's typeInversionsByPair and the gate's checkTypeInversions run the same loop over the same resolveImportEdges output, so asserting that the report reproduces TYPE_INVERSION_BASELINE over the real tree checked one code path against itself. Replace the tree-wide cross-check with a synthetic test of the report's own counting rule (raw edges, once per file pair). * chore(gates): retitle the depgraph gate as the report's model tests The Layering Guard step no longer claims to agree the report with the gate; it runs the depgraph model and blast-radius tests, which the gate manifest requires a registered check to own. * docs: clarify inversion ratchet ownership
1 parent 5eebba5 commit 6a24dc1

6 files changed

Lines changed: 40 additions & 51 deletions

File tree

.github/workflows/ci.yml

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,14 @@ jobs:
115115
uses: ./.github/actions/run-gate
116116
with: { gate: layering }
117117

118-
- name: Check the depgraph report agrees with the gate
119-
# scripts/depgraph reads the same model as the gate, so its inversion count must
120-
# reproduce TYPE_INVERSION_BASELINE. Free two-sources check: if the tree changes
121-
# and only one side is updated, this fails and names the difference. Runs beside
122-
# the layering gate so the two can never be green independently.
118+
# Model tests for the dependency-graph report and its blast-radius query. The report
119+
# reads the gate's model (scripts/layering/model.ts) and applies the gate's own R6
120+
# counting rule, so it is not a second measurement of TYPE_INVERSION_BASELINE.
121+
- name: Check the depgraph report model
123122
uses: ./.github/actions/run-gate
124123
with: { gate: depgraph }
125124

126-
# Tests for the TMPDIR redirection itself, hidden the same way as the check above.
125+
# Tests for the TMPDIR redirection itself.
127126
#
128127
# Deliberately NOT in Coverage next to `check:tmpdir-leaks`, where the subject matter
129128
# would put it: vitest-tmpdir-global-setup.test.ts proves the lifecycle by spawning a

scripts/check-affected/checks.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [
9595
gate('affected-selector', 'Affected-check selector model', 'check:affected:test'),
9696
gate('gate-manifest', 'Gate manifest — every gate owned and wired', 'check:gate-manifest'),
9797
gate('gate-manifest-model', 'Gate manifest model', 'check:gate-manifest:test'),
98-
gate('depgraph', 'Dependency graph report agrees with the gate', 'depgraph:test'),
98+
gate('depgraph', 'Dependency graph report model', 'depgraph:test'),
9999
gate('tmpdir-leaks', 'Leaked test tmpdir detector', 'check:tmpdir-leaks'),
100100
gate('tmpdir-leaks-model', 'TMPDIR redirection model', 'check:tmpdir-leaks:test'),
101101
gate('coverage-model', 'Changed-line coverage model', 'check:coverage-changed:test'),

scripts/depgraph/README.md

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ pnpm depgraph
6565
# Zone pairs that invert the ranked spine. Read `typeInversions` rather than deriving it from
6666
# `zoneEdges`: those counts come from the COLLAPSED edge list, where one edge per file pair
6767
# survives and `dynamic` outranks `type`, so a module imported both lazily and for its types
68-
# would drop out. `typeInversions` is counted by the gate's own rule and is what CI compares
69-
# against TYPE_INVERSION_BASELINE.
68+
# would drop out. `typeInversions` is counted by the gate's own rule.
7069
node -e "const j=require('./.tmp/depgraph/graph.json');
7170
Object.entries(j.typeInversions)
7271
.sort((a, b) => b[1] - a[1])
@@ -80,20 +79,15 @@ it returns an empty list, which is the gate passing, not a broken query.
8079

8180
## What is authoritative
8281

83-
`pnpm check:layering` is. The viewer reads the same model, so the numbers should agree — and that
84-
agreement is now enforced rather than hoped for: the **Layering Guard job runs
85-
`scripts/depgraph/model.test.ts`**, whose last test asserts this report's inversion count reproduces
86-
`TYPE_INVERSION_BASELINE`. If the tree changes and only one side is updated, CI fails and names the
87-
difference. The two cannot be green independently.
82+
`pnpm check:layering` is. The report reads the same model (`scripts/layering/model.ts`) and applies
83+
the gate's own counting rule — `typeInversionsByPair` counts once per file pair over the raw edges,
84+
exactly as `checkTypeInversions` in `scripts/layering/check.ts` does — so `typeInversions` reproduces
85+
`TYPE_INVERSION_BASELINE` by construction, not by a second measurement. CI used to assert that
86+
equality; it was a duplicate detector of the same code path and was removed. In particular the count
87+
does NOT come from the collapsed edge list, where `dynamic` outranks `type` and a module imported
88+
both lazily and for its types would drop out.
8889

89-
What that check proves precisely: the report's graph build, over the real tree, agrees with the
90-
gate's baseline. It is a cross-check of the extraction and the baseline against reality, not two
91-
independent algorithms — `typeInversionsByPair` deliberately applies the gate's counting rule (once
92-
per file pair, over raw edges) so the numbers cannot diverge for a reason unrelated to layering. In
93-
particular it does NOT count from the collapsed edge list, where `dynamic` outranks `type` and a
94-
module imported both lazily and for its types would drop out.
95-
96-
If they ever disagree, the gate is right and the baseline or the tree is wrong.
90+
If the report ever disagrees with the gate, the gate is right.
9791

9892
## Why it reuses the layering gate
9993

scripts/depgraph/model.test.ts

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { existsSync, mkdtempSync, readFileSync } from 'node:fs';
44
import { tmpdir } from 'node:os';
55
import { join } from 'node:path';
66
import { test } from 'node:test';
7-
import { listSourceFiles, TYPE_INVERSION_BASELINE } from '../layering/check.ts';
87
import { ARCHITECTURE_OWNERSHIP } from '../layering/architecture-ownership.ts';
98
import { resolveImportEdges } from '../layering/model.ts';
109
import {
@@ -320,31 +319,28 @@ test('buildGraph reports zone membership, degrees, and cross-zone edge counts',
320319
);
321320
});
322321

323-
// Two-sources-of-truth check, run by the Layering Guard job.
324-
//
325-
// The report and the gate read the same model, so their inversion counts must agree. This locks
326-
// that: if the tree changes and only one side is updated, or if the report's extraction diverges
327-
// from what the gate sees, this fails and names the difference.
328-
//
329-
// What it proves precisely: the report's own graph build, over the real tree, reproduces
330-
// TYPE_INVERSION_BASELINE. It is a cross-check of the extraction and the baseline against reality,
331-
// not two independent algorithms — `typeInversionsByPair` deliberately applies the gate's counting
332-
// rule so the numbers cannot differ for a reason unrelated to layering. The gate stays the
333-
// authority; if these disagree, the baseline or the tree is wrong, never this test.
334-
test("the report's inversion count reproduces the gate's TYPE_INVERSION_BASELINE", () => {
335-
const files = listSourceFiles();
336-
const sources = new Map(files.map((file) => [file, readFileSync(file, 'utf8')]));
337-
const actual = typeInversionsByPair(resolveImportEdges(sources));
322+
// `typeInversions` is the report's view of R6 over the RAW edges: a module imported both lazily
323+
// and for its types keeps its type-only edge, where `collapseEdges` ranks `dynamic` above `type`
324+
// and would lose it.
325+
test('typeInversionsByPair counts raw type-only edges once per file pair', () => {
326+
const files = sources({
327+
'src/commands/tap.ts': 'export type TapOptions = { retries: number };\n',
328+
'src/core/interactors/tap.ts': [
329+
"import type { TapOptions } from '../../commands/tap.ts';",
330+
"import type { TapOptions as Again } from '../../commands/tap.ts';",
331+
'export type Both = TapOptions | Again;',
332+
].join('\n'),
333+
'src/core/interactors/lazy.ts': [
334+
"import type { TapOptions } from '../../commands/tap.ts';",
335+
"export const load = (): Promise<unknown> => import('../../commands/tap.ts');",
336+
'export type Options = TapOptions;',
337+
].join('\n'),
338+
'src/core/interactors/value.ts': "import '../../commands/tap.ts';\n",
339+
});
340+
const edges = resolveImportEdges(files);
338341

339-
assert.deepEqual(
340-
actual,
341-
// Object key order differs between the two literals; compare as sorted entries.
342-
Object.fromEntries(
343-
Object.entries(TYPE_INVERSION_BASELINE).sort(([left], [right]) => left.localeCompare(right)),
344-
),
345-
'depgraph and scripts/layering/check.ts disagree about type-only spine inversions. ' +
346-
'Regenerate with `pnpm depgraph` and update TYPE_INVERSION_BASELINE, or fix the edge.',
347-
);
342+
assert.deepEqual(typeInversionsByPair(edges), { 'core -> commands': 2 });
343+
assert.deepEqual(buildGraph(files, edges).typeInversions, { 'core -> commands': 2 });
348344
});
349345

350346
// A raw NUL byte in a source file makes Git classify it as binary, which hides the whole diff

scripts/depgraph/model.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,8 +414,8 @@ function aggregateZones(nodes: ReadonlyMap<string, GraphNode>): GraphData['zones
414414
* Deliberately not derived from the collapsed edge list. `collapseEdges` keeps one edge per file
415415
* pair, strongest kind wins, and `dynamic` outranks `type` — so a module imported both lazily and
416416
* for its types would collapse to `dynamic` and drop out of the count. No such pair exists today,
417-
* but the count feeding a CI equality check must not be able to drift for a reason unrelated to
418-
* layering.
417+
* but the report's inversion count must not drift for a reason unrelated to layering. This is
418+
* report data only; the layering gate owns and enforces the inversion ratchet.
419419
*/
420420
export function typeInversionsByPair(edges: readonly ResolvedImportEdge[]): Record<string, number> {
421421
const seen = new Set<string>();

scripts/layering/check.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,8 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation
233233
// See docs/dependency-graph-findings.md §0 for the long form. The counts may only go DOWN. Fixing edges without lowering the number fails too, so the baseline
234234
// cannot quietly stop describing the tree.
235235
//
236-
// Exported so scripts/depgraph can assert its own graph build reproduces it — see the
237-
// baseline-parity test there. The gate remains the authority; the report follows.
236+
// This gate is the sole owner of the ratchet. The depgraph report reuses the shared inversion
237+
// classifier for observability, but does not compare its report output with this baseline.
238238
export const TYPE_INVERSION_BASELINE: Readonly<Record<string, number>> = {
239239
'commands -> client': 3,
240240
'commands -> daemon-server': 1,

0 commit comments

Comments
 (0)