mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-26 14:45:33 -04:00
fix(tasks): normalize task endpoint URL to /chat/completions before model call (#4619)
Upstream bug (present in pewdiepie-archdaemon/odysseus main): the task executor passes task.endpoint_url VERBATIM to the model HTTP call, unlike the chat path which stores build_chat_url(normalize_base(base)) on the session. A task carrying an explicit bare OpenAI-compatible base such as "http://host:11434/v1" therefore POSTs to a 404 ("page not found"); the agent loop swallows the empty body into "The model returned an empty response" and marks the run success, so nothing surfaces the failure. Tasks that omit an endpoint dodge this only because _resolve_defaults() cribs an already-full URL from a recent chat session. The API/token path (e.g. an external client that POSTs /api/tasks with endpoint_url=".../v1") hits it every time. Fix: route every resolved task endpoint through _normalize_chat_endpoint() at the three resolution sites (_execute_llm_task, the persona/research session path, and _execute_research_task). The helper is idempotent (strips any existing chat suffix, re-appends the correct one) and leaves native-Ollama (/api...) and already-concrete URLs untouched, so other providers are unaffected. Proven via isolated repro: ".../v1" -> 404 -> empty; ".../v1/chat/completions" -> 200 -> real gemma4:31b output. Regression test asserts the bare-/v1 -> full-chat-URL mapping, idempotency, and the native-Ollama/empty passthroughs. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -289,6 +289,42 @@ def _checkin_calendar_events(db, owner, start, end):
|
||||
)
|
||||
|
||||
|
||||
def _normalize_chat_endpoint(url: str) -> str:
|
||||
"""Repair a resolved task endpoint to a full chat-completions URL.
|
||||
|
||||
Unlike the chat path — which stores ``build_chat_url(normalize_base(base))``
|
||||
on the session — the task executor passes ``task.endpoint_url`` verbatim to
|
||||
the model HTTP call. A bare OpenAI-compatible base such as
|
||||
``http://host:11434/v1`` therefore POSTs to a 404 ("page not found") and the
|
||||
model silently appears to "return an empty response".
|
||||
|
||||
Repair only bare OpenAI-compatible bases. Native-Ollama URLs (``/api...``)
|
||||
and URLs that already point at a concrete endpoint are returned untouched, so
|
||||
their own downstream normalizers keep working. Idempotent: a URL already
|
||||
ending in ``/chat/completions`` is left as-is.
|
||||
"""
|
||||
if not url:
|
||||
return url
|
||||
# Imports kept function-local (endpoint_resolver pulls in heavy deps) but
|
||||
# OUTSIDE the try: an import failure is a real bug that should surface, not
|
||||
# be silently swallowed into the un-normalized URL this function exists to
|
||||
# repair.
|
||||
from urllib.parse import urlparse
|
||||
from src.endpoint_resolver import normalize_base, build_chat_url
|
||||
path = (urlparse(url).path or "").rstrip("/")
|
||||
if path == "/api" or path.startswith("/api/"):
|
||||
return url # native Ollama — handled by the native path downstream
|
||||
if path.endswith(("/chat/completions", "/messages", "/responses", "/completions")):
|
||||
return url # already a concrete endpoint
|
||||
try:
|
||||
return build_chat_url(normalize_base(url))
|
||||
except Exception:
|
||||
# Guard only the actual normalization. Returning the URL un-normalized
|
||||
# reverts to the 404 this fixes, so make the silent revert visible.
|
||||
logger.debug("task endpoint normalization failed for %r; using as-is", url, exc_info=True)
|
||||
return url
|
||||
|
||||
|
||||
class TaskScheduler:
|
||||
def __init__(self, session_manager):
|
||||
self._session_manager = session_manager
|
||||
@@ -1357,6 +1393,7 @@ class TaskScheduler:
|
||||
endpoint_url, model = self._resolve_defaults(db, task.owner)
|
||||
if not endpoint_url or not model:
|
||||
raise RuntimeError("No model/endpoint configured")
|
||||
endpoint_url = _normalize_chat_endpoint(endpoint_url)
|
||||
# Record the resolved model so _execute_task_locked can persist it on
|
||||
# the run (tasks rarely pin a model, so this is the only record of
|
||||
# which model actually produced the output).
|
||||
@@ -1548,6 +1585,8 @@ class TaskScheduler:
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
endpoint_url = _normalize_chat_endpoint(endpoint_url)
|
||||
|
||||
session_id = task.session_id
|
||||
if not session_id:
|
||||
session_id = str(uuid.uuid4())
|
||||
@@ -1821,6 +1860,7 @@ class TaskScheduler:
|
||||
endpoint_url, model = self._resolve_defaults(db, task.owner)
|
||||
if not endpoint_url or not model:
|
||||
raise RuntimeError("No model/endpoint configured for research")
|
||||
endpoint_url = _normalize_chat_endpoint(endpoint_url)
|
||||
# Record the resolved model for the run record (see _execute_task_locked).
|
||||
self._last_run_model = model
|
||||
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
"""Regression test for the task-path endpoint-URL normalization fix.
|
||||
|
||||
Bug: the task executor passed ``task.endpoint_url`` verbatim to the model HTTP
|
||||
call (unlike the chat path, which normalizes via ``build_chat_url``). A bare
|
||||
OpenAI-compatible base such as ``http://host:11434/v1`` POSTed to a 404 and the
|
||||
run silently reported "The model returned an empty response".
|
||||
|
||||
The fix routes every resolved task endpoint through ``_normalize_chat_endpoint``.
|
||||
"""
|
||||
from src.task_scheduler import _normalize_chat_endpoint
|
||||
|
||||
|
||||
def test_bare_v1_base_gets_chat_completions_suffix():
|
||||
# The exact failure case: a bare /v1 base must become a full chat URL.
|
||||
assert (
|
||||
_normalize_chat_endpoint("http://localhost:11434/v1")
|
||||
== "http://localhost:11434/v1/chat/completions"
|
||||
)
|
||||
|
||||
|
||||
def test_full_chat_url_is_unchanged_idempotent():
|
||||
full = "http://localhost:11434/v1/chat/completions"
|
||||
assert _normalize_chat_endpoint(full) == full
|
||||
# Idempotent under repeated application.
|
||||
assert _normalize_chat_endpoint(_normalize_chat_endpoint(full)) == full
|
||||
|
||||
|
||||
def test_native_ollama_url_left_alone():
|
||||
# Native Ollama (/api...) has its own downstream normalizer — don't touch it.
|
||||
assert _normalize_chat_endpoint("http://localhost:11434/api") == "http://localhost:11434/api"
|
||||
assert _normalize_chat_endpoint("http://localhost:11434/api/chat") == "http://localhost:11434/api/chat"
|
||||
|
||||
|
||||
def test_empty_and_none_are_passthrough():
|
||||
assert _normalize_chat_endpoint("") == ""
|
||||
assert _normalize_chat_endpoint(None) is None
|
||||
|
||||
|
||||
def test_trailing_slash_base_normalized():
|
||||
assert (
|
||||
_normalize_chat_endpoint("http://localhost:11434/v1/")
|
||||
== "http://localhost:11434/v1/chat/completions"
|
||||
)
|
||||
Reference in New Issue
Block a user