Skip to content

fix(provider): fall back when upstream omits tool_call id #9681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LIKIQ wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(provider): fall back when upstream omits tool_call id #9681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions astrbot/core/agent/runners/tool_loop_agent_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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. "
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
62 changes: 51 additions & 11 deletions astrbot/core/provider/entities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -403,41 +423,61 @@ 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

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
Expand Down
43 changes: 38 additions & 5 deletions astrbot/core/provider/sources/openai_source.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -637,6 +642,11 @@ async def _query_stream(
llm_response = LLMResponse("assistant", is_chunk=True)

state = ChatCompletionStreamState()
# 上游返回的 tool_call.index 可能从 1 开始、乱序或缺失，而 openai SDK 直接把它

@sourcery-ai sourcery-ai Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting tool_call index normalization and ID fallback into dedicated helper methods to keep the hot-path streaming and parsing logic focused and easier to read.

The new logic is functionally solid, but both index normalization and ID fallback are tightly inlined into hot paths. You can reduce cognitive load by pushing these concerns into small helpers that encapsulate state, while keeping behavior identical.

1. Extract tool_call index normalization into a helper

Instead of mutating tool_call_index_map inline inside the streaming loop, encapsulate the mapping in a tiny helper or state object:

# near ChatCompletionStreamState

class ToolCallIndexNormalizer:
    def __init__(self) -> None:
        # raw_index -> normalized_index
        self._map: dict[int, int] = {}

    def normalize(self, raw_index: int) -> int:
        if raw_index not in self._map:
            self._map[raw_index] = len(self._map)
        normalized_index = self._map[raw_index]
        if normalized_index != raw_index:
            logger.debug(
                f"normalize tool_call index {raw_index} -> {normalized_index}"
            )
        return normalized_index

Then _query_stream becomes:

llm_response = LLMResponse("assistant", is_chunk=True)
state = ChatCompletionStreamState()
index_normalizer = ToolCallIndexNormalizer()

async for chunk in stream:
    choice = chunk.choices[0] if chunk.choices else None
    delta = choice.delta if choice else None

    if delta and (dtcs := delta.tool_calls):
        for idx, tc in enumerate(dtcs):
            if tc.function and tc.function.arguments:
                tc.type = "function"

            raw_index = getattr(tc, "index", None)
            if raw_index is None:
                raw_index = idx

            tc.index = index_normalizer.normalize(raw_index)

    ...

This keeps the streaming loop focused on “what” is happening, and hides the “how” of normalization.

2. Centralize tool_call id normalization

You’ve added fallback_tool_call_id, and the reviewer notes LLMResponse._safe_tool_call_id already exists. Right now, _parse_openai_completion has its own ID normalization; you can delegate ID normalization to LLMResponse and avoid duplicating responsibility.

For example, keep _parse_openai_completion collecting raw IDs (including None), and let LLMResponse normalize when constructing the final response:

# in _parse_openai_completion
raw_call_id = getattr(tool_call, "id", None)
tool_call_ids.append(raw_call_id)

extra_content = getattr(tool_call, "extra_content", None)
if extra_content is not None:
    # temporarily key by raw id; will be remapped by LLMResponse
    tool_call_extra_content_dict[raw_call_id] = extra_content

Then in LLMResponse:

class LLMResponse:
    ...

    def _safe_tool_call_id(self, raw_id: str | None, index: int) -> str:
        if isinstance(raw_id, str) and raw_id.strip():
            return raw_id
        return fallback_tool_call_id(index)

    def finalize_tool_calls(self) -> None:
        if not self.tools_call_ids:
            return

        normalized_ids: list[str] = []
        normalized_extra: dict[str, Any] = {}

        for idx, raw_id in enumerate(self.tools_call_ids):
            call_id = self._safe_tool_call_id(raw_id, idx)
            normalized_ids.append(call_id)

            extra = self.tools_call_extra_content.get(raw_id)
            if extra is not None:
                normalized_extra[call_id] = extra

        self.tools_call_ids = normalized_ids
        self.tools_call_extra_content = normalized_extra

Call llm_response.finalize_tool_calls() at the end of _parse_openai_completion. This keeps all ID fallback logic (including alignment with list index and extra_content mapping) in one place, and removes the need for inline getattr + warning + fallback_tool_call_id in _parse_openai_completion, while preserving the current behavior.

# 当成 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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading