From b32201979b79be858cf961c5ff48330fb95bed99 Mon Sep 17 00:00:00 2001 From: Br1an67 <932039080@qq.com> Date: Sun, 15 Mar 2026 00:51:02 +0800 Subject: [PATCH] fix(compaction): content-aware isRealConversationMessage skips heartbeat/empty messages The compaction safeguard counted all user/assistant messages as real, including HEARTBEAT_OK and NO_REPLY boilerplate. This caused premature compaction. Now checks for meaningful text content. Closes #40727 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../compaction-safeguard.test.ts | 231 ++++++++++++++++++ .../pi-extensions/compaction-safeguard.ts | 74 +++++- 2 files changed, 302 insertions(+), 3 deletions(-) diff --git a/src/agents/pi-extensions/compaction-safeguard.test.ts b/src/agents/pi-extensions/compaction-safeguard.test.ts index 882099f3569..d7aee251a99 100644 --- a/src/agents/pi-extensions/compaction-safeguard.test.ts +++ b/src/agents/pi-extensions/compaction-safeguard.test.ts @@ -40,6 +40,9 @@ const { computeAdaptiveChunkRatio, isOversizedForSummary, readWorkspaceContextForSummary, + isRealConversationMessage, + hasMeaningfulText, + hasNearbyMeaningfulUserMessage, BASE_CHUNK_RATIO, MIN_CHUNK_RATIO, SAFETY_MARGIN, @@ -1583,3 +1586,231 @@ describe("readWorkspaceContextForSummary", () => { }, ); }); + +// --------------------------------------------------------------------------- +// isRealConversationMessage — content-aware checks (issue #40727) +// --------------------------------------------------------------------------- + +describe("isRealConversationMessage", () => { + it("accepts a user message with meaningful text", () => { + const msg = castAgentMessage({ role: "user", content: "Fix the bug", timestamp: 0 }); + expect(isRealConversationMessage(msg)).toBe(true); + }); + + it("accepts an assistant message with meaningful text", () => { + const msg = castAgentMessage({ + role: "assistant", + content: [{ type: "text", text: "Here is the fix" }], + timestamp: 0, + }); + expect(isRealConversationMessage(msg)).toBe(true); + }); + + it("rejects a user message with empty content", () => { + const msg = castAgentMessage({ role: "user", content: "", timestamp: 0 }); + expect(isRealConversationMessage(msg)).toBe(false); + }); + + it("rejects an assistant message that is only HEARTBEAT_OK", () => { + const msg = castAgentMessage({ + role: "assistant", + content: "HEARTBEAT_OK", + timestamp: 0, + }); + expect(isRealConversationMessage(msg)).toBe(false); + }); + + it("rejects an assistant message that is only NO_REPLY", () => { + const msg = castAgentMessage({ + role: "assistant", + content: "NO_REPLY", + timestamp: 0, + }); + expect(isRealConversationMessage(msg)).toBe(false); + }); + + it("rejects a user message with only whitespace", () => { + const msg = castAgentMessage({ role: "user", content: " \n ", timestamp: 0 }); + expect(isRealConversationMessage(msg)).toBe(false); + }); + + it("accepts an assistant message with HEARTBEAT_OK embedded in real text", () => { + const msg = castAgentMessage({ + role: "assistant", + content: "Status: HEARTBEAT_OK — also deployed v2", + timestamp: 0, + }); + expect(isRealConversationMessage(msg)).toBe(true); + }); + + it("rejects a system message", () => { + const msg = castAgentMessage({ role: "system", content: "You are helpful", timestamp: 0 }); + expect(isRealConversationMessage(msg)).toBe(false); + }); + + it("accepts a toolResult when no window is provided (conservative)", () => { + const msg = castAgentMessage({ + role: "toolResult", + toolCallId: "tc1", + toolName: "read_file", + content: "file contents", + isError: false, + timestamp: 0, + }); + expect(isRealConversationMessage(msg)).toBe(true); + }); + + it("accepts a toolResult with a nearby meaningful user message", () => { + const window: AgentMessage[] = [ + castAgentMessage({ role: "user", content: "Please read main.ts", timestamp: 0 }), + castAgentMessage({ + role: "assistant", + content: [{ type: "text", text: "Reading..." }], + timestamp: 0, + }), + castAgentMessage({ + role: "toolResult", + toolCallId: "tc1", + toolName: "read_file", + content: "file data", + isError: false, + timestamp: 0, + }), + ]; + expect(isRealConversationMessage(window[2], window, 2)).toBe(true); + }); + + it("rejects a toolResult when nearby user messages are heartbeat-only", () => { + const window: AgentMessage[] = [ + castAgentMessage({ role: "user", content: "", timestamp: 0 }), + castAgentMessage({ + role: "assistant", + content: "HEARTBEAT_OK", + timestamp: 0, + }), + castAgentMessage({ + role: "toolResult", + toolCallId: "tc1", + toolName: "heartbeat_check", + content: "ok", + isError: false, + timestamp: 0, + }), + ]; + expect(isRealConversationMessage(window[2], window, 2)).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// hasMeaningfulText +// --------------------------------------------------------------------------- + +describe("hasMeaningfulText", () => { + it("returns true for normal text", () => { + const msg = castAgentMessage({ role: "user", content: "Hello world", timestamp: 0 }); + expect(hasMeaningfulText(msg)).toBe(true); + }); + + it("returns false for empty string", () => { + const msg = castAgentMessage({ role: "user", content: "", timestamp: 0 }); + expect(hasMeaningfulText(msg)).toBe(false); + }); + + it("returns false for HEARTBEAT_OK only", () => { + const msg = castAgentMessage({ role: "assistant", content: "HEARTBEAT_OK", timestamp: 0 }); + expect(hasMeaningfulText(msg)).toBe(false); + }); + + it("returns false for NO_REPLY only", () => { + const msg = castAgentMessage({ role: "assistant", content: "NO_REPLY", timestamp: 0 }); + expect(hasMeaningfulText(msg)).toBe(false); + }); + + it("returns true when boilerplate is mixed with real text", () => { + const msg = castAgentMessage({ + role: "assistant", + content: "Done. NO_REPLY", + timestamp: 0, + }); + expect(hasMeaningfulText(msg)).toBe(true); + }); + + it("returns false for array content with no text blocks", () => { + const msg = castAgentMessage({ + role: "assistant", + content: [{ type: "image", source: "data" }], + timestamp: 0, + }); + expect(hasMeaningfulText(msg)).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// hasNearbyMeaningfulUserMessage +// --------------------------------------------------------------------------- + +describe("hasNearbyMeaningfulUserMessage", () => { + it("finds a meaningful user message within lookback window", () => { + const window: AgentMessage[] = [ + castAgentMessage({ role: "user", content: "Do something", timestamp: 0 }), + castAgentMessage({ role: "assistant", content: "ok", timestamp: 0 }), + castAgentMessage({ + role: "toolResult", + toolCallId: "tc1", + toolName: "x", + content: "y", + isError: false, + timestamp: 0, + }), + ]; + expect(hasNearbyMeaningfulUserMessage(window, 2)).toBe(true); + }); + + it("returns false when user messages are empty", () => { + const window: AgentMessage[] = [ + castAgentMessage({ role: "user", content: "", timestamp: 0 }), + castAgentMessage({ + role: "toolResult", + toolCallId: "tc1", + toolName: "x", + content: "y", + isError: false, + timestamp: 0, + }), + ]; + expect(hasNearbyMeaningfulUserMessage(window, 1)).toBe(false); + }); + + it("returns false when meaningful user message is beyond lookback", () => { + const window: AgentMessage[] = [ + castAgentMessage({ role: "user", content: "Real message", timestamp: 0 }), + // 6 filler messages (beyond lookback of 5) + ...Array.from({ length: 6 }, () => + castAgentMessage({ role: "assistant", content: "HEARTBEAT_OK", timestamp: 0 }), + ), + castAgentMessage({ + role: "toolResult", + toolCallId: "tc1", + toolName: "x", + content: "y", + isError: false, + timestamp: 0, + }), + ]; + expect(hasNearbyMeaningfulUserMessage(window, 7)).toBe(false); + }); + + it("returns false when index is 0 (no preceding messages)", () => { + const window: AgentMessage[] = [ + castAgentMessage({ + role: "toolResult", + toolCallId: "tc1", + toolName: "x", + content: "y", + isError: false, + timestamp: 0, + }), + ]; + expect(hasNearbyMeaningfulUserMessage(window, 0)).toBe(false); + }); +}); diff --git a/src/agents/pi-extensions/compaction-safeguard.ts b/src/agents/pi-extensions/compaction-safeguard.ts index 4461b97d3e0..540d17a3924 100644 --- a/src/agents/pi-extensions/compaction-safeguard.ts +++ b/src/agents/pi-extensions/compaction-safeguard.ts @@ -179,8 +179,72 @@ function formatToolFailuresSection(failures: ToolFailure[]): string { return `\n\n## Tool Failures\n${lines.join("\n")}`; } -function isRealConversationMessage(message: AgentMessage): boolean { - return message.role === "user" || message.role === "assistant" || message.role === "toolResult"; +/** Boilerplate tokens that do not represent real user/assistant conversation. */ +const BOILERPLATE_TOKENS = new Set(["HEARTBEAT_OK", "NO_REPLY"]); + +/** + * Returns `true` when `message` represents genuine conversation content that + * justifies keeping a compaction window. + * + * The old implementation only checked `role`, which let heartbeat polls + * (empty user content) and HEARTBEAT_OK / NO_REPLY assistant replies slip + * through — see issue #40727. + * + * Rules: + * • user / assistant — must contain meaningful text (non-empty after trimming + * boilerplate tokens). + * • toolResult — only counts when a nearby user message in the same window + * carries meaningful text (prevents heartbeat-only tool-use chains from + * being treated as real conversation). + */ +function isRealConversationMessage( + message: AgentMessage, + window?: AgentMessage[], + index?: number, +): boolean { + if (message.role === "user" || message.role === "assistant") { + return hasMeaningfulText(message); + } + if (message.role === "toolResult") { + // Without a surrounding window we cannot verify context — be conservative + // and accept the message. + if (!window || index === undefined) { + return true; + } + return hasNearbyMeaningfulUserMessage(window, index); + } + return false; +} + +/** Check whether a message carries non-boilerplate text content. */ +function hasMeaningfulText(message: AgentMessage): boolean { + const text = extractMessageText(message); + if (text.length === 0) { + return false; + } + // Strip known boilerplate tokens and see if anything remains. + let stripped = text; + for (const token of BOILERPLATE_TOKENS) { + stripped = stripped.replaceAll(token, ""); + } + return stripped.trim().length > 0; +} + +/** + * Scan backwards (up to 5 messages) from `index` looking for a user message + * that contains meaningful text. This lets tool-result messages inherit + * "realness" from the user turn that triggered them. + */ +function hasNearbyMeaningfulUserMessage(window: AgentMessage[], index: number): boolean { + const lookback = 5; + const start = Math.max(0, index - lookback); + for (let i = index - 1; i >= start; i--) { + const msg = window[i]; + if (msg.role === "user" && hasMeaningfulText(msg)) { + return true; + } + } + return false; } function computeFileLists(fileOps: FileOperations): { @@ -702,7 +766,8 @@ async function readWorkspaceContextForSummary(): Promise { export default function compactionSafeguardExtension(api: ExtensionAPI): void { api.on("session_before_compact", async (event, ctx) => { const { preparation, customInstructions: eventInstructions, signal } = event; - if (!preparation.messagesToSummarize.some(isRealConversationMessage)) { + const allMessages = [...preparation.turnPrefixMessages, ...preparation.messagesToSummarize]; + if (!allMessages.some((msg, idx, arr) => isRealConversationMessage(msg, arr, idx))) { log.warn( "Compaction safeguard: cancelling compaction with no real conversation messages to summarize.", ); @@ -1005,6 +1070,9 @@ export const __testing = { computeAdaptiveChunkRatio, isOversizedForSummary, readWorkspaceContextForSummary, + isRealConversationMessage, + hasMeaningfulText, + hasNearbyMeaningfulUserMessage, BASE_CHUNK_RATIO, MIN_CHUNK_RATIO, SAFETY_MARGIN,