diff --git a/scripts/layering/daemon-modularity.test.ts b/scripts/layering/daemon-modularity.test.ts index 91b9d24df..ae8afd1e6 100644 --- a/scripts/layering/daemon-modularity.test.ts +++ b/scripts/layering/daemon-modularity.test.ts @@ -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', () => { diff --git a/scripts/layering/daemon-modularity.ts b/scripts/layering/daemon-modularity.ts index 09e47613e..7d908f497 100644 --- a/scripts/layering/daemon-modularity.ts +++ b/scripts/layering/daemon-modularity.ts @@ -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.`, }); } @@ -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 { - const counts = new Map(); +function groupBy( + values: readonly string[], + keyOf: (value: string) => string, +): Map { + const groups = new Map(); 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 {