fix(recovery): restore compaction pipeline sufficient check and conservative charsPerToken
Fixes critical v2.10.0 compaction regression where truncation ALWAYS returned early without checking if it was sufficient, causing PHASE 3 (Summarize) to be skipped. This led to "history disappears" symptom where all context was lost after compaction. Changes: - Restored aggressiveResult.sufficient check before early return in executor - Only return from Truncate phase if truncation successfully reduced tokens below limit - Otherwise fall through to Summarize phase when truncation is insufficient - Restored conservative charsPerToken=4 (was changed to 2, too aggressive) - Added 2 regression tests: * Test 1: Verify Summarize is called when truncation is insufficient * Test 2: Verify Summarize is skipped when truncation is sufficient Regression details: - v2.10.0 changed charsPerToken from 4 to 2, making truncation too aggressive - Early return removed sufficient check, skipping fallback to Summarize - Users reported complete loss of conversation history after compaction 🤖 Generated with assistance of OhMyOpenCode (https://github.com/code-yeongyu/oh-my-opencode)
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
import { describe, test, expect, mock, beforeEach } from "bun:test"
|
import { describe, test, expect, mock, beforeEach, spyOn } from "bun:test"
|
||||||
import { executeCompact } from "./executor"
|
import { executeCompact } from "./executor"
|
||||||
import type { AutoCompactState } from "./types"
|
import type { AutoCompactState } from "./types"
|
||||||
|
import * as storage from "./storage"
|
||||||
|
|
||||||
describe("executeCompact lock management", () => {
|
describe("executeCompact lock management", () => {
|
||||||
let autoCompactState: AutoCompactState
|
let autoCompactState: AutoCompactState
|
||||||
@@ -224,4 +225,86 @@ describe("executeCompact lock management", () => {
|
|||||||
// The continuation happens in setTimeout, but lock is cleared in finally before that
|
// The continuation happens in setTimeout, but lock is cleared in finally before that
|
||||||
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
|
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("falls through to summarize when truncation is insufficient", async () => {
|
||||||
|
// #given: Over token limit with truncation returning insufficient
|
||||||
|
autoCompactState.errorDataBySession.set(sessionID, {
|
||||||
|
errorType: "token_limit",
|
||||||
|
currentTokens: 250000,
|
||||||
|
maxTokens: 200000,
|
||||||
|
})
|
||||||
|
|
||||||
|
const truncateSpy = spyOn(storage, "truncateUntilTargetTokens").mockReturnValue({
|
||||||
|
success: true,
|
||||||
|
sufficient: false,
|
||||||
|
truncatedCount: 3,
|
||||||
|
totalBytesRemoved: 10000,
|
||||||
|
targetBytesToRemove: 50000,
|
||||||
|
truncatedTools: [
|
||||||
|
{ toolName: "Grep", originalSize: 5000 },
|
||||||
|
{ toolName: "Read", originalSize: 3000 },
|
||||||
|
{ toolName: "Bash", originalSize: 2000 },
|
||||||
|
],
|
||||||
|
})
|
||||||
|
|
||||||
|
// #when: Execute compaction
|
||||||
|
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
|
||||||
|
|
||||||
|
// #then: Truncation was attempted
|
||||||
|
expect(truncateSpy).toHaveBeenCalled()
|
||||||
|
|
||||||
|
// #then: Summarize should be called (fall through from insufficient truncation)
|
||||||
|
expect(mockClient.session.summarize).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
path: { id: sessionID },
|
||||||
|
body: { providerID: "anthropic", modelID: "claude-opus-4-5" },
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
|
||||||
|
// #then: Lock should be cleared
|
||||||
|
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
|
||||||
|
|
||||||
|
truncateSpy.mockRestore()
|
||||||
|
})
|
||||||
|
|
||||||
|
test("does NOT call summarize when truncation is sufficient", async () => {
|
||||||
|
// #given: Over token limit with truncation returning sufficient
|
||||||
|
autoCompactState.errorDataBySession.set(sessionID, {
|
||||||
|
errorType: "token_limit",
|
||||||
|
currentTokens: 250000,
|
||||||
|
maxTokens: 200000,
|
||||||
|
})
|
||||||
|
|
||||||
|
const truncateSpy = spyOn(storage, "truncateUntilTargetTokens").mockReturnValue({
|
||||||
|
success: true,
|
||||||
|
sufficient: true,
|
||||||
|
truncatedCount: 5,
|
||||||
|
totalBytesRemoved: 60000,
|
||||||
|
targetBytesToRemove: 50000,
|
||||||
|
truncatedTools: [
|
||||||
|
{ toolName: "Grep", originalSize: 30000 },
|
||||||
|
{ toolName: "Read", originalSize: 30000 },
|
||||||
|
],
|
||||||
|
})
|
||||||
|
|
||||||
|
// #when: Execute compaction
|
||||||
|
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
|
||||||
|
|
||||||
|
// Wait for setTimeout callback
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 600))
|
||||||
|
|
||||||
|
// #then: Truncation was attempted
|
||||||
|
expect(truncateSpy).toHaveBeenCalled()
|
||||||
|
|
||||||
|
// #then: Summarize should NOT be called (early return from sufficient truncation)
|
||||||
|
expect(mockClient.session.summarize).not.toHaveBeenCalled()
|
||||||
|
|
||||||
|
// #then: prompt_async should be called (Continue after successful truncation)
|
||||||
|
expect(mockClient.session.prompt_async).toHaveBeenCalled()
|
||||||
|
|
||||||
|
// #then: Lock should be cleared
|
||||||
|
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
|
||||||
|
|
||||||
|
truncateSpy.mockRestore()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -401,6 +401,9 @@ export async function executeCompact(
|
|||||||
|
|
||||||
log("[auto-compact] aggressive truncation completed", aggressiveResult);
|
log("[auto-compact] aggressive truncation completed", aggressiveResult);
|
||||||
|
|
||||||
|
// Only return early if truncation was sufficient to get under token limit
|
||||||
|
// Otherwise fall through to PHASE 3 (Summarize)
|
||||||
|
if (aggressiveResult.sufficient) {
|
||||||
clearSessionState(autoCompactState, sessionID);
|
clearSessionState(autoCompactState, sessionID);
|
||||||
setTimeout(async () => {
|
setTimeout(async () => {
|
||||||
try {
|
try {
|
||||||
@@ -413,9 +416,16 @@ export async function executeCompact(
|
|||||||
}, 500);
|
}, 500);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
// Truncation was insufficient - fall through to Summarize
|
||||||
|
log("[auto-compact] truncation insufficient, falling through to summarize", {
|
||||||
|
sessionID,
|
||||||
|
truncatedCount: aggressiveResult.truncatedCount,
|
||||||
|
sufficient: aggressiveResult.sufficient,
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// PHASE 3: Summarize - fallback when no tool outputs to truncate
|
// PHASE 3: Summarize - fallback when truncation insufficient or no tool outputs
|
||||||
const retryState = getOrCreateRetryState(autoCompactState, sessionID);
|
const retryState = getOrCreateRetryState(autoCompactState, sessionID);
|
||||||
|
|
||||||
if (errorData?.errorType?.includes("non-empty content")) {
|
if (errorData?.errorType?.includes("non-empty content")) {
|
||||||
|
|||||||
@@ -44,5 +44,5 @@ export const TRUNCATE_CONFIG = {
|
|||||||
maxTruncateAttempts: 20,
|
maxTruncateAttempts: 20,
|
||||||
minOutputSizeToTruncate: 500,
|
minOutputSizeToTruncate: 500,
|
||||||
targetTokenRatio: 0.5,
|
targetTokenRatio: 0.5,
|
||||||
charsPerToken: 2,
|
charsPerToken: 4,
|
||||||
} as const
|
} as const
|
||||||
|
|||||||
Reference in New Issue
Block a user