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

fix(compaction): charged pre-archive kept region in rescue budget

Browse Source 
Also stamped dead-end warnings before the auto_compaction_end event so the result-driven TUI rebuild shows the badge (Codex round 6 on #6362).
This commit is contained in:
can1357
2026-07-23 17:03:10 +02:00
parent 15f1fb2c7c
commit 2c42715263
2 changed files with 79 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/session/agent-session.ts
+40 -25
View File
@@ -13940,18 +13940,27 @@ export class AgentSession {
const staleEntry = getLatestCompactionEntry(branchEntries);
if (!staleEntry) return undefined;
// Only rescue when the archive is the actual source of the overflow.
// The frame budget below charges the kept tail AFTER the archive plus
// the fixed context, mirroring what #compactionCreatedHeadroom will
// measure. When not even one frame fits (e.g. a huge kept tool result
// dominates), rebuilding would append the replacement compaction at
// the leaf — turning the branch tail into a compaction entry, which
// prepareCompaction's last-entry guard can never summarize past even
// after an elide shrinks the real culprit. Bail and let the
// elide/image tiers handle that tail instead.
// The frame budget below charges every kept entry the rebuilt context
// will still carry — the kept-recent region from `firstKeptEntryId`
// (re-emitted before the archive by buildSessionContext) plus the
// entries after the archive — on top of the fixed context, mirroring
// what #compactionCreatedHeadroom will measure. When not even one
// frame fits (e.g. a huge kept tool result dominates), rebuilding
// would append the replacement compaction at the leaf — turning the
// branch tail into a compaction entry, which prepareCompaction's
// last-entry guard can never summarize past even after an elide
// shrinks the real culprit. Bail and let the elide/image tiers handle
// that tail instead.
let keptTailTokens = 0;
for (let i = branchEntries.length - 1; i >= 0; i--) {
const entry = branchEntries[i];
if (entry.id === staleEntry.id) break;
let inKeptRegion = false;
for (const entry of branchEntries) {
if (entry.id === staleEntry.firstKeptEntryId) inKeptRegion = true;
if (entry.id === staleEntry.id) {
// Everything after the archive is always kept.
inKeptRegion = true;
continue;
}
if (!inKeptRegion) continue;
const message = (entry as { message?: AgentMessage }).message;
if (message) keptTailTokens += estimateTokens(message);
}
@@ -14292,6 +14301,25 @@ export class AgentSession {
}
}
if (!preparation) {
const noProgressDeadEnd = reason !== "idle" && !frameRescueCreatedHeadroom;
const deadEndWarning = noProgressDeadEnd
? compactionDeadEndWarning("shrink it (e.g. clear large tool output)")
: undefined;
// A rescue that appended a rebuilt archive without creating
// headroom must carry the dead-end badge on the entry the
// transcript actually shows (the rebuilt one), or the pause
// loses its explanation once the notice scrolls away. Stamp it
// BEFORE the auto_compaction_end event: a result-carrying event
// makes the TUI rebuild the chat from the current entries
// immediately, so a later stamp would not appear until some
// unrelated rebuild.
if (deadEndWarning && frameRescueResult) {
const stampEntry = getLatestCompactionEntry(this.sessionManager.getBranch());
if (stampEntry) {
stampEntry.warning = deadEndWarning;
await this.sessionManager.rewriteEntries();
}
}
// A successful frame rescue rewrote history and activated a new
// compaction entry — surface it as a real (non-skipped) result so
// the TUI rebuilds the transcript instead of treating the pass as
@@ -14304,7 +14332,6 @@ export class AgentSession {
willRetry: false,
skipped: frameRescueResult === undefined,
});
const noProgressDeadEnd = reason !== "idle" && !frameRescueCreatedHeadroom;
let continuationScheduled = false;
if (frameRescueCreatedHeadroom) {
continuationScheduled = this.#scheduleCompactionContinuation({
@@ -14321,20 +14348,8 @@ export class AgentSession {
});
continuationScheduled = true;
}
if (noProgressDeadEnd) {
const deadEndWarning = compactionDeadEndWarning("shrink it (e.g. clear large tool output)");
if (deadEndWarning) {
this.emitNotice("warning", deadEndWarning, "compaction");
// A rescue that appended a rebuilt archive without creating
// headroom must carry the dead-end badge on the entry the
// transcript actually shows (the rebuilt one), or the pause
// loses its explanation once the notice scrolls away.
if (frameRescueResult) {
const stampEntry = getLatestCompactionEntry(this.sessionManager.getBranch());
if (stampEntry) {
stampEntry.warning = deadEndWarning;
await this.sessionManager.rewriteEntries();
}
}
}
// A rescue that offloaded content but still could not produce a
// preparation rewrote the branch; flag it so the overflow-recovery
packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts
+39
View File
@@ -70,6 +70,9 @@ describe("AgentSession snapcompact frame dead-end rescue", () => {
* many frames — exercising the POST-PASS dead-end (a completed pass
* whose just-written archive is itself the over-budget cost). */
hookArchiveFrames?: number;
/** Seed a kept-recent entry between firstKeptEntryId and the archive —
* re-emitted by buildSessionContext, so the rescue budget must charge it. */
preArchiveKeptText?: string;
}): Promise<void> {
tempDir = TempDir.createSync("@pi-snapcompact-frame-dead-end-");
authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
@@ -135,6 +138,19 @@ describe("AgentSession snapcompact frame dead-end rescue", () => {
content: "hello",
timestamp: Date.now(),
});
if (options.preArchiveKeptText !== undefined && options.hookArchiveFrames === undefined) {
// A kept-recent entry BETWEEN firstKeptEntryId and the archive:
// buildSessionContext re-emits it before the compaction entry, so
// the rescue budget must charge it too.
sessionManager.appendMessage({
role: "toolResult",
toolCallId: "call-kept",
toolName: "bash",
content: [{ type: "text", text: options.preArchiveKeptText }],
isError: false,
timestamp: Date.now(),
});
}
if (options.hookArchiveFrames === undefined) {
sessionManager.appendCompaction(
"Archived history onto stale snapcompact frames.",
@@ -413,6 +429,29 @@ describe("AgentSession snapcompact frame dead-end rescue", () => {
expect(sessionManager.getBranch().at(-1)?.type).not.toBe("compaction");
});
it("bails when the kept region BEFORE the archive leaves no frame budget", async () => {
// Codex review on #6362 (round 6): the rebuilt compaction preserves
// firstKeptEntryId, and buildSessionContext re-emits the kept messages
// that sit BEFORE the compaction entry — so a large pre-archive kept
// region costs the rebuilt prompt exactly like a post-archive tail.
// The budget must charge it, or the rescue appends a still-over-band
// archive and the dead-end persists.
await createSession({ frameCount: SEEDED_FRAME_COUNT, preArchiveKeptText: "y".repeat(160_000) });
vi.spyOn(compactionModule, "prepareCompaction").mockReturnValue(undefined);
vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never);
vi.spyOn(session.agent, "continue").mockResolvedValue();
vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: 190000, contextWindow: 200000, percent: 95 });
const shakeSpy = vi
.spyOn(session, "shake")
.mockResolvedValue({ mode: "elide", toolResultsDropped: 0, blocksDropped: 0, tokensFreed: 0 });
const compactSpy = vi.spyOn(snapcompact, "compact");
await triggerMaintenance();
expect(compactSpy).not.toHaveBeenCalled();
expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything());
});
it("leaves an oversized non-archive tail to the elide tiers instead of rescuing the archive", async () => {
// Codex review on #6362 (round 4): with […, archive, HUGE kept tool
// result], rebuilding the archive would append the replacement at the