From 588080cdaa1abef4dfbc9d8a36b3dcb299d654d3 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Wed, 15 Jul 2026 15:44:57 -0600 Subject: [PATCH] fix(providers): drop duplicate timeout kwarg on direct cloud generate OpenHands SDK 1.27.1 merges the LLM object's `timeout` with call kwargs via `dict(...)`, so passing `timeout` in `_generate_call_kwargs` collided: TypeError on the completion path for google/anthropic and silent override on the openai responses path. Timeout now lives solely on the LLM object built in `_build_generate_llm`; `_generate_call_kwargs` no longer takes or emits `timeout`. Adds an SDK-faithful fake LLM where completion raises on duplicate timeout and responses overrides, plus real-boundary sync and async regression tests across google, anthropic, and openai. Validation probes (timeout_s=10, num_retries=0) are unaffected. --- solstone/think/providers/openhands.py | 5 +- tests/openhands_fakes.py | 47 ++++++++++++++ tests/test_openhands_generate.py | 90 ++++++++++++++++++++++++--- 3 files changed, 131 insertions(+), 11 deletions(-) diff --git a/solstone/think/providers/openhands.py b/solstone/think/providers/openhands.py index 98a8231ee..36ab40139 100644 --- a/solstone/think/providers/openhands.py +++ b/solstone/think/providers/openhands.py @@ -370,10 +370,9 @@ def _generate_call_kwargs( json_output: bool, json_schema: dict | None, thinking_budget: int | None, - timeout_s: float, responses_api: bool, ) -> dict[str, Any]: - kwargs: dict[str, Any] = {"timeout": timeout_s} + kwargs: dict[str, Any] = {} # Preserve each direct provider's accepted parameter contract. OpenAI's # reasoning models reject temperature, and Anthropic rejects it while # extended thinking is enabled (plus a few model-specific cases). @@ -561,7 +560,6 @@ def _run_generate( json_output=json_output, json_schema=json_schema, thinking_budget=thinking_budget, - timeout_s=timeout_s, responses_api=responses_api, ) response = ( @@ -602,7 +600,6 @@ async def _run_agenerate( json_output=json_output, json_schema=json_schema, thinking_budget=thinking_budget, - timeout_s=timeout_s, responses_api=responses_api, ) response = ( diff --git a/tests/openhands_fakes.py b/tests/openhands_fakes.py index f3b00e402..8c14dc238 100644 --- a/tests/openhands_fakes.py +++ b/tests/openhands_fakes.py @@ -122,11 +122,57 @@ class Metrics(FakeModel): ) +def _direct_generation_response(model: str) -> SimpleNamespace: + message = SimpleNamespace( + content=[SimpleNamespace(text="fake response")], + thinking_blocks=[], + reasoning_content=None, + responses_reasoning_item=None, + ) + return SimpleNamespace( + message=message, + metrics=Metrics(), + raw_response={ + "model": model, + "choices": [{"finish_reason": "stop"}], + }, + ) + + class LLM(FakeModel): + instances: list[LLM] = [] + def __init__(self, **kwargs: Any) -> None: super().__init__(**kwargs) self.metrics = Metrics() self.effective_max_input_tokens = None + self.last_completion_messages = None + self.last_completion_kwargs = None + self.last_completion_merged_kwargs = None + self.last_responses_messages = None + self.last_responses_kwargs = None + self.last_responses_merged_kwargs = None + type(self).instances.append(self) + + def completion(self, messages: Any, **kwargs: Any) -> SimpleNamespace: + merged = dict(timeout=self.timeout, **kwargs) + self.last_completion_messages = messages + self.last_completion_kwargs = dict(kwargs) + self.last_completion_merged_kwargs = merged + return _direct_generation_response(self.model) + + def responses(self, messages: Any, **kwargs: Any) -> SimpleNamespace: + merged = {"timeout": self.timeout, **kwargs} + self.last_responses_messages = messages + self.last_responses_kwargs = dict(kwargs) + self.last_responses_merged_kwargs = merged + return _direct_generation_response(self.model) + + async def acompletion(self, messages: Any, **kwargs: Any) -> SimpleNamespace: + return self.completion(messages, **kwargs) + + async def aresponses(self, messages: Any, **kwargs: Any) -> SimpleNamespace: + return self.responses(messages, **kwargs) class Agent(FakeModel): @@ -190,6 +236,7 @@ class ConversationErrorEvent(FakeModel): def install_fake_openhands(monkeypatch: Any) -> types.SimpleNamespace: Conversation.instances = [] Conversation.arun_impl = None + LLM.instances = [] _REGISTERED_TOOLS.clear() root_mod = types.ModuleType("openhands") diff --git a/tests/test_openhands_generate.py b/tests/test_openhands_generate.py index 91b00479b..1f00d6129 100644 --- a/tests/test_openhands_generate.py +++ b/tests/test_openhands_generate.py @@ -11,6 +11,12 @@ import pytest from PIL import Image from solstone.think.providers import openhands +from tests.openhands_fakes import install_fake_openhands + + +@pytest.fixture +def fake_openhands(monkeypatch): + return install_fake_openhands(monkeypatch) def _response( @@ -67,6 +73,7 @@ def test_generate_google_uses_openhands_chat_and_normalizes_result(monkeypatch): assert messages[0].role == "user" assert messages[0].content[0].text == "hello" kwargs = llm.completion.call_args.kwargs + assert "timeout" not in kwargs assert kwargs["thinking"] == {"type": "enabled", "budget_tokens": 128} assert kwargs["response_format"]["json_schema"]["name"] == "Reply" assert result == { @@ -127,7 +134,9 @@ def test_generate_openai_uses_responses_json_schema(monkeypatch): llm.responses.assert_called_once() assert not llm.completion.called - assert llm.responses.call_args.kwargs["text"]["format"] == { + kwargs = llm.responses.call_args.kwargs + assert "timeout" not in kwargs + assert kwargs["text"]["format"] == { "type": "json_schema", "name": "response", "schema": {"type": "object"}, @@ -155,16 +164,17 @@ def test_generate_provider_thinking_and_token_budget_mapping(caplog): assert openhands._generate_token_budget("anthropic", 100, 200) == 1201 assert openhands._generate_token_budget("openai", 100, 200) == 100 assert openhands._parse_openai_effort("gpt-5.5-xhigh") == ("gpt-5.5", "xhigh") - assert openhands._generate_call_kwargs( + google_kwargs = openhands._generate_call_kwargs( "google", "gemini-flash-latest", temperature=None, json_output=False, json_schema=None, thinking_budget=0, - timeout_s=10, responses_api=False, - )["thinking"] == {"type": "disabled", "budget_tokens": 0} + ) + assert google_kwargs["thinking"] == {"type": "disabled", "budget_tokens": 0} + assert "timeout" not in google_kwargs assert "temperature" not in openhands._generate_call_kwargs( "openai", @@ -173,7 +183,6 @@ def test_generate_provider_thinking_and_token_budget_mapping(caplog): json_output=False, json_schema=None, thinking_budget=None, - timeout_s=10, responses_api=True, ) assert "temperature" not in openhands._generate_call_kwargs( @@ -183,7 +192,6 @@ def test_generate_provider_thinking_and_token_budget_mapping(caplog): json_output=False, json_schema=None, thinking_budget=1_024, - timeout_s=10, responses_api=False, ) @@ -209,13 +217,81 @@ async def test_agenerate_uses_async_openhands_transport(monkeypatch): ) llm.acompletion.assert_awaited_once() - assert llm.acompletion.call_args.kwargs["thinking"] == { + kwargs = llm.acompletion.call_args.kwargs + assert "timeout" not in kwargs + assert kwargs["thinking"] == { "type": "enabled", "budget_tokens": 1_024, } assert result["text"] == "hello" +def test_fake_openhands_completion_raises_on_duplicate_timeout(fake_openhands): + llm = fake_openhands.LLM(model="google/gemini-flash-latest", timeout=10) + + with pytest.raises( + TypeError, + match="dict\\(\\) got multiple values for keyword argument 'timeout'", + ): + llm.completion([], timeout=1) + + +@pytest.mark.parametrize( + ("provider", "model", "transport_attr"), + [ + ("google", "gemini-flash-latest", "last_completion_kwargs"), + ("anthropic", "claude-sonnet-4-6", "last_completion_kwargs"), + ("openai", "gpt-5.5", "last_responses_kwargs"), + ], +) +def test_run_generate_transport_kwargs_do_not_shadow_llm_timeout( + fake_openhands, + monkeypatch, + provider, + model, + transport_attr, +): + monkeypatch.setenv(openhands._API_KEY_ENV[provider], "test-key") + + result = openhands.run_generate("hello", model, provider=provider, timeout_s=7) + + llm = fake_openhands.LLM.instances[-1] + assert llm.timeout == 7 + assert result["text"] == "fake response" + assert "timeout" not in getattr(llm, transport_attr) + + +@pytest.mark.parametrize( + ("provider", "model", "transport_attr"), + [ + ("google", "gemini-flash-latest", "last_completion_kwargs"), + ("anthropic", "claude-sonnet-4-6", "last_completion_kwargs"), + ("openai", "gpt-5.5", "last_responses_kwargs"), + ], +) +@pytest.mark.asyncio +async def test_run_agenerate_transport_kwargs_do_not_shadow_llm_timeout( + fake_openhands, + monkeypatch, + provider, + model, + transport_attr, +): + monkeypatch.setenv(openhands._API_KEY_ENV[provider], "test-key") + + result = await openhands.run_agenerate( + "hello", + model, + provider=provider, + timeout_s=7, + ) + + llm = fake_openhands.LLM.instances[-1] + assert llm.timeout == 7 + assert result["text"] == "fake response" + assert "timeout" not in getattr(llm, transport_attr) + + def test_validation_uses_runtime_probe_and_classifies_results(monkeypatch): monkeypatch.setattr(openhands, "_probe", lambda *args: None) assert openhands.validate_key("google", "key") == {"valid": True} -- 2.51.2