From 3573e53b1d539c90f44f44f1fa23fd04914b3e2f Mon Sep 17 00:00:00 2001 From: Daoyang Shan Date: Mon, 30 Mar 2026 16:31:17 +0800 Subject: [PATCH] Fix Codex named tool_choice in reflect (#734) Co-authored-by: Sapientropic --- .../engine/providers/codex_llm.py | 30 ++++- .../tests/test_codex_tool_choice.py | 107 ++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 hindsight-api-slim/tests/test_codex_tool_choice.py diff --git a/hindsight-api-slim/hindsight_api/engine/providers/codex_llm.py b/hindsight-api-slim/hindsight_api/engine/providers/codex_llm.py index bbaddd3d..3da61140 100644 --- a/hindsight-api-slim/hindsight_api/engine/providers/codex_llm.py +++ b/hindsight-api-slim/hindsight_api/engine/providers/codex_llm.py @@ -126,6 +126,32 @@ class CodexLLM(LLMInterface): } return mapping.get(effort.lower(), "auto") + def _normalize_tool_choice(self, tool_choice: str | dict[str, Any]) -> str | dict[str, Any]: + """Normalize forced function tool choice for the Codex Responses API. + + Older agent paths may still pass OpenAI chat-completions style named + tool choice payloads such as: + + {"type": "function", "function": {"name": "recall"}} + + Codex Responses expects the named function at the top level instead: + + {"type": "function", "name": "recall"} + """ + if not isinstance(tool_choice, dict): + return tool_choice + if str(tool_choice.get("type") or "").strip() != "function": + return tool_choice + function_payload = tool_choice.get("function") + if isinstance(function_payload, dict): + function_name = str(function_payload.get("name") or "").strip() + if function_name: + return {"type": "function", "name": function_name} + function_name = str(tool_choice.get("name") or "").strip() + if function_name: + return {"type": "function", "name": function_name} + return tool_choice + async def verify_connection(self) -> None: """Verify Codex connection by making a simple test call.""" try: @@ -425,7 +451,7 @@ class CodexLLM(LLMInterface): max_retries: Maximum retry attempts. initial_backoff: Initial backoff time in seconds. max_backoff: Maximum backoff time in seconds. - tool_choice: How to choose tools - "auto", "none", "required", or specific function. + tool_choice: How to choose tools - "auto", "none", "required", or a specific function. Returns: LLMToolCallResult with content and/or tool_calls. @@ -482,7 +508,7 @@ class CodexLLM(LLMInterface): "instructions": system_instruction, "input": user_messages, "tools": codex_tools, - "tool_choice": tool_choice, + "tool_choice": self._normalize_tool_choice(tool_choice), "parallel_tool_calls": True, "reasoning": {"summary": reasoning_summary}, "store": False, diff --git a/hindsight-api-slim/tests/test_codex_tool_choice.py b/hindsight-api-slim/tests/test_codex_tool_choice.py new file mode 100644 index 00000000..e35ced8d --- /dev/null +++ b/hindsight-api-slim/tests/test_codex_tool_choice.py @@ -0,0 +1,107 @@ +import asyncio +import sys +import types +import unittest +from unittest.mock import AsyncMock, MagicMock, patch +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +PACKAGE_ROOT = ROOT / "hindsight_api" +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + + +def ensure_package(name: str, path: Path) -> None: + module = sys.modules.get(name) + if module is None: + module = types.ModuleType(name) + module.__path__ = [str(path)] + sys.modules[name] = module + + +ensure_package("hindsight_api", PACKAGE_ROOT) +ensure_package("hindsight_api.engine", PACKAGE_ROOT / "engine") +ensure_package("hindsight_api.engine.providers", PACKAGE_ROOT / "engine" / "providers") +fake_metrics = types.ModuleType("hindsight_api.metrics") +fake_metrics.get_metrics_collector = lambda: types.SimpleNamespace(record_llm_call=lambda **kwargs: None) +sys.modules["hindsight_api.metrics"] = fake_metrics + +from hindsight_api.engine.providers.codex_llm import CodexLLM + + +TOOLS = [ + { + "type": "function", + "function": { + "name": "recall", + "description": "Recall semantic memories", + "parameters": { + "type": "object", + "properties": {"query": {"type": "string"}}, + "required": ["query"], + }, + }, + } +] + + +def build_llm() -> CodexLLM: + with patch.object(CodexLLM, "_load_codex_auth", return_value=("token", "account")): + return CodexLLM( + provider="openai-codex", + api_key="ignored", + base_url="https://chatgpt.com/backend-api", + model="gpt-5.4-mini", + ) + + +class CodexToolChoiceTests(unittest.TestCase): + def test_codex_normalizes_legacy_named_tool_choice_shape(self) -> None: + async def scenario() -> dict: + llm = build_llm() + response = MagicMock() + response.status_code = 200 + response.raise_for_status.return_value = None + with patch.object(llm._client, "post", new_callable=AsyncMock) as mock_post: + mock_post.return_value = response + with patch.object(llm, "_parse_sse_tool_stream", new_callable=AsyncMock) as mock_parse: + mock_parse.return_value = (None, []) + await llm.call_with_tools( + messages=[{"role": "user", "content": "recall the memory"}], + tools=TOOLS, + tool_choice={"type": "function", "function": {"name": "recall"}}, + max_retries=0, + ) + return mock_post.call_args.kwargs["json"] + + sent_payload = asyncio.run(scenario()) + self.assertEqual(sent_payload["tool_choice"], {"type": "function", "name": "recall"}) + + def test_codex_forced_tool_choice_still_yields_tool_calls(self) -> None: + async def scenario() -> tuple[object, dict]: + llm = build_llm() + response = MagicMock() + response.status_code = 200 + response.raise_for_status.return_value = None + tool_call = {"id": "call-1", "name": "recall", "arguments": {"query": "memory"}} + with patch.object(llm._client, "post", new_callable=AsyncMock) as mock_post: + mock_post.return_value = response + with patch.object(llm, "_parse_sse_tool_stream", new_callable=AsyncMock) as mock_parse: + mock_parse.return_value = (None, [tool_call]) + result = await llm.call_with_tools( + messages=[{"role": "user", "content": "recall the memory"}], + tools=TOOLS, + tool_choice={"type": "function", "function": {"name": "recall"}}, + max_retries=0, + ) + return result, mock_post.call_args.kwargs["json"] + + result, sent_payload = asyncio.run(scenario()) + self.assertEqual(len(result.tool_calls), 1) + self.assertEqual(result.tool_calls[0].name, "recall") + self.assertEqual(sent_payload["tool_choice"], {"type": "function", "name": "recall"}) + + +if __name__ == "__main__": + unittest.main()