From d971b6a77f7cff44210ec3885cba6f0f15f1cb63 Mon Sep 17 00:00:00 2001 From: ranxianglei Date: Thu, 9 Jul 2026 01:47:46 +0800 Subject: [PATCH 1/2] fix: compression summaries always role:assistant (Bug 39) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, compression summaries were merged into the following user message when one existed (Bug 36 merge path), giving them role:'user'. This caused models to misattribute their own prior compression recaps as user instructions. Bug 37 already solved the original reason for the merge path (AssistantMessage field fabrication). This commit removes the merge path entirely — all summaries are now standalone role:'assistant'. Changes: - prune.ts: Remove dual-path logic, always use standalone assistant - prune.ts: Remove dead findNextSurvivingMessage function - tests: Update 6 tests across prune.test.ts + e2e-message-transform.test.ts issue #13 --- .../2026-07-09_summary-role-assistant/REQ.md | 48 ++++++++++ .../WORKLOG.md | 37 ++++++++ lib/messages/prune.ts | 87 ++++++------------- tests/e2e-message-transform.test.ts | 44 ++++++---- tests/prune.test.ts | 30 ++++--- 5 files changed, 153 insertions(+), 93 deletions(-) create mode 100644 devlog/2026-07-09_summary-role-assistant/REQ.md create mode 100644 devlog/2026-07-09_summary-role-assistant/WORKLOG.md diff --git a/devlog/2026-07-09_summary-role-assistant/REQ.md b/devlog/2026-07-09_summary-role-assistant/REQ.md new file mode 100644 index 0000000..3e90524 --- /dev/null +++ b/devlog/2026-07-09_summary-role-assistant/REQ.md @@ -0,0 +1,48 @@ +# REQ: Summary Role — Always Assistant + +## Problem + +Compression summaries were sometimes injected as `role: "user"` when the next +surviving message after a pruned range was a user turn. The model could then +misattribute its own prior compression recap as a user instruction. + +This was the Bug 36 "merge into user" path, which merged the summary text into +the following user message's text part, giving it `role: "user"`. + +## Root Cause + +`filterCompressedRanges()` in `lib/messages/prune.ts` had two paths: + +1. **Merge path** (Bug 36 fix): If the next surviving message was `role: "user"`, + `prependCompressionSummary()` merged the summary into that user message. + Result: model sees summary text with `role: "user"`. + +2. **Standalone path** (Bug 37 fix): Otherwise, `createSyntheticMessage(..., + "assistant")` created a standalone assistant message. Result: model sees + summary text with `role: "assistant"`. + +Bug 36 originally rejected the standalone path because `AssistantMessage` +required fields that a synthetic message lacked. Bug 37 solved this by teaching +`createSyntheticMessage` to fabricate safe defaults. The merge path was therefore +no longer necessary but remained as a source of role confusion. + +## Solution + +Remove the merge path entirely. All compression summaries are now standalone +`role: "assistant"` synthetic messages, regardless of what follows. + +## Scope + +- `lib/messages/prune.ts`: Remove dual-path logic, always use standalone assistant +- `lib/messages/utils.ts`: Dead code cleanup deferred (prependCompressionSummary, + MERGED_SUMMARY_HEADER/FOOTER remain as unused exports — harmless, cleanup TBD) +- `tests/prune.test.ts`: 3 tests updated for standalone assertions +- `tests/e2e-message-transform.test.ts`: 3 tests updated for standalone assertions + +## Acceptance Criteria + +1. All compression summaries emitted as `role: "assistant"` +2. No summary text appears in surviving user messages +3. No consecutive user turns introduced +4. All tests pass +5. Bug 28 (stale ref stripping) still works on standalone summaries diff --git a/devlog/2026-07-09_summary-role-assistant/WORKLOG.md b/devlog/2026-07-09_summary-role-assistant/WORKLOG.md new file mode 100644 index 0000000..3664215 --- /dev/null +++ b/devlog/2026-07-09_summary-role-assistant/WORKLOG.md @@ -0,0 +1,37 @@ +# WORKLOG: Summary Role — Always Assistant + +## 2026-07-09 + +### Investigation +- Read `lib/messages/prune.ts` L201-300: identified two injection paths +- Path 1 (merge into user): `findNextSurvivingMessage()` + `prependCompressionSummary()` +- Path 2 (standalone assistant): `createSyntheticMessage(..., "assistant")` +- Reviewed Bug 36 DESIGN.md rejection rationale: AssistantMessage field requirements +- Verified Bug 37 already solved this: `createSyntheticMessage` fabricates safe defaults + +### Implementation +- `prune.ts`: + - Removed `prependCompressionSummary` from import + - Replaced L250-291 dual-path logic with always-assistant standalone path + - Added [FIX Bug 39] comment block explaining the change + - Removed dead `findNextSurvivingMessage` function (L310-328) +- `utils.ts`: + - Attempted to remove `MERGED_SUMMARY_HEADER/FOOTER` + `prependCompressionSummary` + - Edit tool corrupted `DCP_BLOCK_ID_TAG_REGEX` bytes → reverted utils.ts to HEAD + - Dead code remains as unused exports — typecheck passes, cleanup deferred + +### Test Updates +- `prune.test.ts`: + - L163: summary search changed from `role: "user"` to `role: "assistant"` + - L167-210: Renamed to "prune always emits standalone assistant summary (Bug 39 fix)" + - L292-327: Bug 28 test updated to check standalone assistant msg, not m1 +- `e2e-message-transform.test.ts`: + - L387-424: standalone assistant EXISTS, u2 does NOT contain summary + - L428-516: u2 should NOT contain recap, standalone assistant should exist + - L520-607: Updated comments + role assertion from user to assistant + +### Verification +- typecheck: PASS +- build: PASS (354.25 KB) +- tests: 587 pass, 1 fail (pre-existing Bun quirk in prompts.test.ts) +- Reverted other agent's (issue #14) incomplete test change in protected-tool-exclusion.test.ts diff --git a/lib/messages/prune.ts b/lib/messages/prune.ts index c553a4c..14432d7 100644 --- a/lib/messages/prune.ts +++ b/lib/messages/prune.ts @@ -2,7 +2,7 @@ import type { SessionState, WithParts } from "../state" import type { Logger } from "../logger" import type { PluginConfig } from "../config" import { isMessageCompacted } from "../state/utils" -import { createSyntheticMessage, prependCompressionSummary, replaceBlockIdsWithBlocked, stripStaleMessageRefs } from "./utils" +import { createSyntheticMessage, replaceBlockIdsWithBlocked, stripStaleMessageRefs } from "./utils" import { getLastUserMessage } from "./query" const PRUNED_TOOL_OUTPUT_REPLACEMENT = @@ -247,48 +247,30 @@ const filterCompressedRanges = ( ? replaceBlockIdsWithBlocked(_cleaned) : _cleaned - // [FIX Bug 36] When the next surviving message is a user turn, merge the - // summary into it instead of emitting a standalone user-role summary - // message. The old behavior placed a synthetic user message immediately - // before the user's real turn ([summary(user), user(user)]), which the - // model often read as two user turns — misattributing the assistant's - // prior output to the user and triggering "self-Q&A" loops. Merging - // yields a single user turn ([user: recap ‖ real reply]) so no fake - // conversational turn is perceived. - const nextSurviving = findNextSurvivingMessage(messages, i, state) + // [FIX Bug 39] Always emit compression summaries as standalone + // role: "assistant" messages. Previously, when the next surviving + // message was a user turn, the summary was merged INTO that user + // message (role: "user"), causing the model to treat its own prior + // compression recap as user content. Bug 37 already established that + // createSyntheticMessage("assistant") safely fabricates the required + // AssistantMessage fields, removing the original Bug 36 constraint. const blockRange = computeBlockRange(summary.startId, summary.endId) - const merged = - nextSurviving !== null && - nextSurviving.info.role === "user" && - prependCompressionSummary(nextSurviving, summaryContent, summary.blockId, blockRange) - - if (merged) { - logger.info("Merged compress summary into following user message", { - anchorMessageId: msgId, - targetMessageId: nextSurviving!.info.id, - summaryLength: summaryContent.length, - }) - } else { - // [FIX Bug 37] Emit standalone summary as role: "assistant" with - // system-metadata tags so the model cannot misattribute its own - // prior compression recap as a user instruction. - const taggedContent = - STANDALONE_SUMMARY_HEADER(summary.blockId, blockRange) + - summaryContent + - STANDALONE_SUMMARY_FOOTER - const summarySeed = `${summary.blockId}:${summary.anchorMessageId}` - const userMessage = getLastUserMessage(messages, i) - const baseForSummary = userMessage ?? msg - result.push( - createSyntheticMessage(baseForSummary, taggedContent, summarySeed, "assistant"), - ) - - logger.info("Injected compress summary as assistant role", { - anchorMessageId: msgId, - summaryLength: taggedContent.length, - hadUserBase: userMessage !== null, - }) - } + const taggedContent = + STANDALONE_SUMMARY_HEADER(summary.blockId, blockRange) + + summaryContent + + STANDALONE_SUMMARY_FOOTER + const summarySeed = `${summary.blockId}:${summary.anchorMessageId}` + const userMessage = getLastUserMessage(messages, i) + const baseForSummary = userMessage ?? msg + result.push( + createSyntheticMessage(baseForSummary, taggedContent, summarySeed, "assistant"), + ) + + logger.info("Injected compress summary as assistant role", { + anchorMessageId: msgId, + summaryLength: taggedContent.length, + hadUserBase: userMessage !== null, + }) } } @@ -307,23 +289,4 @@ const filterCompressedRanges = ( messages.push(...result) } -// [FIX Bug 36] First surviving (non-pruned) message at or after startIndex. -// Starts the scan at startIndex inclusive to handle both anchor layouts: when -// the anchor is itself part of the pruned range (message-start ranges) it is -// skipped, and when the anchor survives (block-anchor ranges) it is returned — -// in either case this yields the next real turn the model sees after the recap. -const findNextSurvivingMessage = ( - messages: WithParts[], - startIndex: number, - state: SessionState, -): WithParts | null => { - for (let j = startIndex; j < messages.length; j++) { - const candidate = messages[j]! - const entry = state.prune.messages.byMessageId.get(candidate.info.id) - if (entry && entry.activeBlockIds.length > 0) { - continue - } - return candidate - } - return null -} + diff --git a/tests/e2e-message-transform.test.ts b/tests/e2e-message-transform.test.ts index e29365c..7bfc27c 100644 --- a/tests/e2e-message-transform.test.ts +++ b/tests/e2e-message-transform.test.ts @@ -384,15 +384,13 @@ test("compression blocks: compressed messages are replaced with summaries", asyn assert.ok(remainingIds.includes("u2"), "u2 should survive") assert.ok(remainingIds.includes("a2"), "a2 should survive") - // [FIX Bug 36] The summary is now merged into the following user message (u2) - // instead of a standalone synthetic message, so the model no longer sees two - // consecutive user turns ([summary(user), u2(user)]) which caused role - // confusion / self-Q&A loops. - // The recap content is checked (not the msg_dcp_summary_ id prefix) because - // the unrelated suffix-guidance nudge message reuses that same id prefix. + // [FIX Bug 39] The summary is now ALWAYS a standalone role: "assistant" + // synthetic message, regardless of what follows. Previously (Bug 36) it + // was merged into the following user message (u2), causing role confusion. const standaloneWithRecap = output.messages.find( (m: any) => m.info.id?.startsWith("msg_dcp_summary_") && + m.info.role === "assistant" && m.parts.some( (p: any) => p.type === "text" && @@ -401,12 +399,11 @@ test("compression blocks: compressed messages are replaced with summaries", asyn ), ) assert.ok( - !standaloneWithRecap, - "recap should be merged into the following user message, not emitted as a standalone synthetic message", + standaloneWithRecap, + "recap should be a standalone assistant synthetic message (Bug 39)", ) - // The summary content should be prepended into u2 (the following user message), - // and u2's original text must still be present after the recap. + // The summary content should NOT be in u2 — u2 keeps only its own text. const u2Msg = output.messages.find((m: any) => m.info.id === "u2") assert.ok(u2Msg, "u2 should survive") const u2Text = u2Msg!.parts @@ -414,12 +411,12 @@ test("compression blocks: compressed messages are replaced with summaries", asyn .map((p: any) => p.text) .join("") assert.ok( - u2Text.includes("Previous conversation about greetings"), - "compressed summary should be merged into u2", + !u2Text.includes("Previous conversation about greetings"), + "u2 should NOT contain the compressed summary (Bug 39)", ) assert.ok( u2Text.includes("How are you?"), - "u2's original text should be preserved after the summary is prepended", + "u2's original text should be preserved", ) }) @@ -504,15 +501,25 @@ test("compression summary: never produces two consecutive user turns (Bug 36)", ) } - // The surviving user turn must carry both the recap and its own original text. + // The surviving user turn must NOT carry the recap (Bug 39: always standalone). const u2 = historical.find((m: WithParts) => m.info.id === "u2") assert.ok(u2, "u2 should survive") const u2Text = u2!.parts .filter((p) => p.type === "text") .map((p) => (p as any).text) .join("") - assert.ok(u2Text.includes("The assistant explained the plan"), "recap merged into u2") + assert.ok(!u2Text.includes("The assistant explained the plan"), "recap should NOT be merged into u2 (Bug 39)") assert.ok(u2Text.includes("Sounds good, continue."), "u2 original text preserved") + + // The recap must exist as a standalone assistant message. + const recapMsg = historical.find( + (m: WithParts) => + m.info.role === "assistant" && + m.parts.some( + (p) => p.type === "text" && typeof (p as any).text === "string" && (p as any).text.includes("The assistant explained the plan"), + ), + ) + assert.ok(recapMsg, "recap should be a standalone assistant message (Bug 39)") }) // ─── Test: Fallback — standalone summary when no following user turn (Bug 36) ── @@ -577,11 +584,12 @@ test("compression summary: emits standalone summary when range is last (no user assert.ok(!remainingIds.includes("u2"), "u2 (covered by block) should be pruned") assert.ok(!remainingIds.includes("a2"), "a2 (covered by block) should be pruned") - // No following user turn exists to merge into, so the recap must be emitted - // as a standalone synthetic user message carrying the summary content. + // [Bug 39] All summaries are standalone assistant messages. This test + // verifies the standalone path when the pruned range is at the end. const standalone = output.messages.find( (m: any) => m.info.id?.startsWith("msg_dcp_summary_") && + m.info.role === "assistant" && m.parts.some( (p: any) => p.type === "text" && @@ -591,7 +599,7 @@ test("compression summary: emits standalone summary when range is last (no user ) assert.ok(standalone, "fallback path must emit a standalone synthetic summary") - // The standalone summary (user) follows a1 (assistant), so alternation holds. + // The standalone summary (assistant) follows a1 (assistant), so no role confusion. const lastIdx = output.messages.length - 1 const checkMessages = output.messages.filter( (m: any, idx: number) => !(idx === lastIdx && isSyntheticMessage(m)), diff --git a/tests/prune.test.ts b/tests/prune.test.ts index f7144a0..c5c0e5a 100644 --- a/tests/prune.test.ts +++ b/tests/prune.test.ts @@ -160,11 +160,11 @@ test("prune removes messages in active compression ranges", () => { assert.ok(!ids.includes("m3"), "m3 should be pruned") assert.ok(ids.includes("m1"), "anchor m1 should survive") assert.ok(ids.includes("m4"), "m4 should survive") - const summaryMsg = messages.find((m) => m.info.role === "user" && m.parts.some((p: any) => p.type === "text" && p.text?.includes("Summary of m2-m3"))) + const summaryMsg = messages.find((m) => m.info.role === "assistant" && m.parts.some((p: any) => p.type === "text" && p.text?.includes("Summary of m2-m3"))) assert.ok(summaryMsg, "summary should be injected") }) -test("prune merges summary into surviving user message at anchor (Bug 36 fix)", () => { +test("prune always emits standalone assistant summary (Bug 39 fix)", () => { const state = createSessionState() state.prune.messages.byMessageId.set("m2", { tokenCount: 100, allBlockIds: [1], activeBlockIds: [1] }) state.prune.messages.activeByAnchorMessageId.set("m1", 1) @@ -183,8 +183,8 @@ test("prune merges summary into surviving user message at anchor (Bug 36 fix)", directToolIds: [], effectiveToolIds: [], anchorMessageId: "m1", - topic: "merged topic", - summary: "Merged summary text", + topic: "test topic", + summary: "Standalone summary text", } as CompressionBlock) const messages: WithParts[] = [ @@ -201,12 +201,13 @@ test("prune merges summary into surviving user message at anchor (Bug 36 fix)", const m1 = messages.find((m) => m.info.id === "m1") assert.ok(m1, "m1 (anchor) should survive") const m1Text = m1!.parts.map((p: any) => p.text ?? "").join("") - assert.ok(m1Text.includes("Merged summary text"), "summary should be merged into anchor m1") + assert.ok(!m1Text.includes("Standalone summary text"), "summary should NOT be merged into m1") assert.ok(m1Text.includes("first question"), "original m1 text should be preserved") - const syntheticCount = messages.filter( - (m) => m.info.role === "user" && m.parts.some((p: any) => p.type === "text" && p.text?.includes("Merged summary text") && m.info.id !== "m1"), - ).length - assert.equal(syntheticCount, 0, "should not create standalone summary when merged into anchor") + + const summaryMsg = messages.find( + (m) => m.info.role === "assistant" && m.parts.some((p: any) => p.type === "text" && p.text?.includes("Standalone summary text")), + ) + assert.ok(summaryMsg, "should create standalone assistant summary message") }) test("prune creates standalone summary when anchor is assistant and no user follows", () => { @@ -319,10 +320,13 @@ test("prune strips stale mNNNN refs from summary content (Bug 28 fix)", () => { prune(state, logger, buildConfig(), messages) - const m1 = messages.find((m) => m.info.id === "m1") - const m1Text = m1!.parts.map((p: any) => p.text ?? "").join("") - assert.ok(!m1Text.includes(""), "stale dcp-message-id tag should be stripped") - assert.ok(m1Text.includes("Summary with"), "summary content should remain after stripping") + const summaryMsg = messages.find( + (m) => m.info.role === "assistant" && m.parts.some((p: any) => p.type === "text" && p.text?.includes("Summary with")), + ) + assert.ok(summaryMsg, "standalone assistant summary should exist") + const summaryText = summaryMsg!.parts.map((p: any) => p.text ?? "").join("") + assert.ok(!summaryText.includes("dcp-message-id"), "stale dcp-message-id tag should be stripped from summary") + assert.ok(summaryText.includes("Summary with"), "summary content should remain after stripping") }) // ===================================================================== From ea7db98beb9443359b1d18037891994de0f5090a Mon Sep 17 00:00:00 2001 From: ranxianglei Date: Thu, 9 Jul 2026 02:35:32 +0800 Subject: [PATCH 2/2] feat: increase max tool compression candidates from 5 to 15 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The breakdown and tool-output reminder only showed 5 candidates, causing the model to compress too narrowly (1 message per batch). Increasing to 15 gives the model more compression candidates at once. Changed: - utils.ts: largestRanges 10→15, largestToolRanges 5→15 - inject.ts: toolOutputReminder topRanges 5→15 issue #13 --- lib/messages/inject/inject.ts | 2 +- lib/messages/inject/utils.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/messages/inject/inject.ts b/lib/messages/inject/inject.ts index 67a83ab..75997c9 100644 --- a/lib/messages/inject/inject.ts +++ b/lib/messages/inject/inject.ts @@ -211,7 +211,7 @@ export const injectCompressNudges = ( const toolGrowth = composition.toolTokens - state.nudges.lastToolOutputNudgeTokens if (toolGrowth >= toolOutputThreshold) { const fmt = (n: number) => (n >= 1000 ? `${(n / 1000).toFixed(1)}K` : String(n)) - const topRanges = composition.largestRanges.slice(0, 5).map((r) => `${r.ref} (${fmt(r.tokens)})`).join(", ") + const topRanges = composition.largestRanges.slice(0, 15).map((r) => `${r.ref} (${fmt(r.tokens)})`).join(", ") toolOutputReminder = `\n\n⚠️ ${fmt(toolGrowth)} new tool outputs accumulated (${fmt(composition.toolTokens)} total). Largest: ${topRanges}. Use compress tool to compress these ranges now.` state.nudges.lastToolOutputNudgeTokens = composition.toolTokens anchorsChanged = true diff --git a/lib/messages/inject/utils.ts b/lib/messages/inject/utils.ts index c8a5a5e..d28b124 100644 --- a/lib/messages/inject/utils.ts +++ b/lib/messages/inject/utils.ts @@ -649,8 +649,8 @@ export function estimateContextComposition( messageTokens, textTokens: Math.max(0, messageTokens - codeTokens), total: toolTokens + summaryTokens + messageTokens, - largestRanges: perMessage.slice(0, 10), - largestToolRanges: perTool.slice(0, 5), + largestRanges: perMessage.slice(0, 15), + largestToolRanges: perTool.slice(0, 15), largestCodeRanges: perCode.slice(0, 5), largestMessageRanges: perText.slice(0, 5), }