diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index 0991fc011e2..5e58cd00c68 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -16,7 +16,9 @@ import traceback import weakref from collections import defaultdict +from dataclasses import dataclass from datetime import datetime +from unittest.mock import Mock # Optional dependency: used to convert locale codes (eg ``en_US``) # into human-readable language names (eg ``English``). @@ -26,7 +28,7 @@ Locale = None from json.decoder import JSONDecodeError from pathlib import Path -from typing import List +from typing import List, Optional import httpx from litellm import experimental_mcp_client @@ -82,6 +84,16 @@ class FinishReasonLength(Exception): pass +@dataclass +class ConsolidatedResponseMetadata: + """LiteLLM response metadata extracted during chunk consolidation.""" + + choice_has_finish_reason: bool = False + choice_has_content: bool = False + choice_has_tool_calls: bool = False + choice_content_text: str = "" + + def wrap_fence(name): return f"<{name}>", f"" @@ -525,7 +537,7 @@ def __init__( max_code_line_length=map_max_line_length, repo_root=self.root, use_memory_cache=repomap_in_memory, - use_enhanced_map=False if not self.args or self.args.use_enhanced_map else True, + use_enhanced_map=(False if not self.args or self.args.use_enhanced_map else True), ) self.summarizer = summarizer or ChatSummary( @@ -1128,7 +1140,8 @@ def get_readonly_files_messages(self): if read_only_content: readonly_messages += [ dict( - role="user", content=self.gpt_prompts.read_only_files_prefix + read_only_content + role="user", + content=self.gpt_prompts.read_only_files_prefix + read_only_content, ), dict( role="assistant", @@ -1143,7 +1156,10 @@ def get_readonly_files_messages(self): if images_message is not None: readonly_messages += [ images_message, - dict(role="assistant", content="Ok, I will use these images as references."), + dict( + role="assistant", + content="Ok, I will use these images as references.", + ), ] return readonly_messages @@ -1244,7 +1260,10 @@ def get_images_message(self, fnames): if mime_type.startswith("image/") and supports_images: image_messages += [ {"type": "text", "text": f"Image file: {rel_fname}"}, - {"type": "image_url", "image_url": {"url": image_url, "detail": "high"}}, + { + "type": "image_url", + "image_url": {"url": image_url, "detail": "high"}, + }, ] elif mime_type == "application/pdf" and supports_pdfs: image_messages += [ @@ -2072,7 +2091,8 @@ def format_chat_chunks(self): if self.gpt_prompts.system_reminder: reminder_message = [ dict( - role="system", content=self.fmt_system_prompt(self.gpt_prompts.system_reminder) + role="system", + content=self.fmt_system_prompt(self.gpt_prompts.system_reminder), ), ] else: @@ -2300,7 +2320,11 @@ async def send_message(self, inp): messages[-1]["content"] = self.multi_response_content else: messages.append( - dict(role="assistant", content=self.multi_response_content, prefix=True) + dict( + role="assistant", + content=self.multi_response_content, + prefix=True, + ) ) except Exception as err: self.mdstream = None @@ -2355,7 +2379,10 @@ async def send_message(self, inp): else: self.cur_messages += [dict(role="user", content="^C KeyboardInterrupt")] self.cur_messages += [ - dict(role="assistant", content="I see that you interrupted my previous reply.") + dict( + role="assistant", + content="I see that you interrupted my previous reply.", + ) ] return @@ -2382,7 +2409,7 @@ async def send_message(self, inp): # Process any tools using MCP servers try: if self.partial_response_tool_calls: - tool_call_response, a, b = self.consolidate_chunks() + tool_call_response, a, b, _ = self.consolidate_chunks() if await self.process_tool_calls(tool_call_response): self.num_tool_calls += 1 self.reflected_message = True @@ -2641,7 +2668,9 @@ async def _exec_server_tools(server, tool_calls_list): # Handle non-text blobs gracefully name = getattr(resource, "name", "unnamed") mime_type = getattr( - resource, "mimeType", "unknown mime type" + resource, + "mimeType", + "unknown mime type", ) content_parts.append( "[embedded binary resource:" @@ -2668,21 +2697,33 @@ async def _exec_server_tools(server, tool_calls_list): f" Error: {e}\n" ) tool_responses.append( - {"role": "tool", "tool_call_id": tool_call.id, "content": tool_error} + { + "role": "tool", + "tool_call_id": tool_call.id, + "content": tool_error, + } ) except httpx.RemoteProtocolError as e: connection_error = f"Server {server.name} disconnected unexpectedly: {e}" self.io.tool_warning(connection_error) for tool_call in tool_calls_list: tool_responses.append( - {"role": "tool", "tool_call_id": tool_call.id, "content": connection_error} + { + "role": "tool", + "tool_call_id": tool_call.id, + "content": connection_error, + } ) except Exception as e: connection_error = f"Could not connect to server {server.name}\n{e}" self.io.tool_warning(connection_error) for tool_call in tool_calls_list: tool_responses.append( - {"role": "tool", "tool_call_id": tool_call.id, "content": connection_error} + { + "role": "tool", + "tool_call_id": tool_call.id, + "content": connection_error, + } ) return tool_responses @@ -2972,7 +3013,10 @@ async def check_for_file_mentions(self, content): self.io.tool_output(rel_fname) if await self.io.confirm_ask( - "Add file to the chat?", subject=rel_fname, group=group, allow_never=True + "Add file to the chat?", + subject=rel_fname, + group=group, + allow_never=True, ): self.add_rel_fname(rel_fname) added_fnames.append(rel_fname) @@ -3042,6 +3086,87 @@ async def send(self, messages, model=None, functions=None, tools=None): if args: self.io.ai_output(json.dumps(args, indent=4)) + @staticmethod + def _safe_getattr(obj, attr, default=None): + if obj is None: + return default + try: + return object.__getattribute__(obj, attr) + except AttributeError: + if isinstance(obj, Mock): + return default + return getattr(obj, attr, default) + except Exception: + return getattr(obj, attr, default) + + @classmethod + def _choice_has_finish_reason(cls, choice): + if not choice: + return False + finish_reason = cls._safe_getattr(choice, "finish_reason") + return bool(finish_reason) + + @classmethod + def _choice_content_text(cls, choice): + if not choice: + return "" + + message = cls._safe_getattr(choice, "message") + content = cls._safe_getattr(message, "content") + if isinstance(content, list): + content = "".join( + block.get("text", "") + for block in content + if isinstance(block, dict) and block.get("type") in {"output_text", "text"} + ) + + if content: + return content + + delta = cls._safe_getattr(choice, "delta") + content = cls._safe_getattr(delta, "content") + return content or "" + + @classmethod + def _choice_has_content(cls, choice): + if not choice: + return False + return bool(cls._choice_content_text(choice)) + + @classmethod + def _choice_has_tool_calls(cls, choice): + if not choice: + return False + message = cls._safe_getattr(choice, "message") + tool_calls = cls._safe_getattr(message, "tool_calls") + if tool_calls: + return len(tool_calls) > 0 + delta = cls._safe_getattr(choice, "delta") + tool_calls = cls._safe_getattr(delta, "tool_calls") + return bool(tool_calls) + + def _warn_if_truly_empty_response( + self, + response_meta: ConsolidatedResponseMetadata, + response_has_content: Optional[bool] = None, + response_has_tool_calls: Optional[bool] = None, + ) -> bool: + if response_has_content is None: + response_has_content = ( + bool(self.partial_response_content) or response_meta.choice_has_content + ) + if response_has_tool_calls is None: + response_has_tool_calls = ( + bool(self.partial_response_tool_calls) or response_meta.choice_has_tool_calls + ) + + has_valid_finish_reason = response_meta.choice_has_finish_reason + + if not response_has_content and not response_has_tool_calls and not has_valid_finish_reason: + self.io.tool_warning("Empty response received from LLM. Check your provider account?") + return True + return False + def show_send_output(self, completion): if self.verbose: print(completion) @@ -3056,7 +3181,7 @@ def show_send_output(self, completion): self.partial_response_chunks.append(completion) - response, func_err, content_err = self.consolidate_chunks() + response, func_err, content_err, response_meta = self.consolidate_chunks() resp_hash = dict( function_call=str(self.partial_response_function_call), @@ -3070,6 +3195,31 @@ def show_send_output(self, completion): self.io.tool_error(content_err) raise Exception("No data found in LLM response!") + completion_choice = None + if completion and hasattr(completion, "choices") and completion.choices: + completion_choice = completion.choices[0] + + response_has_content = ( + bool(self.partial_response_content) or response_meta.choice_has_content + ) + response_has_tool_calls = ( + len(self.partial_response_tool_calls) > 0 or response_meta.choice_has_tool_calls + ) + + if response_has_content and not self.partial_response_content: + fallback_content = response_meta.choice_content_text or self._choice_content_text( + completion_choice + ) + if fallback_content: + self.partial_response_content = fallback_content + + if self._warn_if_truly_empty_response( + response_meta, + response_has_content=response_has_content, + response_has_tool_calls=response_has_tool_calls, + ): + return + show_resp = self.render_incremental_response(True) if self.partial_response_reasoning_content: @@ -3080,6 +3230,9 @@ def show_send_output(self, completion): show_resp = replace_reasoning_tags(show_resp, self.reasoning_tag_name) + if not show_resp: + return + self.io.assistant_output(show_resp, pretty=self.show_pretty()) if ( @@ -3089,8 +3242,7 @@ def show_send_output(self, completion): raise FinishReasonLength() async def show_send_output_stream(self, completion): - received_content = False - + """Yield streaming chunks from the LLM completion.""" async for chunk in completion: if self.args.debug: with open(".cecli/logs/chunks.log", "a") as f: @@ -3115,7 +3267,6 @@ async def show_send_output_stream(self, completion): try: if chunk.choices[0].delta.tool_calls: - received_content = True self.token_profiler.on_token() for tool_call_chunk in chunk.choices[0].delta.tool_calls: self.tool_reflection = True @@ -3143,7 +3294,6 @@ async def show_send_output_stream(self, completion): self.tool_reflection = True self.io.update_spinner_suffix(v) - received_content = True self.token_profiler.on_token() except AttributeError: pass @@ -3163,7 +3313,6 @@ async def show_send_output_stream(self, completion): text += f"<{REASONING_TAG}>\n\n" text += reasoning_content self.got_reasoning_content = True - received_content = True self.token_profiler.on_token() self.io.update_spinner_suffix(reasoning_content) self.partial_response_reasoning_content += reasoning_content @@ -3176,7 +3325,6 @@ async def show_send_output_stream(self, completion): self.ended_reasoning_content = True text += content - received_content = True self.token_profiler.on_token() self.io.update_spinner_suffix(content) except AttributeError: @@ -3204,10 +3352,20 @@ async def show_send_output_stream(self, completion): yield text # The Part Doing the Heavy Lifting Now - self.consolidate_chunks() + response, _, _, response_meta = self.consolidate_chunks() - if not received_content and len(self.partial_response_tool_calls) == 0: - self.io.tool_warning("Empty response received from LLM. Check your provider account?") + response_has_content = ( + bool(self.partial_response_content) or response_meta.choice_has_content + ) + response_has_tool_calls = ( + len(self.partial_response_tool_calls) > 0 or response_meta.choice_has_tool_calls + ) + + self._warn_if_truly_empty_response( + response_meta, + response_has_content=response_has_content, + response_has_tool_calls=response_has_tool_calls, + ) def consolidate_chunks(self): response = ( @@ -3217,6 +3375,7 @@ def consolidate_chunks(self): ) func_err = None content_err = None + response_meta = ConsolidatedResponseMetadata() # Collect provider-specific fields from chunks to preserve them # We need to track both by ID (primary) and index (fallback) since @@ -3319,7 +3478,15 @@ def consolidate_chunks(self): except AttributeError as e: content_err = e - return response, func_err, content_err + response_choice = None + if response and hasattr(response, "choices") and response.choices: + response_choice = response.choices[0] + response_meta.choice_has_finish_reason = self._choice_has_finish_reason(response_choice) + response_meta.choice_content_text = self._choice_content_text(response_choice) or "" + response_meta.choice_has_content = bool(response_meta.choice_content_text) + response_meta.choice_has_tool_calls = self._choice_has_tool_calls(response_choice) + + return response, func_err, content_err, response_meta def stream_wrapper(self, content, final): if not hasattr(self, "_streaming_buffer_length"): diff --git a/tests/basic/test_coder.py b/tests/basic/test_coder.py index febe38028e3..49d4531baa1 100644 --- a/tests/basic/test_coder.py +++ b/tests/basic/test_coder.py @@ -1,14 +1,21 @@ import base64 +import inspect import os import tempfile from pathlib import Path +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import git import pytest +from litellm.types.utils import Choices, ModelResponse from cecli.coders import Coder -from cecli.coders.base_coder import FinishReasonLength, UnknownEditFormat +from cecli.coders.base_coder import ( + ConsolidatedResponseMetadata, + FinishReasonLength, + UnknownEditFormat, +) from cecli.commands import SwitchCoderSignal from cecli.dump import dump # noqa: F401 from cecli.io import InputOutput @@ -25,6 +32,57 @@ def setup(self, gpt35_model): self.webbrowser_patcher = patch("cecli.io.webbrowser.open") self.mock_webbrowser = self.webbrowser_patcher.start() + async def _drain_show_send_output_stream(self, coder, completion): + self._reset_partial_response_state(coder) + reset_args = False + if coder.args is None: + coder.args = SimpleNamespace(debug=False, tui=False) + reset_args = True + stream_output = coder.show_send_output_stream(completion) + try: + if inspect.isasyncgen(stream_output): + return [chunk async for chunk in stream_output] + return list(stream_output) + finally: + if reset_args: + coder.args = None + + @staticmethod + def _make_model_response(*, finish_reason=None, content=None): + response = ModelResponse( + id="test-response", + model="test-model", + created=0, + object="chat.completion", + choices=[Choices()], + ) + response.choices[0].finish_reason = finish_reason + response.choices[0].message.content = content + return response + + @staticmethod + def _make_consolidated_tuple(response): + choice = None + if response and getattr(response, "choices", None): + choice = response.choices[0] + meta = ConsolidatedResponseMetadata( + choice_has_finish_reason=Coder._choice_has_finish_reason(choice), + choice_has_content=Coder._choice_has_content(choice), + choice_has_tool_calls=Coder._choice_has_tool_calls(choice), + choice_content_text=Coder._choice_content_text(choice) if choice else "", + ) + return (response, None, None, meta) + + @staticmethod + def _reset_partial_response_state(coder): + coder.partial_response_content = "" + coder.partial_response_reasoning_content = "" + coder.partial_response_chunks = [] + coder.partial_response_tool_calls = [] + coder.partial_response_function_call = {} + coder.got_reasoning_content = False + coder.ended_reasoning_content = False + async def test_allowed_to_edit(self): with GitTemporaryDirectory(): repo = git.Repo() @@ -232,7 +290,9 @@ async def test_check_for_file_mentions_with_mocked_confirm(self): mock_args = MagicMock(tui=False) coder = await Coder.create(self.GPT35, None, io, args=mock_args) - coder.get_file_mentions = MagicMock(return_value=set(["file1.txt", "file2.txt"])) + coder.get_file_mentions = MagicMock( + return_value=set(["file1.txt", "file2.txt"]) + ) await coder.check_for_file_mentions("Please check file1.txt for the info") @@ -242,7 +302,9 @@ async def test_check_for_file_mentions_with_mocked_confirm(self): io.confirm_ask.reset_mock() - await coder.check_for_file_mentions("Please check file1.txt and file2.txt again") + await coder.check_for_file_mentions( + "Please check file1.txt and file2.txt again" + ) assert io.confirm_ask.call_count == 1 assert len(coder.abs_fnames) == 1 @@ -413,7 +475,9 @@ async def test_get_file_mentions_path_formats(self): ] for content, addable_files in test_cases: - coder.get_addable_relative_files = MagicMock(return_value=set(addable_files)) + coder.get_addable_relative_files = MagicMock( + return_value=set(addable_files) + ) mentioned_files = coder.get_file_mentions(content) expected_files = set(addable_files) assert ( @@ -537,7 +601,9 @@ async def mock_send(*args, **kwargs): await coder.run(with_message="hi") assert len(coder.abs_fnames) == 2 - some_content_which_will_error_if_read_with_encoding_utf8 = "ÅÍÎÏ".encode(encoding) + some_content_which_will_error_if_read_with_encoding_utf8 = "ÅÍÎÏ".encode( + encoding + ) with open(file1, "wb") as f: f.write(some_content_which_will_error_if_read_with_encoding_utf8) @@ -613,7 +679,9 @@ async def test_only_commit_gpt_edited_file(self): fname1.write_text("ONE\n") io = InputOutput(yes=True) - coder = await Coder.create(self.GPT35, "diff", io=io, fnames=[str(fname1), str(fname2)]) + coder = await Coder.create( + self.GPT35, "diff", io=io, fnames=[str(fname1), str(fname2)] + ) async def mock_send(*args, **kwargs): coder.partial_response_content = f""" @@ -637,7 +705,9 @@ def mock_get_commit_message(diffs, context, user_language=None): return "commit message" coder.send = mock_send - coder.repo.get_commit_message = MagicMock(side_effect=mock_get_commit_message) + coder.repo.get_commit_message = MagicMock( + side_effect=mock_get_commit_message + ) await coder.run(with_message="hi") @@ -956,7 +1026,9 @@ async def test_coder_from_coder_with_subdir(self): # Create the first coder io = InputOutput(yes=True) - coder1 = await Coder.create(self.GPT35, None, io=io, fnames=[test_file.name]) + coder1 = await Coder.create( + self.GPT35, None, io=io, fnames=[test_file.name] + ) # Create a new coder from the first coder coder2 = await Coder.create(from_coder=coder1) @@ -966,7 +1038,9 @@ async def test_coder_from_coder_with_subdir(self): # Ensure the abs_fnames contain the correct absolute path expected_abs_path = os.path.realpath(str(test_file)) - coder1_abs_fnames = set(os.path.realpath(path) for path in coder1.abs_fnames) + coder1_abs_fnames = set( + os.path.realpath(path) for path in coder1.abs_fnames + ) assert expected_abs_path in coder1_abs_fnames assert expected_abs_path in coder2.abs_fnames @@ -978,6 +1052,12 @@ async def test_suggest_shell_commands(self): with GitTemporaryDirectory(): io = InputOutput(yes=True) coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) async def mock_send(*args, **kwargs): coder.partial_response_content = """Here's a shell command to run: @@ -1010,7 +1090,9 @@ async def mock_send(*args, **kwargs): async def test_no_suggest_shell_commands(self): with GitTemporaryDirectory(): io = InputOutput(yes=True) - coder = await Coder.create(self.GPT35, "diff", io=io, suggest_shell_commands=False) + coder = await Coder.create( + self.GPT35, "diff", io=io, suggest_shell_commands=False + ) assert not coder.suggest_shell_commands async def test_detect_urls_enabled(self): @@ -1019,7 +1101,9 @@ async def test_detect_urls_enabled(self): mock_args = MagicMock() mock_args.yes_always_commands = False mock_args.disable_scraping = False - coder = await Coder.create(self.GPT35, "diff", io=io, detect_urls=True, args=mock_args) + coder = await Coder.create( + self.GPT35, "diff", io=io, detect_urls=True, args=mock_args + ) # Track calls to do_run do_run_calls = [] @@ -1060,9 +1144,7 @@ def test_unknown_edit_format_exception(self): invalid_format = "invalid_format" valid_formats = ["diff", "whole", "map"] exc = UnknownEditFormat(invalid_format, valid_formats) - expected_msg = ( - f"Unknown edit format {invalid_format}. Valid formats are: {', '.join(valid_formats)}" - ) + expected_msg = f"Unknown edit format {invalid_format}. Valid formats are: {', '.join(valid_formats)}" assert str(exc) == expected_msg async def test_unknown_edit_format_creation(self): @@ -1117,6 +1199,12 @@ async def test_show_exhausted_error(self): with GitTemporaryDirectory(): io = InputOutput(yes=True) coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) # Set up some real done_messages and cur_messages coder.done_messages = [ @@ -1177,6 +1265,12 @@ async def test_keyboard_interrupt_handling(self): with GitTemporaryDirectory(): io = InputOutput(yes=True) coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) # Simulate keyboard interrupt during message processing async def mock_send(*args, **kwargs): @@ -1203,6 +1297,12 @@ async def test_token_limit_error_handling(self): with GitTemporaryDirectory(): io = InputOutput(yes=True) coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) + self._reset_partial_response_state(coder) # Simulate token limit error async def mock_send(*args, **kwargs): @@ -1229,6 +1329,7 @@ async def test_message_sanity_after_partial_response(self): with GitTemporaryDirectory(): io = InputOutput(yes=True) coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) # Simulate partial response then interrupt async def mock_send(*args, **kwargs): @@ -1302,13 +1403,17 @@ async def test_get_user_language(self): # 1. Test with self.chat_language set coder.chat_language = "fr_CA" - with patch.object(coder, "normalize_language", return_value="French Canadian") as mock_norm: + with patch.object( + coder, "normalize_language", return_value="French Canadian" + ) as mock_norm: assert coder.get_user_language() == "French Canadian" mock_norm.assert_called_once_with("fr_CA") coder.chat_language = None # Reset # 2. Test with locale.getlocale() - with patch("locale.getlocale", return_value=("en_GB", "UTF-8")) as mock_getlocale: + with patch( + "locale.getlocale", return_value=("en_GB", "UTF-8") + ) as mock_getlocale: with patch.object( coder, "normalize_language", return_value="British English" ) as mock_norm: @@ -1318,7 +1423,9 @@ async def test_get_user_language(self): # Test with locale.getlocale() returning None or empty with patch("locale.getlocale", return_value=(None, None)) as mock_getlocale: - with patch("os.environ.get") as mock_env_get: # Ensure env vars are not used yet + with patch( + "os.environ.get" + ) as mock_env_get: # Ensure env vars are not used yet mock_env_get.return_value = None # Should be None if nothing found assert coder.get_user_language() is None @@ -1328,8 +1435,12 @@ async def test_get_user_language(self): "locale.getlocale", side_effect=Exception("locale error") ): # Mock locale to fail with patch("os.environ.get") as mock_env_get: - mock_env_get.side_effect = lambda key: "de_DE.UTF-8" if key == "LANG" else None - with patch.object(coder, "normalize_language", return_value="German") as mock_norm: + mock_env_get.side_effect = lambda key: ( + "de_DE.UTF-8" if key == "LANG" else None + ) + with patch.object( + coder, "normalize_language", return_value="German" + ) as mock_norm: assert coder.get_user_language() == "German" mock_env_get.assert_any_call("LANG") mock_norm.assert_called_once_with("de_DE") @@ -1338,8 +1449,12 @@ async def test_get_user_language(self): # by os.environ.get, but our code checks in order, so we mock the first one it finds) with patch("locale.getlocale", side_effect=Exception("locale error")): with patch("os.environ.get") as mock_env_get: - mock_env_get.side_effect = lambda key: "es_ES" if key == "LANGUAGE" else None - with patch.object(coder, "normalize_language", return_value="Spanish") as mock_norm: + mock_env_get.side_effect = lambda key: ( + "es_ES" if key == "LANGUAGE" else None + ) + with patch.object( + coder, "normalize_language", return_value="Spanish" + ) as mock_norm: assert coder.get_user_language() == "Spanish" # LANG would be called first mock_env_get.assert_any_call("LANGUAGE") @@ -1347,7 +1462,9 @@ async def test_get_user_language(self): # 4. Test priority: chat_language > locale > env coder.chat_language = "it_IT" - with patch("locale.getlocale", return_value=("en_US", "UTF-8")) as mock_getlocale: + with patch( + "locale.getlocale", return_value=("en_US", "UTF-8") + ) as mock_getlocale: with patch("os.environ.get", return_value="de_DE") as mock_env_get: with patch.object( coder, "normalize_language", side_effect=lambda x: x.upper() @@ -1385,7 +1502,9 @@ async def test_architect_coder_auto_accept_true(self): with pytest.raises(SwitchCoderSignal): await coder.reply_completed() - io.confirm_ask.assert_called_once_with("Edit the files?", allow_tweak=False) + io.confirm_ask.assert_called_once_with( + "Edit the files?", allow_tweak=False + ) mock_editor.generate.assert_called_once() async def test_architect_coder_auto_accept_false_confirmed(self): @@ -1410,7 +1529,9 @@ async def test_architect_coder_auto_accept_false_confirmed(self): with pytest.raises(SwitchCoderSignal): await coder.reply_completed() - io.confirm_ask.assert_called_once_with("Edit the files?", allow_tweak=False) + io.confirm_ask.assert_called_once_with( + "Edit the files?", allow_tweak=False + ) mock_editor.generate.assert_called_once() async def test_architect_coder_auto_accept_false_rejected(self): @@ -1430,7 +1551,9 @@ async def test_architect_coder_auto_accept_false_rejected(self): result = await coder.reply_completed() assert result is None - io.confirm_ask.assert_called_once_with("Edit the files?", allow_tweak=False) + io.confirm_ask.assert_called_once_with( + "Edit the files?", allow_tweak=False + ) mock_create.assert_not_called() @patch("cecli.coders.base_coder.experimental_mcp_client") @@ -1450,7 +1573,9 @@ async def test_mcp_server_connection(self, mock_mcp_client): # Create coder with mock MCP server with patch.object(Coder, "initialize_mcp_tools", return_value=mock_tools): - coder = await Coder.create(self.GPT35, "diff", io=io, mcp_servers=[mock_server]) + coder = await Coder.create( + self.GPT35, "diff", io=io, mcp_servers=[mock_server] + ) # Manually set mcp_tools since we're bypassing initialize_mcp_tools coder.mcp_tools = mock_tools @@ -1562,6 +1687,7 @@ async def test_process_tool_calls_none_response(self): with GitTemporaryDirectory(): io = InputOutput(yes=True) coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) # Test with None response result = await coder.process_tool_calls(None) @@ -1572,6 +1698,7 @@ async def test_process_tool_calls_no_tool_calls(self): with GitTemporaryDirectory(): io = InputOutput(yes=True) coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) # Create a response with no tool calls response = MagicMock() @@ -1712,19 +1839,23 @@ async def test_process_tool_calls_user_rejects(self): assert not result # Verify that confirm_ask was called - io.confirm_ask.assert_called_once_with("Run tools?", group_response="Run MCP Tools") + io.confirm_ask.assert_called_once_with( + "Run tools?", group_response="Run MCP Tools" + ) # Verify that no messages were added assert len(coder.cur_messages) == 0 @patch( - "cecli.coders.base_coder.experimental_mcp_client.call_openai_tool", new_callable=AsyncMock + "cecli.coders.base_coder.experimental_mcp_client.call_openai_tool", + new_callable=AsyncMock, ) async def test_execute_tool_calls(self, mock_call_tool): """Test that _execute_tool_calls executes tool calls correctly.""" with GitTemporaryDirectory(): io = InputOutput(yes=True) coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) # Create mock server and tool call mock_server = MagicMock() @@ -1792,7 +1923,9 @@ async def mock_get_commit_message(diffs, context, user_language=None): assert "ASSISTANT: \n" in context return "commit message" - coder.repo.get_commit_message = AsyncMock(side_effect=mock_get_commit_message) + coder.repo.get_commit_message = AsyncMock( + side_effect=mock_get_commit_message + ) # To trigger a commit, the file must be modified fname.write_text("one changed\n") @@ -1881,7 +2014,9 @@ async def test_execute_tool_calls_blob_content(self, mock_call_openai_tool): # Mock BlobResourceContents for text text_blob_content = "Hello from blob! " - encoded_text_blob = base64.b64encode(text_blob_content.encode("utf-8")).decode("utf-8") + encoded_text_blob = base64.b64encode( + text_blob_content.encode("utf-8") + ).decode("utf-8") mock_text_blob_resource = MagicMock(spec=["blob"]) mock_text_blob_resource.blob = encoded_text_blob @@ -1927,3 +2062,235 @@ async def test_execute_tool_calls_blob_content(self, mock_call_openai_tool): " (application/octet-stream)]" ) assert result[0]["content"] == expected_content + + async def test_show_send_output_stream_valid_empty_with_finish_reason(self): + """Test that valid empty responses with finish_reason don't trigger warnings in streaming mode.""" + with GitTemporaryDirectory(): + io = InputOutput(yes=True) + coder = await Coder.create(self.GPT35, "diff", io=io) + + # Mock a valid response with finish_reason but no content + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].finish_reason = "stop" + mock_response.choices[0].delta = MagicMock() + del mock_response.choices[0].delta.content # No content + mock_response.choices[0].delta.reasoning_content = None + mock_response.choices[0].delta.reasoning = None + mock_response.__aiter__.return_value = iter([mock_response]) + + # Mock the consolidate_chunks method to return our response + coder.consolidate_chunks = MagicMock( + return_value=self._make_consolidated_tuple(mock_response) + ) + + # Mock tool_warning to check if it's called + io.tool_warning = MagicMock() + + # Call the streaming output method + await self._drain_show_send_output_stream(coder, mock_response) + + # Verify that tool_warning was NOT called (valid empty response) + io.tool_warning.assert_not_called() + + async def test_show_send_output_stream_truly_empty_response(self): + """Test that truly empty responses trigger warnings in streaming mode.""" + with GitTemporaryDirectory(): + io = InputOutput(yes=True) + coder = await Coder.create(self.GPT35, "diff", io=io) + + # Mock a truly empty response (no finish_reason, no content, no tool calls) + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].finish_reason = None # No finish reason + mock_response.choices[0].delta = MagicMock() + del mock_response.choices[0].delta.content # No content + mock_response.choices[0].delta.reasoning_content = None + mock_response.choices[0].delta.reasoning = None + mock_response.__aiter__.return_value = iter([mock_response]) + + # Mock the consolidate_chunks method to return our response + coder.consolidate_chunks = MagicMock( + return_value=self._make_consolidated_tuple(mock_response) + ) + + # Mock tool_warning to check if it's called + io.tool_warning = MagicMock() + + # Call the streaming output method + await self._drain_show_send_output_stream(coder, mock_response) + + # Verify that tool_warning WAS called (truly empty response) + io.tool_warning.assert_called_once_with( + "Empty response received from LLM. Check your provider account?" + ) + + async def test_show_send_output_stream_response_with_content(self): + """Test that responses with content don't trigger warnings in streaming mode.""" + with GitTemporaryDirectory(): + io = InputOutput(yes=True) + coder = await Coder.create(self.GPT35, "diff", io=io) + + # Mock a response with content + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].finish_reason = "stop" + mock_response.choices[0].delta = MagicMock() + mock_response.choices[0].delta.content = "Some response content" + mock_response.choices[0].delta.reasoning_content = None + mock_response.choices[0].delta.reasoning = None + mock_response.__aiter__.return_value = iter([mock_response]) + + # Mock the consolidate_chunks method to return our response + coder.consolidate_chunks = MagicMock( + return_value=self._make_consolidated_tuple(mock_response) + ) + + # Mock tool_warning to check if it's called + io.tool_warning = MagicMock() + + # Call the streaming output method + await self._drain_show_send_output_stream(coder, mock_response) + + # Verify that tool_warning was NOT called (has content) + io.tool_warning.assert_not_called() + + async def test_show_send_output_stream_response_with_tool_calls(self): + """Test that responses with tool calls don't trigger warnings in streaming mode.""" + with GitTemporaryDirectory(): + io = InputOutput(yes=True) + coder = await Coder.create(self.GPT35, "diff", io=io) + + # Mock a response with tool calls but no content + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].finish_reason = "tool_calls" + mock_response.choices[0].delta = MagicMock() + del mock_response.choices[0].delta.content # No content + mock_response.choices[0].delta.reasoning_content = None + mock_response.choices[0].delta.reasoning = None + mock_response.__aiter__.return_value = iter([mock_response]) + + # Mock the consolidate_chunks method to return our response + coder.consolidate_chunks = MagicMock( + return_value=self._make_consolidated_tuple(mock_response) + ) + + # Mock tool_warning to check if it's called + io.tool_warning = MagicMock() + + # Mock partial_response_tool_calls to simulate tool calls + coder.partial_response_tool_calls = [{"id": "test_tool"}] + + # Call the streaming output method + await self._drain_show_send_output_stream(coder, mock_response) + + # Verify that tool_warning was NOT called (has tool calls) + io.tool_warning.assert_not_called() + + async def test_show_send_output_valid_empty_with_finish_reason(self): + """Test that valid empty responses with finish_reason don't trigger warnings.""" + with GitTemporaryDirectory(): + io = InputOutput(yes=True) + coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) + + # Mock a valid response with finish_reason but no content + mock_response = self._make_model_response( + finish_reason="stop", content=None + ) + + # Mock the consolidate_chunks method to return our response + coder.consolidate_chunks = MagicMock( + return_value=self._make_consolidated_tuple(mock_response) + ) + + # Mock tool_warning to check if it's called + io.tool_warning = MagicMock() + + # Call the non-streaming output method + coder.show_send_output(mock_response) + + # Verify that tool_warning was NOT called (valid empty response) + io.tool_warning.assert_not_called() + + async def test_show_send_output_truly_empty_response(self): + """Test that truly empty responses trigger warnings.""" + with GitTemporaryDirectory(): + io = InputOutput(yes=True) + coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) + + # Mock a truly empty response (no finish_reason, no content, no tool calls) + mock_response = self._make_model_response(finish_reason=None, content=None) + + # Mock the consolidate_chunks method to return our response + coder.consolidate_chunks = MagicMock( + return_value=self._make_consolidated_tuple(mock_response) + ) + + # Mock tool_warning to check if it's called + io.tool_warning = MagicMock() + + # Call the non-streaming output method + coder.show_send_output(mock_response) + + # Verify that tool_warning WAS called (truly empty response) + io.tool_warning.assert_called_once_with( + "Empty response received from LLM. Check your provider account?" + ) + + async def test_show_send_output_response_with_content(self): + """Test that responses with content don't trigger warnings.""" + with GitTemporaryDirectory(): + io = InputOutput(yes=True) + coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) + + # Mock a response with content + mock_response = self._make_model_response( + finish_reason="stop", content="Some response content" + ) + + # Mock the consolidate_chunks method to return our response + coder.consolidate_chunks = MagicMock( + return_value=self._make_consolidated_tuple(mock_response) + ) + + # Mock tool_warning to check if it's called + io.tool_warning = MagicMock() + + # Call the non-streaming output method + coder.show_send_output(mock_response) + + # Verify that tool_warning was NOT called (has content) + io.tool_warning.assert_not_called() + + async def test_show_send_output_response_with_tool_calls(self): + """Test that responses with tool calls don't trigger warnings.""" + with GitTemporaryDirectory(): + io = InputOutput(yes=True) + coder = await Coder.create(self.GPT35, "diff", io=io) + self._reset_partial_response_state(coder) + + # Mock a response with tool calls but no content + mock_response = self._make_model_response( + finish_reason="tool_calls", content=None + ) + + # Mock the consolidate_chunks method to return our response + coder.consolidate_chunks = MagicMock( + return_value=self._make_consolidated_tuple(mock_response) + ) + + # Mock tool_warning to check if it's called + io.tool_warning = MagicMock() + + # Mock partial_response_tool_calls to simulate tool calls + coder.partial_response_tool_calls = [{"id": "test_tool"}] + + # Call the non-streaming output method + coder.show_send_output(mock_response) + + # Verify that tool_warning was NOT called (has tool calls) + io.tool_warning.assert_not_called()