Fix Codex named tool_choice in reflect (#734)
Co-authored-by: Sapientropic <noreply@github.com>
This commit is contained in:
parent
585ac76f39
commit
3573e53b1d
2 changed files with 135 additions and 2 deletions
|
|
@ -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,
|
||||
|
|
|
|||
107
hindsight-api-slim/tests/test_codex_tool_choice.py
Normal file
107
hindsight-api-slim/tests/test_codex_tool_choice.py
Normal file
|
|
@ -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()
|
||||
Loading…
Reference in a new issue