From 5a6d94c0517129fda343364ad5135be329b4daa1 Mon Sep 17 00:00:00 2001 From: samzong Date: Tue, 10 Mar 2026 21:12:42 +0800 Subject: [PATCH] fix(compaction): share real-conversation predicate and ignore tool-call-only assistant blocks --- src/agents/compaction-real-conversation.ts | 77 +++++++++++++++++++ .../pi-embedded-runner/compact.hooks.test.ts | 60 ++++++++++----- src/agents/pi-embedded-runner/compact.ts | 67 +++------------- .../compaction-safeguard.test.ts | 20 ++++- .../pi-extensions/compaction-safeguard.ts | 68 +--------------- 5 files changed, 151 insertions(+), 141 deletions(-) create mode 100644 src/agents/compaction-real-conversation.ts diff --git a/src/agents/compaction-real-conversation.ts b/src/agents/compaction-real-conversation.ts new file mode 100644 index 00000000000..67ca24714c0 --- /dev/null +++ b/src/agents/compaction-real-conversation.ts @@ -0,0 +1,77 @@ +import type { AgentMessage } from "@mariozechner/pi-agent-core"; +import { stripHeartbeatToken } from "../auto-reply/heartbeat.js"; +import { isSilentReplyText } from "../auto-reply/tokens.js"; + +export const TOOL_RESULT_REAL_CONVERSATION_LOOKBACK = 20; +const TOOL_ONLY_BLOCK_TYPES = new Set(["toolCall", "toolUse", "functionCall"]); + +function hasMeaningfulText(text: string): boolean { + const trimmed = text.trim(); + if (!trimmed) { + return false; + } + if (isSilentReplyText(trimmed)) { + return false; + } + const heartbeat = stripHeartbeatToken(trimmed, { mode: "message" }); + if (heartbeat.didStrip) { + return heartbeat.text.trim().length > 0; + } + return true; +} + +export function hasMeaningfulConversationContent(message: AgentMessage): boolean { + const content = (message as { content?: unknown }).content; + if (typeof content === "string") { + return hasMeaningfulText(content); + } + if (!Array.isArray(content)) { + return false; + } + let sawMeaningfulNonTextBlock = false; + for (const block of content) { + if (!block || typeof block !== "object") { + continue; + } + const type = (block as { type?: unknown }).type; + if (type !== "text") { + if (typeof type === "string" && TOOL_ONLY_BLOCK_TYPES.has(type)) { + continue; + } + sawMeaningfulNonTextBlock = true; + continue; + } + const text = (block as { text?: unknown }).text; + if (typeof text !== "string") { + continue; + } + if (hasMeaningfulText(text)) { + return true; + } + } + return sawMeaningfulNonTextBlock; +} + +export function isRealConversationMessage( + message: AgentMessage, + messages: AgentMessage[], + index: number, +): boolean { + if (message.role === "user" || message.role === "assistant") { + return hasMeaningfulConversationContent(message); + } + if (message.role !== "toolResult") { + return false; + } + const start = Math.max(0, index - TOOL_RESULT_REAL_CONVERSATION_LOOKBACK); + for (let i = index - 1; i >= start; i -= 1) { + const candidate = messages[i]; + if (!candidate || candidate.role !== "user") { + continue; + } + if (hasMeaningfulConversationContent(candidate)) { + return true; + } + } + return false; +} diff --git a/src/agents/pi-embedded-runner/compact.hooks.test.ts b/src/agents/pi-embedded-runner/compact.hooks.test.ts index 3819ca98a17..dbe4a9182c5 100644 --- a/src/agents/pi-embedded-runner/compact.hooks.test.ts +++ b/src/agents/pi-embedded-runner/compact.hooks.test.ts @@ -1,3 +1,4 @@ +import type { AgentMessage } from "@mariozechner/pi-agent-core"; import { getApiProvider, unregisterApiProviders } from "@mariozechner/pi-ai"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { getCustomApiRegistrySourceId } from "../custom-api-registry.js"; @@ -23,6 +24,7 @@ import { let compactEmbeddedPiSessionDirect: typeof import("./compact.js").compactEmbeddedPiSessionDirect; let compactEmbeddedPiSession: typeof import("./compact.js").compactEmbeddedPiSession; +let compactTesting: typeof import("./compact.js").__testing; let onSessionTranscriptUpdate: typeof import("../../sessions/transcript-events.js").onSessionTranscriptUpdate; const TEST_SESSION_ID = "session-1"; @@ -109,6 +111,7 @@ beforeAll(async () => { const loaded = await loadCompactHooksHarness(); compactEmbeddedPiSessionDirect = loaded.compactEmbeddedPiSessionDirect; compactEmbeddedPiSession = loaded.compactEmbeddedPiSession; + compactTesting = loaded.__testing; onSessionTranscriptUpdate = loaded.onSessionTranscriptUpdate; }); @@ -509,7 +512,7 @@ describe("compactEmbeddedPiSessionDirect hooks", () => { sessionMessages.splice( 0, sessionMessages.length, - { role: "user", content: "HEARTBEAT_OK", timestamp: 1 }, + { role: "user", content: "HEARTBEAT_OK", timestamp: 1 }, { role: "toolResult", toolCallId: "t1", @@ -536,31 +539,50 @@ describe("compactEmbeddedPiSessionDirect hooks", () => { expect(sessionCompactImpl).not.toHaveBeenCalled(); }); - it("keeps compaction enabled when tool output follows a meaningful user request", async () => { - sessionMessages.splice( - 0, - sessionMessages.length, - { role: "user", content: "please inspect the failing PR", timestamp: 1 }, + it("does not treat assistant-only tool-call blocks as meaningful conversation", () => { + expect( + compactTesting.hasMeaningfulConversationContent({ + role: "assistant", + content: [{ type: "toolCall", id: "call_1", name: "exec", arguments: {} }], + } as AgentMessage), + ).toBe(false); + }); + + it("counts tool output as real only when a meaningful user ask exists in the lookback window", () => { + const heartbeatToolResultWindow = [ + { role: "user", content: "HEARTBEAT_OK" }, { role: "toolResult", toolCallId: "t1", toolName: "exec", content: [{ type: "text", text: "checked" }], - isError: false, - timestamp: 2, }, - ); + ] as AgentMessage[]; + expect( + compactTesting.hasRealConversationContent( + heartbeatToolResultWindow[1], + heartbeatToolResultWindow, + 1, + ), + ).toBe(false); - const result = await compactEmbeddedPiSessionDirect({ - sessionId: "session-1", - sessionKey: "agent:main:session-1", - sessionFile: "/tmp/session.jsonl", - workspaceDir: "/tmp", - customInstructions: "focus on decisions", - }); - - expect(result.ok).toBe(true); - expect(sessionCompactImpl).toHaveBeenCalled(); + const realAskToolResultWindow = [ + { role: "assistant", content: "NO_REPLY" }, + { role: "user", content: "please inspect the failing PR" }, + { + role: "toolResult", + toolCallId: "t2", + toolName: "exec", + content: [{ type: "text", text: "checked" }], + }, + ] as AgentMessage[]; + expect( + compactTesting.hasRealConversationContent( + realAskToolResultWindow[2], + realAskToolResultWindow, + 2, + ), + ).toBe(true); }); it("registers the Ollama api provider before compaction", async () => { diff --git a/src/agents/pi-embedded-runner/compact.ts b/src/agents/pi-embedded-runner/compact.ts index 622273f250f..cb1787d3545 100644 --- a/src/agents/pi-embedded-runner/compact.ts +++ b/src/agents/pi-embedded-runner/compact.ts @@ -38,6 +38,10 @@ import { resolveSessionAgentId, resolveSessionAgentIds } from "../agent-scope.js import type { ExecElevatedDefaults } from "../bash-tools.js"; import { makeBootstrapWarn, resolveBootstrapContextForRun } from "../bootstrap-files.js"; import { listChannelSupportedActions, resolveChannelMessageToolHints } from "../channel-tools.js"; +import { + hasMeaningfulConversationContent, + isRealConversationMessage, +} from "../compaction-real-conversation.js"; import { resolveContextWindowInfo } from "../context-window-guard.js"; import { ensureCustomApiRegistered } from "../custom-api-registry.js"; import { formatUserTime, resolveUserTimeFormat, resolveUserTimezone } from "../date-time.js"; @@ -167,68 +171,12 @@ type CompactionMessageMetrics = { contributors: Array<{ role: string; chars: number; tool?: string }>; }; -const BOILERPLATE_REPLY_TEXT = new Set(["HEARTBEAT_OK", "NO_REPLY"]); -const TOOL_RESULT_REAL_CONVERSATION_LOOKBACK = 20; - -function hasMeaningfulConversationContent(msg: AgentMessage): boolean { - const content = (msg as { content?: unknown }).content; - if (typeof content === "string") { - const trimmed = content.trim(); - if (!trimmed) { - return false; - } - return !BOILERPLATE_REPLY_TEXT.has(trimmed); - } - if (!Array.isArray(content)) { - return false; - } - let sawNonTextBlock = false; - for (const block of content) { - if (!block || typeof block !== "object") { - continue; - } - const type = (block as { type?: unknown }).type; - if (type !== "text") { - sawNonTextBlock = true; - continue; - } - const text = (block as { text?: unknown }).text; - if (typeof text !== "string") { - continue; - } - const trimmed = text.trim(); - if (!trimmed) { - continue; - } - if (!BOILERPLATE_REPLY_TEXT.has(trimmed)) { - return true; - } - } - return sawNonTextBlock; -} - function hasRealConversationContent( msg: AgentMessage, messages: AgentMessage[], index: number, ): boolean { - if (msg.role === "user" || msg.role === "assistant") { - return hasMeaningfulConversationContent(msg); - } - if (msg.role !== "toolResult") { - return false; - } - const start = Math.max(0, index - TOOL_RESULT_REAL_CONVERSATION_LOOKBACK); - for (let i = index - 1; i >= start; i -= 1) { - const candidate = messages[i]; - if (!candidate || candidate.role !== "user") { - continue; - } - if (hasMeaningfulConversationContent(candidate)) { - return true; - } - } - return false; + return isRealConversationMessage(msg, messages, index); } function createCompactionDiagId(): string { @@ -1314,3 +1262,8 @@ export async function compactEmbeddedPiSession( }), ); } + +export const __testing = { + hasRealConversationContent, + hasMeaningfulConversationContent, +} as const; diff --git a/src/agents/pi-extensions/compaction-safeguard.test.ts b/src/agents/pi-extensions/compaction-safeguard.test.ts index 41906b36de7..9901316745b 100644 --- a/src/agents/pi-extensions/compaction-safeguard.test.ts +++ b/src/agents/pi-extensions/compaction-safeguard.test.ts @@ -1684,7 +1684,7 @@ describe("compaction-safeguard double-compaction guard", () => { content: [{ type: "text", text: "done" }], } as AgentMessage, [ - { role: "user", content: "HEARTBEAT_OK" } as AgentMessage, + { role: "user", content: "HEARTBEAT_OK" } as AgentMessage, { role: "toolResult", toolCallId: "t1", @@ -1717,6 +1717,24 @@ describe("compaction-safeguard double-compaction guard", () => { ), ).toBe(true); }); + + it("does not treat assistant-only tool calls as meaningful conversation", () => { + expect( + __testing.hasMeaningfulConversationContent({ + role: "assistant", + content: [{ type: "toolCall", id: "call_1", name: "exec", arguments: {} }], + } as AgentMessage), + ).toBe(false); + }); + + it("treats markup-wrapped heartbeat tokens as boilerplate", () => { + expect( + __testing.hasMeaningfulConversationContent({ + role: "assistant", + content: "HEARTBEAT_OK", + } as AgentMessage), + ).toBe(false); + }); }); async function expectWorkspaceSummaryEmptyForAgentsAlias( diff --git a/src/agents/pi-extensions/compaction-safeguard.ts b/src/agents/pi-extensions/compaction-safeguard.ts index 7cea062d216..5d5e0e9b059 100644 --- a/src/agents/pi-extensions/compaction-safeguard.ts +++ b/src/agents/pi-extensions/compaction-safeguard.ts @@ -6,6 +6,10 @@ import { extractSections } from "../../auto-reply/reply/post-compaction-context. import { openBoundaryFile } from "../../infra/boundary-file-read.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { extractKeywords, isQueryStopWordToken } from "../../memory/query-expansion.js"; +import { + hasMeaningfulConversationContent, + isRealConversationMessage, +} from "../compaction-real-conversation.js"; import { BASE_CHUNK_RATIO, type CompactionSummarizationInstructions, @@ -179,70 +183,6 @@ function formatToolFailuresSection(failures: ToolFailure[]): string { return `\n\n## Tool Failures\n${lines.join("\n")}`; } -const BOILERPLATE_REPLY_TEXT = new Set(["HEARTBEAT_OK", "NO_REPLY"]); -const TOOL_RESULT_REAL_CONVERSATION_LOOKBACK = 20; - -function hasMeaningfulConversationContent(message: AgentMessage): boolean { - const content = (message as { content?: unknown }).content; - if (typeof content === "string") { - const trimmed = content.trim(); - if (!trimmed) { - return false; - } - return !BOILERPLATE_REPLY_TEXT.has(trimmed); - } - if (!Array.isArray(content)) { - return false; - } - let sawNonTextBlock = false; - for (const block of content) { - if (!block || typeof block !== "object") { - continue; - } - const type = (block as { type?: unknown }).type; - if (type !== "text") { - sawNonTextBlock = true; - continue; - } - const text = (block as { text?: unknown }).text; - if (typeof text !== "string") { - continue; - } - const trimmed = text.trim(); - if (!trimmed) { - continue; - } - if (!BOILERPLATE_REPLY_TEXT.has(trimmed)) { - return true; - } - } - return sawNonTextBlock; -} - -function isRealConversationMessage( - message: AgentMessage, - messages: AgentMessage[], - index: number, -): boolean { - if (message.role === "user" || message.role === "assistant") { - return hasMeaningfulConversationContent(message); - } - if (message.role !== "toolResult") { - return false; - } - const start = Math.max(0, index - TOOL_RESULT_REAL_CONVERSATION_LOOKBACK); - for (let i = index - 1; i >= start; i -= 1) { - const candidate = messages[i]; - if (!candidate || candidate.role !== "user") { - continue; - } - if (hasMeaningfulConversationContent(candidate)) { - return true; - } - } - return false; -} - function computeFileLists(fileOps: FileOperations): { readFiles: string[]; modifiedFiles: string[];