From 0aa954118fda96fd1ccfd8627ed5a37968f254f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?luoye=E3=82=B9=E3=82=AD?= <100058663+luoye520ww@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:11:17 +0800 Subject: [PATCH] fix(compaction): avoid repeating retained user instructions --- kun/src/loop/compaction-summary.ts | 14 +++- kun/src/loop/context-compactor.test.ts | 14 +++- kun/src/loop/context-compactor.ts | 16 ++++- .../loop/history-compaction-service.test.ts | 64 ++++++++++++++++- kun/src/loop/history-compaction-service.ts | 68 ++++++++++++------- kun/src/services/turn-service.test.ts | 12 +++- kun/src/services/turn-service.ts | 66 +++++++++++------- kun/tests/loop.test.ts | 9 ++- .../src/store/chat-store-side-actions.test.ts | 30 ++++++++ .../src/store/chat-store-side-actions.ts | 21 ++++-- 10 files changed, 244 insertions(+), 70 deletions(-) diff --git a/kun/src/loop/compaction-summary.ts b/kun/src/loop/compaction-summary.ts index 40537968d..ee7886851 100644 --- a/kun/src/loop/compaction-summary.ts +++ b/kun/src/loop/compaction-summary.ts @@ -48,7 +48,10 @@ export const COMPACTION_SYSTEM_PROMPT = [ * write a self-contained handoff. Optional pinned constraints are appended so * durable rules survive even in the free-form summary text. */ -export function buildCompactionContinuationMessage(pinnedConstraints?: readonly string[]): string { +export function buildCompactionContinuationMessage( + pinnedConstraints?: readonly string[], + pinnedSkillPins?: readonly string[] +): string { const lines = [ 'Provide a detailed summary of our conversation above, written so a new session with no access to ' + 'this history can continue the work seamlessly. Cover what we set out to do, what has been done, ' + @@ -62,6 +65,12 @@ export function buildCompactionContinuationMessage(pinnedConstraints?: readonly lines.push('Durable constraints that MUST be preserved in your summary:') for (const pin of pins) lines.push(`- ${pin}`) } + const skills = (pinnedSkillPins ?? []).map((pin) => pin.trim()).filter((pin) => pin.length > 0) + if (skills.length > 0) { + lines.push('') + lines.push('Active skill pins that MUST be preserved in your summary:') + for (const skill of skills) lines.push(`- ${skill}`) + } return lines.join('\n') } @@ -171,6 +180,7 @@ export async function summarizeCompactionWithModel(input: { prefix: ImmutablePrefix contextCompaction?: ContextCompactionConfig items: TurnItem[] + pinnedSkillPins?: readonly string[] heuristicSummary: string signal: AbortSignal recordUsage?: (usage: UsageSnapshot) => Promise | void @@ -209,7 +219,7 @@ export async function summarizeCompactionWithModel(input: { createdAt: new Date().toISOString(), finishedAt: new Date().toISOString(), kind: 'user_message' as const, - text: buildCompactionContinuationMessage(input.prefix.pinnedConstraints) + text: buildCompactionContinuationMessage(input.prefix.pinnedConstraints, input.pinnedSkillPins) } let text = '' for await (const chunk of input.modelClient.stream({ diff --git a/kun/src/loop/context-compactor.test.ts b/kun/src/loop/context-compactor.test.ts index b93b638b3..7714e97cd 100644 --- a/kun/src/loop/context-compactor.test.ts +++ b/kun/src/loop/context-compactor.test.ts @@ -17,7 +17,7 @@ describe('ContextCompactor', () => { id: 'item_user_start', threadId, turnId, - text: 'Please keep the complete issue list when compacting.' + text: 'Repeat this instruction verbatim.' }), makeAssistantTextItem({ id: 'item_problem_list', @@ -39,7 +39,7 @@ describe('ContextCompactor', () => { id: 'item_recent_tail', threadId, turnId, - text: 'Recent tail request kept verbatim.' + text: 'Active Skill: retained-tail-only-skill\nRepeat this instruction verbatim.' }) ] @@ -60,5 +60,15 @@ describe('ContextCompactor', () => { expect(result.summaryItem.summary).toContain('Problem 42: preserve finding 42') expect(result.summaryItem.summary).toContain('Problem 80: preserve finding 80') expect(result.summaryItem.summary).not.toContain('middle item(s) omitted from this compact summary') + // The retained tail is sent verbatim after the summary. It must not be + // repeated inside the summary as well. A repeated instruction in a + // different turn is still preserved: the folded copy is summarized and + // the newest copy stays verbatim in the tail. + expect(result.summaryItem.summary).toContain('Repeat this instruction verbatim.') + expect(result.summaryItem.summary).not.toContain('Active Skill: retained-tail-only-skill') + expect(result.next.at(-1)).toMatchObject({ + id: 'item_recent_tail', + text: 'Active Skill: retained-tail-only-skill\nRepeat this instruction verbatim.' + }) }) }) diff --git a/kun/src/loop/context-compactor.ts b/kun/src/loop/context-compactor.ts index da9f56115..ad4d3c323 100644 --- a/kun/src/loop/context-compactor.ts +++ b/kun/src/loop/context-compactor.ts @@ -190,11 +190,20 @@ export class ContextCompactor { const replacedTokens = this.estimator.estimateItems(head) const sourceDigest = computeShortHash(compactedItemsDigestSource(head)) const digestMarker = createToolDigestMarker(sourceDigest) + // The tail is sent verbatim after this summary. Summarizing it as well + // duplicates the current user request (and can make the model treat one + // instruction as two). Keep the summary source explicitly limited to the + // folded head; the retained tail remains the single source of truth for + // recent instructions. const summaryBase = input.summaryOverride?.trim() || buildCompactionSummary({ - history, + history: head, head, tail, prefix: input.prefix, + // A skill pin in the retained tail is already sent verbatim with the + // request. Copy only folded pins into the summary so the tail has one + // source of truth, just like ordinary user instructions. + skillPins: extractSkillPins(head), reason: input.reason, mode: input.mode, budgetTokens: input.budgetTokens @@ -318,6 +327,7 @@ function buildCompactionSummary(input: { head: TurnItem[] tail: TurnItem[] prefix: ImmutablePrefix + skillPins?: readonly string[] reason?: string mode?: CompactionMode budgetTokens?: number @@ -341,7 +351,7 @@ function buildCompactionSummary(input: { lines.push(`- ${pinned}`) } } - const skillPins = extractSkillPins(input.history) + const skillPins = input.skillPins ?? extractSkillPins(input.history) if (skillPins.length > 0) { lines.push('Pinned skills (preserved across compaction):') for (const skillPin of skillPins) { @@ -377,7 +387,7 @@ function buildCompactionSummary(input: { return lines.join('\n') } -function extractSkillPins(history: TurnItem[]): string[] { +export function extractSkillPins(history: readonly TurnItem[]): string[] { const pins = new Set() for (const item of history) { if (item.kind !== 'assistant_text' && item.kind !== 'user_message' && item.kind !== 'compaction') continue diff --git a/kun/src/loop/history-compaction-service.test.ts b/kun/src/loop/history-compaction-service.test.ts index b36b61afb..64f4e24fc 100644 --- a/kun/src/loop/history-compaction-service.test.ts +++ b/kun/src/loop/history-compaction-service.test.ts @@ -52,11 +52,12 @@ async function seedLongHistory(sessionStore: InMemorySessionStore, prefix: strin function modelCompactionService( sessionStore: InMemorySessionStore, model: ModelClient, - contextCompaction: ContextCompactionConfig + contextCompaction: ContextCompactionConfig, + compactor = new ContextCompactor({ softThreshold: 1, hardThreshold: 2 }) ): HistoryCompactionService { return new HistoryCompactionService({ sessionStore, - compactor: new ContextCompactor({ softThreshold: 1, hardThreshold: 2 }), + compactor, prefix: createImmutablePrefix({ systemPrompt: 'stable prefix' }), model, usage: new UsageService(), @@ -195,7 +196,9 @@ describe('HistoryCompactionService', () => { id: `live_item_${index}`, threadId, turnId, - text: `live runtime config ${index} ${'x'.repeat(120)}` + text: index === 4 + ? `Active Skill: retained-auto-tail-only\nlive runtime config ${index} ${'x'.repeat(120)}` + : `live runtime config ${index} ${'x'.repeat(120)}` })) } const requests: ModelRequest[] = [] @@ -242,9 +245,64 @@ describe('HistoryCompactionService', () => { expect(seenPreCompact).toHaveBeenCalledWith(expect.objectContaining({ phase: 'PreCompact' })) expect(requests).toHaveLength(1) expect(requests[0]).toMatchObject({ model: 'live-summary-model' }) + const summaryUserMessages = requests[0]?.history + .filter((item) => item.kind === 'user_message') + .map((item) => item.text) ?? [] + expect(summaryUserMessages.some((text) => text.includes('live runtime config 0'))).toBe(true) + expect(summaryUserMessages.some((text) => text.includes('live runtime config 4'))).toBe(false) + const continuation = summaryUserMessages.at(-1) ?? '' + expect(continuation).toContain('Provide a detailed summary of our conversation above') + expect(continuation).not.toContain('Active Skill: retained-auto-tail-only') expect(history[0]).toMatchObject({ kind: 'compaction', summary: expect.stringContaining('summary from live config') }) }) + it('uses the heuristic summary when folded source ids are unavailable instead of sending the retained tail', async () => { + const sessionStore = new InMemorySessionStore() + await seedLongHistory(sessionStore, 'missing_source') + const requests: ModelRequest[] = [] + const model: ModelClient = { + provider: 'test', + model: 'summary-model', + async *stream(request) { + requests.push(request) + yield { kind: 'assistant_text_delta' as const, text: 'this must not be used' } + yield { kind: 'completed' as const, stopReason: 'stop' as const } + } + } + const compactor = new ContextCompactor({ softThreshold: 1, hardThreshold: 2 }) + const compact = compactor.compact.bind(compactor) + vi.spyOn(compactor, 'compact').mockImplementation((input) => { + const result = compact(input) + if (result.summaryItem.kind !== 'compaction') return result + return { + ...result, + summaryItem: { ...result.summaryItem, sourceItemIds: undefined } + } + }) + const service = modelCompactionService( + sessionStore, + model, + { summaryMode: 'model' }, + compactor + ) + + await service.compactIfNeeded({ + items: await sessionStore.loadItems(threadId), + model: 'summary-model', + signal: new AbortController().signal, + threadId, + turnId + }) + + expect(requests).toHaveLength(0) + const events = await sessionStore.loadEventsSince(threadId, 0) + expect(events).toContainEqual(expect.objectContaining({ + kind: 'error', + code: 'compaction_summary_fallback', + message: expect.stringContaining('no folded source items') + })) + }) + it('charges model-summary usage to the active Goal before the main-model budget recheck', async () => { const sessionStore = new InMemorySessionStore() await seedLongHistory(sessionStore, 'goal_budget') diff --git a/kun/src/loop/history-compaction-service.ts b/kun/src/loop/history-compaction-service.ts index 165f78133..7f92b0b21 100644 --- a/kun/src/loop/history-compaction-service.ts +++ b/kun/src/loop/history-compaction-service.ts @@ -22,6 +22,7 @@ import { recordLifecycleHookWarnings } from './turn-lifecycle-hooks.js' import type { ContextCompactionConfig } from './model-context-profile.js' import { estimateRequestOverheadTokens } from './model-request-estimator.js' import type { LoopTelemetry } from './loop-telemetry.js' +import { extractSkillPins } from './context-compactor.js' export type HistoryCompactionServiceDeps = { sessionStore: SessionStore @@ -171,31 +172,48 @@ export class HistoryCompactionService { : 'Model compaction summary skipped because its model-request budget is exhausted; using heuristic summary.' ) } else { - modelSummary = await summarizeCompactionWithModel({ - threadId: input.threadId, - turnId: input.turnId, - model: compactionModel.model, - ...(compactionModel.providerId ? { providerId: compactionModel.providerId } : {}), - ...(compactionModel.accountId ? { accountId: compactionModel.accountId } : {}), - modelClient: this.deps.model, - prefix: this.deps.prefix, - contextCompaction, - items: currentItems, - heuristicSummary: result.summaryItem.kind === 'compaction' ? result.summaryItem.summary : '', - signal: input.signal, - recordUsage: async (usageSnapshot) => { - const usage = this.deps.usage.record(input.threadId, usageSnapshot) - await this.deps.recordGoalUsage(input.threadId, usageSnapshot.totalTokens) - await this.deps.events.record({ - kind: 'usage', - threadId: input.threadId, - turnId: input.turnId, - model: compactionModel.model, - usage - }) - }, - recordFallback - }) + const foldedItemIds = new Set( + result.summaryItem.kind === 'compaction' + ? result.summaryItem.sourceItemIds ?? [] + : [] + ) + // The compaction summary is sent alongside the retained tail in + // the main request. Feed only the folded source items to the + // summarizer so the latest user instruction is not reproduced + // inside both the summary and the verbatim tail. + const summaryItems = currentItems.filter((item) => foldedItemIds.has(item.id)) + if (summaryItems.length === 0) { + await recordFallback( + 'Model compaction summary skipped because no folded source items were available; using heuristic summary.' + ) + } else { + modelSummary = await summarizeCompactionWithModel({ + threadId: input.threadId, + turnId: input.turnId, + model: compactionModel.model, + ...(compactionModel.providerId ? { providerId: compactionModel.providerId } : {}), + ...(compactionModel.accountId ? { accountId: compactionModel.accountId } : {}), + modelClient: this.deps.model, + prefix: this.deps.prefix, + contextCompaction, + items: summaryItems, + pinnedSkillPins: extractSkillPins(summaryItems), + heuristicSummary: result.summaryItem.kind === 'compaction' ? result.summaryItem.summary : '', + signal: input.signal, + recordUsage: async (usageSnapshot) => { + const usage = this.deps.usage.record(input.threadId, usageSnapshot) + await this.deps.recordGoalUsage(input.threadId, usageSnapshot.totalTokens) + await this.deps.events.record({ + kind: 'usage', + threadId: input.threadId, + turnId: input.turnId, + model: compactionModel.model, + usage + }) + }, + recordFallback + }) + } } } if (input.signal.aborted) { diff --git a/kun/src/services/turn-service.test.ts b/kun/src/services/turn-service.test.ts index 478a95c3c..cc2cd5049 100644 --- a/kun/src/services/turn-service.test.ts +++ b/kun/src/services/turn-service.test.ts @@ -544,7 +544,7 @@ describe('TurnService compact', () => { const items: TurnItem[] = [ makeUserItem({ id: 'item_1', threadId, turnId, text: 'Initial task: fix /compact.' }), makeAssistantTextItem({ id: 'item_2', threadId, turnId, text: 'I found the service path.', status: 'completed' }), - makeUserItem({ id: 'item_3', threadId, turnId, text: 'Please preserve this clue.' }), + makeUserItem({ id: 'item_3', threadId, turnId, text: 'Active Skill: retained-manual-tail-only\nPlease preserve this clue.' }), makeAssistantTextItem({ id: 'item_4', threadId, turnId, text: 'Recent tail A.', status: 'completed' }), makeUserItem({ id: 'item_5', threadId, turnId, text: 'Recent tail B.' }), makeAssistantTextItem({ id: 'item_6', threadId, turnId, text: 'Recent tail C.', status: 'completed' }) @@ -590,14 +590,20 @@ describe('TurnService compact', () => { expect(model.requests[0].systemPrompt).toBe(COMPACTION_SYSTEM_PROMPT) expect(model.requests[0].prefix).toEqual([]) const summaryHistory = model.requests[0].history - expect(summaryHistory[0]?.kind === 'user_message' ? summaryHistory[0].text : '') - .toContain('Initial task: fix /compact.') + const summaryUserMessages = summaryHistory + .filter((item) => item.kind === 'user_message') + .map((item) => item.text) + expect(summaryUserMessages[0]).toContain('Initial task: fix /compact.') + expect(summaryUserMessages).not.toContain('Active Skill: retained-manual-tail-only\nPlease preserve this clue.') + expect(summaryUserMessages).not.toContain('Recent tail B.') + expect(summaryUserMessages).not.toContain('Recent tail C.') const continuationItem = summaryHistory[summaryHistory.length - 1] expect(continuationItem?.kind).toBe('user_message') if (!continuationItem || continuationItem.kind !== 'user_message') { throw new Error('expected compaction continuation message to be a user message') } expect(continuationItem.text).toContain('Provide a detailed summary of our conversation above') + expect(continuationItem.text).not.toContain('Active Skill: retained-manual-tail-only') expect(response.summary).toContain('MODEL SUMMARY kept the durable state.') expect(response.pinnedConstraints).toEqual(prefix.pinnedConstraints) diff --git a/kun/src/services/turn-service.ts b/kun/src/services/turn-service.ts index 3c3363457..ad0032fb4 100644 --- a/kun/src/services/turn-service.ts +++ b/kun/src/services/turn-service.ts @@ -17,7 +17,7 @@ import type { ModelClient } from '../ports/model-client.js' import type { ImmutablePrefix } from '../cache/immutable-prefix.js' import type { InflightTracker } from '../loop/inflight-tracker.js' import type { SteeringQueue } from '../loop/steering-queue.js' -import { ContextCompactor } from '../loop/context-compactor.js' +import { ContextCompactor, extractSkillPins } from '../loop/context-compactor.js' import { effectiveHistoryAfterLatestCompaction, insertCompactionIntoVisibleHistory @@ -490,30 +490,46 @@ export class TurnService { `${reservation.reason} Model compaction summary was not sent; using heuristic summary.` ) } else { - modelSummary = await summarizeCompactionWithModel({ - threadId: input.threadId, - turnId, - model, - ...(compactionModel.providerId ? { providerId: compactionModel.providerId } : {}), - ...(compactionModel.accountId ? { accountId: compactionModel.accountId } : {}), - modelClient: this.deps.model, - prefix, - contextCompaction: this.deps.contextCompaction, - items: history, - heuristicSummary: result.summaryItem.kind === 'compaction' ? result.summaryItem.summary : '', - signal: input.signal ?? new AbortController().signal, - recordUsage: async (usageSnapshot) => { - const usage = this.deps.usage?.record(input.threadId, usageSnapshot) ?? usageSnapshot - await this.deps.events.record({ - kind: 'usage', - threadId: input.threadId, - turnId, - model, - usage - }) - }, - recordFallback - }) + const foldedItemIds = new Set( + result.summaryItem.kind === 'compaction' + ? result.summaryItem.sourceItemIds ?? [] + : [] + ) + // Keep the manual compaction summarizer aligned with the + // automatic path: recent tail items are sent verbatim after the + // summary and must not be summarized a second time. + const summaryItems = history.filter((item) => foldedItemIds.has(item.id)) + if (summaryItems.length === 0) { + await recordFallback( + 'Model compaction summary skipped because no folded source items were available; using heuristic summary.' + ) + } else { + modelSummary = await summarizeCompactionWithModel({ + threadId: input.threadId, + turnId, + model, + ...(compactionModel.providerId ? { providerId: compactionModel.providerId } : {}), + ...(compactionModel.accountId ? { accountId: compactionModel.accountId } : {}), + modelClient: this.deps.model, + prefix, + contextCompaction: this.deps.contextCompaction, + items: summaryItems, + pinnedSkillPins: extractSkillPins(summaryItems), + heuristicSummary: result.summaryItem.kind === 'compaction' ? result.summaryItem.summary : '', + signal: input.signal ?? new AbortController().signal, + recordUsage: async (usageSnapshot) => { + const usage = this.deps.usage?.record(input.threadId, usageSnapshot) ?? usageSnapshot + await this.deps.events.record({ + kind: 'usage', + threadId: input.threadId, + turnId, + model, + usage + }) + }, + recordFallback + }) + } } } if (modelSummary) { diff --git a/kun/tests/loop.test.ts b/kun/tests/loop.test.ts index 9c9b02ca3..b8933ded2 100644 --- a/kun/tests/loop.test.ts +++ b/kun/tests/loop.test.ts @@ -2634,7 +2634,7 @@ describe('AgentLoop', () => { }) }) - it('trims trailing tool calls and preserves skill pins in compaction summaries', () => { + it('trims trailing tool calls and retains tail skill pins verbatim', () => { const compactor = new ContextCompactor({ softThreshold: 1, hardThreshold: 2 }) const prefix = createImmutablePrefix({ systemPrompt: 'system' }) const result = compactor.compact({ @@ -2664,7 +2664,12 @@ describe('AgentLoop', () => { expect(result.next.some((item) => item.kind === 'tool_call')).toBe(false) expect(result.summaryItem.kind === 'compaction' ? result.summaryItem.summary : '') - .toContain('Active Skill: documents (documents)') + .not.toContain('Active Skill: documents (documents)') + expect(result.next.at(-1)).toMatchObject({ + kind: 'assistant_text', + id: 'a1', + text: 'Active Skill: documents (documents)' + }) }) it('keeps the latest user turn when force compaction would orphan a tool result', () => { diff --git a/src/renderer/src/store/chat-store-side-actions.test.ts b/src/renderer/src/store/chat-store-side-actions.test.ts index f324a1346..d555c1056 100644 --- a/src/renderer/src/store/chat-store-side-actions.test.ts +++ b/src/renderer/src/store/chat-store-side-actions.test.ts @@ -349,6 +349,36 @@ describe('chat-store-side-actions', () => { expect(state.busy).toBe(true) }) + it('deduplicates replayed compaction lifecycle events by item id', async () => { + const { actions, state, provider } = buildHarness() + const id = (await actions.spawnSideConversation())! + const sink = provider.subscribeMock.mock.calls.at(-1)?.[2] as ThreadEventSink + + sink.onCompaction({ + itemId: 'compaction_side_1', + summary: 'Compacting context', + status: 'running', + createdAt: '2026-06-02T00:00:00.000Z' + }) + sink.onCompaction({ + itemId: 'compaction_side_1', + summary: 'Compacted context', + status: 'success', + createdAt: '2026-06-02T00:00:01.000Z', + messagesBefore: 120 + }) + + const blocks = state.sideConversations[id].blocks.filter((block) => block.kind === 'compaction') + expect(blocks).toHaveLength(1) + expect(blocks[0]).toMatchObject({ + id: 'compaction_side_1', + status: 'success', + summary: 'Compacted context', + messagesBefore: 120, + createdAt: '2026-06-02T00:00:00.000Z' + }) + }) + it('updates approval resolution inside the matching side conversation', async () => { const { actions, state, provider } = buildHarness() const id = (await actions.spawnSideConversation())! diff --git a/src/renderer/src/store/chat-store-side-actions.ts b/src/renderer/src/store/chat-store-side-actions.ts index 571df3de0..3bdec055b 100644 --- a/src/renderer/src/store/chat-store-side-actions.ts +++ b/src/renderer/src/store/chat-store-side-actions.ts @@ -189,16 +189,27 @@ function buildSideSink(sideId: string, ctx: SideContext, sinceSeq = 0): ThreadEv ctx.set((s) => patchSide(s, sideId, (side) => { const flushed = flushSideLiveBlocks(side) + const index = flushed.blocks.findIndex( + (block) => block.kind === 'compaction' && block.id === ev.itemId + ) + const current = index >= 0 ? flushed.blocks[index] : undefined const block: CompactionBlock = { kind: 'compaction', id: ev.itemId, - createdAt: ev.createdAt ?? new Date().toISOString(), - summary: ev.summary, + createdAt: current?.kind === 'compaction' + ? current.createdAt + : ev.createdAt ?? new Date().toISOString(), + summary: ev.summary || (current?.kind === 'compaction' ? current.summary : ''), status: ev.status, - detail: ev.detail, - auto: ev.auto + detail: ev.detail ?? (current?.kind === 'compaction' ? current.detail : undefined), + auto: ev.auto ?? (current?.kind === 'compaction' ? current.auto : undefined), + messagesBefore: ev.messagesBefore ?? (current?.kind === 'compaction' ? current.messagesBefore : undefined), + messagesAfter: ev.messagesAfter ?? (current?.kind === 'compaction' ? current.messagesAfter : undefined) } - return { ...flushed.side, blocks: [...flushed.blocks, block] } + const blocks = [...flushed.blocks] + if (index >= 0) blocks[index] = block + else blocks.push(block) + return { ...flushed.side, blocks } }) ) },