smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(session): preserved compaction data for rewinds

Browse Source 
Kept superseded summaries and preserveData durable while deriving forward transcript elision from the active compaction.

Added branch and rewind coverage for snapcompact archives and OpenAI remote replacement history.

Fixes #4090
This commit is contained in:
roboomp
2026-07-23 19:53:05 +00:00
parent fcf2274b2f
commit cb63ebd9f5
6 changed files with 82 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1
View File
@@ -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.
packages/coding-agent/src/session/session-context.ts
+6 -3
View File
@@ -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),
packages/coding-agent/src/session/session-loader.ts
-46
View File
@@ -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<string> {
const byId = new Map<string, SessionEntry>();
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<string>();
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;
packages/coding-agent/src/session/session-manager.ts
-26
View File
@@ -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, unknown>,
): string {
const elidedSupersededCompactions = this.#elideSupersededCompactionsOnBranch(this.#index.leafId());
const entry: CompactionEntry<T> = {
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;
}
packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts
+17 -11
View File
@@ -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);
packages/coding-agent/test/session-manager/large-session-memory.test.ts
+58 -19
View File
@@ -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);
});