From c9abe1dcaf61e16142a508caeb903706431c5222 Mon Sep 17 00:00:00 2001 From: TechSphrex TA <42131590+KhaiTrang1995@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:11:56 +0700 Subject: [PATCH] fix(loop-metrics): stop successRatePct from going negative on multi-escalation runs successRatePct was computed as ((totalRuns - totalEscalations) / totalRuns) * 100. totalEscalations is a sum of each run's escalation *count* (a run can log more than one -- e.g. several items escalated in a single triage pass; this repo's own loop-run-log.md has real entries with escalations: 4 and escalations: 5), not a count of runs-that-escalated. Subtracting an event count from a run count conflates the two: any window where totalEscalations exceeds totalRuns (trivially reachable with real data, e.g. two runs where one logs escalations: 5) produces a negative percentage, which the CLI dashboard prints and color-codes as-is with no clamping. Success rate should mean "what fraction of runs didn't escalate at all." Count runs with escalations === 0 instead of subtracting the raw event sum, so the result stays correctly bounded to [0, 100] regardless of how many escalations any single run logged. totalEscalations itself is unchanged (still a raw sum) since roiScore correctly treats each individual escalation event as a cost. Test plan: added a regression test with one run logging escalations: 5 and one clean run, asserting successRatePct is 50 (not -150). Confirmed it fails with the exact -150 result against the old formula and passes with the fix. Full clean rebuild + npm test: 3/3 passing. Co-Authored-By: Claude Sonnet 5 --- tools/loop-metrics/dist/metrics.js | 13 ++++++++++++- tools/loop-metrics/src/metrics.ts | 12 +++++++++++- tools/loop-metrics/test/metrics.test.mjs | 18 ++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tools/loop-metrics/dist/metrics.js b/tools/loop-metrics/dist/metrics.js index 2b41b2b4..aa4797d3 100644 --- a/tools/loop-metrics/dist/metrics.js +++ b/tools/loop-metrics/dist/metrics.js @@ -48,14 +48,25 @@ export function aggregateMetrics(entries) { let totalDurationS = 0; let totalActionsTaken = 0; let totalEscalations = 0; + let runsWithoutEscalation = 0; for (const entry of entries) { totalTokens += entry.tokens_estimate || 0; totalDurationS += entry.duration_s || 0; totalActionsTaken += entry.actions_taken || 0; totalEscalations += entry.escalations || 0; + if (!entry.escalations) + runsWithoutEscalation++; } const totalRuns = entries.length; - const successRatePct = totalRuns > 0 ? ((totalRuns - totalEscalations) / totalRuns) * 100 : 0; + // totalEscalations is a sum of each run's escalation *count* (a run can + // log more than one, e.g. several items escalated in one triage pass) -- + // subtracting it from totalRuns (a count of runs) conflates events with + // runs and can go negative for a real, ordinary run log (this repo's own + // loop-run-log.md has entries with escalations: 4 and escalations: 5). + // Success rate is "what fraction of runs didn't escalate at all", which + // stays correctly bounded to [0, 100] regardless of how many escalations + // any single run logged. + const successRatePct = totalRuns > 0 ? (runsWithoutEscalation / totalRuns) * 100 : 0; const avgDurationS = totalRuns > 0 ? totalDurationS / totalRuns : 0; // Simple heuristic: Each successful action is worth +10, each escalation is -5. const roiScore = (totalActionsTaken * 10) - (totalEscalations * 5); diff --git a/tools/loop-metrics/src/metrics.ts b/tools/loop-metrics/src/metrics.ts index 14afcad5..b4448a53 100644 --- a/tools/loop-metrics/src/metrics.ts +++ b/tools/loop-metrics/src/metrics.ts @@ -77,16 +77,26 @@ export function aggregateMetrics(entries: RunEntry[]): MetricsDashboard { let totalDurationS = 0; let totalActionsTaken = 0; let totalEscalations = 0; + let runsWithoutEscalation = 0; for (const entry of entries) { totalTokens += entry.tokens_estimate || 0; totalDurationS += entry.duration_s || 0; totalActionsTaken += entry.actions_taken || 0; totalEscalations += entry.escalations || 0; + if (!entry.escalations) runsWithoutEscalation++; } const totalRuns = entries.length; - const successRatePct = totalRuns > 0 ? ((totalRuns - totalEscalations) / totalRuns) * 100 : 0; + // totalEscalations is a sum of each run's escalation *count* (a run can + // log more than one, e.g. several items escalated in one triage pass) -- + // subtracting it from totalRuns (a count of runs) conflates events with + // runs and can go negative for a real, ordinary run log (this repo's own + // loop-run-log.md has entries with escalations: 4 and escalations: 5). + // Success rate is "what fraction of runs didn't escalate at all", which + // stays correctly bounded to [0, 100] regardless of how many escalations + // any single run logged. + const successRatePct = totalRuns > 0 ? (runsWithoutEscalation / totalRuns) * 100 : 0; const avgDurationS = totalRuns > 0 ? totalDurationS / totalRuns : 0; // Simple heuristic: Each successful action is worth +10, each escalation is -5. diff --git a/tools/loop-metrics/test/metrics.test.mjs b/tools/loop-metrics/test/metrics.test.mjs index 056d793b..a331dba9 100644 --- a/tools/loop-metrics/test/metrics.test.mjs +++ b/tools/loop-metrics/test/metrics.test.mjs @@ -17,6 +17,24 @@ test('loop-metrics filters and aggregates', () => { assert.strictEqual(metrics.totalActionsTaken, 3); assert.strictEqual(metrics.totalEscalations, 1); assert.strictEqual(metrics.roiScore, (3 * 10) - (1 * 5)); // 25 + assert.strictEqual(metrics.successRatePct, 50); // 1 of 2 runs had no escalation +}); + +test('successRatePct stays within [0, 100] when a single run logs more than one escalation', () => { + // A run can escalate several items in one pass (this repo's own + // loop-run-log.md has real entries with escalations: 4 and 5), so + // totalEscalations (a sum of per-run counts) can exceed totalRuns. + // successRatePct must still reflect "runs that didn't escalate", not go + // negative from subtracting an event count as if it were a run count. + const entries = [ + { run_id: 'a', pattern: 'ci-sweeper', duration_s: 1, items_found: 5, actions_taken: 1, escalations: 5, tokens_estimate: 1000, outcome: 'escalated' }, + { run_id: 'b', pattern: 'ci-sweeper', duration_s: 1, items_found: 0, actions_taken: 0, escalations: 0, tokens_estimate: 1000, outcome: 'report-only' }, + ]; + + const metrics = aggregateMetrics(entries); + assert.strictEqual(metrics.totalEscalations, 5); + assert.strictEqual(metrics.successRatePct, 50); // 1 of 2 runs had no escalation + assert.ok(metrics.successRatePct >= 0 && metrics.successRatePct <= 100); }); test('filterEntries keeps entries with unparseable run_id when a timeframe is set', () => {