mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): recovery omits tool results with the truncated tool call
Pre-prompt recovery of a recoverable length stop omitted the assistant message but not the tool results that answered its calls, because only the end-of-run caller passed them. The orphaned results were persisted and providers rejected every later request. _omitRecoveryAttempt now finds the matching tool results in the projected context itself, so every caller omits the pair together. closes #11227
This commit is contained in:
parent
8c4e873899
commit
02726086d1
2 changed files with 100 additions and 12 deletions
|
|
@ -427,7 +427,6 @@ export class AgentSession {
|
|||
private readonly _entryIdsByMessage = new WeakMap<object, string>();
|
||||
private readonly _boundaryDispatchedMessages = new WeakSet<object>();
|
||||
private _lastAssistantMessage: AssistantMessage | undefined;
|
||||
private _lastAssistantToolResults: AgentMessage[] = [];
|
||||
private _lastActivityOutcome: AgentActivityOutcome = "completed";
|
||||
private _isBeforeSettle = false;
|
||||
private _abortDuringBeforeSettle = false;
|
||||
|
|
@ -1194,7 +1193,6 @@ export class AgentSession {
|
|||
// extension and listener dispatch above also picks up messages that turn_end
|
||||
// handlers queued.
|
||||
if (event.type === "turn_end") {
|
||||
this._lastAssistantToolResults = event.toolResults;
|
||||
this._flushPendingCustomMessages();
|
||||
}
|
||||
};
|
||||
|
|
@ -1238,7 +1236,24 @@ export class AgentSession {
|
|||
return undefined;
|
||||
}
|
||||
|
||||
private _omitRecoveryAttempt(message: AssistantMessage, toolResults: AgentMessage[] = []): void {
|
||||
/**
|
||||
* Persistently omit a failed attempt and its tool results from model context. Results are
|
||||
* found in the projected context, so the pair stays together whether the attempt comes from
|
||||
* the current run or a resumed session. An unpaired result fails the next request (#11227).
|
||||
*/
|
||||
private _omitRecoveryAttempt(message: AssistantMessage): void {
|
||||
const toolCallIds = new Set(message.content.flatMap((block) => (block.type === "toolCall" ? [block.id] : [])));
|
||||
// A context edit replaces the projected copy, so match it by source entry as well.
|
||||
const entryId = this._findPersistedMessageEntryId(message);
|
||||
const messages = this.agent.state.messages;
|
||||
const index = messages.findIndex(
|
||||
(m) => m === message || (entryId !== undefined && this._entryIdsByMessage.get(m) === entryId),
|
||||
);
|
||||
const toolResults: AgentMessage[] = [];
|
||||
for (const m of index < 0 ? [] : messages.slice(index + 1)) {
|
||||
if (m.role === "assistant") break;
|
||||
if (m.role === "toolResult" && toolCallIds.has(m.toolCallId)) toolResults.push(m);
|
||||
}
|
||||
const targets = [message, ...toolResults];
|
||||
const targetIds = targets.map((target) => this._findPersistedMessageEntryId(target));
|
||||
const unresolvedProjectedTarget = targets.some(
|
||||
|
|
@ -1963,9 +1978,7 @@ export class AgentSession {
|
|||
|
||||
private async _handlePostAgentRun(): Promise<boolean> {
|
||||
const message = this._lastAssistantMessage;
|
||||
const toolResults = this._lastAssistantToolResults;
|
||||
this._lastAssistantMessage = undefined;
|
||||
this._lastAssistantToolResults = [];
|
||||
if (this._agentRunAbortRequested) {
|
||||
this._finishCancelledRetry();
|
||||
return false;
|
||||
|
|
@ -1992,7 +2005,7 @@ export class AgentSession {
|
|||
this._retryAttempt = 0;
|
||||
}
|
||||
|
||||
if (await this._checkCompaction(message, true, toolResults)) {
|
||||
if (await this._checkCompaction(message)) {
|
||||
return !this._agentRunAbortRequested;
|
||||
}
|
||||
|
||||
|
|
@ -2995,11 +3008,7 @@ export class AgentSession {
|
|||
* @param skipAbortedCheck If false, include aborted messages (for pre-prompt check). Default: true
|
||||
* @returns Whether the post-run loop should call `agent.continue()` for overflow recovery or queued messages
|
||||
*/
|
||||
private async _checkCompaction(
|
||||
assistantMessage: AssistantMessage,
|
||||
skipAbortedCheck = true,
|
||||
toolResults: AgentMessage[] = [],
|
||||
): Promise<boolean> {
|
||||
private async _checkCompaction(assistantMessage: AssistantMessage, skipAbortedCheck = true): Promise<boolean> {
|
||||
const settings = this.settingsManager.getCompactionSettings(this.model);
|
||||
if (!settings.enabled) return false;
|
||||
|
||||
|
|
@ -3091,7 +3100,7 @@ export class AgentSession {
|
|||
|
||||
// Persistently omit the selected final attempt before post-run recovery compaction.
|
||||
this._overflowRecoveryAttempted = true;
|
||||
this._omitRecoveryAttempt(assistantMessage, toolResults);
|
||||
this._omitRecoveryAttempt(assistantMessage);
|
||||
const retry = await this._runAutoCompaction("overflow", willRetry);
|
||||
if (retry) this._failedResponse = assistantMessage;
|
||||
return retry;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,79 @@
|
|||
import { type AssistantMessage, fauxAssistantMessage, fauxToolCall, type Message } from "@earendil-works/pi-ai";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createHarness, type Harness } from "../harness.ts";
|
||||
|
||||
// https://github.com/earendil-works/pi/issues/11227
|
||||
describe("issue #11227 recovery omits tool results with their tool call", () => {
|
||||
const harnesses: Harness[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
while (harnesses.length > 0) {
|
||||
harnesses.pop()?.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it("pre-prompt recovery of a resumed length stop omits the truncated call and its result", async () => {
|
||||
const harness = await createHarness({
|
||||
models: [{ id: "faux-1", contextWindow: 100_000, maxTokens: 1_000 }],
|
||||
settings: { compaction: { enabled: true } },
|
||||
extensionFactories: [
|
||||
(pi) => {
|
||||
pi.on("session_before_compact", async (event) => ({
|
||||
compaction: {
|
||||
summary: "summary",
|
||||
firstKeptEntryId: event.preparation.firstKeptEntryId,
|
||||
tokensBefore: event.preparation.tokensBefore,
|
||||
details: {},
|
||||
},
|
||||
}));
|
||||
},
|
||||
],
|
||||
});
|
||||
harnesses.push(harness);
|
||||
|
||||
const now = Date.now();
|
||||
const model = harness.getModel();
|
||||
harness.sessionManager.appendMessage({ role: "user", content: "check status", timestamp: now - 1000 });
|
||||
const truncated: AssistantMessage = {
|
||||
...fauxAssistantMessage([fauxToolCall("bash", { command: "echo" }, { id: "call_truncated" })], {
|
||||
stopReason: "length",
|
||||
timestamp: now - 500,
|
||||
}),
|
||||
api: model.api,
|
||||
provider: model.provider,
|
||||
model: model.id,
|
||||
usage: {
|
||||
input: 100,
|
||||
output: 16,
|
||||
cacheRead: 0,
|
||||
cacheWrite: 0,
|
||||
totalTokens: 116,
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||
},
|
||||
};
|
||||
harness.sessionManager.appendMessage(truncated);
|
||||
harness.sessionManager.appendMessage({
|
||||
role: "toolResult",
|
||||
toolCallId: "call_truncated",
|
||||
toolName: "bash",
|
||||
isError: true,
|
||||
content: [{ type: "text", text: "Not executed: output token limit reached." }],
|
||||
timestamp: now - 400,
|
||||
});
|
||||
harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages;
|
||||
|
||||
let requestMessages: Message[] = [];
|
||||
harness.setResponses([
|
||||
(context) => {
|
||||
requestMessages = context.messages;
|
||||
return fauxAssistantMessage("done");
|
||||
},
|
||||
]);
|
||||
|
||||
await harness.session.prompt("continue");
|
||||
|
||||
const requestRoles = requestMessages.map((message) => message.role);
|
||||
expect(requestRoles).not.toContain("toolResult");
|
||||
expect(requestMessages.some((m) => m.role === "assistant" && m.stopReason === "length")).toBe(false);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue