From 944212b78b2cc213e44a1e2865d6414ead16d06a Mon Sep 17 00:00:00 2001 From: LIKIQ Date: Fri, 14 Aug 2026 14:09:36 +0800 Subject: [PATCH] fix(provider): fall back when upstream omits tool_call id Some OpenAI-compatible upstreams (Gemini's compatible endpoint, several proxy gateways) return tool_calls without an id. The openai SDK deserializes responses leniently, so the required field silently becomes None instead of raising. Streaming responses can additionally grow a ghost tool_call carrying a null id when the upstream numbers tool_call.index from 1, because the SDK uses that index as a list subscript and inserts a second, argument-only entry. Either way the whole agent turn died with `ValidationError: 1 validation error for ToolCall` while assembling the assistant message, i.e. after the tool had already been executed, so users saw neither the tool result nor an answer. - normalize streaming tool_call indexes to a contiguous 0-based sequence, which also stops the real arguments from being accumulated onto a ghost entry - fall back to a deterministic placeholder id when the upstream omits one - normalize ids in the agent runner as well, so the executed tool result and the assistant tool call always reference the same id as the OpenAI spec requires (only patching the serializer would orphan the tool message) - make LLMResponse.to_openai_tool_calls* index-safe and null-safe, since FunctionBody.name raises the very same error when it is None refs #9590 --- .../agent/runners/tool_loop_agent_runner.py | 52 +++++- astrbot/core/provider/entities.py | 62 +++++-- .../core/provider/sources/openai_source.py | 43 ++++- tests/test_openai_source.py | 159 ++++++++++++++++++ tests/test_tool_loop_agent_runner.py | 143 ++++++++++++++++ 5 files changed, 441 insertions(+), 18 deletions(-) diff --git a/astrbot/core/agent/runners/tool_loop_agent_runner.py b/astrbot/core/agent/runners/tool_loop_agent_runner.py index 8c91adbbfd..4ba6142a27 100644 --- a/astrbot/core/agent/runners/tool_loop_agent_runner.py +++ b/astrbot/core/agent/runners/tool_loop_agent_runner.py @@ -36,10 +36,14 @@ from astrbot.core.persona_error_reply import ( extract_persona_custom_error_message_from_event, ) +from astrbot.core.provider.entities import ( + MALFORMED_TOOL_NAME_PLACEHOLDER as _MALFORMED_TOOL_NAME_PLACEHOLDER, +) from astrbot.core.provider.entities import ( LLMResponse, ProviderRequest, ToolCallsResult, + fallback_tool_call_id, ) from astrbot.core.provider.modalities import ( log_context_sanitize_stats, @@ -144,7 +148,7 @@ class ToolLoopAgentRunner(BaseAgentRunner[TContext]): REPEATED_TOOL_NOTICE_L1_THRESHOLD = 3 REPEATED_TOOL_NOTICE_L2_THRESHOLD = 4 REPEATED_TOOL_NOTICE_L3_THRESHOLD = 5 - MALFORMED_TOOL_NAME_PLACEHOLDER = "__malformed_tool_name__" + MALFORMED_TOOL_NAME_PLACEHOLDER = _MALFORMED_TOOL_NAME_PLACEHOLDER REPEATED_TOOL_NOTICE_L1_TEMPLATE = ( "\n\n[SYSTEM NOTICE] By the way, you have executed the same tool " "`{tool_name}` with the same arguments {streak} times consecutively. " @@ -771,7 +775,15 @@ def _sanitize_malformed_tool_calls( self, llm_resp: LLMResponse, ) -> None: - """Normalize malformed tool call names. + """Normalize malformed tool call names and ids. + + Some OpenAI-compatible upstreams return tool calls without a usable + ``name`` or ``id`` (refs: AstrBot#8911 / AstrBot#9590). ``ToolCall.id`` and + ``ToolCall.FunctionBody.name`` are both required strings, so leaving them + as ``None`` blows up later when the assistant message is assembled. + Normalizing here — the single funnel every provider response passes + through — guarantees the executed tool result and the assistant tool call + share the very same id, which the OpenAI spec requires. Args: llm_resp: The LLM response whose tool call lists should be sanitized. @@ -783,6 +795,42 @@ def _sanitize_malformed_tool_calls( for tool_name in llm_resp.tools_call_name ] + total = max(len(llm_resp.tools_call_name), len(llm_resp.tools_call_args)) + original_ids = list(llm_resp.tools_call_ids) + ids: list[str | None] = original_ids[:total] + # pad so that zip() in _handle_function_tools never silently truncates + ids.extend([None] * (total - len(ids))) + + seen: set[str] = set() + normalized_ids: list[str] = [] + for idx, call_id in enumerate(ids): + if isinstance(call_id, str) and call_id.strip(): + candidate = call_id + else: + candidate = fallback_tool_call_id(idx) + if candidate in seen: + # extremely unlikely, but a placeholder must never collide with a + # real id, otherwise tool results get paired with the wrong call + candidate = f"{candidate}_{uuid.uuid4().hex[:6]}" + seen.add(candidate) + normalized_ids.append(candidate) + if ( + candidate != call_id + and isinstance(call_id, str) + and call_id in llm_resp.tools_call_extra_content + ): + # keep provider-specific payloads reachable under the new id + # (e.g. gemini thought_signature) + llm_resp.tools_call_extra_content[candidate] = ( + llm_resp.tools_call_extra_content[call_id] + ) + + if normalized_ids != original_ids: + logger.warning( + f"Normalized malformed tool call ids: {original_ids} -> {normalized_ids}" + ) + llm_resp.tools_call_ids = normalized_ids + @override async def step(self): """Process a single step of the agent. diff --git a/astrbot/core/provider/entities.py b/astrbot/core/provider/entities.py index 2fab40ca78..c7826d5dca 100644 --- a/astrbot/core/provider/entities.py +++ b/astrbot/core/provider/entities.py @@ -25,6 +25,26 @@ from astrbot.core.message.message_event_result import MessageChain from astrbot.core.utils.media_utils import MediaResolver +MALFORMED_TOOL_NAME_PLACEHOLDER = "__malformed_tool_name__" +"""Placeholder used when the upstream returns a tool call without a usable name.""" + + +def fallback_tool_call_id(idx: int) -> str: + """Build a deterministic placeholder id for a tool call missing its ``id``. + + Some OpenAI-compatible upstreams (Gemini's compatible endpoint, several + proxy gateways) omit ``tool_calls[].id``. The OpenAI spec still requires the + following ``role="tool"`` message to reference the very same id, so both the + provider layer and the agent runner must derive the same placeholder. + + Args: + idx: The position of the tool call within the current response. + + Returns: + A deterministic placeholder tool call id. + """ + return f"call_{idx}" + class ProviderType(enum.Enum): CHAT_COMPLETION = "chat_completion" @@ -403,23 +423,44 @@ def completion_text(self, value) -> None: else: self._completion_text = value + def _safe_tool_call_id(self, idx: int) -> str: + """Return a valid tool call id for ``idx``. + + Falls back to :func:`fallback_tool_call_id` when the upstream omitted the + id or the id list is shorter than the argument list. + """ + call_id = self.tools_call_ids[idx] if idx < len(self.tools_call_ids) else None + if isinstance(call_id, str) and call_id.strip(): + return call_id + return fallback_tool_call_id(idx) + + def _safe_tool_call_name(self, idx: int) -> str: + """Return a valid tool call name for ``idx``. + + ``ToolCall.FunctionBody.name`` is a required string, so a ``None`` name + would raise the very same validation error as a missing id. + """ + name = self.tools_call_name[idx] if idx < len(self.tools_call_name) else None + if isinstance(name, str) and name.strip(): + return name + return MALFORMED_TOOL_NAME_PLACEHOLDER + @deprecated(reason="Use to_openai_tool_calls_model instead.") def to_openai_tool_calls(self) -> list[dict]: """Convert to OpenAI tool calls format. Deprecated, use to_openai_tool_calls_model instead.""" ret = [] for idx, tool_call_arg in enumerate(self.tools_call_args): + call_id = self._safe_tool_call_id(idx) payload = { - "id": self.tools_call_ids[idx], + "id": call_id, "function": { - "name": self.tools_call_name[idx], + "name": self._safe_tool_call_name(idx), "arguments": json.dumps(tool_call_arg), }, "type": "function", } - if self.tools_call_extra_content.get(self.tools_call_ids[idx]): - payload["extra_content"] = self.tools_call_extra_content[ - self.tools_call_ids[idx] - ] + if self.tools_call_extra_content.get(call_id): + payload["extra_content"] = self.tools_call_extra_content[call_id] ret.append(payload) return ret @@ -427,17 +468,16 @@ def to_openai_tool_calls_model(self) -> list[ToolCall]: """The same as to_openai_tool_calls but return pydantic model.""" ret = [] for idx, tool_call_arg in enumerate(self.tools_call_args): + call_id = self._safe_tool_call_id(idx) ret.append( ToolCall( - id=self.tools_call_ids[idx], + id=call_id, function=ToolCall.FunctionBody( - name=self.tools_call_name[idx], + name=self._safe_tool_call_name(idx), arguments=json.dumps(tool_call_arg), ), # the extra_content will not serialize if it's None when calling ToolCall.model_dump() - extra_content=self.tools_call_extra_content.get( - self.tools_call_ids[idx] - ), + extra_content=self.tools_call_extra_content.get(call_id), ), ) return ret diff --git a/astrbot/core/provider/sources/openai_source.py b/astrbot/core/provider/sources/openai_source.py index f7870b7137..57faab163e 100644 --- a/astrbot/core/provider/sources/openai_source.py +++ b/astrbot/core/provider/sources/openai_source.py @@ -28,7 +28,12 @@ from astrbot.core.agent.tool import ToolSet from astrbot.core.exceptions import EmptyModelOutputError from astrbot.core.message.message_event_result import MessageChain -from astrbot.core.provider.entities import LLMResponse, TokenUsage, ToolCallsResult +from astrbot.core.provider.entities import ( + LLMResponse, + TokenUsage, + ToolCallsResult, + fallback_tool_call_id, +) from astrbot.core.utils.media_utils import ( describe_media_ref, resolve_media_ref_to_base64_data, @@ -637,6 +642,11 @@ async def _query_stream( llm_response = LLMResponse("assistant", is_chunk=True) state = ChatCompletionStreamState() + # 上游返回的 tool_call.index 可能从 1 开始、乱序或缺失，而 openai SDK 直接把它 + # 当成 tool_calls 列表的下标使用（_build_events / accumulate_delta），一旦错位就会 + # insert 出一个只有 arguments、没有 id / name 的幽灵 tool_call（refs: AstrBot#9590）。 + # 这里按出现顺序把它重映射成从 0 开始的连续序号，正常上游是恒等映射。 + tool_call_index_map: dict[int, int] = {} async for chunk in stream: choice = chunk.choices[0] if chunk.choices else None @@ -649,8 +659,18 @@ async def _query_stream( tc.type = "function" # Fix for #6661: Add missing 'index' field to tool_call deltas # Gemini and some OpenAI-compatible proxies omit this field - if not hasattr(tc, "index") or tc.index is None: - tc.index = idx + raw_index = getattr(tc, "index", None) + if raw_index is None: + raw_index = idx + # Fix for #9590: normalize 1-based / non-contiguous indexes + if raw_index not in tool_call_index_map: + tool_call_index_map[raw_index] = len(tool_call_index_map) + normalized_index = tool_call_index_map[raw_index] + if normalized_index != raw_index: + logger.debug( + f"normalize tool_call index {raw_index} -> {normalized_index}" + ) + tc.index = normalized_index # 跳过 delta=None 的 chunk，避免 SDK 内部 _convert_initial_chunk_into_snapshot # 第 747 行 choice.delta.to_dict() 抛出 NoneType 错误。 # refs: AstrBot#6689 / openai-python#5069 / #5047 @@ -907,12 +927,25 @@ async def _parse_openai_completion( args = {} args_ls.append(args) func_name_ls.append(tool_call.function.name) - tool_call_ids.append(tool_call.id) + # Some OpenAI-compatible upstreams omit tool_calls[].id. + # The openai SDK deserializes responses leniently, so the + # required field silently becomes None instead of raising. + raw_call_id = getattr(tool_call, "id", None) + if isinstance(raw_call_id, str) and raw_call_id.strip(): + call_id = raw_call_id + else: + # keep the placeholder aligned with the final list index so + # that the agent runner and the assistant message agree on it + call_id = fallback_tool_call_id(len(tool_call_ids)) + logger.warning( + f"上游未返回 tool_call.id，回退为 {call_id}（tool={tool_call.function.name}）" + ) + tool_call_ids.append(call_id) # gemini-2.5 / gemini-3 series extra_content handling extra_content = getattr(tool_call, "extra_content", None) if extra_content is not None: - tool_call_extra_content_dict[tool_call.id] = extra_content + tool_call_extra_content_dict[call_id] = extra_content llm_response.role = "tool" llm_response.tools_call_args = args_ls diff --git a/tests/test_openai_source.py b/tests/test_openai_source.py index 911b76131f..f931a4dd11 100644 --- a/tests/test_openai_source.py +++ b/tests/test_openai_source.py @@ -11,6 +11,7 @@ import astrbot.core.provider.sources.openai_source as openai_source_module import astrbot.core.provider.sources.request_retry as request_retry +from astrbot.core.agent.tool import ToolSet from astrbot.core.exceptions import EmptyModelOutputError from astrbot.core.provider.entities import LLMResponse from astrbot.core.provider.sources.groq_source import ProviderGroq @@ -2201,3 +2202,161 @@ async def fake_create(**kwargs): assert messages[1] == {"role": "user", "content": "again"} finally: await provider.terminate() + + +def _make_tool_call_chunk(tool_call_delta: dict) -> ChatCompletionChunk: + return ChatCompletionChunk.model_validate( + { + "id": "chatcmpl-toolcall", + "object": "chat.completion.chunk", + "created": 0, + "model": "gpt-4o-mini", + "choices": [ + { + "index": 0, + "delta": {"role": "assistant", "tool_calls": [tool_call_delta]}, + "finish_reason": None, + } + ], + } + ) + + +def _make_finish_chunk(finish_reason: str = "tool_calls") -> ChatCompletionChunk: + return ChatCompletionChunk.model_validate( + { + "id": "chatcmpl-toolcall", + "object": "chat.completion.chunk", + "created": 0, + "model": "gpt-4o-mini", + "choices": [ + {"index": 0, "delta": {}, "finish_reason": finish_reason}, + ], + } + ) + + +async def _collect_stream_responses( + provider: ProviderOpenAIOfficial, + monkeypatch, + chunks: list[ChatCompletionChunk], +) -> list[LLMResponse]: + async def fake_stream(): + for chunk in chunks: + yield chunk + + async def fake_create(**kwargs): + return fake_stream() + + monkeypatch.setattr(provider.client.chat.completions, "create", fake_create) + + return [ + response + async for response in provider._query_stream( + payloads={ + "model": "gpt-4o-mini", + "messages": [{"role": "user", "content": "天气"}], + }, + tools=ToolSet(), + ) + ] + +@pytest.mark.asyncio +async def test_query_stream_normalizes_one_based_tool_call_index(monkeypatch): + """上游 index 从 1 开始时不应产生 id=None 的幽灵 tool_call（refs #9590）。""" + provider = _make_provider() + try: + chunks = [ + _make_tool_call_chunk( + { + "index": 1, + "id": "call_abc", + "type": "function", + "function": {"name": "get_weather", "arguments": ""}, + } + ), + _make_tool_call_chunk( + {"index": 1, "function": {"arguments": '{"city": '}} + ), + _make_tool_call_chunk({"index": 1, "function": {"arguments": '"北京"}'}}), + _make_finish_chunk(), + ] + + responses = await _collect_stream_responses(provider, monkeypatch, chunks) + + final_response = responses[-1] + assert final_response.tools_call_ids == ["call_abc"] + assert final_response.tools_call_name == ["get_weather"] + assert final_response.tools_call_args == [{"city": "北京"}] + tool_calls = final_response.to_openai_tool_calls_model() + assert [tc.id for tc in tool_calls] == ["call_abc"] + finally: + await provider.terminate() + + +@pytest.mark.asyncio +async def test_query_stream_falls_back_when_tool_call_id_missing(monkeypatch): + """上游完全不返回 tool_call.id 时应回退到占位 id 而非 None。""" + provider = _make_provider() + try: + chunks = [ + _make_tool_call_chunk( + { + "index": 0, + "type": "function", + "function": {"name": "get_weather", "arguments": '{"city":'}, + } + ), + _make_tool_call_chunk({"index": 0, "function": {"arguments": '"上海"}'}}), + _make_finish_chunk(), + ] + + responses = await _collect_stream_responses(provider, monkeypatch, chunks) + + final_response = responses[-1] + assert final_response.tools_call_ids == ["call_0"] + assert final_response.tools_call_args == [{"city": "上海"}] + assert final_response.to_openai_tool_calls_model()[0].id == "call_0" + finally: + await provider.terminate() + + +@pytest.mark.asyncio +async def test_parse_openai_completion_falls_back_when_tool_call_id_missing(): + """非流式响应缺 tool_call.id 时不应把 None 带进 ToolCall 模型。""" + provider = _make_provider() + try: + completion = ChatCompletion.model_construct( + id="chatcmpl-noid", + object="chat.completion", + created=0, + model="gemini-3-pro", + choices=[ + { + "index": 0, + "message": { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "type": "function", + "function": { + "name": "get_weather", + "arguments": '{"city": "广州"}', + }, + } + ], + }, + "finish_reason": "tool_calls", + } + ], + ) + + response = await provider._parse_openai_completion(completion, tools=ToolSet()) + + assert response.tools_call_ids == ["call_0"] + assert response.tools_call_name == ["get_weather"] + assert response.tools_call_args == [{"city": "广州"}] + assert response.to_openai_tool_calls_model()[0].id == "call_0" + finally: + await provider.terminate() diff --git a/tests/test_tool_loop_agent_runner.py b/tests/test_tool_loop_agent_runner.py index 1e679de4aa..59b3fd2892 100644 --- a/tests/test_tool_loop_agent_runner.py +++ b/tests/test_tool_loop_agent_runner.py @@ -2051,6 +2051,149 @@ async def test_follow_up_after_stop_not_merged_into_tool_result( assert ticket_before.resolved.is_set() +class MissingToolCallIdProvider(Provider): + """模拟上游不返回 tool_call.id 的 Provider（refs #9590）。""" + + def __init__(self): + super().__init__({}, {}) + self.call_count = 0 + + def get_current_key(self) -> str: + return "test_key" + + def set_key(self, key: str): + pass + + async def get_models(self) -> list[str]: + return ["test_model"] + + async def text_chat(self, **kwargs) -> LLMResponse: + self.call_count += 1 + if self.call_count == 1: + return LLMResponse( + role="assistant", + completion_text="", + tools_call_name=["test_tool"], + tools_call_args=[{"query": "test"}], + tools_call_ids=[None], # type: ignore[list-item] + usage=TokenUsage(input_other=10, output=5), + ) + return LLMResponse( + role="assistant", + completion_text="这是我的最终回答", + usage=TokenUsage(input_other=10, output=5), + ) + + async def text_chat_stream(self, **kwargs): + response = await self.text_chat(**kwargs) + response.is_chunk = True + yield response + response.is_chunk = False + yield response + + +def test_sanitize_malformed_tool_calls_fills_missing_ids(runner): + """缺失/空白的 tool_call id 必须被回退成确定性占位 id。""" + resp = LLMResponse( + role="tool", + completion_text="", + tools_call_name=["tool_a", "tool_b"], + tools_call_args=[{}, {}], + tools_call_ids=[None, " "], # type: ignore[list-item] + ) + + runner._sanitize_malformed_tool_calls(resp) + + assert resp.tools_call_ids == ["call_0", "call_1"] + # 关键：不再抛 ValidationError + assert [tc.id for tc in resp.to_openai_tool_calls_model()] == ["call_0", "call_1"] + + +def test_sanitize_malformed_tool_calls_pads_missing_ids(runner): + """id 列表比工具列表短时必须补齐，避免 zip 静默丢工具。""" + resp = LLMResponse( + role="tool", + completion_text="", + tools_call_name=["tool_a", "tool_b"], + tools_call_args=[{}, {}], + tools_call_ids=[], + ) + + runner._sanitize_malformed_tool_calls(resp) + + assert resp.tools_call_ids == ["call_0", "call_1"] + + +def test_sanitize_malformed_tool_calls_avoids_id_collision(runner): + """占位 id 与上游真实 id 撞车时必须去重。""" + resp = LLMResponse( + role="tool", + completion_text="", + tools_call_name=["tool_a", "tool_b"], + tools_call_args=[{}, {}], + tools_call_ids=[None, "call_0"], # type: ignore[list-item] + ) + + runner._sanitize_malformed_tool_calls(resp) + + assert resp.tools_call_ids[0] == "call_0" + assert resp.tools_call_ids[1] != "call_0" + assert resp.tools_call_ids[1].startswith("call_0_") + + +def test_sanitize_malformed_tool_calls_keeps_valid_ids(runner): + """上游给了合法 id 时不得改写。""" + resp = LLMResponse( + role="tool", + completion_text="", + tools_call_name=["tool_a"], + tools_call_args=[{"query": "x"}], + tools_call_ids=["call_upstream_abc"], + ) + + runner._sanitize_malformed_tool_calls(resp) + + assert resp.tools_call_ids == ["call_upstream_abc"] + + +@pytest.mark.asyncio +async def test_tool_call_id_none_does_not_break_step( + runner, provider_request, mock_tool_executor, mock_hooks +): + """tool_call.id 为 None 时整轮不应失败，且 assistant/tool 消息 id 必须配对。""" + provider = MissingToolCallIdProvider() + + await runner.reset( + provider=provider, + request=provider_request, + run_context=ContextWrapper(context=None), + tool_executor=mock_tool_executor, + agent_hooks=mock_hooks, + streaming=False, + ) + + responses = [] + async for response in runner.step_until_done(3): + responses.append(response) + + assert [r for r in responses if r.type == "err"] == [] + + assistant_messages = [ + msg + for msg in runner.run_context.messages + if msg.role == "assistant" and msg.tool_calls + ] + assert len(assistant_messages) == 1 + tool_calls = assistant_messages[0].tool_calls + assert tool_calls is not None + assert tool_calls[0].id == "call_0" + + tool_messages = [msg for msg in runner.run_context.messages if msg.role == "tool"] + assert tool_messages + # OpenAI 硬约束：tool 结果消息必须引用同一个 tool_call_id + assert tool_messages[0].tool_call_id == tool_calls[0].id + + if __name__ == "__main__": # 运行测试 pytest.main([__file__, "-v"])