diff --git a/api/streaming.py b/api/streaming.py index 8b294684..9c9ea54e 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -6512,7 +6512,10 @@ def _terminal_turn_duration(session, *, now: float | None = None) -> float | Non return None if not math.isfinite(started) or not math.isfinite(ended) or started <= 0: return None - return round(max(0.0, ended - started), 3) + elapsed = ended - started + if elapsed < 0: + return None + return round(elapsed, 3) def _build_partial_message(content_text, reasoning_text, tool_calls) -> dict | None: diff --git a/tests/test_issue3929_error_preserves_partial.py b/tests/test_issue3929_error_preserves_partial.py index 520bb54d..cda3931d 100644 --- a/tests/test_issue3929_error_preserves_partial.py +++ b/tests/test_issue3929_error_preserves_partial.py @@ -15,7 +15,7 @@ import api.streaming as streaming from api.models import Session -@pytest.mark.parametrize("started_at", [None, 0, -1, "invalid", float("nan"), float("inf")]) +@pytest.mark.parametrize("started_at", [None, 0, -1, "invalid", float("nan"), float("inf"), 101.0]) def test_terminal_turn_duration_omits_invalid_origin(started_at): session = types.SimpleNamespace(pending_started_at=started_at) @@ -179,6 +179,57 @@ def test_silent_failure_preserves_partials(tmp_path): assert err_msg.get("_turnDuration", 0) >= 12 +@pytest.mark.parametrize("started_at", [None, "future"]) +def test_result_error_omits_turn_duration_for_missing_or_future_origin(tmp_path, started_at): + """Result-error writeback must not manufacture durations from invalid timer origins.""" + label = "future" if started_at == "future" else "missing" + fake_session = Session(session_id=f"test_sess_silent_{label}", title="Test Session") + fake_session.pending_user_message = "What is python?" + fake_session.pending_started_at = time.time() + 30 if started_at == "future" else started_at + fake_session.active_stream_id = f"test_stream_silent_{label}" + fake_session.save() + models.SESSIONS[fake_session.session_id] = fake_session + + class SilentFailureAgent(MockAgent): + def run_conversation(self, **kwargs): + if self.stream_delta_callback: + self.stream_delta_callback("Python is a programming language.") + return { + "status": "error", + "error": "Silent failure details", + "messages": kwargs.get("conversation_history") or [], + } + + fake_queue = queue.Queue() + streaming.STREAMS[fake_session.active_stream_id] = fake_queue + config.STREAM_PARTIAL_TEXT[fake_session.active_stream_id] = "" + + with mock.patch.object(streaming, "get_session", return_value=fake_session), \ + mock.patch.object(streaming, "_get_ai_agent", return_value=SilentFailureAgent), \ + mock.patch.object(streaming, "resolve_model_provider", return_value=("test-model", "test-provider", None)), \ + mock.patch("api.config.get_config", return_value={}), \ + mock.patch("api.config._resolve_cli_toolsets", return_value=[]): + + streaming._run_agent_streaming( + session_id=fake_session.session_id, + msg_text="What is python?", + model="test-model", + workspace=str(tmp_path), + stream_id=fake_session.active_stream_id, + ) + + saved = Session.load(fake_session.session_id) + assert saved is not None + assert saved.messages[-1].get("_error") is True + assert "_turnDuration" not in saved.messages[-1] + + apperrors = [item[1] for item in list(fake_queue.queue) if item[0] == "apperror"] + assert apperrors + payload_error = apperrors[-1]["session"]["messages"][-1] + assert payload_error.get("_error") is True + assert "_turnDuration" not in payload_error + + def test_exception_preserves_partials(tmp_path): """Test that an unhandled exception preserves partial streamed text.""" fake_session = Session(session_id="test_sess_exc", title="Test Session") @@ -225,6 +276,47 @@ def test_exception_preserves_partials(tmp_path): assert err_msg.get("_turnDuration", 0) >= 9 +@pytest.mark.parametrize("started_at", [None, "future"]) +def test_exception_error_omits_turn_duration_for_missing_or_future_origin(tmp_path, started_at): + """Exception writeback must not persist durations from invalid timer origins.""" + label = "future" if started_at == "future" else "missing" + fake_session = Session(session_id=f"test_sess_exc_{label}", title="Test Session") + fake_session.pending_user_message = "Exception test" + fake_session.pending_started_at = time.time() + 30 if started_at == "future" else started_at + fake_session.active_stream_id = f"test_stream_exc_{label}" + fake_session.save() + models.SESSIONS[fake_session.session_id] = fake_session + + class ExceptionAgent(MockAgent): + def run_conversation(self, **kwargs): + if self.stream_delta_callback: + self.stream_delta_callback("Stream before crash.") + raise RuntimeError("Fake provider crash!") + + fake_queue = queue.Queue() + streaming.STREAMS[fake_session.active_stream_id] = fake_queue + config.STREAM_PARTIAL_TEXT[fake_session.active_stream_id] = "" + + with mock.patch.object(streaming, "get_session", return_value=fake_session), \ + mock.patch.object(streaming, "_get_ai_agent", return_value=ExceptionAgent), \ + mock.patch.object(streaming, "resolve_model_provider", return_value=("test-model", "test-provider", None)), \ + mock.patch("api.config.get_config", return_value={}), \ + mock.patch("api.config._resolve_cli_toolsets", return_value=[]): + + streaming._run_agent_streaming( + session_id=fake_session.session_id, + msg_text="Exception test", + model="test-model", + workspace=str(tmp_path), + stream_id=fake_session.active_stream_id, + ) + + saved = Session.load(fake_session.session_id) + assert saved is not None + assert saved.messages[-1].get("_error") is True + assert "_turnDuration" not in saved.messages[-1] + + def test_empty_partials_do_not_create_spurious_messages(tmp_path): """Test that if no content has been streamed, no _partial message is created on error.""" fake_session = Session(session_id="test_sess_empty", title="Test Session") diff --git a/tests/test_webui_gateway_chat_backend.py b/tests/test_webui_gateway_chat_backend.py index 7140c501..be666c83 100644 --- a/tests/test_webui_gateway_chat_backend.py +++ b/tests/test_webui_gateway_chat_backend.py @@ -4,6 +4,7 @@ from email.message import Message import json from pathlib import Path import re +import time import urllib.error import api.gateway_chat as gateway_chat @@ -519,6 +520,38 @@ def test_gateway_chat_worker_classifies_terminal_provider_error_without_text(tmp unknown_errors = [item[1] for item in unknown_events if item[0] == "apperror"] assert unknown_errors[-1]["type"] == "error" assert "Gateway provider failed" in unknown_errors[-1]["message"] + unknown_payload_error = unknown_errors[-1]["session"]["messages"][-1] + assert unknown_payload_error.get("_error") is True + assert "_turnDuration" not in unknown_payload_error + + response_error[0] = error_text + future_stream_id = "stream-gateway-future-duration-terminal-error-test" + s = new_session() + s.active_stream_id = future_stream_id + s.pending_user_message = "Say hello" + s.pending_started_at = time.time() + 30 + s.pending_attachments = [] + s.save() + future_events = [] + future_channel = MagicMock() + future_channel.put_nowait = lambda item: future_events.append(item) + STREAMS[future_stream_id] = future_channel + gateway_chat._run_gateway_chat_streaming( + s.session_id, + "Say hello", + "test-model", + str(tmp_path), + future_stream_id, + [], + ) + future_errors = [item[1] for item in future_events if item[0] == "apperror"] + assert future_errors[-1]["type"] in {"model_not_found", "auth_mismatch"} + saved = models.get_session(s.session_id) + assert saved.messages[-1].get("_error") is True + assert "_turnDuration" not in saved.messages[-1] + future_payload_error = future_errors[-1]["session"]["messages"][-1] + assert future_payload_error.get("_error") is True + assert "_turnDuration" not in future_payload_error response_error[0] = "partial" partial_stream_id = "stream-gateway-partial-terminal-error-test" @@ -553,6 +586,7 @@ def test_gateway_chat_worker_classifies_terminal_provider_error_without_text(tmp assert payload_messages[-2]["_partial"] is True assert payload_messages[-2]["content"] == "partial" assert payload_messages[-1]["_error"] is True + assert "_turnDuration" not in payload_messages[-1] def test_gateway_chat_worker_persists_reasoning_and_tool_state_on_terminal_error(tmp_path, monkeypatch):