Skip to content

fix(historian): stop sending temperature to reasoning models - #399

Merged
ualtinok merged 2 commits into
cortexkit:masterfrom
tickernelz:fix/historian-temperature-opt-in
Aug 31, 2026
Merged

fix(historian): stop sending temperature to reasoning models#399
ualtinok merged 2 commits into
cortexkit:masterfrom
tickernelz:fix/historian-temperature-opt-in

Conversation

@tickernelz

@tickernelz tickernelz commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Problem

Every historian run fails with no_assistant: pi assistant produced empty text when 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 check magic-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:

Provider / model Response to the injected temperature
OpenAI Responses (gpt-5.x reasoning) 400 Unsupported parameter: temperature
Anthropic (claude-* 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_end with an empty assistant message, and extractFinalAssistant() 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 in v0.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

MAGIC_CONTEXT_HISTORIAN_TEMPERATURE=0.1 \
MAGIC_CONTEXT_HISTORIAN_MAX_OUTPUT_TOKENS=32000 \
pi --print --mode json --no-session --no-skills \
   --extension dist/historian-calibration-extension.js \
   --model <reasoning-model> --thinking max "Reply with exactly: PONG"

Yields stopReason: "error", content: [], and errorMessage: "OpenAI API error (400): Unsupported parameter: temperature". Dropping the temperature env var makes the identical command return normal text.

Root cause

temperature is defaulted to 0.1 in two independent places, so the historian always sends it even when the user never configured one:

  • packages/pi-plugin/src/index.tstemperature: historian?.temperature ?? 0.1
  • packages/pi-plugin/src/pi-historian-runner.tstemperature = 0.1 destructuring default

Removing only the first is not enough; the runner default silently reapplies 0.1.

historian-calibration-extension.ts also 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 the finalStopReason === "error" branch. A provider error whose message carries no text therefore always settles as no_assistant, and the captured finalErrorMessage holding the real 400 is discarded. The user only ever sees "produced empty text".

Changes

  • temperature is now opt-in. When it is not configured, no temperature is sent to the provider at all — restoring the request shape used before v0.41.0. Users whose models accept it can still set historian.temperature.
  • temperature and maxOutputTokens are 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.
  • The no_assistant failure 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.md documents the opt-in behaviour and the reasoning-model constraint.

No config migration is required: existing configs that set historian.temperature explicitly keep their exact behaviour.

Tests

  • 4 new cases in historian-calibration-extension.test.ts covering unset temperature, nested provider shapes, temperature without an output budget, and neither knob set.
  • 1 new regression test in subagent-runner.test.ts asserting the provider error survives into the failure message.
  • Verified with a negative control: reverting only the subagent-runner.ts change fails exactly that one test (86 pass / 1 fail), and passes with it (87 pass).

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Stops the historian from always sending temperature to providers, which caused reasoning models to reject requests with HTTP 400 and surface as a misleading no_assistant error. Temperature is now opt-in: if unconfigured, no temperature is sent at all.

Bug Fixes

  • temperature and maxOutputTokens are now applied independently, so the output-token budget still works on its own.
  • no_assistant failures append the captured provider error so rejected requests are diagnosable.
  • No config migration is needed; existing configs that set historian.temperature behave exactly as before.

Written for commit 55ad3d1. Summary will update on new commits.

Review in cubic

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.

  • Removes the implicit 0.1 temperature from configuration resolution and historian execution.
  • Applies temperature and output-token limits independently across supported provider payload shapes.
  • Includes captured provider errors in empty-assistant failures.
  • Adds regression coverage and documents reasoning-model compatibility.

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

Filename Overview
packages/pi-plugin/src/historian-calibration-extension.ts Makes both calibration knobs independently optional while preserving the recognized top-level and nested provider mappings.
packages/pi-plugin/src/index.ts Stops synthesizing a historian temperature when the shared configuration omits it.
packages/pi-plugin/src/pi-historian-runner.ts Removes the runner-level temperature fallback so omission survives through all historian model attempts.
packages/pi-plugin/src/subagent-runner.ts Preserves captured provider diagnostics when terminal protocol output contains no assistant text without changing fallback eligibility.
packages/pi-plugin/src/historian-calibration-extension.test.ts Covers independent calibration, nested payloads, omitted temperature, and the no-calibration case.
packages/pi-plugin/src/subagent-runner.test.ts Verifies that provider error details survive the empty-assistant failure path.
CONFIGURATION.md Documents temperature as opt-in and warns that reasoning models may reject it.

Reviews (1): Last reviewed commit: "fix(subagent): surface provider error be..." | Re-trigger Greptile

Context used (3)

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).

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

Re-trigger cubic

@tickernelz

Copy link
Copy Markdown
Contributor Author

One more data point that raises the severity: there is no config-level workaround.

The calibration extension bails when finiteNumber() rejects its input (Number.isFinite(parsed) && parsed >= 0), so a negative historian.temperature looks like it should disable the injection. It cannot: agent-overrides.ts declares temperature: z.number().min(0).max(2), and the config loader's schema-recovery path prunes invalid nested leaves. The out-of-range value is silently dropped, historian.temperature returns to undefined, and ?? 0.1 reinstates exactly the value that breaks the request.

I confirmed this end to end: with "temperature": -1 written to the config and the pi session fully restarted, historian runs still failed with the same no_assistant error.

So for anyone running a reasoning model as historian on 0.41.0, the only remedies today are patching the installed dist/historian-calibration-extension.js by hand or switching the historian to a model that accepts temperature. That is what motivated including the diagnostics change in this PR — without the provider error in the failure message, the 400 is effectively invisible and the warning points at the user's model configuration instead.

@magic-alfonso magic-alfonso Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting two focused changes before merge. The Pi diagnosis and implementation are strong; in particular, finding that the empty-assistant branch discarded finalErrorMessage is excellent, and I reproduced the exact OpenAI failure (gpt-5.4: empty content, stopReason:error, Unsupported parameter: temperature) followed by a successful PONG when temperature was absent.

  1. Please pin the two root-cause removals themselves. I restored both historian?.temperature ?? 0.1 in index.ts and temperature = 0.1 in pi-historian-runner.ts under a negative-control marker, and all 93 PR-focused tests still passed. A resolver test should prove omitted → undefined and explicit 0.10.1; a runner test should independently prove an omitted value stays omitted at the spawned attempt. Each test should fail if its corresponding default returns.
  2. Please narrow the common docs/description to the Pi leg. This PR does not make temperature opt-in repository-wide: OpenCode still adds temperature: 0.1 in resolveHistorianAgentOverrides, and Rust still unconditionally sends generation.temperature=0.1 to Broca. The docs sentence “Reasoning models reject the parameter outright” is also too absolute; Anthropic thinking constrains it to 1. “Some reasoning models reject or constrain temperature” is precise.

I do not recommend expanding this contributor PR into the other runtime legs. I will track a maintainer follow-up to (a) remove OpenCode's implicit default while preserving explicit flash calibration and (b) make Rust's Broca generation temperature optional, with absent/explicit request-shape tests across fallback attempts.

Other review points are green: explicit Pi historian.temperature still reaches every calibrated payload shape; max-output and temperature calibration are independent; legitimate empty stop completions remain no_assistant; fallback eligibility and the isolated-retry marker are unchanged; the focused suite is 93/93; typecheck and lint pass; the lockfile, manifests, and fences are untouched. The full Pi suite was 885/886 with only the unrelated 250k-tail p95 timing assertion over budget.

@magic-alfonso

magic-alfonso Bot commented Aug 31, 2026

Copy link
Copy Markdown

Merging now rather than waiting for a revision round: with the #401 connection established (a perpetually failing historian can starve queued drop application on Pi), this regression effectively breaks Pi for reasoning-model historian chains on 0.41.0, and your fix is correct as delivered. We will add the two remaining items ourselves on master — the mutation-pinning tests for the default removals and the docs narrowing — together with the OpenCode and Rust legs, and cut a patch release shortly. Thank you for the diagnosis quality here: the error-shadowing find and the provider table made this a one-read conviction.

@ualtinok
ualtinok merged commit 19546e6 into cortexkit:master Aug 31, 2026
7 checks passed
ualtinok added a commit that referenced this pull request Aug 31, 2026
… tests (#399 follow-up)

Co-authored-by: Alfonso <alfonso-magic-context@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants