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.
This commit is contained in:
danielhanchen 2026-07-25 06:45:47 +00:00
commit b6dfd67b72
2 changed files with 37 additions and 3 deletions

View file

@ -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

View file

@ -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