fix(ollama): normalize hosts, errors, and reasoning output

Separate bind and client addresses, include Ollama's response body in HTTP failures, and strip inline <think> blocks from complete and streamed replies.
This commit is contained in:
2026-08-26 03:32:45 -05:00
parent d56d579755
commit 2ebe93b4f7
3 changed files with 297 additions and 16 deletions
+103 -5
View File
@@ -217,16 +217,23 @@ def test_icon_source_requires_real_allowed_file_boundary(tmp_path):
def test_ollama_stream_propagates_transport_errors(monkeypatch):
"""A failing stream must surface, not be swallowed into an empty reply —
and it must carry Ollama's own explanation, since that is the only part the
user can act on. The response here is a real httpx.Response because the
error path reads the body, which a stubbed raise_for_status never exercised."""
import httpx
class FailingResponse:
async def __aenter__(self):
return self
return httpx.Response(
503,
json={"error": "Ollama unavailable"},
request=httpx.Request("POST", "http://127.0.0.1:11434/api/chat"),
)
async def __aexit__(self, *args):
return False
def raise_for_status(self):
raise RuntimeError("Ollama unavailable")
class FailingClient:
def __init__(self, **kwargs):
pass
@@ -246,7 +253,7 @@ def test_ollama_stream_propagates_transport_errors(monkeypatch):
async for _ in OllamaManager()._chat_stream([], "model", 0):
pass
with pytest.raises(RuntimeError, match="Ollama unavailable"):
with pytest.raises(httpx.HTTPStatusError, match="Ollama unavailable"):
import asyncio
asyncio.run(consume())
@@ -537,3 +544,94 @@ def test_update_apply_spawns_detached_and_refuses_a_second_run(monkeypatch):
# Double-click must not launch a second pull/rebuild over the first.
assert client.post("/update/apply").json()["started"] is False
def test_ollama_failures_surface_the_reason_not_just_the_status():
"""Ollama answers every failure with {"error": "..."} and httpx's default
message throws it away. A user hitting a retired cloud model saw
"Client error '410 Gone' for url ..." when the body said exactly why."""
import asyncio
import httpx
import pytest
from synapse.ollama_manager import _raise_for_ollama
req = httpx.Request("POST", "http://127.0.0.1:11434/api/chat")
retired = httpx.Response(410, json={"error": "glm-4.6 was retired at 2026-06-16"}, request=req)
with pytest.raises(httpx.HTTPStatusError) as ei:
asyncio.run(_raise_for_ollama(retired))
assert "retired" in str(ei.value) and "410" in str(ei.value)
# The common case, not just the exotic one.
missing = httpx.Response(404, json={"error": "model 'foo' not found"}, request=req)
with pytest.raises(httpx.HTTPStatusError) as ei:
asyncio.run(_raise_for_ollama(missing))
assert "model 'foo' not found" in str(ei.value)
# No usable body -> keep httpx's own wording rather than inventing one.
blank = httpx.Response(500, content=b"", request=req)
with pytest.raises(httpx.HTTPStatusError):
asyncio.run(_raise_for_ollama(blank))
# Success stays silent.
asyncio.run(_raise_for_ollama(httpx.Response(200, json={"ok": True}, request=req)))
def test_ollama_host_is_normalized_for_clients_but_not_for_binding():
"""OLLAMA_HOST is Ollama's *bind* variable, and `OLLAMA_HOST=0.0.0.0:11434`
is the normal way to expose it on a LAN. Used verbatim as a client base URL
it is unusable — no scheme, and 0.0.0.0 is not a destination — and every
request failed into list_models()'s bare `except: return []`, so the model
picker just went empty with no error anywhere."""
from synapse.nexus_config import _normalize_ollama_host as norm
assert norm("0.0.0.0:11434") == "http://127.0.0.1:11434"
assert norm("[::]:11434") == "http://127.0.0.1:11434"
assert norm("http://0.0.0.0:11434/") == "http://127.0.0.1:11434"
assert norm("127.0.0.1:11434") == "http://127.0.0.1:11434" # scheme supplied
assert norm("") == "http://127.0.0.1:11434"
# A real remote is deliberate — leave it alone.
assert norm("https://ollama.lan:11434") == "https://ollama.lan:11434"
assert norm("192.168.1.50:11434") == "http://192.168.1.50:11434"
def test_spawned_serve_keeps_the_users_bind_address(monkeypatch):
"""Normalizing for the client must not quietly un-expose a server we spawn."""
import importlib
from synapse import nexus_config
monkeypatch.setenv("OLLAMA_HOST", "0.0.0.0:11434")
reloaded = importlib.reload(nexus_config)
try:
assert reloaded.settings.ollama_bind == "0.0.0.0:11434" # listens everywhere
assert reloaded.settings.ollama_host == "http://127.0.0.1:11434" # we connect here
finally:
monkeypatch.delenv("OLLAMA_HOST", raising=False)
importlib.reload(nexus_config)
def test_think_blocks_never_reach_the_reply():
"""A reasoning model emits <think> inline in `content` even with think off,
and the whole internal monologue reached the chat window — including the
literal closing tags. Streaming has to cope with a tag split across chunks,
and with the shape actually observed: a stray </think> and no opening."""
from synapse.ollama_manager import ThinkStripper, strip_think
def stream(chunks):
s = ThinkStripper()
return "".join(s.feed(c) for c in chunks) + s.flush()
assert stream(["hello ", "<think>", "noise", "</think>", "world"]) == "hello world"
# tag split across chunk boundaries
assert stream(["a<th", "ink>x</thi", "nk>b"]) == "ab"
# never closed -> it was all reasoning
assert stream(["keep", "<think>", "runs off the end"]) == "keep"
# stray close, no open: at minimum the tag itself must not be shown
assert "</think>" not in stream(["reasoning...", "</think>", "the answer"])
# ordinary text is untouched, including angle brackets
assert stream(["a < b ", "and c > d"]) == "a < b and c > d"
# With the complete message the stray-close case can be handled properly:
# everything before it was reasoning.
assert strip_think("rambling\n</think>\nThe answer") == "The answer"
assert strip_think("a<think>b</think>c") == "ac"
assert strip_think("no tags here") == "no tags here"