fix(core): strip assistant-turn prefill and fix thinking.display on 4.6+ models - #8040

Local verification report — maintainer review

I built and tested this branch locally (isolated worktree at b9cf3b0, merge-base cc617e6) and ran a mutation matrix against the new tests. Both bugs are real, the fix is correctly targeted, and the design holds up on the edge cases I could construct. I found two test-coverage gaps and one open question that needs a live API check.

Recommendation: merge after the one-character fixture fix below. The other two items are optional / informational.

What I ran

The 197/198 "pre-existing User-Agent failure" mentioned in the PR description does not reproduce here — I get a clean 198/198. Worth double-checking whether that was a local artifact.

Premises check

Both claims match Anthropic's current documented behaviour:

• Prefill — assistant-turn prefill is rejected from 4.6 onward (Opus 4.6 and Sonnet 4.6 included), and on every 5.x family. Gating on the existing modelSupportsAdaptiveThinking() (major > 4 || (major === 4 && minor >= 6)) is therefore the correct cutoff, not an approximation. Good call reusing it rather than adding a second version predicate.
• display — thinking.display does default to omitted on Opus 4.7+/Sonnet 5/Fable 5, and was summarized on 4.6. So the diagnosis and the chosen value are right.

Correctness: the design holds

My main worry reading the diff was phase 2 appending a text-only "Continue." turn after an assistant turn carrying an unanswered tool_use — Anthropic rejects that with tool_use ids were found without tool_result blocks immediately after. It cannot happen: cleanOrphanedToolCalls runs earlier in the pipeline and drops the orphaned block (and the message, if that leaves it empty). I verified this against the real converter rather than by reading:

Also checked and fine: the two phases compose correctly (empty turn popped, then the synthetic turn appended); a non-text block such as an inline image is correctly treated as real content; and cache_control lands on the deterministic "Continue." text, which makes the cached prefix strictly longer rather than breaking caching. The empty-messages[] result for a degenerate history is identical with the option off — pre-existing, not a regression.

Mutation matrix

The display fix is well covered — dropping the field (M8) or flipping it back to omitted (M9) both fail the suite, and they do so through the real request-building path. Phase 2 of the prefill fix is well covered too (M2–M5, M7).

Gap 1 — the drop-empty test is vacuous (please fix)

converter.test.ts:2103 (drops a trailing empty assistant message…) uses { role: 'model', parts: [{ text: '' }] }. processContent only pushes a text block when part.text is truthy, and never emits a message with zero blocks — so that fixture produces no assistant message at all, and phase 1 has nothing to drop. The identical assertion passes with stripTrailingAssistantPrefill off:

One-character fix, verified: change the fixture to text: ' ' (whitespace-only). That does produce an assistant message, so it actually reaches phase 1 — and it's the case isEmptyAssistantMessage's .trim() exists for. I re-ran the matrix with the patched fixture: M1 and M6 both go from SURVIVED to KILLED.

Gap 2 — no generator-level coverage of the prefill wiring (optional)

stripTrailingAssistantPrefill appears in anthropicContentGenerator.ts (L692, L724) but in no generator test. Consequently:

• M11 — hardcoding stripTrailingAssistantPrefill = false (feature dead on every model) → suite still green.
• M10 — hardcoding it to true (4.6+ gate removed, prefill stripped on pre-4.6 models too) → suite still green.

The 5 new converter tests all pass the option in by hand, so nothing pins that the generator actually turns it on, nor that the version gate discriminates. Contrast the display change, which is pinned through the real path — that's why M8/M9 die and M10/M11 don't. Two assertions on anthropicState.lastCreateArgs (trailing assistant stripped on claude-opus-4-6, preserved on claude-opus-4-5) would close it and would also lock in the gate you deliberately reused.

Note: M12 is an equivalent mutant, not a gap

Collapsing phase 1's while to a single pop keeps the suite green — correctly. mergeConsecutiveAssistantMessages runs before this pass and guarantees no two adjacent assistant messages, so the loop can never iterate twice. The while is harmless defensive generality; an if would behave identically. No action needed.

Open question — is display accepted on Opus 4.6 / Sonnet 4.6?

This is the one thing I could not verify: this machine has no Anthropic credentials, so I could not re-run your live-API checks.

The concern is narrow but worth one curl before merge. display became the knob you need to set at Opus 4.7, when the default changed. This PR sets it unconditionally on the { type: 'adaptive' } shape, which modelSupportsAdaptiveThinking() also matches for claude-opus-4-6, claude-sonnet-4-6, claude-haiku-4-6. If 4.6 rejects the unknown field rather than ignoring it, this turns a silent-empty-thinking bug into a hard 400 on every 4.6 request. Your PR description covers 4.7/4.8/5.x live but doesn't mention testing a 4.6 model with display set.

Two things make this worth an explicit check rather than an assumption:

1. The pinned @anthropic-ai/sdk is 0.36.3, which predates adaptive thinking entirely — it has no ThinkingConfigAdaptive type. The repo hand-rolls AnthropicThinkingParam, so nothing client-side validates this; the server is the only check.
2. display is not gated on isAnthropicNativeBaseUrl, so it also goes to Anthropic-compatible third-party gateways — and anthropicContentGenerator.test.ts:2649-2654 now explicitly pins that (internal-proxy.example → display: 'summarized'). A stricter proxy that validates the thinking object could reject the field. Note the file already has precedent for baseURL-gating a newer wire feature (dropUnsignedAssistantThinking), so this is a deliberate divergence worth a sentence in the PR body either way.

One request settles it:

curl -s -X POST https://api.anthropic.com/v1/messages \
  -H content-type:application/json -H "x-api-key: $ANTHROPIC_API_KEY" \
  -H "anthropic-version: 2023-06-01" \
  -d '{"model":"claude-sonnet-4-6","max_tokens":16,
       "thinking":{"type":"adaptive","display":"summarized"},
       "messages":[{"role":"user","content":"hi"}]}'

200 → ship as-is. 400 → the display line needs the same version gate the budget_tokens path already has (modelRejectsManualThinking() is 4.7+ and would be the natural predicate).

Summary

Nice, well-scoped fix otherwise — reusing the existing version predicate instead of duplicating it, and preserving model content via the synthetic turn rather than discarding it, are both the right calls.

_{Verified locally by the maintainer on macOS (darwin 24.6.0), isolated git worktree at b9cf3b0. Evidence images are rendered from actual command output and pinned at pr-assets/pr8040-anthropic-prefill-display. Live Anthropic API behaviour was not re-tested (no credentials on this machine).}

Thanks for the thorough review — all three items addressed:

Vacuous drop-empty fixture: fixed exactly as suggested (text: '' → text: ' '). Confirmed by hand that this kills M1 and M6 — reverted the fixture, watched the test pass with stripTrailingAssistantPrefill off, restored it.

Generator-level prefill wiring coverage: added the two assertions you outlined (trailing turn stripped + "Continue." appended on claude-opus-4-6, left untouched on claude-opus-4-5). Verified against M10/M11 by hand: hardcoding the gate to false or true in anthropicContentGenerator.ts now fails one of the two new tests each time; reverting the mutation passes both again.

display on 4.6 models: I don't have direct api.anthropic.com credentials in this environment, but I do have access to our org's Vertex-backed Anthropic proxy, which I'd expect to be at least as strict as the native API on an unrecognized-field question like this one. Ran your exact payload shape against it:

claude-sonnet-4.6 + {"type":"adaptive","display":"summarized"} → HTTP 200
claude-opus-4.6   + {"type":"adaptive","display":"summarized"} → HTTP 200

(claude-haiku-4.6 isn't configured on this particular proxy — 400 for "invalid model name," unrelated to the field itself — so I can't add a data point there.) Both requests returned a normal thinking + text response with display silently accepted, not a 400. Not a substitute for your native-API curl if someone here can run it, but it's a second independent backend saying the field doesn't break 4.6, for whatever that's worth.

Full mutation matrix re-run: 198/198 passing, same as before.

Local verification report

I built this branch locally and ran it against a real HTTP endpoint rather than only reading the diff. Both bugs reproduce on main and are fixed by this PR. No blocking issues found. Details, including two pre-existing gaps this PR does not cover, are below.

Verified at f854786230, merge-base cc617e6707.

How I tested

The PR's own unit tests mock @anthropic-ai/sdk, so they prove the request body is built correctly but never exercise the wire. I wrote a separate harness that does not mock the SDK: a local HTTP server stands in for the Messages API, and the real AnthropicContentGenerator + real @anthropic-ai/sdk talk to it over real HTTP. The stub enforces Anthropic's documented behaviour:

1. Models from the 4.6 generation onward reject a request whose final message has role: "assistant" with a hard 400.
2. Adaptive thinking returns thinking blocks with empty text unless thinking.display === "summarized".

The model→behaviour membership table in the stub is written out explicitly rather than derived from parseClaudeModelVersion(), so the oracle shares no logic with the code under test.

I want to be upfront: I have no Anthropic credentials on this machine, so this is a faithful stub, not the live API. The two rules it encodes match Anthropic's published behaviour — assistant prefill 400s on Opus 4.6/4.7/4.8/5, Sonnet 4.6/5 and Fable 5 but not on Haiku 4.5; and display defaults to summarized on Opus 4.6 / Sonnet 4.6 but silently to omitted on Opus 4.7+ and every 5.x family. That matches the premise in the PR description and #8039.

Results

Negative control. Reverting only the two production files to the merge-base while keeping my identical test flips 3 of 4 cases red; restoring turns them green. So the test discriminates the fix rather than just observing green.

The claude-haiku-4-5 row is the one I care most about: it is the discriminator showing the modelSupportsAdaptiveThinking() gate is generation-scoped, not a blanket rewrite of every request.

Adversarial edge cases

I then hardened the stub to also enforce messages non-empty, first message must be user, and every tool_use answered — so that a "fix" which merely trades one 400 for a different 400 would be caught. I also drove the streaming path, since that is what the TUI actually uses.

Two things worth calling out:

• Streaming is covered. generateContent and generateContentStream share a single buildRequest(), so the fix applies to both. I confirmed end-to-end: on main the streaming call 400s; on this branch it streams and carries thought: true blocks through.
• A trailing turn with an unanswered tool call is safe. cleanOrphanedToolCalls() runs before the new pass, so the orphaned tool_use is removed first and the request never ends up with a tool_use followed by a synthetic text-only user turn. This was my main correctness worry going in; it does not materialise.

Two pre-existing gaps (not regressions, not blockers)

Both of these fail on main and on this branch, so they are not caused by the PR. Flagging them as possible follow-up since they are adjacent:

• A history that starts on a model turn (leading user turn trimmed away) still 400s with "first message must use the user role". This PR fixes the tail of the conversation; the head has the symmetric problem, and the same context-trimming scenario cited in the PR description could produce it.
• A history consisting solely of an empty model turn now yields an empty messages array (400 "at least one message is required") instead of the previous 400. Same outcome, different message.

Checks

Minor notes

1. The branch is behind main (based on cc617e6707, current main is 01afcb0bba). Worth rebasing before merge so CI runs against current main.
2. The PR description's test note is now stale. It says 197/198 with a pre-existing User-Agent failure; at this SHA I get a clean 200/200 across the whole directory, so that caveat can be dropped.
3. "Continue." is a hardcoded English string. It is injected as a user turn that steers the model, and we have general.outputLanguage elsewhere. In a Chinese session this may nudge the reply toward English. Low severity, but a language-neutral marker or one derived from the configured output language might be tidier.
4. display? is added to the { type: 'enabled'; budget_tokens } arm of AnthropicThinkingParam but is never set on that path (correctly — the test pins this). Harmless dead type surface; not worth changing.

None of these block merge. My take: rebase, drop the stale test note from the description, and this is good to go.

🤖 Generated with Claude Code — Claude Opus 5 (1M context)

@qwen-code /triage