diff --git a/src/llm/backends/openai.py b/src/llm/backends/openai.py index 672709ff..5b910ae9 100644 --- a/src/llm/backends/openai.py +++ b/src/llm/backends/openai.py @@ -441,10 +441,19 @@ class OpenAIBackend: ) cache_creation, cache_read = extract_openai_cache_tokens(usage) + # content_override=None means no override, not "force content to None" + if content_override is not None: + content: Any = content_override + elif message.content is not None: + content = message.content + elif tool_calls: + # Preserve null content on tool-call turns for history replay + content = None + else: + content = "" + return CompletionResult( - content=content_override - if content_override is not None - else (message.content or ""), + content=content, input_tokens=usage.prompt_tokens if usage else 0, output_tokens=usage.completion_tokens if usage else 0, cache_creation_input_tokens=cache_creation, diff --git a/tests/live_llm/README.md b/tests/live_llm/README.md index 5cf39e4b..9590185e 100644 --- a/tests/live_llm/README.md +++ b/tests/live_llm/README.md @@ -63,8 +63,8 @@ export OPENROUTER_API_KEY="sk-or-v1-..." Coverage by provider: - Anthropic: structured output path, prompt caching metrics, thinking blocks, multi-turn tool replay -- OpenAI GPT-4 class: structured outputs, prompt caching -- OpenAI GPT-5 class (incl. gpt-5.x point-releases): structured outputs, prompt caching, `reasoning_effort`, `max_completion_tokens` routing +- OpenAI GPT-4 class: structured outputs, prompt caching, multi-turn tool replay (null `content` preserved) +- OpenAI GPT-5 class (incl. gpt-5.x point-releases): structured outputs, prompt caching, `reasoning_effort`, `max_completion_tokens` routing, multi-turn tool replay (null `content` preserved) - OpenAI transport → OpenRouter non-reasoning models (e.g. `inception/mercury-2`): non-chat / diffusion architectures must stay on `max_tokens`, no `reasoning_effort`, tool-calling parameter-schema compatibility is the canary for exotic OR-served providers - Gemini 2.5/3.0 classes: structured outputs, cached-content reuse, thought signatures, multi-turn tool replay - Gemini 3.1 class: thinking and tool replay coverage by default; structured-output/caching coverage should only be added once Google documents support for that path diff --git a/tests/live_llm/model_matrix.py b/tests/live_llm/model_matrix.py index 2478c6c3..3a18a84e 100644 --- a/tests/live_llm/model_matrix.py +++ b/tests/live_llm/model_matrix.py @@ -58,6 +58,7 @@ MODEL_FAMILIES: tuple[LiveModelFamily, ...] = ( default_models=("gpt-4.1",), supports_structured_output=True, supports_caching=True, + supports_tool_replay=True, docs_url="https://platform.openai.com/docs/models/gpt-4.1", ), LiveModelFamily( @@ -68,6 +69,7 @@ MODEL_FAMILIES: tuple[LiveModelFamily, ...] = ( supports_structured_output=True, supports_caching=True, supports_reasoning=True, + supports_tool_replay=True, docs_url="https://platform.openai.com/docs/models/gpt-5", ), # OpenAI-compatible transport → OpenRouter-served non-reasoning models. diff --git a/tests/live_llm/test_live_openai.py b/tests/live_llm/test_live_openai.py index 8b0544ec..b69818c6 100644 --- a/tests/live_llm/test_live_openai.py +++ b/tests/live_llm/test_live_openai.py @@ -2,10 +2,13 @@ from __future__ import annotations import pytest +from src.llm.history_adapters import OpenAIHistoryAdapter from src.llm.request_builder import execute_completion from .conftest import ( StructuredLiveResponse, + execute_local_tool, + favorite_prime_tools, make_backend, make_large_system_prompt, require_provider_key, @@ -30,6 +33,11 @@ _JSON_OBJECT_SPECS = tuple( for spec in get_live_model_specs(provider="openai") if spec.family == "openai_json_object" ) +_TOOL_REPLAY_SPECS = tuple( + spec + for spec in get_live_model_specs(provider="openai") + if spec.supports_tool_replay +) @pytest.mark.asyncio @@ -194,3 +202,83 @@ async def test_live_openai_json_object_structured_output( assert parse_calls == [] assert create_calls, "expected a chat.completions.create call" assert create_calls[0]["kwargs"]["response_format"] == {"type": "json_object"} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("model_spec", _TOOL_REPLAY_SPECS, ids=lambda spec: spec.id) +async def test_live_openai_tool_replay_preserves_null_content( + model_spec: LiveModelSpec, +) -> None: + """Tool-call turns with provider content=null must stay null through + normalize + history replay, and the continuation must still succeed.""" + require_provider_key(model_spec) + # Leave reasoning_effort unset: gpt-5.4 rejects function tools with any + # explicit reasoning_effort other than 'none' on /v1/chat/completions. + backend, config = make_backend(model_spec) + tools = favorite_prime_tools() + adapter = OpenAIHistoryAdapter() + + initial_messages = [ + { + "role": "user", + "content": ( + "Before answering, call the get_favorite_prime tool exactly once. " + "Do not answer with plain text on this turn. " + "After you receive the tool result, answer in one sentence that " + "includes the number and the word 'prime'." + ), + } + ] + + first = await execute_completion( + backend, + config, + messages=initial_messages, + max_tokens=1024, + tools=tools, + tool_choice="required", + ) + + assert first.tool_calls, "OpenAI should issue a tool call in the first turn" + raw_message = first.raw_response.choices[0].message + raw_content = raw_message.content + if raw_content is None: + assert first.content is None + else: + assert first.content == raw_content + + assistant_message = adapter.format_assistant_tool_message(first) + assert assistant_message["content"] is ( + first.content if isinstance(first.content, str) else None + ) + if raw_content is None: + assert assistant_message["content"] is None + + tool_call = first.tool_calls[0] + tool_result = execute_local_tool(tool_call.name, tool_call.input) + replay_messages = initial_messages + [ + assistant_message, + *adapter.format_tool_results( + [ + { + "tool_id": tool_call.id, + "tool_name": tool_call.name, + "result": tool_result, + } + ] + ), + ] + + second = await execute_completion( + backend, + config, + messages=replay_messages, + max_tokens=1024, + tools=tools, + tool_choice="auto", + ) + + assert not second.tool_calls, "continuation should answer without another tool call" + assert isinstance(second.content, str) + assert "13" in second.content + assert "prime" in second.content.lower() diff --git a/tests/llm/test_backends/test_openai.py b/tests/llm/test_backends/test_openai.py index fbd8e719..b4df2e66 100644 --- a/tests/llm/test_backends/test_openai.py +++ b/tests/llm/test_backends/test_openai.py @@ -1138,12 +1138,114 @@ async def test_openai_backend_structured_with_tools_uses_create_not_parse() -> N assert "strict" not in call["tools"][0]["function"] +def _tool_call_message( + *, + content: str | None, + reasoning_details: list[Any] | None = None, +) -> SimpleNamespace: + return SimpleNamespace( + content=content, + tool_calls=[ + SimpleNamespace( + id="call_probe", + function=SimpleNamespace( + name="search", + arguments='{"query":"honcho"}', + ), + ) + ], + reasoning_details=reasoning_details or [], + reasoning_content=None, + ) + + +def _completion_response( + message: SimpleNamespace, finish_reason: str +) -> SimpleNamespace: + return SimpleNamespace( + choices=[SimpleNamespace(finish_reason=finish_reason, message=message)], + usage=SimpleNamespace( + prompt_tokens=10, + completion_tokens=5, + prompt_tokens_details=None, + ), + ) + + +def test_openai_normalize_preserves_null_content_on_tool_call_turns() -> None: + reasoning_details = [ + { + "type": "reasoning.encrypted", + "data": "opaque", + "format": "openai-responses-v1", + "id": "binding", + "index": 0, + } + ] + response = _completion_response( + _tool_call_message(content=None, reasoning_details=reasoning_details), + "tool_calls", + ) + + result = OpenAIBackend(Mock())._normalize_response( # pyright: ignore[reportPrivateUsage] + response + ) + + assert result.content is None + assert result.tool_calls[0].id == "call_probe" + assert result.tool_calls[0].name == "search" + assert result.tool_calls[0].input == {"query": "honcho"} + assert result.reasoning_details == reasoning_details + + +def test_openai_normalize_coerces_null_content_without_tool_calls() -> None: + message = SimpleNamespace( + content=None, + tool_calls=[], + reasoning_details=[], + reasoning_content=None, + ) + response = _completion_response(message, "stop") + + result = OpenAIBackend(Mock())._normalize_response( # pyright: ignore[reportPrivateUsage] + response + ) + + assert result.content == "" + + +def test_openai_normalize_keeps_empty_string_content_on_tool_call_turns() -> None: + response = _completion_response( + _tool_call_message(content=""), + "tool_calls", + ) + + result = OpenAIBackend(Mock())._normalize_response( # pyright: ignore[reportPrivateUsage] + response + ) + + assert result.content == "" + + +def test_openai_normalize_content_override_is_authoritative() -> None: + response = _completion_response( + _tool_call_message(content=None), + "tool_calls", + ) + + result = OpenAIBackend(Mock())._normalize_response( # pyright: ignore[reportPrivateUsage] + response, content_override="override" + ) + + assert result.content == "override" + + @pytest.mark.asyncio async def test_openai_backend_structured_with_tools_skips_parsing_tool_call_turn() -> ( None ): """A tool-call turn under tools + response_format must not attempt JSON - parsing (its content is empty and _parse_or_repair raises on that).""" + parsing (provider content is null and _parse_or_repair raises on that).""" client = Mock() client.chat.completions.create = AsyncMock( return_value=SimpleNamespace( @@ -1183,7 +1285,7 @@ async def test_openai_backend_structured_with_tools_skips_parsing_tool_call_turn response_format=_StructuredResponse, ) - assert result.content == "" # raw empty text, not a parsed model + assert result.content is None # provider null, not a parsed model assert result.tool_calls[0].name == "search" assert result.tool_calls[0].input == {"query": "honcho"} diff --git a/tests/llm/test_history_adapters.py b/tests/llm/test_history_adapters.py index a992398d..343cee9c 100644 --- a/tests/llm/test_history_adapters.py +++ b/tests/llm/test_history_adapters.py @@ -113,3 +113,31 @@ def test_openai_history_adapter_omits_empty_thinking_content( message = adapter.format_assistant_tool_message(result) assert "reasoning_content" not in message + + +def test_openai_history_adapter_preserves_null_content_on_tool_call_turns() -> None: + adapter = OpenAIHistoryAdapter() + reasoning_details = [ + { + "type": "reasoning.encrypted", + "data": "opaque", + "format": "openai-responses-v1", + "id": "binding", + "index": 0, + } + ] + result = CompletionResult( + content=None, + reasoning_details=reasoning_details, + tool_calls=[ + ToolCallResult(id="call_probe", name="search", input={"query": "honcho"}) + ], + ) + + message = adapter.format_assistant_tool_message(result) + + assert message["content"] is None + assert message["reasoning_details"] == reasoning_details + assert message["tool_calls"][0]["id"] == "call_probe" + assert message["tool_calls"][0]["function"]["name"] == "search" + assert message["tool_calls"][0]["function"]["arguments"] == '{"query": "honcho"}' diff --git a/tests/llm/test_tool_loop_reasoning_content.py b/tests/llm/test_tool_loop_reasoning_content.py index cd4ec91b..53b65fe2 100644 --- a/tests/llm/test_tool_loop_reasoning_content.py +++ b/tests/llm/test_tool_loop_reasoning_content.py @@ -102,3 +102,81 @@ async def test_tool_loop_replays_reasoning_content_on_continuation() -> None: "tool_call_id": "call_1", "content": "result", } + + +@pytest.mark.asyncio +async def test_tool_loop_replays_null_assistant_content_on_continuation() -> None: + calls: list[list[dict[str, Any]]] = [] + responses = iter( + [ + HonchoLLMCallResponse( + content=None, + output_tokens=5, + finish_reasons=["tool_calls"], + tool_calls_made=[ + { + "id": "call_1", + "name": "search", + "input": {"query": "honcho"}, + } + ], + reasoning_details=[ + { + "type": "reasoning.encrypted", + "data": "opaque", + "format": "openai-responses-v1", + "id": "binding", + "index": 0, + } + ], + ), + HonchoLLMCallResponse( + content="done", + output_tokens=3, + finish_reasons=["stop"], + tool_calls_made=[], + ), + ] + ) + + async def fake_call(*_args: Any, **kwargs: Any) -> HonchoLLMCallResponse[Any]: + calls.append(deepcopy(kwargs["messages"])) + return next(responses) + + async def execute_search(_name: str, _input: dict[str, Any]) -> str: + return "result" + + with patch.object(tool_loop, "honcho_llm_call_inner", new=fake_call): + result = await execute_tool_loop( + prompt="hi", + max_tokens=64, + messages=[{"role": "user", "content": "hi"}], + tools=[ + { + "name": "search", + "description": "Search", + "input_schema": {"type": "object"}, + } + ], + tool_choice="auto", + tool_executor=execute_search, + max_tool_iterations=5, + response_model=None, + json_mode=False, + temperature=None, + stop_seqs=None, + verbosity=None, + enable_retry=False, + retry_attempts=1, + max_input_tokens=None, + get_attempt_plan=_make_plan, + before_retry_callback=lambda _retry_state: None, + stream_final=False, + telemetry=None, + ) + + assert isinstance(result, HonchoLLMCallResponse) + assert len(calls) == 2 + assert calls[1][1]["content"] is None + assert calls[1][1]["reasoning_details"][0]["data"] == "opaque" + assert calls[1][1]["tool_calls"][0]["function"]["name"] == "search"