mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-27 21:05:34 +00:00
fix: forward tools through OpenRouter engine (#511)
* fix: forward tools through OpenRouter engine The OpenRouter chat completion path built the request from only `model`, `messages`, `max_tokens`, and `temperature`. `tools` and `tool_choice` passed via `kwargs` were silently dropped, so function definitions never reached the model. Symptom on a managed deep_research agent: the model answered every query from its own prior knowledge and never invoked `knowledge_search`, `knowledge_sql`, etc. The same path also discarded `choice.message.tool_calls` from the response — when a model did return a tool call (verified directly against OpenRouter with `google/gemma-4-31b-it:free` and `nvidia/nemotron-3-ultra-550b-a55b:free`), the agent loop never saw it. This patch: - forwards `tools` and `tool_choice` into the OpenAI-compatible request in both `_generate_openrouter` (sync) and `_stream_openrouter` (async stream), - extracts `tool_calls` from the response in `_generate_openrouter` in the same shape used by the OpenAI / Anthropic paths. Verified end-to-end against a `deep_research` managed agent using an OpenRouter preset with Gemma 4 31B + Nemotron 3 Ultra fallback: before, the agent stated "I don't have access to your vault"; after, it calls `knowledge_search`, cites results, and produces a structured answer. * test(engine): regression test for OpenRouter tool forwarding Asserts the OpenRouter path forwards tools/tool_choice to the OpenAI-compatible API and parses tool_calls back into the result (#511). Verified: passes on the fix, fails (KeyError 'tools') against pre-fix main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Jon Saad-Falcon <jonsaadfalcon@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jon Saad-Falcon
Claude Opus 4.8
parent
d908372eb5
commit
c76c80d6b4
@@ -893,6 +893,13 @@ class CloudEngine(InferenceEngine):
|
||||
"max_tokens": max_tokens,
|
||||
"temperature": temperature,
|
||||
}
|
||||
# Forward tools / tool_choice (OpenRouter is OpenAI-compatible).
|
||||
tools = kwargs.pop("tools", None)
|
||||
if tools:
|
||||
create_kwargs["tools"] = tools
|
||||
tool_choice = kwargs.pop("tool_choice", None)
|
||||
if tool_choice is not None:
|
||||
create_kwargs["tool_choice"] = tool_choice
|
||||
t0 = time.monotonic()
|
||||
resp = self._openrouter_client.chat.completions.create(**create_kwargs)
|
||||
elapsed = time.monotonic() - t0
|
||||
@@ -900,7 +907,7 @@ class CloudEngine(InferenceEngine):
|
||||
usage = resp.usage
|
||||
prompt_tokens = usage.prompt_tokens if usage else 0
|
||||
completion_tokens = usage.completion_tokens if usage else 0
|
||||
return {
|
||||
result: Dict[str, Any] = {
|
||||
"content": choice.message.content or "",
|
||||
"usage": {
|
||||
"prompt_tokens": prompt_tokens,
|
||||
@@ -911,6 +918,19 @@ class CloudEngine(InferenceEngine):
|
||||
"finish_reason": choice.finish_reason or "stop",
|
||||
"ttft": elapsed,
|
||||
}
|
||||
if getattr(choice.message, "tool_calls", None):
|
||||
result["tool_calls"] = [
|
||||
{
|
||||
"id": tc.id,
|
||||
"type": tc.type,
|
||||
"function": {
|
||||
"name": tc.function.name,
|
||||
"arguments": tc.function.arguments,
|
||||
},
|
||||
}
|
||||
for tc in choice.message.tool_calls
|
||||
]
|
||||
return result
|
||||
|
||||
def _generate_minimax(
|
||||
self,
|
||||
@@ -1195,6 +1215,13 @@ class CloudEngine(InferenceEngine):
|
||||
"temperature": temperature,
|
||||
"stream": True,
|
||||
}
|
||||
# Forward tools / tool_choice (OpenRouter is OpenAI-compatible).
|
||||
tools = kwargs.pop("tools", None)
|
||||
if tools:
|
||||
create_kwargs["tools"] = tools
|
||||
tool_choice = kwargs.pop("tool_choice", None)
|
||||
if tool_choice is not None:
|
||||
create_kwargs["tool_choice"] = tool_choice
|
||||
resp = self._openrouter_client.chat.completions.create(**create_kwargs)
|
||||
for chunk in resp:
|
||||
delta = chunk.choices[0].delta if chunk.choices else None
|
||||
|
||||
@@ -381,3 +381,59 @@ class TestCodexGenerate:
|
||||
engine._codex_client = {"token": "t", "url": "http://test"}
|
||||
engine.close()
|
||||
assert engine._codex_client is None
|
||||
|
||||
|
||||
class TestOpenRouterToolForwarding:
|
||||
"""Regression for #511: the OpenRouter engine must forward tools/tool_choice
|
||||
to the (OpenAI-compatible) API and parse tool_calls back out of the response.
|
||||
Pre-fix, both were dropped, silently breaking function-calling via OpenRouter.
|
||||
"""
|
||||
|
||||
def test_generate_forwards_tools_and_parses_tool_calls(
|
||||
self, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.delenv("OPENAI_API_KEY", raising=False)
|
||||
|
||||
fake_tc = SimpleNamespace(
|
||||
id="call_1",
|
||||
type="function",
|
||||
function=SimpleNamespace(name="get_weather", arguments='{"city": "NYC"}'),
|
||||
)
|
||||
fake_choice = SimpleNamespace(
|
||||
message=SimpleNamespace(content=None, tool_calls=[fake_tc]),
|
||||
finish_reason="tool_calls",
|
||||
)
|
||||
fake_resp = SimpleNamespace(
|
||||
choices=[fake_choice],
|
||||
usage=SimpleNamespace(prompt_tokens=3, completion_tokens=2, total_tokens=5),
|
||||
model="openai/gpt-4o",
|
||||
)
|
||||
fake_client = mock.MagicMock()
|
||||
fake_client.chat.completions.create.return_value = fake_resp
|
||||
|
||||
EngineRegistry.register_value("cloud", CloudEngine)
|
||||
engine = CloudEngine()
|
||||
engine._openrouter_client = fake_client
|
||||
|
||||
tools = [
|
||||
{
|
||||
"type": "function",
|
||||
"function": {"name": "get_weather", "parameters": {}},
|
||||
}
|
||||
]
|
||||
result = engine.generate(
|
||||
[Message(role=Role.USER, content="weather in NYC?")],
|
||||
model="openrouter/openai/gpt-4o",
|
||||
tools=tools,
|
||||
tool_choice="auto",
|
||||
)
|
||||
|
||||
# tools / tool_choice are forwarded to the API call
|
||||
sent = fake_client.chat.completions.create.call_args.kwargs
|
||||
assert sent["tools"] == tools
|
||||
assert sent["tool_choice"] == "auto"
|
||||
|
||||
# tool_calls from the response are parsed back into the result
|
||||
assert result["tool_calls"][0]["id"] == "call_1"
|
||||
assert result["tool_calls"][0]["function"]["name"] == "get_weather"
|
||||
assert result["tool_calls"][0]["function"]["arguments"] == '{"city": "NYC"}'
|
||||
|
||||
Reference in New Issue
Block a user