fix: gate preflight compaction on totalTokensFresh === true

kewang-pika · claude · kewang-pika · commit c149fe41f0b0 · 2026-04-14T13:34:02.000-05:00
The fresh-token branch in runPreflightCompactionIfNeeded ran when shouldUseTranscriptFallback was false, but that predicate treated totalTokensFresh: undefined as NOT needing fallback (undefined === false evaluates to false). For legacy sessions where totalTokensFresh was never set, this caused the fresh-token path to use potentially stale persisted totals, triggering unnecessary compaction. Three changes: 1. Replace shouldUseTranscriptFallback with hasFreshPersistedTokens that requires totalTokensFresh === true explicitly. When undefined (legacy) or false (known-stale), falls through to transcript estimation. 2. In the transcript fallback branch, always run transcript estimation instead of skipping when resolveFreshSessionTotalTokens returns a value (that function also treats undefined freshness as fresh). 3. Add bail-out before shouldRunPreflightCompaction when transcript estimation returns no count and freshness is unconfirmed, preventing the gate function from falling back to stale totals via resolveFreshSessionTotalTokens. Resolves Codex P2 review on PR #66716. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/src/auto-reply/reply/agent-runner-memory.test.ts b/src/auto-reply/reply/agent-runner-memory.test.ts
@@ -518,6 +518,48 @@ describe("runPreflightCompactionIfNeeded", () => {
     expect(result).toBe(sessionEntry);
   });
 
+  it("skips compaction when totalTokensFresh is undefined (legacy sessions)", async () => {
+    // Legacy session: totalTokensFresh was never set (undefined), but totalTokens
+    // has a stale persisted value above the threshold. Previously this was treated
+    // as fresh (undefined !== false → !shouldUseTranscriptFallback was true),
+    // causing the fresh-token branch to use stale persisted totals and trigger
+    // unnecessary compaction.
+    const sessionEntry: SessionEntry = {
+      sessionId: "session",
+      updatedAt: Date.now(),
+      totalTokens: 305_000,
+      // totalTokensFresh is intentionally omitted (undefined)
+    };
+    const sessionStore = { main: sessionEntry };
+
+    const result = await runPreflightCompactionIfNeeded({
+      cfg: {
+        agents: {
+          defaults: {
+            compaction: {
+              reserveTokensFloor: 30_000,
+            },
+          },
+        },
+      },
+      followupRun: createFollowupRun(),
+      defaultModel: "claude-sonnet-4-6",
+      agentCfgContextTokens: 200_000,
+      sessionEntry,
+      sessionStore,
+      sessionKey: "main",
+      isHeartbeat: false,
+      replyOperation: createReplyOperation(),
+    });
+
+    // With undefined freshness and no transcript file to read, transcript
+    // estimation returns undefined. The function bails out because there is
+    // no reliable token count and freshness is not confirmed, rather than
+    // falling back to stale persisted totals.
+    expect(compactEmbeddedPiSessionMock).not.toHaveBeenCalled();
+    expect(result).toBe(sessionEntry);
+  });
+
   it("skips compaction on heartbeat even with fresh tokens above threshold", async () => {
     const sessionEntry: SessionEntry = {
       sessionId: "session",
diff --git a/src/auto-reply/reply/agent-runner-memory.ts b/src/auto-reply/reply/agent-runner-memory.ts
@@ -390,7 +390,11 @@ export async function runPreflightCompactionIfNeeded(params: {
     typeof persistedTotalTokens === "number" &&
     Number.isFinite(persistedTotalTokens) &&
     persistedTotalTokens > 0;
-  const shouldUseTranscriptFallback = entry.totalTokensFresh === false || !hasPersistedTotalTokens;
+  // Only use persisted totals when totalTokensFresh is explicitly true.
+  // When totalTokensFresh is undefined (legacy sessions where freshness was
+  // never tracked), fall through to transcript estimation so we don't gate
+  // compaction on potentially stale persisted values.
+  const hasFreshPersistedTokens = hasPersistedTotalTokens && entry.totalTokensFresh === true;
 
   // Resolve the token count for the compaction threshold check.
   //
@@ -404,14 +408,15 @@ export async function runPreflightCompactionIfNeeded(params: {
   // threshold, so compaction never triggered — even at 153% of the context
   // window — when the prompt cache absorbed all tokens. (#66520)
   //
-  // When totalTokens is stale/unknown, fall back to reading the session
-  // transcript and estimating token counts from the message content.
+  // When totalTokens is stale/unknown (totalTokensFresh is false OR undefined),
+  // fall back to reading the session transcript and estimating token counts
+  // from the message content.
   const promptTokenEstimate = estimatePromptTokensForMemoryFlush(
     params.promptForEstimate ?? params.followupRun.prompt,
   );
   let tokenCountForCompaction: number | undefined;
   let transcriptPromptTokens: number | undefined;
-  if (!shouldUseTranscriptFallback) {
+  if (hasFreshPersistedTokens) {
     // Fresh persisted tokens available — project forward with prompt estimate
     const projectedFreshTokens = resolveEffectivePromptTokens(
       freshPersistedTokens,
@@ -425,15 +430,16 @@ export async function runPreflightCompactionIfNeeded(params: {
         ? projectedFreshTokens
         : freshPersistedTokens;
   } else {
-    // Stale/unknown — fall back to transcript estimation
-    transcriptPromptTokens =
-      typeof freshPersistedTokens === "number"
-        ? undefined
-        : estimatePromptTokensFromSessionTranscript({
-            sessionId: entry.sessionId,
-            storePath: params.storePath,
-            sessionFile: entry.sessionFile ?? params.followupRun.run.sessionFile,
-          });
+    // Stale/unknown — fall back to transcript estimation.
+    // Always estimate from transcript when freshness is not confirmed,
+    // even if resolveFreshSessionTotalTokens would return a value
+    // (it treats totalTokensFresh: undefined as fresh, which is the
+    // exact bug we're guarding against here).
+    transcriptPromptTokens = estimatePromptTokensFromSessionTranscript({
+      sessionId: entry.sessionId,
+      storePath: params.storePath,
+      sessionFile: entry.sessionFile ?? params.followupRun.run.sessionFile,
+    });
     const projectedTokenCount =
       typeof transcriptPromptTokens === "number"
         ? resolveEffectivePromptTokens(transcriptPromptTokens, undefined, promptTokenEstimate)
@@ -457,6 +463,15 @@ export async function runPreflightCompactionIfNeeded(params: {
       `promptTokensEst=${promptTokenEstimate ?? "undefined"}`,
   );
 
+  // If transcript estimation didn't produce a token count and persisted totals
+  // are not confirmed fresh, we have no reliable basis for compaction gating.
+  // Bail out rather than letting shouldRunPreflightCompaction fall back to
+  // resolveFreshSessionTotalTokens (which treats totalTokensFresh: undefined
+  // as fresh).
+  if (typeof tokenCountForCompaction !== "number" && entry.totalTokensFresh !== true) {
+    return entry ?? params.sessionEntry;
+  }
+
   const shouldCompact = shouldRunPreflightCompaction({
     entry,
     tokenCount: tokenCountForCompaction,
