fix(historian): stop sending temperature to reasoning models - #399
fix(historian): stop sending temperature to reasoning models#399tickernelz wants to merge 2 commits into
Conversation
The historian defaulted temperature to 0.1 in two independent places, so every run sent a temperature the user never configured. Reasoning models reject the parameter outright: OpenAI Responses answers 400 "Unsupported parameter: temperature" and Anthropic answers 400 "`temperature` may only be set to 1 when thinking is enabled". The rejected request produces an empty assistant message, which surfaces as no_assistant and points the user at their model config even though model and endpoint are healthy. Every fallback model fails identically, because the request shape is the cause rather than the model. Removing the index.ts default alone is not enough; the destructuring default in pi-historian-runner.ts silently reapplies 0.1. The calibration extension also gated both knobs together, so an output token budget could not be applied without a temperature. Apply them independently, keeping the 32k budget working for reasoning models. All three defaults were introduced together in 10f80e5 and first released in v0.41.0. Verified by the four new calibration cases and the full pi-plugin suite (886 pass, typecheck and lint clean).
The empty-assistant-text branch is evaluated before the stopReason error branch, so a provider rejection whose message carries no text always settles as no_assistant and the captured finalErrorMessage holding the real HTTP error is discarded. The user only ever sees "pi assistant produced empty text", which hides the actual cause and sends debugging toward the model configuration. Append the provider error to the failure message when one was captured, leaving the reason code and retry semantics unchanged. Proven with a negative control: reverting only this change fails exactly the new regression test (86 pass, 1 fail) and passes with it (87 pass).
|
One more data point that raises the severity: there is no config-level workaround. The calibration extension bails when I confirmed this end to end: with So for anyone running a reasoning model as historian on 0.41.0, the only remedies today are patching the installed |
Problem
Every historian run fails with
no_assistant: pi assistant produced empty textwhen the historian model is a reasoning/thinking model. The plugin reports this as a misconfigured or unreachable historian model, so the warning tells the user to checkmagic-context.jsonc— but the model and the endpoint are both healthy.The real failure is an HTTP 400 from the provider, caused by the plugin itself:
temperaturegpt-5.xreasoning)400 Unsupported parameter: temperatureclaude-*with thinking)400 \temperature` may only be set to 1 when thinking is enabled or in adaptive mode`Because the provider rejects the request, pi emits
agent_endwith an empty assistant message, andextractFinalAssistant()reports empty text. Every configured fallback model fails the same way, since the cause is the request shape rather than the model.The three defaults responsible were introduced together in
10f80e58("mason: close producer and maintenance parity"), first released inv0.41.0— the only tag that contains that commit. In my case a healthy setup went from 16 consecutive successful compartings to 32 consecutive failures immediately after upgrading.Reproduction
Yields
stopReason: "error",content: [], anderrorMessage: "OpenAI API error (400): Unsupported parameter: temperature". Dropping the temperature env var makes the identical command return normal text.Root cause
temperatureis defaulted to0.1in two independent places, so the historian always sends it even when the user never configured one:packages/pi-plugin/src/index.ts—temperature: historian?.temperature ?? 0.1packages/pi-plugin/src/pi-historian-runner.ts—temperature = 0.1destructuring defaultRemoving only the first is not enough; the runner default silently reapplies
0.1.historian-calibration-extension.tsalso treats the two knobs as a single unit (if (temperature === undefined || maxOutputTokens === undefined) return), so an output-token budget cannot be applied unless a temperature is supplied too.Why the 400 was invisible
In
subagent-runner.ts, the empty-assistant-text branch is evaluated before thefinalStopReason === "error"branch. A provider error whose message carries no text therefore always settles asno_assistant, and the capturedfinalErrorMessageholding the real 400 is discarded. The user only ever sees "produced empty text".Changes
temperatureis now opt-in. When it is not configured, notemperatureis sent to the provider at all — restoring the request shape used beforev0.41.0. Users whose models accept it can still sethistorian.temperature.temperatureandmaxOutputTokensare applied independently, so the 32k output budget still works on its own. This keeps the calibration feature intact for reasoning models, which is where the output budget matters most.no_assistantfailure now appends the provider error message when one was captured, so a rejected request is diagnosable instead of being reported as an empty model response.CONFIGURATION.mddocuments the opt-in behaviour and the reasoning-model constraint.No config migration is required: existing configs that set
historian.temperatureexplicitly keep their exact behaviour.Tests
historian-calibration-extension.test.tscovering unset temperature, nested provider shapes, temperature without an output budget, and neither knob set.subagent-runner.test.tsasserting the provider error survives into the failure message.subagent-runner.tschange fails exactly that one test (86 pass / 1 fail), and passes with it (87 pass).Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Stops the historian from always sending
temperatureto providers, which caused reasoning models to reject requests with HTTP 400 and surface as a misleadingno_assistanterror. Temperature is now opt-in: if unconfigured, notemperatureis sent at all.Bug Fixes
temperatureandmaxOutputTokensare now applied independently, so the output-token budget still works on its own.no_assistantfailures append the captured provider error so rejected requests are diagnosable.historian.temperaturebehave exactly as before.Written for commit 55ad3d1. Summary will update on new commits.
Greptile Summary
The PR makes Pi historian temperature opt-in while preserving independent output-token calibration, and improves diagnostics when a provider error accompanies an empty assistant response.
0.1temperature from configuration resolution and historian execution.Confidence Score: 5/5
The PR appears safe to merge; no concrete changed-code failure remains after tracing configuration propagation, payload calibration, and fallback handling.
Unconfigured temperature is omitted from the child environment, the output-token budget remains independently applied, explicit temperatures continue to propagate, and enriched empty-response diagnostics preserve existing fallback behavior.
Important Files Changed
Reviews (1): Last reviewed commit: "fix(subagent): surface provider error be..." | Re-trigger Greptile
Context used (3)