Fix transformers /tasks failures: bigger budget, empty-answer salvage, and chat-message run logs (#48)

Every recent huggingface/transformers /tasks job was failing with "LLM returned unparseable output (finish_reason=None)" — 20/20 in a row. Investigation showed two compounding causes, and fixing them surfaced a logging gap and some UI rough edges. This PR bundles the whole fix.
Root cause

    Budget too small. Tasks run in strict mode (every tool call counts). 30 tool calls wasn't enough for a repo the size of transformers — the agent spent its whole budget exploring and never emitted a patch.
    No salvage for empty final answers. When the budget was exhausted, serge forced one tool-free final turn and handed its output straight to the parser. That turn came back empty with finish_reason=None (the provider truncating a huge-context stream), so it surfaced as "unparseable output". The existing salvage only handled finish_reason="length".

Changes

    Salvage empty / None-finish final answers (reviewer.py): both final-answer paths now re-ask once for the JSON only when the answer is empty or hit the output-token limit, bounded by _MAX_TRUNCATION_RETRIES.
    Raise the task tool budget 30 → 80 (prod.yaml) so large-repo fixes can converge.
    Log chat messages in the run transcript: _run_agentic_loop emits a chat event per turn (assistant content, reasoning length, finish_reason, tool-call names/args) plus truncated tool results — so unparseable-output-class failures are debuggable after the fact. chat, not message (the SSE default type). Persisted via PERSIST_EVENT_KINDS (token/reasoning still dropped).
    Web UI: assistant patch/review answers render human-readably (📝 title + body + "patch: N lines", or summary + inline-comment count) instead of raw JSON; tool results collapse to a single line with a size hint (full content stays in stored history); fixed doubled console lines on SSE reconnect via seq + Last-Event-ID resumption; widened the shared page container 960 → 1200px.

Author: tarekziade

deploy/helm/env/prod.yaml

@@ -9,7 +9,7 @@
 
 image:
   # Bump to a newer sha-<commit> (pushed by CI on merge to main) or use latest.
-  tag: sha-09bcf3a
+  tag: sha-bc6c73d
 
 replicas: 1
 
@@ -61,7 +61,14 @@ envVars:
   LLM_MAX_TOKENS: "49152"
   TOOL_MAX_ITERATIONS: "30"
   TASK_LLM_MAX_TOKENS: "49152"
-  TASK_TOOL_MAX_ITERATIONS: "30"
+  # Tasks run in STRICT budget mode (every tool call counts, not just blind
+  # turns — see reviewer._run_tool_loop), to preserve output budget for the
+  # final patch JSON. 30 was too tight for a repo the size of transformers:
+  # the agent spent the whole budget exploring and never emitted a patch, so
+  # the forced tool-free final turn returned empty/unparseable output. Give
+  # large-repo fixes room to converge. Higher = more exploration but larger
+  # per-turn context (cost + latency); tune against the target repo.
+  TASK_TOOL_MAX_ITERATIONS: "80"
   # Normalize gate for /tasks. In the pod-per-task model the runner runs this
   # command in-process (formerly the separate normalize Job). Unset = gate is
   # skipped entirely, so it MUST be set. Argv is shlex-split; bash -lc runs the
@@ -105,7 +112,7 @@ taskExecution:
     # draft on the terminal callback. false = reviews run in-process in serge.
     reviewPods: true
     # transformers toolchain + serge (docker/Dockerfile.task-runner).
-    image: ghcr.io/huggingface/serge-task-runner:sha-09bcf3a
+    image: ghcr.io/huggingface/serge-task-runner:sha-bc6c73d
     # Wall-clock cap per task (matches the old in-process worker budget).
     timeout: 3600
     # The in-pod normalizer (make style / checkers over transformers) is
@@ -115,4 +122,4 @@ taskExecution:
     nodeSelector: "scheduling.cast.ai/node-template=default-by-castai"
     egress:
       # Self-built tinyproxy (docker/Dockerfile.egress-proxy).
-      image: ghcr.io/huggingface/serge-egress:sha-09bcf3a
+      image: ghcr.io/huggingface/serge-egress:sha-bc6c73d

reviewbot/config.py

@@ -502,8 +502,7 @@ def from_env(
                 os.environ.get("TASK_K8S_NODE_SELECTOR") or ""
             ).strip()
             or None,
-            task_egress_name=(os.environ.get("TASK_EGRESS_NAME") or "").strip()
-            or None,
+            task_egress_name=(os.environ.get("TASK_EGRESS_NAME") or "").strip() or None,
             slack_bot_token=(
                 os.environ.get("SLACK_CIFEEDBACK_BOT_TOKEN")
                 or os.environ.get("CI_SLACK_BOT_TOKEN")

reviewbot/review_runner.py

@@ -35,9 +35,7 @@ def build_review_request(spec: RunnerSpec) -> ReviewRequest:
     only the fields the dataclass declares. ``inline`` (webhook follow-up
     context) is left at its default — UI reviews never carry it."""
     fields = {f.name for f in dataclasses.fields(ReviewRequest)}
-    kwargs = {
-        k: v for k, v in spec.request.items() if k in fields and k != "inline"
-    }
+    kwargs = {k: v for k, v in spec.request.items() if k in fields and k != "inline"}
     return ReviewRequest(**kwargs)
 
 

reviewbot/reviewer.py

@@ -656,6 +656,41 @@ def _merge_chunk_event(events: list[str], comments_count: int) -> str:
     "analysis, no explanation, no <think> block, no markdown fences. Keep it "
     "minimal so the whole answer fits within the output limit."
 )
+# An empty final answer (blank content, usually finish_reason=None) means the
+# model returned nothing parseable — often the provider truncated the stream on
+# a very large context. Re-ask once, tool-free, for just the JSON — same
+# recovery path as the length-truncation case, bounded by the same retry cap.
+_EMPTY_ANSWER_RECOVERY_MESSAGE = (
+    "Your previous reply was empty — no JSON object came through. Reply now "
+    "with ONLY the final JSON object the task requires: no analysis, no "
+    "explanation, no <think> block, no markdown fences."
+)
+
+
+def _needs_final_salvage(chat: ChatResult) -> bool:
+    """True when a tool-free final answer should be re-asked rather than
+    parsed: it either hit the output-token limit (``finish_reason="length"``)
+    or came back with blank content (commonly ``finish_reason=None`` when the
+    provider truncates the stream on a very large context)."""
+    return chat.finish_reason == "length" or not (chat.content or "").strip()
+
+
+def _final_recovery_message(chat: ChatResult) -> str:
+    blank = not (chat.content or "").strip()
+    return _EMPTY_ANSWER_RECOVERY_MESSAGE if blank else _TRUNCATION_RECOVERY_MESSAGE
+
+
+def _emit_final_salvage(
+    emit: Optional[Callable[[str, str], None]], chat: ChatResult, attempt: int
+) -> None:
+    if emit is None:
+        return
+    what = "empty" if not (chat.content or "").strip() else "truncated"
+    emit(
+        "log",
+        f"Final answer was {what} (finish_reason={chat.finish_reason}); re-asking "
+        f"for the JSON only (recovery {attempt}/{_MAX_TRUNCATION_RETRIES})",
+    )
 
 
 def _run_agentic_loop(
@@ -806,27 +841,35 @@ def _run_agentic_loop(
         if chat.completion_tokens is not None:
             metrics.completion_tokens += chat.completion_tokens
         _emit_metrics(emit, metrics)
+        # Log what the model emitted this turn (content + finish_reason +
+        # tool-call names). Captures the empty final turn behind
+        # "unparseable output" failures.
+        _emit_chat_message(
+            emit,
+            "assistant",
+            content=chat.content,
+            reasoning_chars=chat.reasoning_chars,
+            finish_reason=chat.finish_reason,
+            tool_calls=chat.tool_calls,
+        )
 
         if not chat.tool_calls:
-            # Salvage a truncated final answer before anything else: the model
-            # ran out of output budget mid-JSON (reasoning ate it). Re-ask for
-            # the JSON only, tool-less and low-reasoning, instead of returning
-            # unparseable content that fails the whole task.
+            # Salvage a truncated OR empty final answer before anything else:
+            # the model either ran out of output budget mid-JSON
+            # (finish_reason="length", reasoning ate it) or returned nothing
+            # parseable at all (blank content — commonly finish_reason=None when
+            # the provider truncates a huge-context stream). Re-ask for the JSON
+            # only, tool-less and low-reasoning, instead of returning content
+            # that just fails the parse.
             if (
-                chat.finish_reason == "length"
+                _needs_final_salvage(chat)
                 and truncation_retries < _MAX_TRUNCATION_RETRIES
             ):
                 truncation_retries += 1
-                if emit is not None:
-                    emit(
-                        "log",
-                        "Final answer hit the output-token limit "
-                        f"(recovery {truncation_retries}/{_MAX_TRUNCATION_RETRIES}); "
-                        "re-asking for the JSON only",
-                    )
+                _emit_final_salvage(emit, chat, truncation_retries)
                 messages.append({"role": "assistant", "content": chat.content or None})
                 messages.append(
-                    {"role": "user", "content": _TRUNCATION_RECOVERY_MESSAGE}
+                    {"role": "user", "content": _final_recovery_message(chat)}
                 )
                 force_json_only = True
                 continue
@@ -901,6 +944,7 @@ def _run_agentic_loop(
                     "content": result,
                 }
             )
+            _emit_chat_message(emit, "tool", content=result, tool_name=tc.name)
 
     # Iteration budget hit — force a final answer with tools disabled.
     # Only reachable when ``tool_max_iterations > 0`` and the model
@@ -942,6 +986,25 @@ def _run_agentic_loop(
         if chat.completion_tokens is not None:
             metrics.completion_tokens += chat.completion_tokens
         _emit_metrics(emit, metrics)
+        _emit_chat_message(
+            emit,
+            "assistant",
+            content=chat.content,
+            reasoning_chars=chat.reasoning_chars,
+            finish_reason=chat.finish_reason,
+        )
+
+        # Salvage an empty/truncated forced-final answer before validating or
+        # returning it. This is the exact failure the budget-exhausted path used
+        # to die on: an empty completion (finish_reason=None) went straight to
+        # the parser and surfaced as "LLM returned unparseable output". Re-ask
+        # for the JSON only instead (bounded by _MAX_TRUNCATION_RETRIES).
+        if _needs_final_salvage(chat) and truncation_retries < _MAX_TRUNCATION_RETRIES:
+            truncation_retries += 1
+            _emit_final_salvage(emit, chat, truncation_retries)
+            messages.append({"role": "assistant", "content": chat.content or None})
+            messages.append({"role": "user", "content": _final_recovery_message(chat)})
+            continue
 
         # The verification gate must run on the forced final answer too —
         # exhausting the tool budget must not silently bypass validation (for
@@ -1033,6 +1096,62 @@ def _emit_metrics(
     emit("metrics", payload)
 
 
+# Per-message log cap. Assistant turns are small (the model's own output),
+# but tool results can be large (a grepped file); truncate so the run log
+# stays debuggable without bloating the SQLite job store with the full
+# 500k-token context that already lives in the prompt.
+_LOG_MSG_MAX_CHARS = 2000
+
+
+def _emit_chat_message(
+    emit: Optional[Callable[[str, str], None]],
+    role: str,
+    *,
+    content: Optional[str] = None,
+    reasoning_chars: int = 0,
+    finish_reason: Optional[str] = None,
+    tool_calls: Optional[list[ToolCall]] = None,
+    tool_name: Optional[str] = None,
+) -> None:
+    """Record one chat turn in the run log as a ``message`` event.
+
+    Captures what the model actually emitted each turn — content,
+    reasoning length, finish_reason, tool-call names/args — plus truncated
+    tool results. Deliberately does NOT re-store the giant diff/user
+    context (it is the same every turn and already accounted for in the
+    metrics). This is what makes "LLM returned unparseable output"
+    failures diagnosable: the empty final turn (content="",
+    finish_reason=None) shows up here. Best-effort — never raises into the
+    agent loop."""
+    if emit is None:
+        return
+    payload: dict[str, Any] = {"role": role}
+    if content:
+        payload["content"] = (
+            content
+            if len(content) <= _LOG_MSG_MAX_CHARS
+            else content[:_LOG_MSG_MAX_CHARS]
+            + f"… [+{len(content) - _LOG_MSG_MAX_CHARS} chars truncated]"
+        )
+    if reasoning_chars:
+        payload["reasoning_chars"] = reasoning_chars
+    if finish_reason is not None:
+        payload["finish_reason"] = finish_reason
+    if tool_calls:
+        payload["tool_calls"] = [
+            {"name": tc.name, "arguments": _summarize_args_str(tc.arguments)}
+            for tc in tool_calls
+        ]
+    if tool_name:
+        payload["name"] = tool_name
+    try:
+        # kind "chat" (not "message"): "message" is the SSE default event type,
+        # which would double-fire the browser's onmessage handler.
+        emit("chat", json.dumps(payload))
+    except Exception:  # never let logging break a run
+        log.debug("failed to emit chat message", exc_info=True)
+
+
 def _assistant_tool_call_dict(tc: ToolCall) -> dict[str, Any]:
     """Serialize a ToolCall back into the OpenAI-compat assistant message.
     Re-attaches Gemini 3's thought_signature at the exact path the API

reviewbot/static/review.js

@@ -168,6 +168,73 @@
   const consolePending = [];
   let consoleFlushScheduled = false;
 
+  // A `message` event carries a JSON payload of what the model emitted that
+  // turn (see reviewer._emit_chat_message). Render it as a readable line.
+  function formatAssistantContent(content) {
+    if (!content) return "";
+    let o;
+    try {
+      o = JSON.parse(content);
+    } catch (_) {
+      return content;
+    }
+    if (!o || typeof o !== "object") return content;
+    // Task patch answer: {title, body, patch}.
+    if ("patch" in o || "title" in o) {
+      const out = [];
+      if (o.title) out.push(`📝 ${o.title}`);
+      if (o.body) out.push(String(o.body));
+      const patch = String(o.patch || "").trim();
+      out.push(
+        patch
+          ? `⟶ patch: ${patch.split("\n").length} lines`
+          : "⟶ no patch (no change proposed)",
+      );
+      return "\n   " + out.join("\n   ");
+    }
+    // Review answer: {summary, event, comments}.
+    if ("summary" in o || "comments" in o) {
+      const n = Array.isArray(o.comments) ? o.comments.length : 0;
+      const ev = o.event ? ` [${o.event}]` : "";
+      return `\n   ${o.summary || ""}${ev}\n   ⟶ ${n} inline comment${n === 1 ? "" : "s"}`;
+    }
+    return content;
+  }
+
+  function formatMessageEvent(text) {
+    let m;
+    try {
+      m = JSON.parse(text);
+    } catch (_) {
+      return text;
+    }
+    if (!m || typeof m !== "object") return text;
+    if (m.role === "tool") {
+      // Don't dump the (often large) tool output into the console; just note
+      // the tool and a size hint. Full result is in stored history.
+      const c = String(m.content || "");
+      const lines = c ? c.split("\n").length : 0;
+      const size = lines ? ` (${lines} line${lines === 1 ? "" : "s"})` : "";
+      return `tool ⟵ ${m.name || "?"}${size}`;
+    }
+    const meta = [];
+    if (m.finish_reason != null && m.finish_reason !== "stop") {
+      meta.push(`finish=${m.finish_reason}`);
+    }
+    if (m.reasoning_chars) meta.push(`reasoning=${m.reasoning_chars}c`);
+    const metaStr = meta.length ? ` [${meta.join(", ")}]` : "";
+    const calls =
+      Array.isArray(m.tool_calls) && m.tool_calls.length
+        ? " → " +
+          m.tool_calls
+            .map((t) => `${t.name}(${String(t.arguments || "").slice(0, 80)})`)
+            .join(", ")
+        : "";
+    const body = formatAssistantContent(m.content);
+    if (!body && !calls) return `assistant: (thinking…)${metaStr}`;
+    return `assistant:${body ? " " + body : ""}${calls}${metaStr}`;
+  }
+
   function flushConsole() {
     consoleFlushScheduled = false;
     if (consolePending.length === 0) return;
@@ -179,8 +246,10 @@
       let prefix = streamed || consoleAtLineStart ? "" : "\n";
       if (kind === "log") prefix += "› ";
       else if (kind === "tool") prefix += "⚙ ";
+      else if (kind === "chat") prefix += "💬 ";
       else if (kind === "error") prefix += "✗ ";
-      const body = prefix + text + (streamed ? "" : "\n");
+      const shown = kind === "chat" ? formatMessageEvent(text) : text;
+      const body = prefix + shown + (streamed ? "" : "\n");
       span.textContent = body;
       frag.appendChild(span);
       consoleAtLineStart = body.endsWith("\n");
@@ -671,6 +740,13 @@
         appendConsole("tool", ev.data);
       }
     });
+    es.addEventListener("chat", (ev) => {
+      try {
+        appendConsole("chat", JSON.parse(ev.data));
+      } catch {
+        appendConsole("chat", ev.data);
+      }
+    });
     es.addEventListener("reasoning", (ev) => {
       try {
         appendConsole("reasoning", JSON.parse(ev.data));

reviewbot/static/styles.css

@@ -37,7 +37,7 @@ body {
 }
 
 main {
-  max-width: 960px;
+  max-width: 1200px;
   margin: 0 auto;
   padding: 24px 16px 80px;
 }