mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
chore(gates): layering baselines ratchet against merge-base (#2299)
* refactor(layering): ratchet R6, R9 and R10 against the merge-base tree R6 type-spine inversions, R9's largest type cycle and R10's R7 ownership pressure now compare the working tree with the same measurement taken over the merge-base with origin/main, read through the shared committed-tree reader (one git ls-tree, one git cat-file --batch, no second checkout). Growth still fails with the same message shape, a shrink needs no edit, and no change can bank headroom by leaving a number above the tree. R9's per-zone check gains membership from the reference, so the overflow message names the file that joined instead of listing the whole zone. * chore(gates): delete the R6, R9 and R10 pins the merge-base now supplies TYPE_INVERSION_BASELINE, LARGEST_TYPE_CYCLE_ZONE_CEILINGS, TYPE_CYCLE_BASELINE and DAEMON_MODULARITY_BASELINE.sessionState were the hand-edited references these three ratchets compared against. The merge-base measurement replaces them, so there is no number left to leave above the tree and no entry to raise. externalDaemonTypesImporters stays: it names files, not a count.
This commit is contained in:
committed by
GitHub
parent
17e0c8688f
commit
006f2d9f60
@@ -100,8 +100,12 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 15
|
||||
steps:
|
||||
# The layering ratchets (R6/R9/R10) measure the merge-base with origin/main, which a
|
||||
# shallow checkout cannot reach.
|
||||
- name: Checkout
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
fetch-depth: 0
|
||||
|
||||
# The layering gate parses production sources with `oxc-parser`, so
|
||||
# dependencies are required; keep install-deps enabled.
|
||||
@@ -117,7 +121,7 @@ jobs:
|
||||
|
||||
# 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.
|
||||
# counting rule, so it is not a second measurement of the R6 ratchet.
|
||||
- name: Check the depgraph report model
|
||||
uses: ./.github/actions/run-gate
|
||||
with: { gate: depgraph }
|
||||
|
||||
@@ -22,8 +22,8 @@ const files = listSourceFiles();
|
||||
const sources = new Map(files.map((f) => [f, fs.readFileSync(f, 'utf8')]));
|
||||
const edges = resolveImportEdges(sources);
|
||||
|
||||
// e.g. R6 inversions per zone pair, deduplicated by file pair — reproduces
|
||||
// TYPE_INVERSION_BASELINE, so a mismatch means one of the two is stale.
|
||||
// e.g. R6 inversions per zone pair, deduplicated by file pair — the same count the gate
|
||||
// ratchets against the merge-base with origin/main.
|
||||
const seen = new Set<string>();
|
||||
const byPair = new Map<string, number>();
|
||||
for (const edge of edges) {
|
||||
@@ -126,8 +126,9 @@ narrow name replaced both.
|
||||
from `daemon-command-registry.ts` to key an exhaustive owner-file map; that remaining inversion
|
||||
is the commands-zone consumer, not a second source of truth for the union.
|
||||
|
||||
All remaining inversions are argued at `TYPE_INVERSION_BASELINE` in `scripts/layering/check.ts`, next
|
||||
to the numbers they explain.
|
||||
All remaining inversions are argued here. R6 (`scripts/layering/type-inversion-ratchet.ts`) records
|
||||
no numbers of its own: its reference is the same count taken at the merge-base with `origin/main`,
|
||||
so a zone pair can only shrink.
|
||||
|
||||
## 0b. The biggest structural finding is not an inversion
|
||||
|
||||
@@ -156,13 +157,13 @@ but it is a comprehension one, and it is the single largest obstacle to reading
|
||||
isolation. At the current measured commit it spans `commands` (33), `daemon-server` (30),
|
||||
`platforms` (19), `core` (12), root composition (5), `contracts` (2), and `client` (1).
|
||||
|
||||
Now ratcheted for growth by **R9** (`TYPE_CYCLE_BASELINE`, derived from the zone ceilings in
|
||||
`scripts/layering/daemon-modularity.ts`), so it cannot get worse
|
||||
Now ratcheted for growth by **R9** (`scripts/layering/daemon-modularity.ts`), so it cannot get worse
|
||||
while nobody is looking — a type-only import that closes a new loop fails the gate, verified by
|
||||
adding one type-only import that closes a loop and watching the gate reject it. It was growth-only
|
||||
here; #1781 A6 made it an equality pin, so a baseline left above the measured size fails too and a
|
||||
shrink is banked by the change that earns it. The refactor itself is still deliberately not
|
||||
attempted; it starts at those four hubs.
|
||||
here; #1781 A6 made it an equality pin, and the pin is now the merge-base's own measurement
|
||||
(`scripts/layering/ratchet-reference.ts`), so a shrink is banked the moment it merges and there is
|
||||
no slack left to spend. The refactor itself is still deliberately not attempted; it starts at those
|
||||
four hubs.
|
||||
|
||||
### The facade cycle: investigated, no narrower port exists
|
||||
|
||||
@@ -183,14 +184,15 @@ duplicate the public API shape — a second source of truth for it — or derive
|
||||
carry the same dependency.
|
||||
|
||||
Those four files are therefore the minimum number of naming sites, not an accident: they are the
|
||||
choke point. Accepted as a position, argued at `TYPE_INVERSION_BASELINE`. The option this section
|
||||
choke point. Accepted as a position, argued in §0 above. The option this section
|
||||
used to hold open — moving `NAVIGATION_COMMAND_PROJECTIONS` out of `commands/` — was answered by
|
||||
deleting it: five direct signatures replaced the registry, so there is no longer a projection
|
||||
registry whose home is in question.
|
||||
|
||||
## 1. The two remaining type-inversion clusters
|
||||
|
||||
`TYPE_INVERSION_BASELINE` in `scripts/layering/check.ts` holds both, with the reasoning inline.
|
||||
§0 above holds both, with the reasoning inline; the gate measures them against the merge-base
|
||||
rather than recording them.
|
||||
|
||||
**28 + 1 edges → `client/client-types.ts`** — *done, mostly.* Now 5 edges. The vocabulary moved into
|
||||
the `contracts/client-*.ts` family files — one file per command/domain family, largest 137 LOC —
|
||||
|
||||
@@ -44,6 +44,56 @@ export function renamedSince(repoRoot: string, base: string): ReadonlyMap<string
|
||||
return renamed;
|
||||
}
|
||||
|
||||
/** Every path tracked at `treeish`, repo-root-relative, from ONE `git ls-tree`. */
|
||||
function listCommittedTree(repoRoot: string, treeish: string): string[] {
|
||||
const listing = git(repoRoot, ['ls-tree', '-r', '--name-only', '-z', treeish]).toString('utf8');
|
||||
return listing.split('\0').filter(Boolean);
|
||||
}
|
||||
|
||||
/**
|
||||
* The sources this reader serves, split by kind: production TypeScript under `src/` and
|
||||
* `packages/<pkg>/src/`, and workspace package manifests. One definition, so a consumer reading
|
||||
* a committed tree cannot classify it differently from the walker.
|
||||
*/
|
||||
function committedSourceSet(tracked: readonly string[]): {
|
||||
sources: string[];
|
||||
manifests: string[];
|
||||
} {
|
||||
return {
|
||||
sources: tracked.filter((file) => WALKED_SOURCE.test(file) && isProductionSourceFile(file)),
|
||||
manifests: tracked.filter((file) => WALKED_MANIFEST.test(file)),
|
||||
};
|
||||
}
|
||||
|
||||
/** Contents of `files` at `treeish`, through ONE long-lived `git cat-file --batch`. */
|
||||
function readCommittedBlobs(
|
||||
repoRoot: string,
|
||||
treeish: string,
|
||||
files: readonly string[],
|
||||
): Map<string, string> {
|
||||
if (files.length === 0) return new Map();
|
||||
const requests = files.map((file) => `${treeish}:${file}\n`).join('');
|
||||
return parseCatFileBatch(git(repoRoot, ['cat-file', '--batch'], requests), files);
|
||||
}
|
||||
|
||||
/**
|
||||
* The same enumeration and blob read as `createCommittedSourceTree`, handed over as text: the
|
||||
* production sources and workspace manifests committed at `treeish`. A whole-tree measurement
|
||||
* (the layering ratchets) needs the corpus rather than a reader, and taking it from here is what
|
||||
* keeps its file set identical to the closure walker's.
|
||||
*/
|
||||
// fallow-ignore-next-line unused-export -- consumed by scripts/layering, outside fallow's scope
|
||||
export function readCommittedSources(
|
||||
repoRoot: string,
|
||||
treeish: string,
|
||||
): { sources: Map<string, string>; manifests: Map<string, string> } {
|
||||
const { sources, manifests } = committedSourceSet(listCommittedTree(repoRoot, treeish));
|
||||
const blobs = readCommittedBlobs(repoRoot, treeish, [...sources, ...manifests]);
|
||||
const only = (files: readonly string[]) =>
|
||||
new Map(files.flatMap((file) => (blobs.has(file) ? [[file, blobs.get(file)!] as const] : [])));
|
||||
return { sources: only(sources), manifests: only(manifests) };
|
||||
}
|
||||
|
||||
/**
|
||||
* `<sha> blob <size>\n<size bytes>\n` per hit and `<request> missing\n` per miss, in request
|
||||
* order. Sizes are bytes, so this walks the raw buffer rather than a string offset.
|
||||
@@ -82,15 +132,11 @@ function directoriesOf(files: ReadonlySet<string>): Set<string> {
|
||||
* whole tree, never one per file. A read outside that set is a widening request, not a fallback.
|
||||
*/
|
||||
export function createCommittedSourceTree(repoRoot: string, treeish: string): SourceTreeReader {
|
||||
const listing = git(repoRoot, ['ls-tree', '-r', '--name-only', '-z', treeish]).toString('utf8');
|
||||
const tracked = new Set(listing.split('\0').filter(Boolean));
|
||||
const listing = listCommittedTree(repoRoot, treeish);
|
||||
const tracked = new Set(listing);
|
||||
const directories = directoriesOf(tracked);
|
||||
const walked = [...tracked].filter(
|
||||
(file) =>
|
||||
WALKED_MANIFEST.test(file) || (WALKED_SOURCE.test(file) && isProductionSourceFile(file)),
|
||||
);
|
||||
const requests = walked.map((file) => `${treeish}:${file}\n`).join('');
|
||||
const contents = parseCatFileBatch(git(repoRoot, ['cat-file', '--batch'], requests), walked);
|
||||
const { sources, manifests } = committedSourceSet(listing);
|
||||
const contents = readCommittedBlobs(repoRoot, treeish, [...sources, ...manifests]);
|
||||
const relative = (file: string) => path.relative(repoRoot, file).split(path.sep).join('/');
|
||||
return {
|
||||
exists: (file) => tracked.has(relative(file)) || directories.has(relative(file)),
|
||||
|
||||
@@ -81,9 +81,10 @@ it returns an empty list, which is the gate passing, not a broken query.
|
||||
|
||||
`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
|
||||
exactly as `typeInversionCounts` in `scripts/layering/model.ts` does — so `typeInversions` reproduces
|
||||
the gate's R6 measurement by construction, not by a second measurement. The gate compares that
|
||||
measurement with the merge-base's; CI used to assert the report agreed with a recorded baseline,
|
||||
which 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.
|
||||
|
||||
|
||||
+41
-123
@@ -22,10 +22,9 @@
|
||||
// record is store-owned mutable state that any daemon module can write; and the terminal
|
||||
// concrete-platform boundary (R65), which rejects every import form into the retired
|
||||
// src/platforms path or a platform package.
|
||||
// - Over the TYPE GRAPH: the largest type-level import cycle is pinned by
|
||||
// equality (R9). R4 keeps the value graph acyclic, so these cycles are free at
|
||||
// runtime but bound what can be read in isolation; growth fails, and so does a
|
||||
// baseline left above the measured size.
|
||||
// - Over the TYPE GRAPH: the largest type-level import cycle may not grow past the
|
||||
// merge-base (R9). R4 keeps the value graph acyclic, so these cycles are free at
|
||||
// runtime but bound what can be read in isolation.
|
||||
// - Across the DAEMON MODULARITY MIGRATION: R7 ownership pressure and external
|
||||
// daemon/types.ts importers only shrink, R9 zone membership cannot grow or absorb
|
||||
// engine files, and planned logical modules start with zero forbidden/internal imports (R10).
|
||||
@@ -37,6 +36,9 @@
|
||||
// code cannot manufacture or repair a narrowed runtime proof (R66).
|
||||
// - Over CONTRACTS PRODUCTION SOURCE: contracts owns vocabulary only — host, process, and timer
|
||||
// mechanics belong in capture-kit or an adapter (R18).
|
||||
// R6, R9, and the R10 R7 counts are ratchets with no written-down reference: each is the same
|
||||
// measurement taken over the merge-base with origin/main (`ratchet-reference.ts`), so growth
|
||||
// fails, a shrink needs no edit, and no change can bank headroom.
|
||||
// `(root)` holds entrypoints and composition roots. The retired `src/utils` zone is deliberately
|
||||
// outside the spine and is rejected separately by R14; extracted workspace package zones are
|
||||
// classified separately and held behind R11 instead of the src folder spine.
|
||||
@@ -56,13 +58,19 @@ import {
|
||||
import {
|
||||
backEdgePair,
|
||||
findValueImportCycles,
|
||||
largestTypeCycleMembers,
|
||||
memoizedImportParser,
|
||||
resolveImportEdges,
|
||||
topFolder,
|
||||
typeInversionPair,
|
||||
type LayeringViolation,
|
||||
type ResolvedImportEdge,
|
||||
} from './model.ts';
|
||||
import { checkTypeInversions } from './type-inversion-ratchet.ts';
|
||||
import {
|
||||
measureRatchets,
|
||||
mergeBaseRatchets,
|
||||
type LayeringRatchets,
|
||||
type MergeBaseRatchets,
|
||||
} from './ratchet-reference.ts';
|
||||
import {
|
||||
checkDaemonModularityRatchets,
|
||||
checkRetiredInteractionPaths,
|
||||
@@ -224,110 +232,6 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation
|
||||
});
|
||||
}
|
||||
|
||||
// Catches: a type-only import against the ranked spine's declared order — a design-level
|
||||
// dependency (zone A is stated in terms of zone B) that R5 is blind to because it costs
|
||||
// nothing at runtime, so nothing else flags "the type shape leaks the wrong direction."
|
||||
// Evidence: the R5-adjacent commits in check.ts's history introduced this ratchet; the 61-to-5
|
||||
// reduction and the two remaining deliberate inversions are recorded below and in
|
||||
// docs/dependency-graph-findings.md.
|
||||
// Cost: not attributed (folded into check.ts's whole-graph pass; no standalone module or test
|
||||
// file to size separately).
|
||||
// Kill criterion: none enforced today; retire only by maintainer decision that type-only spine
|
||||
// inversions no longer matter. Reaching zero remaining inversions does not retire it: at zero
|
||||
// the ratchet is what keeps the count from regrowing, and tsc never rejects a type-only edge.
|
||||
//
|
||||
// R6 ratchet: type-only spine inversions, per zone pair. R5 cannot see these (a type-only import
|
||||
// is free at runtime), but "zone A is declared in terms of zone B" is still a boundary claim, and
|
||||
// ranking type edges surfaced 61 of them. Down to 5, and every one of the 5 is now a deliberate
|
||||
// architectural position rather than a misplaced declaration:
|
||||
//
|
||||
// commands/mcp -> client (4) `AgentDeviceClient`, used as an opaque handle ("the client this
|
||||
// command runs against"). The facade no longer reaches back into
|
||||
// commands/ — the navigation projection it was once built from is
|
||||
// retired — so this is no longer a zone-level cycle, just a port
|
||||
// that would have to cover the whole facade: 4 files NAME it, but 26
|
||||
// call sites use methods across 13 of its namespaces, so any port
|
||||
// would re-declare the public API. R5 is zero here: nothing imports
|
||||
// the client at runtime, only its type.
|
||||
//
|
||||
// commands -> daemon-server (1) `DaemonCommandRoute` is declared in core so descriptors can
|
||||
// name a route without importing the daemon. `command-explain.ts`
|
||||
// still type-imports the re-export from `daemon-command-registry.ts`
|
||||
// to key an exhaustive `Record<DaemonCommandRoute, string>` of
|
||||
// owner files; that remaining inversion is the commands-zone
|
||||
// consumer, not a second source of truth for the union.
|
||||
//
|
||||
// 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.
|
||||
//
|
||||
// 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,
|
||||
'mcp -> client': 1,
|
||||
};
|
||||
|
||||
function checkTypeInversions(edges: readonly ResolvedImportEdge[]): LayeringViolation[] {
|
||||
const seen = new Set<string>();
|
||||
const countsByPair = new Map<string, number>();
|
||||
const firstEdgeByPair = new Map<string, ResolvedImportEdge>();
|
||||
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);
|
||||
countsByPair.set(pair, (countsByPair.get(pair) ?? 0) + 1);
|
||||
if (!firstEdgeByPair.has(pair)) firstEdgeByPair.set(pair, edge);
|
||||
}
|
||||
|
||||
const violations: LayeringViolation[] = [];
|
||||
for (const [pair, count] of [...countsByPair].sort(([left], [right]) =>
|
||||
left.localeCompare(right),
|
||||
)) {
|
||||
const allowed = TYPE_INVERSION_BASELINE[pair];
|
||||
const edge = firstEdgeByPair.get(pair)!;
|
||||
if (allowed === undefined) {
|
||||
violations.push({
|
||||
rule: 'R6 type-spine-inversion',
|
||||
file: edge.file,
|
||||
line: edge.line,
|
||||
message:
|
||||
`new type-only ${pair} inversion (${count} edge(s), e.g. ${edge.file} -> ${edge.target}). ` +
|
||||
`Declare the shared type below both zones instead of adding it to TYPE_INVERSION_BASELINE.`,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (count > allowed) {
|
||||
violations.push({
|
||||
rule: 'R6 type-spine-inversion',
|
||||
file: edge.file,
|
||||
line: edge.line,
|
||||
message:
|
||||
`type-only ${pair} inversions grew to ${count} (baseline ${allowed}). ` +
|
||||
`Move the shared type below both zones; the baseline may only shrink.`,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
for (const [pair, allowed] of Object.entries(TYPE_INVERSION_BASELINE)) {
|
||||
const count = countsByPair.get(pair) ?? 0;
|
||||
if (count >= allowed) continue;
|
||||
const message =
|
||||
count === 0
|
||||
? `type-only ${pair} inversions are all gone — delete this entry from TYPE_INVERSION_BASELINE.`
|
||||
: `type-only ${pair} inversions dropped to ${count} — lower TYPE_INVERSION_BASELINE to ${count}.`;
|
||||
violations.push({
|
||||
rule: 'R6 type-spine-inversion',
|
||||
file: 'scripts/layering/check.ts',
|
||||
line: 1,
|
||||
message,
|
||||
});
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
function checkSessionStateOwnership(sources: ReadonlyMap<string, string>): LayeringViolation[] {
|
||||
const types = sources.get('src/daemon/types.ts');
|
||||
if (!types) {
|
||||
@@ -438,17 +342,23 @@ function checkSessionStateOwnership(sources: ReadonlyMap<string, string>): Layer
|
||||
function report(
|
||||
files: readonly string[],
|
||||
violations: readonly LayeringViolation[],
|
||||
typeCycle: number,
|
||||
ratchets: LayeringRatchets,
|
||||
reference: MergeBaseRatchets,
|
||||
): number {
|
||||
if (violations.length === 0) {
|
||||
const inversions = Object.values(ratchets.typeInversions).reduce(
|
||||
(sum, count) => sum + count,
|
||||
0,
|
||||
);
|
||||
process.stdout.write(
|
||||
`Layering guard: OK — ${files.length} source files satisfy R2 and contain no ` +
|
||||
`value-import cycles (both checked globally); the ranked target spine contains no ` +
|
||||
`back-edges; the ranked spine's type-only inversions match the R6 ratchet (${Object.values(TYPE_INVERSION_BASELINE).reduce((sum, count) => sum + count, 0)} remaining); ` +
|
||||
`back-edges; the ranked spine's type-only inversions hold at or under the merge-base ` +
|
||||
`${reference.ref.slice(0, 10)} per zone pair (R6, ${inversions} remaining); ` +
|
||||
`${RETIRED_PATH_RULES.R14.rule} permits no tracked paths under retired src/utils; ` +
|
||||
`all ${sessionStateFieldCount()} SessionState fields are classified and every write is ` +
|
||||
`inside its declared owner (R7); the largest type-level cycle is ${typeCycle} files ` +
|
||||
`(R9); ${daemonModularitySummary()}; ` +
|
||||
`inside its declared owner (R7); the largest type-level cycle is ` +
|
||||
`${ratchets.largestTypeCycle.length} files (R9); ${daemonModularitySummary(reference)}; ` +
|
||||
`${packageBoundariesSummary(repoRoot)}; ${platformPackagePolicySummary()}; ` +
|
||||
`runtime facts remain the only device-command admission authority and daemon code cannot ` +
|
||||
`manufacture narrowed runtime proof (R66); and R65 keeps production src/daemon free of ` +
|
||||
@@ -485,7 +395,10 @@ export type LayeringContext = Readonly<{
|
||||
allTypeScriptSources: ReadonlyMap<string, string>;
|
||||
trackedSrcUtilsFiles: readonly string[];
|
||||
edges: readonly ResolvedImportEdge[];
|
||||
typeCycleMembers: readonly string[];
|
||||
/** The ratcheted measurements of this tree. */
|
||||
ratchets: LayeringRatchets;
|
||||
/** The same measurements at the merge-base with origin/main. */
|
||||
reference: LayeringRatchets;
|
||||
}>;
|
||||
|
||||
export type LayeringRule = (context: LayeringContext) => LayeringViolation[];
|
||||
@@ -544,10 +457,11 @@ export const LAYERING_RULES: Readonly<Record<LayeringRuleId, LayeringRule>> = {
|
||||
'selector-pipeline-ownership': (context) =>
|
||||
selectorPipelineOwnershipViolations(context.edges, workspaceSpecifierTargets(repoRoot)),
|
||||
'back-edges': (context) => checkBackEdges(context.edges),
|
||||
'type-spine-inversions': (context) => checkTypeInversions(context.edges),
|
||||
'type-spine-inversions': (context) =>
|
||||
checkTypeInversions(context.edges, context.reference.typeInversions),
|
||||
'session-state-ownership': (context) => checkSessionStateOwnership(context.sources),
|
||||
'daemon-modularity-ratchets': (context) => [
|
||||
...checkDaemonModularityRatchets(context.edges, context.typeCycleMembers),
|
||||
...checkDaemonModularityRatchets(context.edges, context.ratchets, context.reference),
|
||||
...checkRetiredSessionLifecyclePaths(context.sourceFiles),
|
||||
...checkRetiredSessionObservabilityPaths(context.sourceFiles),
|
||||
...checkRetiredSnapshotExecutionPaths(context.sourceFiles),
|
||||
@@ -579,20 +493,24 @@ export function main(): number {
|
||||
const sources = readSources(sourceFiles);
|
||||
const allTypeScriptSources = readSources(listTypeScriptFiles());
|
||||
const trackedSrcUtilsFiles = listTrackedSrcUtilsFiles(repoRoot);
|
||||
const edges = resolveImportEdges(sources, workspaceSpecifierTargets(repoRoot));
|
||||
// Computed once and threaded: the rule and the success line must report the same number.
|
||||
const typeCycleMembers = largestTypeCycleMembers(edges);
|
||||
const typeCycle = typeCycleMembers.length;
|
||||
// One memoizing parser for both trees: every file the merge-base shares with the working tree
|
||||
// is parsed once, whichever scan reaches it first.
|
||||
const parse = memoizedImportParser();
|
||||
const edges = resolveImportEdges(sources, workspaceSpecifierTargets(repoRoot), parse);
|
||||
// Measured once and threaded: the rules and the success line must report the same numbers.
|
||||
const ratchets = measureRatchets(sources, edges);
|
||||
const reference = mergeBaseRatchets(repoRoot, parse);
|
||||
const context: LayeringContext = {
|
||||
sourceFiles,
|
||||
sources,
|
||||
allTypeScriptSources,
|
||||
trackedSrcUtilsFiles,
|
||||
edges,
|
||||
typeCycleMembers,
|
||||
ratchets,
|
||||
reference,
|
||||
};
|
||||
const violations = Object.values(LAYERING_RULES).flatMap((rule) => rule(context));
|
||||
return report(sourceFiles, violations, typeCycle);
|
||||
return report(sourceFiles, violations, ratchets, reference);
|
||||
}
|
||||
|
||||
if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) {
|
||||
|
||||
@@ -7,10 +7,9 @@ import {
|
||||
checkRetiredSessionObservabilityPaths,
|
||||
checkRetiredSnapshotExecutionPaths,
|
||||
DAEMON_MODULARITY_BASELINE,
|
||||
TYPE_CYCLE_BASELINE,
|
||||
} from './daemon-modularity.ts';
|
||||
import { SESSION_STATE_FIELD_OWNERS } from './session-state.ts';
|
||||
import { resolveImportEdges, targetDagZone, type ResolvedImportEdge } from './model.ts';
|
||||
import type { LayeringRatchets } from './ratchet-reference.ts';
|
||||
|
||||
function importEdge(file: string, target: string): ResolvedImportEdge {
|
||||
return {
|
||||
@@ -45,13 +44,8 @@ const ZONE_DIRECTORY: Readonly<Record<string, string>> = {
|
||||
'provider-webdriver': 'packages/provider-webdriver/src/',
|
||||
};
|
||||
|
||||
/**
|
||||
* A cycle membership that exactly fills the zone ceilings. R9 is equality-pinned, so a test
|
||||
* probing anything else starts from the pinned size the way `baselineEdges` starts from the
|
||||
* pinned edges; `overrides` re-counts one zone without disturbing the others.
|
||||
*/
|
||||
function baselineTypeCycleMembers(overrides: Readonly<Record<string, number>> = {}): string[] {
|
||||
const zones = { ...DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers, ...overrides };
|
||||
/** A cycle membership of `count` files per zone, so a zone count becomes file paths. */
|
||||
function typeCycleMembers(zones: Readonly<Record<string, number>>): string[] {
|
||||
return Object.entries(zones).flatMap(([zone, count]) =>
|
||||
Array.from(
|
||||
{ length: count },
|
||||
@@ -60,18 +54,55 @@ function baselineTypeCycleMembers(overrides: Readonly<Record<string, number>> =
|
||||
);
|
||||
}
|
||||
|
||||
test('daemon modularity baseline records the measured R7 ownership pressure', () => {
|
||||
assert.equal(
|
||||
Object.keys(SESSION_STATE_FIELD_OWNERS).length,
|
||||
DAEMON_MODULARITY_BASELINE.sessionState.writerOwnedFields,
|
||||
/**
|
||||
* The merge-base measurement every test ratchets against. R9 and R10's R7 counts have no recorded
|
||||
* numbers any more, so a test states its own reference tree instead of importing one.
|
||||
*/
|
||||
const REFERENCE: LayeringRatchets = {
|
||||
typeInversions: {},
|
||||
largestTypeCycle: typeCycleMembers({ 'provider-webdriver': 6 }),
|
||||
sessionState: { writerOwnedFields: 19, ownerFileClaims: 22 },
|
||||
};
|
||||
|
||||
/** The same measurement as the reference except where a test moves one number. */
|
||||
function measured(overrides: Partial<LayeringRatchets> = {}): LayeringRatchets {
|
||||
return { ...REFERENCE, ...overrides };
|
||||
}
|
||||
|
||||
test('R10 rejects R7 ownership pressure that grew past the merge-base', () => {
|
||||
const grownFields = checkDaemonModularityRatchets(
|
||||
baselineEdges(),
|
||||
measured({ sessionState: { writerOwnedFields: 20, ownerFileClaims: 23 } }),
|
||||
REFERENCE,
|
||||
);
|
||||
assert.equal(
|
||||
Object.values(SESSION_STATE_FIELD_OWNERS).reduce((sum, owners) => sum + owners.length, 0),
|
||||
DAEMON_MODULARITY_BASELINE.sessionState.ownerFileClaims,
|
||||
assert.deepEqual(
|
||||
grownFields.map(({ rule, message }) => ({ rule, message })),
|
||||
[
|
||||
{
|
||||
rule: 'R10 daemon-modularity',
|
||||
message:
|
||||
'R7 writerOwnedFields grew to 20 (baseline 19 at the merge-base). Route the new write ' +
|
||||
'through an existing owner instead.',
|
||||
},
|
||||
{
|
||||
rule: 'R10 daemon-modularity',
|
||||
message:
|
||||
'R7 ownerFileClaims grew to 23 (baseline 22 at the merge-base). Route the new write ' +
|
||||
'through an existing owner instead.',
|
||||
},
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
test('R10 banks an R7 shrink with no edit anywhere', () => {
|
||||
assert.deepEqual(
|
||||
checkDaemonModularityRatchets(
|
||||
baselineEdges(),
|
||||
measured({ sessionState: { writerOwnedFields: 18, ownerFileClaims: 20 } }),
|
||||
REFERENCE,
|
||||
),
|
||||
[],
|
||||
);
|
||||
assert.equal(TYPE_CYCLE_BASELINE, 6);
|
||||
assert.equal(DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers['provider-webdriver'], 6);
|
||||
assert.equal('daemon-server' in DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers, false);
|
||||
});
|
||||
|
||||
test('external daemon/types.ts importer membership changes require the baseline to change', () => {
|
||||
@@ -84,14 +115,16 @@ test('external daemon/types.ts importer membership changes require the baseline
|
||||
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
[...baselineEdges(), ...edges],
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.equal(violations.length, 1);
|
||||
assert.match(violations[0]!.message, /may only shrink from the recorded 2/);
|
||||
|
||||
const removed = checkDaemonModularityRatchets(
|
||||
baselineDaemonTypesEdges().slice(1),
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.equal(removed.length, 1);
|
||||
assert.match(removed[0]!.message, /delete it from externalDaemonTypesImporters/);
|
||||
@@ -110,7 +143,8 @@ test('logical modules reject forbidden imports', () => {
|
||||
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
[...baselineEdges(), ...edges],
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.equal(violations.length, 1);
|
||||
assert.match(violations[0]!.message, /replay-test must not import/);
|
||||
@@ -138,7 +172,8 @@ test('replay-test rejects request-global and engine-internal imports', () => {
|
||||
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
[...baselineEdges(), ...edges],
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.deepEqual(
|
||||
violations.map(({ message }) => message.replace(/;.*/, '')),
|
||||
@@ -162,7 +197,7 @@ test('replay-test may still import its own files inside the package', () => {
|
||||
);
|
||||
|
||||
assert.deepEqual(
|
||||
checkDaemonModularityRatchets([...baselineEdges(), ...edges], baselineTypeCycleMembers()),
|
||||
checkDaemonModularityRatchets([...baselineEdges(), ...edges], REFERENCE, REFERENCE),
|
||||
[],
|
||||
);
|
||||
});
|
||||
@@ -177,7 +212,8 @@ test('internal trees reject deep imports globally, including from daemon', () =>
|
||||
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
[...baselineEdges(), ...edges],
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.equal(violations.length, 1);
|
||||
assert.match(violations[0]!.message, /must not import maestro's internal tree/);
|
||||
@@ -218,7 +254,8 @@ test('daemon replay rejects handler, owner, session-store, and engine deep edges
|
||||
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
[...baselineEdges(), ...edges],
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.deepEqual(
|
||||
violations.map(({ file, line, message }) => ({
|
||||
@@ -283,7 +320,8 @@ test('session lifecycle rejects handler deep imports in both directions', () =>
|
||||
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
[...baselineEdges(), ...edges],
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.deepEqual(
|
||||
violations.map(({ file, line, message }) => ({
|
||||
@@ -348,7 +386,8 @@ test('interaction rejects handler crossings and deep imports around its facade',
|
||||
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
[...baselineEdges(), ...edges],
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.equal(violations.length, 5);
|
||||
assert.ok(
|
||||
@@ -393,7 +432,8 @@ test('session observability rejects handler deep imports in both directions', ()
|
||||
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
[...baselineEdges(), ...edges],
|
||||
baselineTypeCycleMembers(),
|
||||
REFERENCE,
|
||||
REFERENCE,
|
||||
);
|
||||
assert.deepEqual(
|
||||
violations.map(({ file, line, message }) => ({
|
||||
@@ -552,17 +592,19 @@ test('session observability rejects restored handler paths', () => {
|
||||
);
|
||||
});
|
||||
|
||||
test('R9 records zone ceilings and keeps engine files outside the largest component', () => {
|
||||
test('R9 holds each zone to the merge-base and keeps engine files outside the component', () => {
|
||||
// One commands file and one engine file traded for two provider-webdriver ones, so the
|
||||
// total stays at the baseline and only the per-zone claims are on trial.
|
||||
const zones = DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers;
|
||||
// total stays at the merge-base's size and only the per-zone claims are on trial.
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
baselineEdges(),
|
||||
baselineTypeCycleMembers({
|
||||
commands: 1,
|
||||
'ad-replay': 1,
|
||||
'provider-webdriver': zones['provider-webdriver']! - 2,
|
||||
measured({
|
||||
largestTypeCycle: typeCycleMembers({
|
||||
commands: 1,
|
||||
'ad-replay': 1,
|
||||
'provider-webdriver': 4,
|
||||
}),
|
||||
}),
|
||||
REFERENCE,
|
||||
);
|
||||
|
||||
assert.equal(violations.length, 3);
|
||||
@@ -573,42 +615,54 @@ test('R9 records zone ceilings and keeps engine files outside the largest compon
|
||||
|
||||
// #1837: the zone violation used to name the alphabetically-first zone member — a file that had
|
||||
// been in the cycle all along — so the +1 was found only by diffing member lists between commits.
|
||||
// The ceiling records a count, not a membership, so the message lists every zone member instead.
|
||||
test('R10 zone overflow lists the whole zone so the joining member is visible', () => {
|
||||
const zones = DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers;
|
||||
// Sorts after the provider-webdriver probes: the old first-member pick could not name it by luck.
|
||||
// The merge-base carries membership, so the message names exactly the files that joined.
|
||||
test('R10 zone overflow names the member that joined the cycle', () => {
|
||||
// Sorts after the provider-webdriver probes: a first-member pick could not name it by luck.
|
||||
const joined = 'src/daemon/snapshot-interactor-capture.ts';
|
||||
const members = [
|
||||
...baselineTypeCycleMembers({ 'provider-webdriver': zones['provider-webdriver']! - 1 }),
|
||||
joined,
|
||||
].sort();
|
||||
const daemonMembers = members.filter((member) => member.startsWith('src/daemon/'));
|
||||
assert.deepEqual(daemonMembers, [joined]);
|
||||
const members = [...typeCycleMembers({ 'provider-webdriver': 5 }), joined].sort();
|
||||
|
||||
const violations = checkDaemonModularityRatchets(baselineEdges(), members);
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
baselineEdges(),
|
||||
measured({ largestTypeCycle: members }),
|
||||
REFERENCE,
|
||||
);
|
||||
|
||||
assert.equal(violations.length, 1);
|
||||
const [violation] = violations;
|
||||
assert.equal(violation!.rule, 'R10 daemon-modularity');
|
||||
assert.equal(violation!.file, 'scripts/layering/daemon-modularity.ts');
|
||||
assert.match(violation!.message, /contains 1 daemon-server file\(s\) \(baseline 0\)/);
|
||||
for (const member of daemonMembers) {
|
||||
assert.ok(violation!.message.includes(member), `${member} missing from: ${violation!.message}`);
|
||||
}
|
||||
assert.match(violation!.message, /1 over the ceiling — the member\(s\) that joined are among/);
|
||||
assert.match(
|
||||
violation!.message,
|
||||
/contains 1 daemon-server file\(s\) \(baseline 0 at the merge-base\)/,
|
||||
);
|
||||
assert.match(
|
||||
violation!.message,
|
||||
new RegExp(`1 over the merge-base — the daemon-server file\\(s\\) that joined: ${joined}\\.`),
|
||||
);
|
||||
});
|
||||
|
||||
// Growth was always rejected; a baseline left ABOVE the measured size used to be a suggestion
|
||||
// in the success line, which is headroom the next change spends without a number moving.
|
||||
test('R9 rejects a baseline left above the measured cycle', () => {
|
||||
const zones = DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers;
|
||||
test('R9 rejects a cycle grown past the merge-base', () => {
|
||||
const violations = checkDaemonModularityRatchets(
|
||||
baselineEdges(),
|
||||
baselineTypeCycleMembers({ 'provider-webdriver': zones['provider-webdriver']! - 1 }),
|
||||
measured({ largestTypeCycle: typeCycleMembers({ 'provider-webdriver': 7 }) }),
|
||||
REFERENCE,
|
||||
);
|
||||
|
||||
assert.equal(violations.length, 1);
|
||||
assert.equal(violations.length, 2);
|
||||
assert.match(violations[0]!.rule, /^R9 /);
|
||||
assert.match(violations[0]!.message, /dropped to 5 files \(baseline 6\)/);
|
||||
assert.match(violations[0]!.message, /Lower LARGEST_TYPE_CYCLE_ZONE_CEILINGS by the same 1/);
|
||||
assert.match(violations[0]!.message, /grew to 7 files \(baseline 6 at the merge-base\)/);
|
||||
assert.match(violations[1]!.rule, /^R10 /);
|
||||
});
|
||||
|
||||
// The shrink direction used to need an edit in the same change, or the ceiling kept headroom the
|
||||
// next change could spend. Measuring the merge-base banks it on merge, with nothing to lower.
|
||||
test('R9 banks a shrink with no edit anywhere', () => {
|
||||
assert.deepEqual(
|
||||
checkDaemonModularityRatchets(
|
||||
baselineEdges(),
|
||||
measured({ largestTypeCycle: typeCycleMembers({ 'provider-webdriver': 5 }) }),
|
||||
REFERENCE,
|
||||
),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
@@ -9,35 +9,18 @@ import {
|
||||
type LogicalModulePolicy,
|
||||
} from './architecture-ownership.ts';
|
||||
import { targetDagZone, type LayeringViolation, type ResolvedImportEdge } from './model.ts';
|
||||
import { SESSION_STATE_FIELD_OWNERS } from './session-state.ts';
|
||||
|
||||
const LARGEST_TYPE_CYCLE_ZONE_CEILINGS: Readonly<Record<string, number>> = {
|
||||
// co-defined-contract pair (`capabilities.ts` ↔ `runtime.ts` and the four files they
|
||||
// pull in). The standard shrink is a third module holding the shared type.
|
||||
'provider-webdriver': 6,
|
||||
};
|
||||
import type { LayeringRatchets } from './ratchet-reference.ts';
|
||||
|
||||
// R7 ownership pressure and the largest type cycle (whole and per zone) are ratcheted against the
|
||||
// merge-base with origin/main (`ratchet-reference.ts`); the importer membership below stays a
|
||||
// recorded list, because it names files rather than counting them.
|
||||
export const DAEMON_MODULARITY_BASELINE = {
|
||||
sessionState: {
|
||||
// R60 moved `audioProbe` to store-owned. R64 does the same for the neutral `perfCapture`
|
||||
// and `lastPerfProfile` records after retiring the two platform-specific perf fields.
|
||||
writerOwnedFields: 19,
|
||||
ownerFileClaims: 22,
|
||||
},
|
||||
largestTypeCycle: {
|
||||
zoneMembers: LARGEST_TYPE_CYCLE_ZONE_CEILINGS,
|
||||
},
|
||||
externalDaemonTypesImporters: [
|
||||
'src/client/client-normalizers.ts',
|
||||
'src/remote/daemon-artifacts.ts',
|
||||
],
|
||||
} as const;
|
||||
|
||||
export const TYPE_CYCLE_BASELINE = Object.values(LARGEST_TYPE_CYCLE_ZONE_CEILINGS).reduce(
|
||||
(sum, count) => sum + count,
|
||||
0,
|
||||
);
|
||||
|
||||
const ENGINE_FILE_PREFIXES = [
|
||||
'packages/ad-replay/src/',
|
||||
'packages/maestro/src/',
|
||||
@@ -55,17 +38,18 @@ const ENGINE_FILE_PREFIXES = [
|
||||
* Cost: 937 LOC total for the file (323 rule + 614 test; shared with R9's checkTypeCycleBaseline
|
||||
* below, not attributed separately).
|
||||
* Kill criterion: none enforced today; retire only by maintainer decision that the daemon
|
||||
* modularity baselines (SessionState field-owner counts, logical-module import policies and
|
||||
* facades, the external daemon/types.ts importer list, per-zone cycle ceilings) no longer
|
||||
* modularity measurements (SessionState field-owner counts, logical-module import policies and
|
||||
* facades, the external daemon/types.ts importer list, per-zone cycle membership) no longer
|
||||
* matter. Every one is a count or an import edge the compiler accepts either way.
|
||||
*/
|
||||
export function checkDaemonModularityRatchets(
|
||||
edges: readonly ResolvedImportEdge[],
|
||||
largestTypeCycleMembers: readonly string[],
|
||||
measured: LayeringRatchets,
|
||||
reference: LayeringRatchets,
|
||||
): LayeringViolation[] {
|
||||
return [
|
||||
...checkSessionStateBaseline(),
|
||||
...checkTypeCycleBaseline(largestTypeCycleMembers),
|
||||
...checkSessionStateBaseline(measured.sessionState, reference.sessionState),
|
||||
...checkTypeCycleBaseline(measured.largestTypeCycle, reference.largestTypeCycle),
|
||||
...checkDaemonTypesImporters(edges),
|
||||
...checkLogicalModuleImports(edges),
|
||||
];
|
||||
@@ -132,94 +116,74 @@ export function checkRetiredInteractionPaths(sourceFiles: readonly string[]): La
|
||||
);
|
||||
}
|
||||
|
||||
function checkSessionStateBaseline(): LayeringViolation[] {
|
||||
const actual = {
|
||||
writerOwnedFields: Object.keys(SESSION_STATE_FIELD_OWNERS).length,
|
||||
ownerFileClaims: Object.values(SESSION_STATE_FIELD_OWNERS).reduce(
|
||||
(sum, owners) => sum + owners.length,
|
||||
0,
|
||||
),
|
||||
};
|
||||
function checkSessionStateBaseline(
|
||||
measured: LayeringRatchets['sessionState'],
|
||||
reference: LayeringRatchets['sessionState'],
|
||||
): LayeringViolation[] {
|
||||
const violations: LayeringViolation[] = [];
|
||||
for (const metric of ['writerOwnedFields', 'ownerFileClaims'] as const) {
|
||||
const baseline = DAEMON_MODULARITY_BASELINE.sessionState[metric];
|
||||
if (actual[metric] === baseline) continue;
|
||||
if (measured[metric] <= reference[metric]) continue;
|
||||
violations.push({
|
||||
rule: 'R10 daemon-modularity',
|
||||
file: 'scripts/layering/daemon-modularity.ts',
|
||||
line: 1,
|
||||
message:
|
||||
actual[metric] > baseline
|
||||
? `R7 ${metric} grew to ${actual[metric]} (baseline ${baseline}). Route the new write through an existing owner instead.`
|
||||
: `R7 ${metric} dropped to ${actual[metric]} — lower the daemon modularity baseline in the same capability move so it cannot regrow.`,
|
||||
`R7 ${metric} grew to ${measured[metric]} (baseline ${reference[metric]} at the ` +
|
||||
`merge-base). Route the new write through an existing owner instead.`,
|
||||
});
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
/**
|
||||
* Catches: the largest type-only import cycle growing past its pinned size, or the baseline
|
||||
* shrinking without the ceiling being lowered to match — R4 keeps the value graph acyclic, so
|
||||
* these cycles cost nothing at runtime, but an ungoverned type cycle can grow without bound
|
||||
* while every individual edge still looks locally reasonable.
|
||||
* Catches: the largest type-only import cycle growing past what the merge-base holds, whole or
|
||||
* in any one zone — R4 keeps the value graph acyclic, so these cycles cost nothing at runtime,
|
||||
* but an ungoverned type cycle can grow without bound while every individual edge still looks
|
||||
* locally reasonable.
|
||||
* Evidence: 6984a1e095 (#1852) fixed R10's zone listing when this ceiling trips, evidence the
|
||||
* check fires in practice; ef6ec2995b (#1825, #1781 A6) made the R9 shrink direction
|
||||
* mandatory rather than advisory.
|
||||
* check fires in practice; ef6ec2995b (#1825, #1781 A6) made a banked shrink mandatory rather
|
||||
* than advisory, which measuring the merge-base now does without an edit.
|
||||
* Cost: 937 LOC total for the file (323 rule + 614 test; shared with R10's ratchets above, not
|
||||
* attributed separately).
|
||||
* Kill criterion: none enforced today; retire only by maintainer decision that a bounded
|
||||
* type-only cycle size no longer matters. tsc never rejects a type-only cycle, and emptying
|
||||
* LARGEST_TYPE_CYCLE_ZONE_CEILINGS pins the size at zero rather than retiring the check.
|
||||
* type-only cycle size no longer matters. tsc never rejects a type-only cycle, and a merge-base
|
||||
* with no cycle pins the size at zero rather than retiring the check.
|
||||
*/
|
||||
function checkTypeCycleBaseline(members: readonly string[]): LayeringViolation[] {
|
||||
function checkTypeCycleBaseline(
|
||||
members: readonly string[],
|
||||
referenceMembers: readonly string[],
|
||||
): LayeringViolation[] {
|
||||
const violations: LayeringViolation[] = [];
|
||||
const baseline = DAEMON_MODULARITY_BASELINE.largestTypeCycle;
|
||||
if (members.length > TYPE_CYCLE_BASELINE) {
|
||||
if (members.length > referenceMembers.length) {
|
||||
violations.push({
|
||||
rule: 'R9 type-cycle-size',
|
||||
file: 'scripts/layering/daemon-modularity.ts',
|
||||
line: 1,
|
||||
message:
|
||||
`the largest type-level import cycle grew to ${members.length} files (baseline ` +
|
||||
`${TYPE_CYCLE_BASELINE}). A type-only import that closes a loop makes every file in the ` +
|
||||
`loop unreadable in isolation. Declare the shared type below both modules, or if the growth ` +
|
||||
`is genuinely warranted, raise the zone ceilings in the same commit and say why.`,
|
||||
});
|
||||
} else if (members.length < TYPE_CYCLE_BASELINE) {
|
||||
// A ceiling left above the measured size is headroom a later change spends without a
|
||||
// reviewer ever seeing a number move, so the shrink is recorded in the change that earns
|
||||
// it — the same equality pin R6 and the R10 R7 counts already carry.
|
||||
violations.push({
|
||||
rule: 'R9 type-cycle-size',
|
||||
file: 'scripts/layering/daemon-modularity.ts',
|
||||
line: 1,
|
||||
message:
|
||||
`the largest type-level import cycle dropped to ${members.length} files (baseline ` +
|
||||
`${TYPE_CYCLE_BASELINE}). Lower LARGEST_TYPE_CYCLE_ZONE_CEILINGS by the same ${TYPE_CYCLE_BASELINE - members.length} ` +
|
||||
`in this change so the cycle cannot regrow into slack nobody chose.`,
|
||||
`${referenceMembers.length} at the merge-base). A type-only import that closes a loop makes ` +
|
||||
`every file in the loop unreadable in isolation. Declare the shared type below both modules.`,
|
||||
});
|
||||
}
|
||||
|
||||
const membersByZone = groupBy(members, targetDagZone);
|
||||
for (const [zone, zoneMembers] of membersByZone) {
|
||||
const allowed = baseline.zoneMembers[zone] ?? 0;
|
||||
const referenceByZone = groupBy(referenceMembers, targetDagZone);
|
||||
for (const [zone, zoneMembers] of groupBy(members, targetDagZone)) {
|
||||
const referenceZoneMembers = new Set(referenceByZone.get(zone) ?? []);
|
||||
const allowed = referenceZoneMembers.size;
|
||||
if (zoneMembers.length <= allowed) continue;
|
||||
// The ceiling records a count, not a membership, so the gate cannot name the file that
|
||||
// joined; naming the alphabetically-first member instead sent #1837's diagnosis to a file
|
||||
// that had been in the cycle all along. List the whole zone so the joining edge is one
|
||||
// diff away from the author, who knows which of these files the change touched. The
|
||||
// overflow is net growth (a join and a departure cancel out), so it bounds nothing about
|
||||
// how many members are new — only that at least one of the listed files is.
|
||||
// A ceiling recorded a count, so the gate could only list the whole zone and #1837's
|
||||
// diagnosis landed on a file that had been in the cycle all along. The merge-base carries
|
||||
// membership, so the files that joined are named exactly.
|
||||
const joined = zoneMembers.filter((member) => !referenceZoneMembers.has(member));
|
||||
violations.push({
|
||||
rule: 'R10 daemon-modularity',
|
||||
file: 'scripts/layering/daemon-modularity.ts',
|
||||
line: 1,
|
||||
message:
|
||||
`the largest type cycle now contains ${zoneMembers.length} ${zone} file(s) (baseline ` +
|
||||
`${allowed}); extraction must not trade one zone's locality for another's. ` +
|
||||
`${zoneMembers.length - allowed} over the ceiling — the member(s) that joined are among ` +
|
||||
`these ${zone} files: ${zoneMembers.join(', ')}. Cut the edge that pulled them in ` +
|
||||
`rather than raising the ceiling.`,
|
||||
`${allowed} at the merge-base); extraction must not trade one zone's locality for ` +
|
||||
`another's. ${zoneMembers.length - allowed} over the merge-base — the ${zone} file(s) ` +
|
||||
`that joined: ${joined.join(', ')}. Cut the edge that pulled them in.`,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -340,11 +304,11 @@ function groupBy(
|
||||
return groups;
|
||||
}
|
||||
|
||||
export function daemonModularitySummary(): string {
|
||||
const session = DAEMON_MODULARITY_BASELINE.sessionState;
|
||||
export function daemonModularitySummary(reference: LayeringRatchets): string {
|
||||
const session = reference.sessionState;
|
||||
return (
|
||||
`R10 pins R7 at ${session.writerOwnedFields} writer-owned fields / ` +
|
||||
`${session.ownerFileClaims} owner claims, R9 at ${TYPE_CYCLE_BASELINE} files with zone ceilings, ` +
|
||||
`R10 holds R7 at the merge-base's ${session.writerOwnedFields} writer-owned fields / ` +
|
||||
`${session.ownerFileClaims} owner claims, R9 at its ${reference.largestTypeCycle.length} files per zone, ` +
|
||||
`${DAEMON_MODULARITY_BASELINE.externalDaemonTypesImporters.length} external daemon/types.ts importers, ` +
|
||||
'and zero forbidden logical-module imports'
|
||||
);
|
||||
|
||||
@@ -325,14 +325,33 @@ function resolveTargetFile(
|
||||
return candidates.find((candidate) => sourceFiles.has(candidate)) ?? null;
|
||||
}
|
||||
|
||||
export type ImportParser = (source: string) => ImportEdge[];
|
||||
|
||||
/**
|
||||
* `parseImports` memoized by source text. A ratchet parses two trees that share almost every
|
||||
* file, so the second tree costs a parse only where its text differs from the first.
|
||||
*/
|
||||
export function memoizedImportParser(): ImportParser {
|
||||
const edgesBySource = new Map<string, ImportEdge[]>();
|
||||
return (source) => {
|
||||
let edges = edgesBySource.get(source);
|
||||
if (!edges) {
|
||||
edges = parseImports(source);
|
||||
edgesBySource.set(source, edges);
|
||||
}
|
||||
return edges;
|
||||
};
|
||||
}
|
||||
|
||||
export function resolveImportEdges(
|
||||
sources: ReadonlyMap<string, string>,
|
||||
workspaceExportTargets?: ReadonlyMap<string, string>,
|
||||
parse: ImportParser = parseImports,
|
||||
): ResolvedImportEdge[] {
|
||||
const sourceFiles = new Set(sources.keys());
|
||||
const edges: ResolvedImportEdge[] = [];
|
||||
for (const [file, source] of sources) {
|
||||
for (const edge of parseImports(source)) {
|
||||
for (const edge of parse(source)) {
|
||||
const target = resolveTargetFile(file, edge.spec, sourceFiles, workspaceExportTargets);
|
||||
if (!target) continue;
|
||||
edges.push({
|
||||
@@ -452,12 +471,34 @@ export function backEdgePair(edge: ResolvedImportEdge): string | null {
|
||||
// a type-only import costs nothing at runtime and does not affect cold start — but a
|
||||
// type-only edge still says "this zone is declared in terms of that one", and that IS a
|
||||
// boundary claim. Ranking them found 61 inversions the gate had never seen, which is why
|
||||
// they are ratcheted rather than merely reported: see `TYPE_INVERSION_BASELINE`.
|
||||
// they are ratcheted rather than merely reported against the merge-base with origin/main: see
|
||||
// `typeInversionCounts` and scripts/layering/type-inversion-ratchet.ts.
|
||||
export function typeInversionPair(edge: ResolvedImportEdge): string | null {
|
||||
if (edge.dynamic || !edge.typeOnly) return null;
|
||||
return spineInversionPair(edge);
|
||||
}
|
||||
|
||||
/**
|
||||
* R6's measurement: distinct type-only spine inversions per zone pair, keyed `from -> to` and
|
||||
* sorted by pair. The ratchet compares this record across two trees, so it is a pure function of
|
||||
* the edge set rather than a count taken inside the rule.
|
||||
*/
|
||||
export function typeInversionCounts(
|
||||
edges: readonly ResolvedImportEdge[],
|
||||
): Readonly<Record<string, number>> {
|
||||
const seen = new Set<string>();
|
||||
const counts = new Map<string, number>();
|
||||
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);
|
||||
counts.set(pair, (counts.get(pair) ?? 0) + 1);
|
||||
}
|
||||
return Object.fromEntries([...counts].sort(([left], [right]) => left.localeCompare(right)));
|
||||
}
|
||||
|
||||
export function collectBackEdges(edges: readonly ResolvedImportEdge[]): BackEdgeMap {
|
||||
const identitiesByPair = new Map<string, Set<string>>();
|
||||
for (const edge of edges) {
|
||||
|
||||
@@ -80,11 +80,24 @@ export function specifierSites(file: string, source: string): SpecifierSite[] {
|
||||
* filtering the output.
|
||||
*/
|
||||
export function readWorkspacePackages(repoRoot: string): WorkspacePackage[] {
|
||||
return workspacePackagesFromManifests(
|
||||
new Map(
|
||||
listTrackedPackageManifests(repoRoot).map((manifestFile) => [
|
||||
manifestFile,
|
||||
fs.readFileSync(path.join(repoRoot, manifestFile), 'utf8'),
|
||||
]),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/** The same package model over manifest sources already in hand, e.g. read from a git ref. */
|
||||
export function workspacePackagesFromManifests(
|
||||
manifests: ReadonlyMap<string, string>,
|
||||
): WorkspacePackage[] {
|
||||
const packages: WorkspacePackage[] = [];
|
||||
for (const manifestFile of listTrackedPackageManifests(repoRoot).sort()) {
|
||||
for (const manifestFile of [...manifests.keys()].sort()) {
|
||||
const entry = path.posix.basename(path.posix.dirname(manifestFile));
|
||||
const manifestPath = path.join(repoRoot, manifestFile);
|
||||
const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')) as {
|
||||
const manifest = JSON.parse(manifests.get(manifestFile)!) as {
|
||||
name?: string;
|
||||
private?: boolean;
|
||||
exports?: Record<string, { default?: string } | string>;
|
||||
@@ -334,8 +347,19 @@ function walkTsFiles(repoRoot: string, relativeDir: string): string[] {
|
||||
|
||||
/** Flat `specifier -> repo-relative source` map across all workspace packages. */
|
||||
export function workspaceSpecifierTargets(repoRoot: string): Map<string, string> {
|
||||
return specifierTargetsOf(readWorkspacePackages(repoRoot));
|
||||
}
|
||||
|
||||
/** The same flat map for a manifest set read elsewhere, e.g. at a git ref. */
|
||||
export function workspaceSpecifierTargetsFromManifests(
|
||||
manifests: ReadonlyMap<string, string>,
|
||||
): Map<string, string> {
|
||||
return specifierTargetsOf(workspacePackagesFromManifests(manifests));
|
||||
}
|
||||
|
||||
function specifierTargetsOf(packages: readonly WorkspacePackage[]): Map<string, string> {
|
||||
const targets = new Map<string, string>();
|
||||
for (const pkg of readWorkspacePackages(repoRoot)) {
|
||||
for (const pkg of packages) {
|
||||
for (const [specifier, target] of pkg.exportTargets) targets.set(specifier, target);
|
||||
}
|
||||
return targets;
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import assert from 'node:assert/strict';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { test } from 'node:test';
|
||||
import { memoizedImportParser, resolveImportEdges } from './model.ts';
|
||||
import { measureRatchets, mergeBaseRatchets } from './ratchet-reference.ts';
|
||||
import { sessionStateWritePressure } from './session-state.ts';
|
||||
import { listTrackedProductionSources } from './tracked-sources.ts';
|
||||
import { readCommittedSources } from '../__tests__/committed-source-tree.ts';
|
||||
|
||||
const SESSION_TYPES = [
|
||||
'export type SessionState = {',
|
||||
' snapshot?: string;',
|
||||
' trace?: string;',
|
||||
' name: string;',
|
||||
'};',
|
||||
].join('\n');
|
||||
|
||||
function tree(extra: Record<string, string> = {}) {
|
||||
return new Map<string, string>([
|
||||
['src/daemon/types.ts', SESSION_TYPES],
|
||||
['src/daemon/session-snapshot.ts', 'session.snapshot = "a"; nextSession.snapshot = "b";'],
|
||||
['src/daemon/ref-frame.ts', 'session.snapshot = undefined;'],
|
||||
['src/daemon/handlers/trace-runtime.ts', 'session.trace ??= "t";'],
|
||||
['src/client/client.ts', "import type { Loop } from '../commands/loop.ts';"],
|
||||
['src/commands/loop.ts', "import type { Client } from '../client/client.ts';"],
|
||||
...Object.entries(extra),
|
||||
]);
|
||||
}
|
||||
|
||||
const repoRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], {
|
||||
encoding: 'utf8',
|
||||
}).trim();
|
||||
|
||||
test('sessionStateWritePressure counts written fields and (field, writer) claims, not writes', () => {
|
||||
assert.deepEqual(sessionStateWritePressure(tree()), {
|
||||
writerOwnedFields: 2,
|
||||
ownerFileClaims: 3,
|
||||
});
|
||||
assert.deepEqual(
|
||||
sessionStateWritePressure(tree({ 'src/daemon/other.ts': 'session[key] = 1;' })),
|
||||
{
|
||||
writerOwnedFields: 2,
|
||||
ownerFileClaims: 3,
|
||||
},
|
||||
);
|
||||
assert.deepEqual(sessionStateWritePressure(new Map()), {
|
||||
writerOwnedFields: 0,
|
||||
ownerFileClaims: 0,
|
||||
});
|
||||
});
|
||||
|
||||
test('measureRatchets reports all three ratchets from one tree', () => {
|
||||
const sources = tree();
|
||||
assert.deepEqual(measureRatchets(sources, resolveImportEdges(sources)), {
|
||||
typeInversions: { 'commands -> client': 1 },
|
||||
largestTypeCycle: ['src/client/client.ts', 'src/commands/loop.ts'],
|
||||
sessionState: { writerOwnedFields: 2, ownerFileClaims: 3 },
|
||||
});
|
||||
});
|
||||
|
||||
test('memoizedImportParser parses each distinct source text once', () => {
|
||||
let parses = 0;
|
||||
const a = "import type { Client } from '../client/client.ts';";
|
||||
const sources = new Map([
|
||||
['src/commands/a.ts', a],
|
||||
['src/commands/b.ts', `${a.slice(0, 5)}${a.slice(5)}`],
|
||||
['src/client/client.ts', 'export type Client = {};'],
|
||||
]);
|
||||
const parse = memoizedImportParser();
|
||||
assert.deepEqual(resolveImportEdges(sources, undefined, parse), resolveImportEdges(sources));
|
||||
resolveImportEdges(sources, undefined, (source) => {
|
||||
parses++;
|
||||
return parse(source);
|
||||
});
|
||||
assert.equal(parses, 3);
|
||||
assert.equal(parse(a), parse(a));
|
||||
});
|
||||
|
||||
// The reference is a committed tree read through the shared committed-tree reader, so its file
|
||||
// set must be the one the working-tree scan uses. A divergence here means the git-side
|
||||
// enumeration or blob read no longer matches the layering scan input.
|
||||
test('the committed enumeration at HEAD is the working-tree scan input', () => {
|
||||
const { sources, manifests } = readCommittedSources(repoRoot, 'HEAD');
|
||||
assert.deepEqual([...sources.keys()], listTrackedProductionSources(repoRoot));
|
||||
assert.ok(manifests.has('packages/kernel/package.json'));
|
||||
});
|
||||
|
||||
test('the merge-base reference names its ref and measures the real tree', () => {
|
||||
const reference = mergeBaseRatchets(repoRoot);
|
||||
assert.match(reference.ref, /^[0-9a-f]{40}$/);
|
||||
assert.ok(reference.largestTypeCycle.length >= 1);
|
||||
assert.ok(reference.sessionState.writerOwnedFields > 0);
|
||||
assert.ok(reference.sessionState.ownerFileClaims >= reference.sessionState.writerOwnedFields);
|
||||
assert.ok(Object.keys(reference.typeInversions).length > 0);
|
||||
});
|
||||
@@ -0,0 +1,60 @@
|
||||
// The three ratcheted measurements (R6 type-spine inversions, R9 largest type cycle, R10's R7
|
||||
// ownership pressure) and where their reference numbers come from: the merge-base with
|
||||
// origin/main, measured by the same functions that measure the working tree. Growth fails, a
|
||||
// shrink needs no edit, and no recorded number can sit above what main actually holds.
|
||||
//
|
||||
// The base tree is read through the shared committed-tree reader (one `git ls-tree`, one
|
||||
// `git cat-file --batch`), never a second checkout and never a read per file.
|
||||
|
||||
import { mergeBaseWithMain, readCommittedSources } from '../__tests__/committed-source-tree.ts';
|
||||
import {
|
||||
largestTypeCycleMembers,
|
||||
memoizedImportParser,
|
||||
resolveImportEdges,
|
||||
typeInversionCounts,
|
||||
type ImportParser,
|
||||
type ResolvedImportEdge,
|
||||
} from './model.ts';
|
||||
import { workspaceSpecifierTargetsFromManifests } from './package-boundaries.ts';
|
||||
import { sessionStateWritePressure, type SessionStateWritePressure } from './session-state.ts';
|
||||
|
||||
export type LayeringRatchets = Readonly<{
|
||||
/** R6: distinct type-only spine inversions per `from -> to` zone pair. */
|
||||
typeInversions: Readonly<Record<string, number>>;
|
||||
/** R9 and R10's zone membership: sorted members of the largest type-level cycle. */
|
||||
largestTypeCycle: readonly string[];
|
||||
/** R10: R7 ownership pressure. */
|
||||
sessionState: SessionStateWritePressure;
|
||||
}>;
|
||||
|
||||
export type MergeBaseRatchets = LayeringRatchets & Readonly<{ ref: string }>;
|
||||
|
||||
export function measureRatchets(
|
||||
sources: ReadonlyMap<string, string>,
|
||||
edges: readonly ResolvedImportEdge[],
|
||||
): LayeringRatchets {
|
||||
return {
|
||||
typeInversions: typeInversionCounts(edges),
|
||||
largestTypeCycle: largestTypeCycleMembers(edges),
|
||||
sessionState: sessionStateWritePressure(sources),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The reference measurement: the merge-base tree, enumerated and classified by the same reader
|
||||
* the eager-closure ratchet uses, then measured exactly like the working tree. A file whose text
|
||||
* is byte-identical at both ends costs no second parse, because `parse` memoizes by source text.
|
||||
*/
|
||||
export function mergeBaseRatchets(
|
||||
repoRoot: string,
|
||||
parse: ImportParser = memoizedImportParser(),
|
||||
): MergeBaseRatchets {
|
||||
const ref = mergeBaseWithMain(repoRoot);
|
||||
const { sources, manifests } = readCommittedSources(repoRoot, ref);
|
||||
const edges = resolveImportEdges(
|
||||
sources,
|
||||
workspaceSpecifierTargetsFromManifests(manifests),
|
||||
parse,
|
||||
);
|
||||
return { ref, ...measureRatchets(sources, edges) };
|
||||
}
|
||||
@@ -11,9 +11,16 @@ import {
|
||||
listTrackedTypeScriptFiles,
|
||||
} from './tracked-sources.ts';
|
||||
import { RETIRED_PATH_RULES, retiredPathRuleViolations } from './retired-paths-policy.ts';
|
||||
import type { LayeringRatchets } from './ratchet-reference.ts';
|
||||
|
||||
const repoRoot = path.resolve(import.meta.dirname, '../..');
|
||||
|
||||
const EMPTY_RATCHETS: LayeringRatchets = {
|
||||
typeInversions: {},
|
||||
largestTypeCycle: [],
|
||||
sessionState: { writerOwnedFields: 0, ownerFileClaims: 0 },
|
||||
};
|
||||
|
||||
function contextWithFiles(
|
||||
files: Partial<Pick<LayeringContext, 'sourceFiles' | 'trackedSrcUtilsFiles'>>,
|
||||
): LayeringContext {
|
||||
@@ -23,7 +30,8 @@ function contextWithFiles(
|
||||
allTypeScriptSources: new Map(),
|
||||
trackedSrcUtilsFiles: [],
|
||||
edges: [],
|
||||
typeCycleMembers: [],
|
||||
ratchets: EMPTY_RATCHETS,
|
||||
reference: EMPTY_RATCHETS,
|
||||
...files,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -274,3 +274,30 @@ export function findSessionStateWrites(
|
||||
(left, right) => left.file.localeCompare(right.file) || left.line - right.line,
|
||||
);
|
||||
}
|
||||
|
||||
export type SessionStateWritePressure = Readonly<{
|
||||
/** Declared fields that some daemon module writes directly. */
|
||||
writerOwnedFields: number;
|
||||
/** Distinct (field, writing module) pairs — what `SESSION_STATE_FIELD_OWNERS` claims. */
|
||||
ownerFileClaims: number;
|
||||
}>;
|
||||
|
||||
/**
|
||||
* R10's measurement of R7 pressure: how many declared fields have a direct writer, and how many
|
||||
* module claims that takes. Read from the tree rather than from the ownership table, so the same
|
||||
* function measures a merge-base tree whose table is not in scope. On a tree R7 accepts, both
|
||||
* numbers equal the table's own size.
|
||||
*/
|
||||
export function sessionStateWritePressure(
|
||||
sources: ReadonlyMap<string, string>,
|
||||
): SessionStateWritePressure {
|
||||
const types = sources.get('src/daemon/types.ts');
|
||||
if (!types) return { writerOwnedFields: 0, ownerFileClaims: 0 };
|
||||
const writes = findSessionStateWrites(sources, sessionStateFields(types)).filter(
|
||||
(write) => write.field !== '[computed]',
|
||||
);
|
||||
return {
|
||||
writerOwnedFields: new Set(writes.map((write) => write.field)).size,
|
||||
ownerFileClaims: new Set(writes.map((write) => `${write.field}\0${write.file}`)).size,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import assert from 'node:assert/strict';
|
||||
import { test } from 'node:test';
|
||||
import { resolveImportEdges, typeInversionCounts } from './model.ts';
|
||||
import { checkTypeInversions } from './type-inversion-ratchet.ts';
|
||||
|
||||
/** `commandsFiles` commands files type-importing the client, plus one value import R6 ignores. */
|
||||
function inversionEdges(commandsFiles: number) {
|
||||
const sources = new Map<string, string>([
|
||||
['src/client/client.ts', 'export type AgentDeviceClient = { run(): void };'],
|
||||
['src/commands/value-user.ts', "import { run } from '../client/client.ts';"],
|
||||
]);
|
||||
for (let index = 0; index < commandsFiles; index++) {
|
||||
sources.set(
|
||||
`src/commands/typed-${index}.ts`,
|
||||
"import type { AgentDeviceClient } from '../client/client.ts';",
|
||||
);
|
||||
}
|
||||
return resolveImportEdges(sources);
|
||||
}
|
||||
|
||||
test('typeInversionCounts keys distinct type-only inversions by zone pair, sorted', () => {
|
||||
assert.deepEqual(typeInversionCounts(inversionEdges(2)), { 'commands -> client': 2 });
|
||||
assert.deepEqual(typeInversionCounts([]), {});
|
||||
});
|
||||
|
||||
test('R6 is quiet at the merge-base count, and banks a shrink with no edit', () => {
|
||||
assert.deepEqual(checkTypeInversions(inversionEdges(2), { 'commands -> client': 2 }), []);
|
||||
assert.deepEqual(checkTypeInversions(inversionEdges(1), { 'commands -> client': 2 }), []);
|
||||
assert.deepEqual(checkTypeInversions(inversionEdges(0), { 'commands -> client': 2 }), []);
|
||||
});
|
||||
|
||||
test('R6 rejects a pair that grew past the merge-base and names the first edge', () => {
|
||||
const violations = checkTypeInversions(inversionEdges(3), { 'commands -> client': 2 });
|
||||
assert.equal(violations.length, 1);
|
||||
assert.equal(violations[0]!.rule, 'R6 type-spine-inversion');
|
||||
assert.equal(violations[0]!.file, 'src/commands/typed-0.ts');
|
||||
assert.match(
|
||||
violations[0]!.message,
|
||||
/type-only commands -> client inversions grew to 3 \(baseline 2 at the merge-base\)/,
|
||||
);
|
||||
});
|
||||
|
||||
test('R6 rejects a pair the merge-base does not have at all', () => {
|
||||
const violations = checkTypeInversions(inversionEdges(1), {});
|
||||
assert.equal(violations.length, 1);
|
||||
assert.match(
|
||||
violations[0]!.message,
|
||||
/new type-only commands -> client inversion \(1 edge\(s\), e\.g\. src\/commands\/typed-0\.ts -> src\/client\/client\.ts\); the merge-base has none/,
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,69 @@
|
||||
// R6: type-only spine inversions, per zone pair. R5 cannot see these (a type-only import is free
|
||||
// at runtime), but "zone A is declared in terms of zone B" is still a boundary claim, and ranking
|
||||
// type edges surfaced 61 of them. The survivors are argued in docs/dependency-graph-findings.md
|
||||
// §0; the reference is the same count taken at the merge-base with origin/main, so a pair can
|
||||
// only shrink and no change can bank headroom by recording a number above the tree.
|
||||
//
|
||||
// Catches: a type-only import against the ranked spine's declared order — a design-level
|
||||
// dependency (zone A is stated in terms of zone B) that R5 is blind to because it costs
|
||||
// nothing at runtime, so nothing else flags "the type shape leaks the wrong direction."
|
||||
// Evidence: the R5-adjacent commits in check.ts's history introduced this ratchet; the 61-to-5
|
||||
// reduction and the surviving deliberate inversions are recorded in
|
||||
// docs/dependency-graph-findings.md.
|
||||
// Cost: 118 LOC (61 rule + 57 test), plus the shared merge-base measurement in
|
||||
// ratchet-reference.ts.
|
||||
// Kill criterion: none enforced today; retire only by maintainer decision that type-only spine
|
||||
// inversions no longer matter. Reaching zero remaining inversions does not retire it: at zero
|
||||
// the ratchet is what keeps the count from regrowing, and tsc never rejects a type-only edge.
|
||||
|
||||
import { typeInversionPair, type LayeringViolation, type ResolvedImportEdge } from './model.ts';
|
||||
|
||||
const RULE = 'R6 type-spine-inversion';
|
||||
|
||||
export function checkTypeInversions(
|
||||
edges: readonly ResolvedImportEdge[],
|
||||
reference: Readonly<Record<string, number>>,
|
||||
): LayeringViolation[] {
|
||||
const seen = new Set<string>();
|
||||
const countsByPair = new Map<string, number>();
|
||||
const firstEdgeByPair = new Map<string, ResolvedImportEdge>();
|
||||
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);
|
||||
countsByPair.set(pair, (countsByPair.get(pair) ?? 0) + 1);
|
||||
if (!firstEdgeByPair.has(pair)) firstEdgeByPair.set(pair, edge);
|
||||
}
|
||||
|
||||
const violations: LayeringViolation[] = [];
|
||||
for (const [pair, count] of [...countsByPair].sort(([left], [right]) =>
|
||||
left.localeCompare(right),
|
||||
)) {
|
||||
const allowed = reference[pair];
|
||||
const edge = firstEdgeByPair.get(pair)!;
|
||||
if (allowed === undefined) {
|
||||
violations.push({
|
||||
rule: RULE,
|
||||
file: edge.file,
|
||||
line: edge.line,
|
||||
message:
|
||||
`new type-only ${pair} inversion (${count} edge(s), e.g. ${edge.file} -> ${edge.target}); ` +
|
||||
`the merge-base has none. Declare the shared type below both zones.`,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (count > allowed) {
|
||||
violations.push({
|
||||
rule: RULE,
|
||||
file: edge.file,
|
||||
line: edge.line,
|
||||
message:
|
||||
`type-only ${pair} inversions grew to ${count} (baseline ${allowed} at the merge-base). ` +
|
||||
`Move the shared type below both zones; the count may only shrink.`,
|
||||
});
|
||||
}
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
Reference in New Issue
Block a user