diff --git a/CONFIGURATION.md b/CONFIGURATION.md index 5bb030905..af5c70730 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -663,7 +663,7 @@ It is useful when starting a new session. It's better to choose a fast and cheap |-------|------|-------------| | `model` | `string` | Primary model. | | `fallback_models` | `string` or `string[]` | Fallback models. | -| `temperature` | `number` (0–2) | Sampling temperature. | +| `temperature` | `number` (0–2) | Sampling temperature. Omitted by default: no `temperature` is sent to the provider unless you set one. Reasoning models reject the parameter outright, so only set it for models that accept it. | | `variant` | `string` | **OpenCode only.** Agent variant — selects a thinking/reasoning preset. Pi uses `thinking_level` instead. | | `thinking_level` | `string` | **Pi only.** Explicit reasoning level (`off`/`low`/`medium`/`high`) passed to Pi for sidekick subagent runs. See `historian.thinking_level`. | | `prompt` | `string` | Persistent agent-level system prompt override. Applies to every sidekick run. | diff --git a/packages/pi-plugin/src/historian-calibration-extension.test.ts b/packages/pi-plugin/src/historian-calibration-extension.test.ts index cadbc3b87..e7bf493c6 100644 --- a/packages/pi-plugin/src/historian-calibration-extension.test.ts +++ b/packages/pi-plugin/src/historian-calibration-extension.test.ts @@ -42,4 +42,56 @@ describe("historian provider calibration", () => { ), ).toEqual({ inferenceConfig: { temperature: 0.1, maxTokens: 32_000 } }); }); + + it("never sends temperature when it is not configured", () => { + const fixtures = [ + { input: { max_tokens: 4096 }, key: "max_tokens" }, + { input: { max_completion_tokens: 4096 }, key: "max_completion_tokens" }, + { input: { max_output_tokens: 4096 }, key: "max_output_tokens" }, + { input: { maxTokens: 4096 }, key: "maxTokens" }, + ] as const; + for (const fixture of fixtures) { + const result = calibrateHistorianProviderPayload( + fixture.input, + undefined, + 32_000, + ) as Record; + expect("temperature" in result).toBe(false); + expect(result[fixture.key]).toBe(32_000); + } + }); + + it("omits temperature from nested provider shapes when unconfigured", () => { + expect( + calibrateHistorianProviderPayload( + { generationConfig: { topP: 0.9, maxOutputTokens: 4096 } }, + undefined, + 32_000, + ), + ).toEqual({ generationConfig: { topP: 0.9, maxOutputTokens: 32_000 } }); + expect( + calibrateHistorianProviderPayload( + { inferenceConfig: { maxTokens: 4096 } }, + undefined, + 32_000, + ), + ).toEqual({ inferenceConfig: { maxTokens: 32_000 } }); + }); + + it("applies temperature alone when no output budget is configured", () => { + const result = calibrateHistorianProviderPayload( + { max_tokens: 4096 }, + 0.1, + undefined, + ) as Record; + expect(result.temperature).toBe(0.1); + expect(result.max_tokens).toBe(4096); + }); + + it("returns the payload untouched when neither knob is configured", () => { + const payload = { max_tokens: 4096 }; + expect( + calibrateHistorianProviderPayload(payload, undefined, undefined), + ).toEqual({ max_tokens: 4096 }); + }); }); diff --git a/packages/pi-plugin/src/historian-calibration-extension.ts b/packages/pi-plugin/src/historian-calibration-extension.ts index 71a0e7d2b..67b362555 100644 --- a/packages/pi-plugin/src/historian-calibration-extension.ts +++ b/packages/pi-plugin/src/historian-calibration-extension.ts @@ -10,14 +10,22 @@ function finiteNumber(value: string | undefined): number | undefined { return Number.isFinite(parsed) && parsed >= 0 ? parsed : undefined; } -/** Apply the historian calibration to each provider's serialized request shape. */ +/** + * Apply the historian calibration to each provider's serialized request shape. + * + * Both knobs are optional and applied independently: reasoning models reject + * `temperature` outright, so an output-token budget must still be applicable + * on its own. + */ export function calibrateHistorianProviderPayload( payload: unknown, - temperature: number, - maxOutputTokens: number, + temperature: number | undefined, + maxOutputTokens: number | undefined, ): unknown { if (typeof payload !== "object" || payload === null || Array.isArray(payload)) return payload; + if (temperature === undefined && maxOutputTokens === undefined) + return payload; const calibrated = { ...(payload as Record) }; const generationConfig = calibrated.generationConfig; if ( @@ -27,8 +35,8 @@ export function calibrateHistorianProviderPayload( ) { calibrated.generationConfig = { ...(generationConfig as Record), - temperature, - maxOutputTokens, + ...(temperature !== undefined ? { temperature } : {}), + ...(maxOutputTokens !== undefined ? { maxOutputTokens } : {}), }; return calibrated; } @@ -40,21 +48,25 @@ export function calibrateHistorianProviderPayload( ) { calibrated.inferenceConfig = { ...(inferenceConfig as Record), - temperature, - maxTokens: maxOutputTokens, + ...(temperature !== undefined ? { temperature } : {}), + ...(maxOutputTokens !== undefined ? { maxTokens: maxOutputTokens } : {}), }; return calibrated; } - calibrated.temperature = temperature; - if ("max_output_tokens" in calibrated) { - calibrated.max_output_tokens = maxOutputTokens; - } else if ("max_completion_tokens" in calibrated) { - calibrated.max_completion_tokens = maxOutputTokens; - } else if ("max_tokens" in calibrated) { - calibrated.max_tokens = maxOutputTokens; - } else if ("maxTokens" in calibrated) { - calibrated.maxTokens = maxOutputTokens; + if (temperature !== undefined) { + calibrated.temperature = temperature; + } + if (maxOutputTokens !== undefined) { + if ("max_output_tokens" in calibrated) { + calibrated.max_output_tokens = maxOutputTokens; + } else if ("max_completion_tokens" in calibrated) { + calibrated.max_completion_tokens = maxOutputTokens; + } else if ("max_tokens" in calibrated) { + calibrated.max_tokens = maxOutputTokens; + } else if ("maxTokens" in calibrated) { + calibrated.maxTokens = maxOutputTokens; + } } return calibrated; } @@ -64,7 +76,7 @@ export default function historianCalibrationExtension(pi: ExtensionAPI): void { const maxOutputTokens = finiteNumber( process.env[HISTORIAN_MAX_OUTPUT_TOKENS_ENV], ); - if (temperature === undefined || maxOutputTokens === undefined) return; + if (temperature === undefined && maxOutputTokens === undefined) return; pi.on("before_provider_request", (event) => calibrateHistorianProviderPayload( event.payload, diff --git a/packages/pi-plugin/src/index.ts b/packages/pi-plugin/src/index.ts index 846e45e5e..e396fd379 100644 --- a/packages/pi-plugin/src/index.ts +++ b/packages/pi-plugin/src/index.ts @@ -703,7 +703,7 @@ export function resolveHistorianFromConfig( fallbackModels, historianChunkTokens, timeoutMs: config.historian_timeout_ms, - temperature: historian?.temperature ?? 0.1, + temperature: historian?.temperature, maxOutputTokens: historian?.maxTokens ?? 32_000, // `historian.two_pass` runs an editor pass after a successful // first pass to clean low-signal U: lines and cross-compartment diff --git a/packages/pi-plugin/src/pi-historian-runner.ts b/packages/pi-plugin/src/pi-historian-runner.ts index 465caea22..760279dbc 100644 --- a/packages/pi-plugin/src/pi-historian-runner.ts +++ b/packages/pi-plugin/src/pi-historian-runner.ts @@ -444,7 +444,7 @@ export async function runPiHistorian(deps: PiHistorianDeps): Promise { refreshBoundarySnapshot, currentContextLimit, historianTimeoutMs = DEFAULT_HISTORIAN_TIMEOUT_MS, - temperature = 0.1, + temperature, maxOutputTokens = 32_000, signal, retryBackoffMs, diff --git a/packages/pi-plugin/src/subagent-runner.test.ts b/packages/pi-plugin/src/subagent-runner.test.ts index b02d91a6b..660534d69 100644 --- a/packages/pi-plugin/src/subagent-runner.test.ts +++ b/packages/pi-plugin/src/subagent-runner.test.ts @@ -1558,6 +1558,34 @@ describe("PiSubagentRunner spawn lifecycle", () => { }); }); + it("surfaces the provider error behind empty assistant text", async () => { + const child = createMockChild(); + const { runner } = runnerWith(child); + + const resultPromise = runner.run(baseOptions); + child.writeStdoutLine( + agentEnd([ + { + role: "assistant", + content: [], + stopReason: "error", + errorMessage: + "OpenAI API error (400): Unsupported parameter: temperature", + }, + ]), + ); + child.emitClose(0); + + expect(await resultPromise).toEqual({ + ok: false, + reason: "no_assistant", + error: + "pi assistant produced empty text (provider error: OpenAI API error (400): Unsupported parameter: temperature)", + durationMs: expect.any(Number), + meta: { stderr: undefined, sawProtocolOutput: true }, + }); + }); + it("returns no_assistant for empty stdout and successful exit", async () => { // Issue #238: an empty-stdout exit-0 primary now fires the one-shot // isolated retry. When the isolated attempt ALSO exits 0 with no output, diff --git a/packages/pi-plugin/src/subagent-runner.ts b/packages/pi-plugin/src/subagent-runner.ts index bfeea7f74..d0a88fd23 100644 --- a/packages/pi-plugin/src/subagent-runner.ts +++ b/packages/pi-plugin/src/subagent-runner.ts @@ -1378,13 +1378,16 @@ export class PiSubagentRunner implements SubagentRunner { trimmedAssistantText === null || trimmedAssistantText.length === 0 ) { + const emptyAssistantReason = + trimmedAssistantText === null + ? "pi agent_end did not include an assistant message" + : "pi assistant produced empty text"; settle({ ok: false, reason: "no_assistant", - error: - trimmedAssistantText === null - ? "pi agent_end did not include an assistant message" - : "pi assistant produced empty text", + error: finalErrorMessage + ? `${emptyAssistantReason} (provider error: ${finalErrorMessage})` + : emptyAssistantReason, durationMs: Date.now() - startTime, // Pi machinery worked (agent_end / terminal message_end seen); // the model just returned empty text. Mark protocol output as