From 4701941c0b1db88d78b0128c89ade419c59c2dc8 Mon Sep 17 00:00:00 2001 From: Henry Lach Date: Mon, 25 May 2026 14:25:39 -0400 Subject: [PATCH] fix(#562): align merge_success counter denominator with segment-level numerator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge_success event emission was pairing a segment-round numerator (waveIndex = 0..N-1 of the segment-expanded merge rounds) with a task- level denominator (taskLevelWaveCount, the pre-expansion wave count). In a clean polyrepo run with 3 task-level waves expanded into 6 segment-level rounds, the operator-visible alert overflowed: Wave 1 merged successfully (1/3). Tests pass. Wave 2 merged successfully (2/3). Tests pass. Wave 3 merged successfully (3/3). Tests pass. Wave 4 merged successfully (4/3). Tests pass. <- denominator stale Wave 5 merged successfully (5/3). Tests pass. Wave 6 merged successfully (6/3). Tests pass. The supervisor formatter renders 'Wave N merged successfully (X/Y)' where N and X both derive from event.waveIndex + 1, but Y comes from event.totalWaves. Both N/X were already segment-level, so the denominator needs to be segment-level too — that's batchState.totalWaves (set to rawWaves.length, the segment-expanded round count) and NOT batchState.taskLevelWaveCount (the pre-expansion count). One-field source-of-truth swap at the engine.ts emission site, fully described inline as a comment so the segment/task-wave distinction is preserved for future readers. Functional behavior was unaffected — every wave merged successfully — this is purely a cosmetic display fix. Reporter's preferred fix (per issue body) and the same approach taken: keep both numerator and denominator in segment-round units. Adds a regression test (5.8a) that emits 6 successive merge_success events with waveIndex 0..5 against a denominator of 6 and asserts: 1. Each rendered counter reads (waveIdx+1)/6. 2. None of the rendered counters contain the buggy '/3' denominator pattern, which was the visual signature in the original report. Validation: npm run typecheck pass npm run lint no new warnings (286/671 identical to main) npm run format:check pass supervisor.test.ts 114/114 pass (new 5.8a included) Closes #562 --- extensions/taskplane/engine.ts | 13 +++++++++++-- extensions/tests/supervisor.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/extensions/taskplane/engine.ts b/extensions/taskplane/engine.ts index 43db3812..c54252cb 100644 --- a/extensions/taskplane/engine.ts +++ b/extensions/taskplane/engine.ts @@ -4225,14 +4225,23 @@ export async function executeOrchBatch( "info", ); - // TP-040: Emit merge_success event + // TP-040: Emit merge_success event. + // + // `waveIndex` is the segment-round index (0-based), and the supervisor + // formatter renders the (N/M) counter using `waveIndex + 1` for N. + // For unit consistency we therefore pair it with the segment-level + // `batchState.totalWaves` (segment-expanded round count), not + // `taskLevelWaveCount` (pre-expansion). Using the task-level count as + // the denominator while the numerator counts segment rounds produced + // `(4/3)`, `(5/3)`, `(6/3)` style overflow once segments expanded — + // see issue #562. emitEvent( stateRoot, { ...buildEngineEventBase("merge_success", batchState.batchId, waveIdx, batchState.phase), laneCount: mergedCount, durationMs: mergeResult.totalDurationMs, - totalWaves: taskLevelWaveCount, + totalWaves: batchState.totalWaves, }, onEngineEvent, ); diff --git a/extensions/tests/supervisor.test.ts b/extensions/tests/supervisor.test.ts index d00ce6c3..701328d6 100644 --- a/extensions/tests/supervisor.test.ts +++ b/extensions/tests/supervisor.test.ts @@ -924,6 +924,33 @@ describe("5.x — formatEventNotification", () => { expect(text).toContain("42"); }); + it("5.8a: merge_success counter denominator stays >= numerator across segment-expanded waves (regression for #562)", () => { + // Polyrepo scenario from #562: 3 task-level waves expand into 6 + // segment-level merge rounds. The engine emits one merge_success + // per segment round, with waveIndex = segment-round index (0..5) + // and totalWaves = segment-expanded round count (6). The formatter + // must therefore render (1/6) through (6/6), never (4/3), (5/3), + // or (6/3) as observed in the bug report. + const denominator = 6; + for (let waveIdx = 0; waveIdx < denominator; waveIdx++) { + const event = { + timestamp: "t", + type: "merge_success" as any, + batchId: "b", + waveIndex: waveIdx, + totalWaves: denominator, + }; + const text = formatEventNotification(event, "supervised"); + const expectedNumerator = waveIdx + 1; + expect(text).toContain(`(${expectedNumerator}/${denominator})`); + // Belt-and-suspenders: the bug surfaced as a numerator that + // exceeded the denominator. Pin that the rendered text never + // contains `(N/3)` for any N — the only valid denominator in + // the segment-expanded case is the segment count. + expect(text).not.toMatch(/\(\d+\/3\)/); + } + }); + it("5.9: formats merge_failed differently for autonomous vs interactive", () => { const event = { timestamp: "t",