fix(coding-agent): reuse session thinking for compaction closes #3438
This commit is contained in:
@@ -9,6 +9,7 @@
|
|||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
- Fixed missing `@sinclair/typebox` runtime dependency in `@mariozechner/pi-coding-agent`, so strict pnpm installs no longer fail with `ERR_MODULE_NOT_FOUND` when starting `pi` ([#3434](https://github.com/badlogic/pi-mono/issues/3434))
|
- Fixed missing `@sinclair/typebox` runtime dependency in `@mariozechner/pi-coding-agent`, so strict pnpm installs no longer fail with `ERR_MODULE_NOT_FOUND` when starting `pi` ([#3434](https://github.com/badlogic/pi-mono/issues/3434))
|
||||||
|
- Fixed `/compact` to reuse the session thinking level for compaction summaries instead of forcing `high`, avoiding invalid reasoning-effort errors on `github-copilot/claude-opus-4.7` sessions configured for `medium` thinking ([#3438](https://github.com/badlogic/pi-mono/issues/3438))
|
||||||
- Fixed shared/exported plain-text tool output to preserve indentation instead of collapsing leading whitespace in the web share page ([#3440](https://github.com/badlogic/pi-mono/issues/3440))
|
- Fixed shared/exported plain-text tool output to preserve indentation instead of collapsing leading whitespace in the web share page ([#3440](https://github.com/badlogic/pi-mono/issues/3440))
|
||||||
- Fixed skill resolution to dedupe symlinked aliases by canonical path, so `pi config` no longer shows duplicate skill entries when `~/.pi/agent/skills` points to `~/.agents/skills` ([#3405](https://github.com/badlogic/pi-mono/issues/3405))
|
- Fixed skill resolution to dedupe symlinked aliases by canonical path, so `pi config` no longer shows duplicate skill entries when `~/.pi/agent/skills` points to `~/.agents/skills` ([#3405](https://github.com/badlogic/pi-mono/issues/3405))
|
||||||
- Fixed OpenRouter request attribution to include Pi app headers (`HTTP-Referer: https://pi.dev`, `X-OpenRouter-Title: pi`, `X-OpenRouter-Categories: cli-agent`) when sessions are created through the coding-agent SDK and install telemetry is enabled ([#3414](https://github.com/badlogic/pi-mono/issues/3414))
|
- Fixed OpenRouter request attribution to include Pi app headers (`HTTP-Referer: https://pi.dev`, `X-OpenRouter-Title: pi`, `X-OpenRouter-Categories: cli-agent`) when sessions are created through the coding-agent SDK and install telemetry is enabled ([#3414](https://github.com/badlogic/pi-mono/issues/3414))
|
||||||
|
|||||||
@@ -1667,6 +1667,7 @@ export class AgentSession {
|
|||||||
headers,
|
headers,
|
||||||
customInstructions,
|
customInstructions,
|
||||||
this._compactionAbortController.signal,
|
this._compactionAbortController.signal,
|
||||||
|
this.thinkingLevel,
|
||||||
);
|
);
|
||||||
summary = result.summary;
|
summary = result.summary;
|
||||||
firstKeptEntryId = result.firstKeptEntryId;
|
firstKeptEntryId = result.firstKeptEntryId;
|
||||||
@@ -1931,6 +1932,7 @@ export class AgentSession {
|
|||||||
headers,
|
headers,
|
||||||
undefined,
|
undefined,
|
||||||
this._autoCompactionAbortController.signal,
|
this._autoCompactionAbortController.signal,
|
||||||
|
this.thinkingLevel,
|
||||||
);
|
);
|
||||||
summary = compactResult.summary;
|
summary = compactResult.summary;
|
||||||
firstKeptEntryId = compactResult.firstKeptEntryId;
|
firstKeptEntryId = compactResult.firstKeptEntryId;
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
* and after compaction the session is reloaded.
|
* and after compaction the session is reloaded.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import type { AgentMessage } from "@mariozechner/pi-agent-core";
|
import type { AgentMessage, ThinkingLevel } from "@mariozechner/pi-agent-core";
|
||||||
import type { AssistantMessage, Model, Usage } from "@mariozechner/pi-ai";
|
import type { AssistantMessage, Model, Usage } from "@mariozechner/pi-ai";
|
||||||
import { completeSimple } from "@mariozechner/pi-ai";
|
import { completeSimple } from "@mariozechner/pi-ai";
|
||||||
import {
|
import {
|
||||||
@@ -536,6 +536,7 @@ export async function generateSummary(
|
|||||||
signal?: AbortSignal,
|
signal?: AbortSignal,
|
||||||
customInstructions?: string,
|
customInstructions?: string,
|
||||||
previousSummary?: string,
|
previousSummary?: string,
|
||||||
|
thinkingLevel?: ThinkingLevel,
|
||||||
): Promise<string> {
|
): Promise<string> {
|
||||||
const maxTokens = Math.floor(0.8 * reserveTokens);
|
const maxTokens = Math.floor(0.8 * reserveTokens);
|
||||||
|
|
||||||
@@ -565,9 +566,10 @@ export async function generateSummary(
|
|||||||
},
|
},
|
||||||
];
|
];
|
||||||
|
|
||||||
const completionOptions = model.reasoning
|
const completionOptions =
|
||||||
? { maxTokens, signal, apiKey, headers, reasoning: "high" as const }
|
model.reasoning && thinkingLevel && thinkingLevel !== "off"
|
||||||
: { maxTokens, signal, apiKey, headers };
|
? { maxTokens, signal, apiKey, headers, reasoning: thinkingLevel }
|
||||||
|
: { maxTokens, signal, apiKey, headers };
|
||||||
|
|
||||||
const response = await completeSimple(
|
const response = await completeSimple(
|
||||||
model,
|
model,
|
||||||
@@ -719,6 +721,7 @@ export async function compact(
|
|||||||
headers?: Record<string, string>,
|
headers?: Record<string, string>,
|
||||||
customInstructions?: string,
|
customInstructions?: string,
|
||||||
signal?: AbortSignal,
|
signal?: AbortSignal,
|
||||||
|
thinkingLevel?: ThinkingLevel,
|
||||||
): Promise<CompactionResult> {
|
): Promise<CompactionResult> {
|
||||||
const {
|
const {
|
||||||
firstKeptEntryId,
|
firstKeptEntryId,
|
||||||
@@ -747,9 +750,18 @@ export async function compact(
|
|||||||
signal,
|
signal,
|
||||||
customInstructions,
|
customInstructions,
|
||||||
previousSummary,
|
previousSummary,
|
||||||
|
thinkingLevel,
|
||||||
)
|
)
|
||||||
: Promise.resolve("No prior history."),
|
: Promise.resolve("No prior history."),
|
||||||
generateTurnPrefixSummary(turnPrefixMessages, model, settings.reserveTokens, apiKey, headers, signal),
|
generateTurnPrefixSummary(
|
||||||
|
turnPrefixMessages,
|
||||||
|
model,
|
||||||
|
settings.reserveTokens,
|
||||||
|
apiKey,
|
||||||
|
headers,
|
||||||
|
signal,
|
||||||
|
thinkingLevel,
|
||||||
|
),
|
||||||
]);
|
]);
|
||||||
// Merge into single summary
|
// Merge into single summary
|
||||||
summary = `${historyResult}\n\n---\n\n**Turn Context (split turn):**\n\n${turnPrefixResult}`;
|
summary = `${historyResult}\n\n---\n\n**Turn Context (split turn):**\n\n${turnPrefixResult}`;
|
||||||
@@ -764,6 +776,7 @@ export async function compact(
|
|||||||
signal,
|
signal,
|
||||||
customInstructions,
|
customInstructions,
|
||||||
previousSummary,
|
previousSummary,
|
||||||
|
thinkingLevel,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -793,6 +806,7 @@ async function generateTurnPrefixSummary(
|
|||||||
apiKey: string,
|
apiKey: string,
|
||||||
headers?: Record<string, string>,
|
headers?: Record<string, string>,
|
||||||
signal?: AbortSignal,
|
signal?: AbortSignal,
|
||||||
|
thinkingLevel?: ThinkingLevel,
|
||||||
): Promise<string> {
|
): Promise<string> {
|
||||||
const maxTokens = Math.floor(0.5 * reserveTokens); // Smaller budget for turn prefix
|
const maxTokens = Math.floor(0.5 * reserveTokens); // Smaller budget for turn prefix
|
||||||
const llmMessages = convertToLlm(messages);
|
const llmMessages = convertToLlm(messages);
|
||||||
@@ -809,7 +823,9 @@ async function generateTurnPrefixSummary(
|
|||||||
const response = await completeSimple(
|
const response = await completeSimple(
|
||||||
model,
|
model,
|
||||||
{ systemPrompt: SUMMARIZATION_SYSTEM_PROMPT, messages: summarizationMessages },
|
{ systemPrompt: SUMMARIZATION_SYSTEM_PROMPT, messages: summarizationMessages },
|
||||||
{ maxTokens, signal, apiKey, headers },
|
model.reasoning && thinkingLevel && thinkingLevel !== "off"
|
||||||
|
? { maxTokens, signal, apiKey, headers, reasoning: thinkingLevel }
|
||||||
|
: { maxTokens, signal, apiKey, headers },
|
||||||
);
|
);
|
||||||
|
|
||||||
if (response.stopReason === "error") {
|
if (response.stopReason === "error") {
|
||||||
|
|||||||
@@ -56,18 +56,58 @@ describe("generateSummary reasoning options", () => {
|
|||||||
completeSimpleMock.mockResolvedValue(mockSummaryResponse);
|
completeSimpleMock.mockResolvedValue(mockSummaryResponse);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("sets reasoning=high for reasoning-capable models", async () => {
|
it("uses the provided thinking level for reasoning-capable models", async () => {
|
||||||
await generateSummary(messages, createModel(true), 2000, "test-key");
|
await generateSummary(
|
||||||
|
messages,
|
||||||
|
createModel(true),
|
||||||
|
2000,
|
||||||
|
"test-key",
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
"medium",
|
||||||
|
);
|
||||||
|
|
||||||
expect(completeSimpleMock).toHaveBeenCalledTimes(1);
|
expect(completeSimpleMock).toHaveBeenCalledTimes(1);
|
||||||
expect(completeSimpleMock.mock.calls[0][2]).toMatchObject({
|
expect(completeSimpleMock.mock.calls[0][2]).toMatchObject({
|
||||||
reasoning: "high",
|
reasoning: "medium",
|
||||||
apiKey: "test-key",
|
apiKey: "test-key",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not set reasoning when thinking is off", async () => {
|
||||||
|
await generateSummary(
|
||||||
|
messages,
|
||||||
|
createModel(true),
|
||||||
|
2000,
|
||||||
|
"test-key",
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
"off",
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(completeSimpleMock).toHaveBeenCalledTimes(1);
|
||||||
|
expect(completeSimpleMock.mock.calls[0][2]).toMatchObject({
|
||||||
|
apiKey: "test-key",
|
||||||
|
});
|
||||||
|
expect(completeSimpleMock.mock.calls[0][2]).not.toHaveProperty("reasoning");
|
||||||
|
});
|
||||||
|
|
||||||
it("does not set reasoning for non-reasoning models", async () => {
|
it("does not set reasoning for non-reasoning models", async () => {
|
||||||
await generateSummary(messages, createModel(false), 2000, "test-key");
|
await generateSummary(
|
||||||
|
messages,
|
||||||
|
createModel(false),
|
||||||
|
2000,
|
||||||
|
"test-key",
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
"medium",
|
||||||
|
);
|
||||||
|
|
||||||
expect(completeSimpleMock).toHaveBeenCalledTimes(1);
|
expect(completeSimpleMock).toHaveBeenCalledTimes(1);
|
||||||
expect(completeSimpleMock.mock.calls[0][2]).toMatchObject({
|
expect(completeSimpleMock.mock.calls[0][2]).toMatchObject({
|
||||||
|
|||||||
Reference in New Issue
Block a user