Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions devlog/2026-07-09_summary-role-assistant/REQ.md
Original file line number Diff line number Diff line change
@@ -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
37 changes: 37 additions & 0 deletions devlog/2026-07-09_summary-role-assistant/WORKLOG.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion lib/messages/inject/inject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/messages/inject/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
87 changes: 25 additions & 62 deletions lib/messages/prune.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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,
})
}
}

Expand All @@ -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
}

44 changes: 26 additions & 18 deletions tests/e2e-message-transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" &&
Expand All @@ -401,25 +399,24 @@ 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
.filter((p: any) => p.type === "text")
.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",
)
})

Expand Down Expand Up @@ -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) ──
Expand Down Expand Up @@ -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" &&
Expand All @@ -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)),
Expand Down
30 changes: 17 additions & 13 deletions tests/prune.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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[] = [
Expand All @@ -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", () => {
Expand Down Expand Up @@ -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("<dcp-message-id>"), "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")
})

// =====================================================================
Expand Down
Loading