mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-21 10:15:47 +00:00
fix(middleware): recover malformed tool-call ids in dangling repair (#4246)
* fix(middleware): recover malformed tool-call ids in dangling repair DanglingToolCallMiddleware normalizes malformed tool-call names (#4008) and arguments (#4193) so strict OpenAI-compatible providers do not reject the next request. The id is the third field of that same recovery contract and was left alone. A provider that emits an empty id -- or omits it -- parses into a well-formed tool_calls entry, so it reaches the middleware through the normal path. The empty id never enters the pairing set, so the orphan pass drops the call's already-produced ToolMessage and the placeholder pass skips the call. The request then goes out carrying an empty id and with the real tool result gone. Normalize ids up front and re-point each already-paired ToolMessage at its call's new id, so the existing pairing/orphan/placeholder logic no longer sees a malformed id. Only the view that is actually read and serialized is relabelled; a valid id is left byte-for-byte alone, since it is matched verbatim against ToolMessage.tool_call_id. * fix(middleware): scope malformed-id result pairing to its own turn Malformed tool-call ids are all equally empty, so pairing recovered results by the original id alone was a global FIFO over the whole transcript. An earlier dangling call then consumed a later turn's result: the real result was served to the wrong call while the call that actually ran got the interrupted placeholder. An orphan result whose originating AIMessage was already gone could likewise be adopted by a later malformed call, resurrecting it instead of being dropped as the orphan pass intends. Walk the messages once in document order so only the most recent AIMessage's unanswered calls are claimable, which keeps a result answering the turn that issued it, and rule out the wrong parallel sibling within a turn by tool name. A result whose name matches no open call is left malformed for the existing orphan pass to drop rather than repurposed as some other call's answer. * fix(middleware): keep the shadowed raw view out of id recovery * fix(middleware): only claim a malformed result when the pairing is forced * docs(middleware): cite ToolNode's ordering guarantee for positional pairing
This commit is contained in:
Aari
+158
-3
@@ -37,12 +37,98 @@ logger = logging.getLogger(__name__)
|
||||
_MAX_RECOVERY_ERROR_DETAIL_LEN = 500
|
||||
_UNKNOWN_TOOL_NAME = "unknown_tool"
|
||||
_EMPTY_TOOL_NAME_ERROR = "Tool call could not be executed because its name was missing or empty."
|
||||
_SYNTHETIC_TOOL_CALL_ID_PREFIX = "deerflow_synthetic_tool_call_"
|
||||
|
||||
|
||||
def _valid_tool_name(name: object) -> bool:
|
||||
return isinstance(name, str) and bool(name.strip())
|
||||
|
||||
|
||||
def _valid_tool_call_id(tool_call_id: object) -> bool:
|
||||
return isinstance(tool_call_id, str) and bool(tool_call_id.strip())
|
||||
|
||||
|
||||
def _tool_call_name(tool_call: dict) -> object:
|
||||
"""Return a call's declared name, mirroring _message_tool_calls' raw-payload fallback."""
|
||||
name = tool_call.get("name")
|
||||
if _valid_tool_name(name):
|
||||
return name
|
||||
function = tool_call.get("function")
|
||||
return function.get("name") if isinstance(function, dict) else name
|
||||
|
||||
|
||||
def _names_can_pair(call_name: object, result_name: object) -> bool:
|
||||
"""Whether a result's name contradicts a call's name.
|
||||
|
||||
Either side may legitimately be missing (the empty-name sibling recovery exists
|
||||
for exactly that), and a missing name cannot contradict anything — only two
|
||||
usable names that differ rule the pairing out.
|
||||
"""
|
||||
if not _valid_tool_name(call_name) or not _valid_tool_name(result_name):
|
||||
return True
|
||||
return call_name.strip() == result_name.strip()
|
||||
|
||||
|
||||
def _relabel_tool_call_ids(tool_calls: list, msg_index: int, source: str) -> tuple[list, list[dict], bool]:
|
||||
"""Replace malformed ids in one tool-call list with stable synthetic ids.
|
||||
|
||||
The id is derived from the call's position so both the pairing pass and the
|
||||
model-bound message agree on it without threading state between them.
|
||||
|
||||
Returns the rewritten list, one ``{original, synthetic, name}`` claim entry per
|
||||
relabelled call, and whether anything changed.
|
||||
"""
|
||||
relabeled: list = []
|
||||
assigned: list[dict] = []
|
||||
changed = False
|
||||
for position, tool_call in enumerate(tool_calls):
|
||||
if not isinstance(tool_call, dict) or _valid_tool_call_id(tool_call.get("id")):
|
||||
relabeled.append(tool_call)
|
||||
continue
|
||||
synthetic = f"{_SYNTHETIC_TOOL_CALL_ID_PREFIX}{msg_index}_{source}_{position}"
|
||||
relabeled.append({**tool_call, "id": synthetic})
|
||||
changed = True
|
||||
assigned.append({"original": tool_call.get("id"), "synthetic": synthetic, "name": _tool_call_name(tool_call)})
|
||||
return relabeled, assigned, changed
|
||||
|
||||
|
||||
def _turn_malformed_result_count(messages: list, start: int) -> int:
|
||||
"""Count the malformed results issued by the turn opened at ``start``."""
|
||||
count = 0
|
||||
for msg in messages[start + 1 :]:
|
||||
if getattr(msg, "type", None) == "ai":
|
||||
break
|
||||
if isinstance(msg, ToolMessage) and not _valid_tool_call_id(msg.tool_call_id):
|
||||
count += 1
|
||||
return count
|
||||
|
||||
|
||||
def _claim_synthetic_id(open_calls: list[dict], result: ToolMessage, positional: bool) -> str | None:
|
||||
"""Consume the open malformed call that ``result`` answers, returning its new id.
|
||||
|
||||
Malformed originals are all equally empty, so they cannot identify their own
|
||||
result; ``open_calls`` is already scoped to the issuing turn. Within that turn the
|
||||
result's name narrows the candidates, and only a *forced* choice is taken:
|
||||
|
||||
* one compatible call — its name, or being the turn's only call, identifies it;
|
||||
* several compatible calls — position identifies them, but only while ``positional``
|
||||
holds, i.e. every open call in the turn has a result. Identical parallel calls
|
||||
(two ``bash``) are distinguishable by nothing else, and order here is a
|
||||
construction guarantee rather than an assumption about the provider: LangGraph's
|
||||
``ToolNode`` builds the results with ``asyncio.gather`` / ``executor.map`` over
|
||||
``tool_calls``, both of which yield in input order however the tools interleave.
|
||||
A *missing* result means a call was interrupted — this middleware's own trigger —
|
||||
so the surviving results can no longer be trusted to line up with the calls.
|
||||
|
||||
Returning ``None`` leaves the result malformed for the orphan pass to drop, which is
|
||||
what an unattributable result gets today — better than inventing a pairing.
|
||||
"""
|
||||
candidates = [position for position, entry in enumerate(open_calls) if entry["original"] == result.tool_call_id and _names_can_pair(entry["name"], result.name)]
|
||||
if not candidates or (len(candidates) > 1 and not positional):
|
||||
return None
|
||||
return open_calls.pop(candidates[0])["synthetic"]
|
||||
|
||||
|
||||
def _normalize_tool_name(name: object) -> str:
|
||||
return name.strip() if _valid_tool_name(name) else _UNKNOWN_TOOL_NAME
|
||||
|
||||
@@ -267,19 +353,88 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]):
|
||||
return msg
|
||||
return msg.model_copy(update=update)
|
||||
|
||||
@staticmethod
|
||||
def _normalize_tool_call_ids(messages: list) -> list:
|
||||
"""Return messages with malformed tool-call ids replaced by synthetic ids.
|
||||
|
||||
A provider that omits a tool-call id parses into a well-formed ``tool_calls``
|
||||
entry with an empty/``None`` id. Such an id can never enter the pairing set
|
||||
below, so the call's own result is dropped as an orphan and no placeholder
|
||||
replaces it — the request then reaches the provider with an empty id and the
|
||||
tool result gone. Normalizing ids up front lets the pairing and placeholder
|
||||
logic treat the call like any other, mirroring the empty-name recovery.
|
||||
"""
|
||||
rewritten: dict[int, object] = {}
|
||||
# Malformed calls from the most recent AIMessage that are still unanswered.
|
||||
# Walking in document order and resetting here is what scopes a result to the
|
||||
# turn that issued it: a result never answers a call from an earlier turn, so
|
||||
# an earlier dangling call must not consume a later turn's result.
|
||||
open_calls: list[dict] = []
|
||||
# Whether this turn's results line up 1:1 with its malformed calls, which is what
|
||||
# lets position break a tie between otherwise indistinguishable siblings.
|
||||
positional = False
|
||||
|
||||
for index, msg in enumerate(messages):
|
||||
if getattr(msg, "type", None) == "ai":
|
||||
update: dict = {}
|
||||
assigned: list[dict] = []
|
||||
structured = getattr(msg, "tool_calls", None) or []
|
||||
additional_kwargs = getattr(msg, "additional_kwargs", None) or {}
|
||||
raw_tool_calls = additional_kwargs.get("tool_calls")
|
||||
|
||||
invalid = getattr(msg, "invalid_tool_calls", None) or []
|
||||
sources: list[tuple[str, list, str]] = [
|
||||
("call", structured, "tool_calls"),
|
||||
("invalid", invalid, "invalid_tool_calls"),
|
||||
]
|
||||
# The raw payload is a fallback view of the same calls, so relabel it only when
|
||||
# it is the view actually serialized: the OpenAI serializer reaches for it only
|
||||
# once *both* structured views are empty. Minting an id for a shadowed raw view
|
||||
# would owe a placeholder to a call the provider never sees, putting an orphan
|
||||
# tool result on the wire.
|
||||
if not structured and not invalid and isinstance(raw_tool_calls, list):
|
||||
sources.append(("raw", raw_tool_calls, "additional_kwargs"))
|
||||
|
||||
for source, tool_calls, field in sources:
|
||||
relabeled, source_assigned, changed = _relabel_tool_call_ids(tool_calls, index, source)
|
||||
assigned.extend(source_assigned)
|
||||
if not changed:
|
||||
continue
|
||||
update[field] = {**additional_kwargs, "tool_calls": relabeled} if field == "additional_kwargs" else relabeled
|
||||
|
||||
open_calls = assigned
|
||||
positional = _turn_malformed_result_count(messages, index) == len(assigned)
|
||||
if update:
|
||||
rewritten[index] = msg.model_copy(update=update)
|
||||
continue
|
||||
|
||||
# Re-point an already-paired result at its call's new id so the pairing
|
||||
# below keeps it instead of dropping it as an orphan.
|
||||
if not isinstance(msg, ToolMessage) or _valid_tool_call_id(msg.tool_call_id):
|
||||
continue
|
||||
synthetic = _claim_synthetic_id(open_calls, msg, positional)
|
||||
if synthetic is not None:
|
||||
rewritten[index] = msg.model_copy(update={"tool_call_id": synthetic})
|
||||
|
||||
if not rewritten:
|
||||
return messages
|
||||
return [rewritten.get(index, msg) for index, msg in enumerate(messages)]
|
||||
|
||||
def _build_patched_messages(self, messages: list) -> list | None:
|
||||
"""Return messages with tool results grouped after their tool-call AIMessage.
|
||||
|
||||
This normalizes model-bound causal order before provider serialization while
|
||||
preserving already-valid transcripts unchanged.
|
||||
"""
|
||||
normalized = self._normalize_tool_call_ids(messages)
|
||||
|
||||
tool_messages_by_id: dict[str, deque[ToolMessage]] = defaultdict(deque)
|
||||
for msg in messages:
|
||||
for msg in normalized:
|
||||
if isinstance(msg, ToolMessage):
|
||||
tool_messages_by_id[msg.tool_call_id].append(msg)
|
||||
|
||||
tool_call_ids: set[str] = set()
|
||||
for msg in messages:
|
||||
for msg in normalized:
|
||||
if getattr(msg, "type", None) != "ai":
|
||||
continue
|
||||
for tc in self._message_tool_calls(msg):
|
||||
@@ -290,7 +445,7 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]):
|
||||
patched: list = []
|
||||
patch_count = 0
|
||||
drop_count = 0
|
||||
for msg in messages:
|
||||
for msg in normalized:
|
||||
if isinstance(msg, ToolMessage):
|
||||
if msg.tool_call_id in tool_call_ids:
|
||||
continue # Will be re-emitted after its AIMessage
|
||||
|
||||
@@ -727,6 +727,388 @@ class TestBuildPatchedMessagesPatching:
|
||||
assert ai_message.invalid_tool_calls[0]["args"] == _invalid_tc()["args"]
|
||||
|
||||
|
||||
class TestMalformedToolCallIdRecovery:
|
||||
"""Empty/missing tool-call ids get the same recovery as empty names (#4008).
|
||||
|
||||
A provider that omits the id parses into a well-formed ``tool_calls`` entry with
|
||||
``id=""``/``None``, so these reach the middleware through the normal path.
|
||||
"""
|
||||
|
||||
@pytest.mark.parametrize("tc_id", ["", " ", None])
|
||||
def test_malformed_structured_tool_call_id_is_normalized(self, tc_id):
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [_ai_with_tool_calls([_tc("bash", tc_id)])]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
normalized_id = patched[0].tool_calls[0]["id"]
|
||||
assert normalized_id
|
||||
assert normalized_id.strip()
|
||||
payload = _convert_message_to_dict(patched[0])
|
||||
assert payload["tool_calls"][0]["id"] == normalized_id
|
||||
assert isinstance(patched[1], ToolMessage)
|
||||
assert patched[1].tool_call_id == normalized_id
|
||||
assert patched[1].status == "error"
|
||||
|
||||
def test_empty_tool_call_id_keeps_its_paired_tool_result(self):
|
||||
"""The destructive case: the result exists, so it must survive and stay paired.
|
||||
|
||||
Without id normalization the empty id never enters the pairing set, so the
|
||||
real result is dropped as an orphan while the AIMessage keeps ``id=""``.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("bash", "")]),
|
||||
ToolMessage(content="REAL RESULT", tool_call_id="", name="bash"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
assert len(patched) == 2
|
||||
assert isinstance(patched[1], ToolMessage)
|
||||
assert patched[1].content == "REAL RESULT"
|
||||
assert patched[1].tool_call_id == patched[0].tool_calls[0]["id"]
|
||||
assert patched[1].status != "error"
|
||||
|
||||
def test_multiple_empty_ids_get_distinct_ids_and_pair_in_order(self):
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("bash", ""), _tc("ls", "")]),
|
||||
ToolMessage(content="first", tool_call_id="", name="bash"),
|
||||
ToolMessage(content="second", tool_call_id="", name="ls"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
first_id, second_id = (tc["id"] for tc in patched[0].tool_calls)
|
||||
assert first_id != second_id
|
||||
assert [m.content for m in patched[1:]] == ["first", "second"]
|
||||
assert [m.tool_call_id for m in patched[1:]] == [first_id, second_id]
|
||||
|
||||
def test_empty_id_invalid_tool_call_is_normalized(self):
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [_ai_with_invalid_tool_calls([_invalid_tc(tc_id="")])]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
normalized_id = patched[0].invalid_tool_calls[0]["id"]
|
||||
assert normalized_id
|
||||
payload = _convert_message_to_dict(patched[0])
|
||||
assert payload["tool_calls"][0]["id"] == normalized_id
|
||||
assert patched[1].tool_call_id == normalized_id
|
||||
|
||||
def test_empty_id_raw_provider_tool_call_is_normalized(self):
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
AIMessage.model_construct(
|
||||
content="",
|
||||
type="ai",
|
||||
tool_calls=[],
|
||||
invalid_tool_calls=[],
|
||||
additional_kwargs={"tool_calls": [{"id": "", "type": "function", "function": {"name": "bash", "arguments": "{}"}}]},
|
||||
response_metadata={},
|
||||
)
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
normalized_id = patched[0].additional_kwargs["tool_calls"][0]["id"]
|
||||
assert normalized_id
|
||||
payload = _convert_message_to_dict(patched[0])
|
||||
assert payload["tool_calls"][0]["id"] == normalized_id
|
||||
assert patched[1].tool_call_id == normalized_id
|
||||
|
||||
def test_only_the_serialized_view_of_a_call_gets_a_recovered_id(self):
|
||||
"""When both views of one call are present, only the read one is relabelled.
|
||||
|
||||
``_message_tool_calls`` and the OpenAI serializer both prefer structured
|
||||
tool_calls and ignore the raw payload while they coexist. Relabelling raw here
|
||||
would invent a *second* id for a call that already got one, so the two views of
|
||||
the same call would disagree; the raw ids cannot simply be copied from
|
||||
structured either, since a partially-parsed turn splits calls across
|
||||
``invalid_tool_calls`` and breaks positional alignment.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
AIMessage.model_construct(
|
||||
content="",
|
||||
type="ai",
|
||||
tool_calls=[_tc("bash", "")],
|
||||
invalid_tool_calls=[],
|
||||
additional_kwargs={"tool_calls": [{"id": "", "type": "function", "function": {"name": "bash", "arguments": "{}"}}]},
|
||||
response_metadata={},
|
||||
)
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
recovered_id = patched[0].tool_calls[0]["id"]
|
||||
assert recovered_id
|
||||
payload = _convert_message_to_dict(patched[0])
|
||||
assert payload["tool_calls"][0]["id"] == recovered_id
|
||||
assert patched[1].tool_call_id == recovered_id
|
||||
# The unread view keeps the provider's value rather than a second invented id.
|
||||
assert patched[0].additional_kwargs["tool_calls"][0]["id"] == ""
|
||||
# ...which is only safe while the serializer ignores raw once structured exists.
|
||||
# If that ever changes, the id="" above would ride onto the wire as a second
|
||||
# tool_calls entry and cause the 400 this recovery exists to prevent.
|
||||
assert len(payload["tool_calls"]) == 1
|
||||
|
||||
def test_raw_view_shadowed_by_invalid_calls_gets_no_placeholder(self):
|
||||
"""A shadowed raw payload is not a call of its own, so it is owed no placeholder.
|
||||
|
||||
``_convert_message_to_dict`` serializes ``tool_calls + invalid_tool_calls`` and
|
||||
reaches for the raw ``additional_kwargs`` view only in its ``elif`` — i.e. never
|
||||
while *either* structured view is non-empty. Relabelling raw here would mint a
|
||||
second id for a call the provider never sees, and that id's placeholder would
|
||||
reach the wire as a tool result with no matching tool_call: the exact HTTP 400
|
||||
this middleware exists to prevent. Sibling of
|
||||
``test_only_the_serialized_view_of_a_call_gets_a_recovered_id`` — there the
|
||||
shadowing view is ``tool_calls``, here it is ``invalid_tool_calls``.
|
||||
|
||||
This is the realistic malformed-``write_file`` shape (#2894): LangChain parks the
|
||||
parse failure in ``invalid_tool_calls`` while the raw payload stays in
|
||||
``additional_kwargs``, so both views describe one call.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
bad_args = '{"path": "/mnt/user-data/outputs/report.md", "content": "## Report {"'
|
||||
msgs = [
|
||||
AIMessage.model_construct(
|
||||
content="",
|
||||
type="ai",
|
||||
tool_calls=[],
|
||||
invalid_tool_calls=[{"name": "write_file", "args": bad_args, "id": "", "error": "Unterminated string"}],
|
||||
additional_kwargs={"tool_calls": [{"id": "", "type": "function", "function": {"name": "write_file", "arguments": bad_args}}]},
|
||||
response_metadata={},
|
||||
),
|
||||
ToolMessage(content="REAL RESULT", tool_call_id="", name="write_file"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
# Assert on the serialized request, because that is where a strict provider rejects.
|
||||
wire = [_convert_message_to_dict(m) for m in patched]
|
||||
call_ids = [tc["id"] for m in wire if m["role"] == "assistant" for tc in m.get("tool_calls", [])]
|
||||
result_ids = [m["tool_call_id"] for m in wire if m["role"] == "tool"]
|
||||
assert [i for i in result_ids if i not in call_ids] == []
|
||||
assert len(call_ids) == 1
|
||||
assert result_ids == call_ids
|
||||
assert patched[1].content == "REAL RESULT"
|
||||
|
||||
def test_none_id_call_reclaims_its_own_none_id_result(self):
|
||||
"""A ``None``-id result is an orphan only when no call used ``None`` as its id.
|
||||
|
||||
Sibling of ``test_tool_call_id_none_orphan_is_dropped``: there the call's id is
|
||||
valid, so the result stays an orphan. Here the call itself carries the ``None``
|
||||
id, so the result is its own and must follow the call to its recovered id
|
||||
instead of being dropped. Both are reachable only from a corrupt serialized
|
||||
payload, which is why the ToolMessage is built with ``model_construct``.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("bash", None)]),
|
||||
ToolMessage.model_construct(content="REAL RESULT", tool_call_id=None),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
assert len(patched) == 2
|
||||
assert isinstance(patched[1], ToolMessage)
|
||||
assert patched[1].content == "REAL RESULT"
|
||||
assert patched[1].tool_call_id == patched[0].tool_calls[0]["id"]
|
||||
|
||||
def test_dangling_call_does_not_consume_a_later_turns_result(self):
|
||||
"""A result answers the turn that issued it, never an earlier dangling call.
|
||||
|
||||
Malformed originals are all equally empty, so pairing on the original id alone
|
||||
lets the first empty-id call claim a later turn's result: the real result is
|
||||
served to the wrong call while the call that actually ran gets the placeholder.
|
||||
The retried tool shares the interrupted one's name — the realistic shape, and
|
||||
the one where nothing but document order can tell the two turns apart.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("bash", "")]), # interrupted: never produced a result
|
||||
_ai_with_tool_calls([_tc("bash", "")]), # retried, and this one ran
|
||||
ToolMessage(content="REAL RESULT", tool_call_id="", name="bash"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
interrupted_call, interrupted_result, retried_call, retried_result = patched
|
||||
assert interrupted_result.tool_call_id == interrupted_call.tool_calls[0]["id"]
|
||||
assert interrupted_result.status == "error"
|
||||
assert retried_result.tool_call_id == retried_call.tool_calls[0]["id"]
|
||||
assert retried_result.content == "REAL RESULT"
|
||||
|
||||
def test_orphan_result_is_not_adopted_by_a_later_malformed_call(self):
|
||||
"""An orphan malformed result stays an orphan and is dropped.
|
||||
|
||||
A result whose originating AIMessage is gone (e.g. dropped by summarization)
|
||||
has no call to return to. Pairing on the original id alone lets a *later*
|
||||
malformed call adopt it, resurrecting a stale result as the answer to a call
|
||||
that never produced it — and swallowing the placeholder that call is owed.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
ToolMessage(content="STALE ORPHAN", tool_call_id="", name="search"),
|
||||
HumanMessage(content="continue"),
|
||||
_ai_with_tool_calls([_tc("write_file", "")]),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
assert all(getattr(m, "content", None) != "STALE ORPHAN" for m in patched)
|
||||
human, write_call, write_result = patched
|
||||
assert isinstance(human, HumanMessage)
|
||||
assert write_result.tool_call_id == write_call.tool_calls[0]["id"]
|
||||
assert write_result.status == "error"
|
||||
|
||||
def test_sibling_call_does_not_consume_its_neighbours_result(self):
|
||||
"""Parallel calls in one turn: the result goes to the sibling that ran.
|
||||
|
||||
Interrupting one of several parallel calls is this middleware's own trigger,
|
||||
and within a turn the empty originals cannot tell the siblings apart either.
|
||||
The result's name can, so it must not be handed to whichever sibling is first.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("search", ""), _tc("write_file", "")]),
|
||||
ToolMessage(content="REAL RESULT", tool_call_id="", name="write_file"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
search_id, write_id = (tc["id"] for tc in patched[0].tool_calls)
|
||||
results = {m.tool_call_id: m for m in patched[1:]}
|
||||
assert results[write_id].content == "REAL RESULT"
|
||||
assert results[search_id].status == "error"
|
||||
|
||||
def test_nameless_result_is_dropped_when_several_siblings_are_eligible(self):
|
||||
"""With nothing to tell two malformed siblings apart, the result names neither.
|
||||
|
||||
``ToolMessage.name`` is optional, and a missing name cannot contradict any call,
|
||||
so every open call in the turn stays eligible. Handing the result to whichever
|
||||
sibling comes first is a guess: the interrupted call may be the first one, which
|
||||
would serve a real result to the wrong call and give the one that actually ran a
|
||||
placeholder — the same corruption as the cross-turn case, inside one turn.
|
||||
Position cannot break the tie here either: one call has no result at all, so the
|
||||
two no longer line up.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("search", ""), _tc("write_file", "")]),
|
||||
ToolMessage.model_construct(content="REAL RESULT", tool_call_id="", name=None),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
assert all(getattr(m, "content", None) != "REAL RESULT" for m in patched)
|
||||
assert [m.status for m in patched[1:]] == ["error", "error"]
|
||||
|
||||
def test_identical_parallel_calls_pair_with_their_results_in_order(self):
|
||||
"""Indistinguishable siblings that all answered are paired by position.
|
||||
|
||||
Two ``bash`` calls cannot be told apart by name, so a per-result "claim only a
|
||||
unique candidate" rule would drop *both* real results and report two interrupted
|
||||
calls. Position is real evidence precisely because nothing is missing: LangGraph's
|
||||
``ToolNode`` builds these results with ``asyncio.gather`` / ``executor.map`` over
|
||||
``tool_calls``, which yield in input order regardless of completion order, so a
|
||||
fully answered turn lines up by construction. Contrast the test above, where one
|
||||
call is unanswered and that alignment is gone.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("bash", ""), _tc("bash", "")]),
|
||||
ToolMessage(content="RESULT A", tool_call_id="", name="bash"),
|
||||
ToolMessage(content="RESULT B", tool_call_id="", name="bash"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
first_id, second_id = (tc["id"] for tc in patched[0].tool_calls)
|
||||
assert [m.tool_call_id for m in patched[1:]] == [first_id, second_id]
|
||||
assert [m.content for m in patched[1:]] == ["RESULT A", "RESULT B"]
|
||||
|
||||
def test_identical_parallel_calls_drop_a_lone_ambiguous_result(self):
|
||||
"""Position is only evidence while the turn is fully answered.
|
||||
|
||||
Same two indistinguishable ``bash`` calls, but one result: the alignment that
|
||||
justifies pairing by position no longer holds, and the name cannot narrow the
|
||||
pair either, so there is no evidence tying the result to a call. Guards against
|
||||
the positional tie-break widening into a first-sibling default.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("bash", ""), _tc("bash", "")]),
|
||||
ToolMessage(content="LONE RESULT", tool_call_id="", name="bash"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
assert all(getattr(m, "content", None) != "LONE RESULT" for m in patched)
|
||||
assert [m.status for m in patched[1:]] == ["error", "error"]
|
||||
|
||||
def test_result_naming_no_call_is_dropped_rather_than_misattributed(self):
|
||||
"""An unattributable result is dropped, not repurposed.
|
||||
|
||||
When the name matches no open call there is no evidence tying the result to
|
||||
any of them. Dropping it is what it already gets today; handing it to an
|
||||
arbitrary call would invent a pairing and corrupt the transcript instead.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("search", ""), _tc("write_file", "")]),
|
||||
ToolMessage(content="RESULT OF NEITHER", tool_call_id="", name="grep"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
assert all(getattr(m, "content", None) != "RESULT OF NEITHER" for m in patched)
|
||||
assert [m.status for m in patched[1:]] == ["error", "error"]
|
||||
|
||||
def test_valid_ids_are_left_byte_for_byte_unchanged(self):
|
||||
"""Delta-set guard: normalization must only fire on malformed ids.
|
||||
|
||||
A valid id is matched against ``ToolMessage.tool_call_id`` verbatim, so
|
||||
rewriting (e.g. stripping) one would break pairing that works today. The
|
||||
malformed sibling is what makes this assert on the patched output rather than
|
||||
on the no-op path: with only the whitespace id present the transcript is fully
|
||||
responded, ``_build_patched_messages`` returns ``None``, and the guarantee is
|
||||
never actually exercised through the rewriting code.
|
||||
"""
|
||||
mw = DanglingToolCallMiddleware()
|
||||
msgs = [
|
||||
_ai_with_tool_calls([_tc("bash", " call_1 "), _tc("ls", "")]),
|
||||
ToolMessage(content="result", tool_call_id=" call_1 ", name="bash"),
|
||||
]
|
||||
|
||||
patched = mw._build_patched_messages(msgs)
|
||||
|
||||
assert patched is not None
|
||||
kept_id, recovered_id = (tc["id"] for tc in patched[0].tool_calls)
|
||||
assert kept_id == " call_1 "
|
||||
assert recovered_id and recovered_id != " call_1 "
|
||||
results = {m.tool_call_id: m for m in patched[1:]}
|
||||
assert results[" call_1 "].content == "result"
|
||||
assert results[recovered_id].status == "error"
|
||||
|
||||
|
||||
class TestWrapModelCall:
|
||||
def test_no_patch_passthrough(self):
|
||||
mw = DanglingToolCallMiddleware()
|
||||
|
||||
Reference in New Issue
Block a user