From 8615650e203298d4353fa44dbe69fbc2945c898a Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Fri, 10 Apr 2026 22:41:05 +0100 Subject: [PATCH 1/8] Remove rich dependency, merge display into orchestrator, drop execute_batch Replace rich-styled output with plain print/stdout.write, eliminating the display.py module and the rich dependency entirely. Inline the ChatClient from chat.py. Remove unused execute_batch() and its test. Co-Authored-By: Claude Opus 4.6 (1M context) --- pyproject.toml | 1 - src/cli_chat/display.py | 87 ------------------------------------ src/cli_chat/main.py | 5 +-- src/cli_chat/orchestrator.py | 41 +++++++---------- src/cli_chat/tools.py | 9 ---- tests/test_e2e.py | 10 ----- uv.lock | 36 --------------- 7 files changed, 18 insertions(+), 171 deletions(-) delete mode 100644 src/cli_chat/display.py diff --git a/pyproject.toml b/pyproject.toml index fc940ec..db8ac91 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,6 @@ dependencies = [ "pydantic>=2.10.0", "pydantic-settings>=2.7.0", "python-dotenv>=1.0.0", - "rich>=13.9.0", "tenacity>=9.0.0", ] diff --git a/src/cli_chat/display.py b/src/cli_chat/display.py deleted file mode 100644 index fcbae72..0000000 --- a/src/cli_chat/display.py +++ /dev/null @@ -1,87 +0,0 @@ -"""Display helpers — styled output, spinners, and streaming.""" - -from __future__ import annotations - -import sys - -from rich.console import Console -from rich.live import Live -from rich.spinner import Spinner -from rich.theme import Theme - -_THEME = Theme( - { - "user": "bold cyan", - "assistant": "bold green", - "tool": "yellow", - "tool.name": "bold yellow", - "error": "bold red", - "meta": "dim", - } -) - -console = Console(theme=_THEME) - -# ANSI codes for raw stdout (used in input prompt where rich can't help) -_CYAN_BOLD = "\033[1;36m" -_RESET = "\033[0m" - - -def print_input_prompt() -> None: - """Print separator + colored 'You:' prompt. Cursor stays on the same line.""" - console.print() - console.rule(style="meta") - sys.stdout.write(f"{_CYAN_BOLD}You:{_RESET} ") - sys.stdout.flush() - - -def print_assistant_header() -> None: - console.print() - console.print("Assistant:", style="assistant") - - -def print_streaming_token(token: str) -> None: - sys.stdout.write(token) - sys.stdout.flush() - - -def finish_streaming() -> None: - sys.stdout.write("\n") - sys.stdout.flush() - - -def print_tool_call(tool_name: str, args: dict) -> None: - if tool_name == "get_weather": - detail = args.get("location", "?") - elif tool_name == "research_topic": - detail = args.get("topic", "?") - else: - detail = str(args) - console.print(f" [tool]⚡ [tool.name]{tool_name}[/tool.name]({detail})[/tool]") - - -def tool_spinner(tool_name: str, args: dict) -> Live: - if tool_name == "get_weather": - label = f"Getting weather for {args.get('location', '?')}..." - elif tool_name == "research_topic": - label = f"Researching {args.get('topic', '?')}... (Ctrl+C to cancel)" - else: - label = f"Running {tool_name}..." - spinner = Spinner("dots", text=f"[tool]{label}[/tool]") - return Live(spinner, console=console, transient=True) - - -def print_tool_result_ok(tool_name: str) -> None: - console.print(f" [green]✓[/green] [meta]{tool_name} completed[/meta]") - - -def print_tool_result_error(tool_name: str, message: str) -> None: - console.print(f" [error]✗ {tool_name}:[/error] {message}") - - -def print_error(msg: str) -> None: - console.print(f"[error]{msg}[/error]") - - -def print_dim(msg: str) -> None: - console.print(f"[meta]{msg}[/meta]") diff --git a/src/cli_chat/main.py b/src/cli_chat/main.py index 821e4a5..8b89d4a 100644 --- a/src/cli_chat/main.py +++ b/src/cli_chat/main.py @@ -8,7 +8,6 @@ import signal import uuid -from cli_chat.display import console, print_dim from cli_chat.models import Settings from cli_chat.orchestrator import Orchestrator @@ -39,14 +38,14 @@ async def _run() -> None: loop = asyncio.get_running_loop() loop.add_signal_handler(signal.SIGINT, orch.handle_interrupt) - console.print("[bold]CLI Chat[/bold] — type 'exit' to quit, Ctrl+C to cancel\n") + print("CLI Chat — type 'exit' to quit, Ctrl+C to cancel\n") try: await orch.run() finally: loop.remove_signal_handler(signal.SIGINT) await orch.close() - print_dim("\nGoodbye!") + print("\nGoodbye!") logger.info("Session ended") diff --git a/src/cli_chat/orchestrator.py b/src/cli_chat/orchestrator.py index 1278cd3..f25e516 100644 --- a/src/cli_chat/orchestrator.py +++ b/src/cli_chat/orchestrator.py @@ -13,18 +13,6 @@ from openai.types.chat import ChatCompletionToolMessageParam, ChatCompletionUserMessageParam from openai.types.chat.chat_completion_message_tool_call import ChatCompletionMessageToolCall, Function -from cli_chat.display import ( - finish_streaming, - print_assistant_header, - print_dim, - print_error, - print_input_prompt, - print_streaming_token, - print_tool_call, - print_tool_result_error, - print_tool_result_ok, - tool_spinner, -) from cli_chat.tools import TOOL_DEFINITIONS, ToolExecutor if TYPE_CHECKING: @@ -83,7 +71,9 @@ async def run(self) -> None: async def _read_input(self) -> str | None: """Read from stdin without threads, cancellable via Ctrl+C.""" loop = asyncio.get_running_loop() - print_input_prompt() + print(flush=True) + sys.stdout.write("You: ") + sys.stdout.flush() line_future: asyncio.Future[str] = loop.create_future() @@ -121,7 +111,8 @@ async def _process_turn(self, user_input: str) -> None: self._append_assistant_message(content, tool_calls) if not tool_calls: - finish_streaming() + sys.stdout.write("\n") + sys.stdout.flush() logger.info("Turn %d: assistant responded (%d chars)", self._turn_count, len(content)) break @@ -159,7 +150,7 @@ async def _stream_response(self) -> tuple[str, list[ChatCompletionMessageToolCal ) except Exception as exc: # pylint: disable=broad-exception-caught logger.error("LLM stream creation failed: %s", exc, exc_info=True) - print_error(f"LLM error: {exc}") + print(f"Error: LLM error: {exc}", file=sys.stderr) return None content = "" @@ -171,7 +162,7 @@ async def _stream_response(self) -> tuple[str, list[ChatCompletionMessageToolCal if self._cancel_event.is_set(): logger.info("Stream cancelled by user (content so far: %d chars)", len(content)) await stream.close() - print_dim("\n[cancelled]") + print("\n[cancelled]") return None delta = chunk.choices[0].delta if chunk.choices else None @@ -180,10 +171,11 @@ async def _stream_response(self) -> tuple[str, list[ChatCompletionMessageToolCal if delta.content: if not header_printed: - print_assistant_header() + print("\nAssistant:") header_printed = True content += delta.content - print_streaming_token(delta.content) + sys.stdout.write(delta.content) + sys.stdout.flush() if delta.tool_calls: for tc_delta in delta.tool_calls: @@ -201,7 +193,7 @@ async def _stream_response(self) -> tuple[str, list[ChatCompletionMessageToolCal except Exception as exc: # pylint: disable=broad-exception-caught logger.error("Stream error: %s", exc, exc_info=True) - print_error(f"\nStream error: {exc}") + print(f"\nError: Stream error: {exc}", file=sys.stderr) return None logger.debug("Stream completed: %d chars content, %d tool calls", len(content), len(tool_calls_by_index)) @@ -219,21 +211,20 @@ async def _execute_tools(self, tool_calls: list[ChatCompletionMessageToolCall]) for tc in tool_calls: if self._cancel_event.is_set(): logger.info("Tool execution cancelled before %s", tc.function.name) - print_dim("[cancelled]") + print("[cancelled]") return None args = {} with contextlib.suppress(json.JSONDecodeError): args = json.loads(tc.function.arguments) - print_tool_call(tc.function.name, args) - with tool_spinner(tc.function.name, args): - result = await self._tools.execute(tc, self._cancel_event) + print(f" > {tc.function.name}({args})") + result = await self._tools.execute(tc, self._cancel_event) if result.error: - print_tool_result_error(tc.function.name, result.content) + print(f" x {tc.function.name}: {result.content}") else: - print_tool_result_ok(tc.function.name) + print(f" OK {tc.function.name}") results.append(result) return results diff --git a/src/cli_chat/tools.py b/src/cli_chat/tools.py index 9fa0371..f5a7d44 100644 --- a/src/cli_chat/tools.py +++ b/src/cli_chat/tools.py @@ -211,12 +211,3 @@ async def _cancellable_request( if cancel_task in done: raise asyncio.CancelledError return request_task.result() - - async def execute_batch( - self, - tool_calls: list[tc_module.ChatCompletionMessageToolCall], - cancel_event: asyncio.Event | None = None, - ) -> list[models.ToolResult]: - logger.info("Executing %d tool calls in parallel", len(tool_calls)) - tasks = [self.execute(tc, cancel_event) for tc in tool_calls] - return list(await asyncio.gather(*tasks)) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index c432c88..2711588 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -160,16 +160,6 @@ async def execute() -> models.ToolResult: result, _ = await asyncio.gather(execute(), cancel_after_delay()) assert result.content # should have some content regardless - @pytest.mark.asyncio - async def test_batch_cancellation(self, executor: tools_module.ToolExecutor) -> None: - """Batch execute respects cancellation across all tool calls.""" - cancel = asyncio.Event() - cancel.set() - results = await executor.execute_batch( - [_weather_call("London"), _research_call("test")], # type: ignore[arg-type] - cancel_event=cancel, - ) - assert all(r.error for r in results) # ── Error handling tests ────────────────────────────────────────────────────── diff --git a/uv.lock b/uv.lock index f6fb99e..6006f65 100644 --- a/uv.lock +++ b/uv.lock @@ -52,7 +52,6 @@ dependencies = [ { name = "pydantic" }, { name = "pydantic-settings" }, { name = "python-dotenv" }, - { name = "rich" }, { name = "tenacity" }, ] @@ -72,7 +71,6 @@ requires-dist = [ { name = "pydantic", specifier = ">=2.10.0" }, { name = "pydantic-settings", specifier = ">=2.7.0" }, { name = "python-dotenv", specifier = ">=1.0.0" }, - { name = "rich", specifier = ">=13.9.0" }, { name = "tenacity", specifier = ">=9.0.0" }, ] @@ -248,18 +246,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/da/e9/1f9ada30cef7b05e74bb06f52127e7a724976c225f46adb65c37b1dadfb6/jiter-0.14.0-graalpy312-graalpy250_312_native-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:67f00d94b281174144d6532a04b66a12cb866cbdc47c3af3bfe2973677f9861a", size = 349613, upload-time = "2026-04-10T14:28:40.066Z" }, ] -[[package]] -name = "markdown-it-py" -version = "4.0.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "mdurl" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/5b/f5/4ec618ed16cc4f8fb3b701563655a69816155e79e24a17b651541804721d/markdown_it_py-4.0.0.tar.gz", hash = "sha256:cb0a2b4aa34f932c007117b194e945bd74e0ec24133ceb5bac59009cda1cb9f3", size = 73070, upload-time = "2025-08-11T12:57:52.854Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/94/54/e7d793b573f298e1c9013b8c4dade17d481164aa517d1d7148619c2cedbf/markdown_it_py-4.0.0-py3-none-any.whl", hash = "sha256:87327c59b172c5011896038353a81343b6754500a08cd7a4973bb48c6d578147", size = 87321, upload-time = "2025-08-11T12:57:51.923Z" }, -] - [[package]] name = "mccabe" version = "0.7.0" @@ -269,15 +255,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/27/1a/1f68f9ba0c207934b35b86a8ca3aad8395a3d6dd7921c0686e23853ff5a9/mccabe-0.7.0-py2.py3-none-any.whl", hash = "sha256:6c2d30ab6be0e4a46919781807b4f0d834ebdd6c6e3dca0bda5a15f863427b6e", size = 7350, upload-time = "2022-01-24T01:14:49.62Z" }, ] -[[package]] -name = "mdurl" -version = "0.1.2" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/d6/54/cfe61301667036ec958cb99bd3efefba235e65cdeb9c84d24a8293ba1d90/mdurl-0.1.2.tar.gz", hash = "sha256:bb413d29f5eea38f31dd4754dd7377d4465116fb207585f97bf925588687c1ba", size = 8729, upload-time = "2022-08-14T12:40:10.846Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/b3/38/89ba8ad64ae25be8de66a6d463314cf1eb366222074cfda9ee839c56a4b4/mdurl-0.1.2-py3-none-any.whl", hash = "sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8", size = 9979, upload-time = "2022-08-14T12:40:09.779Z" }, -] - [[package]] name = "nodeenv" version = "1.10.0" @@ -511,19 +488,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/0b/d7/1959b9648791274998a9c3526f6d0ec8fd2233e4d4acce81bbae76b44b2a/python_dotenv-1.2.2-py3-none-any.whl", hash = "sha256:1d8214789a24de455a8b8bd8ae6fe3c6b69a5e3d64aa8a8e5d68e694bbcb285a", size = 22101, upload-time = "2026-03-01T16:00:25.09Z" }, ] -[[package]] -name = "rich" -version = "14.3.3" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "markdown-it-py" }, - { name = "pygments" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/b3/c6/f3b320c27991c46f43ee9d856302c70dc2d0fb2dba4842ff739d5f46b393/rich-14.3.3.tar.gz", hash = "sha256:b8daa0b9e4eef54dd8cf7c86c03713f53241884e814f4e2f5fb342fe520f639b", size = 230582, upload-time = "2026-02-19T17:23:12.474Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/14/25/b208c5683343959b670dc001595f2f3737e051da617f66c31f7c4fa93abc/rich-14.3.3-py3-none-any.whl", hash = "sha256:793431c1f8619afa7d3b52b2cdec859562b950ea0d4b6b505397612db8d5362d", size = 310458, upload-time = "2026-02-19T17:23:13.732Z" }, -] - [[package]] name = "ruff" version = "0.15.10" From 7e05ab2969cdbdc76e1370c558cc153846da4849 Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Fri, 10 Apr 2026 22:46:10 +0100 Subject: [PATCH 2/8] Add rich spinner back for tool calls, keep rest as plain print MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rich is only used for Live/Spinner during tool execution — all other output stays as plain print/stdout.write. Co-Authored-By: Claude Opus 4.6 (1M context) --- pyproject.toml | 1 + src/cli_chat/orchestrator.py | 5 ++++- uv.lock | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index db8ac91..fc940ec 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,6 +9,7 @@ dependencies = [ "pydantic>=2.10.0", "pydantic-settings>=2.7.0", "python-dotenv>=1.0.0", + "rich>=13.9.0", "tenacity>=9.0.0", ] diff --git a/src/cli_chat/orchestrator.py b/src/cli_chat/orchestrator.py index f25e516..bd70329 100644 --- a/src/cli_chat/orchestrator.py +++ b/src/cli_chat/orchestrator.py @@ -12,6 +12,8 @@ from openai import AsyncOpenAI from openai.types.chat import ChatCompletionToolMessageParam, ChatCompletionUserMessageParam from openai.types.chat.chat_completion_message_tool_call import ChatCompletionMessageToolCall, Function +from rich.live import Live +from rich.spinner import Spinner from cli_chat.tools import TOOL_DEFINITIONS, ToolExecutor @@ -219,7 +221,8 @@ async def _execute_tools(self, tool_calls: list[ChatCompletionMessageToolCall]) args = json.loads(tc.function.arguments) print(f" > {tc.function.name}({args})") - result = await self._tools.execute(tc, self._cancel_event) + with Live(Spinner("dots", text=f"{tc.function.name}..."), transient=True): + result = await self._tools.execute(tc, self._cancel_event) if result.error: print(f" x {tc.function.name}: {result.content}") diff --git a/uv.lock b/uv.lock index 6006f65..f6fb99e 100644 --- a/uv.lock +++ b/uv.lock @@ -52,6 +52,7 @@ dependencies = [ { name = "pydantic" }, { name = "pydantic-settings" }, { name = "python-dotenv" }, + { name = "rich" }, { name = "tenacity" }, ] @@ -71,6 +72,7 @@ requires-dist = [ { name = "pydantic", specifier = ">=2.10.0" }, { name = "pydantic-settings", specifier = ">=2.7.0" }, { name = "python-dotenv", specifier = ">=1.0.0" }, + { name = "rich", specifier = ">=13.9.0" }, { name = "tenacity", specifier = ">=9.0.0" }, ] @@ -246,6 +248,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/da/e9/1f9ada30cef7b05e74bb06f52127e7a724976c225f46adb65c37b1dadfb6/jiter-0.14.0-graalpy312-graalpy250_312_native-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:67f00d94b281174144d6532a04b66a12cb866cbdc47c3af3bfe2973677f9861a", size = 349613, upload-time = "2026-04-10T14:28:40.066Z" }, ] +[[package]] +name = "markdown-it-py" +version = "4.0.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "mdurl" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/5b/f5/4ec618ed16cc4f8fb3b701563655a69816155e79e24a17b651541804721d/markdown_it_py-4.0.0.tar.gz", hash = "sha256:cb0a2b4aa34f932c007117b194e945bd74e0ec24133ceb5bac59009cda1cb9f3", size = 73070, upload-time = "2025-08-11T12:57:52.854Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/94/54/e7d793b573f298e1c9013b8c4dade17d481164aa517d1d7148619c2cedbf/markdown_it_py-4.0.0-py3-none-any.whl", hash = "sha256:87327c59b172c5011896038353a81343b6754500a08cd7a4973bb48c6d578147", size = 87321, upload-time = "2025-08-11T12:57:51.923Z" }, +] + [[package]] name = "mccabe" version = "0.7.0" @@ -255,6 +269,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/27/1a/1f68f9ba0c207934b35b86a8ca3aad8395a3d6dd7921c0686e23853ff5a9/mccabe-0.7.0-py2.py3-none-any.whl", hash = "sha256:6c2d30ab6be0e4a46919781807b4f0d834ebdd6c6e3dca0bda5a15f863427b6e", size = 7350, upload-time = "2022-01-24T01:14:49.62Z" }, ] +[[package]] +name = "mdurl" +version = "0.1.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d6/54/cfe61301667036ec958cb99bd3efefba235e65cdeb9c84d24a8293ba1d90/mdurl-0.1.2.tar.gz", hash = "sha256:bb413d29f5eea38f31dd4754dd7377d4465116fb207585f97bf925588687c1ba", size = 8729, upload-time = "2022-08-14T12:40:10.846Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b3/38/89ba8ad64ae25be8de66a6d463314cf1eb366222074cfda9ee839c56a4b4/mdurl-0.1.2-py3-none-any.whl", hash = "sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8", size = 9979, upload-time = "2022-08-14T12:40:09.779Z" }, +] + [[package]] name = "nodeenv" version = "1.10.0" @@ -488,6 +511,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/0b/d7/1959b9648791274998a9c3526f6d0ec8fd2233e4d4acce81bbae76b44b2a/python_dotenv-1.2.2-py3-none-any.whl", hash = "sha256:1d8214789a24de455a8b8bd8ae6fe3c6b69a5e3d64aa8a8e5d68e694bbcb285a", size = 22101, upload-time = "2026-03-01T16:00:25.09Z" }, ] +[[package]] +name = "rich" +version = "14.3.3" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "markdown-it-py" }, + { name = "pygments" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b3/c6/f3b320c27991c46f43ee9d856302c70dc2d0fb2dba4842ff739d5f46b393/rich-14.3.3.tar.gz", hash = "sha256:b8daa0b9e4eef54dd8cf7c86c03713f53241884e814f4e2f5fb342fe520f639b", size = 230582, upload-time = "2026-02-19T17:23:12.474Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/14/25/b208c5683343959b670dc001595f2f3737e051da617f66c31f7c4fa93abc/rich-14.3.3-py3-none-any.whl", hash = "sha256:793431c1f8619afa7d3b52b2cdec859562b950ea0d4b6b505397612db8d5362d", size = 310458, upload-time = "2026-02-19T17:23:13.732Z" }, +] + [[package]] name = "ruff" version = "0.15.10" From 666e6f45e437f8d6d2367d478fb71d199237d7e0 Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Sat, 11 Apr 2026 11:45:27 +0100 Subject: [PATCH 3/8] Compact codebase to 2 modules, drop pydantic/tenacity deps Merge main.py and models.py into orchestrator.py and tools.py, replacing Pydantic models with plain dicts and tenacity retry with a simple loop. Removes 3 dependencies (pydantic, pydantic-settings, tenacity). Updates tests and docs to match the new interface. Co-Authored-By: Claude Opus 4.6 (1M context) --- ARCHITECTURE.md | 105 +++----- CLAUDE.md | 12 +- README.md | 10 +- pyproject.toml | 5 +- src/cli_chat/main.py | 57 ----- src/cli_chat/models.py | 82 ------- src/cli_chat/orchestrator.py | 449 +++++++++++++++++------------------ src/cli_chat/tools.py | 233 ++++++++---------- tests/test_e2e.py | 203 +++++++--------- uv.lock | 29 --- 10 files changed, 450 insertions(+), 735 deletions(-) delete mode 100644 src/cli_chat/main.py delete mode 100644 src/cli_chat/models.py diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a0b28cf..303cf61 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -2,60 +2,48 @@ ## Overview -The application follows an **orchestrator pattern** where a central coordinator manages the turn lifecycle, delegating to stateless workers for LLM interaction and tool execution. +The application is structured as two modules: `orchestrator.py` handles the conversation loop, LLM streaming, and cancellation; `tools.py` handles API calls, retry logic, and response formatting. ```mermaid graph TD - A[main.py] -->|creates, configures logging, wires signals| B[Orchestrator] - B -->|streams LLM requests| C[ChatClient] - B -->|dispatches tool calls| D[ToolExecutor] - B -->|renders output| E[Display] - C -->|OpenAI SDK| F[OpenRouter API] - D -->|httpx async + tenacity retry| G[Elyos Weather API] - D -->|httpx async + tenacity retry| H[Elyos Research API] + A[orchestrator.py] -->|entry point, turn loop, streaming| B[AsyncOpenAI] + A -->|dispatches tool calls| C[ToolExecutor] + B -->|OpenAI SDK| D[OpenRouter API] + C -->|httpx async + retry loop| E[Elyos Weather API] + C -->|httpx async + retry loop| F[Elyos Research API] ``` ## Module Responsibilities -| Module | Role | Stateful? | -| ---------------- | ------------------------------------------------------- | --------- | -| `main.py` | Entry point, logging config, asyncio loop, SIGINT wiring | No | -| `orchestrator.py`| Turn lifecycle, conversation history, cancellation | Yes | -| `chat.py` | LLM streaming via OpenRouter | No | -| `tools.py` | API calls, tenacity retry, cancellable requests | No | -| `models.py` | Pydantic models for API responses, settings, tool results| No | -| `display.py` | Themed output (rich), spinners, tool indicators | No | +| Module | Role | Stateful? | +| ---------------- | ----------------------------------------------------------- | --------- | +| `orchestrator.py`| Entry point, logging, turn lifecycle, LLM streaming, cancel | Yes | +| `tools.py` | API calls, retry loop, cancellable requests, formatting | No | ## Turn Lifecycle ```mermaid sequenceDiagram participant U as User - participant O as Orchestrator - participant C as ChatClient + participant O as _process_turn + participant S as _stream_response participant T as ToolExecutor - participant D as Display U->>O: input text - O->>C: stream(history) + O->>S: stream(history) loop streaming chunks - C-->>O: content delta / tool_call delta - O->>D: print_streaming_token() + S-->>O: content delta / tool_call delta + O-->>U: sys.stdout.write(token) end alt tool calls detected - O->>D: print_tool_call() - O->>D: tool_spinner() O->>T: execute(tool_call, cancel_event) - T-->>O: ToolResult - O->>D: print_tool_result_ok/error() - O->>C: stream(history + tool results) + T-->>O: result dict + O->>S: stream(history + tool results) loop streaming final response - O->>D: print_assistant_header() - C-->>O: content delta - O->>D: print_streaming_token() + S-->>O: content delta + O-->>U: sys.stdout.write(token) end end - O->>D: finish_streaming() ``` ## Cancellation Flow @@ -75,7 +63,7 @@ The signal handler is context-dependent: - **During input**: No custom SIGINT handler — `loop.add_reader(stdin)` races against `cancel_event.wait()`. Ctrl+C sets the cancel event, `_read_input()` returns `None`, and the app exits. - **During processing**: Custom handler is installed. 1st Ctrl+C sets `cancel_event`; 2nd sets `should_exit`. -- **During HTTP requests**: `_cancellable_request()` races the httpx coroutine against `cancel_event` via `asyncio.wait(FIRST_COMPLETED)`, so cancellation is instant even during slow API calls. +- **During HTTP requests**: `_race_with_cancel()` races the httpx coroutine against `cancel_event` via `asyncio.wait(FIRST_COMPLETED)`, so cancellation is instant even during slow API calls. - **On cleanup**: Signal handler is removed before `asyncio.run()` shutdown to avoid stale handlers. The cancel event is cleared at the start of each new turn. @@ -93,58 +81,41 @@ Using `asyncio.to_thread(input)` spawns a thread that blocks on `input()`. When ```mermaid flowchart TD REQ[API Request] --> CHECK{Response throttled?} - CHECK -->|No| PARSE[Parse response] + CHECK -->|No| PARSE[Parse + format response] CHECK -->|Yes| RETRY{Attempts < 3?} RETRY -->|Yes| WAIT[Wait retry_after_seconds] --> REQ - RETRY -->|No| ERR[_RateLimitError] + RETRY -->|No| ERR[RuntimeError] ``` -Retry is handled declaratively via a tenacity `@_throttle_retry` decorator: +Retry is handled by `_request_with_retry`, a simple loop: -- **Trigger**: `_ThrottledError` raised when API returns `status: "throttled"` -- **Wait**: Dynamic — reads `retry_after_seconds` from the throttled response (capped at 15s) +- **Trigger**: API returns `status: "throttled"` in JSON (HTTP 200) +- **Wait**: Reads `retry_after_seconds` from the throttled response (capped at 15s) - **Stop**: After 3 attempts -- **On exhaustion**: `retry_error_callback` converts to `_RateLimitError` -- **Logging**: `before_sleep` callback logs each retry with attempt count and wait time - -Both `_get_weather` and `_research_topic` use the same decorator. The method bodies contain only the happy path + throttle guard. +- **On exhaustion**: Raises `RuntimeError` with retry guidance +- **Cancellation**: Each wait is cancellable via `_wait_or_cancel` ## Data Flow ```mermaid flowchart LR subgraph API Responses - W1[Flat weather JSON] -->|from_api| WR[WeatherResponse] - W2[Array weather JSON] -->|from_api| WR - R1[Research JSON] --> RR[ResearchResponse] - T1[Throttled JSON] --> TE[_ThrottledError] + W1[Flat weather JSON] -->|_format_weather| S[String for LLM] + W2[Array weather JSON] -->|_format_weather| S + R1[Research JSON] -->|_format_research| S + T1[Throttled JSON] -->|_request_with_retry| W1 + T1 -->|_request_with_retry| R1 HTML[HTML error] --> DE[DecodingError] end - WR -->|display| S[String for LLM] - RR -->|display| S - TE -->|@_throttle_retry| W1 - TE -->|@_throttle_retry| R1 DE -->|error result| S ``` -## Display Theme - -| Element | Style | Usage | -| ---------- | ----------- | ---------------------------------------- | -| User | Bold cyan | `You:` prompt label | -| Assistant | Bold green | `Assistant:` header before streamed text | -| Tool call | Yellow | `⚡ tool_name(args)` indicator | -| Tool OK | Green | `✓ tool_name completed` | -| Tool error | Bold red | `✗ tool_name: message` | -| Separator | Dim | `────` rule between turns | -| Meta | Dim | `[cancelled]`, `Goodbye!` | - ## Key Design Decisions -1. **Orchestrator owns all state** — ChatClient and ToolExecutor are stateless workers. This makes the system easy to reason about and test. -2. **Cancel via asyncio.Event** — shared between orchestrator and tool executor, checked cooperatively. HTTP requests are raced against the event for instant cancellation. -3. **Pydantic normalization** — `WeatherResponse.from_api()` handles the non-deterministic API schemas at the boundary, so downstream code always sees a consistent model. -4. **Tenacity for retry** — `@_throttle_retry` decorator with custom wait strategy reading `retry_after_seconds` from the API response. Keeps method bodies clean. -5. **Content-type guard** — `_request()` checks for `application/json` before parsing, handling infrastructure-level HTML errors (e.g., unicode input → Cloud Run 400). +1. **Flat functions over classes** — `_process_turn`, `_stream_response`, `_execute_tools` are plain async functions. State is just the `history` list and `cancel_event`, threaded through arguments. +2. **Cancel via asyncio.Event** — shared across the call chain, checked cooperatively. HTTP requests are raced against the event for instant cancellation. +3. **Dict-based formatting** — `_format_weather()` handles both flat and array API schemas with dict access. No Pydantic models needed for two simple response shapes. +4. **Simple retry loop** — `_request_with_retry` reads `retry_after_seconds` from the throttled response, sleeps (cancellably), and retries. Three attempts, capped at 15s wait. +5. **Content-type guard** — `_request()` checks for `application/json` before parsing, handling infrastructure-level HTML errors (e.g., unicode input causing Cloud Run 400). 6. **History integrity on cancel** — stub tool results ensure the conversation history is always valid, preventing LLM 400 errors after interrupted tool calls. -7. **File-only logging** — comprehensive DEBUG-level logs to timestamped session files, with third-party loggers silenced to WARNING. No console noise. +7. **File-only logging** — DEBUG-level logs to timestamped session files, with third-party loggers silenced to WARNING. No console noise. diff --git a/CLAUDE.md b/CLAUDE.md index 0356d02..66a5276 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,7 +7,7 @@ Take-home interview project: CLI chat app with streaming LLM + tool calling agai ## Quick reference - `make install` → `make run` to use -- `make check` runs lint + test (21 e2e tests) +- `make check` runs lint + test (20 e2e tests) - API keys in `.env` (OPENROUTER_API_KEY, ELYOS_API_KEY, optional LLM_MODEL) - Logs: `cli_chat__.log` per session - Design docs: README.md, ARCHITECTURE.md, DISCOVERIES.md @@ -16,19 +16,19 @@ Take-home interview project: CLI chat app with streaming LLM + tool calling agai - OpenRouter (OpenAI-compatible) for LLM, model configurable via LLM_MODEL in .env - httpx async for external API calls, with `asyncio.wait` racing requests against cancel events -- Pydantic for all data models + settings -- Tenacity `@retry` decorator for throttle retry (custom wait from `retry_after_seconds`) -- Orchestrator pattern: separates turn lifecycle from chat/tool execution +- Plain dicts for API responses, formatted by `_format_weather` / `_format_research` helpers +- Simple retry loop for throttle handling (reads `retry_after_seconds`, max 3 attempts) +- Flat async functions: `_process_turn` → `_stream_response` → `_execute_tools` - Async stdin via `loop.add_reader` (not `asyncio.to_thread(input)`) — avoids dangling threads - Cancellation adds stub tool results to keep conversation history valid for the LLM -- Styled display via rich: cyan user, green assistant, yellow tools, red errors +- Rich spinner for tool call pending state - File-only logging (DEBUG level) with timestamped + UUID session files ## Ctrl+C behavior - During input: exits cleanly - During processing: first Ctrl+C cancels current operation, second exits -- During HTTP requests: instant cancellation via `_cancellable_request` (asyncio.wait race) +- During HTTP requests: instant cancellation via `_race_with_cancel` (asyncio.wait race) ## API quirks (see DISCOVERIES.md for details) diff --git a/README.md b/README.md index abe802b..c9011b6 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ make check # lint + test (CI gate) ## Design -The application uses an **orchestrator pattern** — a central coordinator manages the conversation turn lifecycle, delegating to stateless workers for LLM streaming and tool execution. See [ARCHITECTURE.md](ARCHITECTURE.md) for diagrams and detailed design rationale. +The application uses flat async functions to manage the conversation turn lifecycle, with a `ToolExecutor` class handling API calls. See [ARCHITECTURE.md](ARCHITECTURE.md) for diagrams and detailed design rationale. **API quirks** discovered during development are documented in [DISCOVERIES.md](DISCOVERIES.md). @@ -65,10 +65,6 @@ The application uses an **orchestrator pattern** — a central coordinator manag ``` src/cli_chat/ -├── main.py # entry point, signal wiring -├── orchestrator.py # turn lifecycle, history, cancellation -├── chat.py # LLM streaming via OpenRouter -├── tools.py # API calls with retry + quirk handling -├── models.py # pydantic models (settings, responses) -└── display.py # streaming output, spinners +├── orchestrator.py # entry point, turn lifecycle, LLM streaming, cancellation +└── tools.py # API calls with retry, response formatting ``` diff --git a/pyproject.toml b/pyproject.toml index fc940ec..bc7e9d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,15 +6,12 @@ requires-python = ">=3.12" dependencies = [ "openai>=1.60.0", "httpx>=0.27.0", - "pydantic>=2.10.0", - "pydantic-settings>=2.7.0", "python-dotenv>=1.0.0", "rich>=13.9.0", - "tenacity>=9.0.0", ] [project.scripts] -cli-chat = "cli_chat.main:main" +cli-chat = "cli_chat.orchestrator:main" [build-system] requires = ["hatchling"] diff --git a/src/cli_chat/main.py b/src/cli_chat/main.py deleted file mode 100644 index 8b89d4a..0000000 --- a/src/cli_chat/main.py +++ /dev/null @@ -1,57 +0,0 @@ -"""Entry point — CLI setup, logging configuration, and signal wiring.""" - -from __future__ import annotations - -import asyncio -import datetime -import logging -import signal -import uuid - -from cli_chat.models import Settings -from cli_chat.orchestrator import Orchestrator - -logger = logging.getLogger(__name__) - - -def _configure_logging() -> str: - timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") - session_id = uuid.uuid4().hex[:8] - log_file = f"cli_chat_{timestamp}_{session_id}.log" - logging.basicConfig( - filename=log_file, - level=logging.DEBUG, - format="%(asctime)s [%(levelname)s] %(name)s: %(message)s", - datefmt="%Y-%m-%d %H:%M:%S", - ) - for noisy in ("httpx", "openai", "httpcore"): - logging.getLogger(noisy).setLevel(logging.WARNING) - return log_file - - -async def _run() -> None: - settings = Settings() # type: ignore[call-arg] - log_file = _configure_logging() - logger.info("Session started (model=%s, log_file=%s)", settings.llm_model, log_file) - - orch = Orchestrator(settings) - loop = asyncio.get_running_loop() - loop.add_signal_handler(signal.SIGINT, orch.handle_interrupt) - - print("CLI Chat — type 'exit' to quit, Ctrl+C to cancel\n") - - try: - await orch.run() - finally: - loop.remove_signal_handler(signal.SIGINT) - await orch.close() - print("\nGoodbye!") - logger.info("Session ended") - - -def main() -> None: - asyncio.run(_run()) - - -if __name__ == "__main__": - main() diff --git a/src/cli_chat/models.py b/src/cli_chat/models.py deleted file mode 100644 index 98c8f61..0000000 --- a/src/cli_chat/models.py +++ /dev/null @@ -1,82 +0,0 @@ -"""Pydantic models for API responses and application state.""" - -from __future__ import annotations - -from pydantic import BaseModel, Field -from pydantic_settings import BaseSettings - - -class Settings(BaseSettings): - model_config = {"env_file": ".env", "extra": "ignore"} - - openrouter_api_key: str = Field(alias="OPENROUTER_API_KEY") - elyos_api_key: str = Field(alias="ELYOS_API_KEY") - elyos_base_url: str = "https://elyos-interview-907656039105.europe-west2.run.app" - llm_model: str = Field(default="openai/gpt-4o-mini", alias="LLM_MODEL") - - -class WeatherCondition(BaseModel): - temperature_c: float - condition: str - humidity: int | float - - -class WeatherResponse(BaseModel): - """Normalized weather response — always uses a list of conditions.""" - - location: str - conditions: list[WeatherCondition] - note: str | None = None - - @classmethod - def from_api(cls, data: dict) -> WeatherResponse: - """Handle both flat and array response shapes from the API.""" - if "conditions" in data: - return cls(**data) - return cls( - location=data["location"], - conditions=[ - WeatherCondition( - temperature_c=data["temperature_c"], condition=data["condition"], humidity=data["humidity"] - ) - ], - ) - - def display(self) -> str: - parts = [f"Weather in {self.location}:"] - for c in self.conditions: - parts.append(f" {c.condition}, {c.temperature_c}°C, {c.humidity}% humidity") - if self.note: - parts.append(f" Note: {self.note}") - return "\n".join(parts) - - -class ResearchResponse(BaseModel): - topic: str - summary: str - sources: list[str] = [] - generated_at: str | None = None - cached: bool = False - cache_age_seconds: int | None = None - - def display(self) -> str: - parts = [self.summary] - if self.sources: - parts.append(f"Sources: {', '.join(self.sources)}") - if self.cached and self.cache_age_seconds is not None: - parts.append(f"Note: cached result ({self.cache_age_seconds // 86400} days old)") - return "\n".join(parts) - - -class ThrottledResponse(BaseModel): - status: str # "throttled" - message: str - retry_after_seconds: int - data: None = None - - -class ToolResult(BaseModel): - tool_call_id: str - name: str - content: str - error: bool = False diff --git a/src/cli_chat/orchestrator.py b/src/cli_chat/orchestrator.py index 3fb1fe8..289caa6 100644 --- a/src/cli_chat/orchestrator.py +++ b/src/cli_chat/orchestrator.py @@ -1,261 +1,240 @@ -"""Orchestrator — manages the turn lifecycle, history, and cancellation.""" - from __future__ import annotations import asyncio import contextlib +import datetime import json import logging +import os +import signal import sys -from typing import TYPE_CHECKING +import uuid from openai import AsyncOpenAI -from openai.types.chat import ChatCompletionToolMessageParam, ChatCompletionUserMessageParam -from openai.types.chat.chat_completion_message_tool_call import ChatCompletionMessageToolCall, Function from rich.live import Live from rich.spinner import Spinner from cli_chat.tools import TOOL_DEFINITIONS, ToolExecutor -if TYPE_CHECKING: - from openai.types.chat import ChatCompletionMessageParam - - from cli_chat.models import Settings, ToolResult - -logger = logging.getLogger(__name__) - SYSTEM_PROMPT = "You are a helpful assistant. Be concise and helpful." +OPENROUTER_BASE_URL = "https://openrouter.ai/api/v1" +ELYOS_BASE_URL = "https://elyos-interview-907656039105.europe-west2.run.app" +DEFAULT_MODEL = "openai/gpt-4o-mini" +logger = logging.getLogger(__name__) -class Orchestrator: - def __init__(self, settings: Settings) -> None: - self._client = AsyncOpenAI(api_key=settings.openrouter_api_key, base_url="https://openrouter.ai/api/v1") - self._model = settings.llm_model - self._tools = ToolExecutor(settings) - self._history: list[ChatCompletionMessageParam] = [] - self._cancel_event = asyncio.Event() - self._should_exit = False - self._turn_count = 0 - - async def close(self) -> None: - logger.info("Orchestrator closing (turns=%d, history_len=%d)", self._turn_count, len(self._history)) - await self._tools.close() +def _configure_logging() -> str: + timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") + session_id = uuid.uuid4().hex[:8] + log_file = f"cli_chat_{timestamp}_{session_id}.log" + logging.basicConfig( + filename=log_file, + level=logging.DEBUG, + format="%(asctime)s [%(levelname)s] %(name)s: %(message)s", + datefmt="%Y-%m-%d %H:%M:%S", + ) + for noisy in ("httpx", "openai", "httpcore"): + logging.getLogger(noisy).setLevel(logging.WARNING) + return log_file + +def _load_config() -> tuple[str, str, str, str]: + return ( + os.environ["OPENROUTER_API_KEY"], + os.environ["ELYOS_API_KEY"], + os.getenv("LLM_MODEL", DEFAULT_MODEL), + os.getenv("ELYOS_BASE_URL", ELYOS_BASE_URL), + ) + +async def _read_input(cancel_event: asyncio.Event) -> str | None: + loop = asyncio.get_running_loop() + print(flush=True) + sys.stdout.write("You: ") + sys.stdout.flush() + line_future: asyncio.Future[str] = loop.create_future() + + def _on_stdin_ready() -> None: + if not line_future.done(): + line_future.set_result(sys.stdin.readline()) + loop.add_reader(sys.stdin.fileno(), _on_stdin_ready) + cancel_task = asyncio.create_task(cancel_event.wait()) + try: + await asyncio.wait({line_future, cancel_task}, return_when=asyncio.FIRST_COMPLETED) + finally: + loop.remove_reader(sys.stdin.fileno()) + cancel_task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await cancel_task + if not line_future.done(): + return None + line = line_future.result() + return line.rstrip("\n") if line else None + +async def _stream_response( # pylint: disable=too-many-branches + client: AsyncOpenAI, + model: str, + history: list[dict], + cancel_event: asyncio.Event, +) -> tuple[str, list[dict]] | None: + logger.info("LLM stream request (model=%s, messages=%d)", model, len(history)) + try: + messages = [{"role": "system", "content": SYSTEM_PROMPT}, *history] + stream = await client.chat.completions.create( # pyright: ignore[reportCallIssue, reportArgumentType] + model=model, messages=messages, tools=TOOL_DEFINITIONS, stream=True, # pyright: ignore[reportArgumentType] + ) + except Exception as exc: # pylint: disable=broad-exception-caught + logger.error("LLM stream creation failed: %s", exc, exc_info=True) + print(f"Error: LLM error: {exc}", file=sys.stderr) + return None + content = "" + tool_calls_by_index: dict[int, dict] = {} + header_printed = False + try: + async for chunk in stream: + if cancel_event.is_set(): + logger.info("Stream cancelled by user (content so far: %d chars)", len(content)) + await stream.close() + if content: + sys.stdout.write("\n[cancelled]\n") + else: + print("[cancelled]") + return content, [] + delta = chunk.choices[0].delta if chunk.choices else None + if delta is None: + continue + if delta.content: + if not header_printed: + print("\nAssistant:") + header_printed = True + content += delta.content + sys.stdout.write(delta.content) + sys.stdout.flush() - def handle_interrupt(self) -> None: - """Called by signal handler. First call cancels; second exits.""" - if self._cancel_event.is_set(): - logger.info("SIGINT: second interrupt, requesting exit") - self._should_exit = True + if delta.tool_calls: + for tc_delta in delta.tool_calls: + index = tc_delta.index + entry = tool_calls_by_index.setdefault( + index, + {"id": tc_delta.id or "", "type": "function", "function": {"name": "", "arguments": ""}}, + ) + if tc_delta.id: + entry["id"] = tc_delta.id + if tc_delta.function: + if tc_delta.function.name: + entry["function"]["name"] = tc_delta.function.name + if tc_delta.function.arguments: + entry["function"]["arguments"] += tc_delta.function.arguments + except Exception as exc: # pylint: disable=broad-exception-caught + logger.error("Stream error: %s", exc, exc_info=True) + print(f"\nError: Stream error: {exc}", file=sys.stderr) + return None + logger.debug("Stream completed: %d chars content, %d tool calls", len(content), len(tool_calls_by_index)) + return content, list(tool_calls_by_index.values()) + +async def _execute_tools( + tools: ToolExecutor, + tool_calls: list[dict], + cancel_event: asyncio.Event, +) -> list[dict] | None: + results = [] + for tool_call in tool_calls: + if cancel_event.is_set(): + print("[cancelled]") + return None + name = tool_call["function"]["name"] + args = {} + with contextlib.suppress(json.JSONDecodeError): + args = json.loads(tool_call["function"]["arguments"] or "{}") + if name == "research_topic": + topic = args.get("topic", "").strip() or "topic" + print(f"Researching {topic}... (Ctrl+C to cancel)") else: - logger.info("SIGINT: cancelling current operation") - self._cancel_event.set() + print(f"Calling {name}({args})") + with Live(Spinner("dots", text=f"{name}..."), transient=True): + result = await tools.execute(tool_call, cancel_event) + if result["error"]: + print(f"{name} failed: {result['content']}") + results.append(result) + return results + +async def _process_turn( # pylint: disable=too-many-arguments,too-many-positional-arguments + client: AsyncOpenAI, + model: str, + tools: ToolExecutor, + history: list[dict], + user_input: str, + cancel_event: asyncio.Event, +) -> None: + logger.info("User input: %s", user_input) + history.append({"role": "user", "content": user_input}) + while True: + result = await _stream_response(client, model, history, cancel_event) + if result is None: + return + content, tool_calls = result + if cancel_event.is_set(): + if content: + logger.info("Saved partial response (%d chars)", len(content)) + history.append({"role": "assistant", "content": content + "\n[cancelled]"}) + return + assistant_message: dict = {"role": "assistant"} + if content: + assistant_message["content"] = content + if tool_calls: + assistant_message["tool_calls"] = tool_calls # pyright: ignore[reportArgumentType] + history.append(assistant_message) + if not tool_calls: + sys.stdout.write("\n") + sys.stdout.flush() + return + tool_results = await _execute_tools(tools, tool_calls, cancel_event) + if tool_results is None: + for tool_call in tool_calls: + history.append( + {"role": "tool", "tool_call_id": tool_call["id"], "content": "[cancelled by user]"} + ) + return + for result in tool_results: + history.append( + {"role": "tool", "tool_call_id": result["tool_call_id"], "content": result["content"]} + ) - async def run(self) -> None: - """Main input loop.""" - while not self._should_exit: - self._cancel_event.clear() - user_input = await self._read_input() +async def run_chat() -> None: + openrouter_api_key, elyos_api_key, model, elyos_base_url = _load_config() + log_file = _configure_logging() + client = AsyncOpenAI(api_key=openrouter_api_key, base_url=OPENROUTER_BASE_URL) + tools = ToolExecutor(elyos_base_url, elyos_api_key) + history: list[dict] = [] + cancel_event = asyncio.Event() + state = {"should_exit": False} + def _handle_interrupt() -> None: + if cancel_event.is_set(): + state["should_exit"] = True + else: + cancel_event.set() + loop = asyncio.get_running_loop() + loop.add_signal_handler(signal.SIGINT, _handle_interrupt) + logger.info("Session started (model=%s, log_file=%s)", model, log_file) + print("CLI Chat — type 'exit' to quit, Ctrl+C to cancel\n") + try: + while not state["should_exit"]: + cancel_event.clear() + user_input = await _read_input(cancel_event) if user_input is None: - logger.info("Input cancelled or EOF, exiting loop") break - user_input = user_input.strip() if not user_input: continue - if user_input.lower() in ("exit", "quit"): - logger.info("User requested exit via '%s'", user_input) + if user_input.lower() in {"exit", "quit"}: break - - self._turn_count += 1 - logger.info("Turn %d: user input: %s", self._turn_count, user_input) - await self._process_turn(user_input) - - async def _read_input(self) -> str | None: - """Read from stdin without threads, cancellable via Ctrl+C.""" - loop = asyncio.get_running_loop() - print(flush=True) - sys.stdout.write("You: ") - sys.stdout.flush() - - line_future: asyncio.Future[str] = loop.create_future() - - def _on_stdin_ready() -> None: - if not line_future.done(): - line_future.set_result(sys.stdin.readline()) - - fd = sys.stdin.fileno() - loop.add_reader(fd, _on_stdin_ready) - cancel_task = asyncio.create_task(self._cancel_event.wait()) - - try: - await asyncio.wait({asyncio.ensure_future(line_future), cancel_task}, return_when=asyncio.FIRST_COMPLETED) - finally: - loop.remove_reader(fd) - if not cancel_task.done(): - cancel_task.cancel() - with contextlib.suppress(asyncio.CancelledError): - await cancel_task - - if not line_future.done(): - return None - line = line_future.result() - return line.rstrip("\n") if line else None - - async def _process_turn(self, user_input: str) -> None: - self._history.append(ChatCompletionUserMessageParam(role="user", content=user_input)) - - while not self._should_exit: - result = await self._stream_response() - if result is None: - break - - content, tool_calls = result - - # Cancelled mid-stream: save partial content to history and stop - if self._cancel_event.is_set(): - if content: - self._append_assistant_message(content + "\n[cancelled]", []) - logger.info("Turn %d: saved partial response (%d chars)", self._turn_count, len(content)) - break - - self._append_assistant_message(content, tool_calls) - - if not tool_calls: - sys.stdout.write("\n") - sys.stdout.flush() - logger.info("Turn %d: assistant responded (%d chars)", self._turn_count, len(content)) - break - - logger.info( - "Turn %d: LLM requested %d tool call(s): %s", - self._turn_count, - len(tool_calls), - [tc.function.name for tc in tool_calls], - ) - - tool_results = await self._execute_tools(tool_calls) - if tool_results is None: - # Cancelled: add stub tool results so history stays valid for the LLM - for tc in tool_calls: - self._history.append( - ChatCompletionToolMessageParam( - role="tool", tool_call_id=tc.id, content="[cancelled by user]" - ) - ) - break - - for tr in tool_results: - self._history.append( - ChatCompletionToolMessageParam(role="tool", tool_call_id=tr.tool_call_id, content=tr.content) - ) - - async def _stream_response(self) -> tuple[str, list[ChatCompletionMessageToolCall]] | None: # pylint: disable=too-many-branches - try: - logger.info("LLM stream request (model=%s, messages=%d)", self._model, len(self._history)) - stream = await self._client.chat.completions.create( - model=self._model, - messages=[{"role": "system", "content": SYSTEM_PROMPT}, *self._history], - tools=TOOL_DEFINITIONS, # type: ignore[arg-type] - stream=True, - ) - except Exception as exc: # pylint: disable=broad-exception-caught - logger.error("LLM stream creation failed: %s", exc, exc_info=True) - print(f"Error: LLM error: {exc}", file=sys.stderr) - return None - - content = "" - tool_calls_by_index: dict[int, dict] = {} - header_printed = False - - try: - async for chunk in stream: - if self._cancel_event.is_set(): - logger.info("Stream cancelled by user (content so far: %d chars)", len(content)) - await stream.close() - if content: - finish_streaming() - print_dim("[cancelled]") - else: - print_dim("\n[cancelled]") - # Return partial content so it can be saved to history - return content, [] - - delta = chunk.choices[0].delta if chunk.choices else None - if delta is None: - continue - - if delta.content: - if not header_printed: - print("\nAssistant:") - header_printed = True - content += delta.content - sys.stdout.write(delta.content) - sys.stdout.flush() - - if delta.tool_calls: - for tc_delta in delta.tool_calls: - idx = tc_delta.index - if idx not in tool_calls_by_index: - tool_calls_by_index[idx] = {"id": tc_delta.id or "", "name": "", "arguments": ""} - entry = tool_calls_by_index[idx] - if tc_delta.id: - entry["id"] = tc_delta.id - if tc_delta.function: - if tc_delta.function.name: - entry["name"] = tc_delta.function.name - if tc_delta.function.arguments: - entry["arguments"] += tc_delta.function.arguments - - except Exception as exc: # pylint: disable=broad-exception-caught - logger.error("Stream error: %s", exc, exc_info=True) - print(f"\nError: Stream error: {exc}", file=sys.stderr) - return None - - logger.debug("Stream completed: %d chars content, %d tool calls", len(content), len(tool_calls_by_index)) - - tool_calls = [ - ChatCompletionMessageToolCall( - id=tc["id"], type="function", function=Function(name=tc["name"], arguments=tc["arguments"]) - ) - for tc in tool_calls_by_index.values() - ] - return content, tool_calls - - async def _execute_tools(self, tool_calls: list[ChatCompletionMessageToolCall]) -> list[ToolResult] | None: - results: list[ToolResult] = [] - for tc in tool_calls: - if self._cancel_event.is_set(): - logger.info("Tool execution cancelled before %s", tc.function.name) - print("[cancelled]") - return None - - args = {} - with contextlib.suppress(json.JSONDecodeError): - args = json.loads(tc.function.arguments) - - print(f" > {tc.function.name}({args})") - with Live(Spinner("dots", text=f"{tc.function.name}..."), transient=True): - result = await self._tools.execute(tc, self._cancel_event) - - if result.error: - print(f" x {tc.function.name}: {result.content}") - else: - print(f" OK {tc.function.name}") - results.append(result) - - return results - - def _append_assistant_message(self, content: str, tool_calls: list[ChatCompletionMessageToolCall]) -> None: - msg: dict = {"role": "assistant"} - if content: - msg["content"] = content - if tool_calls: - msg["tool_calls"] = [ - { - "id": tc.id, - "type": "function", - "function": {"name": tc.function.name, "arguments": tc.function.arguments}, - } - for tc in tool_calls - ] - self._history.append(msg) # type: ignore[arg-type] + await _process_turn(client, model, tools, history, user_input, cancel_event) + finally: + loop.remove_signal_handler(signal.SIGINT) + await tools.close() + print("\nGoodbye!") + logger.info("Session ended") + +def main() -> None: + asyncio.run(run_chat()) + +if __name__ == "__main__": + main() diff --git a/src/cli_chat/tools.py b/src/cli_chat/tools.py index f5a7d44..85df741 100644 --- a/src/cli_chat/tools.py +++ b/src/cli_chat/tools.py @@ -1,21 +1,11 @@ -"""Tool executor — weather and research API calls with retry and quirk handling.""" - from __future__ import annotations import asyncio import contextlib import json import logging -import typing -from typing import TYPE_CHECKING import httpx -from tenacity import RetryCallState, retry, retry_if_exception_type, stop_after_attempt - -from cli_chat import models - -if TYPE_CHECKING: - from openai.types.chat import chat_completion_message_tool_call as tc_module logger = logging.getLogger(__name__) @@ -46,88 +36,59 @@ }, ] -MAX_RETRIES = 3 REQUEST_TIMEOUT = 15.0 +MAX_RETRIES = 3 MAX_THROTTLE_WAIT = 15 - -# ── Retry infrastructure ───────────────────────────────────────────────────────────────────────── - - -class _ThrottledError(Exception): - """Raised when the API returns a throttled response (HTTP 200).""" - - def __init__(self, retry_after: int, endpoint: str) -> None: - super().__init__(f"{endpoint} throttled, retry after {retry_after}s") - self.retry_after = retry_after - self.endpoint = endpoint - - -class _RateLimitError(Exception): - """Raised when retries are exhausted due to API rate limiting.""" - - -def _throttle_wait(retry_state: RetryCallState) -> float: - exc = retry_state.outcome.exception() # type: ignore[union-attr] - return min(exc.retry_after, MAX_THROTTLE_WAIT) if isinstance(exc, _ThrottledError) else 1 - - -def _log_before_retry(retry_state: RetryCallState) -> None: - exc = retry_state.outcome.exception() # type: ignore[union-attr] - if isinstance(exc, _ThrottledError): - logger.warning( - "%s throttled (attempt %d/%d), retrying in %ds", - exc.endpoint, - retry_state.attempt_number, - MAX_RETRIES, - min(exc.retry_after, MAX_THROTTLE_WAIT), +def _format_weather(data: dict) -> str: + conditions = data.get("conditions") + if not conditions: + conditions = [ + { + "temperature_c": data["temperature_c"], + "condition": data["condition"], + "humidity": data["humidity"], + } + ] + + parts = [f"Weather in {data['location']}:"] + for condition in conditions: + parts.append( + f" {condition['condition']}, {condition['temperature_c']}°C, {condition['humidity']}% humidity" ) - - -def _on_retries_exhausted(retry_state: RetryCallState) -> typing.NoReturn: - exc = retry_state.outcome.exception() # type: ignore[union-attr] - if isinstance(exc, _ThrottledError): - raise _RateLimitError(f"{exc.endpoint} API is rate-limited. Please try again in {exc.retry_after}s.") from exc - raise _RateLimitError("Request failed after retries.") - - -_throttle_retry = retry( - retry=retry_if_exception_type(_ThrottledError), - wait=_throttle_wait, # type: ignore[arg-type] - stop=stop_after_attempt(MAX_RETRIES), - before_sleep=_log_before_retry, # type: ignore[arg-type] - retry_error_callback=_on_retries_exhausted, # type: ignore[arg-type] -) - + if data.get("note"): + parts.append(f" Note: {data['note']}") + return "\n".join(parts) + +def _format_research(data: dict) -> str: + parts = [data.get("summary", "No research summary returned.")] + if data.get("sources"): + parts.append(f"Sources: {', '.join(data['sources'])}") + if data.get("cached") and data.get("cache_age_seconds") is not None: + days_old = data["cache_age_seconds"] // 86400 + parts.append(f"Note: cached result ({days_old} days old)") + return "\n".join(parts) class ToolExecutor: - def __init__(self, settings: models.Settings) -> None: - self._settings = settings + def __init__(self, base_url: str, api_key: str) -> None: self._client = httpx.AsyncClient( - base_url=settings.elyos_base_url, - headers={"X-API-Key": settings.elyos_api_key}, + base_url=base_url, + headers={"X-API-Key": api_key}, timeout=REQUEST_TIMEOUT, ) async def close(self) -> None: await self._client.aclose() - async def execute( - self, - tool_call: tc_module.ChatCompletionMessageToolCall, - cancel_event: asyncio.Event | None = None, - ) -> models.ToolResult: - name = tool_call.function.name + async def execute(self, tool_call: dict, cancel_event: asyncio.Event) -> dict: + name = tool_call["function"]["name"] try: - args = json.loads(tool_call.function.arguments) + args = json.loads(tool_call["function"]["arguments"] or "{}") except json.JSONDecodeError: - logger.error("Invalid JSON arguments for tool %s: %s", name, tool_call.function.arguments) - return models.ToolResult( - tool_call_id=tool_call.id, name=name, content="Error: invalid tool arguments", error=True - ) + logger.error("Invalid JSON arguments for tool %s: %s", name, tool_call["function"]["arguments"]) + return {"tool_call_id": tool_call["id"], "content": "Error: invalid tool arguments", "error": True} logger.info("Tool call: %s(%s)", name, args) - try: if name == "get_weather": content = await self._get_weather(args.get("location", ""), cancel_event) @@ -136,78 +97,76 @@ async def execute( else: logger.warning("Unknown tool requested: %s", name) content = f"Unknown tool: {name}" - logger.info("Tool %s completed successfully", name) + logger.info("Tool %s completed", name) logger.debug("Tool %s result: %s", name, content[:200]) - return models.ToolResult(tool_call_id=tool_call.id, name=name, content=content) + return {"tool_call_id": tool_call["id"], "content": content, "error": False} except asyncio.CancelledError: logger.warning("Tool %s cancelled by user", name) - return models.ToolResult( - tool_call_id=tool_call.id, name=name, content="Tool call was cancelled by the user.", error=True - ) + return {"tool_call_id": tool_call["id"], "content": "Tool call was cancelled by the user.", "error": True} except httpx.HTTPStatusError as exc: logger.error("Tool %s HTTP error %d: %s", name, exc.response.status_code, exc.response.text[:200]) - return models.ToolResult( - tool_call_id=tool_call.id, - name=name, - content=f"API error ({exc.response.status_code}): {exc.response.text}", - error=True, - ) - except (httpx.RequestError, httpx.TimeoutException, httpx.DecodingError) as exc: + msg = f"API error ({exc.response.status_code}): {exc.response.text}" + return {"tool_call_id": tool_call["id"], "content": msg, "error": True} + except (httpx.RequestError, httpx.TimeoutException, httpx.DecodingError, RuntimeError) as exc: logger.error("Tool %s request failed: %s", name, exc) - return models.ToolResult(tool_call_id=tool_call.id, name=name, content=f"Request failed: {exc}", error=True) - except _RateLimitError as exc: - logger.warning("Tool %s rate-limited after %d retries: %s", name, MAX_RETRIES, exc) - return models.ToolResult(tool_call_id=tool_call.id, name=name, content=str(exc), error=True) - - @_throttle_retry - async def _get_weather(self, location: str, cancel_event: asyncio.Event | None) -> str: - resp = await self._request("/weather", {"location": location}, cancel_event) - if resp.get("status") == "throttled": - raise _ThrottledError(models.ThrottledResponse(**resp).retry_after_seconds, "Weather") - weather = models.WeatherResponse.from_api(resp) - logger.debug("Weather response format: %s", "array" if "conditions" in resp else "flat") - return weather.display() - - @_throttle_retry - async def _research_topic(self, topic: str, cancel_event: asyncio.Event | None) -> str: - resp = await self._request("/research", {"topic": topic}, cancel_event) - if resp.get("status") == "throttled": - raise _ThrottledError(models.ThrottledResponse(**resp).retry_after_seconds, "Research") - research = models.ResearchResponse(**resp) - if research.cached: - logger.info("Research returned cached result (age=%ds)", research.cache_age_seconds or 0) - return research.display() - - async def _request(self, path: str, params: dict, cancel_event: asyncio.Event | None) -> dict: - if cancel_event and cancel_event.is_set(): + return {"tool_call_id": tool_call["id"], "content": f"Request failed: {exc}", "error": True} + + async def _get_weather(self, location: str, cancel_event: asyncio.Event) -> str: + data = await self._request_with_retry("Weather", "/weather", {"location": location}, cancel_event) + return _format_weather(data) + + async def _research_topic(self, topic: str, cancel_event: asyncio.Event) -> str: + data = await self._request_with_retry("Research", "/research", {"topic": topic}, cancel_event) + return _format_research(data) + + async def _request_with_retry( + self, + label: str, + path: str, + params: dict, + cancel_event: asyncio.Event, + ) -> dict: + retry_after = 1 + for attempt in range(MAX_RETRIES): + data = await self._request(path, params, cancel_event) + if data.get("status") != "throttled": + return data + + retry_after = min(int(data.get("retry_after_seconds", 1)), MAX_THROTTLE_WAIT) + logger.warning("%s throttled (attempt %d/%d), retry in %ds", label, attempt + 1, MAX_RETRIES, retry_after) + if attempt < MAX_RETRIES - 1: + await _wait_or_cancel(retry_after, cancel_event) + + raise RuntimeError(f"{label} API is rate-limited. Please try again in {retry_after}s.") + async def _request(self, path: str, params: dict, cancel_event: asyncio.Event) -> dict: + if cancel_event.is_set(): raise asyncio.CancelledError logger.debug("API request: GET %s params=%s", path, params) - request_coro = self._client.get(path, params=params) - resp = await (self._cancellable_request(request_coro, cancel_event) if cancel_event else request_coro) - - logger.debug("API response: %d %s", resp.status_code, resp.headers.get("content-type", "")) - resp.raise_for_status() + response = await _race_with_cancel(self._client.get(path, params=params), cancel_event) + logger.debug("API response: %d %s", response.status_code, response.headers.get("content-type", "")) + response.raise_for_status() - content_type = resp.headers.get("content-type", "") + content_type = response.headers.get("content-type", "") if "json" not in content_type: raise httpx.DecodingError(f"Unexpected response format (got {content_type})") - return resp.json() - - @staticmethod - async def _cancellable_request( - request_coro: typing.Coroutine[typing.Any, typing.Any, httpx.Response], - cancel_event: asyncio.Event, - ) -> httpx.Response: - """Race an HTTP request against a cancel event.""" - request_task = asyncio.create_task(request_coro) - cancel_task = asyncio.create_task(cancel_event.wait()) - done, pending = await asyncio.wait({request_task, cancel_task}, return_when=asyncio.FIRST_COMPLETED) - for task in pending: - task.cancel() - with contextlib.suppress(asyncio.CancelledError): - await task - if cancel_task in done: - raise asyncio.CancelledError - return request_task.result() + return response.json() + +async def _wait_or_cancel(delay: float, cancel_event: asyncio.Event) -> None: + await _race_with_cancel(asyncio.sleep(delay), cancel_event) + +async def _race_with_cancel(awaitable, cancel_event: asyncio.Event): + work_task = asyncio.create_task(awaitable) + cancel_task = asyncio.create_task(cancel_event.wait()) + done, pending = await asyncio.wait({work_task, cancel_task}, return_when=asyncio.FIRST_COMPLETED) + for task in pending: + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + if cancel_task in done: + work_task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await work_task + raise asyncio.CancelledError + return work_task.result() diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 2711588..1509f8a 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -3,46 +3,43 @@ from __future__ import annotations import asyncio +import os import pytest -from cli_chat import models from cli_chat import tools as tools_module +from cli_chat.tools import _format_research, _format_weather # ── Fixtures ────────────────────────────────────────────────────────────────── - -@pytest.fixture -def settings() -> models.Settings: - return models.Settings() # type: ignore[call-arg] +ELYOS_BASE_URL = os.getenv( + "ELYOS_BASE_URL", "https://elyos-interview-907656039105.europe-west2.run.app" +) @pytest.fixture -def executor(settings: models.Settings) -> tools_module.ToolExecutor: - return tools_module.ToolExecutor(settings) - +def executor() -> tools_module.ToolExecutor: + return tools_module.ToolExecutor(ELYOS_BASE_URL, os.environ["ELYOS_API_KEY"]) -class _FakeToolCall: - """Minimal stand-in for ChatCompletionMessageToolCall.""" - def __init__(self, name: str, arguments: str, call_id: str = "test-id") -> None: - self.id = call_id - self.type = "function" - self.function = _FakeFunction(name, arguments) +def _weather_call(location: str) -> dict: + return { + "id": "test-id", + "type": "function", + "function": {"name": "get_weather", "arguments": f'{{"location": "{location}"}}'}, + } -class _FakeFunction: - def __init__(self, name: str, arguments: str) -> None: - self.name = name - self.arguments = arguments +def _research_call(topic: str) -> dict: + return { + "id": "test-id", + "type": "function", + "function": {"name": "research_topic", "arguments": f'{{"topic": "{topic}"}}'}, + } -def _weather_call(location: str) -> _FakeToolCall: - return _FakeToolCall("get_weather", f'{{"location": "{location}"}}') - - -def _research_call(topic: str) -> _FakeToolCall: - return _FakeToolCall("research_topic", f'{{"topic": "{topic}"}}') +def _no_cancel() -> asyncio.Event: + return asyncio.Event() # ── Weather API tests ───────────────────────────────────────────────────────── @@ -51,50 +48,48 @@ def _research_call(topic: str) -> _FakeToolCall: class TestWeatherAPI: @pytest.mark.asyncio async def test_valid_city_returns_data(self, executor: tools_module.ToolExecutor) -> None: - result = await executor.execute(_weather_call("London")) # type: ignore[arg-type] - assert not result.error - assert "London" in result.content + result = await executor.execute(_weather_call("London"), _no_cancel()) + assert not result["error"] + assert "London" in result["content"] @pytest.mark.asyncio async def test_handles_array_format(self, executor: tools_module.ToolExecutor) -> None: """Weather API non-deterministically returns flat or array format.""" - result = await executor.execute(_weather_call("Tokyo")) # type: ignore[arg-type] - assert not result.error - assert "Tokyo" in result.content - # Should work regardless of which format the API returns + result = await executor.execute(_weather_call("Tokyo"), _no_cancel()) + assert not result["error"] + assert "Tokyo" in result["content"] @pytest.mark.asyncio async def test_multi_word_city(self, executor: tools_module.ToolExecutor) -> None: - result = await executor.execute(_weather_call("San Francisco")) # type: ignore[arg-type] - assert not result.error - assert "San Francisco" in result.content + result = await executor.execute(_weather_call("San Francisco"), _no_cancel()) + assert not result["error"] + assert "San Francisco" in result["content"] @pytest.mark.asyncio async def test_invalid_city_returns_error(self, executor: tools_module.ToolExecutor) -> None: - result = await executor.execute(_weather_call("FakeCity999")) # type: ignore[arg-type] - assert result.error + result = await executor.execute(_weather_call("FakeCity999"), _no_cancel()) + assert result["error"] @pytest.mark.asyncio async def test_empty_location_returns_error(self, executor: tools_module.ToolExecutor) -> None: """Empty location returns 404 or may hit rate limit — both are errors.""" - result = await executor.execute(_weather_call("")) # type: ignore[arg-type] - assert result.error - assert result.content # should have an error message + result = await executor.execute(_weather_call(""), _no_cancel()) + assert result["error"] + assert result["content"] # should have an error message @pytest.mark.asyncio async def test_format_consistency_across_calls(self, executor: tools_module.ToolExecutor) -> None: """Regardless of flat vs array format, our display is consistent.""" successful = [] for _ in range(3): - r = await executor.execute(_weather_call("Berlin")) # type: ignore[arg-type] - if not r.error: + r = await executor.execute(_weather_call("Berlin"), _no_cancel()) + if not r["error"]: successful.append(r) await asyncio.sleep(1) # avoid triggering rate limit - # At least one should succeed; all successes should be consistent assert len(successful) >= 1, "All 3 calls were rate-limited" for r in successful: - assert "Berlin" in r.content - assert "°C" in r.content + assert "Berlin" in r["content"] + assert "°C" in r["content"] # ── Research API tests ──────────────────────────────────────────────────────── @@ -103,25 +98,22 @@ async def test_format_consistency_across_calls(self, executor: tools_module.Tool class TestResearchAPI: @pytest.mark.asyncio async def test_returns_summary(self, executor: tools_module.ToolExecutor) -> None: - result = await executor.execute(_research_call("solar energy")) # type: ignore[arg-type] - assert result.content - # Either real result or rate-limited message — both are valid - assert not result.error or "rate-limited" in result.content + result = await executor.execute(_research_call("solar energy"), _no_cancel()) + assert result["content"] + assert not result["error"] or "rate-limited" in result["content"] @pytest.mark.asyncio async def test_cached_result_shows_age(self, executor: tools_module.ToolExecutor) -> None: """Some topics return stale cached results with age metadata.""" - result = await executor.execute(_research_call("climate change")) # type: ignore[arg-type] - # If cached, should mention age; if throttled, that's ok too - if not result.error: - assert "climate change" in result.content.lower() or "cached" in result.content.lower() + result = await executor.execute(_research_call("climate change"), _no_cancel()) + if not result["error"]: + assert "climate change" in result["content"].lower() or "cached" in result["content"].lower() @pytest.mark.asyncio async def test_empty_topic_handled(self, executor: tools_module.ToolExecutor) -> None: """API accepts empty topic without error — we should handle it.""" - result = await executor.execute(_research_call("")) # type: ignore[arg-type] - # Should not crash, either returns a result or throttled - assert result.content + result = await executor.execute(_research_call(""), _no_cancel()) + assert result["content"] # ── Cancellation tests ──────────────────────────────────────────────────────── @@ -132,17 +124,17 @@ class TestCancellation: async def test_pre_cancelled_weather(self, executor: tools_module.ToolExecutor) -> None: cancel = asyncio.Event() cancel.set() - result = await executor.execute(_weather_call("London"), cancel_event=cancel) # type: ignore[arg-type] - assert result.error - assert "cancelled" in result.content.lower() + result = await executor.execute(_weather_call("London"), cancel) + assert result["error"] + assert "cancelled" in result["content"].lower() @pytest.mark.asyncio async def test_pre_cancelled_research(self, executor: tools_module.ToolExecutor) -> None: cancel = asyncio.Event() cancel.set() - result = await executor.execute(_research_call("test"), cancel_event=cancel) # type: ignore[arg-type] - assert result.error - assert "cancelled" in result.content.lower() + result = await executor.execute(_research_call("test"), cancel) + assert result["error"] + assert "cancelled" in result["content"].lower() @pytest.mark.asyncio async def test_cancel_during_research_sleep(self, executor: tools_module.ToolExecutor) -> None: @@ -153,13 +145,11 @@ async def cancel_after_delay() -> None: await asyncio.sleep(0.5) cancel.set() - async def execute() -> models.ToolResult: - return await executor.execute(_research_call("test cancel"), cancel_event=cancel) # type: ignore[arg-type] + async def execute() -> dict: + return await executor.execute(_research_call("test cancel"), cancel) - # Run both concurrently — cancel fires mid-execution result, _ = await asyncio.gather(execute(), cancel_after_delay()) - assert result.content # should have some content regardless - + assert result["content"] # ── Error handling tests ────────────────────────────────────────────────────── @@ -168,49 +158,47 @@ async def execute() -> models.ToolResult: class TestErrorHandling: @pytest.mark.asyncio async def test_invalid_json_arguments(self, executor: tools_module.ToolExecutor) -> None: - tc = _FakeToolCall("get_weather", "not valid json") - result = await executor.execute(tc) # type: ignore[arg-type] - assert result.error - assert "invalid" in result.content.lower() + tc = {"id": "test-id", "type": "function", "function": {"name": "get_weather", "arguments": "not valid json"}} + result = await executor.execute(tc, _no_cancel()) + assert result["error"] + assert "invalid" in result["content"].lower() @pytest.mark.asyncio async def test_unknown_tool_name(self, executor: tools_module.ToolExecutor) -> None: - tc = _FakeToolCall("nonexistent_tool", '{"arg": "val"}') - result = await executor.execute(tc) # type: ignore[arg-type] - assert "Unknown tool" in result.content + tc = {"id": "test-id", "type": "function", "function": {"name": "nonexistent_tool", "arguments": '{"arg": "val"}'}} + result = await executor.execute(tc, _no_cancel()) + assert "Unknown tool" in result["content"] @pytest.mark.asyncio async def test_unicode_location_handled_gracefully(self, executor: tools_module.ToolExecutor) -> None: """Unicode input causes HTML 400 from Cloud Run infra — shouldn't crash.""" - result = await executor.execute(_weather_call("東京")) # type: ignore[arg-type] - assert result.error or "Tokyo" not in result.content - # Main assertion: no crash + result = await executor.execute(_weather_call("東京"), _no_cancel()) + assert result["error"] or "Tokyo" not in result["content"] @pytest.mark.asyncio async def test_special_chars_in_location(self, executor: tools_module.ToolExecutor) -> None: """Special characters shouldn't cause crashes.""" - result = await executor.execute(_weather_call("London'; DROP TABLE --")) # type: ignore[arg-type] - # Should either return an error or handle gracefully - assert result.content + result = await executor.execute(_weather_call("London'; DROP TABLE --"), _no_cancel()) + assert result["content"] -# ── Model normalization tests ───────────────────────────────────────────────── +# ── Formatting tests ───────────────────────────────────────────────────────── -class TestModelNormalization: - def test_flat_weather_normalized(self) -> None: +class TestFormatting: + def test_flat_weather_formatted(self) -> None: data = { "location": "London", "temperature_c": 13.0, "condition": "Overcast", "humidity": 38, } - weather = models.WeatherResponse.from_api(data) - assert len(weather.conditions) == 1 - assert weather.conditions[0].temperature_c == 13.0 - assert "London" in weather.display() + output = _format_weather(data) + assert "London" in output + assert "13.0°C" in output + assert "Overcast" in output - def test_array_weather_normalized(self) -> None: + def test_array_weather_formatted(self) -> None: data = { "location": "Tokyo", "conditions": [ @@ -219,31 +207,24 @@ def test_array_weather_normalized(self) -> None: ], "note": "Multiple conditions reported", } - weather = models.WeatherResponse.from_api(data) - assert len(weather.conditions) == 2 - assert weather.note == "Multiple conditions reported" - display = weather.display() - assert "Tokyo" in display - assert "Note:" in display + output = _format_weather(data) + assert "Tokyo" in output + assert "Partly Cloudy" in output + assert "Note:" in output def test_research_cached_display(self) -> None: - resp = models.ResearchResponse( - topic="test", - summary="Test summary", - sources=["a.com"], - cached=True, - cache_age_seconds=86400 * 30, - ) - display = resp.display() - assert "cached" in display.lower() - assert "30 days" in display - - def test_throttled_response_parsed(self) -> None: data = { - "status": "throttled", - "message": "Rate limit exceeded.", - "retry_after_seconds": 5, - "data": None, + "summary": "Test summary", + "sources": ["a.com"], + "cached": True, + "cache_age_seconds": 86400 * 30, } - throttled = models.ThrottledResponse(**data) - assert throttled.retry_after_seconds == 5 + output = _format_research(data) + assert "cached" in output.lower() + assert "30 days" in output + + def test_research_basic_display(self) -> None: + data = {"summary": "Solar energy is growing.", "sources": ["source1.com", "source2.com"]} + output = _format_research(data) + assert "Solar energy" in output + assert "source1.com" in output diff --git a/uv.lock b/uv.lock index f6fb99e..577feb8 100644 --- a/uv.lock +++ b/uv.lock @@ -49,11 +49,8 @@ source = { editable = "." } dependencies = [ { name = "httpx" }, { name = "openai" }, - { name = "pydantic" }, - { name = "pydantic-settings" }, { name = "python-dotenv" }, { name = "rich" }, - { name = "tenacity" }, ] [package.dev-dependencies] @@ -69,11 +66,8 @@ dev = [ requires-dist = [ { name = "httpx", specifier = ">=0.27.0" }, { name = "openai", specifier = ">=1.60.0" }, - { name = "pydantic", specifier = ">=2.10.0" }, - { name = "pydantic-settings", specifier = ">=2.7.0" }, { name = "python-dotenv", specifier = ">=1.0.0" }, { name = "rich", specifier = ">=13.9.0" }, - { name = "tenacity", specifier = ">=9.0.0" }, ] [package.metadata.requires-dev] @@ -419,20 +413,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f7/07/34573da085946b6a313d7c42f82f16e8920bfd730665de2d11c0c37a74b5/pydantic_core-2.41.5-graalpy312-graalpy250_312_native-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:76d0819de158cd855d1cbb8fcafdf6f5cf1eb8e470abe056d5d161106e38062b", size = 2139017, upload-time = "2025-11-04T13:42:59.471Z" }, ] -[[package]] -name = "pydantic-settings" -version = "2.13.1" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "pydantic" }, - { name = "python-dotenv" }, - { name = "typing-inspection" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/52/6d/fffca34caecc4a3f97bda81b2098da5e8ab7efc9a66e819074a11955d87e/pydantic_settings-2.13.1.tar.gz", hash = "sha256:b4c11847b15237fb0171e1462bf540e294affb9b86db4d9aa5c01730bdbe4025", size = 223826, upload-time = "2026-02-19T13:45:08.055Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/00/4b/ccc026168948fec4f7555b9164c724cf4125eac006e176541483d2c959be/pydantic_settings-2.13.1-py3-none-any.whl", hash = "sha256:d56fd801823dbeae7f0975e1f8c8e25c258eb75d278ea7abb5d9cebb01b56237", size = 58929, upload-time = "2026-02-19T13:45:06.034Z" }, -] - [[package]] name = "pygments" version = "2.20.0" @@ -558,15 +538,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/e9/44/75a9c9421471a6c4805dbf2356f7c181a29c1879239abab1ea2cc8f38b40/sniffio-1.3.1-py3-none-any.whl", hash = "sha256:2f6da418d1f1e0fddd844478f41680e794e6051915791a034ff65e5f100525a2", size = 10235, upload-time = "2024-02-25T23:20:01.196Z" }, ] -[[package]] -name = "tenacity" -version = "9.1.4" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/47/c6/ee486fd809e357697ee8a44d3d69222b344920433d3b6666ccd9b374630c/tenacity-9.1.4.tar.gz", hash = "sha256:adb31d4c263f2bd041081ab33b498309a57c77f9acf2db65aadf0898179cf93a", size = 49413, upload-time = "2026-02-07T10:45:33.841Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/d7/c1/eb8f9debc45d3b7918a32ab756658a0904732f75e555402972246b0b8e71/tenacity-9.1.4-py3-none-any.whl", hash = "sha256:6095a360c919085f28c6527de529e76a06ad89b23659fa881ae0649b867a9d55", size = 28926, upload-time = "2026-02-07T10:45:32.24Z" }, -] - [[package]] name = "tomlkit" version = "0.14.0" From 58271325b791bb1c4b3594a0bf9da88f98f3d994 Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Sat, 11 Apr 2026 12:45:32 +0100 Subject: [PATCH 4/8] Updated readme with instructions for bare bones python setup --- README.md | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index c9011b6..032fc15 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,6 @@ A command-line chat application with streaming LLM responses and tool calling. B ## Requirements - Python 3.12+ -- [uv](https://docs.astral.sh/uv/) - API keys in `.env`: ``` OPENROUTER_API_KEY=sk-or-... @@ -14,14 +13,30 @@ A command-line chat application with streaming LLM responses and tool calling. B ## Setup +### With uv (recommended) + +```bash +make install # runs uv sync +make run # runs uv run cli-chat +``` + +### Without uv (bare Python) + ```bash -make install +python -m venv .venv +source .venv/bin/activate # Windows: .venv\Scripts\activate +pip install . ``` ## Usage ```bash +# With uv make run + +# Without uv +source .venv/bin/activate +cli-chat ``` **Example session:** From dc908ded4aa1899b6ec5ab20f22235be7b370ca4 Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Sun, 12 Apr 2026 19:53:50 +0100 Subject: [PATCH 5/8] Make turn atomic on infra failure; refresh DISCOVERIES references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Roll back history to a pre-turn snapshot when _stream_response returns None, so an LLM/API failure after the user message (or after partial tool-call/result pairs) doesn't leave dangling entries that would poison the next turn. The Ctrl+C-with-partial-content path is unchanged — that reflects user intent, not infra failure. Also update DISCOVERIES.md to match the post-pydantic/tenacity code: _format_weather shape detection, plain-dict tool results, _race_with_cancel / _wait_or_cancel, and RuntimeError for exhausted throttle retries. Co-Authored-By: Claude Opus 4.6 (1M context) --- DISCOVERIES.md | 12 ++++++------ src/cli_chat/orchestrator.py | 3 +++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/DISCOVERIES.md b/DISCOVERIES.md index c963f55..b3e57c5 100644 --- a/DISCOVERIES.md +++ b/DISCOVERIES.md @@ -40,7 +40,7 @@ The API returns two different response shapes **randomly for the same city**. Co } ``` -**Handling:** `WeatherResponse.from_api()` detects the shape and normalizes both into a consistent `conditions` list. +**Handling:** `_format_weather` detects the shape — the flat case is wrapped into a single-element `conditions` list before rendering, so both shapes render identically. #### 2. Weather Also Rate-Limits (HTTP 200) @@ -371,9 +371,9 @@ Two distinct behaviors: Some inputs (observed with whitespace-only `" "` and XSS payloads) **occasionally return an empty `{}`** with HTTP 200. This is non-deterministic — the same input returns a normal response most of the time, but rarely returns `{}`. -**Impact:** Without a guard, `ResearchResponse(**{})` raises a Pydantic `ValidationError` (missing required `topic` and `summary`). +**Impact:** Formatting an empty dict would raise a `KeyError` on `summary`. -**Handling:** `_research_topic()` checks for empty or incomplete responses (missing `topic` or empty `summary`) before Pydantic parsing, returning a graceful "no results" message instead of crashing. +**Handling:** `_research_topic` checks for missing `topic` or empty `summary` before calling `_format_research`, returning a graceful "no results" message instead of crashing. #### 6. Standard Error Responses - `422` for missing `topic` param @@ -423,13 +423,13 @@ Both endpoints share a single rate limit pool. Behavior: **Root cause:** The cancel event was only checked before and after the HTTP request, not during it. The httpx `await` held the coroutine for the full request duration. -**Fix:** Added `_cancellable_request()` which races the httpx coroutine against `cancel_event.wait()` via `asyncio.wait(FIRST_COMPLETED)`. When Ctrl+C fires, the HTTP request task is immediately cancelled and the connection closed. Same pattern used by `_read_input` and `_cancellable_sleep`. +**Fix:** Added `_race_with_cancel`, which races any awaitable against `cancel_event.wait()` via `asyncio.wait(FIRST_COMPLETED)`. `_request` routes the httpx coroutine through it, so when Ctrl+C fires the HTTP request task is immediately cancelled and the connection closed. The same helper backs `_wait_or_cancel` for throttle-retry sleeps, and `_read_input` uses the equivalent `add_reader` + `asyncio.wait` pattern. ### 3. Rate-Limited Results Not Marked as Errors (Fixed) -**Problem:** When the weather or research API returned a throttled response and retries were exhausted, the rate-limit message was returned as a successful `ToolResult(error=False)`. The LLM treated it as a valid tool response, and the user saw no error indication. +**Problem:** When the weather or research API returned a throttled response and retries were exhausted, the rate-limit message was returned as a successful tool result (`error=False`). The LLM treated it as a valid tool response, and the user saw no error indication. -**Fix:** Introduced `_RateLimitError` exception. Exhausted throttle retries now raise this exception, which is caught by `execute()` and returned as `ToolResult(error=True)`. +**Fix:** `_request_with_retry` now raises `RuntimeError` when throttle retries are exhausted. `execute()` catches it alongside the other `httpx`/request failures and returns the tool result dict with `error=True`. ### 4. Unresponsive Exit After Goodbye (Fixed) diff --git a/src/cli_chat/orchestrator.py b/src/cli_chat/orchestrator.py index 56150b1..9e25b27 100644 --- a/src/cli_chat/orchestrator.py +++ b/src/cli_chat/orchestrator.py @@ -166,10 +166,13 @@ async def _process_turn( # pylint: disable=too-many-arguments,too-many-position cancel_event: asyncio.Event, ) -> None: logger.info("User input: %s", user_input) + rollback_point = len(history) history.append({"role": "user", "content": user_input}) while True: result = await _stream_response(client, model, history, cancel_event) if result is None: + logger.info("Rolling back turn (%d entries) after stream failure", len(history) - rollback_point) + del history[rollback_point:] return content, tool_calls = result if cancel_event.is_set(): From e1d3ecb76d800122cd4e4207da67ce8469a9f376 Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Sun, 12 Apr 2026 22:08:53 +0100 Subject: [PATCH 6/8] Slim tool-call plumbing for readability Inline the two history.append loops in _process_turn and default _tool_result's error flag to False so success sites drop the positional bool. Net fewer lines and clearer call sites. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cli_chat/orchestrator.py | 46 +++++++++++++++++++----------------- src/cli_chat/tools.py | 38 ++++++++++++++++++----------- tests/test_e2e.py | 27 +-------------------- 3 files changed, 49 insertions(+), 62 deletions(-) diff --git a/src/cli_chat/orchestrator.py b/src/cli_chat/orchestrator.py index 9e25b27..ec72061 100644 --- a/src/cli_chat/orchestrator.py +++ b/src/cli_chat/orchestrator.py @@ -36,7 +36,6 @@ def _configure_logging() -> str: for noisy in ("httpx", "openai", "httpcore"): logging.getLogger(noisy).setLevel(logging.WARNING) return log_file - def _load_config() -> tuple[str, str, str, str, str]: return ( os.environ["LLM_API_KEY"], @@ -70,6 +69,21 @@ def _on_stdin_ready() -> None: line = line_future.result() return line.rstrip("\n") if line else None + +def _merge_tool_call_delta(tool_calls_by_index: dict[int, dict], tc_delta) -> None: + entry = tool_calls_by_index.setdefault( + tc_delta.index, + {"id": tc_delta.id or "", "type": "function", "function": {"name": "", "arguments": ""}}, + ) + if tc_delta.id: + entry["id"] = tc_delta.id + if not tc_delta.function: + return + if tc_delta.function.name: + entry["function"]["name"] = tc_delta.function.name + if tc_delta.function.arguments: + entry["function"]["arguments"] += tc_delta.function.arguments + async def _stream_response( # pylint: disable=too-many-branches client: AsyncOpenAI, model: str, @@ -109,21 +123,9 @@ async def _stream_response( # pylint: disable=too-many-branches content += delta.content sys.stdout.write(delta.content) sys.stdout.flush() - if delta.tool_calls: for tc_delta in delta.tool_calls: - index = tc_delta.index - entry = tool_calls_by_index.setdefault( - index, - {"id": tc_delta.id or "", "type": "function", "function": {"name": "", "arguments": ""}}, - ) - if tc_delta.id: - entry["id"] = tc_delta.id - if tc_delta.function: - if tc_delta.function.name: - entry["function"]["name"] = tc_delta.function.name - if tc_delta.function.arguments: - entry["function"]["arguments"] += tc_delta.function.arguments + _merge_tool_call_delta(tool_calls_by_index, tc_delta) except Exception as exc: # pylint: disable=broad-exception-caught logger.error("Stream error: %s", exc, exc_info=True) print(f"\nError: Stream error: {exc}", file=sys.stderr) @@ -131,6 +133,7 @@ async def _stream_response( # pylint: disable=too-many-branches logger.debug("Stream completed: %d chars content, %d tool calls", len(content), len(tool_calls_by_index)) return content, list(tool_calls_by_index.values()) + async def _execute_tools( tools: ToolExecutor, tool_calls: list[dict], @@ -192,15 +195,11 @@ async def _process_turn( # pylint: disable=too-many-arguments,too-many-position return tool_results = await _execute_tools(tools, tool_calls, cancel_event) if tool_results is None: - for tool_call in tool_calls: - history.append( - {"role": "tool", "tool_call_id": tool_call["id"], "content": "[cancelled by user]"} - ) + for tc in tool_calls: + history.append({"role": "tool", "tool_call_id": tc["id"], "content": "[cancelled by user]"}) return - for result in tool_results: - history.append( - {"role": "tool", "tool_call_id": result["tool_call_id"], "content": result["content"]} - ) + for r in tool_results: + history.append({"role": "tool", "tool_call_id": r["tool_call_id"], "content": r["content"]}) async def run_chat() -> None: llm_api_key, llm_base_url, model, elyos_api_key, elyos_base_url = _load_config() @@ -210,6 +209,7 @@ async def run_chat() -> None: history: list[dict] = [] cancel_event = asyncio.Event() state = {"should_exit": False} + def _handle_interrupt() -> None: if cancel_event.is_set(): state["should_exit"] = True @@ -237,8 +237,10 @@ def _handle_interrupt() -> None: print("\nGoodbye!") logger.info("Session ended") + def main() -> None: asyncio.run(run_chat()) + if __name__ == "__main__": main() diff --git a/src/cli_chat/tools.py b/src/cli_chat/tools.py index a401b02..1789c21 100644 --- a/src/cli_chat/tools.py +++ b/src/cli_chat/tools.py @@ -40,6 +40,7 @@ MAX_RETRIES = 3 MAX_THROTTLE_WAIT = 15 + def _format_weather(data: dict) -> str: conditions = data.get("conditions") if not conditions: @@ -69,6 +70,11 @@ def _format_research(data: dict) -> str: parts.append(f"Note: cached result ({days_old} days old)") return "\n".join(parts) + +def _tool_result(tool_call_id: str, content: str, error: bool = False) -> dict: + return {"tool_call_id": tool_call_id, "content": content, "error": error} + + class ToolExecutor: def __init__(self, base_url: str, api_key: str) -> None: self._client = httpx.AsyncClient( @@ -80,36 +86,39 @@ def __init__(self, base_url: str, api_key: str) -> None: async def close(self) -> None: await self._client.aclose() + async def _execute_named_tool(self, name: str, args: dict, cancel_event: asyncio.Event) -> str: + if name == "get_weather": + return await self._get_weather(args.get("location", ""), cancel_event) + if name == "research_topic": + return await self._research_topic(args.get("topic", ""), cancel_event) + logger.warning("Unknown tool requested: %s", name) + return f"Unknown tool: {name}" + async def execute(self, tool_call: dict, cancel_event: asyncio.Event) -> dict: + tool_call_id = tool_call["id"] name = tool_call["function"]["name"] try: args = json.loads(tool_call["function"]["arguments"] or "{}") except json.JSONDecodeError: logger.error("Invalid JSON arguments for tool %s: %s", name, tool_call["function"]["arguments"]) - return {"tool_call_id": tool_call["id"], "content": "Error: invalid tool arguments", "error": True} + return _tool_result(tool_call_id, "Error: invalid tool arguments", error=True) logger.info("Tool call: %s(%s)", name, args) try: - if name == "get_weather": - content = await self._get_weather(args.get("location", ""), cancel_event) - elif name == "research_topic": - content = await self._research_topic(args.get("topic", ""), cancel_event) - else: - logger.warning("Unknown tool requested: %s", name) - content = f"Unknown tool: {name}" + content = await self._execute_named_tool(name, args, cancel_event) logger.info("Tool %s completed", name) logger.debug("Tool %s result: %s", name, content[:200]) - return {"tool_call_id": tool_call["id"], "content": content, "error": False} + return _tool_result(tool_call_id, content) except asyncio.CancelledError: logger.warning("Tool %s cancelled by user", name) - return {"tool_call_id": tool_call["id"], "content": "Tool call was cancelled by the user.", "error": True} + return _tool_result(tool_call_id, "Tool call was cancelled by the user.", error=True) except httpx.HTTPStatusError as exc: logger.error("Tool %s HTTP error %d: %s", name, exc.response.status_code, exc.response.text[:200]) msg = f"API error ({exc.response.status_code}): {exc.response.text}" - return {"tool_call_id": tool_call["id"], "content": msg, "error": True} + return _tool_result(tool_call_id, msg, error=True) except (httpx.RequestError, httpx.TimeoutException, httpx.DecodingError, RuntimeError) as exc: logger.error("Tool %s request failed: %s", name, exc) - return {"tool_call_id": tool_call["id"], "content": f"Request failed: {exc}", "error": True} + return _tool_result(tool_call_id, f"Request failed: {exc}", error=True) async def _get_weather(self, location: str, cancel_event: asyncio.Event) -> str: data = await self._request_with_retry("Weather", "/weather", {"location": location}, cancel_event) @@ -139,8 +148,8 @@ async def _request_with_retry( logger.warning("%s throttled (attempt %d/%d), retry in %ds", label, attempt + 1, MAX_RETRIES, retry_after) if attempt < MAX_RETRIES - 1: await _wait_or_cancel(retry_after, cancel_event) - raise RuntimeError(f"{label} API is rate-limited. Please try again in {retry_after}s.") + async def _request(self, path: str, params: dict, cancel_event: asyncio.Event) -> dict: if cancel_event.is_set(): raise asyncio.CancelledError @@ -153,12 +162,13 @@ async def _request(self, path: str, params: dict, cancel_event: asyncio.Event) - content_type = response.headers.get("content-type", "") if "json" not in content_type: raise httpx.DecodingError(f"Unexpected response format (got {content_type})") - return response.json() + async def _wait_or_cancel(delay: float, cancel_event: asyncio.Event) -> None: await _race_with_cancel(asyncio.sleep(delay), cancel_event) + async def _race_with_cancel(awaitable, cancel_event: asyncio.Event): work_task = asyncio.create_task(awaitable) cancel_task = asyncio.create_task(cancel_event.wait()) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 1509f8a..dde0c5e 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -10,12 +10,7 @@ from cli_chat import tools as tools_module from cli_chat.tools import _format_research, _format_weather -# ── Fixtures ────────────────────────────────────────────────────────────────── - -ELYOS_BASE_URL = os.getenv( - "ELYOS_BASE_URL", "https://elyos-interview-907656039105.europe-west2.run.app" -) - +ELYOS_BASE_URL = os.getenv("ELYOS_BASE_URL", "https://elyos-interview-907656039105.europe-west2.run.app") @pytest.fixture def executor() -> tools_module.ToolExecutor: @@ -41,10 +36,6 @@ def _research_call(topic: str) -> dict: def _no_cancel() -> asyncio.Event: return asyncio.Event() - -# ── Weather API tests ───────────────────────────────────────────────────────── - - class TestWeatherAPI: @pytest.mark.asyncio async def test_valid_city_returns_data(self, executor: tools_module.ToolExecutor) -> None: @@ -91,10 +82,6 @@ async def test_format_consistency_across_calls(self, executor: tools_module.Tool assert "Berlin" in r["content"] assert "°C" in r["content"] - -# ── Research API tests ──────────────────────────────────────────────────────── - - class TestResearchAPI: @pytest.mark.asyncio async def test_returns_summary(self, executor: tools_module.ToolExecutor) -> None: @@ -115,10 +102,6 @@ async def test_empty_topic_handled(self, executor: tools_module.ToolExecutor) -> result = await executor.execute(_research_call(""), _no_cancel()) assert result["content"] - -# ── Cancellation tests ──────────────────────────────────────────────────────── - - class TestCancellation: @pytest.mark.asyncio async def test_pre_cancelled_weather(self, executor: tools_module.ToolExecutor) -> None: @@ -151,10 +134,6 @@ async def execute() -> dict: result, _ = await asyncio.gather(execute(), cancel_after_delay()) assert result["content"] - -# ── Error handling tests ────────────────────────────────────────────────────── - - class TestErrorHandling: @pytest.mark.asyncio async def test_invalid_json_arguments(self, executor: tools_module.ToolExecutor) -> None: @@ -181,10 +160,6 @@ async def test_special_chars_in_location(self, executor: tools_module.ToolExecut result = await executor.execute(_weather_call("London'; DROP TABLE --"), _no_cancel()) assert result["content"] - -# ── Formatting tests ───────────────────────────────────────────────────────── - - class TestFormatting: def test_flat_weather_formatted(self) -> None: data = { From 4bd000d0831da137e65697d1a0822e5ca63ea86c Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Mon, 13 Apr 2026 09:11:48 +0100 Subject: [PATCH 7/8] Port research timeout fix and env-var README clarification from main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cherry-picks from main, adapted to slim-display's architecture: - Bump REQUEST_TIMEOUT from 15s to 20s so the research API's 15s slow path always finishes server-side (DISCOVERIES.md), instead of racing our httpx.ReadTimeout — which can wrap an empty asyncio.TimeoutError and surface as a cryptic "Request failed: " error. - Wrap the catch-all httpx exception handler to include the exception class name when str(exc) is empty, so future empty-message exceptions remain diagnosable. - Reword README "Requirements" to list env vars as the primary interface, noting that .env is one option (picked up automatically by uv run). Old wording implied keys had to live in .env. Co-Authored-By: Claude Opus 4.6 (1M context) --- README.md | 10 ++++++++-- src/cli_chat/tools.py | 12 +++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index ff05263..dbb4163 100644 --- a/README.md +++ b/README.md @@ -5,12 +5,18 @@ A command-line chat application with streaming LLM responses and tool calling. B ## Requirements - Python 3.12+ -- API keys in `.env`: +- [uv](https://docs.astral.sh/uv/) +- Environment variables: + - `LLM_API_KEY` (required) + - `ELYOS_API_KEY` (required) + - `LLM_BASE_URL` (optional, default: OpenRouter) + - `LLM_MODEL` (optional, default: `openai/gpt-4o-mini`) + + Set them however you prefer — exported in your shell, via CI secrets, or dropped into a local `.env` file, which `uv run` picks up automatically: ``` LLM_API_KEY=sk-... ELYOS_API_KEY=... ``` - Optional overrides: `LLM_BASE_URL` (default: OpenRouter), `LLM_MODEL` (default: `openai/gpt-4o-mini`) ## Setup diff --git a/src/cli_chat/tools.py b/src/cli_chat/tools.py index 1789c21..ead8da7 100644 --- a/src/cli_chat/tools.py +++ b/src/cli_chat/tools.py @@ -36,7 +36,10 @@ }, ] -REQUEST_TIMEOUT = 15.0 +# Set above the research API's 15s server-side timeout (DISCOVERIES.md) so the +# server's empty-response path always wins the race and we never raise our own +# httpx.ReadTimeout (which can carry an empty message and looks like an infra bug). +REQUEST_TIMEOUT = 20.0 MAX_RETRIES = 3 MAX_THROTTLE_WAIT = 15 @@ -117,8 +120,11 @@ async def execute(self, tool_call: dict, cancel_event: asyncio.Event) -> dict: msg = f"API error ({exc.response.status_code}): {exc.response.text}" return _tool_result(tool_call_id, msg, error=True) except (httpx.RequestError, httpx.TimeoutException, httpx.DecodingError, RuntimeError) as exc: - logger.error("Tool %s request failed: %s", name, exc) - return _tool_result(tool_call_id, f"Request failed: {exc}", error=True) + # Some httpx exceptions (notably timeouts wrapping asyncio.TimeoutError) can + # carry an empty str(), so always include the class name for diagnosability. + detail = f"{type(exc).__name__}: {exc}" if str(exc) else type(exc).__name__ + logger.error("Tool %s request failed: %s", name, detail) + return _tool_result(tool_call_id, f"Request failed: {detail}", error=True) async def _get_weather(self, location: str, cancel_event: asyncio.Event) -> str: data = await self._request_with_retry("Weather", "/weather", {"location": location}, cancel_event) From c519a9996f28858b1747fedefc433ed8b2e77797 Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Mon, 13 Apr 2026 09:20:36 +0100 Subject: [PATCH 8/8] Align CLAUDE.md env-var wording with README CLAUDE.md still framed .env as the required key store; README now treats env vars as the primary interface with .env as one option (picked up by uv run). Update CLAUDE.md to match. Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d151cbe..48ec00d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,13 +8,13 @@ Take-home interview project: CLI chat app with streaming LLM + tool calling agai - `make install` → `make run` to use - `make check` runs lint + test (20 e2e tests) -- API keys in `.env` (LLM_API_KEY, ELYOS_API_KEY; optional LLM_BASE_URL, LLM_MODEL) +- Env vars: LLM_API_KEY, ELYOS_API_KEY (required); LLM_BASE_URL, LLM_MODEL (optional). `uv run` auto-loads `.env` if present. - Logs: `cli_chat__.log` per session - Design docs: README.md, ARCHITECTURE.md, DISCOVERIES.md ## Key decisions -- Any OpenAI-compatible endpoint for LLM, configurable via LLM_BASE_URL and LLM_MODEL in .env +- Any OpenAI-compatible endpoint for LLM, configurable via LLM_BASE_URL and LLM_MODEL env vars - httpx async for external API calls, with `asyncio.wait` racing requests against cancel events - Plain dicts for API responses, formatted by `_format_weather` / `_format_research` helpers - Simple retry loop for throttle handling (reads `retry_after_seconds`, max 3 attempts)