mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(bedrock): sanitize toolUse + toolResult ids on Converse emit
Bedrock Converse rejects requests pre-flight with ValidationException when toolUse.name violates ^[a-zA-Z0-9_-]+$ or when toolUse.toolUseId / toolResult.toolUseId violate ^[a-zA-Z0-9_.:-]+$. Some upstream models (notably Kimi K2.5 on Bedrock Custom Model Import) intermittently emit ids like "functions read_file:4" (space) due to tokenizer drift. The existing make_valid_bedrock_tool_name helper (PR #5138) covers tool *definitions* but not the toolUse.name / toolUse.toolUseId fields LiteLLM emits when translating messages to Converse, and not the mirrored toolResult.toolUseId on the user/tool side. This change: - Adds make_valid_bedrock_tool_use_id next to make_valid_bedrock_tool_name (replaces any char outside [a-zA-Z0-9_.:-] with underscore). - Applies make_valid_bedrock_tool_name + make_valid_bedrock_tool_use_id once at the top of _convert_to_bedrock_tool_call_invoke so the split-objects path inherits a clean base id. - Mirrors the toolUseId sanitization in _convert_to_bedrock_tool_call_result so call/result pairing survives the rewrite deterministically. - Adds 7 unit tests covering whitespace/slash/hash inputs, empty/None pass-through, drifted-id round trip, and the [a-zA-Z0-9_.:-]+ allow-list. Refs upstream issue #5007 / PR #5138 as prior art.
This commit is contained in:
parent
123a9ce487
commit
007e97d61e
2 changed files with 161 additions and 2 deletions
|
|
@ -3996,7 +3996,12 @@ def _convert_to_bedrock_tool_call_invoke(
|
|||
_parts_list: List[BedrockContentBlock] = []
|
||||
for tool in tool_calls:
|
||||
if "function" in tool:
|
||||
tool_id = tool["id"]
|
||||
# Sanitize once at the top of the loop iteration so the
|
||||
# split-objects path below (``block_id = f"{tool_id}_{obj_idx}"``)
|
||||
# inherits a clean base id, and the assistant-side toolUse.name
|
||||
# matches Bedrock's pattern even when an upstream model emits
|
||||
# invalid characters (e.g. Kimi K2.5 tokenizer drift).
|
||||
tool_id = make_valid_bedrock_tool_use_id(tool["id"])
|
||||
name = make_valid_bedrock_tool_name(tool["function"].get("name", ""))
|
||||
arguments = tool["function"].get("arguments", "")
|
||||
|
||||
|
|
@ -4184,7 +4189,12 @@ def _convert_to_bedrock_tool_call_result(
|
|||
)
|
||||
|
||||
message.get("name", "")
|
||||
id = str(message.get("tool_call_id", str(uuid.uuid4())))
|
||||
# Mirror the sanitization applied to ``toolUse.toolUseId`` in
|
||||
# ``_convert_to_bedrock_tool_call_invoke`` so the call-result pairing
|
||||
# survives a rewrite of invalid characters.
|
||||
id = make_valid_bedrock_tool_use_id(
|
||||
str(message.get("tool_call_id", str(uuid.uuid4())))
|
||||
)
|
||||
|
||||
tool_result = BedrockToolResultBlock(
|
||||
content=tool_result_content_blocks,
|
||||
|
|
@ -5351,6 +5361,37 @@ def make_valid_bedrock_tool_name(input_tool_name: str) -> str:
|
|||
return valid_string
|
||||
|
||||
|
||||
def make_valid_bedrock_tool_use_id(input_tool_use_id: str) -> str:
|
||||
"""
|
||||
Replaces any invalid characters in the input tool_use_id with underscores
|
||||
so it matches Bedrock Converse's required pattern ``^[a-zA-Z0-9_.:-]+$``.
|
||||
|
||||
Bedrock Converse rejects ``toolUse.toolUseId`` and ``toolResult.toolUseId``
|
||||
values that contain whitespace, slashes, ``#``, etc., even when the
|
||||
matching ``toolUse.name`` is valid. Some upstream models (notably Kimi
|
||||
K2.5 on Bedrock CMI) intermittently emit ids like ``"functions read_file:4"``
|
||||
(space) due to tokenizer drift; sanitizing here lets the request reach
|
||||
the model rather than failing pre-flight validation.
|
||||
|
||||
The same sanitization must be applied to ``toolUse.toolUseId`` on the
|
||||
assistant side and the mirrored ``toolResult.toolUseId`` on the user /
|
||||
tool message side so the call-result pairing survives the rewrite
|
||||
deterministically.
|
||||
"""
|
||||
|
||||
def replace_invalid(char: str) -> str:
|
||||
"""
|
||||
Bedrock tool_use_ids allow alphanumerics, underscore, period, colon, hyphen.
|
||||
"""
|
||||
if char.isalnum() or char in ("_", ".", ":", "-"):
|
||||
return char
|
||||
return "_"
|
||||
|
||||
if input_tool_use_id is None or len(input_tool_use_id) == 0:
|
||||
return input_tool_use_id
|
||||
return "".join(replace_invalid(char) for char in input_tool_use_id)
|
||||
|
||||
|
||||
def add_cache_point_tool_block(
|
||||
tool: dict, model: Optional[str] = None
|
||||
) -> Optional[BedrockToolBlock]:
|
||||
|
|
|
|||
|
|
@ -2766,3 +2766,121 @@ def test_bedrock_converse_messages_pt_document_rejects_url_source():
|
|||
_bedrock_converse_messages_pt(
|
||||
messages, "anthropic.claude-sonnet-4-6", "bedrock"
|
||||
)
|
||||
|
||||
|
||||
# ── make_valid_bedrock_tool_use_id + round-trip pairing tests ──
|
||||
# Regression coverage for the PLT-716 / Kimi K2.5 tokenizer drift case where
|
||||
# an assistant emits a tool call whose ``name`` and ``id`` carry invalid
|
||||
# characters (e.g. whitespace, slash). Bedrock Converse rejects such requests
|
||||
# pre-flight with three validation errors:
|
||||
# 1. messages[N].toolUse.name fails ^[a-zA-Z0-9_-]+$
|
||||
# 2. messages[N].toolUse.toolUseId fails ^[a-zA-Z0-9_.:-]+$
|
||||
# 3. messages[N+1].toolResult.toolUseId fails ^[a-zA-Z0-9_.:-]+$
|
||||
# Sanitizing all three so call/result pairing survives the rewrite is what
|
||||
# this group of tests verifies.
|
||||
|
||||
|
||||
def test_make_valid_bedrock_tool_use_id_whitespace():
|
||||
"""Whitespace is replaced with underscores (period and colon preserved)."""
|
||||
from litellm.litellm_core_utils.prompt_templates.factory import (
|
||||
make_valid_bedrock_tool_use_id,
|
||||
)
|
||||
|
||||
assert (
|
||||
make_valid_bedrock_tool_use_id("functions read_file:4")
|
||||
== "functions_read_file:4"
|
||||
)
|
||||
|
||||
|
||||
def test_make_valid_bedrock_tool_use_id_preserves_allowed_chars():
|
||||
"""The Bedrock toolUseId pattern allows underscore, period, colon, hyphen."""
|
||||
from litellm.litellm_core_utils.prompt_templates.factory import (
|
||||
make_valid_bedrock_tool_use_id,
|
||||
)
|
||||
|
||||
raw = "functions.read_file:0-suffix_v2"
|
||||
assert make_valid_bedrock_tool_use_id(raw) == raw
|
||||
|
||||
|
||||
def test_make_valid_bedrock_tool_use_id_replaces_slash_and_hash():
|
||||
"""Slashes, hashes, and other invalid chars all become underscores."""
|
||||
from litellm.litellm_core_utils.prompt_templates.factory import (
|
||||
make_valid_bedrock_tool_use_id,
|
||||
)
|
||||
|
||||
assert make_valid_bedrock_tool_use_id("a/b#c") == "a_b_c"
|
||||
|
||||
|
||||
def test_make_valid_bedrock_tool_use_id_empty_passthrough():
|
||||
"""Empty input is returned unchanged (matches make_valid_bedrock_tool_name)."""
|
||||
from litellm.litellm_core_utils.prompt_templates.factory import (
|
||||
make_valid_bedrock_tool_use_id,
|
||||
)
|
||||
|
||||
assert make_valid_bedrock_tool_use_id("") == ""
|
||||
assert make_valid_bedrock_tool_use_id(None) is None # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_bedrock_tool_call_invoke_sanitizes_drifted_name_and_id():
|
||||
"""
|
||||
Assistant emit with whitespace-bearing ``name`` and ``id`` (Kimi K2.5
|
||||
drift shape) is rewritten so both fields match Bedrock's regex.
|
||||
"""
|
||||
tool_calls = [
|
||||
{
|
||||
"id": "functions read_file:4",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "functions read_file",
|
||||
"arguments": '{"path": "x.sv"}',
|
||||
},
|
||||
}
|
||||
]
|
||||
result = _convert_to_bedrock_tool_call_invoke(tool_calls)
|
||||
assert len(result) == 1
|
||||
assert result[0]["toolUse"]["name"] == "functions_read_file"
|
||||
assert result[0]["toolUse"]["toolUseId"] == "functions_read_file:4"
|
||||
assert result[0]["toolUse"]["input"] == {"path": "x.sv"}
|
||||
|
||||
|
||||
def test_bedrock_tool_call_result_sanitizes_drifted_id():
|
||||
"""User-side ``toolResult.toolUseId`` mirrors the same sanitization."""
|
||||
message: ChatCompletionToolMessage = {
|
||||
"role": "tool",
|
||||
"tool_call_id": "functions read_file:4",
|
||||
"content": "result body",
|
||||
}
|
||||
result = _convert_to_bedrock_tool_call_result(message)
|
||||
assert result["toolResult"]["toolUseId"] == "functions_read_file:4"
|
||||
|
||||
|
||||
def test_bedrock_tool_pairing_round_trip_after_sanitization():
|
||||
"""
|
||||
Toolcall id sanitized on the assistant side must match the toolResult id
|
||||
sanitized on the user side so Bedrock can pair the two blocks.
|
||||
"""
|
||||
drifted_id = "functions read_file:4"
|
||||
|
||||
assistant_blocks = _convert_to_bedrock_tool_call_invoke(
|
||||
[
|
||||
{
|
||||
"id": drifted_id,
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "functions read_file",
|
||||
"arguments": "{}",
|
||||
},
|
||||
}
|
||||
]
|
||||
)
|
||||
tool_message: ChatCompletionToolMessage = {
|
||||
"role": "tool",
|
||||
"tool_call_id": drifted_id,
|
||||
"content": "ok",
|
||||
}
|
||||
result_block = _convert_to_bedrock_tool_call_result(tool_message)
|
||||
|
||||
assert (
|
||||
assistant_blocks[0]["toolUse"]["toolUseId"]
|
||||
== result_block["toolResult"]["toolUseId"]
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue