Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 }
Expand Down
24 changes: 13 additions & 11 deletions docs/dependency-graph-findings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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 —
Expand Down
62 changes: 54 additions & 8 deletions scripts/__tests__/committed-source-tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)),
Expand Down
7 changes: 4 additions & 3 deletions scripts/depgraph/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Loading
Loading