fix(coding-agent): treat at-band residual as compaction headroom
The post-maintenance headroom guard returned residualTokens < triggerContextTokens as a secondary check after the recovery-band test. When stale/tool-output pruning already drove the trigger (postMaintenanceContextTokens) below the band before this pass, that strict-less comparison made a residual which merely held the line at/under the band report a false no-progress, suppressing a valid auto-continue and emitting a spurious warning even though the next turn could no longer re-trip threshold compaction. The recovery band sits strictly under the compaction threshold, so reaching it already guarantees the next turn cannot re-trip. Make the band authoritative (residual <= band) and drop the trigger argument. Adds a regression covering the sub-band trigger case. (cherry picked from commit 31cf2dc7a30c4134f7a351d61065eca76543f154)
This commit is contained in:
Wolfgang Schoenberger
committed by
can1357
can1357
parent
4eda8ee59f
commit
6958b86822
@@ -70,6 +70,7 @@
|
||||
|
||||
- Fixed the advisor entering a spam loop in which it emitted hundreds of repeated `Stop.`, `Done.`, and `No issue; continue.` `<advisory severity="blocker">` injections, polluting the primary transcript and destabilizing the watched agent after the task was already complete. The advisor system prompt's rules ("at most one `advise` per update", "NEVER send the same advice twice") are now enforced in code by a new `AdvisorEmissionGuard` on the `enqueueAdvice` boundary in `AgentSession`: it normalizes each note (case-insensitive, punctuation-folded), drops content-free self-talk filler (`stop`/`done`/`no issue continue`/`lgtm`/etc.), dedupes by exact normalized text across the session (bounded FIFO history), and rate-limits to one accepted note per advisor model prompt cycle. Reset on advisor reset (compaction, session switch, `/new`) so a re-primed reviewer can re-raise old issues. ([#3520](https://github.com/can1357/oh-my-pi/issues/3520))
|
||||
|
||||
- Fixed auto-compaction thrashing on a session whose single most-recent kept turn already exceeds the compaction threshold. `prepareCompaction` keeps that turn verbatim (`findCutPoint` never cuts at tool results), so the rewritten context stays above threshold; the context-full / snapcompact success tail scheduled the agent-authored auto-continue (and the overflow/incomplete retry) unconditionally, so the next `agent_end` re-entered `#checkCompaction` over the same oversized tail and re-fired forever. This is the residual loop left after #3247 capped snapcompact's own frame projection — once the frame cap drops below one frame, snapcompact is skipped and the context-full summarizer path still made no headroom. `#runAutoCompaction` now gates the threshold auto-continue on a post-maintenance headroom check (`#compactionCreatedHeadroom`, sharing shake's `COMPACTION_RECOVERY_BAND` hysteresis from #2275) and the overflow/incomplete retry on a separate fit check (`#compactionCreatedRetryFit`, measured after the failed turn is dropped) so a recoverable overflow that fits the window still retries; when a pass frees too little for the relevant path it pauses automatic maintenance and emits a single warning instead of looping. The post-turn threshold check also ignores an assistant's stale pre-compaction `usage` so the scheduled auto-continue cannot re-trip on the kept assistant's old high token count. The headroom check now treats any residual at or below the recovery band as progress: the band sits strictly under the compaction threshold, so a stale/tool-output prune that already pushed the trigger sub-band no longer makes a residual that merely holds the line report a false "no progress" and suppress a valid auto-continue.
|
||||
## [16.1.20] - 2026-06-25
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -10138,12 +10138,14 @@ export class AgentSession {
|
||||
* blows the threshold cannot be reduced by compaction (findCutPoint keeps that
|
||||
* turn verbatim), so re-firing on the next agent_end just thrashes. We only
|
||||
* report progress when residual context lands at or below
|
||||
* `COMPACTION_RECOVERY_BAND × threshold`.
|
||||
* `COMPACTION_RECOVERY_BAND × threshold` — a band that sits strictly under the
|
||||
* compaction threshold, so reaching it guarantees the next turn cannot
|
||||
* re-trip threshold compaction.
|
||||
*
|
||||
* When the model/window is unknown we cannot evaluate the band, so we
|
||||
* optimistically allow the continuation (preserving prior behavior).
|
||||
*/
|
||||
#compactionCreatedHeadroom(triggerContextTokens?: number): boolean {
|
||||
#compactionCreatedHeadroom(): boolean {
|
||||
const contextWindow = this.model?.contextWindow ?? 0;
|
||||
if (contextWindow <= 0) return true;
|
||||
const compactionSettings = this.settings.getGroup("compaction");
|
||||
@@ -10153,19 +10155,15 @@ export class AgentSession {
|
||||
);
|
||||
const thresholdTokens = resolveThresholdTokens(contextWindow, compactionSettings);
|
||||
const recoveryBand = Math.floor(thresholdTokens * COMPACTION_RECOVERY_BAND);
|
||||
// A genuine reduction past the band always counts as progress. The
|
||||
// triggerContextTokens comparison is a secondary guard: if residual context
|
||||
// is not meaningfully smaller than what triggered this pass, treat it as a
|
||||
// no-op even when it nominally sits under the band.
|
||||
if (residualTokens > recoveryBand) return false;
|
||||
if (
|
||||
typeof triggerContextTokens === "number" &&
|
||||
Number.isFinite(triggerContextTokens) &&
|
||||
triggerContextTokens > 0
|
||||
) {
|
||||
return residualTokens < triggerContextTokens;
|
||||
}
|
||||
return true;
|
||||
// Residual at/below the band is authoritative headroom: the band sits
|
||||
// strictly under the compaction threshold, so the next turn cannot
|
||||
// re-trip threshold compaction regardless of how little this pass shaved.
|
||||
// Don't add a secondary "smaller than the trigger" guard — when stale/
|
||||
// tool-output pruning already dropped context under the band before this
|
||||
// pass, the trigger is itself sub-band, and requiring a strict reduction
|
||||
// would suppress a valid continuation and emit a false no-progress warning
|
||||
// even though compaction left the session safe.
|
||||
return residualTokens <= recoveryBand;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -10736,7 +10734,7 @@ export class AgentSession {
|
||||
// landed residual context under `COMPACTION_RECOVERY_BAND × threshold`.
|
||||
// Re-firing on a history that still sits just over the line is the
|
||||
// snapcompact thrash, so require genuine headroom, not a bare fit.
|
||||
if (this.#compactionCreatedHeadroom(options.triggerContextTokens)) {
|
||||
if (this.#compactionCreatedHeadroom()) {
|
||||
this.#scheduleAutoContinuePrompt(generation);
|
||||
continuationScheduled = true;
|
||||
} else {
|
||||
|
||||
@@ -344,6 +344,115 @@ describe("AgentSession auto-compaction progress guard", () => {
|
||||
expect(noProgress.length).toBe(0);
|
||||
});
|
||||
|
||||
/**
|
||||
* Seed a single large `useless` tool result (plus tiny follow-up turns that
|
||||
* keep its suffix inside the cache-warm window) so the per-turn maintenance
|
||||
* passes free ~40k tokens before compaction runs — the same shape as the
|
||||
* #3174 pruning regression. This drives `postMaintenanceContextTokens` (the
|
||||
* trigger handed to the headroom guard) well below the recovery band.
|
||||
*/
|
||||
function seedPrunableMaintenance(now: number) {
|
||||
sessionManager.appendMessage({ role: "user", content: "Investigate everything.", timestamp: now - 200 });
|
||||
const bigCallId = "call-big-useless";
|
||||
sessionManager.appendMessage({
|
||||
role: "assistant",
|
||||
content: [{ type: "toolCall", id: bigCallId, name: "search", arguments: { pattern: "TODO" } }],
|
||||
api: "anthropic-messages",
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-5",
|
||||
stopReason: "toolUse",
|
||||
usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } },
|
||||
timestamp: now - 180,
|
||||
});
|
||||
sessionManager.appendMessage({
|
||||
role: "toolResult",
|
||||
toolCallId: bigCallId,
|
||||
toolName: "search",
|
||||
content: [{ type: "text", text: "match line\n".repeat(20000) }], // ~40k+ tokens
|
||||
isError: false,
|
||||
useless: true,
|
||||
timestamp: now - 170,
|
||||
});
|
||||
for (let i = 0; i < 4; i++) {
|
||||
const smallId = `call-small-${i}`;
|
||||
const ts = now - 160 + i * 2;
|
||||
sessionManager.appendMessage({
|
||||
role: "assistant",
|
||||
content: [{ type: "toolCall", id: smallId, name: "read", arguments: { path: `note-${i}.md` } }],
|
||||
api: "anthropic-messages",
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-5",
|
||||
stopReason: "toolUse",
|
||||
usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } },
|
||||
timestamp: ts,
|
||||
});
|
||||
sessionManager.appendMessage({
|
||||
role: "toolResult",
|
||||
toolCallId: smallId,
|
||||
toolName: "read",
|
||||
content: [{ type: "text", text: `tiny note ${i}` }],
|
||||
isError: false,
|
||||
timestamp: ts + 1,
|
||||
});
|
||||
}
|
||||
session.agent.replaceMessages(session.buildDisplaySessionContext().messages);
|
||||
}
|
||||
|
||||
it("auto-continues when residual sits at the recovery band but the trigger was already sub-band", async () => {
|
||||
// Regression for the #3412 review: when stale/tool-output pruning already
|
||||
// dropped context under the recovery band BEFORE this pass, the trigger
|
||||
// (postMaintenanceContextTokens) is itself sub-band. The old guard returned
|
||||
// `residual < trigger`, so a residual that merely held the line at/under the
|
||||
// band — not strictly smaller than the already-safe trigger — was reported
|
||||
// as no-progress and the auto-continue was suppressed with a false warning,
|
||||
// even though the next turn could no longer re-trip threshold compaction.
|
||||
const now = Date.now();
|
||||
// Pin the threshold so the recovery band is exact: floor(76384 * 0.8) = 61107.
|
||||
session.settings.set("compaction.thresholdTokens", 76384);
|
||||
session.settings.set("compaction.thresholdPercent", -1);
|
||||
session.settings.set("compaction.strategy", "context-full");
|
||||
session.settings.set("compaction.dropUseless", true);
|
||||
session.settings.set("compaction.supersedeReads", true);
|
||||
session.settings.set("compaction.keepRecentTokens", 10000);
|
||||
session.settings.set("compaction.reserveTokens", 16384);
|
||||
seedPrunableMaintenance(now);
|
||||
|
||||
const promptSpy = vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never);
|
||||
vi.spyOn(session.agent, "continue").mockResolvedValue();
|
||||
// Residual lands AT the band (61000 <= 61107). Maintenance pruning already
|
||||
// drove the trigger below this, so the old strict-less guard would have
|
||||
// suppressed; the band check proves headroom and continues.
|
||||
vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: 61000, contextWindow: 200000, percent: 30.5 });
|
||||
|
||||
const notices = collectNotices();
|
||||
|
||||
const { promise: compactionDone, resolve: onCompactionDone } = Promise.withResolvers<void>();
|
||||
session.subscribe(event => {
|
||||
if (event.type === "auto_compaction_end") onCompactionDone();
|
||||
});
|
||||
|
||||
// Final turn billed above the 76384 threshold so threshold compaction fires.
|
||||
const finalAssistant = {
|
||||
role: "assistant" as const,
|
||||
content: [{ type: "text" as const, text: "continuing." }],
|
||||
api: "anthropic-messages" as const,
|
||||
provider: "anthropic" as const,
|
||||
model: "claude-sonnet-4-5",
|
||||
stopReason: "stop" as const,
|
||||
usage: { input: 5000, output: 1000, cacheRead: 85000, cacheWrite: 0, totalTokens: 91000, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } },
|
||||
timestamp: now,
|
||||
};
|
||||
session.agent.emitExternalEvent({ type: "message_end", message: finalAssistant });
|
||||
session.agent.emitExternalEvent({ type: "agent_end", messages: [finalAssistant] });
|
||||
|
||||
await compactionDone;
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(promptSpy).toHaveBeenCalledTimes(1);
|
||||
const noProgress = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes(NO_PROGRESS_FRAGMENT));
|
||||
expect(noProgress.length).toBe(0);
|
||||
});
|
||||
|
||||
it("pauses (single warning) when an overflow recovery still does not fit the window", async () => {
|
||||
// The genuine dead-end the retry guard must still catch: even after dropping
|
||||
// the failed turn the rebuilt prompt is over the window, so retrying would
|
||||
|
||||
Reference in New Issue
Block a user