diff --git a/CHANGELOG.md b/CHANGELOG.md index cb61aad82..97944c7e1 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/api/streaming.py b/api/streaming.py index 47a171790..4bc663e17 100644 --- a/api/streaming.py +++ b/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'} diff --git a/tests/test_issue5121_provider_auth_terminal_error.py b/tests/test_issue5121_provider_auth_terminal_error.py index 877c62bdc..2cdaf854c 100644 --- a/tests/test_issue5121_provider_auth_terminal_error.py +++ b/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="") diff --git a/tests/test_issue5686_completed_output_text.py b/tests/test_issue5686_completed_output_text.py new file mode 100644 index 000000000..b8d11b4a2 --- /dev/null +++ b/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 diff --git a/tests/test_issue5940_terminal_error_surfaced.py b/tests/test_issue5940_terminal_error_surfaced.py index b3769693c..9eefdfbda 100644 --- a/tests/test_issue5940_terminal_error_surfaced.py +++ b/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