Skip to content

fix(llm): route DeepSeek, Gemini, and OpenRouter through dedicated rig-core clients (#3201, #3225)#3326

Merged
ilblackdragon merged 4 commits into
mainfrom
fix/3225-3201-route-to-dedicated-rig-providers
May 7, 2026
Merged

fix(llm): route DeepSeek, Gemini, and OpenRouter through dedicated rig-core clients (#3201, #3225)#3326
ilblackdragon merged 4 commits into
mainfrom
fix/3225-3201-route-to-dedicated-rig-providers

Conversation

@ilblackdragon
Copy link
Copy Markdown
Member

@ilblackdragon ilblackdragon commented May 7, 2026

Summary

Fixes #3201 and #3225 — both were caused by wrong provider routing in providers.json. DeepSeek, Gemini, and OpenRouter were configured with protocol: "open_ai_completions", dispatching them through rig-core's generic OpenAI client. That client silently strips the fields these APIs require to be echoed back on the next turn:

rig-core has dedicated deepseek::Client, gemini::Client, and openrouter::Client implementations that handle the round-trip natively. The fix is to route to them.

Change

  • Add ProviderProtocol::DeepSeek, Gemini, and OpenRouter variants
  • Add three factory functions (mirror the existing create_anthropic_from_registry pattern, wrap the dedicated rig-core client in our existing RigAdapter); the OpenRouter factory preserves attribution headers (HTTP-Referer, X-Title)
  • Update providers.json to use the new protocols and clear default_base_url so the dedicated clients use their built-in endpoints

No agent-loop changes, no new HTTP code, no changes to other providers (OpenAI, OpenAI-compat, Tinfoil, Groq, Together, Mistral, Cerebras, Fireworks, etc. all unchanged).

Why this approach (and the previous PR #3325 wasn't)

PR #3325 built a 731-line custom OpenAI-compat client and plumbed reasoning_content/signature through 31 files. After verifying rig-core's source, all three providers' dedicated clients already do the round-trip correctly:

  • rig-core/src/providers/deepseek.rs:500-527 — captures reasoning_content, writes back on next request
  • rig-core/src/providers/gemini/completion.rs:1016thought_signature: tool_call.signature round-trip
  • rig-core/src/providers/openrouter/completion.rs:290-308reasoning_details::Encrypted carries signatures across turns

This PR is ~150 lines and changes nothing about how OpenAI / OpenAI-compat / Tinfoil / Groq / Together / etc. behave.

What this fixes vs. doesn't

Fixes (the bug): API-level round-trip — multi-turn tool calling stops failing for thinking-mode models on these three providers.

Does not (filed as a separate follow-up issue (#3327)):

  • Surface reasoning content in the chat UI
  • Persist reasoning in the thread DB
  • Show reasoning in a debug panel
  • Stream reasoning chunks via SSE while the model is "thinking"

These are independently valuable but require schema migrations, SSE event additions, and UI work.

Regression coverage

  • reasoning_aware_providers_use_dedicated_protocol_not_openai_compat — locks the protocol routing in providers.json so a future edit can't silently regress any of the three providers back to OpenAiCompletions.

Test plan

  • cargo fmt
  • cargo clippy --all --benches --tests --examples --all-features — zero warnings
  • cargo test --lib — 5620 passed
  • scripts/pre-commit-safety.sh — clean
  • Manual: gemini-3.1-flash-lite-preview API-key — confirm tool calling continues across iterations
  • Manual: DeepSeek thinking-mode (deepseek-reasoner / deepseek-v4-flash) — confirm tool calling continues
  • Manual: OpenRouter with a thinking model (e.g. anthropic/claude-opus-4.5:thinking) — confirm tool calling continues

🤖 Generated with Claude Code

Issue linkage audit

Fixes #3201.
Fixes #3225.

Both issues are open as of this audit; this PR is the active closing PR for both.

#3201, #3225)

DeepSeek thinking-mode (#3201) and Gemini API-key tool calling (#3225)
both failed deterministically on the second LLM turn:

- DeepSeek: HTTP 400 "The reasoning_content in the thinking mode must
  be passed back to the API"
- Gemini: HTTP 400 "Function call is missing a thought_signature in
  functionCall parts"

Both providers were configured in `providers.json` as
`protocol: "open_ai_completions"`, which dispatched them through
rig-core's generic OpenAI client. That client silently strips
`reasoning_content` from assistant messages and `thought_signature`
from tool calls when serializing the next turn — so the field that
the upstream API requires to be echoed back was never sent.

rig-core actually has dedicated `deepseek::Client` and `gemini::Client`
implementations that handle the round-trip correctly:

- `deepseek.rs:500-527` — captures `reasoning_content` from the response
  and writes it back onto the last assistant message in the next request.
- `gemini/completion.rs:1016` — round-trips `thought_signature` on every
  `ToolCall` via `ToolCall.signature`.

Add `ProviderProtocol::DeepSeek` and `ProviderProtocol::Gemini` variants
plus matching factory branches in `create_registry_provider`, and
update the two registry entries to use them. Both factories follow the
same shape as `create_anthropic_from_registry` and wrap the rig-core
client in our existing `RigAdapter` — no new HTTP code, no agent-loop
changes.

The `default_base_url` for both entries is now empty so the dedicated
clients use their built-in endpoints (`api.deepseek.com` /
`generativelanguage.googleapis.com`) rather than the OpenAI-compat
shims.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions github-actions Bot added size: M 50-199 changed lines risk: low Changes to docs, tests, or low-risk modules scope: llm LLM integration contributor: core 20+ merged PRs labels May 7, 2026
Same bug class as DeepSeek (#3201) and Gemini (#3225): OpenRouter was
configured as `protocol: "open_ai_completions"`, dispatching it through
rig-core's generic OpenAI client. That client strips OpenRouter's
`reasoning`, `reasoning_details` (Summary / Encrypted / Text), and
per-tool-call signatures when serializing the next turn — breaking
tool calling for every thinking-mode model OpenRouter exposes (Claude
with thinking, OpenAI o-series, DeepSeek-R1, Gemini 2.5+, Qwen QwQ,
…).

rig-core's dedicated OpenRouter client (`openrouter/completion.rs`)
round-trips all of it correctly, including the `reasoning_details` ->
`tool_call.signature` mapping at lines 290-308.

Add `ProviderProtocol::OpenRouter`, a `create_openrouter_from_registry`
factory that preserves OpenRouter attribution headers (`HTTP-Referer`,
`X-Title`), and update the registry entry. Extend the regression test
to lock the routing for all three providers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 7, 2026 05:47
@github-actions github-actions Bot added size: L 200-499 changed lines and removed size: M 50-199 changed lines labels May 7, 2026
@ilblackdragon ilblackdragon changed the title fix(llm): route DeepSeek and Gemini through dedicated rig-core clients (#3201, #3225) fix(llm): route DeepSeek, Gemini, and OpenRouter through dedicated rig-core clients (#3201, #3225) May 7, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Routes DeepSeek, Gemini, and OpenRouter through rig-core’s dedicated clients to preserve provider-specific “reasoning artifacts” across turns (fixing the HTTP 400s caused by OpenAI-compat routing).

Changes:

  • Added ProviderProtocol::{DeepSeek,Gemini,OpenRouter} and wired them into provider creation.
  • Implemented dedicated factory functions for DeepSeek/Gemini/OpenRouter using rig-core clients (wrapped by RigAdapter).
  • Updated providers.json to use the new protocols (and added a regression test that locks this routing in).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/llm/registry.rs Adds new protocol variants and a regression test ensuring providers aren’t routed via OpenAI-compat.
src/llm/mod.rs Routes new protocols to dedicated rig-core clients; adds DeepSeek/Gemini/OpenRouter client construction.
providers.json Switches DeepSeek/Gemini/OpenRouter to dedicated protocols and clears default base URLs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/llm/mod.rs
Comment on lines +505 to +512
for (key, value) in &config.extra_headers {
let name = match reqwest::header::HeaderName::from_bytes(key.as_bytes()) {
Ok(n) => n,
Err(e) => {
tracing::warn!(header = %key, error = %e, "Skipping extra header: invalid name");
continue;
}
};
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

False positive — http::HeaderName::from_bytes normalizes input (see http-1.4.0/src/header/name.rs:1115: "This function normalizes the input"); uppercase ASCII parses fine and is lowercased internally via the HEADER_CHARS table. HTTP-Referer and X-Title round-trip without warnings. Added a clarifying comment in 3a0b3a5 so the warn block doesn't read as suspect.

Comment thread src/llm/mod.rs Outdated
Comment on lines +509 to +516
tracing::warn!(header = %key, error = %e, "Skipping extra header: invalid name");
continue;
}
};
let val = match reqwest::header::HeaderValue::from_str(value) {
Ok(v) => v,
Err(e) => {
tracing::warn!(header = %key, error = %e, "Skipping extra header: invalid value");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 3a0b3a5 — both header-warn sites (create_openrouter_from_registry and create_openai_compat_from_registry) now include provider = %config.provider_id so misconfigurations are easy to attribute in multi-provider deployments.

Comment thread src/llm/mod.rs Outdated
Comment on lines +509 to +516
tracing::warn!(header = %key, error = %e, "Skipping extra header: invalid name");
continue;
}
};
let val = match reqwest::header::HeaderValue::from_str(value) {
Ok(v) => v,
Err(e) => {
tracing::warn!(header = %key, error = %e, "Skipping extra header: invalid value");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Duplicate of #discussion_r3199146864 — fixed in 3a0b3a5 (added provider_id to both header-warn sites).

Comment thread src/llm/registry.rs
/// providers expose. They must route through the dedicated rig-core
/// clients which round-trip the artifacts on the next request.
#[test]
fn reasoning_aware_providers_use_dedicated_protocol_not_openai_compat() {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The PR description already references the actual test name (reasoning_aware_providers_use_dedicated_protocol_not_openai_compat); the bot may have read a stale revision. The body has been refreshed in 3a0b3a5 along with the new round-trip regression tests.

@ilblackdragon
Copy link
Copy Markdown
Member Author

Closing briefly to regenerate refs/pull/3326/head — GitHub failed to create it on initial push, blocking the regression-test-check workflow which fetches that ref.

@ilblackdragon
Copy link
Copy Markdown
Member Author

Reopening — refs/pull/3326/head should regenerate now.

@ilblackdragon ilblackdragon reopened this May 7, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread providers.json
Comment on lines +127 to 131
"protocol": "open_router",
"default_base_url": "",
"api_key_env": "OPENROUTER_API_KEY",
"api_key_required": true,
"model_env": "OPENROUTER_MODEL",
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 3a0b3a5providers.json now exposes "extra_headers_env": "OPENROUTER_EXTRA_HEADERS". Users can now configure HTTP-Referer:…,X-Title:… for the built-in openrouter backend.

Comment thread src/llm/mod.rs
"Using DeepSeek provider (preserves reasoning_content across turns)"
);

Ok(Arc::new(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

High Severity

The dedicated clients are still wrapped in RigAdapter, which drops required reasoning/signature metadata before the next tool turn.

rig-core can now parse DeepSeek/Gemini/OpenRouter provider-specific state, but RigAdapter::extract_response() only converts AssistantContent::ToolCall into IronClaw ToolCall { id, name, arguments, reasoning } and ignores AssistantContent::Reasoning, ToolCall.signature, and ToolCall.additional_params. On the next iteration, dispatcher stores ChatMessage::assistant_with_tool_calls(...), and RigAdapter::convert_messages() rebuilds bare rig tool calls without those fields. A Gemini thinking model that returned a signed functionCall will therefore still be replayed without thought_signature; DeepSeek/OpenRouter reasoning artifacts are similarly lost, so the second turn can still fail with the same 400s this PR is intended to fix.

Please either extend IronClaw's message/tool-call types plus RigAdapter conversions to round-trip AssistantContent::Reasoning, ToolCall.signature, and OpenRouter additional params across the tool loop, or use provider-specific LlmProvider implementations. Add a caller-level integration test that simulates provider response with tool call → tool result → second provider request and asserts the echoed fields are present.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch — confirmed by reading rig-core's deepseek::Message::try_from (src/providers/deepseek.rs:299-322) and gemini::Part::From<message::ToolCall> (src/providers/gemini/completion.rs:1012-1023). The dedicated clients only round-trip inside rig-core; bridging through RigAdapter was where reasoning + signatures were dropped.

Fixed end-to-end in 3a0b3a5:

  • ChatMessage carries reasoning: Option<String>; ToolCall carries signature: Option<String>; ToolCompletionResponse carries reasoning.
  • RigAdapter::extract_response captures AssistantContent::Reasoning and per-tool-call signature.
  • RigAdapter::convert_messages re-emits AssistantContent::Reasoning and pushes ToolCall::with_signature(…) when rebuilding rig messages on the next turn.
  • Plumbed via ChatMessage::with_reasoning through dispatcher (src/agent/dispatcher.rs:783-791), job worker (src/worker/job.rs:1736-1742), container worker (src/worker/container.rs:530-535), routine engine (src/agent/routine_engine.rs:1940-1948), and the orchestrator-worker proxy (src/worker/api.rs + src/orchestrator/api.rs).
  • Regression test reasoning_and_signature_round_trip_through_chat_message in src/llm/rig_adapter.rs simulates a 2-turn tool loop end-to-end: rig response with Reasoning + signed ToolCall → IronClaw ChatMessage → rebuilt rig message, asserting both fields survive.

OpenRouter's additional_params (per-tool-call reasoning_details) is not plumbed yet — left for follow-up #3327 since it requires a typed payload field rather than the simple string round-trip used for DeepSeek/Gemini.

Comment thread src/llm/mod.rs Outdated
} else {
gemini::Client::builder()
.api_key(&api_key)
.base_url(&config.base_url)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

High Severity

Native Gemini will use stale OpenAI-shim base URL overrides from existing installs.

Before this PR, the UI/config default for Gemini was the OpenAI-compatible shim https://generativelanguage.googleapis.com/v1beta/openai. Existing users may have this saved in llm_builtin_overrides[gemini].base_url; config resolution still prefers DB overrides over the new empty/native registry default. This factory then passes any non-empty URL directly to gemini::Client::builder(), whose native client appends paths like /v1beta/models/{model}:generateContent. Result: an upgraded user can send native Gemini requests to .../v1beta/openai/v1beta/models/..., breaking normal Gemini calls instead of using the native default endpoint.

Please detect/migrate/ignore the old Gemini OpenAI-shim URL when ProviderProtocol::Gemini is selected, or otherwise clear stale persisted overrides. Add a regression test for resolving a saved https://generativelanguage.googleapis.com/v1beta/openai override after this protocol switch.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 3a0b3a5. New sanitize_gemini_base_url in src/llm/mod.rs discards any persisted …/v1beta/openai (or …/v1/openai) shim URL when ProviderProtocol::Gemini is selected, with a warn! log telling the operator how to clear the stale override. Custom proxies and region endpoints pass through unchanged.

Regression: sanitize_gemini_base_url_strips_legacy_openai_shim (covers exact match, trailing slash, mixed case, and the /v1/openai variant) plus sanitize_gemini_base_url_passes_through_empty and sanitize_gemini_base_url_preserves_custom_endpoints.

Comment thread providers.json
"protocol": "open_ai_completions",
"default_base_url": "https://generativelanguage.googleapis.com/v1beta/openai",
"protocol": "gemini",
"default_base_url": "",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Medium Severity

Gemini is still advertised as list-models capable, but the UI/setup list-models paths do not speak native Gemini.

This registry entry now exposes adapter gemini with an empty default base URL while keeping can_list_models: true. The web configure surface shows the fetch button, then blocks on a missing base URL before calling /api/llm/list_models; if a base URL is supplied, the handler falls through to generic GET {base}/models with Bearer auth rather than Gemini's native /v1beta/models?key=... shape. The setup wizard has the same issue: it falls through to fetch_openai_compatible_models(def.default_base_url.unwrap_or("")), which returns no models for the new empty default. So users configuring Gemini are told model listing is supported, but it reliably fails/falls back to manual entry.

Please either add native Gemini model-listing support in the web handler/setup wizard or set can_list_models to false for Gemini until that path exists. Add a test covering adapter: "gemini" with empty/native default base URL.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 3a0b3a5 by setting "can_list_models": false on Gemini in providers.json. The setup wizard's fall-through path (fetch_openai_compatible_models(def.default_base_url.unwrap_or(""), …)) wouldn't speak the native /v1beta/models?key=… shape, and the web list-models handler is the same. Native Gemini list-models support is filed as a follow-up rather than blocking this PR.

Addresses review feedback on PR #3326. The original PR routed DeepSeek,
Gemini, and OpenRouter through rig-core's dedicated clients, but the
fix only worked inside rig-core. IronClaw's RigAdapter sits between
the agent loop and rig-core and was dropping AssistantContent::Reasoning
(DeepSeek `reasoning_content`) and per-tool-call `signature` (Gemini
`thought_signature`) on the response → IronClaw conversion. On the
next request it rebuilt rig messages without either field, so the
provider rejected the follow-up turn — same HTTP 400s the PR claimed
to fix.

Round-trip both fields end-to-end:
- Add `ChatMessage::reasoning` and `ToolCall.signature` (skip-serialized
  when None) plus `ToolCompletionResponse.reasoning` to carry artifacts
  out of the provider.
- Update `RigAdapter::extract_response` to capture both, and
  `convert_messages` to push `AssistantContent::Reasoning` and
  `ToolCall.signature` back when rebuilding rig messages on the next turn.
- Plumb response.reasoning through dispatcher, job worker, container
  worker, routine engine, and the orchestrator-worker proxy via a new
  `ChatMessage::with_reasoning` builder.

Other review fixes:
- Set `can_list_models: false` for Gemini in providers.json — setup
  wizard and web list-models handler don't speak native Gemini, so
  exposing the button reliably falls back to manual entry.
- Add `extra_headers_env: OPENROUTER_EXTRA_HEADERS` so users of the
  built-in `openrouter` backend can configure HTTP-Referer / X-Title
  attribution headers.
- New `sanitize_gemini_base_url` discards the legacy
  `…/v1beta/openai` shim URL persisted by pre-3225 installs in
  `llm_builtin_overrides[gemini].base_url`. Without this, upgraded
  users would hit `…/v1beta/openai/v1beta/models/...:generateContent`.
- Include `provider_id` in extra-header warning logs (OpenRouter +
  OpenAI-compat).

Regression tests:
- `reasoning_and_signature_round_trip_through_chat_message` simulates
  the 2-turn tool loop end-to-end.
- `chat_message_with_reasoning_drops_empty_input` locks the empty-input
  contract so we don't echo `reasoning_content: ""`.
- `sanitize_gemini_base_url_*` cover legacy-shim discard, empty input,
  and custom-endpoint pass-through.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel/web Web gateway channel scope: tool/builder Dynamic tool builder scope: orchestrator Container orchestrator scope: worker Container worker labels May 7, 2026
@github-actions github-actions Bot added size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules and removed size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules labels May 7, 2026
@ilblackdragon ilblackdragon merged commit 481a34a into main May 7, 2026
39 checks passed
@ilblackdragon ilblackdragon deleted the fix/3225-3201-route-to-dedicated-rig-providers branch May 7, 2026 19:29
@nickpismenkov nickpismenkov mentioned this pull request May 7, 2026
@ironclaw-ci ironclaw-ci Bot mentioned this pull request May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: agent Agent core (agent loop, router, scheduler) scope: channel/web Web gateway channel scope: llm LLM integration scope: orchestrator Container orchestrator scope: tool/builder Dynamic tool builder scope: worker Container worker size: XL 500+ changed lines

Projects

None yet

3 participants