Skip to content

Retry and omit temperature when rejected by endpoint (fixes #23) #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nandal wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Retry and omit temperature when rejected by endpoint (fixes #23) #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 70 additions & 22 deletions openhack/agents/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)

@greptile-apps greptile-apps Bot Oct 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Temperature settings cross endpoints

The shared cache uses only self.model. If one endpoint rejects temperature, another endpoint using the same model name will also stop receiving the client's configured temperature. The TUI lets users switch providers and models without restarting, so this state can carry across those switches. Key the cache by endpoint and model, or keep it on the client.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: openhack/agents/llm.py
Line: 790

Comment:
**Temperature settings cross endpoints**

The shared cache uses only `self.model`. If one endpoint rejects temperature, another endpoint using the same model name will also stop receiving the client's configured temperature. The TUI lets users switch providers and models without restarting, so this state can carry across those switches. Key the cache by endpoint and model, or keep it on the client.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code Fix in Cursor Fix in Conductor

logger.warning(
f"Model '{self.model}' rejected temperature parameter — retrying without it."
)
self._status(f"model '{self.model}' rejected temperature — retrying without it")
continue
Comment on lines +794 to +795

@greptile-apps greptile-apps Bot Oct 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Retry warning stays visible

The temperature retry notice stays set when the retry succeeds on its first attempt. Each call to _chat starts its attempt count over, but _status("") runs only when attempt > 0. The TUI keeps showing “retrying without it” during later work until the scan ends. Clear the notice when this retry recovers.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: openhack/agents/llm.py
Line: 794-795

Comment:
**Retry warning stays visible**

The temperature retry notice stays set when the retry succeeds on its first attempt. Each call to `_chat` starts its attempt count over, but `_status("")` runs only when `attempt > 0`. The TUI keeps showing “retrying without it” during later work until the scan ends. Clear the notice when this retry recovers.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code Fix in Cursor Fix in Conductor


raise

async def _chat(
self,
Expand All @@ -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.
Expand Down Expand Up @@ -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),
Expand Down
142 changes: 142 additions & 0 deletions tests/test_llm_retry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +153 to +154

@greptile-apps greptile-apps Bot Oct 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Tests leave shared flags changed

The new tests change shared state without restoring it. The combined test leaves _cache_key_unsupported true and the model in _temperature_unsupported_models. Later tests inherit those settings and may skip sending the parameters they are meant to check. Use a fixture with monkeypatch.setattr to give each test fresh flags and restore them afterward.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: tests/test_llm_retry.py
Line: 153-154

Comment:
**Tests leave shared flags changed**

The new tests change shared state without restoring it. The combined test leaves `_cache_key_unsupported` true and the model in `_temperature_unsupported_models`. Later tests inherit those settings and may skip sending the parameters they are meant to check. Use a fixture with `monkeypatch.setattr` to give each test fresh flags and restore them afterward.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code Fix in Cursor Fix in Conductor

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]