fix(ai): send temperature only to models that accept it

[NathanTarbert]

Closes #149.

All five Anthropic calls in packages/outpost/ai set a temperature while the model on each is env-overridable. Checked against the current API reference: the sampling parameters — temperature, top_p, top_k — are removed on Fable 5, Mythos 5, Opus 5, Opus 4.8, Opus 4.7 and Sonnet 5, and sending any of them returns 400 invalid_request_error. The defaults today (claude-sonnet-4-6, claude-haiku-4-5-20251001) all still accept one, so nothing is broken right now.

What makes it worth fixing ahead of time is the shape of the failure. Set AI_RESPONSE_MODEL=claude-opus-5, an entirely reasonable upgrade, and every messages.create in the generator 400s, the generator catches it, and "I apologize, but I was unable to generate a response at this time" is what gets posted publicly to every Discord thread and GitHub issue. Classifier, sentiment and confidence each swallow their own 400 and degrade to heuristics. The bot keeps answering; every answer is the apology.

samplingParams() is spread into the request rather than assigning the field, so the key is absent rather than undefined for models that reject it.

Why an allowlist rather than a list of models to avoid

The two directions fail differently and only one fails safely. A deny-list leaves a model released after the file was last touched unlisted, so we send the parameter and reproduce exactly the outage above. An allowlist leaves it out instead, and the request succeeds at the model's default sampling. Omitting is always accepted by the API; sending is not. onOmitted reports the drop so a silent sampling change still leaves a trace.

Two things riding along, because removing the 400 isn't enough on its own

A review pass turned these up, and the first one is the reason this PR is bigger than its title:

• classifier.ts, confidence.ts and sentiment.ts read content[0] only. Every model this change unblocks runs thinking on by default, which puts a thinking block first and the JSON second — so text came out empty, parsing found nothing, and each call degraded to its heuristic with no error at all. Same silent degradation, one layer down. All three now use extractResponseText, as the generator already did after fix(ai): read every text block from the model, and fail loudly on none #223.
• The omission warning is deduped per model. It ran per request, so a single env change meant thousands of identical lines a day burying the errors it sits next to.

The allowlist also gains claude-opus-4-0 and claude-sonnet-4-0 — they accept a temperature and are deprecated but not yet retired. claude-2.x and claude-opus-4-1 are left out on purpose, since both are already retired and unreachable; there's a comment recording that so the list doesn't read as an incomplete model registry.

Tests

14 new. Mutation-checked rather than trusted green:

Mutation	Result
revert classifier.ts to content[0]-only	killed should read past a leading thinking block
remove the warn dedupe	killed warns once per model rather than once per request

Each kills exactly one test. Also pinned: an unknown model omits rather than sends, and {} is asserted as an absent key rather than undefined, since an undefined serializes to null through some JSON paths — a 400 on exactly the models this protects.

ai package 270 → 289. Full repo turbo run test 10/10 packages. tsc --noEmit clean.

Still open after this

• maxClassifierTokens (512) and maxConfidenceTokens (256) are both below a thinking turn, so those budgets want revisiting before anyone actually makes the swap. That's a cost and behaviour decision rather than a mechanical one, so it isn't here.
• The fail-loudly-on-400 half of Roadmap: temperature is sent unconditionally while models are env-overridable — a model bump silently degrades every AI reply to the apology fallback #149. A BadRequestError is a deploy-time bug and should surface distinctly from a rate limit, but that changes the never-crash contract at generator.ts:163 that every caller relies on. Tracked in generator: fail loudly on the unusable-response modes that actually happen #231.

Independent of #239 — branched from main, not from it, so they can merge in either order.

[linear-code]

CPK-8074 Phase 0.2 — Stop a model bump turning every reply into the apology fallback (#149)

GitHub: #149. Fix is written and green — see below.

All five Anthropic calls in packages/outpost/ai sent temperature unconditionally while every model is env-overridable. Verified against the current Anthropic API reference: temperature / top_p / top_k are removed on Fable 5, Mythos 5, Opus 5, Opus 4.8, Opus 4.7 and Sonnet 5 — sending any of them returns 400 invalid_request_error. Current defaults (claude-sonnet-4-6, claude-haiku-4-5-20251001) all still accept it, so nothing is broken today.

The failure is silent and total. AI_RESPONSE_MODEL=claude-opus-5 — a reasonable upgrade — and every messages.create in the generator 400s, the generator catches it, and "I apologize, but I was unable to generate a response at this time" is posted publicly to every Discord thread and GitHub issue. Classifier, sentiment and confidence each swallow their own 400 and degrade to heuristics. The bot keeps answering; every answer is the apology.

Fix: new model-capabilities.ts with supportsTemperature() and samplingParams(), spread into the request so the key is absent rather than undefined. Wired into all five sites — generator.ts ×2, confidence.ts, classifier.ts, sentiment.ts.

Allowlist, not deny-list, deliberately. A deny-list means a model released after the file was last touched is unlisted, so we send the parameter and reproduce the exact outage. An allowlist means an unlisted model just doesn't get it: the request succeeds at the model's default sampling. Omitting is always accepted by the API; sending is not. onOmitted logs the drop so a sampling change doesn't ride along silently.

14 tests, including the two that pin the direction: unknown model → omitted, and {} asserted as an absent key rather than undefined (an undefined serializes to null through some JSON paths, which is a 400 on exactly the models this protects).

ai package: 270 → 289 tests. Full repo: 10/10. Typecheck clean. CopilotKit/outpost#240 open, awaiting review.

Two more fixes rode along, from the review pass

Removing the 400 is not on its own enough to make the swap safe:

• classifier.ts, confidence.ts and sentiment.ts read content[0] only. Every model this unblocks runs thinking on by default, so the thinking block is first and the JSON second — text came out empty, parsing found nothing, and each call degraded to its heuristic with no error. Same silent degradation, one layer down. All three now use extractResponseText, as the generator already did after fix(ai): read every text block from the model, and fail loudly on none #223.
• The omission warning is deduped per model. It ran per request, so one env change meant thousands of identical lines a day burying real errors.

Both mutation-checked: reverting the classifier to content[0]-only, and removing the dedupe, each kill exactly one test.

Allowlist also gained claude-opus-4-0 and claude-sonnet-4-0 (deprecated, retirement TBD, still callable). claude-2.x and claude-opus-4-1 left out on purpose — already retired.

Still open after this

maxClassifierTokens (512) and maxConfidenceTokens (256) are both below a thinking turn, so those budgets want revisiting before anyone actually makes the swap. Cost and behaviour decision rather than a mechanical one, so not in the PR.

Not taken here: the "fail loudly on a 400" half of #149. A BadRequestError is a deploy-time bug and should surface distinctly from a rate limit, but that changes the never-crash contract at generator.ts:163 that every caller relies on. Wants its own change — tracked in #231.

[jerelvelarde]

Reviewed deeply, then put every blocking finding through an adversarial round. The core mechanism is sound and I verified the parts that matter:

• samplingParams returns {} — a genuinely absent key, not undefined — for unlisted models, and all five call sites are converted.
• The allowlist has no dangerous-direction hole: none of the rejecting IDs you name (opus-4-7/4-8, opus-5, sonnet-5, fable-5, mythos-5) is a prefix-match of any of the eight allowlisted stems.
• The warn Set is bounded, because every model originates from env config rather than user input.
• The three thinking-block tests genuinely exercise real code — aimock's buildClaudeTextResponse pushes the thinking block first over real HTTP, so a revert to content[0] really does kill each one.

The allowlist-not-denylist argument is right and the reasoning is the best part of the PR. Two things before I approve.

The PR's central behavioural claim has no test at any call site

Revert any one call site from ...samplingParams(this.model, config.classifierTemperature) back to temperature: config.classifierTemperature and the whole suite stays green. Every existing test constructs its client with an allowlisted model (claude-haiku-4-5-20251001, claude-sonnet-4-6), so the parameter is sent either way and no assertion can tell the difference.

samplingParams itself is unit-tested — that part is fine. What is untested is that the call sites use it. So "send temperature only to models that accept it", the title claim, is pinned nowhere. One test per call site with a rejecting model, asserting the outgoing request has no temperature key, closes it. This is the same class you killed my #224 over, and you were right to, so I am holding this to the same line.

The seam this PR opens is fail-open in sentiment.ts

generator.ts:137 throws on empty extracted text. The three call sites you converted copied the extraction without that guard, and the consequences differ:

• sentiment.ts — an empty response flips a fail-closed DB gate to fail-open, in precisely the env-swap scenario this PR exists to enable. That is the one I would gate on.
• classifier.ts:67-89 (via the parseClassification catch at :208) — with any thinking-default model and maxClassifierTokens: 512, the response is thinking-only, extractResponseText returns '', and the catch returns a hardcoded MEDIUM/OTHER while classify reports degraded: false. It throws away the heuristic verdict it had already computed — and it contradicts the PR body's own claim that these calls "degraded to the heuristic". They do not; they degrade to a constant, and say they did not.

This is the same silent-degradation class you found one layer down and fixed here. It is worth closing at the same time, because the whole point of the change is to make the model swap safe, and right now the swap trades a loud 400 for a quiet wrong answer.

You already note that maxClassifierTokens (512) and maxConfidenceTokens (256) are both below a thinking turn. That is the same finding from the budget side, and it is why the empty-text path is not hypothetical for anyone who actually makes the swap.

Everything else here is good — the per-model warn dedupe, claude-opus-4-0/claude-sonnet-4-0 included as deprecated-but-live with claude-2.x/opus-4-1 deliberately out and a comment saying why, and asserting {} as an absent key rather than undefined because an undefined serializes to null through some JSON paths. That last one is a 400 on exactly the models this protects, and it is the kind of thing that only shows up in production.

Send the call-site tests and the sentiment guard and I will re-verify straight away.

[NathanTarbert]

Thanks Jerel — both blockers were right, and the second one was worse than I'd described. Fixed in 536d69b, CI green.

The call sites had no test

You were right that reverting any one of them left the suite green. Every existing test constructs its client with an allowlisted model, so the parameter went out either way and nothing could tell the difference. samplingParams was unit-tested; that the call sites use it was pinned nowhere.

There's now one test per call site configuring a rejecting model and reading aimock's request journal. One detail worth recording for whoever reads them next: that journal is a normalised view and always carries a temperature key, holding undefined when we sent none — so key-absence isn't assertable there. The call-site tests assert the value; absence on the wire stays pinned in model-capabilities.test.ts.

The empty-response seam

The sentiment.ts one you flagged as the gating item traced further than I'd looked: account-scoring.ts skips its DB write only when degraded is set, so an empty response returning a fabricated NEUTRAL with degraded: false flipped a fail-closed gate to fail-open and persisted a sentiment nobody measured.

And you caught an error in my PR body, not just in the code. I wrote that these calls "degrade to the heuristic". They don't — parseClassification's catch returns a hardcoded MEDIUM/OTHER while classify reports degraded: false, throwing away the heuristic verdict it had already computed. That was a wrong claim rather than an incomplete one, and the body is corrected.

All three sites now treat an empty extraction as a failure, routing to each one's existing catch — the heuristic for the classifier and scorer, the degraded fallback for sentiment. So the change no longer trades a loud 400 for a quiet wrong answer.

ai 289 → 300, repo 10/10, typecheck clean. Four mutations, each killing exactly one test: reverting either call site, dropping either guard.

Still open and noted rather than fixed: maxClassifierTokens (512) and maxConfidenceTokens (256) both sit below a thinking turn, which is the same finding from the budget side.

Ready when you are.

[jerelvelarde]

Re-reviewed against the tree rather than the description. Both blockers are closed, and I checked the claims by execution this time rather than by reading.

Call-site coverage. ai/src is 300 tests, and each of the three sites now has a test that configures claude-opus-5 and asserts body.temperature is undefined, with a positive control asserting it is defined on an allowlisted model. That positive control is the part that makes the pair meaningful — without it the assertion passes for a request that was never sent.

Ran the mutations rather than taking the count:

Mutation	Result
classifier.ts:62 → temperature: config.classifierTemperature	1 failed / 299 passed
confidence.ts:69 → same	1 failed / 299 passed
sentiment.ts:71 → same	1 failed / 299 passed
sentiment.ts empty-extraction guard deleted	1 failed / 299 passed

Four for four, each killing exactly one test. The title claim is now pinned where it is made.

The sentiment.ts seam. The guard throws into the existing catch, which returns degraded: true (:106) — so account-scoring.ts keeps skipping its DB write, and the fail-closed gate stays fail-closed. That was the one I was going to hold on and it is the right shape.

Also worth saying: you corrected a claim in your own PR body rather than the code around it. parseClassification's catch returning a hardcoded MEDIUM/OTHER while reporting degraded: false is not "degrading to the heuristic", and the body now says so. That is the harder half of a fix round.

Approving.

Not blocking, but it is the gating item for anyone who actually flips the env var: maxClassifierTokens (512) and maxConfidenceTokens (256) are both below a thinking turn. You have flagged this twice now from two directions. Removing the 400 makes the swap possible; those two budgets are what make it safe. Worth an issue with a linked owner rather than a third mention in a PR body.