From b6dfd67b72a80cb2f75cf73f6e813d05d0ee4c26 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Sat, 25 Jul 2026 06:45:47 +0000 Subject: [PATCH] Cover MTP precedence and loosen the replay assertion for PR #7424 Add a regression test for the MTP branch of the tool-loop respawn retry: the file-wide _make_backend stub forces _maybe_recover_from_mtp_crash to False, so nothing exercised the case where an MTP crash reload is already claimed and an ordinary same-config respawn must not run on top of it. Cover both the next tool-loop request and the final synthesis pass. Replace the whole-payload equality assertions with a field-wise check. Comparing the full dict pins max_tokens to the value derived from the dead server's effective context, so a later fix that rebuilds server-derived defaults after a respawn would read as a test failure rather than an improvement. Document that the one-retry budget is per model request, not per chat turn. --- studio/backend/core/inference/llama_cpp.py | 3 +- .../backend/tests/test_llama_cpp_tool_loop.py | 37 ++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index a9e123bab6..da49de000f 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -10567,7 +10567,8 @@ class LlamaCppBackend: control has reached a streaming consumer that may have emitted content or tool events, so replaying the request could duplicate visible output or side effects. Resolve ``base_url`` on each attempt because a respawn may use a new - port. + port. The one-retry budget is per model request, not per chat turn, so a long + tool loop never discards a completed tool just because an earlier turn recovered. """ for attempt in range(2): response_opened = False diff --git a/studio/backend/tests/test_llama_cpp_tool_loop.py b/studio/backend/tests/test_llama_cpp_tool_loop.py index 1268d16210..35f95ccca9 100644 --- a/studio/backend/tests/test_llama_cpp_tool_loop.py +++ b/studio/backend/tests/test_llama_cpp_tool_loop.py @@ -119,6 +119,12 @@ def _patch_successful_respawn( return calls +def _assert_same_request(first: dict, second: dict) -> None: + """A replay resends the same request; server-derived defaults may be rebuilt.""" + for key in ("messages", "tools", "tool_choice", "temperature", "top_p", "seed"): + assert first.get(key) == second.get(key), key + + def _tool_names(payload: dict) -> list[str]: return [ (tool.get("function") or {}).get("name") @@ -2334,7 +2340,7 @@ def test_connect_error_before_tool_stream_respawns_and_retries(monkeypatch): assert respawn_calls == [True] assert len(payloads) == 2 - assert payloads[0] == payloads[1] + _assert_same_request(payloads[0], payloads[1]) assert urls == [ "http://127.0.0.1:48847/v1/chat/completions", "http://127.0.0.1:49999/v1/chat/completions", @@ -2379,7 +2385,7 @@ def test_connect_error_after_tool_result_recovers_both_generation_paths(monkeypa assert respawn_calls == [True] assert tool_calls == [("python", {"code": "print(1)"})] assert len(payloads) == 3 - assert payloads[1] == payloads[2] + _assert_same_request(payloads[1], payloads[2]) assert any(e.get("type") == "content" and e.get("text") == final_text for e in events) @@ -2416,6 +2422,33 @@ def test_connect_error_retry_is_bounded(monkeypatch): assert len(payloads) == 2 +def test_mtp_crash_recovery_wins_over_respawn(monkeypatch): + """An MTP crash reloads without MTP, so never respawn the same config on top.""" + import httpx + for max_tool_iterations in (2, 1): + payloads: list[dict] = [] + backend = _make_backend(monkeypatch, [httpx.ConnectError("mtp crash")], payloads) + monkeypatch.setattr(backend, "_maybe_recover_from_mtp_crash", lambda *_a, **_k: True) + respawn_calls = _patch_successful_respawn(monkeypatch, backend) + + raised = False + try: + list( + backend.generate_chat_completion_with_tools( + messages = [{"role": "user", "content": "hello"}], + tools = [{"type": "function", "function": {"name": "python"}}], + max_tool_iterations = max_tool_iterations, + ) + ) + except RuntimeError as exc: + raised = True + assert "Lost connection" in str(exc) + + assert raised + assert respawn_calls == [] + assert len(payloads) == 1 + + def test_empty_tool_call_id_does_not_emit_provisional_card(monkeypatch): """llama.cpp can stream a tool call whose id is an empty string. A provisional card keyed by "" cannot reconcile with the real tool_start (the frontend mints