fix(openai): fix content normalization in openai backend history adapter (#1064)
* fix(llm): preserve null content on OpenAI tool-call turns OpenAI-compatible providers can return assistant tool-call messages with content=null. Coercing that to "" before history replay breaks providers that bind opaque reasoning state to the exact assistant message shape. Keep null only when the normalized response has tool calls; tool-less null still becomes "", and content_override stays authoritative. Fixes #1061 * test(live_llm): cover OpenAI null content tool-call replay Add a live multi-turn tool replay that asserts provider content=null stays null through normalize + OpenAIHistoryAdapter and that the continuation still answers. Mark gpt_4/gpt_5 families as supports_tool_replay. * docs(llm): note content_override None sentinel semantics None means no override, not force-null content. Addresses review on #1064.
This commit is contained in:
parent
370232e139
commit
dea2917fa9
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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"}
|
||||
|
||||
|
|
|
|||
|
|
@ -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"}'
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Reference in New Issue