fix(providers): replay empty DeepSeek reasoning on tool continuations

[guix4ever]

Summary

Normalize outgoing DeepSeek V4 tool continuation messages so an assistant tool call retains an empty reasoning_content field when the upstream response contained no reasoning tokens.

• Apply the normalization at the LiteLLM Provider boundary for streaming and non-streaming requests.
• Cover direct DeepSeek routing and DeepSeek models routed through OpenRouter.
• Leave persisted history, non-thinking DeepSeek models, and other Provider payloads unchanged.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

Verification

• uv run pytest tests/test_litellm_provider_stream.py tests/test_agent_loop_stream.py tests/test_subagent_manager.py -x -q: 58 passed.
• uv run ruff check .: passed.
• uv run ruff format --check .: 920 files already formatted.
• npx --yes --package=@commitlint/cli@21.1.0 commitlint --from origin/main --to HEAD --config commitlint.config.cjs: passed.
• PYTHONPATH=. uv run python scripts/check_commit_messages.py origin/main..HEAD: passed.
• PYTHONPATH=. uv run python scripts/check_large_files.py origin/main..HEAD: passed.
• git diff --check origin/main...HEAD: passed.
• No user-facing documentation or screenshots are needed for this wire-format fix.
• Relevant tests pass locally
• Relevant lint / type checks pass locally
• User-facing docs or screenshots are updated when needed

Risk

• The behavior change is limited to DeepSeek V4 requests that carry tools and replay assistant tool calls without a reasoning_content key.
• Existing non-empty and explicit null reasoning values remain unchanged, as do non-thinking DeepSeek models and other Providers.
• Rollback is to revert this change; no data migration or persistent format change is involved.
• Security impact considered
• Backward compatibility considered
• Rollback path is clear for risky changes

Related Issues

#297

[gloryfromca]

No blockers; this can merge as far as I am concerned. Two scope notes below, neither of which is a bug today.

The fix matches the contract the reporters actually established in #297: DeepSeek thinking mode rejects only an absent reasoning_content key on the assistant message that issued the pending tool calls -- any value passes, including "" (bisect variant H). setdefault is the right primitive here: a real reasoning string, or an explicit null, both survive untouched, and the key is only added on the wire, so persisted history and the archive keep the shape they had. Injecting it on every assistant-with-tool_calls rather than just the last one is harmless and cheaper to reason about than tracking which one is pending.

What I checked

• The diff, its callers, and _sanitize_messages' ordering. reasoning_content is already in _ALLOWED_MSG_KEYS, so an existing value survives _sanitize_request_messages and setdefault is a no-op on it. _sanitize_empty_content runs first and only touches content, so nothing races.
• Whether reasoning_content: None could reach the sanitizer and defeat setdefault. build_assistant_message (raven/utils/helpers.py:252) only writes the key when the value is not None, and the curator / history-trimmer allowlists copy keys rather than synthesise them -- so the key is either absent or a real string. The Risk section's "explicit null values remain unchanged" holds.
• Model resolution for both id shapes. A bare deepseek-v4-pro misses on original_model but matches on resolved_model after wire_model prefixes it; openrouter/deepseek/deepseek-v4-* matches after _strip_gateway_prefix. Checking both arguments is what makes that work.
• Backward compatibility: gated on model and on tools, so no other provider's payload changes. Confirmed by the negative test over deepseek/deepseek-chat and openai/gpt-4o.
• Tests are not weakened. I reverted litellm_provider.py to origin/main with the new tests in place: 4 of the 6 new cases fail with KeyError on reasoning_content, so they genuinely pin the behaviour rather than passing vacuously. With the fix: uv run pytest tests/test_litellm_provider_stream.py tests/test_agent_loop_stream.py tests/test_subagent_manager.py tests/test_provider_resolution_invariants.py -q -> 219 passed, 30 skipped (the skips are that file's pre-existing per-provider parametrize guards -- "not a gateway" etc. -- not anything this PR touches).
• AGENTS.md: commit grammar and scope are fine, the message is ASCII, the tests went into the existing test_litellm_provider_stream.py with its "Covers:" docstring updated rather than a new file, and no assets are added. The hardcoded model prefix in the provider is consistent with the neighbouring _extra_msg_keys / _ANTHROPIC_EXTRA_KEYS precedent, so I am not asking for it to move into the registry.

One thing I could not verify: the OpenRouter row is inferred rather than measured. capabilities.py is explicit that its own tables list only what was tested against a live key. Sending reasoning_content to OpenRouter is very likely inert, but if you have a key handy it is worth one real call before merging.

[gloryfromca]

Non-blocking: the bool(tools) half of this gate is narrower than the rule #297 actually established, and it buys nothing.

The reporter's bisect variant B was "same payload, tools param removed" -> still a 400. DeepSeek's own doc says a request that once carried tools must replay reasoning_content in all subsequent requests. What decides the rejection is the message shape -- an assistant with pending tool_calls and no reasoning_content key, followed by a tool result at the end of the array -- not whether this particular request declares tools.

I could not find a live Raven path that this breaks today, which is why it is not a blocker. The one tools=None call site that replays tool-bearing history is _synthesize_final_on_exhaustion (raven/agent/loop/main.py:2034), and it appends a user message after the tool result, which is bisect variant G -> accepted. No TokenWise strategy empties the tool list either.

So the gate is safe now and wrong in principle: the day someone adds a tools-less continuation that ends on a tool result, this comes back as a 400 that looks unrelated to this PR. Dropping bool(tools) and costs one dict key on messages that already carry tool_calls. If you keep it, a line saying it is deliberate would help the next reader, since the issue explicitly ruled tools out as the trigger.

[gloryfromca]

Non-blocking scope note: this matches direct DeepSeek and OpenRouter, and cannot match AiHubMix -- which is in Raven's own onboarding catalog and fronts DeepSeek's official API.

AiHubMix's spec sets strip_model_prefix=True and via_driver="openai" (raven/providers/registry.py:254), so a stored aihubmix/deepseek-v4-pro resolves to openai/deepseek-v4-pro, and _strip_gateway_prefix -- which strips the gateway's model_prefix, i.e. openai/ -- leaves deepseek-v4-pro. Neither argument starts with deepseek/, so the replay never fires. The same applies to an OpenAI-compatible provider pointed at api.deepseek.com, where the id is stored under the custom provider's prefix.

That is not a regression -- those routes are broken today too -- and I have not measured whether AiHubMix relays DeepSeek's 400 verbatim, so I am not asking you to widen the match blind. Just worth knowing the issue stays open for those users. If you want the cheap version, matching "deepseek-v4-" in upstream_model instead of startswith("deepseek/deepseek-v4-") would cover them, at the cost of also firing for self-hosted deepseek-ai/DeepSeek-V4-* on SiliconFlow etc., where the extra key is inert anyway.