From c9d3d14ecbf66b8e36517cfb4353960beb8c4538 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 25 Sep 2026 17:34:46 -0700 Subject: [PATCH 1/8] cli-65: retry config fix --- cecli/coders/base_coder.py | 28 +- cecli/models.py | 67 +++- cecli/sessions.py | 502 ++++++++++++++++++++++++++++ requirements.txt | 4 +- requirements/common-constraints.txt | 2 +- tests/basic/test_retry_config.py | 64 ++++ 6 files changed, 630 insertions(+), 37 deletions(-) create mode 100644 cecli/sessions.py create mode 100644 tests/basic/test_retry_config.py diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index cc62c69493e..f616f1fba3f 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -54,7 +54,6 @@ from cecli.linter import Linter from cecli.llm import litellm from cecli.mcp import LocalServer -from cecli.models import RETRY_TIMEOUT from cecli.reasoning_tags import ( REASONING_TAG, format_reasoning_content, @@ -2829,24 +2828,14 @@ async def format_in_executor(): except EmptyResponseError: self.io.tool_warning(self.empty_llm_tool_warning()) - retry_on_empty = False - retries_config = self.get_active_model().retries - if isinstance(retries_config, str): - try: - retries_config = json.loads(retries_config) - except json.JSONDecodeError: - self.io.tool_warning( - f"Could not parse retries config: {retries_config}" - ) - retries_config = {} - if isinstance(retries_config, dict): - retry_on_empty = retries_config.get("retry_on_empty", False) + retry_config = models._parse_retry_config(self.get_active_model().retries) + retry_on_empty = retry_config["retry_on_empty"] if not retry_on_empty: break - retry_delay *= 2 - if retry_delay > RETRY_TIMEOUT: + retry_delay *= retry_config["retry_backoff_factor"] + if retry_delay > retry_config["retry_timeout"]: self.io.tool_error("Retry timeout exceeded on empty response.") break @@ -2866,10 +2855,15 @@ async def format_in_executor(): exhausted = True break + retry_config = models._parse_retry_config(self.get_active_model().retries) + should_retry = ex_info.retry + if ex_info.name == "ServiceUnavailableError": + should_retry = should_retry or retry_config["retry_on_unavailable"] + if should_retry: - retry_delay *= 2 - if retry_delay > RETRY_TIMEOUT: + retry_delay *= retry_config["retry_backoff_factor"] + if retry_delay > retry_config["retry_timeout"]: should_retry = False if not should_retry: diff --git a/cecli/models.py b/cecli/models.py index 827f433e35f..7532c4aa57c 100644 --- a/cecli/models.py +++ b/cecli/models.py @@ -1452,21 +1452,10 @@ async def send_completion( litellm_ex = LiteLLMExceptions() retry_delay = 0.125 - if self.retries: - retry_config = dict() - try: - retry_config = json.loads(self.retries) - except (json.JSONDecodeError, TypeError, ValueError): - retry_config = dict() - pass - - self.retry_on_unavailable = bool( - nested.getter(retry_config, "retry-on-unavailable", True) - ) - self.retry_backoff_factor = float( - nested.getter(retry_config, "retry-backoff-factor", 1.5) - ) - self.retry_timeout = float(nested.getter(retry_config, "retry-timeout", 30)) + retry_config = _parse_retry_config(self.retries) + self.retry_on_unavailable = retry_config["retry_on_unavailable"] + self.retry_backoff_factor = retry_config["retry_backoff_factor"] + self.retry_timeout = retry_config["retry_timeout"] while True: try: @@ -1557,6 +1546,11 @@ async def simple_send_with_retries( temperature = None tools = None + retry_config = _parse_retry_config(self.retries) + retry_backoff_factor = retry_config["retry_backoff_factor"] + retry_timeout = retry_config["retry_timeout"] + retry_on_unavailable = retry_config["retry_on_unavailable"] + if self.verbose: dump(messages) @@ -1607,14 +1601,17 @@ async def simple_send_with_retries( if ex_info.description: print(ex_info.description) should_retry = ex_info.retry + if ex_info.name == "ServiceUnavailableError": + should_retry = should_retry or retry_on_unavailable + custom_retry_delay = self._extract_retry_delay(err) if custom_retry_delay is not None: retry_delay = custom_retry_delay should_retry = True elif should_retry: - retry_delay *= 2 + retry_delay *= retry_backoff_factor - if retry_delay > RETRY_TIMEOUT: + if retry_delay > retry_timeout: should_retry = False if not should_retry: @@ -1792,6 +1789,42 @@ def _configured_provider(self) -> str: return provider +def _parse_retry_config(retries_input): + """ + Parse and normalize retry configuration from a JSON string or dict. + Returns a unified dict with defaults: + retry_timeout: 30 + retry_backoff_factor: 1.5 + retry_on_unavailable: True + retry_on_empty: False + """ + config = dict() + if isinstance(retries_input, str): + try: + config = json.loads(retries_input) + except (json.JSONDecodeError, TypeError, ValueError): + config = dict() + elif isinstance(retries_input, dict): + config = retries_input.copy() + + # Helper to get either hyphenated or underscored key + def _get(key, default): + val = config.get(key) + if val is not None: + return val + val = config.get(key.replace("_", "-")) + if val is not None: + return val + return default + + return { + "retry_timeout": float(_get("retry_timeout", 30)), + "retry_backoff_factor": float(_get("retry_backoff_factor", 1.5)), + "retry_on_unavailable": bool(_get("retry_on_unavailable", True)), + "retry_on_empty": bool(_get("retry_on_empty", False)), + } + + def register_models(model_settings_fnames): files_loaded = [] for model_settings_fname in model_settings_fnames: diff --git a/cecli/sessions.py b/cecli/sessions.py new file mode 100644 index 00000000000..a8e178ec04e --- /dev/null +++ b/cecli/sessions.py @@ -0,0 +1,502 @@ +"""Session management utilities for cecli.""" + +import json +import os +from pathlib import Path +from typing import Dict, List, Optional + +from cecli import models +from cecli.decoding import safe_open +from cecli.helpers import crypto as session_crypto +from cecli.helpers.conversation import ConversationService, MessageTag + + +class SessionManager: + """Manages chat session saving, listing, and loading.""" + + def __init__(self, coder, io): + self.coder = coder + self.io = io + + def save_session(self, session_name: str, output=True) -> bool: + """Save the current chat session to a named file.""" + if not session_name: + if output: + self.io.tool_error("Please provide a session name.") + return False + + session_name = session_name.replace(".json", "") + session_dir = self._get_session_directory() + session_file = session_dir / f"{session_name}.json" + + if session_file.exists(): + if output: + self.io.tool_warning(f"Session '{session_name}' already exists. Overwriting.") + + try: + session_data = self._build_session_data(session_name) + if not self._write_session_file(session_file, session_data): + return False + + if output: + suffix = " (encrypted)" if self._session_encrypt_settings()[0] else "" + self.io.tool_output(f"Session saved: {session_file}{suffix}") + + return True + + except Exception as e: + self.io.tool_error(f"Error saving session: {e}") + return False + + def list_sessions(self) -> List[Dict]: + """List all saved sessions with metadata.""" + session_dir = self._get_session_directory() + session_files = list(session_dir.glob("*.json")) + + if not session_files: + self.io.tool_output("No saved sessions found.") + return [] + + sessions = [] + for session_file in sorted(session_files, key=lambda x: x.stat().st_mtime, reverse=True): + try: + raw = session_file.read_bytes() + if session_crypto.is_encrypted_payload(raw): + _, key = self._session_encrypt_settings() + if not key: + sessions.append( + { + "name": session_file.stem, + "file": session_file, + "model": "encrypted", + "edit_format": "—", + "num_messages": 0, + "num_files": 0, + "encrypted": True, + } + ) + continue + session_data = session_crypto.decrypt_session_bytes(raw, key) + else: + session_data = json.loads(raw.decode("utf-8")) + if not isinstance(session_data, dict): + raise ValueError("not a session object") + + session_info = { + "name": session_file.stem, + "file": session_file, + "model": session_data.get("model", "unknown"), + "edit_format": session_data.get("edit_format", "unknown"), + "num_messages": ( + len(session_data.get("chat_history", {}).get("done_messages", [])) + + len(session_data.get("chat_history", {}).get("cur_messages", [])) + ), + "num_files": ( + len(session_data.get("files", {}).get("editable", [])) + + len(session_data.get("files", {}).get("read_only", [])) + + len(session_data.get("files", {}).get("read_only_stubs", [])) + ), + "encrypted": session_crypto.is_encrypted_payload(raw), + } + sessions.append(session_info) + + except Exception as e: + self.io.tool_output(f" {session_file.stem} [error reading: {e}]") + + return sessions + + async def load_session(self, session_identifier: str, switch=True, quiet: bool = False) -> bool: + """Load a saved session by name or file path.""" + if not session_identifier: + self.io.tool_error("Please provide a session name or file path.") + return False + + # Try to find the session file + session_file = self._find_session_file(session_identifier) + if not session_file: + return False + + session_data = self._read_session_file(session_file, quiet=quiet) + if session_data is None: + return False + + if not isinstance(session_data, dict) or "version" not in session_data: + if not quiet: + self.io.tool_error("Invalid session format.") + return False + + # Apply session data + applied, loaded_edit_format = await self._apply_session_data(session_data, session_file) + if applied and switch: + from cecli.commands import SwitchCoderSignal + + edit_format_to_switch_to = self.coder.edit_format + if loaded_edit_format: + edit_format_to_switch_to = loaded_edit_format + self.coder.edit_format = loaded_edit_format + + raise SwitchCoderSignal( + edit_format=edit_format_to_switch_to, + from_coder=self.coder, + summarize_from_coder=False, + show_announcements=True, + ) + return applied + + def _get_session_directory(self) -> Path: + """Get the session directory, creating it if necessary.""" + session_dir = Path(self.coder.abs_root_path(".cecli/sessions")) + os.makedirs(session_dir, exist_ok=True) + return session_dir + + def _session_encrypt_settings(self) -> tuple[bool, bytes | None]: + args = getattr(self.coder, "args", None) + if not args or not getattr(args, "session_encrypt", False): + return False, None + key_file = getattr(args, "session_key_file", None) + return True, session_crypto.resolve_key(key_file=key_file) + + def _read_session_file(self, session_file: Path, quiet: bool = False) -> dict | None: + try: + data = session_file.read_bytes() + except OSError as e: + if not quiet: + self.io.tool_error(f"Error reading session: {e}") + return None + try: + if session_crypto.is_encrypted_payload(data): + args = getattr(self.coder, "args", None) + key_file = getattr(args, "session_key_file", None) if args else None + key = session_crypto.resolve_key(key_file=key_file) + if not key: + if not quiet: + self.io.tool_error( + "Session is encrypted but no key is configured " + f"({session_crypto.KEY_ENV} or --session-key-file)." + ) + return None + return session_crypto.decrypt_session_bytes(data, key) + parsed = json.loads(data.decode("utf-8")) + if not isinstance(parsed, dict): + if not quiet: + self.io.tool_error("Invalid session format.") + return None + return parsed + except session_crypto.SessionCryptoError as e: + if not quiet: + self.io.tool_error(str(e)) + return None + except (UnicodeDecodeError, json.JSONDecodeError) as e: + if not quiet: + self.io.tool_error(f"Error loading session: {e}") + return None + + def _write_session_file(self, session_file: Path, session_data: dict) -> bool: + encrypt_enabled, key = self._session_encrypt_settings() + try: + if encrypt_enabled: + if not key: + self.io.tool_error( + "Session encryption is enabled but no key is configured " + f"({session_crypto.KEY_ENV} or --session-key-file)." + ) + return False + session_file.write_bytes(session_crypto.encrypt_session_dict(session_data, key)) + else: + with safe_open(session_file, "w") as f: + json.dump(session_data, f, indent=2) + return True + except session_crypto.SessionCryptoError as e: + self.io.tool_error(str(e)) + return False + except OSError as e: + self.io.tool_error(f"Error saving session: {e}") + return False + + def _build_session_data(self, session_name) -> Dict: + """Build session data dictionary from current coder state.""" + # Get relative paths for all files + editable_files = [ + self.coder.get_rel_fname(abs_fname) for abs_fname in self.coder.abs_fnames + ] + read_only_files = [ + self.coder.get_rel_fname(abs_fname) for abs_fname in self.coder.abs_read_only_fnames + ] + read_only_stubs_files = [ + self.coder.get_rel_fname(abs_fname) + for abs_fname in self.coder.abs_read_only_stubs_fnames + ] + + # Capture todo list content so it can be restored with the session + todo_content = None + try: + todo_path = self.coder.abs_root_path(self.coder.local_agent_folder("todo.txt")) + if os.path.isfile(todo_path): + todo_content = self.io.read_text(todo_path) + if todo_content is None: + todo_content = "" + except Exception as e: + self.io.tool_warning(f"Could not read todo list file: {e}") + + # Get CUR and DONE messages from ConversationManager + connected_mcps = [] + if hasattr(self.coder, "mcp_manager") and self.coder.mcp_manager: + connected_mcps = [server.name for server in self.coder.mcp_manager.connected_servers] + + # Get CUR and DONE messages from ConversationManager + connected_mcps = [] + if hasattr(self.coder, "mcp_manager") and self.coder.mcp_manager: + connected_mcps = [server.name for server in self.coder.mcp_manager.connected_servers] + + skills_data = None + if hasattr(self.coder, "skills_manager") and self.coder.skills_manager: + skills_data = { + "skills_paths": [str(p) for p in self.coder.skills_manager.directory_paths], + "skills_includelist": ( + list(self.coder.skills_manager.include_list) + if self.coder.skills_manager.include_list is not None + else [] + ), + "skills_excludelist": ( + list(self.coder.skills_manager.exclude_list) + if self.coder.skills_manager.exclude_list is not None + else [] + ), + } + + agent_config_data = None + if hasattr(self.coder, "agent_config"): + agent_config_data = { + "tools_paths": self.coder.agent_config.get("tools_paths", []), + "tools_includelist": self.coder.agent_config.get("tools_includelist", []), + "tools_excludelist": self.coder.agent_config.get("tools_excludelist", []), + } + + # Flush any queued messages so the saved chat history is complete + ConversationService.get_manager(self.coder).flush_queue() + + return { + "version": 1, + "session_name": session_name, + "model": self.coder.main_model.name, + "weak_model": self.coder.main_model.weak_model.name, + "editor_model": self.coder.main_model.editor_model.name, + "agent_model": self.coder.main_model.agent_model.name, + "editor_edit_format": self.coder.main_model.editor_edit_format, + "edit_format": self.coder.edit_format, + "chat_history": { + "done_messages": ( + ConversationService.get_manager(self.coder).get_messages_dict(MessageTag.DONE) + ), + "cur_messages": ( + ConversationService.get_manager(self.coder).get_messages_dict(MessageTag.CUR) + ), + }, + "files": { + "editable": editable_files, + "read_only": read_only_files, + "read_only_stubs": read_only_stubs_files, + }, + "settings": { + "auto_commits": self.coder.auto_commits, + "auto_lint": self.coder.auto_lint, + "auto_test": self.coder.auto_test, + }, + "todo_list": todo_content, + "mcps": connected_mcps, + "skills": skills_data, + "tools": agent_config_data, + "usage": { + "total_tokens_sent": self.coder.total_tokens_sent, + "total_tokens_received": self.coder.total_tokens_received, + "total_cached_tokens": self.coder.total_cached_tokens, + "total_cost": self.coder.total_cost, + }, + } + + def _find_session_file(self, session_identifier: str) -> Optional[Path]: + """Find session file by name or path.""" + # Check if it's a direct file path + session_file = Path(session_identifier) + if session_file.exists(): + return session_file + + # Check if it's a session name in the sessions directory + session_dir = self._get_session_directory() + + # Try with .json extension + if not session_identifier.endswith(".json"): + session_file = session_dir / f"{session_identifier}.json" + if session_file.exists(): + return session_file + + session_file = session_dir / f"{session_identifier}" + if session_file.exists(): + return session_file + + self.io.tool_error(f"Session not found: {session_identifier}") + self.io.tool_output("Use /list-sessions to see available sessions.") + return None + + async def _apply_session_data( + self, session_data: Dict, session_file: Path + ) -> (bool, Optional[str]): + """Apply session data to current coder state. + + Returns: + A tuple of (success, edit_format) + """ + try: + # Clear current state + self.coder.abs_fnames = set() + self.coder.abs_read_only_fnames = set() + self.coder.abs_read_only_stubs_fnames = set() + + # Load files + files = session_data.get("files", {}) + for rel_fname in files.get("editable", []): + abs_fname = self.coder.abs_root_path(rel_fname) + if os.path.exists(abs_fname): + self.coder.abs_fnames.add(abs_fname) + else: + self.io.tool_warning(f"File not found, skipping: {rel_fname}") + + for rel_fname in files.get("read_only", []): + abs_fname = self.coder.abs_root_path(rel_fname) + if os.path.exists(abs_fname): + self.coder.abs_read_only_fnames.add(abs_fname) + else: + self.io.tool_warning(f"File not found, skipping: {rel_fname}") + + for rel_fname in files.get("read_only_stubs", []): + abs_fname = self.coder.abs_root_path(rel_fname) + if os.path.exists(abs_fname): + self.coder.abs_read_only_stubs_fnames.add(abs_fname) + else: + self.io.tool_warning(f"File not found, skipping: {rel_fname}") + + # Load usage stats + usage = session_data.get("usage", {}) + self.coder.total_tokens_sent = usage.get("total_tokens_sent", 0) + self.coder.total_tokens_received = usage.get("total_tokens_received", 0) + self.coder.total_cached_tokens = usage.get("total_cached_tokens", 0) + self.coder.total_cost = usage.get("total_cost", 0.0) + if session_data.get("model"): + self.coder.main_model = models.Model( + session_data.get("model", self.coder.args.model), + weak_model=session_data.get("weak_model", self.coder.args.weak_model), + editor_model=session_data.get("editor_model", self.coder.args.editor_model), + agent_model=session_data.get("agent_model", self.coder.args.agent_model), + editor_edit_format=session_data.get( + "editor_edit_format", self.coder.args.editor_edit_format + ), + io=self.io, + verbose=self.coder.args.verbose, + retries=self.coder.main_model.retries, + debug=self.coder.main_model.debug, + ) + + # Load settings + settings = session_data.get("settings", {}) + if "auto_commits" in settings: + self.coder.auto_commits = settings["auto_commits"] + if "auto_lint" in settings: + self.coder.auto_lint = settings["auto_lint"] + if "auto_test" in settings: + self.coder.auto_test = settings["auto_test"] + + # Restore todo list content if present in the session + if "todo_list" in session_data: + todo_path = self.coder.abs_root_path(self.coder.local_agent_folder("todo.txt")) + todo_content = session_data.get("todo_list") + try: + if todo_content is None: + if os.path.exists(todo_path): + os.remove(todo_path) + else: + self.io.write_text(todo_path, todo_content) + except Exception as e: + self.io.tool_warning(f"Could not restore todo list: {e}") + + # Clear CUR and DONE messages from ConversationManager + ConversationService.get_manager(self.coder).reset() + ConversationService.get_files(self.coder).reset() + self.coder.format_chat_chunks() + + # Load chat history + chat_history = session_data.get("chat_history", {}) + done_messages = chat_history.get("done_messages", []) + cur_messages = chat_history.get("cur_messages", []) + + # Add messages to ConversationManager (source of truth) + # Add done messages + for msg in done_messages: + ConversationService.get_manager(self.coder).add_message( + message_dict=msg, + tag=MessageTag.DONE, + ) + # Add current messages + for msg in cur_messages: + ConversationService.get_manager(self.coder).add_message( + message_dict=msg, + tag=MessageTag.CUR, + ) + + self.io.tool_output( + f"Session loaded: {session_data.get('session_name', session_file.stem)}" + ) + self.io.tool_output( + f"Model: {session_data.get('model', 'unknown')}, Edit format:" + f" {session_data.get('edit_format', 'unknown')}" + ) + + # Show summary + num_messages = len(self.coder.done_messages) + len(self.coder.cur_messages) + num_files = ( + len(self.coder.abs_fnames) + + len(self.coder.abs_read_only_fnames) + + len(self.coder.abs_read_only_stubs_fnames) + ) + self.io.tool_output(f"Loaded {num_messages} messages and {num_files} files") + + # Load MCPs + saved_mcps = session_data.get("mcps", []) + if hasattr(self.coder, "mcp_manager") and self.coder.mcp_manager: + current_mcps = {server.name for server in self.coder.mcp_manager.connected_servers} + saved_mcps_set = set(saved_mcps) + + to_disconnect = current_mcps - saved_mcps_set + for mcp_name in to_disconnect: + await self.coder.mcp_manager.disconnect_server(mcp_name) + + to_connect = saved_mcps_set - current_mcps + for mcp_name in to_connect: + await self.coder.mcp_manager.connect_server(mcp_name) + + # Load skills + skills_data = session_data.get("skills") + if skills_data and hasattr(self.coder, "skills_manager") and self.coder.skills_manager: + self.coder.skills_manager.directory_paths = skills_data.get("skills_paths", []) + self.coder.skills_manager.include_list = set( + skills_data.get("skills_includelist", []) + ) + self.coder.skills_manager.exclude_list = set( + skills_data.get("skills_excludelist", []) + ) + + # Load tools config + agent_config_data = session_data.get("tools") + if agent_config_data and hasattr(self.coder, "agent_config"): + self.coder.agent_config.update(agent_config_data) + from cecli.tools.utils.registry import ToolRegistry + + ToolRegistry.build_registry(agent_config=self.coder.agent_config) + self.coder.loaded_custom_tools = ToolRegistry.loaded_custom_tools + + # Return True and the edit format so the Coder can be switched + edit_format = session_data.get("edit_format") + return True, edit_format + + except Exception as e: + self.io.tool_error(f"Error applying session data: {e}") + return False, None diff --git a/requirements.txt b/requirements.txt index 64d185820fb..9e182322f7d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -608,7 +608,7 @@ uvicorn[standard]==0.38.0 # -c requirements/common-constraints.txt # chromadb # mcp -uvloop==0.22.1 +uvloop==0.22.1 ; platform_python_implementation != 'PyPy' and sys_platform != 'cygwin' and sys_platform != 'win32' # via # -c requirements/common-constraints.txt # uvicorn @@ -642,6 +642,6 @@ zipp==3.23.0 # via # -c requirements/common-constraints.txt # importlib-metadata - + tree-sitter==0.23.2; python_version < "3.10" tree-sitter>=0.25.1; python_version >= "3.10" diff --git a/requirements/common-constraints.txt b/requirements/common-constraints.txt index 5f79887e8e4..53f2548b1f7 100644 --- a/requirements/common-constraints.txt +++ b/requirements/common-constraints.txt @@ -514,7 +514,7 @@ uvicorn[standard]==0.38.0 # via # chromadb # mcp -uvloop==0.22.1 +uvloop==0.22.1 ; platform_python_implementation != 'PyPy' and sys_platform != 'cygwin' and sys_platform != 'win32' # via uvicorn virtualenv==20.35.4 # via pre-commit diff --git a/tests/basic/test_retry_config.py b/tests/basic/test_retry_config.py new file mode 100644 index 00000000000..67f38610852 --- /dev/null +++ b/tests/basic/test_retry_config.py @@ -0,0 +1,64 @@ +import json +import pytest +from unittest.mock import AsyncMock, call, patch + +from cecli.models import _parse_retry_config, Model +from cecli.exceptions import LiteLLMExceptions +from cecli.llm import litellm + + +def test_parse_retry_config_string(): + config_str = '{"retry_timeout": 15, "retry-on-empty": true}' + result = _parse_retry_config(config_str) + assert result["retry_timeout"] == 15.0 + assert result["retry_on_empty"] is True + # defaults + assert result["retry_backoff_factor"] == 1.5 + assert result["retry_on_unavailable"] is True + + +def test_parse_retry_config_dict(): + config_dict = {"retry_timeout": 10.0, "retry_backoff_factor": 2.0, "retry-on-unavailable": False} + result = _parse_retry_config(config_dict) + assert result["retry_timeout"] == 10.0 + assert result["retry_backoff_factor"] == 2.0 + assert result["retry_on_unavailable"] is False + assert result["retry_on_empty"] is False + + +@pytest.mark.asyncio +async def test_simple_send_with_retries_honors_timeout(): + # Setup model with a short retry timeout limit + model = Model("gpt-4o", retries={"retry_timeout": 0.5, "retry_backoff_factor": 2.0}) + + # retry_delay starts at 0.125 and is multiplied by the backoff factor + # BEFORE each retry sleep; retry_timeout caps the per-retry delay: + # attempt 1 fails -> 0.125 * 2.0 = 0.25 (<= 0.5, sleep and retry) + # attempt 2 fails -> 0.25 * 2.0 = 0.50 (<= 0.5, sleep and retry) + # attempt 3 fails -> 0.50 * 2.0 = 1.00 (> 0.5, give up) + # We mock send_completion to continually raise a retryable LiteLLM exception. + err = litellm.APIConnectionError( + message="Simulated connection error", + llm_provider="openai", + model="gpt-4o", + request=None + ) + + mock_send = AsyncMock(side_effect=err) + + with patch.object(model, 'send_completion', mock_send), \ + patch('time.sleep') as mock_sleep, \ + patch('builtins.print'): # Mute prints in test output + + content, response = await model.simple_send_with_retries(messages=[]) + + # It should exit yielding None, None because it exhausted retries. + assert content is None + assert response is None + + # The backoff factor is applied before each sleep, so the sleeps are + # 0.25 then 0.50; the third failure would need 1.0 > 0.5, so it stops. + + assert mock_send.call_count == 3 + assert mock_sleep.call_count == 2 + assert mock_sleep.call_args_list == [call(0.25), call(0.5)] From 75ce809d5efb81ca0ee7054118f17ee052c3b258 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 25 Sep 2026 19:54:24 -0700 Subject: [PATCH 2/8] update --- tests/basic/test_retry_config.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/basic/test_retry_config.py b/tests/basic/test_retry_config.py index 67f38610852..753ed24bdcd 100644 --- a/tests/basic/test_retry_config.py +++ b/tests/basic/test_retry_config.py @@ -1,9 +1,7 @@ -import json import pytest from unittest.mock import AsyncMock, call, patch from cecli.models import _parse_retry_config, Model -from cecli.exceptions import LiteLLMExceptions from cecli.llm import litellm @@ -36,7 +34,12 @@ async def test_simple_send_with_retries_honors_timeout(): # attempt 1 fails -> 0.125 * 2.0 = 0.25 (<= 0.5, sleep and retry) # attempt 2 fails -> 0.25 * 2.0 = 0.50 (<= 0.5, sleep and retry) # attempt 3 fails -> 0.50 * 2.0 = 1.00 (> 0.5, give up) - # We mock send_completion to continually raise a retryable LiteLLM exception. + err = litellm.APIConnectionError( + message="Simulated connection error", + llm_provider="openai", + model="gpt-4o", + request=None, + ) err = litellm.APIConnectionError( message="Simulated connection error", llm_provider="openai", From 7e60027b775113d0a1c354cf8da41d4301e9d6af Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 25 Sep 2026 23:06:13 -0700 Subject: [PATCH 3/8] cli-65: update linting --- tests/basic/test_retry_config.py | 37 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/tests/basic/test_retry_config.py b/tests/basic/test_retry_config.py index 753ed24bdcd..56b609ea209 100644 --- a/tests/basic/test_retry_config.py +++ b/tests/basic/test_retry_config.py @@ -1,8 +1,9 @@ -import pytest from unittest.mock import AsyncMock, call, patch -from cecli.models import _parse_retry_config, Model +import pytest + from cecli.llm import litellm +from cecli.models import Model, _parse_retry_config def test_parse_retry_config_string(): @@ -16,7 +17,11 @@ def test_parse_retry_config_string(): def test_parse_retry_config_dict(): - config_dict = {"retry_timeout": 10.0, "retry_backoff_factor": 2.0, "retry-on-unavailable": False} + config_dict = { + "retry_timeout": 10.0, + "retry_backoff_factor": 2.0, + "retry-on-unavailable": False, + } result = _parse_retry_config(config_dict) assert result["retry_timeout"] == 10.0 assert result["retry_backoff_factor"] == 2.0 @@ -40,28 +45,24 @@ async def test_simple_send_with_retries_honors_timeout(): model="gpt-4o", request=None, ) - err = litellm.APIConnectionError( - message="Simulated connection error", - llm_provider="openai", - model="gpt-4o", - request=None - ) - + mock_send = AsyncMock(side_effect=err) - - with patch.object(model, 'send_completion', mock_send), \ - patch('time.sleep') as mock_sleep, \ - patch('builtins.print'): # Mute prints in test output - + + with ( + patch.object(model, "send_completion", mock_send), + patch("time.sleep") as mock_sleep, + patch("builtins.print"), + ): # Mute prints in test output + content, response = await model.simple_send_with_retries(messages=[]) - + # It should exit yielding None, None because it exhausted retries. assert content is None assert response is None - + # The backoff factor is applied before each sleep, so the sleeps are # 0.25 then 0.50; the third failure would need 1.0 > 0.5, so it stops. - + assert mock_send.call_count == 3 assert mock_sleep.call_count == 2 assert mock_sleep.call_args_list == [call(0.25), call(0.5)] From 52c0b3605c626cebeff00061d69bc2dbe18d8373 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 26 Sep 2026 15:32:24 -0700 Subject: [PATCH 4/8] feat: implement retry-on-forbidden feature --- cecli/coders/base_coder.py | 2 ++ cecli/models.py | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index 7b23d50a814..a5bde7ff756 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -2860,6 +2860,8 @@ async def format_in_executor(): should_retry = ex_info.retry if ex_info.name == "ServiceUnavailableError": should_retry = should_retry or retry_config["retry_on_unavailable"] + if ex_info.name == "PermissionDeniedError": + should_retry = should_retry or retry_config["retry_on_forbidden"] if should_retry: retry_delay *= retry_config["retry_backoff_factor"] diff --git a/cecli/models.py b/cecli/models.py index 5d13176e14b..b767a1e56b4 100644 --- a/cecli/models.py +++ b/cecli/models.py @@ -133,6 +133,7 @@ class ModelSettings: retries: Optional[dict] = None retry_backoff_factor: float = 1.5 retry_on_unavailable: bool = True + retry_on_forbidden: bool = False retry_timeout: float = 30 request_timeout: int = request_timeout debug: bool = False @@ -1463,6 +1464,9 @@ async def send_completion( self.retry_on_unavailable = bool( nested.getter(retry_config, "retry-on-unavailable", True) ) + self.retry_on_forbidden = bool( + nested.getter(retry_config, "retry-on-forbidden", False) + ) self.retry_backoff_factor = float( nested.getter(retry_config, "retry-backoff-factor", 1.5) ) @@ -1497,6 +1501,8 @@ async def send_completion( should_retry = ex_info.retry if ex_info.name == "ServiceUnavailableError": should_retry = should_retry or self.retry_on_unavailable + elif ex_info.name == "PermissionDeniedError": + should_retry = should_retry or self.retry_on_forbidden custom_retry_delay = self._extract_retry_delay(err) if custom_retry_delay is not None: @@ -1561,6 +1567,7 @@ async def simple_send_with_retries( retry_backoff_factor = retry_config["retry_backoff_factor"] retry_timeout = retry_config["retry_timeout"] retry_on_unavailable = retry_config["retry_on_unavailable"] + retry_on_forbidden = retry_config["retry_on_forbidden"] if self.verbose: dump(messages) @@ -1614,6 +1621,8 @@ async def simple_send_with_retries( should_retry = ex_info.retry if ex_info.name == "ServiceUnavailableError": should_retry = should_retry or retry_on_unavailable + elif ex_info.name == "PermissionDeniedError": + should_retry = should_retry or retry_on_forbidden custom_retry_delay = self._extract_retry_delay(err) if custom_retry_delay is not None: @@ -1807,6 +1816,7 @@ def parse_retry_config(retries_input): retry_timeout: 30 retry_backoff_factor: 1.5 retry_on_unavailable: True + retry_on_forbidden: False retry_on_empty: False """ config = dict() @@ -1832,6 +1842,7 @@ def _get(key, default): "retry_timeout": float(_get("retry_timeout", 30)), "retry_backoff_factor": float(_get("retry_backoff_factor", 1.5)), "retry_on_unavailable": bool(_get("retry_on_unavailable", True)), + "retry_on_forbidden": bool(_get("retry_on_forbidden", False)), "retry_on_empty": bool(_get("retry_on_empty", False)), } From 8999518ab908e6c740a43195123a260c2f58e365 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 26 Sep 2026 22:33:04 -0700 Subject: [PATCH 5/8] style: fix formatting of retry_on_forbidden assignment --- cecli/models.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cecli/models.py b/cecli/models.py index b767a1e56b4..b06c9dcc6cf 100644 --- a/cecli/models.py +++ b/cecli/models.py @@ -1464,9 +1464,7 @@ async def send_completion( self.retry_on_unavailable = bool( nested.getter(retry_config, "retry-on-unavailable", True) ) - self.retry_on_forbidden = bool( - nested.getter(retry_config, "retry-on-forbidden", False) - ) + self.retry_on_forbidden = bool(nested.getter(retry_config, "retry-on-forbidden", False)) self.retry_backoff_factor = float( nested.getter(retry_config, "retry-backoff-factor", 1.5) ) From 25505e914e26185a4394ae378486aecd1469db6d Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 29 Sep 2026 14:31:34 -0700 Subject: [PATCH 6/8] cli-67: retry on empty addition --- cecli/coders/base_coder.py | 26 +++++++++++++++++++++++--- cecli/website/docs/config/retries.md | 1 + tests/basic/test_retry_config.py | 6 +++--- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index a5bde7ff756..d9051d1ea12 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -3988,7 +3988,7 @@ async def show_send_output(self, completion): if ( not len(self.partial_response_content) and not len(self.partial_response_tool_calls) - and not len(self.partial_response_reasoning_content) + and not _is_meaningful_reasoning(self.partial_response_reasoning_content) ): self.empty_response = True return @@ -4118,7 +4118,8 @@ async def show_send_output_stream(self, completion): text += reasoning_content self.got_reasoning_content = True - received_content = True + if _is_meaningful_reasoning(reasoning_content): + received_content = True self.token_profiler.on_token() self.io.update_spinner_suffix(reasoning_content) @@ -4194,7 +4195,14 @@ async def show_send_output_stream(self, completion): self.io.tool_warning("Execution stopped by on message hook") return - if not received_content and len(self.partial_response_tool_calls) == 0: + # Treat the response as empty when nothing was received, or when the + # only thing received was reasoning made entirely of non-alphanumeric + # characters (e.g. moonshotai/kimi-k3 returning "!!!!"). + if ( + not received_content + and len(self.partial_response_tool_calls) == 0 + and not _is_meaningful_reasoning(self.partial_response_reasoning_content) + ): self.empty_response = True return @@ -5383,3 +5391,15 @@ def _first_usage_tokens(usage: object, paths: list[str], default: int = 0) -> in if value is not None: return value return default + + +def _is_meaningful_reasoning(text): + """Return True if reasoning text contains at least one alphanumeric character. + + Some providers (e.g. moonshotai/kimi-k3) occasionally return completions + with empty ``content`` and a ``reasoning_content`` made entirely of + punctuation (e.g. ``"!!!!"``). Those responses are effectively empty, so + the empty-response detector only lets reasoning count as response + content when it holds at least one alphanumeric character. + """ + return bool(text) and any(ch.isalnum() for ch in text) diff --git a/cecli/website/docs/config/retries.md b/cecli/website/docs/config/retries.md index 73891eec981..ffaaebca736 100644 --- a/cecli/website/docs/config/retries.md +++ b/cecli/website/docs/config/retries.md @@ -11,6 +11,7 @@ Cecli can be configured to retry failed API calls. This is useful for handling i - `retry-timeout`: The timeout in seconds for each retry. - `retry-backoff-factor`: The backoff factor to use between retries. - `retry-on-unavailable`: Whether to retry on 503 Service Unavailable errors. +- `retry-on-empty`: Whether to retry when the model returns an empty response. A response is considered empty when it has no content, no tool calls, and no *meaningful* reasoning. Reasoning that contains no alphanumeric characters (for example a `reasoning_content` of `"!!!!"`) does not count as a response, so it is retried like any other empty response. Example usage in `.cecli.conf.yml`: diff --git a/tests/basic/test_retry_config.py b/tests/basic/test_retry_config.py index 56b609ea209..35a798bac3e 100644 --- a/tests/basic/test_retry_config.py +++ b/tests/basic/test_retry_config.py @@ -3,12 +3,12 @@ import pytest from cecli.llm import litellm -from cecli.models import Model, _parse_retry_config +from cecli.models import Model, parse_retry_config def test_parse_retry_config_string(): config_str = '{"retry_timeout": 15, "retry-on-empty": true}' - result = _parse_retry_config(config_str) + result = parse_retry_config(config_str) assert result["retry_timeout"] == 15.0 assert result["retry_on_empty"] is True # defaults @@ -22,7 +22,7 @@ def test_parse_retry_config_dict(): "retry_backoff_factor": 2.0, "retry-on-unavailable": False, } - result = _parse_retry_config(config_dict) + result = parse_retry_config(config_dict) assert result["retry_timeout"] == 10.0 assert result["retry_backoff_factor"] == 2.0 assert result["retry_on_unavailable"] is False From f82edcb99cf08881ceae18795d74add7d4cfd1cf Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 29 Sep 2026 14:45:19 -0700 Subject: [PATCH 7/8] cli-67: retry on empty addition --- tests/basic/test_empty_response.py | 223 +++++++++++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 tests/basic/test_empty_response.py diff --git a/tests/basic/test_empty_response.py b/tests/basic/test_empty_response.py new file mode 100644 index 00000000000..79a70198cc0 --- /dev/null +++ b/tests/basic/test_empty_response.py @@ -0,0 +1,223 @@ +"""Tests for empty-response detection with non-meaningful reasoning content. + +Regression coverage for the ``retry-on-empty`` fix: a completion with empty +``content``, no tool calls, and a ``reasoning_content`` made entirely of +non-alphanumeric characters (e.g. moonshotai/kimi-k3 returning ``"!!!!"``) must +be classified as an empty response so the existing retry loop engages. +""" + +from types import SimpleNamespace + +import pytest + +from cecli.coders.base_coder import Coder, _is_meaningful_reasoning +from cecli.helpers.threading import ThreadSafeEvent +from cecli.llm import litellm + + +# --------------------------------------------------------------------------- # +# Test doubles +# --------------------------------------------------------------------------- # +class _AlwaysSetEvent: + def is_set(self): + return True + + +class _FakeIO: + def __init__(self): + self.confirmation_in_progress_event = _AlwaysSetEvent() + self.assistant_outputs = [] + + def tool_error(self, *a, **k): + pass + + def tool_warning(self, *a, **k): + pass + + def update_spinner_suffix(self, *a, **k): + pass + + def reset_streaming_response(self): + pass + + def stream_output(self, *a, **k): + pass + + def ai_output(self, *a, **k): + pass + + def tool_output(self, *a, **k): + pass + + def assistant_output(self, *a, **k): + self.assistant_outputs.append(a) + + +class _FakeTokenProfiler: + def start(self): + pass + + def on_token(self): + pass + + def on_error(self): + pass + + def add_to_usage_report(self, *a, **k): + return a[0] if a else "" + + +def _make_coder(stream=False): + coder = Coder.__new__(Coder) + coder.stream = stream + coder.verbose = False + coder.args = SimpleNamespace(debug=False, show_thinking=False) + coder.io = _FakeIO() + coder.interrupt_event = ThreadSafeEvent() + coder.pretty = False + coder.reasoning_tag_name = "THINKING" + coder.got_reasoning_content = False + coder.ended_reasoning_content = False + coder.empty_response = False + coder.tool_reflection = False + coder.partial_response_content = "" + coder.partial_response_reasoning_content = "" + coder.partial_response_chunks = [] + coder.partial_response_tool_calls = [] + coder.partial_response_function_call = dict() + coder.partial_response_consolidated = None + coder.multi_response_content = "" + coder.chat_completion_response_hashes = [] + coder._streaming_buffer_length = 0 + coder.token_profiler = _FakeTokenProfiler() + coder._output_loop_detected = False + coder._output_loop_message = "" + coder._has_empty_reflected = False + coder.edit_format = "code" + return coder + + +def _tc(index, call_id, name, arguments): + return litellm.ChatCompletionMessageToolCall( + id=call_id, + function=litellm.Function(arguments=arguments or "", name=name), + type="function", + index=index, + ) + + +def _non_streaming_completion(content="", reasoning=None, tool_calls=None): + message = {"role": "assistant", "content": content} + if reasoning is not None: + message["reasoning_content"] = reasoning + if tool_calls is not None: + message["tool_calls"] = tool_calls + return litellm.ModelResponse( + id="test-completion", + created=0, + model="gpt-test", + object="chat.completion", + choices=[{"finish_reason": "stop", "index": 0, "message": message}], + usage={"completion_tokens": 1, "prompt_tokens": 1, "total_tokens": 2}, + ) + + +def _reasoning_chunk(text): + delta = litellm.Delta(role="assistant", content=None, reasoning_content=text) + choice = litellm.StreamChoice(finish_reason=None, index=0, delta=delta) + return litellm.StreamChunk( + id="cmpl-test", created=1000, model="gpt-test", choices=[choice], usage=None + ) + + +async def _agen(chunks): + for chunk in chunks: + yield chunk + + +# --------------------------------------------------------------------------- # +# _is_meaningful_reasoning unit tests +# --------------------------------------------------------------------------- # +def test_is_meaningful_reasoning_empty_and_none(): + assert _is_meaningful_reasoning("") is False + assert _is_meaningful_reasoning(None) is False + + +def test_is_meaningful_reasoning_punctuation_only(): + assert _is_meaningful_reasoning("!!!!") is False + assert _is_meaningful_reasoning("...?!,;:") is False + + +def test_is_meaningful_reasoning_whitespace_only(): + assert _is_meaningful_reasoning(" \n\t ") is False + + +def test_is_meaningful_reasoning_normal_text(): + assert _is_meaningful_reasoning("Let me think about this") is True + + +def test_is_meaningful_reasoning_mixed_punctuation_and_alnum(): + assert _is_meaningful_reasoning("42?") is True + assert _is_meaningful_reasoning("!!!a!!!") is True + + +# --------------------------------------------------------------------------- # +# Non-streaming path (show_send_output) +# --------------------------------------------------------------------------- # +@pytest.mark.asyncio +async def test_junk_reasoning_is_empty_non_streaming(): + coder = _make_coder(stream=False) + await coder.show_send_output(_non_streaming_completion(content="", reasoning="!" * 40)) + assert coder.empty_response is True + + +@pytest.mark.asyncio +async def test_meaningful_reasoning_is_not_empty_non_streaming(): + coder = _make_coder(stream=False) + await coder.show_send_output( + _non_streaming_completion(content="", reasoning="Let me think about this") + ) + assert coder.empty_response is False + + +@pytest.mark.asyncio +async def test_tool_calls_are_not_empty_non_streaming(): + coder = _make_coder(stream=False) + await coder.show_send_output( + _non_streaming_completion( + content="", + reasoning="", + tool_calls=[_tc(0, "call_1", "Local--ls", "{}")], + ) + ) + assert coder.empty_response is False + + +@pytest.mark.asyncio +async def test_mixed_reasoning_is_not_empty_non_streaming(): + coder = _make_coder(stream=False) + await coder.show_send_output(_non_streaming_completion(content="", reasoning="42?")) + assert coder.empty_response is False + + +# --------------------------------------------------------------------------- # +# Streaming path (show_send_output_stream) +# --------------------------------------------------------------------------- # +@pytest.mark.asyncio +async def test_junk_reasoning_is_empty_streaming(): + coder = _make_coder(stream=True) + coder.args.show_thinking = True + async for _ in coder.show_send_output_stream(_agen([_reasoning_chunk("!" * 40)])): + pass + assert coder.empty_response is True + + +@pytest.mark.asyncio +async def test_meaningful_reasoning_is_not_empty_streaming(): + coder = _make_coder(stream=True) + coder.args.show_thinking = True + async for _ in coder.show_send_output_stream( + _agen([_reasoning_chunk("Let me think about this")]) + ): + pass + assert coder.empty_response is False \ No newline at end of file From e3107a04279908bfc37e6b78b3d9997333d67e33 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 29 Sep 2026 15:18:56 -0700 Subject: [PATCH 8/8] fixed linting --- tests/basic/test_empty_response.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/basic/test_empty_response.py b/tests/basic/test_empty_response.py index 79a70198cc0..4b562fc7222 100644 --- a/tests/basic/test_empty_response.py +++ b/tests/basic/test_empty_response.py @@ -220,4 +220,4 @@ async def test_meaningful_reasoning_is_not_empty_streaming(): _agen([_reasoning_chunk("Let me think about this")]) ): pass - assert coder.empty_response is False \ No newline at end of file + assert coder.empty_response is False