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
24 changes: 24 additions & 0 deletions scripts/layering/daemon-modularity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,30 @@ test('R9 records zone ceilings and keeps engine files outside the largest compon
assert.ok(violations.some(({ message }) => /engine file entered/.test(message)));
});

// #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 daemon-server probes: the old first-member pick could not name it by luck.
const joined = 'src/daemon/snapshot-interactor-capture.ts';
const members = [...baselineTypeCycleMembers({ commands: zones.commands - 1 }), joined].sort();
const daemonMembers = members.filter((member) => member.startsWith('src/daemon/'));
assert.notEqual(daemonMembers[0], joined);

const violations = checkDaemonModularityRatchets(baselineEdges(), members);

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 17 daemon-server file\(s\) \(baseline 16\)/);
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/);
});

// 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', () => {
Expand Down
34 changes: 25 additions & 9 deletions scripts/layering/daemon-modularity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,26 @@ function checkTypeCycleBaseline(members: readonly string[]): LayeringViolation[]
});
}

const zoneCounts = countBy(members, targetDagZone);
for (const [zone, count] of zoneCounts) {
const membersByZone = groupBy(members, targetDagZone);
for (const [zone, zoneMembers] of membersByZone) {
const allowed = baseline.zoneMembers[zone] ?? 0;
if (count <= allowed) continue;
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.
violations.push({
rule: 'R10 daemon-modularity',
file: members.find((member) => targetDagZone(member) === zone) ?? 'scripts/layering/check.ts',
file: 'scripts/layering/daemon-modularity.ts',
line: 1,
message: `the largest type cycle now contains ${count} ${zone} file(s) (baseline ${allowed}); extraction must not trade one zone's locality for another's.`,
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.`,
});
}

Expand Down Expand Up @@ -285,13 +296,18 @@ function isInsideInternalTree(file: string, roots: readonly string[]): boolean {
return roots.some((root) => file.startsWith(path.posix.join(root, 'internal/')));
}

function countBy(values: readonly string[], keyOf: (value: string) => string): Map<string, number> {
const counts = new Map<string, number>();
function groupBy(
values: readonly string[],
keyOf: (value: string) => string,
): Map<string, string[]> {
const groups = new Map<string, string[]>();
for (const value of values) {
const key = keyOf(value);
counts.set(key, (counts.get(key) ?? 0) + 1);
const group = groups.get(key) ?? [];
group.push(value);
groups.set(key, group);
}
return counts;
return groups;
}

export function daemonModularitySummary(): string {
Expand Down
Loading