Skip to content

fix(bugs): prompt-cache stability, image tool_results, Anthropic-shaped field passthrough - #7

Open
lydiym wants to merge 28 commits into
mainfrom
fix/bugs
Open

fix(bugs): prompt-cache stability, image tool_results, Anthropic-shaped field passthrough#7
lydiym wants to merge 28 commits into
mainfrom
fix/bugs

Conversation

@lydiym

@lydiym lydiym commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Summary

26 commits accumulated on fix/bugs since main@a49d6b4 — small fixes uncovered during Claude Code integration testing. Branch is reviewable as one PR; the changes are independent and additive.

What's fixed

  • Preserve image content in tool_result blocks — Claude Code's Read tool returns images inside tool_result.content. Previously flattened to stringified JSON (~161 KB) and shipped to upstream as text, breaking multimodal tools. Now emits OpenAI image_url parts via the existing convert_image_block helper (server.py:_convert_tool_result_to_parts). Text-only tool results still flow as strings (no wire-format regression).

  • Round-trip reasoning_content between turns_convert_assistant_message now captures type=="thinking" blocks and forwards them as reasoning_content on outgoing OpenAI messages. Required for reasoning models that need their own prior-turn reasoning preserved — without this, each turn loses the chain and the model re-derives from scratch. Whitelisted in sanitize_messages_for_openai.

  • output_config.effortreasoning_effort — Claude Code sends Anthropic's output_config namespace; OpenAI exposes the same axis at the top level for o-series. Translated verbatim, with empty-string guard so effort="" doesn't 422 on the upstream enum.

  • thinking pass-through to Anthropic-compatible backendsrequest.thinking lands in extra_body.thinking (body-only, not top-level — openai SDK rejects unknown kwargs). Whitelisted so litellm forwards it. Required for backends that honour Anthropic's thinking field verbatim.

  • Hoist top-level system before in-band reminders — Anthropic spec only allows system at the top level, but Claude Code 2.1.154+ started embedding system reminders inline. Now combined into one system message at the start with deterministic order (top-level → in-band). Wire-level change with cache-miss fallout for primed deployments; documented in CLAUDE.md.

  • Cache-stable OpenAI wire form for local-inference prefix caching — replaced Cyrillic fixture in unicode test with CJK, picked tool_use stop_reason when upstream omits finish_reason, fixed several prompt-remap edge cases (task-tools reminder variant, WARNING log when something is stripped, show match count).

  • PROXY_DEBUG_INBOUND_DUMP writes the raw Anthropic request to disk before Pydantic validation (pretty-printed JSON). Mirrors the existing PROXY_DEBUG_CACHE_DUMP infrastructure with same-second sequence disambiguation. Best-effort: a client disconnect mid-upload no longer 500s the request.

  • Misc: _parse_tool_result_content no longer crashes when content is a list; inbound body dump middleware honours its best-effort docstring contract; null text in tool_result text blocks collapses to "" instead of reaching "\n".join(...) and crashing.

What's added

  • Anthropic-shaped field translations — documented in README. Covers thinking, output_config.effort, metadata, context_management, plus how body-only keys (thinking) reach the JSON body via extra_body even though the openai SDK rejects them at signature time.
  • Post-/code-review findings triage — codified in CLAUDE.md so every review pass does a scope check against git log main..HEAD, re-verifies file:line anchors, and classifies findings as fix in branch / pre-existing → file ticket / stale → discard before acting.

Testing

uv run python tests.py — 233/233 unit tests pass (was 207 at main@a49d6b4; new tests cover reasoning_content round-trip, tool_result image variants, output_config effort edge cases, debug-dump invariants).

uv run ruff check / uv run ruff format --check — clean.

lydiym and others added 5 commits August 18, 2026 23:35
…ching

Anthropic's wire format leaks into the OpenAI payload in six ways that
produce different tokens than a reference OpenAI client would, busting
llama-server's prefix cache on every tool-using turn.

- _convert_assistant_message: tool_call.arguments now uses
  separators=(",", ":"), sort_keys=True, ensure_ascii=False.
  Default json.dumps inserts whitespace that survives BPE differently.
- tool_call.id and tool_call_id rewritten from Anthropic's ``toolu_*``
  to OpenAI's ``call_*`` via a per-request id_map. Many chat templates
  render the id in the prompt.
- _convert_tool_definitions: strips Anthropic-only ``cache_control``
  and ``strict`` keys from input_schema before forwarding as
  OpenAI ``parameters``.
- sanitize_messages_for_openai: skip the ``"..."`` placeholder for
  role="tool" — empty tool result bodies must pass through verbatim.
- api_key masked in the debug kwargs dump.

Adds 14 prefix-equivalence tests in tests.py that fail on the prior
behaviour and pin the canonical wire form: compact JSON, sorted keys,
UTF-8 unicode passthrough, call_* ids, cache_control stripped, no
Anthropic-specific leakage, byte-stable across repeated conversions.

182/182 tests pass.

Co-Authored-By: Claude <noreply@anthropic.com>
No behaviour change — the test still asserts UTF-8 passthrough and
non-escaped multi-byte characters. Cyrillic has no place in the source
tree.

Co-Authored-By: Claude <noreply@anthropic.com>
Brings in prompt_remap feature (commit 9546bfc) which is already on main.
When the upstream closes the stream mid-tool-call without sending a
finish_reason, the epilogue hardcoded end_turn. The Anthropic SDK treats
end_turn as 'no pending work' and never asks the user for tool results,
so the just-emitted tool_use block was silently dropped.

Track tool_use_emitted in _StreamState, set when a tool_use block opens;
the epilogue picks tool_use when true, end_turn otherwise. The in-flight
block is still closed via tracker.close() before message_delta (Anthropic
SSE requires content_block_stop before message_delta).

Regression test: test_streaming_no_finish_reason_with_tool_call_uses_tool_use_stop.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude Code injects two variants of the same cache-busting reminder:
the TodoWrite variant ("The TodoWrite tool hasn't been used recently...")
and the task-tools variant ("The task tools haven't been used recently..."
mentioning TaskCreate/TaskUpdate). Collapse them into one regex with an
alternation instead of two [[prompt_remap]] entries.

Adds test_prompt_remap_strip_task_tools_reminder and
test_prompt_remap_canonical_across_reminder_variants — the latter
asserts all three states (no reminder / TodoWrite reminder / task-tools
reminder) produce byte-identical outgoing system prompts.

Co-Authored-By: Claude <noreply@anthropic.com>
- tests.py:751 — split composite unicode assertion
- tests.py:971 — blank line before nested def
- tests.py:1037 — drop stray f-prefix
- tests.py:1127 — use falsy check instead of == ""
- server.py:567 — extract _apply_known_section helper so _load_config
  drops below C901 complexity threshold

Co-Authored-By: Claude <noreply@anthropic.com>
@lydiym
lydiym force-pushed the fix/bugs branch 3 times, most recently from 78fbf1f to b3617f8 Compare August 20, 2026 19:28
Move the prefix-equivalence / fuzzy-match cache diagnostic out of
debug-cache-busting.patch into server.py proper, gated by env var.

- PROXY_DEBUG_CACHE_DUMP=1 enables the matcher; default off so the
  hot path is a single os.environ.get + str-to-bool (no overhead)
- Artifacts land in $cwd/.claude-code-proxy/prompts/ instead of
  /tmp/proxy-cache-debug — keeps debug output alongside the project
- Singleton via functools.cache so the matcher is built lazily once
- Adds two unit tests: flag-off is a no-op (no directory created),
  flag-on writes prefix_hit artifacts on a conversation extension
- README + .env.example document the knob

Co-Authored-By: Claude <noreply@anthropic.com>
lydiym and others added 7 commits August 20, 2026 22:35
tc["id"] = "<id>" and msg["tool_call_id"] = "<id>" mutated the
caller's payload dict in-place. Anything reading tool_call ids after
_debug_dump_outgoing_payload returned saw "<id>" placeholders.

Fix: build copies via {**msg} / {**tc} and return a new tools list.
Adds test_debug_cache_dump_does_not_mutate_payload as a regression
guard — observes twice (second call hits history) and asserts the
caller's payload still matches its pre-call snapshot.

Co-Authored-By: Claude <noreply@anthropic.com>
Two bugs surfaced from the live logs:

1. Oldest-first prefix scan reported (msgs 30 vs N) forever — the
   first request in the window was always the smallest, so a
   monotonically-growing conversation always matched against it.
   Reverse the iteration so logs show incremental growth
   (msgs N-1 vs N) — the matched prior is the immediately-prior
   request, which is what operators actually want to see.

2. Stamp collisions on prefix_hit: every prefix_hit has score=1.00,
   and same-second same-pid requests overwrote each other's
   artifacts (the new test for incremental growth caught this — only
   1 diff survived 2 hits).
   Append a monotonic seq to the stamp via itertools.count; no
   global-statement lint, no clock skew.

Adds test_debug_cache_dump_matches_immediately_prior_request.

Co-Authored-By: Claude <noreply@anthropic.com>
The diff was dominated by random per-request tool_call_id changes —
every Claude Code request that included tool_use blocks regenerated
the id, so the diff showed ~30 id-flip blocks per artifact instead of
the actual structural change (which was a 1-line tweak in the system
prompt).

_matcher's _strip_ids already scrubbed ids for the comparison
canonical form, but _record was writing the raw payloads. Re-strip
in _record before writing JSON + diff so the artifacts and the diff
both reflect structure, not random id churn.

Co-Authored-By: Claude <noreply@anthropic.com>
Pretty-printed JSON collapses multi-KB system prompts onto a single
line, so unified diff shows just one giant +/- line and doesn't reveal
what actually changed. Compare -new.json/-old.json with a side-by-side
viewer or a script that splits on \n\n instead.

Co-Authored-By: Claude <noreply@anthropic.com>
'stripped 1269 chars via 1 entry' was misleading — it sounded like one
reminder, but one configured regex pattern can match N times via
subn(). With Claude Code accumulating reminder copies across turns,
the gap between "1 match" and "3 matches" matters for spotting that
quirk.

Co-Authored-By: Claude <noreply@anthropic.com>
Anthropic image messages arrive with content as a list of content blocks
([{type: image, ...}, {type: text, ...}]), not a string. The previous
`msg.get("content") in {None, ""}` set membership check raised
TypeError: unhashable type: 'list'.

Switch to `not msg.get("content")` so list/dict content is preserved
as-is, while None/""/[] still get coerced to the "..." placeholder that
OpenAI requires.
lydiym and others added 10 commits August 21, 2026 00:04
Claude Code's Read tool returns images as structured blocks inside
tool_result.content ([{"type": "image", "source": {...}}]). The old
_parse_tool_result_content routed non-text blocks through json.dumps,
flattening 118KB of binary data into 161KB of stringified JSON sent to
the upstream model as text. Multimodal LLMs see the garbage as text and
hallucinate plausible descriptions instead of describing the image.

Add _convert_tool_result_to_parts which detects image blocks and emits
OpenAI image_url parts via the existing convert_image_block. Returns
str for text-only content (preserves wire format — three existing tests
assert string tool content) and list[dict] when any image is present.

The orphan tool_result branch (truncated tool_use) keeps the existing
prose fallback: a ghost id has no matching assistant turn, so emitting
role=tool would dangle. Orphan images continue to flatten to prose,
matching the existing comment about truncated context.

Tests cover image-only, mixed text+image, multiple images, url-source
images, empty list content, text-only regression, sanitize passthrough,
full pipeline tool_use → tool_result → assistant, and orphan prose
folding.
Regression guard for the malformed-Bash-before-Read pattern observed
against MiniMax M3: when parallel tool calls arrive and the first one's
argument stream ends mid-JSON (literally "{"), the proxy must forward
exactly what upstream sent — no synthetic closing brace, no drop, no
cross-index merge. Claude Code then surfaces the malformed input via
__unparsedToolInput and the model retries with a clean call.

This test pins the behaviour so a future refactor that "helpfully"
coalesces parallel tool-call fragments can't silently corrupt the wire
shape.
PROXY_DEBUG_INBOUND_DUMP=true writes each raw Anthropic request to
.cwd/.claude-code-proxy/anthropic-prompts/<ts>-<pid>-inbound.json
before convert_anthropic_to_litellm runs.

Mirrors the existing PROXY_DEBUG_CACHE_DUMP (outbound, post-conversion
OpenAI shape) so operators can diff what the client sent vs what
upstream received. Useful for spotting mid-conversation reminder
injections by Claude Code (top-level system field, in-band role=system
messages, user-content <system-reminder> tags).

Best-effort: any write failure is logged at DEBUG and swallowed.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude Code injects role=system messages inline (in-band) into the
messages array. The proxy squashes them all into one messages[0] for
upstream OpenAI, but the previous order was [in-band ... + top-level]
— which inverts the natural Anthropic chronology where the top-level
system field is the agent identity and conceptually precedes the
messages array.

Flip to [top-level + in-band ...] so the merged messages[0] mirrors
how Claude Code shaped the request: agent identity first, reminders
after.

Also update test_system_role_message_in_messages_array_is_hoisted
which previously asserted the inverse order.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude Code sends two Anthropic-native fields that don't map 1:1 to
OpenAI Chat Completions:

- output_config.effort → top-level reasoning_effort (OpenAI's reasoning
  axis for o-series etc.)
- thinking → extra_body.thinking (body-only, since the openai SDK
  rejects unknown top-level kwargs at signature time; reaches JSON
  body via litellm's cascade lift in openai_like/chat/handler.py:258)

Both fold into _apply_merged_extra_body alongside the existing
[tier].extra_body merge chain; config wins per leaf.

Adds _BODY_ONLY_KEYS as the whitelist of keys that stay in extra_body
instead of being lifted to top-level kwargs. documented in README
under "Anthropic-shaped field translations".

Tests:
- 3 tests rewritten to assert thinking lives in extra_body (matches
  _BODY_ONLY_KEYS semantics)
- test_extra_body_deep_merge_with_client updated similarly

Co-Authored-By: Claude <noreply@anthropic.com>
Anthropic thinking blocks (which we synthesised from upstream
reasoning_content on the response side) were silently dropped when
Claude Code sent them back as part of a multi-turn conversation —
_convert_assistant_message only handled type='text' and type='tool_use',
so prior-turn reasoning never reached the model on the next turn.

For models trained on reasoning chains (e.g. qwen3.8 with
preserve_thinking=true) this breaks continuity: the model re-derives
its reasoning from scratch every turn instead of building on the prior
chain. Verified live against MiniMax-M3: outbound dumps show the
reasoning_content field absent before the fix, present after.

Fix:
- Capture type='thinking' blocks in _convert_assistant_message and
  attach their content as reasoning_content on the outgoing OpenAI
  assistant message dict. The openai SDK's openapi_dumps is plain
  json.dumps (TypedDict has no runtime validation), so the extra key
  reaches the wire body untouched. litellm's vertex_ai/gemini handler
  reads reasoning_content back at transformation.py:826, confirming
  this is the canonical round-trip field.
- Add reasoning_content to the sanitizer's allowed set; without this
  the round-trip is silent — _convert_assistant_message attaches the
  field but sanitize_messages_for_openai strips it before the wire.
- Multiple thinking blocks per turn concatenate with blank line so the
  model sees one continuous chain.

Tests: 4 new (round-trip, multi-block, no-thinking→no-field,
sanitizer preservation). 230/230 pass.

Co-Authored-By: Claude <noreply@anthropic.com>
Without the guard, a client disconnect or socket error mid-upload raises
out of the middleware and the request 500s — the docstring explicitly
promises 'a failed read or write must never break the request'.

Co-Authored-By: Claude <noreply@anthropic.com>
Empty string is not a valid reasoning-effort enum value for any backend
(Anthropic, OpenAI o-series, Moonshot kimi); forwarding it produces a
422. Truthy guard drops both None and empty, matching the proxy's rule
that client-sent values only flow through when meaningful.

Co-Authored-By: Claude <noreply@anthropic.com>
After every /code-review invocation: scope each finding against the
branch's commits, re-verify file:line anchors against current HEAD,
classify as fix-in-branch / pre-existing / stale before acting.

Co-Authored-By: Claude <noreply@anthropic.com>
…shing

_build_tool_result_part used dict.get('text', '') which only fires when
the key is missing, not when it's null. A tool_result with
[{"type":"text","text":null}] then reached
_tool_result_parts_from_list's "\n".join(...) and raised TypeError —
proxy crash on malformed input.

Co-Authored-By: Claude <noreply@anthropic.com>
lydiym and others added 2 commits August 26, 2026 13:03
- test_tool_result_text_block_with_null_text_does_not_crash guards the
  fix above.
- test_inbound_dump_path_disambiguates_same_second_writes pins the seq
  counter behaviour.
- Existing inbound-dump tests updated their glob from *-inbound.json to
  *-inbound-*.json to match the new suffix.

Co-Authored-By: Claude <noreply@anthropic.com>
introduced by d8f953e on fix/bugs — the spread was added so body-only
keys (thinking) could reach the JSON body via litellm's openai_like
handler, but the spread didn't filter _PROTECTED_KEYS. Existing
test_extra_body_protected_keys_blocked only checks top-level kwargs
and misses the bypass.

Co-Authored-By: Claude <noreply@anthropic.com>
@lydiym lydiym changed the title fix(server): cache-stable OpenAI wire form for llama-server prefix ca… fix(bugs): prompt-cache stability, image tool_results, Anthropic-shaped field passthrough Aug 26, 2026
lydiym and others added 2 commits August 26, 2026 14:32
CONFIG was inferred as Any | dict[Unknown, Unknown] | list[Unknown]
because ty saw two assignments: one from _load_config (returns
dict[str, Any]) and one from the try/except fallback (a literal dict).
Union inference picked the literal shape, leaving every CONFIG.get(...)
and CONFIG["x"]["y"] unresolved. An explicit annotation pins the
type. Cuts ty diagnostics from 32 to 3.

Co-Authored-By: Claude <noreply@anthropic.com>
…tation

- 3948/4572: removed now-unused ty: ignore[invalid-assignment] comments
  on CONFIG-None tests (annotation made those assignments valid).
- 3411/3417: pinned ty ignores on the streaming-events parser where
  current_name narrowing via control flow is correct but ty doesn't
  follow the and-current_name guard.
- 2632/2633: dropped the bogus ty: ignore[list-item] rule (ty doesn't
  have it), added invalid-argument-type on the list-literal line for
  the intentional non-string match input.

Co-Authored-By: Claude <noreply@anthropic.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.

1 participant