diff --git a/openhack/agents/llm.py b/openhack/agents/llm.py index 4ece7c8..85ade92 100644 --- a/openhack/agents/llm.py +++ b/openhack/agents/llm.py @@ -228,6 +228,7 @@ class LLMClient: # Set to True for the rest of the session when the endpoint rejects # prompt_cache_key (e.g. Groq), so we stop sending it. _cache_key_unsupported = False + _temperature_unsupported_models: set[str] = set() def __init__( self, @@ -248,6 +249,7 @@ def __init__( self.model = model or settings.openhack_model_id self.temperature = temperature + self._temperature_unsupported = False self.max_tokens = max_tokens self.prompt_cache_key = prompt_cache_key self.total_cost: float = 0.0 @@ -266,6 +268,13 @@ def __init__( self._init_client() + @property + def _is_temperature_unsupported(self) -> bool: + return ( + getattr(self, "_temperature_unsupported", False) + or self.model in LLMClient._temperature_unsupported_models + ) + def _status(self, text: str) -> None: self._event("model_status", {"message": text}) cb = self.status_callback @@ -727,27 +736,65 @@ async def chat( tool_choice: Optional[str] = None, on_chunk: Optional[Callable] = None, ) -> LLMResponse: - try: - return await self._chat(messages, tools, system, tool_choice=tool_choice, on_chunk=on_chunk) - except openai.APIStatusError as e: - detail = str(e).lower() - if ( - self.prompt_cache_key - and not LLMClient._cache_key_unsupported - and ("prompt_cache_key" in detail or "prompt cache key" in detail) - ): - LLMClient._cache_key_unsupported = True - # NEVER print here: in the TUI this writes straight into the - # full-screen buffer, tearing the layout and leaving text the - # renderer doesn't know about (it survives until that region - # happens to be redrawn). Log it, and tell the UI via the hook. - logger.warning( - "Endpoint doesn't support prompt caching — retrying without it. " - "To disable permanently: /config prompt_caching false" - ) - self._status("endpoint rejected prompt caching — retrying without it") + while True: + try: return await self._chat(messages, tools, system, tool_choice=tool_choice, on_chunk=on_chunk) - raise + except openai.APIStatusError as e: + detail = str(e).lower() + if ( + self.prompt_cache_key + and not LLMClient._cache_key_unsupported + and ("prompt_cache_key" in detail or "prompt cache key" in detail) + ): + LLMClient._cache_key_unsupported = True + # NEVER print here: in the TUI this writes straight into the + # full-screen buffer, tearing the layout and leaving text the + # renderer doesn't know about (it survives until that region + # happens to be redrawn). Log it, and tell the UI via the hook. + logger.warning( + "Endpoint doesn't support prompt caching — retrying without it. " + "To disable permanently: /config prompt_caching false" + ) + self._status("endpoint rejected prompt caching — retrying without it") + continue + + param = "" + if hasattr(e, "body") and isinstance(e.body, dict): + error_obj = e.body.get("error") + if isinstance(error_obj, dict): + param = str(error_obj.get("param") or "").lower() + + if ( + self.temperature is not None + and not self._is_temperature_unsupported + and ( + param == "temperature" + or ( + "temperature" in detail + and any( + phrase in detail + for phrase in ( + "unsupported", + "unknown parameter", + "unexpected keyword", + "not supported", + "not allowed", + "does not support", + "invalid parameter", + ) + ) + ) + ) + ): + self._temperature_unsupported = True + LLMClient._temperature_unsupported_models.add(self.model) + logger.warning( + f"Model '{self.model}' rejected temperature parameter — retrying without it." + ) + self._status(f"model '{self.model}' rejected temperature — retrying without it") + continue + + raise async def _chat( self, @@ -766,10 +813,11 @@ async def _chat( "model": self.model, "messages": openai_messages, "max_tokens": self.max_tokens, - "temperature": self.temperature, "stream": True, "stream_options": {"include_usage": True}, } + if self.temperature is not None and not self._is_temperature_unsupported: + kwargs["temperature"] = self.temperature # OpenHack inference translates this trusted product hint into # OpenRouter's provider.sort="throughput" routing. It is deliberately # a header so it never leaks as an unsupported model parameter. @@ -798,7 +846,7 @@ async def _chat( { "provider": getattr(self, "provider", None), "requested_model": self.model, - "temperature": self.temperature, + "temperature": kwargs.get("temperature"), "max_tokens": self.max_tokens, "stream": True, "tool_choice": tool_choice or ("auto" if tools else None), diff --git a/tests/test_llm_retry.py b/tests/test_llm_retry.py index 787a077..b285857 100644 --- a/tests/test_llm_retry.py +++ b/tests/test_llm_retry.py @@ -34,3 +34,145 @@ async def _boom(**kwargs): asyncio.run(llm._chat([Message(role="user", content="hi")])) assert calls["n"] == 3 # initial attempt + 2 retries (not 1 = no-retry) + + +class _Stream: + def __init__(self, chunks): + self._chunks = list(chunks) + + def __aiter__(self): + return self + + async def __anext__(self): + if not self._chunks: + raise StopAsyncIteration + return self._chunks.pop(0) + + async def close(self): + pass + + +def _chunk(content="ok", model="gpt-5.6-sol"): + from types import SimpleNamespace + return SimpleNamespace( + id="resp-1", + model=model, + usage=SimpleNamespace(prompt_tokens=10, completion_tokens=5), + choices=[ + SimpleNamespace( + finish_reason="stop", + delta=SimpleNamespace(content=content, reasoning_content=None, tool_calls=None), + ) + ], + ) + + +def _make_bad_request_error(message: str, param: str = None): + import httpx + req = httpx.Request("POST", "https://api.openai.com/v1/chat/completions") + body = {"error": {"message": message, "type": "invalid_request_error"}} + if param: + body["error"]["param"] = param + res = httpx.Response(400, request=req, json=body) + return openai.BadRequestError(message, response=res, body=body) + + +def test_unsupported_temperature_value_is_retried_and_remembered(monkeypatch): + LLMClient._temperature_unsupported_models.clear() + llm = LLMClient(model="gpt-5.6-sol", temperature=0.0) + calls = [] + + async def _fake_create(**kwargs): + calls.append(dict(kwargs)) + if "temperature" in kwargs: + raise _make_bad_request_error( + "Unsupported value: 'temperature' does not support 0.0 with this model", + param="temperature", + ) + return _Stream([_chunk()]) + + monkeypatch.setattr(llm.client.chat.completions, "create", _fake_create) + + resp = asyncio.run(llm.chat([Message(role="user", content="hi")])) + assert resp.content == "ok" + assert len(calls) == 2 + assert calls[0].get("temperature") == 0.0 + assert "temperature" not in calls[1] + assert "gpt-5.6-sol" in LLMClient._temperature_unsupported_models + + # Subsequent call with same model omits temperature from the start + calls.clear() + resp2 = asyncio.run(llm.chat([Message(role="user", content="hello again")])) + assert resp2.content == "ok" + assert len(calls) == 1 + assert "temperature" not in calls[0] + + +def test_unsupported_temperature_parameter_is_retried(monkeypatch): + LLMClient._temperature_unsupported_models.clear() + llm = LLMClient(model="gpt-6.1-sol", temperature=0.0) + calls = [] + + async def _fake_create(**kwargs): + calls.append(dict(kwargs)) + if "temperature" in kwargs: + raise _make_bad_request_error( + "Unsupported parameter: 'temperature' is not supported with this model" + ) + return _Stream([_chunk(model="gpt-6.1-sol")]) + + monkeypatch.setattr(llm.client.chat.completions, "create", _fake_create) + + resp = asyncio.run(llm.chat([Message(role="user", content="hi")])) + assert resp.content == "ok" + assert len(calls) == 2 + assert calls[0].get("temperature") == 0.0 + assert "temperature" not in calls[1] + assert "gpt-6.1-sol" in LLMClient._temperature_unsupported_models + + +def test_unrelated_bad_request_error_is_raised(monkeypatch): + LLMClient._temperature_unsupported_models.clear() + llm = LLMClient(model="gpt-5.6-sol", temperature=0.0) + calls = [] + + async def _fake_create(**kwargs): + calls.append(dict(kwargs)) + raise _make_bad_request_error("Context length exceeded", param="messages") + + monkeypatch.setattr(llm.client.chat.completions, "create", _fake_create) + + with pytest.raises(openai.BadRequestError): + asyncio.run(llm.chat([Message(role="user", content="hi")])) + + assert len(calls) == 1 + assert "gpt-5.6-sol" not in LLMClient._temperature_unsupported_models + + +def test_prompt_cache_and_temperature_both_rejected(monkeypatch): + LLMClient._temperature_unsupported_models.clear() + LLMClient._cache_key_unsupported = False + llm = LLMClient(model="gpt-5.6-sol", temperature=0.0, prompt_cache_key="test-key") + calls = [] + + async def _fake_create(**kwargs): + calls.append(dict(kwargs)) + if len(calls) == 1: + # First attempt rejects prompt_cache_key + raise _make_bad_request_error("Unsupported parameter: 'prompt_cache_key'") + if len(calls) == 2: + # Second attempt rejects temperature + raise _make_bad_request_error("Unsupported parameter: 'temperature'") + return _Stream([_chunk()]) + + monkeypatch.setattr(llm.client.chat.completions, "create", _fake_create) + + resp = asyncio.run(llm.chat([Message(role="user", content="hi")])) + assert resp.content == "ok" + assert len(calls) == 3 + assert "prompt_cache_key" in calls[0] + assert "prompt_cache_key" not in calls[1] + assert "temperature" in calls[1] + assert "prompt_cache_key" not in calls[2] + assert "temperature" not in calls[2] +