From 51188fe3bd617532f499854f6f1f3fc4bad1eb28 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:52:42 +0000 Subject: [PATCH] refactor: address code review feedback on MCP tool calling --- steward/bot/telegram.py | 4 ++-- steward/llm/client.py | 15 ++++++++++----- steward/tools/client.py | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/steward/bot/telegram.py b/steward/bot/telegram.py index a56b47a..4e2bb50 100644 --- a/steward/bot/telegram.py +++ b/steward/bot/telegram.py @@ -371,7 +371,7 @@ async def message_handler(update: Update, context: ContextTypes.DEFAULT_TYPE) -> if tool_client is not None: reply = await llm.chat_with_tools(text, tool_client, history=call_history) else: - reply = await llm.chat(text, history=call_history) # type: ignore[arg-type] + reply = await llm.chat(text, history=call_history) history.append({"role": "user", "content": text}) history.append({"role": "assistant", "content": reply}) else: @@ -381,7 +381,7 @@ async def message_handler(update: Update, context: ContextTypes.DEFAULT_TYPE) -> if tool_client is not None: reply = await llm.chat_with_tools(text, tool_client, history=call_history) else: - reply = await llm.chat(text, history=call_history) # type: ignore[arg-type] + reply = await llm.chat(text, history=call_history) user_history.append({"role": "user", "content": text}) user_history.append({"role": "assistant", "content": reply}) if len(user_history) > _MAX_HISTORY * 2: diff --git a/steward/llm/client.py b/steward/llm/client.py index a048d2d..6292af9 100644 --- a/steward/llm/client.py +++ b/steward/llm/client.py @@ -29,12 +29,12 @@ class LLMClient: self, user_message: str, *, - history: list[dict[str, str]] | None = None, + history: list[dict[str, Any]] | None = None, system_prompt: str | None = None, ) -> str: """Send a user message (with optional history) and return the assistant reply.""" system = system_prompt or self._settings.openai_system_prompt - messages: list[dict[str, str]] = [{"role": "system", "content": system}] + messages: list[dict[str, Any]] = [{"role": "system", "content": system}] if history: messages.extend(history) messages.append({"role": "user", "content": user_message}) @@ -131,7 +131,12 @@ class LLMClient: # Execute each tool and append results for tc in msg.tool_calls: try: - args = json.loads(tc.function.arguments) + try: + args = json.loads(tc.function.arguments) + except json.JSONDecodeError as exc: + raise ValueError( + f"Invalid JSON in tool arguments for '{tc.function.name}': {exc}" + ) from exc result = await tool_client.call(tc.function.name, args) logger.info("Tool %s → %d chars", tc.function.name, len(result)) except Exception as exc: @@ -153,12 +158,12 @@ class LLMClient: self, user_message: str, *, - history: list[dict[str, str]] | None = None, + history: list[dict[str, Any]] | None = None, system_prompt: str | None = None, ) -> AsyncIterator[str]: """Stream the assistant reply token by token.""" system = system_prompt or self._settings.openai_system_prompt - messages: list[dict[str, str]] = [{"role": "system", "content": system}] + messages: list[dict[str, Any]] = [{"role": "system", "content": system}] if history: messages.extend(history) messages.append({"role": "user", "content": user_message}) diff --git a/steward/tools/client.py b/steward/tools/client.py index 062a2d8..d8e70b7 100644 --- a/steward/tools/client.py +++ b/steward/tools/client.py @@ -40,7 +40,7 @@ class ToolClient: def _headers(self) -> dict[str, str]: headers: dict[str, str] = {"Accept": "application/json"} if self._api_key: - headers["Authorization"] = "Bearer " + self._api_key + headers["Authorization"] = "Bearer " + self._api_key.strip() return headers async def get_spec(self) -> dict[str, Any]: