Merge pull request 'Render one assistant message per turn, and make the request timeout a setting' (#51) from fix-transcript-and-timeout into main

docs/Tuning-Guide.md

@@ -132,6 +132,21 @@ a local model can be held to less without affecting the frontier roles.
   executor. Frontier planning wants the large default. Leave this high, and
   set a local model's smaller ceiling as its `max_output_tokens` in LiteLLM.
 
+### `llm.request_timeout_s` — default 600
+
+How long one model request may take before it is abandoned. It applies to
+every request, on both routes.
+
+- **Too low:** a slow model's long answer is cut off, and the executor
+  escalates work that was going well. At 120 s, the old fixed value, a dense
+  27B model on the RTX 4090 timed out writing one module in one turn.
+- **Too high:** a hung connection holds the milestone for that long before
+  anything notices.
+- **Deciding:** this is a safety net, not a budget. `max_output_tokens` and
+  `max_turns` bound the work; set this above the slowest single answer you
+  expect, which is the model's output ceiling divided by its generation
+  speed.
+
 ### `tuning.executor.max_turns` — default 24
 
 How many model calls one milestone may make before the executor stops with

nisse-dev.example.yaml

@@ -18,6 +18,9 @@ llm:                                  # A5: LiteLLM is the only LLM destination
   # Ceiling on each response. The model's own max_output_tokens in LiteLLM
   # caps it further, which is where to hold a local model to less.
   max_output_tokens: 16384
+  # How long one request may take before it is abandoned. A safety net for a
+  # hung connection; slow local models can take minutes over a long answer.
+  request_timeout_s: 600
 
 roles:                                # A11: role -> LiteLLM model alias
   executor: local-fast          # the local tier; the benchmark picks the model

nisse-dev/src/nisse_dev/agent.py

@@ -144,7 +144,10 @@ def make_llm_clients(
 
     normalized = budgeted(
         LiteLLMClient(
-            config.llm_base_url, config.llm_token, max_tokens=config.max_output_tokens
+            config.llm_base_url,
+            config.llm_token,
+            max_tokens=config.max_output_tokens,
+            timeout_s=config.request_timeout_s,
         )
     )
     # Delegations prefer the vendor-native route; fidelity is cheap
@@ -152,7 +155,10 @@ def make_llm_clients(
     # select_client, not by where the caller happens to sit.
     passthrough = budgeted(
         AnthropicPassthroughClient(
-            config.llm_base_url, config.llm_token, max_tokens=config.max_output_tokens
+            config.llm_base_url,
+            config.llm_token,
+            max_tokens=config.max_output_tokens,
+            timeout_s=config.request_timeout_s,
         )
     )
     return normalized, passthrough

nisse-dev/src/nisse_dev/config.py

@@ -23,7 +23,7 @@ from typing import Any
 
 import yaml
 
-from nisse_dev.llm import DEFAULT_MAX_OUTPUT_TOKENS, RoleMap
+from nisse_dev.llm import DEFAULT_MAX_OUTPUT_TOKENS, DEFAULT_REQUEST_TIMEOUT_S, RoleMap
 from nisse_dev.matrix import MatrixConfig
 
 DEFAULT_CONFIG_FILE = "nisse-dev.yaml"
@@ -48,7 +48,7 @@ REQUIRED_KEYS = (
     "gitea.repo",
     "gitea.operators",
 )
-OPTIONAL_KEYS = ("llm.max_output_tokens", "gitea.base_branch")
+OPTIONAL_KEYS = ("llm.max_output_tokens", "llm.request_timeout_s", "gitea.base_branch")
 
 _REFERENCE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}")
 
@@ -221,6 +221,7 @@ class AgentConfig:
     gitea_operators: frozenset[str]
     base_branch: str = "main"
     max_output_tokens: int = DEFAULT_MAX_OUTPUT_TOKENS
+    request_timeout_s: float = DEFAULT_REQUEST_TIMEOUT_S
     tuning: Tuning = field(default_factory=Tuning)
     config_file: Path | None = None
 
@@ -291,6 +292,14 @@ class AgentConfig:
                 raise ValueError
         except (TypeError, ValueError):
             errors.append("llm.max_output_tokens: must be a positive integer")
+
+        request_timeout = values.get("llm.request_timeout_s", DEFAULT_REQUEST_TIMEOUT_S)
+        try:
+            request_timeout = float(request_timeout)
+            if request_timeout <= 0:
+                raise ValueError
+        except (TypeError, ValueError):
+            errors.append("llm.request_timeout_s: must be a positive number of seconds")
         if errors:
             raise ConfigError("cannot start: " + "; ".join(errors))
 
@@ -319,6 +328,7 @@ class AgentConfig:
             gitea_operators=_people(values["gitea.operators"]),
             base_branch=str(values.get("gitea.base_branch") or "main"),
             max_output_tokens=max_output,
+            request_timeout_s=request_timeout,
             tuning=tuning,
             config_file=config_file,
         )

nisse-dev/src/nisse_dev/llm.py

@@ -46,6 +46,12 @@ RATE_LIMIT_RETRY_S = 20.0
 # was observed at 4,064 against a 4,096 limit.
 DEFAULT_MAX_OUTPUT_TOKENS = 16384
 
+# How long one request may take before it is abandoned (llm.request_timeout_s).
+# A safety net for a hung connection, not a budget: a dense local model
+# writing a whole module in one turn was measured taking over 120 s, and
+# timing it out failed work that was going well.
+DEFAULT_REQUEST_TIMEOUT_S = 600.0
+
 
 class UpstreamUnavailable(LLMError):
     """The proxy or provider could not be reached, or refused for reasons
@@ -124,7 +130,7 @@ class LiteLLMClient:
         base_url: str,
         token: str,
         *,
-        timeout_s: float = 120.0,
+        timeout_s: float = DEFAULT_REQUEST_TIMEOUT_S,
         transport: httpx.BaseTransport | None = None,
         sleep: Any = time.sleep,
         max_tokens: int | None = None,
@@ -250,7 +256,7 @@ class AnthropicPassthroughClient:
         base_url: str,
         token: str,
         *,
-        timeout_s: float = 300.0,
+        timeout_s: float = DEFAULT_REQUEST_TIMEOUT_S,
         max_tokens: int | None = None,
         transport: httpx.BaseTransport | None = None,
         sleep: Any = time.sleep,

nisse-dev/src/nisse_dev/render.py

@@ -60,7 +60,7 @@ class OpenAIChatRenderer:
         kept = effective_events(events)
         answered = {e.event.call_id for e in kept if isinstance(e.event, ToolResult)}
         for stored in kept:
-            messages.append(self._message_for(stored.event))
+            _append_merged(messages, self._message_for(stored.event))
             # Providers reject a tool call with no result following it. One
             # can legitimately be missing: a crash between the call and its
             # result (A3), or a turn-ending gate tool. Render a placeholder so
@@ -128,3 +128,28 @@ class OpenAIChatRenderer:
                 }
             case _:
                 raise ValueError(f"unrenderable event: {event!r}")
+
+
+def _append_merged(messages: list[dict[str, Any]], message: dict[str, Any]) -> None:
+    """Append, folding into the previous message when both are the same user
+    or assistant role.
+
+    One model turn is often several events (some text, then one or more tool
+    calls), and the OpenAI format carries a turn as one assistant message
+    with a tool_calls list. Strict chat templates enforce that: Devstral's
+    (Mistral) rejected a turn rendered as a text message then a tool-call
+    message with a 500, failing runs whenever the model spoke before acting."""
+    previous = messages[-1] if messages else None
+    role = message["role"]
+    if (
+        previous is None
+        or previous["role"] != role
+        or role not in ("user", "assistant")
+    ):
+        messages.append(message)
+        return
+    texts = [t for t in (previous.get("content"), message.get("content")) if t]
+    previous["content"] = "\n\n".join(texts) if texts else None
+    calls = previous.get("tool_calls", []) + message.get("tool_calls", [])
+    if calls:
+        previous["tool_calls"] = calls

nisse-dev/tests/test_config.py

@@ -5,6 +5,7 @@ from typing import Any
 import pytest
 
 from nisse_dev.config import AgentConfig, ConfigError
+from nisse_dev.llm import DEFAULT_REQUEST_TIMEOUT_S
 
 # A complete config as the YAML file would give it, with secrets by reference.
 COMPLETE: dict[str, Any] = {
@@ -122,6 +123,25 @@ def test_max_output_tokens_is_configurable_and_checked() -> None:
         build(llm__max_output_tokens=0)
 
 
+def test_request_timeout_is_configurable_and_checked() -> None:
+    assert build().request_timeout_s == DEFAULT_REQUEST_TIMEOUT_S
+    assert build(llm__request_timeout_s=900).request_timeout_s == 900.0
+    for bad in (0, -5, "soon"):
+        with pytest.raises(ConfigError, match="llm.request_timeout_s"):
+            build(llm__request_timeout_s=bad)
+
+
+def test_the_request_timeout_reaches_both_clients() -> None:
+    from nisse_dev.agent import make_llm_clients
+    from nisse_dev.catalog import ModelCatalog
+
+    normalized, passthrough = make_llm_clients(
+        build(llm__request_timeout_s=42), ModelCatalog({})
+    )
+    for client in (normalized, passthrough):
+        assert client._inner._http.timeout.read == 42.0
+
+
 def test_redacted_summary_carries_no_secrets() -> None:
     summary = build().redacted()
     flat = " ".join(summary.values())

nisse-dev/tests/test_render.py

@@ -1,4 +1,5 @@
 import json
+from itertools import pairwise
 
 from nisse_dev.canonical import (
     AssistantText,
@@ -68,7 +69,8 @@ def test_compaction_replaces_its_range_only() -> None:
     assert [e.id for e in remaining] == [3, 4]
     body = OpenAIChatRenderer().render(events, tools=None, model="q")
     contents = [m["content"] for m in body["messages"]]
-    assert contents == ["kept", "[summary of earlier conversation]\nthe old stuff"]
+    # Both render as user messages, so they share one: roles must alternate.
+    assert contents == ["kept\n\n[summary of earlier conversation]\nthe old stuff"]
 
 
 def test_tool_call_without_a_result_gets_a_placeholder() -> None:
@@ -103,3 +105,47 @@ def test_gate_event_renders_as_context() -> None:
     body = OpenAIChatRenderer().render(events, tools=None, model="q")
     assert body["messages"][0]["role"] == "user"
     assert "arch" in body["messages"][0]["content"]
+
+
+def _roles(body: dict) -> list[str]:
+    return [m["role"] for m in body["messages"]]
+
+
+def test_one_turn_is_one_assistant_message() -> None:
+    # Text and two parallel tool calls from one model turn: the OpenAI format
+    # carries them as a single assistant message with a tool_calls list.
+    events = stored(
+        [
+            UserMessage(text="go", source="m:1"),
+            AssistantText(text="Looking around first.", model="q"),
+            ToolCall(call_id="a", tool="list_dir", arguments={"path": "."}),
+            ToolCall(call_id="b", tool="read_file", arguments={"path": "README.md"}),
+            ToolResult(call_id="a", result="README.md", success=True),
+            ToolResult(call_id="b", result="hello", success=True),
+        ]
+    )
+    body = OpenAIChatRenderer().render(events, tools=None, model="q")
+    assert _roles(body) == ["user", "assistant", "tool", "tool"]
+    turn = body["messages"][1]
+    assert turn["content"] == "Looking around first."
+    assert [c["id"] for c in turn["tool_calls"]] == ["a", "b"]
+
+
+def test_roles_never_repeat_for_strict_templates() -> None:
+    """Mistral's chat template raises on two user or two assistant messages
+    in a row; every mix of events must render without one."""
+    events = stored(
+        [
+            UserMessage(text="one", source="m:1"),
+            UserMessage(text="two", source="m:2"),
+            AssistantText(text="thinking", model="q"),
+            ToolCall(call_id="a", tool="list_dir", arguments={}),
+            ToolResult(call_id="a", result="x", success=True),
+            AssistantText(text="more", model="q"),
+            AssistantText(text="and more", model="q"),
+            ToolCall(call_id="b", tool="finish", arguments={}),
+        ]
+    )
+    roles = _roles(OpenAIChatRenderer().render(events, tools=None, model="q"))
+    for first, second in pairwise(roles):
+        assert not (first == second and first in ("user", "assistant")), roles