mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
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
This commit is contained in:
committed by
GitHub
parent
5eebba5fcd
commit
6a24dc1b2d
@@ -115,15 +115,14 @@ jobs:
|
||||
uses: ./.github/actions/run-gate
|
||||
with: { gate: layering }
|
||||
|
||||
- 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 beside
|
||||
# the layering gate so the two can never be green independently.
|
||||
# Model tests for the dependency-graph report and its blast-radius query. The report
|
||||
# reads the gate's model (scripts/layering/model.ts) and applies the gate's own R6
|
||||
# counting rule, so it is not a second measurement of TYPE_INVERSION_BASELINE.
|
||||
- name: Check the depgraph report model
|
||||
uses: ./.github/actions/run-gate
|
||||
with: { gate: depgraph }
|
||||
|
||||
# Tests for the TMPDIR redirection itself, hidden the same way as the check above.
|
||||
# Tests for the TMPDIR redirection itself.
|
||||
#
|
||||
# Deliberately NOT in Coverage next to `check:tmpdir-leaks`, where the subject matter
|
||||
# would put it: vitest-tmpdir-global-setup.test.ts proves the lifecycle by spawning a
|
||||
|
||||
@@ -95,7 +95,7 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [
|
||||
gate('affected-selector', 'Affected-check selector model', 'check:affected:test'),
|
||||
gate('gate-manifest', 'Gate manifest — every gate owned and wired', 'check:gate-manifest'),
|
||||
gate('gate-manifest-model', 'Gate manifest model', 'check:gate-manifest:test'),
|
||||
gate('depgraph', 'Dependency graph report agrees with the gate', 'depgraph:test'),
|
||||
gate('depgraph', 'Dependency graph report model', 'depgraph:test'),
|
||||
gate('tmpdir-leaks', 'Leaked test tmpdir detector', 'check:tmpdir-leaks'),
|
||||
gate('tmpdir-leaks-model', 'TMPDIR redirection model', 'check:tmpdir-leaks:test'),
|
||||
gate('coverage-model', 'Changed-line coverage model', 'check:coverage-changed:test'),
|
||||
|
||||
@@ -65,8 +65,7 @@ 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.
|
||||
# would drop out. `typeInversions` is counted by the gate's own rule.
|
||||
node -e "const j=require('./.tmp/depgraph/graph.json');
|
||||
Object.entries(j.typeInversions)
|
||||
.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.
|
||||
|
||||
## 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.
|
||||
`pnpm check:layering` is. The report reads the same model (`scripts/layering/model.ts`) and applies
|
||||
the gate's own counting rule — `typeInversionsByPair` counts once per file pair over the raw edges,
|
||||
exactly as `checkTypeInversions` in `scripts/layering/check.ts` does — so `typeInversions` reproduces
|
||||
`TYPE_INVERSION_BASELINE` by construction, not by a second measurement. CI used to assert that
|
||||
equality; it was a duplicate detector of the same code path and was removed. In particular the count
|
||||
does NOT come from the collapsed edge list, where `dynamic` outranks `type` and a module imported
|
||||
both lazily and for its types would drop out.
|
||||
|
||||
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.
|
||||
If the report ever disagrees with the gate, the gate is right.
|
||||
|
||||
## Why it reuses the layering gate
|
||||
|
||||
|
||||
@@ -4,7 +4,6 @@ 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 { ARCHITECTURE_OWNERSHIP } from '../layering/architecture-ownership.ts';
|
||||
import { resolveImportEdges } from '../layering/model.ts';
|
||||
import {
|
||||
@@ -320,31 +319,28 @@ test('buildGraph reports zone membership, degrees, and cross-zone edge counts',
|
||||
);
|
||||
});
|
||||
|
||||
// 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));
|
||||
// `typeInversions` is the report's view of R6 over the RAW edges: a module imported both lazily
|
||||
// and for its types keeps its type-only edge, where `collapseEdges` ranks `dynamic` above `type`
|
||||
// and would lose it.
|
||||
test('typeInversionsByPair counts raw type-only edges once per file pair', () => {
|
||||
const files = sources({
|
||||
'src/commands/tap.ts': 'export type TapOptions = { retries: number };\n',
|
||||
'src/core/interactors/tap.ts': [
|
||||
"import type { TapOptions } from '../../commands/tap.ts';",
|
||||
"import type { TapOptions as Again } from '../../commands/tap.ts';",
|
||||
'export type Both = TapOptions | Again;',
|
||||
].join('\n'),
|
||||
'src/core/interactors/lazy.ts': [
|
||||
"import type { TapOptions } from '../../commands/tap.ts';",
|
||||
"export const load = (): Promise<unknown> => import('../../commands/tap.ts');",
|
||||
'export type Options = TapOptions;',
|
||||
].join('\n'),
|
||||
'src/core/interactors/value.ts': "import '../../commands/tap.ts';\n",
|
||||
});
|
||||
const edges = resolveImportEdges(files);
|
||||
|
||||
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.',
|
||||
);
|
||||
assert.deepEqual(typeInversionsByPair(edges), { 'core -> commands': 2 });
|
||||
assert.deepEqual(buildGraph(files, edges).typeInversions, { 'core -> commands': 2 });
|
||||
});
|
||||
|
||||
// A raw NUL byte in a source file makes Git classify it as binary, which hides the whole diff
|
||||
|
||||
@@ -414,8 +414,8 @@ function aggregateZones(nodes: ReadonlyMap<string, GraphNode>): GraphData['zones
|
||||
* 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.
|
||||
* but the report's inversion count must not drift for a reason unrelated to layering. This is
|
||||
* report data only; the layering gate owns and enforces the inversion ratchet.
|
||||
*/
|
||||
export function typeInversionsByPair(edges: readonly ResolvedImportEdge[]): Record<string, number> {
|
||||
const seen = new Set<string>();
|
||||
|
||||
@@ -233,8 +233,8 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation
|
||||
// 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
|
||||
// cannot quietly stop describing the tree.
|
||||
//
|
||||
// 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.
|
||||
// This gate is the sole owner of the ratchet. The depgraph report reuses the shared inversion
|
||||
// classifier for observability, but does not compare its report output with this baseline.
|
||||
export const TYPE_INVERSION_BASELINE: Readonly<Record<string, number>> = {
|
||||
'commands -> client': 3,
|
||||
'commands -> daemon-server': 1,
|
||||
|
||||
Reference in New Issue
Block a user