mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-01 19:06:01 +00:00
fix(middleware): drop orphan ToolMessages so strict providers don't 400 (#4080)
* fix(middleware): drop orphan ToolMessages with no matching AIMessage tool_call The rebuild loop only skipped ToolMessages whose tool_call_id matched a known AIMessage tool_call (to be re-emitted after it). An orphan ToolMessage whose tool_call_id has no matching AIMessage tool_calls fell through and was kept, leaving a dangling tool result that strict providers reject. Drop orphan ToolMessages as well, logging at debug. * fix(dangling): demote orphan-drop logs, add tool_call_id=None test - Update module/class docstrings to mention orphan ToolMessage handling - Accumulate orphan drop_count and emit a single logger.warning instead of per-message logger.debug calls - Simplify early-return logic: return None only when no patching AND no orphans were dropped - Add test_tool_call_id_none_orphan_is_dropped — a ToolMessage with tool_call_id=None is always an orphan and must be dropped Closes #4080 Co-Authored-By: Claude <noreply@anthropic.com> * fix(test): use model_construct for None tool_call_id test to bypass pydantic validation ToolMessage content='ghost' tool_call_id=None fails pydantic validation at construction. Use model_construct to simulate a corrupt/edge-case payload without tripping the string-only guard. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
cdefd4a8aa
commit
9c77046d10
@ -1,12 +1,16 @@
|
|||||||
"""Middleware to fix dangling tool calls in message history.
|
"""Middleware to fix dangling tool calls and orphan tool results in message history.
|
||||||
|
|
||||||
A dangling tool call occurs when an AIMessage contains tool_calls but there are
|
A dangling tool call occurs when an AIMessage contains tool_calls but there are
|
||||||
no corresponding ToolMessages in the history (e.g., due to user interruption or
|
no corresponding ToolMessages in the history (e.g., due to user interruption or
|
||||||
request cancellation). This causes LLM errors due to incomplete message format.
|
request cancellation). An orphan ToolMessage occurs when a tool result exists
|
||||||
|
without a matching AIMessage tool_call (e.g., after summarization/branching
|
||||||
|
dropped the upstream AIMessage). Both cause strict-provider rejections.
|
||||||
|
|
||||||
This middleware intercepts the model call to detect and patch such gaps by
|
This middleware intercepts the model call to:
|
||||||
inserting synthetic ToolMessages with an error indicator immediately after the
|
- Insert synthetic ToolMessages with an error indicator for each dangling AIMessage
|
||||||
AIMessage that made the tool calls, ensuring correct message ordering.
|
tool_call, ensuring correct message ordering
|
||||||
|
- Drop orphan ToolMessages whose originating tool_call is no longer present in the
|
||||||
|
request, preventing strict OpenAI-compatible backends from returning HTTP 400
|
||||||
|
|
||||||
Note: Uses wrap_model_call instead of before_model to ensure patches are inserted
|
Note: Uses wrap_model_call instead of before_model to ensure patches are inserted
|
||||||
at the correct positions (immediately after each dangling AIMessage), not appended
|
at the correct positions (immediately after each dangling AIMessage), not appended
|
||||||
@ -47,11 +51,14 @@ def _has_invalid_tool_name(name: object) -> bool:
|
|||||||
|
|
||||||
|
|
||||||
class DanglingToolCallMiddleware(AgentMiddleware[AgentState]):
|
class DanglingToolCallMiddleware(AgentMiddleware[AgentState]):
|
||||||
"""Inserts placeholder ToolMessages for dangling tool calls before model invocation.
|
"""Inserts placeholder ToolMessages for dangling tool calls and drops orphan
|
||||||
|
ToolMessages (tool results whose originating AIMessage tool_call is gone).
|
||||||
|
|
||||||
Scans the message history for AIMessages whose tool_calls lack corresponding
|
Scans the message history for:
|
||||||
ToolMessages, and injects synthetic error responses immediately after the
|
- AIMessages whose tool_calls lack corresponding ToolMessages, and injects
|
||||||
offending AIMessage so the LLM receives a well-formed conversation.
|
synthetic error responses immediately after the offending AIMessage
|
||||||
|
- ToolMessages with no matching AIMessage tool_call (orphans), and drops
|
||||||
|
them so strict OpenAI-compatible backends do not reject the request
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
@ -238,8 +245,17 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]):
|
|||||||
|
|
||||||
patched: list = []
|
patched: list = []
|
||||||
patch_count = 0
|
patch_count = 0
|
||||||
|
drop_count = 0
|
||||||
for msg in messages:
|
for msg in messages:
|
||||||
if isinstance(msg, ToolMessage) and msg.tool_call_id in tool_call_ids:
|
if isinstance(msg, ToolMessage):
|
||||||
|
if msg.tool_call_id in tool_call_ids:
|
||||||
|
continue # Will be re-emitted after its AIMessage
|
||||||
|
# Orphan: ToolMessage whose originating AIMessage tool_call is
|
||||||
|
# no longer in the request (e.g. removed by summarization).
|
||||||
|
# Drop it silently from the model request so strict providers
|
||||||
|
# do not reject it with HTTP 400. Persisted state is untouched;
|
||||||
|
# this only affects the single model call.
|
||||||
|
drop_count += 1
|
||||||
continue
|
continue
|
||||||
|
|
||||||
sanitized_msg = self._sanitize_ai_message_tool_names(msg)
|
sanitized_msg = self._sanitize_ai_message_tool_names(msg)
|
||||||
@ -271,11 +287,14 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]):
|
|||||||
)
|
)
|
||||||
patch_count += 1
|
patch_count += 1
|
||||||
|
|
||||||
if patched == messages:
|
if patched == messages and not drop_count:
|
||||||
return None
|
return None
|
||||||
|
if drop_count or patch_count:
|
||||||
if patch_count:
|
logger.warning(
|
||||||
logger.warning(f"Injecting {patch_count} placeholder ToolMessage(s) for dangling tool calls")
|
"DanglingToolCallMiddleware: %d orphan(s) dropped, %d placeholder(s) injected",
|
||||||
|
drop_count,
|
||||||
|
patch_count,
|
||||||
|
)
|
||||||
return patched
|
return patched
|
||||||
|
|
||||||
@override
|
@override
|
||||||
|
|||||||
@ -464,7 +464,14 @@ class TestBuildPatchedMessagesPatching:
|
|||||||
assert isinstance(patched[4], ToolMessage)
|
assert isinstance(patched[4], ToolMessage)
|
||||||
assert patched[4].tool_call_id == "call_2"
|
assert patched[4].tool_call_id == "call_2"
|
||||||
|
|
||||||
def test_orphan_tool_message_is_preserved_during_grouping(self):
|
def test_orphan_tool_message_is_dropped_during_grouping(self):
|
||||||
|
"""An orphan ToolMessage — one whose tool_call_id has no matching AIMessage
|
||||||
|
tool_call — is dropped from the patched output.
|
||||||
|
|
||||||
|
Behavior intentionally changed: strict OpenAI-compatible providers reject a
|
||||||
|
ToolMessage that does not follow an assistant tool_call, so an orphan left
|
||||||
|
over from interruption/compaction must not be forwarded.
|
||||||
|
"""
|
||||||
mw = DanglingToolCallMiddleware()
|
mw = DanglingToolCallMiddleware()
|
||||||
orphan = _tool_msg("orphan_call", "orphan")
|
orphan = _tool_msg("orphan_call", "orphan")
|
||||||
msgs = [
|
msgs = [
|
||||||
@ -477,12 +484,57 @@ class TestBuildPatchedMessagesPatching:
|
|||||||
patched = mw._build_patched_messages(msgs)
|
patched = mw._build_patched_messages(msgs)
|
||||||
|
|
||||||
assert patched is not None
|
assert patched is not None
|
||||||
|
# The orphan is dropped; call_1's result is regrouped right after its AIMessage.
|
||||||
|
assert isinstance(patched[0], AIMessage)
|
||||||
|
assert isinstance(patched[1], ToolMessage)
|
||||||
|
assert patched[1].tool_call_id == "call_1"
|
||||||
|
assert isinstance(patched[2], HumanMessage)
|
||||||
|
assert orphan not in patched
|
||||||
|
assert patched.count(orphan) == 0
|
||||||
|
assert len(patched) == 3
|
||||||
|
|
||||||
|
def test_leading_orphan_tool_message_is_dropped(self):
|
||||||
|
"""A ToolMessage that leads the transcript with no preceding tool_call is an
|
||||||
|
orphan and must be dropped (leaving a valid grouped transcript)."""
|
||||||
|
mw = DanglingToolCallMiddleware()
|
||||||
|
leading_orphan = _tool_msg("stale_call", "stale")
|
||||||
|
msgs = [
|
||||||
|
leading_orphan,
|
||||||
|
_ai_with_tool_calls([_tc("bash", "call_1")]),
|
||||||
|
_tool_msg("call_1", "bash"),
|
||||||
|
]
|
||||||
|
|
||||||
|
patched = mw._build_patched_messages(msgs)
|
||||||
|
|
||||||
|
assert patched is not None
|
||||||
|
assert leading_orphan not in patched
|
||||||
|
assert isinstance(patched[0], AIMessage)
|
||||||
|
assert isinstance(patched[1], ToolMessage)
|
||||||
|
assert patched[1].tool_call_id == "call_1"
|
||||||
|
assert len(patched) == 2
|
||||||
|
|
||||||
|
def test_tool_call_id_none_orphan_is_dropped(self):
|
||||||
|
"""A ToolMessage whose tool_call_id is None is always an orphan —
|
||||||
|
no valid tool call uses ``None`` as its id — and must be dropped."""
|
||||||
|
mw = DanglingToolCallMiddleware()
|
||||||
|
# Use model_construct to bypass pydantic validation (ToolMessage requires
|
||||||
|
# a string tool_call_id at construction, but a corrupt serialized payload
|
||||||
|
# or edge-case provider could still produce None at runtime).
|
||||||
|
none_id_orphan = ToolMessage.model_construct(content="ghost", tool_call_id=None)
|
||||||
|
msgs = [
|
||||||
|
_ai_with_tool_calls([_tc("bash", "call_1")]),
|
||||||
|
none_id_orphan,
|
||||||
|
_tool_msg("call_1", "bash"),
|
||||||
|
]
|
||||||
|
|
||||||
|
patched = mw._build_patched_messages(msgs)
|
||||||
|
|
||||||
|
assert patched is not None
|
||||||
|
assert none_id_orphan not in patched
|
||||||
|
assert len(patched) == 2
|
||||||
assert isinstance(patched[0], AIMessage)
|
assert isinstance(patched[0], AIMessage)
|
||||||
assert isinstance(patched[1], ToolMessage)
|
assert isinstance(patched[1], ToolMessage)
|
||||||
assert patched[1].tool_call_id == "call_1"
|
assert patched[1].tool_call_id == "call_1"
|
||||||
assert patched[2] is orphan
|
|
||||||
assert isinstance(patched[3], HumanMessage)
|
|
||||||
assert patched.count(orphan) == 1
|
|
||||||
|
|
||||||
def test_invalid_tool_call_is_patched(self):
|
def test_invalid_tool_call_is_patched(self):
|
||||||
mw = DanglingToolCallMiddleware()
|
mw = DanglingToolCallMiddleware()
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user