diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 059389934..003e23100 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -29,6 +29,7 @@ ### Fixed +- Preserved superseded compaction summaries, snapcompact archives, and OpenAI replacement history so branching or rewinding before a newer compaction rebuilds the original model context ([#4090](https://github.com/can1357/oh-my-pi/issues/4090)). - Fixed discarded `Settings` instances keeping debounced save timers and chained background saves armed; discarding an instance now cancels its pending writes so they cannot race a successor's file locks. - Fixed `error.notify` raising a "Stopped with error" toast for provider failures while an auto-retry or async-delivery continuation was pending; the toast now waits for the true terminal settle. - Fixed terminal `yield` results racing post-turn maintenance, which could trigger an unnecessary automatic handoff or compaction. diff --git a/packages/coding-agent/src/session/session-context.ts b/packages/coding-agent/src/session/session-context.ts index abe1502e6..6d64d5f1e 100644 --- a/packages/coding-agent/src/session/session-context.ts +++ b/packages/coding-agent/src/session/session-context.ts @@ -18,6 +18,8 @@ import { type CompactionEntry, EPHEMERAL_MODEL_CHANGE_ROLE, type SessionEntry } const LEGACY_SNAPCOMPACT_FRAME_COUNT_GUARD = 16; const LEGACY_SNAPCOMPACT_ARCHIVE_TEXT_GUARD = 250_000; const LEGACY_SNAPCOMPACT_TRUNCATED_CHARS_GUARD = 1_000_000; +const SUPERSEDED_COMPACTION_SUMMARY = "[Superseded compaction summary elided after a newer compaction]"; +const SUPERSEDED_COMPACTION_SHORT_SUMMARY = "Superseded compaction elided"; function hasLegacySnapcompactFrames(archive: snapcompact.Archive): boolean { return archive.frames.some(frame => frame.font === undefined && frame.variant === undefined); @@ -344,13 +346,14 @@ export function buildSessionContext( for (const entry of path) { handleEntryResetTracking(entry); if (entry.type === "compaction") { - const snapcompactArchive = snapcompact.getPreservedArchive(entry.preserveData); + const active = entry.id === compaction?.id; + const snapcompactArchive = active ? snapcompact.getPreservedArchive(entry.preserveData) : undefined; pushMessage( createCompactionSummaryMessage( - entry.summary, + active ? entry.summary : SUPERSEDED_COMPACTION_SUMMARY, entry.tokensBefore, entry.timestamp, - entry.shortSummary, + active ? entry.shortSummary : SUPERSEDED_COMPACTION_SHORT_SUMMARY, undefined, undefined, snapcompactHistoryBlocksForContext(snapcompactArchive, options), diff --git a/packages/coding-agent/src/session/session-loader.ts b/packages/coding-agent/src/session/session-loader.ts index 05db3fdb5..3ddce2814 100644 --- a/packages/coding-agent/src/session/session-loader.ts +++ b/packages/coding-agent/src/session/session-loader.ts @@ -3,7 +3,6 @@ import { getBlobsDir, isEnoent, parseJsonlLenient } from "@oh-my-pi/pi-utils"; import { BlobStore, isBlobRef, resolveImageData, resolveImageDataUrl } from "./blob-store"; import { buildSessionContext } from "./session-context"; import { - type CompactionEntry, type FileEntry, type RawFileEntry, SESSION_TITLE_SLOT_BYTES, @@ -22,8 +21,6 @@ import { } from "./session-title-slot"; const STREAM_LOAD_THRESHOLD_BYTES = 8 * 1024 * 1024; -const ELIDED_COMPACTION_SUMMARY = "[Superseded compaction summary elided during session load]"; -const ELIDED_COMPACTION_SHORT_SUMMARY = "Superseded compaction elided"; function splitTitleSlot(content: string): { body: string; slot: SessionTitleUpdate | undefined } { const slot = titleUpdateFromSlot(parseTitleSlotFromContent(content)); @@ -59,48 +56,6 @@ export function parseSessionContent(content: string): { return { entries: foldTitleSlot(entries, slot), titleSlot: slot }; } -function elideCompactionSummary(entry: CompactionEntry | undefined): boolean { - if (!entry) return false; - if ( - entry.summary === ELIDED_COMPACTION_SUMMARY && - entry.shortSummary === ELIDED_COMPACTION_SHORT_SUMMARY && - entry.preserveData === undefined - ) { - return false; - } - entry.summary = ELIDED_COMPACTION_SUMMARY; - entry.shortSummary = ELIDED_COMPACTION_SHORT_SUMMARY; - entry.preserveData = undefined; - return true; -} - -function collectActiveBranchIds(entries: FileEntry[]): Set { - const byId = new Map(); - for (const entry of entries) { - const id = (entry as SessionEntry).id; - if (typeof id === "string") byId.set(id, entry as SessionEntry); - } - const branchIds = new Set(); - let cursor = entries[entries.length - 1] as SessionEntry | undefined; - while (cursor && typeof cursor.id === "string" && !branchIds.has(cursor.id)) { - branchIds.add(cursor.id); - const parentId = cursor.parentId; - cursor = parentId ? byId.get(parentId) : undefined; - } - return branchIds; -} - -function elideSupersededCompactionEntries(entries: FileEntry[]): void { - const branchIds = collectActiveBranchIds(entries); - let previousCompaction: CompactionEntry | undefined; - for (const entry of entries) { - if (entry.type !== "compaction") continue; - if (!branchIds.has(entry.id)) continue; - elideCompactionSummary(previousCompaction); - previousCompaction = entry; - } -} - /** Exported for testing — the ≥8MiB streaming path (works on any file size). */ export async function loadEntriesFromFileStream(filePath: string): Promise<{ entries: FileEntry[]; @@ -215,7 +170,6 @@ export async function loadEntriesFromFile( throw err; } const { entries } = loaded; - elideSupersededCompactionEntries(entries); // Validate session header if (entries.length === 0) return entries; diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 6bdc0f6bc..4c971bab7 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -81,8 +81,6 @@ import { const JSONL_SUFFIX_LENGTH = ".jsonl".length; const DRAFT_ONLY_SESSION_MARKER = ".draft-only-session"; -const SUPERSEDED_COMPACTION_SUMMARY = "[Superseded compaction summary elided after a newer compaction]"; -const SUPERSEDED_COMPACTION_SHORT_SUMMARY = "Superseded compaction elided"; function mintSessionId(): string { return Bun.randomUUIDv7(); @@ -747,26 +745,6 @@ export class SessionManager { return this.#forceFileCreation || this.#fileIsCurrent || this.#historyContainsAssistantMessage(); } - #elideSupersededCompactionsOnBranch(leafId: string | null): boolean { - if (!leafId) return false; - let changed = false; - for (const entry of this.#index.pathTo(leafId)) { - if (entry.type !== "compaction") continue; - if ( - entry.summary === SUPERSEDED_COMPACTION_SUMMARY && - entry.shortSummary === SUPERSEDED_COMPACTION_SHORT_SUMMARY && - entry.preserveData === undefined - ) { - continue; - } - entry.summary = SUPERSEDED_COMPACTION_SUMMARY; - entry.shortSummary = SUPERSEDED_COMPACTION_SHORT_SUMMARY; - entry.preserveData = undefined; - changed = true; - } - return changed; - } - /** * Synchronously rewrite the whole file (header + entries) and keep no open * writer; the next append re-opens one. `writeTextSync` returns with the @@ -1979,7 +1957,6 @@ export class SessionManager { fromExtension?: boolean, preserveData?: Record, ): string { - const elidedSupersededCompactions = this.#elideSupersededCompactionsOnBranch(this.#index.leafId()); const entry: CompactionEntry = { type: "compaction", ...this.#freshEntryFields(), @@ -1992,9 +1969,6 @@ export class SessionManager { preserveData, }; this.#recordEntry(entry); - if (elidedSupersededCompactions) { - void this.#rewriteAtomically().catch(err => this.#noteDiskFailure(err)); - } return entry.id; } diff --git a/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts b/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts index f31e9ad29..d07bd7c2e 100644 --- a/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts +++ b/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts @@ -28,8 +28,8 @@ import * as snapcompact from "@oh-my-pi/snapcompact"; * * The fix rebuilds the trailing archive locally via snapcompact.compact() at * a threshold-derived frame budget (planArchive truncates the oldest chars), - * persists it through appendCompaction (write-time elision drops the stale - * frame payload), and skips the misleading no-progress warning. + * persists it through appendCompaction, and skips the misleading no-progress + * warning. */ describe("AgentSession snapcompact frame dead-end rescue", () => { let tempDir: TempDir; @@ -287,13 +287,15 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { expect(compactOptions.maxFrames).toBeDefined(); expect(compactOptions.maxFrames as number).toBeLessThan(SEEDED_FRAME_COUNT); - // The rebuilt entry supersedes the stale one; write-time elision must - // have dropped the stale frame payload from the persisted branch. - const compactions = sessionManager.getBranch().filter(e => e.type === "compaction") as CompactionEntry[]; + // The rebuilt entry supersedes the stale one in active context without + // destroying the stale archive needed by a later rewind. + const compactions = sessionManager + .getBranch() + .filter((entry): entry is CompactionEntry => entry.type === "compaction"); expect(compactions.length).toBe(2); const [stale, rebuilt] = compactions; - expect(stale.summary).toContain("Superseded compaction summary elided"); - expect(stale.preserveData).toBeUndefined(); + expect(stale.summary).toBe("Archived history onto stale snapcompact frames."); + expect(snapcompact.getPreservedArchive(stale.preserveData)?.frames.length).toBe(SEEDED_FRAME_COUNT); const rebuiltArchive = snapcompact.getPreservedArchive(rebuilt.preserveData); expect(rebuiltArchive?.frames.length).toBe(4); @@ -341,11 +343,13 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { await triggerMaintenance(); expect(compactSpy).toHaveBeenCalledTimes(1); - const compactions = sessionManager.getBranch().filter(e => e.type === "compaction") as CompactionEntry[]; + const compactions = sessionManager + .getBranch() + .filter((entry): entry is CompactionEntry => entry.type === "compaction"); expect(compactions.length).toBe(2); const [hookWritten, rebuilt] = compactions; - expect(hookWritten.summary).toContain("Superseded compaction summary elided"); - expect(hookWritten.preserveData).toBeUndefined(); + expect(hookWritten.summary).toBe("compacted"); + expect(snapcompact.getPreservedArchive(hookWritten.preserveData)?.frames.length).toBe(SEEDED_FRAME_COUNT); expect(snapcompact.getPreservedArchive(rebuilt.preserveData)?.frames.length).toBe(4); // Extensions must be notified about the entry that is now active, not // only the hook-written one the rescue superseded. @@ -391,7 +395,9 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { expect(noProgress[0].level).toBe("warning"); // The dead-end badge must live on the ACTIVE (rebuilt) entry — the // collapsed transcript only shows the latest compaction divider. - const compactions = sessionManager.getBranch().filter(e => e.type === "compaction") as CompactionEntry[]; + const compactions = sessionManager + .getBranch() + .filter((entry): entry is CompactionEntry => entry.type === "compaction"); const active = compactions.at(-1); expect(snapcompact.getPreservedArchive(active?.preserveData)?.frames.length).toBe(4); expect(active?.warning).toContain(NO_PROGRESS_FRAGMENT); diff --git a/packages/coding-agent/test/session-manager/large-session-memory.test.ts b/packages/coding-agent/test/session-manager/large-session-memory.test.ts index 5b1cd4858..591187a2a 100644 --- a/packages/coding-agent/test/session-manager/large-session-memory.test.ts +++ b/packages/coding-agent/test/session-manager/large-session-memory.test.ts @@ -7,6 +7,7 @@ import { listSessions } from "@oh-my-pi/pi-coding-agent/session/session-listing" import { loadEntriesFromFile } from "@oh-my-pi/pi-coding-agent/session/session-loader"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { MemorySessionStorage } from "@oh-my-pi/pi-coding-agent/session/session-storage"; +import * as snapcompact from "@oh-my-pi/snapcompact"; class CountingMemorySessionStorage extends MemorySessionStorage { writeTextSyncCalls = 0; @@ -59,7 +60,7 @@ describe("large session memory guards", () => { expect(storage.writeTextSyncCalls).toBe(0); }); - it("elides superseded compactions and rewrites the compacted file", async () => { + it("elides superseded compactions only in the forward transcript", async () => { const storage = new CountingMemorySessionStorage(); const session = SessionManager.create("/work", "/sessions", storage); const firstKeptEntryId = session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); @@ -67,27 +68,66 @@ describe("large session memory guards", () => { const firstSummary = `first-${"x".repeat(4096)}`; const secondSummary = `second-${"y".repeat(4096)}`; - session.appendCompaction(firstSummary, undefined, firstKeptEntryId, 1000, undefined, undefined, { - openaiRemoteCompaction: { provider: "anthropic", replacementHistory: [] }, - }); - session.appendCompaction(secondSummary, undefined, firstKeptEntryId, 1000); + const archivedFrame = btoa("archived frame"); + const replacementHistory = [ + { type: "message", role: "user", content: [{ type: "input_text", text: "Preserved user" }] }, + ]; + const firstPreserve = { + openaiRemoteCompaction: { provider: "openai", replacementHistory }, + [snapcompact.PRESERVE_KEY]: { + frames: [{ data: archivedFrame, mimeType: "image/png", cols: 10, rows: 10, chars: 14 }], + totalChars: 14, + truncatedChars: 0, + textHead: "archived", + textTail: "frame", + }, + }; + const firstCompactionId = session.appendCompaction( + firstSummary, + undefined, + firstKeptEntryId, + 1000, + undefined, + undefined, + firstPreserve, + ); + const rewindId = session.appendMessage({ role: "user", content: "between compactions", timestamp: 3 }); + session.appendCompaction(secondSummary, undefined, rewindId, 2000); await session.flush(); - const compactions = session.getEntries().filter(entry => entry.type === "compaction"); - expect(compactions).toHaveLength(2); - expect(compactions[0]?.summary).not.toBe(firstSummary); - expect(compactions[0]?.summary).toContain("Superseded compaction"); - expect(compactions[0]?.preserveData).toBeUndefined(); - expect(compactions[1]?.summary).toBe(secondSummary); + const firstCompaction = session.getEntry(firstCompactionId); + if (firstCompaction?.type !== "compaction") throw new Error("Expected first compaction"); + expect(firstCompaction.summary).toBe(firstSummary); + expect(firstCompaction.preserveData).toEqual(firstPreserve); + + const transcriptCompactions = session + .buildSessionContext({ transcript: true }) + .messages.filter(message => message.role === "compactionSummary"); + const supersededDisplay = transcriptCompactions[0]; + if (supersededDisplay?.role !== "compactionSummary") throw new Error("Expected superseded transcript compaction"); + expect(supersededDisplay.summary).toContain("Superseded compaction"); + expect((supersededDisplay.blocks ?? []).some(block => block.type === "image")).toBeFalse(); + + session.branch(rewindId); + const rewoundSummary = session.buildSessionContext().messages[0]; + if (rewoundSummary?.role !== "compactionSummary") throw new Error("Expected rewound compaction summary"); + expect(rewoundSummary.summary).toBe(firstSummary); + expect(rewoundSummary.providerPayload).toEqual({ + type: "openaiResponsesHistory", + provider: "openai", + items: replacementHistory, + }); + expect(rewoundSummary.blocks?.find(block => block.type === "image")).toMatchObject({ data: archivedFrame }); const sessionFile = session.getSessionFile(); if (!sessionFile) throw new Error("Expected session file"); const persisted = await storage.readText(sessionFile); - expect(persisted).not.toContain(firstSummary); + expect(persisted).toContain(firstSummary); + expect(persisted).toContain(archivedFrame); expect(persisted).toContain(secondSummary); }); - it("streams large session files and keeps only the latest compaction summary", async () => { + it("streams large session files without discarding historical compactions", async () => { const tempDir = await fsp.mkdtemp(path.join(os.tmpdir(), "omp-large-session-")); tempDirs.push(tempDir); const sessionFile = path.join(tempDir, "large.jsonl"); @@ -135,9 +175,8 @@ describe("large session memory guards", () => { const compactions = entries.filter(entry => entry.type === "compaction"); expect(compactions).toHaveLength(2); - expect(compactions[0]?.summary).not.toBe(oldSummary); - expect(compactions[0]?.summary).toContain("Superseded compaction"); - expect(compactions[0]?.preserveData).toBeUndefined(); + expect(compactions[0]?.summary).toBe(oldSummary); + expect(compactions[0]?.preserveData).toEqual({ stale: true }); expect(compactions[1]?.summary).toBe(latestSummary); }); @@ -177,7 +216,7 @@ describe("large session memory guards", () => { expect(branchBCompactions).toHaveLength(1); }); - it("only elides loaded compactions on the active branch", async () => { + it("preserves loaded compactions on every branch", async () => { const tempDir = await fsp.mkdtemp(path.join(os.tmpdir(), "omp-branch-load-")); tempDirs.push(tempDir); const sessionFile = path.join(tempDir, "branched.jsonl"); @@ -243,8 +282,8 @@ describe("large session memory guards", () => { expect(branchA.summary).toBe(branchASummary); expect(branchA.preserveData).toBeDefined(); - expect(branchBOld.summary).toContain("Superseded compaction"); - expect(branchBOld.preserveData).toBeUndefined(); + expect(branchBOld.summary).toBe(branchBOldSummary); + expect(branchBOld.preserveData).toEqual({ stale: true }); expect(branchBNew.summary).toBe(branchBNewSummary); });