skills/vectorize-io__hindsight
1
0
Fork 0
mirror of https://github.com/vectorize-io/hindsight.git synced 2026-09-14 19:31:49 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(reflect): uniquify tool_call ids so strict APIs accept the turn (#4250)

Browse Source 
The reflect agent built its assistant message from the raw ids in result.tool_calls and emitted one tool_result per call keyed by the same raw id. When a provider or an OpenAI-compatible gateway returns two calls sharing an id -- or blanks one out -- the serialized turn carries two tool_use blocks with the same id and two tool_result blocks pointing at it, and a strict Anthropic API rejects the whole request with 'each tool_use must have a single result'.

Pick the wire ids once, positionally, before serialising, and reuse that list for both the assistant tool_calls and the tool_result messages so the two stay one-to-one. Only colliding or empty ids are rewritten, so a conforming provider keeps the ids it minted.

Uniqueness spans the whole reflect loop rather than a single batch: the loop serialises into one request, and a gateway that blanks an id blanks it on every turn, so per-batch deduping would mint the same replacement twice and reintroduce the collision. The premature-done guardrail, which loops by construction, routes through the same helper instead of writing the raw (possibly empty) id.

The result slots are still indexed by position, never by tool_call_id -- the wire ids are a parallel positional list rather than a map.
This commit is contained in:
Gowtham Sai
2026-09-10 01:55:06 -07:00
committed by GitHub
parent b5034cb690
commit 5df6398fa1
2 changed files with 197 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hindsight-api-slim/hindsight_api/engine/reflect/agent.py
+57 -12
View File
@@ -896,6 +896,13 @@ async def _run_reflect_agent_inner(
# iteration onward and let the agent answer (or retrieve deeper itself)
# under ``auto`` tool choice. None means the full forced path still applies.
stop_forcing_from_iteration: int | None = None
# Every wire id already written into ``messages`` as a tool_use block. The
# whole loop serialises into ONE request, so uniqueness has to hold across
# iterations, not just within a batch: a gateway that blanks (or repeats) an
# id does it every turn, and two turns minting the same replacement puts two
# tool_use blocks with one id back into the request. ``_unique_tool_call_ids``
# reads and extends this set.
emitted_wire_ids: set[str] = set()
for iteration in range(max_iterations):
# Cooperative cancellation checkpoint: abort the agent loop between
# iterations if the caller (e.g. an HTTP client) has gone away, rather
@@ -1077,17 +1084,22 @@ async def _run_reflect_agent_inner(
bool(available_memory_ids) or bool(available_mental_model_ids) or bool(available_observation_ids)
)
if not has_gathered_evidence and iteration < max_iterations - 1:
# Add assistant message and fake tool result asking for evidence
# Add assistant message and fake tool result asking for evidence.
# This branch loops, so its tool_use lands in the same request as
# every later turn -- it needs a deduped wire id just like the
# parallel batch below (a blank ``done_call.id`` is rejected
# outright by a strict API).
(done_wire_id,) = _unique_tool_call_ids([done_call], emitted_wire_ids)
messages.append(
{
"role": "assistant",
"tool_calls": [_tool_call_to_dict(done_call)],
"tool_calls": [_tool_call_to_dict(done_call, done_wire_id)],
}
)
messages.append(
{
"role": "tool",
"tool_call_id": done_call.id,
"tool_call_id": done_wire_id,
"content": json.dumps(
{
"error": "You must search for information first. Use search_mental_models(), search_observations(), or recall() before providing your final answer."
@@ -1145,11 +1157,16 @@ async def _run_reflect_agent_inner(
allowed_tools.append(tc)
allowed_positions.append(position)
# Build assistant message with all tool calls (LLM requires them for history)
# Build assistant message with all tool calls (LLM requires them for history).
# Wire ids are deduped once, up front, and reused for the tool_result
# messages below so the two stay one-to-one -- see _unique_tool_call_ids.
wire_tool_call_ids = _unique_tool_call_ids(other_tools, emitted_wire_ids)
messages.append(
{
"role": "assistant",
"tool_calls": [_tool_call_to_dict(tc) for tc in other_tools],
"tool_calls": [
_tool_call_to_dict(tc, wire_id) for tc, wire_id in zip(other_tools, wire_tool_call_ids)
],
}
)
@@ -1162,8 +1179,7 @@ async def _run_reflect_agent_inner(
# non-conforming OpenAI-compatible gateway can hand back duplicate or
# empty ids for a parallel batch, and an id-keyed map would silently
# drop one tool's evidence and duplicate another's.
ordered_tool_calls = list(other_tools)
tool_outputs: list[str] = [""] * len(ordered_tool_calls)
tool_outputs: list[str] = [""] * len(other_tools)
for position, tc in zip(hallucinated_positions, hallucinated_tools):
tool_outputs[position] = json.dumps(
{
@@ -1312,11 +1328,11 @@ async def _run_reflect_agent_inner(
# Emit tool_result messages in the assistant tool_calls order so the
# serialized history matches the tool_use blocks (Anthropic requires
# tool_result blocks in the same order as the corresponding tool_use).
for tc, tool_output in zip(ordered_tool_calls, tool_outputs):
for wire_id, tool_output in zip(wire_tool_call_ids, tool_outputs):
messages.append(
{
"role": "tool",
"tool_call_id": tc.id,
"tool_call_id": wire_id,
"content": tool_output,
}
)
@@ -1331,10 +1347,39 @@ async def _run_reflect_agent_inner(
)
def _tool_call_to_dict(tc: "LLMToolCall") -> dict[str, Any]:
"""Convert LLMToolCall to OpenAI message format."""
def _unique_tool_call_ids(tool_calls: list["LLMToolCall"], already_emitted: set[str]) -> list[str]:
"""Pick one unique wire id per tool call, by position, for a parallel batch.
A non-conforming OpenAI-compatible gateway can hand back duplicate or empty
ids, and a strict Anthropic API then rejects the turn outright ("each
tool_use must have a single result"). Only the ids that would collide are
rewritten, so a conforming provider keeps the ids it minted.
``already_emitted`` carries every id used earlier in the SAME conversation
and is extended in place. Uniqueness has to span the whole reflect loop, not
one batch: the loop serialises into a single request, and a gateway that
blanks an id blanks it on every turn -- deduping per batch would just mint
the same replacement twice and put the collision back.
"""
wire_ids: list[str] = []
for position, tc in enumerate(tool_calls):
wire_id = (tc.id or "").strip()
if not wire_id or wire_id in already_emitted:
base = f"{wire_id or 'toolcall'}_{position}"
wire_id = base
attempt = 1
while wire_id in already_emitted:
wire_id = f"{base}_{attempt}"
attempt += 1
already_emitted.add(wire_id)
wire_ids.append(wire_id)
return wire_ids
def _tool_call_to_dict(tc: "LLMToolCall", wire_id: str) -> dict[str, Any]:
"""Convert LLMToolCall to OpenAI message format under its deduped wire id."""
d: dict[str, Any] = {
"id": tc.id,
"id": wire_id,
"type": "function",
"function": {
"name": tc.name,
hindsight-api-slim/tests/test_reflect_agent.py
+140
View File
@@ -1334,6 +1334,146 @@ class TestReflectAgentMocked:
assert "first" in contents[0], contents
assert "second" in contents[1], contents
@pytest.mark.asyncio
async def test_duplicate_tool_call_ids_are_uniquified_on_the_wire(
self, mock_llm: MagicMock, mock_functions: dict[str, AsyncMock]
) -> None:
"""Repeated or empty ids get unique wire ids so a strict Anthropic API accepts the turn.
Anthropic rejects a turn where two tool_use blocks share an id
("each tool_use must have a single result"), which used to make the whole
reflect loop fail on the first tool round-trip against a strict endpoint.
"""
mock_functions["recall_fn"].side_effect = [
{"memories": [{"id": "mem-1", "content": "first"}]},
{"memories": [{"id": "mem-2", "content": "second"}]},
{"memories": [{"id": "mem-3", "content": "third"}]},
]
mock_llm.call_with_tools.side_effect = [
LLMToolCallResult(
tool_calls=[
LLMToolCall(id="dup", name="recall", arguments={"query": "q1"}),
LLMToolCall(id="dup", name="recall", arguments={"query": "q2"}),
LLMToolCall(id="", name="recall", arguments={"query": "q3"}),
],
finish_reason="tool_calls",
),
LLMToolCallResult(
tool_calls=[LLMToolCall(id="d", name="done", arguments={"answer": "A", "memory_ids": ["mem-1"]})],
finish_reason="tool_calls",
),
]
await run_reflect_agent(
llm_config=mock_llm,
bank_id="test-bank",
query="test query",
bank_profile={"name": "Test", "mission": "Testing"},
**mock_functions,
)
messages = mock_llm.call_with_tools.call_args_list[-1].kwargs["messages"]
assistant = [m for m in messages if m.get("role") == "assistant" and m.get("tool_calls")][-1]
tool_use_ids = [tc["id"] for tc in assistant["tool_calls"]]
assert len(tool_use_ids) == 3
assert all(tool_use_id for tool_use_id in tool_use_ids), tool_use_ids
assert len(set(tool_use_ids)) == 3, tool_use_ids
# Every tool_result maps to exactly one tool_use, in the model's order.
tool_messages = [m for m in messages if m.get("role") == "tool"]
assert [m["tool_call_id"] for m in tool_messages] == tool_use_ids
# ...and each slot still carries its OWN payload.
assert "first" in tool_messages[0]["content"], tool_messages
assert "second" in tool_messages[1]["content"], tool_messages
assert "third" in tool_messages[2]["content"], tool_messages
@pytest.mark.asyncio
async def test_wire_ids_stay_unique_across_iterations(
self, mock_llm: MagicMock, mock_functions: dict[str, AsyncMock]
) -> None:
"""A gateway that blanks ids blanks them EVERY turn; the replacements must still differ.
The whole reflect loop serializes into one request, so deduping per batch
would mint the same replacement id on turn 1 and turn 2 and hand the
strict API back the collision it was supposed to remove.
"""
mock_functions["recall_fn"].side_effect = [
{"memories": [{"id": "mem-1", "content": "first"}]},
{"memories": [{"id": "mem-2", "content": "second"}]},
]
mock_llm.call_with_tools.side_effect = [
LLMToolCallResult(
tool_calls=[LLMToolCall(id="", name="recall", arguments={"query": "q1"})],
finish_reason="tool_calls",
),
LLMToolCallResult(
tool_calls=[LLMToolCall(id="", name="recall", arguments={"query": "q2"})],
finish_reason="tool_calls",
),
LLMToolCallResult(
tool_calls=[LLMToolCall(id="d", name="done", arguments={"answer": "A", "memory_ids": ["mem-1"]})],
finish_reason="tool_calls",
),
]
await run_reflect_agent(
llm_config=mock_llm,
bank_id="test-bank",
query="test query",
bank_profile={"name": "Test", "mission": "Testing"},
**mock_functions,
)
messages = mock_llm.call_with_tools.call_args_list[-1].kwargs["messages"]
tool_use_ids = [
tc["id"] for m in messages if m.get("role") == "assistant" and m.get("tool_calls") for tc in m["tool_calls"]
]
assert len(tool_use_ids) == 2, tool_use_ids
assert len(set(tool_use_ids)) == 2, tool_use_ids
# Each tool_use is still answered by exactly one tool_result.
assert [m["tool_call_id"] for m in messages if m.get("role") == "tool"] == tool_use_ids
@pytest.mark.asyncio
async def test_premature_done_guardrail_emits_a_usable_wire_id(
self, mock_llm: MagicMock, mock_functions: dict[str, AsyncMock]
) -> None:
"""The "search first" guardrail loops, so its tool_use needs a deduped id too.
An empty id reaches Anthropic verbatim as ``tool_use.id: ""`` and is
rejected, taking the whole reflect run with it.
"""
mock_functions["recall_fn"].side_effect = [{"memories": [{"id": "mem-1", "content": "first"}]}]
mock_llm.call_with_tools.side_effect = [
LLMToolCallResult(
tool_calls=[LLMToolCall(id="", name="done", arguments={"answer": "premature"})],
finish_reason="tool_calls",
),
LLMToolCallResult(
tool_calls=[LLMToolCall(id="", name="recall", arguments={"query": "q"})],
finish_reason="tool_calls",
),
LLMToolCallResult(
tool_calls=[LLMToolCall(id="d", name="done", arguments={"answer": "A", "memory_ids": ["mem-1"]})],
finish_reason="tool_calls",
),
]
await run_reflect_agent(
llm_config=mock_llm,
bank_id="test-bank",
query="test query",
bank_profile={"name": "Test", "mission": "Testing"},
**mock_functions,
)
messages = mock_llm.call_with_tools.call_args_list[-1].kwargs["messages"]
tool_use_ids = [
tc["id"] for m in messages if m.get("role") == "assistant" and m.get("tool_calls") for tc in m["tool_calls"]
]
assert len(tool_use_ids) == 2, tool_use_ids
assert all(tool_use_id for tool_use_id in tool_use_ids), tool_use_ids
assert len(set(tool_use_ids)) == 2, tool_use_ids
assert [m["tool_call_id"] for m in messages if m.get("role") == "tool"] == tool_use_ids
class TestContextOverflowHelpers:
"""Unit tests for context-overflow detection helpers."""