From fb12dfadabdb9f4e5a0e82c40abe342fb97bd3ae Mon Sep 17 00:00:00 2001
From: STiFLeR7 <hillaniljppatel@gmail.com>
Date: Thu, 10 Sep 2026 09:08:31 +0530
Subject: [PATCH] fix(metrics): claw back sessionsSucceeded when a resumed
 session later fails

computeDailyStatsDelta only added sessionsSucceeded on a session's
first report, so a session reported succeeded=1 that later fails
after being resumed (interrupted/corrected) never has that earlier
increment subtracted. Team-wide success rate stays inflated forever.

Compute a signed delta (snapshot.succeeded - previous.succeeded)
every report instead of gating it to the first one, matching the
issue's own suggested minimal fix. sessionsEnded still only increments
once, since the session itself isn't counted twice.

hasDailyDelta's push gate checked sessionsSucceeded > 0, which would
skip writing a report where succeeded is the only thing that changed
and it went negative. Changed to !== 0.

Fixes #473 (part 1 of 2 - see PR description for the other, which
needs a product decision, not a code fix, before it can be resolved)
---
 src/__tests__/session-trends.test.ts | 23 +++++++++++++++++++++++
 src/session-trends.ts                |  6 +++++-
 src/team-push.ts                     |  5 ++++-
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/src/__tests__/session-trends.test.ts b/src/__tests__/session-trends.test.ts
index 1f4a2a4e..001ad9a8 100644
--- a/src/__tests__/session-trends.test.ts
+++ b/src/__tests__/session-trends.test.ts
@@ -61,6 +61,29 @@ describe('daily session trends', () => {
     expect(second.delta['2026-09-03']).toMatchObject({ pricedRequests: 1, costMicros: 150, sessionsEnded: 0 });
   });
 
+  it('claws back sessionsSucceeded when a resumed session later fails (#473)', () => {
+    const succeeded = new Map([
+      ['s1', { date: '2026-09-02', prompts: 1, durationMs: 60_000, succeeded: 1 as const, corrected: 0 as const, requestDaily: {} }],
+    ]);
+    const first = computeDailyStatsDelta(succeeded, {});
+    expect(first.delta['2026-09-02']).toMatchObject({ sessionsEnded: 1, sessionsSucceeded: 1 });
+    const merged = mergeDailyStats(undefined, first.delta);
+    expect(merged['2026-09-02']).toMatchObject({ sessionsEnded: 1, sessionsSucceeded: 1 });
+
+    const interrupted = new Map([
+      ['s1', { ...succeeded.get('s1')!, succeeded: 0 as const, corrected: 1 as const }],
+    ]);
+    const second = computeDailyStatsDelta(interrupted, first.nextReported);
+    expect(second.delta['2026-09-02']).toMatchObject({ sessionsEnded: 0, sessionsSucceeded: -1, sessionsCorrected: 1 });
+    const remerged = mergeDailyStats(merged, second.delta);
+    expect(remerged['2026-09-02']).toMatchObject({ sessionsEnded: 1, sessionsSucceeded: 0 });
+
+    // A later, unrelated re-report of the same now-failed state must not
+    // double-subtract: the delta settles back to 0 once the baseline catches up.
+    const third = computeDailyStatsDelta(interrupted, second.nextReported);
+    expect(third.delta['2026-09-02']).toMatchObject({ sessionsEnded: 0, sessionsSucceeded: 0 });
+  });
+
   it('compares the latest seven UTC days with the prior seven days', () => {
     const daily: Record<string, DailyUserStats> = {
       '2026-08-27': { sessionsEnded: 10, sessionsSucceeded: 5, promptTurns: 80, durationMs: 600_000, sessionsCorrected: 4, pricedRequests: 10, costMicros: 1_000_000, cacheReadTokens: 20, cacheEligibleInputTokens: 100 },
diff --git a/src/session-trends.ts b/src/session-trends.ts
index d91e7a09..3dfa7d6c 100644
--- a/src/session-trends.ts
+++ b/src/session-trends.ts
@@ -104,8 +104,12 @@ export function computeDailyStatsDelta(
     const bucket = delta[date] ?? emptyDaily();
     if (!previous) {
       bucket.sessionsEnded += 1;
-      bucket.sessionsSucceeded += snapshot.succeeded;
     }
+    // Signed, not positiveDelta: unlike the monotonic counters below, a
+    // session can flip from succeeded to failed on a later report (resumed
+    // after an interruption/correction), and that must claw back the earlier
+    // sessionsSucceeded increment, not just skip adding a new one (#473).
+    bucket.sessionsSucceeded += snapshot.succeeded - (previous?.succeeded ?? 0);
     bucket.promptTurns += positiveDelta(snapshot.prompts, previous?.prompts);
     bucket.durationMs += positiveDelta(snapshot.durationMs, previous?.durationMs);
     bucket.sessionsCorrected += positiveDelta(snapshot.corrected, previous?.corrected);
diff --git a/src/team-push.ts b/src/team-push.ts
index f178b500..e7aa948f 100644
--- a/src/team-push.ts
+++ b/src/team-push.ts
@@ -298,7 +298,10 @@ async function writeReportedDailySessions(data: ReportedDailySessions): Promise<
 
 function hasDailyDelta(delta: ReturnType<typeof computeDailyStatsDelta>['delta']): boolean {
   return Object.values(delta).some((bucket) =>
-    bucket.sessionsEnded > 0 || bucket.sessionsSucceeded > 0 || bucket.promptTurns > 0
+    // sessionsSucceeded can be negative (a resumed session that later failed
+    // claws back an earlier increment), so it must not be checked with the
+    // same "> 0" as the other, purely monotonic counters (#473).
+    bucket.sessionsEnded > 0 || bucket.sessionsSucceeded !== 0 || bucket.promptTurns > 0
     || bucket.durationMs > 0 || bucket.sessionsCorrected > 0 || bucket.pricedRequests > 0
     || bucket.costMicros > 0 || bucket.cacheReadTokens > 0 || bucket.cacheEligibleInputTokens > 0,
   );