Merge pull request #6000 from nesquena/stage/5990-false-noresponse

Release exp: no false no-response after final tool (#5990, closes #5686)

CHANGELOG.md

@@ -13,6 +13,8 @@
 
 ### Fixed
 
+- **No more false "No response from provider" after a completed answer that used tools.** When an assistant turn ended with a real final answer delivered as structured content following tool calls (common with reasoning models and multi-tool turns), the completion detector could miss the final text and render a spurious "No response from provider" card under the finished reply. Final-answer detection now understands structured content parts (text after tool-use boundaries), while genuine empty completions still correctly show the no-response notice, and a real terminal provider error still surfaces as an error (never masked as a final answer). Thanks @jtstothard. (#5990, #5686)
+
 - **Cron unread ("Tasks") dots are now scoped to the active profile — no more ghost dots after switching.** In multi-profile setups, the session-sidebar unread dot and the Tasks badge could keep showing a cron completion from a *different* profile after you switched, and the count could resurrect from a stale poll. Cron completion markers now carry their source + owning profile, are cleared for the profile you switch away from (including legacy/untagged markers), and a previous-profile poll response can no longer restore a stale unread. Non-cron completion unread and the current profile's own cron unread are preserved; default ↔ renamed-root aliases are treated as the same profile so a root cron marker is neither wrongly cleared nor wrongly kept. Thanks @jbbottoms. (#5975, #5960)
 
 - **The Docker "manual update" banner instruction is now localized.** On Docker/no-git deployments the update banner shows the manual update command; that guidance string was hardcoded in English. It now goes through the i18n system, so it renders in the active language (English fallback where a translation isn't present yet). Thanks @jbbottoms. (#5973, #5959)

api/streaming.py

@@ -2565,6 +2565,15 @@ def _looks_invalid_generated_title(text: str) -> bool:
     )
 
 
+def _message_content_part_text(part) -> str:
+    """Extract visible text from a structured content part."""
+    if not isinstance(part, dict):
+        return ''
+    return str(
+        part.get('text') or part.get('content') or part.get('input_text') or part.get('output_text') or ''
+    )
+
+
 def _message_text(value) -> str:
     """Extract plain text from mixed message content payloads."""
     if isinstance(value, list):
@@ -2574,11 +2583,49 @@ def _message_text(value) -> str:
                 continue
             ptype = str(p.get('type') or '').lower()
             if ptype in ('', 'text', 'input_text', 'output_text'):
-                parts.append(str(p.get('text') or p.get('content') or ''))
+                parts.append(_message_content_part_text(p))
         return _strip_thinking_markup('\n'.join(parts).strip())
     return _strip_thinking_markup(str(value or '').strip())
 
 
+def _assistant_content_part_is_tool_use(part) -> bool:
+    """Return True when a content[] part represents a tool invocation boundary."""
+    if not isinstance(part, dict):
+        return False
+    part_type = str(part.get('type') or '').lower()
+    if part_type in {'tool_use', 'tool_call'}:
+        return True
+    if part_type:
+        return False
+    if _message_content_part_text(part).strip():
+        return False
+    return any(key in part for key in ('tool_use_id', 'tool_call_id', 'call_id')) and any(
+        key in part for key in ('name', 'tool_name', 'input', 'args')
+    )
+
+
+def _assistant_message_has_final_visible_text(message) -> bool:
+    """Return True when an assistant row carries a settled visible answer."""
+    if not isinstance(message, dict) or message.get('role') != 'assistant':
+        return False
+    content = message.get('content', '')
+    if isinstance(content, list):
+        last_tool_idx = -1
+        for idx, part in enumerate(content):
+            if _assistant_content_part_is_tool_use(part):
+                last_tool_idx = idx
+        if last_tool_idx >= 0:
+            tail_parts = content[last_tool_idx + 1:]
+            return bool(_message_text(tail_parts).strip())
+        if message.get('tool_calls'):
+            return False
+        return bool(_message_text(content).strip())
+    if message.get('tool_calls'):
+        return False
+    return bool(_message_text(content).strip())
+
+
+
 _WORKSPACE_PREFIX_RE = re.compile(r'^\s*\[Workspace::v1:\s*(?:\\.|[^\]\\])+\]\s*')
 _LEGACY_WORKSPACE_PREFIX_RE = re.compile(r'^\s*\[Workspace:[^\]]+\]\s*')
 _WORKSPACE_PREFIX_ANY_RE = re.compile(r'\[Workspace::v1:\s*(?:\\.|[^\]\\])+\]\s*')
@@ -4873,7 +4920,7 @@ def _raw_message_text(value) -> str:
     """
     if isinstance(value, list):
         return ' '.join(
-            str(p.get('text') or p.get('content') or '')
+            _message_content_part_text(p)
             for p in value
             if isinstance(p, dict)
         )
@@ -5582,7 +5629,7 @@ def _assistant_reply_added_after_current_turn(result_messages, previous_context,
         isinstance(m, dict)
         and m.get('role') == 'assistant'
         and not m.get('_error')
-        and str(m.get('content') or '').strip()
+        and _assistant_message_has_final_visible_text(m)
         for m in candidates
     )
 
@@ -5600,18 +5647,7 @@ def _session_lacks_final_assistant_answer(messages) -> bool:
         if role == 'tool':
             return True
         if role == 'assistant':
-            content = msg.get('content')
-            if isinstance(content, list):
-                text = '\n'.join(
-                    str(part.get('text') or part.get('content') or '')
-                    for part in content
-                    if isinstance(part, dict)
-                )
-            else:
-                text = str(content or '')
-            if msg.get('tool_calls'):
-                return True
-            if text.strip():
+            if _assistant_message_has_final_visible_text(msg):
                 return False
             continue
         if role == 'user':
@@ -8764,7 +8800,8 @@ def _run_agent_streaming(
                 # the result/agent, use the captured message so the classifier can
                 # surface the real cause (model_not_found / auth) instead of the
                 # misleading no_response "silent rate limit, try again" fallback.
-                if not _last_err and _captured_terminal_error[0]:
+                _captured_terminal_failure = bool(_captured_terminal_error[0])
+                if not _last_err and _captured_terminal_failure:
                     _last_err = _captured_terminal_error[0]
                 _classification = _classify_provider_error(
                     str(_last_err) if _last_err else '',
@@ -8774,7 +8811,8 @@ def _run_agent_streaming(
                 _is_quota = _classification['type'] == 'quota_exhausted'
                 _is_auth = _classification['type'] == 'auth_mismatch'
                 _drop_replayed_assistant = (
-                    _agent_result_terminal_failure(result)
+                    _captured_terminal_failure
+                    or _agent_result_terminal_failure(result)
                     or bool(getattr(agent, '_last_error', None))
                     or ('error' in result and result.get('error') is not None)
                 )
@@ -8788,7 +8826,8 @@ def _run_agent_streaming(
                 )
                 _is_agent_result_terminal = _agent_result_terminal_failure(result)
                 _terminal_failure = (
-                    _is_agent_result_terminal
+                    _captured_terminal_failure
+                    or _is_agent_result_terminal
                     or (
                         _saved_transcript_lacks_final_answer
                         and _classification['type'] not in {'cancelled', 'interrupted'}

tests/test_issue5121_provider_auth_terminal_error.py

@@ -329,6 +329,53 @@ def test_auth_401_seeded_replayed_assistant_does_not_satisfy_current_turn(tmp_pa
     assert not any(event == "done" for event, _ in events)
 
 
+def test_captured_terminal_http_400_beats_structured_final_answer(tmp_path, monkeypatch):
+    session = _prepare_session("captured_terminal_http_400", "stream_captured_terminal_http_400", pending_user_message="Please use the tool")
+
+    class CapturedTerminalHttp400Agent(MockAgent):
+        def __init__(self, status_callback=None, **kwargs):
+            super().__init__(**kwargs)
+            self.status_callback = status_callback
+
+        def run_conversation(self, **kwargs):
+            history = list(kwargs.get("conversation_history") or [])
+            status_cb = getattr(self, "status_callback", None)
+            if status_cb is not None:
+                status_cb("lifecycle", "❌ Non-retryable error (HTTP 400): invalid model")
+            if self.stream_delta_callback is not None:
+                self.stream_delta_callback("Partial text before failure")
+            return {
+                "messages": history + [
+                    {
+                        "role": "assistant",
+                        "content": [
+                            {"type": "tool_use", "name": "weather.lookup", "input": {"city": "Leeds"}},
+                            {"type": "output_text", "output_text": "It is 18C and sunny."},
+                        ],
+                        "tool_calls": [{"id": "call_1", "type": "function", "function": {"name": "weather.lookup", "arguments": "{}"}}],
+                    }
+                ],
+                "error": "",
+            }
+
+    fake_queue = _run_stream(
+        monkeypatch,
+        session,
+        "stream_captured_terminal_http_400",
+        CapturedTerminalHttp400Agent,
+        workspace=str(tmp_path),
+    )
+    saved = Session.load("captured_terminal_http_400")
+    assert saved is not None
+
+    events = _queue_events(fake_queue)
+    apperrors = [data for event, data in events if event == "apperror"]
+    assert apperrors, "expected apperror for captured terminal HTTP 400"
+    assert apperrors[-1]["type"] == "model_not_found"
+    assert not any(event == "done" for event, _ in events)
+    assert saved.messages[-1]["_error"] is True
+
+
 def test_auth_retry_success_does_not_append_error_turn(tmp_path, monkeypatch):
     session = _prepare_session("auth_retry", "stream_auth_retry", pending_user_message="Please retry")
     agent_cls = _build_auth_failure_agent(token_text="")

tests/test_issue5686_completed_output_text.py

@@ -0,0 +1,105 @@
+from api import streaming
+
+
+PRIOR_DISPLAY = [
+    {"role": "user", "content": "Earlier request"},
+    {"role": "assistant", "content": "Earlier reply"},
+]
+
+
+class TestIssue5686CompletedOutputText:
+    def test_assistant_reply_added_after_current_turn_counts_visible_output_text_after_tool_metadata(self):
+        previous_context = list(PRIOR_DISPLAY) + [
+            {"role": "user", "content": "What is the weather?"},
+        ]
+        result_messages = list(previous_context) + [
+            {
+                "role": "assistant",
+                "content": [
+                    {"type": "tool_use", "name": "weather.lookup", "input": {"city": "Leeds"}},
+                    {"type": "output_text", "output_text": "It is 18C and sunny."},
+                ],
+                "tool_calls": [{"id": "call_1", "type": "function", "function": {"name": "weather.lookup", "arguments": "{}"}}],
+            }
+        ]
+        assert streaming._assistant_reply_added_after_current_turn(
+            result_messages,
+            previous_context,
+            "What is the weather?",
+        ) is True
+
+    def test_session_lacks_final_answer_false_when_visible_output_text_follows_tool_use(self):
+        messages = list(PRIOR_DISPLAY) + [
+            {"role": "user", "content": "What is the weather?"},
+            {
+                "role": "assistant",
+                "content": [
+                    {"type": "tool_use", "name": "weather.lookup", "input": {"city": "Leeds"}},
+                    {"type": "output_text", "output_text": "It is 18C and sunny."},
+                ],
+                "tool_calls": [{"id": "call_1", "type": "function", "function": {"name": "weather.lookup", "arguments": "{}"}}],
+            },
+        ]
+        assert streaming._session_lacks_final_assistant_answer(messages) is False
+
+    def test_session_lacks_final_answer_true_for_tool_only_assistant_content(self):
+        messages = list(PRIOR_DISPLAY) + [
+            {"role": "user", "content": "What is the weather?"},
+            {
+                "role": "assistant",
+                "content": [
+                    {"type": "tool_use", "name": "weather.lookup", "input": {"city": "Leeds"}},
+                ],
+                "tool_calls": [{"id": "call_1", "type": "function", "function": {"name": "weather.lookup", "arguments": "{}"}}],
+            },
+        ]
+        assert streaming._session_lacks_final_assistant_answer(messages) is True
+
+    def test_session_lacks_final_answer_false_for_untyped_visible_text_with_call_metadata(self):
+        messages = list(PRIOR_DISPLAY) + [
+            {"role": "user", "content": "What is the weather?"},
+            {
+                "role": "assistant",
+                "content": [
+                    {"call_id": "call_1", "name": "weather.lookup", "text": "It is 18C and sunny."},
+                ],
+            },
+        ]
+        assert streaming._session_lacks_final_assistant_answer(messages) is False
+
+    def test_session_lacks_final_answer_true_for_untyped_tool_part_with_empty_args(self):
+        messages = list(PRIOR_DISPLAY) + [
+            {"role": "user", "content": "What is the weather?"},
+            {
+                "role": "assistant",
+                "content": [
+                    {"call_id": "call_1", "args": {}},
+                ],
+            },
+        ]
+        assert streaming._session_lacks_final_assistant_answer(messages) is True
+
+    def test_session_lacks_final_answer_true_for_top_level_tool_calls_without_in_content_boundary(self):
+        messages = list(PRIOR_DISPLAY) + [
+            {"role": "user", "content": "What is the weather?"},
+            {
+                "role": "assistant",
+                "content": [
+                    {"type": "text", "text": "Checking the tool result"},
+                ],
+                "tool_calls": [{"id": "call_1", "type": "function", "function": {"name": "weather.lookup", "arguments": "{}"}}],
+            },
+        ]
+        assert streaming._session_lacks_final_assistant_answer(messages) is True
+
+    def test_session_lacks_final_answer_false_for_plain_output_text_parts(self):
+        messages = list(PRIOR_DISPLAY) + [
+            {"role": "user", "content": "Summarize"},
+            {
+                "role": "assistant",
+                "content": [
+                    {"type": "output_text", "output_text": "Summary complete."},
+                ],
+            },
+        ]
+        assert streaming._session_lacks_final_assistant_answer(messages) is False

tests/test_issue5940_terminal_error_surfaced.py

@@ -76,5 +76,6 @@ def test_captured_terminal_error_seeds_last_err_on_completion():
     """At turn completion, the captured terminal error must seed `_last_err` when
     the agent/result carried no error — so the classifier runs on the real cause
     instead of silent_failure=True."""
-    assert "if not _last_err and _captured_terminal_error[0]:" in STREAMING_PY
+    assert "_captured_terminal_failure = bool(_captured_terminal_error[0])" in STREAMING_PY
+    assert "if not _last_err and _captured_terminal_failure:" in STREAMING_PY
     assert "_last_err = _captured_terminal_error[0]" in STREAMING_PY