From 41324ab739b98ba01de3c1d16a7945b692677c5c Mon Sep 17 00:00:00 2001 From: jonnyparris <6400000+jonnyparris@users.noreply.github.com> Date: Mon, 25 May 2026 10:34:01 +0100 Subject: [PATCH] fix(own-loop): prune fires on cumulative tokens; compaction logs truthfully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes that close the gap between PR #75's prune helper and its actual deployment behaviour, found by deploying #75 and watching `wrangler tail` on a Gemma session. ## 1. Prune trigger now considers cumulative tokens, not just projected PR #75's prune block was gated on `projectedInputTokens / tokenBudget >= MID_LOOP_COMPACTION_THRESHOLD`, where `projectedInputTokens` measures the size of the next outbound message array. For typical Gemma sessions each per-step call is ~19k tokens (system prompt + tool defs dominate), so the per-iteration projection stays well below the 50% threshold even as the cumulative total climbs through 58 → 67 → 77 → 87%. The prune never fired. Now triggers on EITHER signal: - projected: any single iteration is already huge - cumulative: the running total of inputs sent this turn has climbed past threshold The cumulative signal is the one that actually matches the Gemma failure mode. Log line gains a `trigger: cumulative | projected` field. ## 2. Compaction logs distinguish summarised vs noop The own-loop's compaction trigger sites all logged "compaction triggered" unconditionally — even when `maybeCompactContext()` no-op'd because Think's persisted history had nothing to summarise. That made the bug invisible during debugging (we hit this twice — first time was PR #74's diagnosis). All three trigger sites (loop-entry, mid-loop, pre-step) now: 1. Read `getCompactionCount()` before the attempt 2. Call `maybeCompactContext` 3. Read again, log `outcome: "summarised" | "noop"` The log line name also changed from `*compaction triggered` to `*compaction attempted` for honesty. New private helper `getCompactionCount()` wraps the `sessions.getCompactions(thinkSessionId).length` lookup so the call sites read clean. ## Tests All 28 compaction + prune tests pass; typecheck clean. No new tests: both changes are observability + trigger-condition fixes, not new algorithms. The behavioural change (prune firing on cumulative) is covered by the existing prune unit tests via the `estimate` callback. --- src/coding-agent.ts | 78 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 15 deletions(-) diff --git a/src/coding-agent.ts b/src/coding-agent.ts index f1f0c62..3b3a45c 100644 --- a/src/coding-agent.ts +++ b/src/coding-agent.ts @@ -926,19 +926,22 @@ export class CodingAgent extends Think { const entryUsage = entryInputTokens / tokenBudget; if (entryUsage >= MID_LOOP_COMPACTION_THRESHOLD) { compactionTriggered = true; - log("info", "own-loop: loop-entry compaction triggered", { - sessionId, - entryInputTokens, - tokenBudget, - entryUsage: `${Math.round(entryUsage * 100)}%`, - }); try { + const compactionsBefore = self.getCompactionCount(); await self.maybeCompactContext({ force: true }); + const compactionsAfter = self.getCompactionCount(); const thinkSessionId = self.getCurrentSessionId(); if (thinkSessionId) { self.messages = self.sessions.getHistory(thinkSessionId); } messages = await self.assembleContext(); + log("info", "own-loop: loop-entry compaction attempted", { + sessionId, + entryInputTokens, + tokenBudget, + entryUsage: `${Math.round(entryUsage * 100)}%`, + outcome: compactionsAfter > compactionsBefore ? "summarised" : "noop", + }); } catch (err) { log("warn", "own-loop: loop-entry compaction failed", { sessionId, @@ -1065,18 +1068,26 @@ export class CodingAgent extends Think { // own-loop has already proved compaction is warranted by // measuring cumulativeInputTokens directly — don't make // maybeCompactContext re-decide. + const compactionsBefore = self.getCompactionCount(); await self.maybeCompactContext({ force: true }); + const compactionsAfter = self.getCompactionCount(); // Refresh messages from storage and re-assemble with compaction summary. const thinkSessionId = self.getCurrentSessionId(); if (thinkSessionId) { self.messages = self.sessions.getHistory(thinkSessionId); } messages = await self.assembleContext(); - log("info", "own-loop: mid-loop compaction triggered", { + // Log the outcome truthfully — distinguishes "we attempted and + // it summarised something" from "we attempted but it no-op'd + // because there's nothing in Think's persisted history yet." + // Without the distinction, debugging compaction issues is a + // wild goose chase (we hit that twice already — see PR #74). + log("info", "own-loop: mid-loop compaction attempted", { sessionId, step, cumulativeInputTokens, tokenBudget, + outcome: compactionsAfter > compactionsBefore ? "summarised" : "noop", }); } catch (err) { log("warn", "own-loop: mid-loop compaction failed", { @@ -1105,15 +1116,10 @@ export class CodingAgent extends Think { if (projectedUsage >= MID_LOOP_COMPACTION_THRESHOLD && !compactionTriggered) { compactionTriggered = true; - log("info", "own-loop: pre-step compaction triggered", { - sessionId, - step, - projectedInputTokens, - tokenBudget, - projectedUsage: `${Math.round(projectedUsage * 100)}%`, - }); try { + const compactionsBefore = self.getCompactionCount(); await self.maybeCompactContext({ force: true }); + const compactionsAfter = self.getCompactionCount(); const thinkSessionId = self.getCurrentSessionId(); if (thinkSessionId) { self.messages = self.sessions.getHistory(thinkSessionId); @@ -1122,6 +1128,14 @@ export class CodingAgent extends Think { finalMessages = injections.length > 0 ? [...messages, ...injections] : messages; + log("info", "own-loop: pre-step compaction attempted", { + sessionId, + step, + projectedInputTokens, + tokenBudget, + projectedUsage: `${Math.round(projectedUsage * 100)}%`, + outcome: compactionsAfter > compactionsBefore ? "summarised" : "noop", + }); } catch (err) { log("warn", "own-loop: pre-step compaction failed", { sessionId, @@ -1145,8 +1159,29 @@ export class CodingAgent extends Think { // back under the mid-loop threshold. Preserves the tool-call // envelope (toolName, toolCallId) so the model's chain of // pending tool calls still resolves. + // + // **Trigger condition.** Two signals can independently warrant + // a prune: + // 1. `projectedInputTokens` — what we're about to send to + // streamText on this iteration. Useful when the current + // iteration's payload is already huge. + // 2. `cumulativeInputTokens` — what we've sent across ALL + // iterations of this turn so far. Useful when the cumulative + // climb is the problem even though any single iteration + // looks small. + // + // Without #2 the prune never fires for the typical Gemma + // failure: each per-step call sends ~19k tokens (system prompt + // + tool defs dominate) so the per-iteration projection stays + // small, but the running total burns through the budget. We + // saw this empirically: 8 successful steps, cumulative 158k + // (77%), zero prune log lines. const projectedAfterCompactionTokens = estimateMessagesTokens(finalMessages); - if (projectedAfterCompactionTokens / tokenBudget >= MID_LOOP_COMPACTION_THRESHOLD) { + const projectedTrigger = + projectedAfterCompactionTokens / tokenBudget >= MID_LOOP_COMPACTION_THRESHOLD; + const cumulativeTrigger = + cumulativeInputTokens / tokenBudget >= MID_LOOP_COMPACTION_THRESHOLD; + if (projectedTrigger || cumulativeTrigger) { const pruneResult = pruneOversizedToolResults(finalMessages, { targetTokens: Math.floor(tokenBudget * MID_LOOP_COMPACTION_THRESHOLD), estimate: estimateMessagesTokens, @@ -1159,6 +1194,7 @@ export class CodingAgent extends Think { bytesRemoved: pruneResult.bytesRemoved, tokensBefore: pruneResult.tokensBefore, tokensAfter: pruneResult.tokensAfter, + trigger: cumulativeTrigger ? "cumulative" : "projected", }); } } @@ -4973,6 +5009,18 @@ export class CodingAgent extends Think { * Triggers when the last turn's input tokens exceed COMPACTION_TRIGGER_PERCENT * of the context budget. */ + /** + * Number of compaction summaries persisted for the current Think session. + * Used by the own-loop to decide whether a `maybeCompactContext()` call + * actually summarised something or no-op'd (see `outcome: "summarised"` + * vs `"noop"` log lines). Returns 0 when there is no active session. + */ + private getCompactionCount(): number { + const thinkSessionId = this.getCurrentSessionId(); + if (!thinkSessionId) return 0; + return this.sessions.getCompactions(thinkSessionId).length; + } + private async maybeCompactContext(options?: { force?: boolean }): Promise { const thinkSessionId = this.getCurrentSessionId(); if (!thinkSessionId) return;