Skip to content

Commit 0ad7567

Browse files
committed
fix(layering): list the whole zone when R10's type-cycle ceiling is exceeded
The per-zone R10 violation named members.find(<zone match>) — the alphabetically-first zone member, a file that had been in the cycle all along — so the +1 in #1825 x #1779 was found only by diffing largestTypeCycleMembers between commits. The ceiling records a count, not a membership, so the gate cannot name the joining file; it now lists every member of the over-budget zone and annotates the ceiling table instead. Closes #1837
1 parent fda81c5 commit 0ad7567

2 files changed

Lines changed: 47 additions & 9 deletions

File tree

scripts/layering/daemon-modularity.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,30 @@ test('R9 records zone ceilings and keeps engine files outside the largest compon
219219
assert.ok(violations.some(({ message }) => /engine file entered/.test(message)));
220220
});
221221

222+
// #1837: the zone violation used to name the alphabetically-first zone member — a file that had
223+
// been in the cycle all along — so the +1 was found only by diffing member lists between commits.
224+
// The ceiling records a count, not a membership, so the message lists every zone member instead.
225+
test('R10 zone overflow lists the whole zone so the joining member is visible', () => {
226+
const zones = DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers;
227+
// Sorts after the daemon-server probes: the old first-member pick could not name it by luck.
228+
const joined = 'src/daemon/snapshot-interactor-capture.ts';
229+
const members = [...baselineTypeCycleMembers({ commands: zones.commands - 1 }), joined].sort();
230+
const daemonMembers = members.filter((member) => member.startsWith('src/daemon/'));
231+
assert.notEqual(daemonMembers[0], joined);
232+
233+
const violations = checkDaemonModularityRatchets(baselineEdges(), members);
234+
235+
assert.equal(violations.length, 1);
236+
const [violation] = violations;
237+
assert.equal(violation!.rule, 'R10 daemon-modularity');
238+
assert.equal(violation!.file, 'scripts/layering/daemon-modularity.ts');
239+
assert.match(violation!.message, /contains 17 daemon-server file\(s\) \(baseline 16\)/);
240+
for (const member of daemonMembers) {
241+
assert.ok(violation!.message.includes(member), `${member} missing from: ${violation!.message}`);
242+
}
243+
assert.match(violation!.message, /1 of these joined with this change/);
244+
});
245+
222246
// Growth was always rejected; a baseline left ABOVE the measured size used to be a suggestion
223247
// in the success line, which is headroom the next change spends without a number moving.
224248
test('R9 rejects a baseline left above the measured cycle', () => {

scripts/layering/daemon-modularity.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,15 +165,24 @@ function checkTypeCycleBaseline(members: readonly string[]): LayeringViolation[]
165165
});
166166
}
167167

168-
const zoneCounts = countBy(members, targetDagZone);
169-
for (const [zone, count] of zoneCounts) {
168+
const membersByZone = groupBy(members, targetDagZone);
169+
for (const [zone, zoneMembers] of membersByZone) {
170170
const allowed = baseline.zoneMembers[zone] ?? 0;
171-
if (count <= allowed) continue;
171+
if (zoneMembers.length <= allowed) continue;
172+
// The ceiling records a count, not a membership, so the gate cannot name the file that
173+
// joined; naming the alphabetically-first member instead sent #1837's diagnosis to a file
174+
// that had been in the cycle all along. List the whole zone so the joining edge is one
175+
// diff away from the author, who knows which of these files the change touched.
172176
violations.push({
173177
rule: 'R10 daemon-modularity',
174-
file: members.find((member) => targetDagZone(member) === zone) ?? 'scripts/layering/check.ts',
178+
file: 'scripts/layering/daemon-modularity.ts',
175179
line: 1,
176-
message: `the largest type cycle now contains ${count} ${zone} file(s) (baseline ${allowed}); extraction must not trade one zone's locality for another's.`,
180+
message:
181+
`the largest type cycle now contains ${zoneMembers.length} ${zone} file(s) (baseline ` +
182+
`${allowed}); extraction must not trade one zone's locality for another's. ` +
183+
`${zone} members: ${zoneMembers.join(', ')}. ${zoneMembers.length - allowed} of these ` +
184+
`joined with this change (a new file, or a new import that closed the loop); cut that ` +
185+
`edge rather than raising the ceiling.`,
177186
});
178187
}
179188

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

288-
function countBy(values: readonly string[], keyOf: (value: string) => string): Map<string, number> {
289-
const counts = new Map<string, number>();
297+
function groupBy(
298+
values: readonly string[],
299+
keyOf: (value: string) => string,
300+
): Map<string, string[]> {
301+
const groups = new Map<string, string[]>();
290302
for (const value of values) {
291303
const key = keyOf(value);
292-
counts.set(key, (counts.get(key) ?? 0) + 1);
304+
const group = groups.get(key) ?? [];
305+
group.push(value);
306+
groups.set(key, group);
293307
}
294-
return counts;
308+
return groups;
295309
}
296310

297311
export function daemonModularitySummary(): string {

0 commit comments

Comments
 (0)