address Greptile review feedback on tool-result PDF fix

- Tighten _is_anthropic_document_data_uri to match the mimes Anthropic
  actually accepts as base64 `document` source ({application/pdf,
  text/plain}). The previous application/* + text/* prefix match would
  route e.g. data:application/json URIs through the document path,
  producing blocks the Anthropic API rejects. Unsupported mimes now
  stay on the existing image code path (same failure mode as before the
  fix — no regression, just stops introducing a new one).

- On the Bedrock tool-result `type: "file"` branch, accept either
  file_data or file_id and raise BadRequestError on both-None, mirroring
  the user-message _process_file_message pattern. Previously a file
  block with only file_id was silently dropped.

- Consolidate the six new PDF tool-result tests under tests/test_litellm/
  only (the PR template's required location and where the unit-test CI
  workflow runs with coverage). The duplicate copies under
  tests/llm_translation/ added drift risk with no additional coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Josh Minzner 2026-04-28 16:48:29 -04:00
parent 5b5363cd54
commit 12e1d02d4e
5 changed files with 200 additions and 244 deletions

View file

@ -1661,14 +1661,18 @@ def _sanitize_anthropic_tool_use_id(tool_use_id: str) -> str:
return sanitized
_ANTHROPIC_DOCUMENT_BASE64_MEDIA_TYPES = {"application/pdf", "text/plain"}
def _is_anthropic_document_data_uri(url: str) -> bool:
# Anthropic document blocks cover non-image mimes the API accepts via base64
# source (application/pdf, text/*). Match the mime-type prefix in a data URI.
# Anthropic's base64 document source accepts only application/pdf and
# text/plain (see select_anthropic_content_block_type_for_file). Routing
# other mimes here would produce a document block the API rejects, so we
# leave them on the image code path.
match = re.match(r"data:([^;,]+)", url)
if not match:
return False
mime_type = match.group(1)
return mime_type.startswith("application/") or mime_type.startswith("text/")
return match.group(1) in _ANTHROPIC_DOCUMENT_BASE64_MEDIA_TYPES
def convert_to_anthropic_tool_result(
@ -4043,22 +4047,36 @@ def _convert_to_bedrock_tool_call_result(
BedrockToolResultContentBlock(document=_block["document"])
)
elif content["type"] == "file":
# Match the user-message path (_process_file_message): accept
# either file_data (base64 data URI) or file_id (server-side
# reference / URL) and hand off to BedrockImageProcessor. Raise
# BadRequestError on both-None rather than silently dropping.
file_obj = content.get("file") or {}
file_data = file_obj.get("file_data")
if isinstance(file_data, str):
_file_block: BedrockContentBlock = (
BedrockImageProcessor.process_image_sync(image_url=file_data)
file_id = file_obj.get("file_id")
if file_data is None and file_id is None:
raise litellm.BadRequestError(
message="file_data and file_id cannot both be None. Got={}".format(
content
),
model="",
llm_provider="bedrock",
)
file_format = file_obj.get("format")
_file_block: BedrockContentBlock = (
BedrockImageProcessor.process_image_sync(
image_url=cast(str, file_id or file_data),
format=file_format,
)
)
if "document" in _file_block:
tool_result_content_blocks.append(
BedrockToolResultContentBlock(document=_file_block["document"])
)
elif "image" in _file_block:
tool_result_content_blocks.append(
BedrockToolResultContentBlock(image=_file_block["image"])
)
if "document" in _file_block:
tool_result_content_blocks.append(
BedrockToolResultContentBlock(
document=_file_block["document"]
)
)
elif "image" in _file_block:
tool_result_content_blocks.append(
BedrockToolResultContentBlock(image=_file_block["image"])
)
message.get("name", "")
id = str(message.get("tool_call_id", str(uuid.uuid4())))

View file

@ -1885,113 +1885,3 @@ def test_metadata_filter_applies_to_azure_anthropic():
headers={},
)
assert data.get("metadata") == {"user_id": "u2"}
def test_anthropic_tool_result_openai_file_pdf_becomes_document():
"""
OpenAI `{type: "file", file: {file_data: "data:application/pdf;..."}}` inside
a tool-message content list should translate to an Anthropic document block
inside the tool_result content. The existing helper
`anthropic_process_openai_file_message` already does this translation for
user messages; it must be reused on the tool-result path.
"""
from litellm.litellm_core_utils.prompt_templates.factory import (
convert_to_anthropic_tool_result,
)
pdf_b64 = "JVBERi0xLjQKJeLjz9MK"
message = {
"tool_call_id": "toolu_pdf_1",
"role": "tool",
"name": "fetch_document",
"content": [
{
"type": "file",
"file": {
"file_data": f"data:application/pdf;base64,{pdf_b64}",
"filename": "summary.pdf",
},
},
],
}
result = convert_to_anthropic_tool_result(message)
assert result["type"] == "tool_result"
assert result["tool_use_id"] == "toolu_pdf_1"
content = result["content"]
assert isinstance(content, list) and len(content) == 1
block = content[0]
assert block["type"] == "document"
assert block["source"]["type"] == "base64"
assert block["source"]["media_type"] == "application/pdf"
assert block["source"]["data"] == pdf_b64
def test_anthropic_tool_result_image_url_pdf_data_uri_becomes_document():
"""
Regression: a PDF sent as an `image_url` data URI on the tool-result path
must translate to an Anthropic document block (not an image block Anthropic
rejects image blocks whose media_type is a non-image like application/pdf).
"""
from litellm.litellm_core_utils.prompt_templates.factory import (
convert_to_anthropic_tool_result,
)
pdf_b64 = "JVBERi0xLjQKJeLjz9MK"
message = {
"tool_call_id": "toolu_pdf_img_1",
"role": "tool",
"name": "fetch_document",
"content": [
{
"type": "image_url",
"image_url": {
"url": f"data:application/pdf;base64,{pdf_b64}",
},
},
],
}
result = convert_to_anthropic_tool_result(message)
content = result["content"]
assert isinstance(content, list) and len(content) == 1
block = content[0]
assert block["type"] == "document"
assert block["source"]["media_type"] == "application/pdf"
assert block["source"]["data"] == pdf_b64
def test_anthropic_tool_result_image_url_png_still_becomes_image():
"""
Regression: image_url with a real image mime type must continue to translate
to an Anthropic image block. Locks in existing behavior after the
data-URI-mime-type branching for PDFs.
"""
from litellm.litellm_core_utils.prompt_templates.factory import (
convert_to_anthropic_tool_result,
)
png_b64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGNgYGBgAAAABQABXvMqOgAAAABJRU5ErkJggg=="
message = {
"tool_call_id": "toolu_png_1",
"role": "tool",
"name": "fetch_image",
"content": [
{
"type": "image_url",
"image_url": {
"url": f"data:image/png;base64,{png_b64}",
},
},
],
}
result = convert_to_anthropic_tool_result(message)
content = result["content"]
assert isinstance(content, list) and len(content) == 1
block = content[0]
assert block["type"] == "image"
assert block["source"]["media_type"] == "image/png"

View file

@ -1282,123 +1282,6 @@ def test_bedrock_converse_translation_tool_message():
]
def test_bedrock_tool_message_openai_file_pdf_becomes_document():
"""
OpenAI Chat Completions `{type: "file", file: {file_data: "data:application/pdf;...", filename}}`
inside a tool message content list should translate to a Bedrock
toolResult.content[].document block. This is the documented OpenAI shape for
PDFs on the Chat Completions API and what downstream callers emit.
"""
pdf_b64 = "JVBERi0xLjQKJeLjz9MK" # tiny "%PDF-1.4\n" header
messages = [
{"role": "user", "content": "Summarize the attached PDF."},
{
"tool_call_id": "tooluse_pdf_1",
"role": "tool",
"name": "fetch_document",
"content": [
{
"type": "file",
"file": {
"file_data": f"data:application/pdf;base64,{pdf_b64}",
"filename": "summary.pdf",
},
},
],
},
]
translated_msg = _bedrock_converse_messages_pt(
messages=messages, model="", llm_provider=""
)
tool_result = translated_msg[-1]["content"][-1]["toolResult"]
assert tool_result["toolUseId"] == "tooluse_pdf_1"
assert len(tool_result["content"]) == 1
block = tool_result["content"][0]
assert "document" in block, f"expected document block, got {block}"
assert block["document"]["format"] == "pdf"
assert block["document"]["source"]["bytes"] == pdf_b64
assert block["document"]["name"].startswith("DocumentPDFmessages_")
assert block["document"]["name"].endswith("_pdf")
def test_bedrock_tool_message_image_url_pdf_data_uri_becomes_document():
"""
Regression for the processor-returns-document-but-wrapper-drops-it bug:
when a caller sends a PDF as an `image_url` data URI on the tool-result path,
BedrockImageProcessor correctly routes it through the document path and
returns a {"document": ...} block, but the tool-result wrapper only
appended the "image" case, silently dropping documents.
"""
pdf_b64 = "JVBERi0xLjQKJeLjz9MK"
messages = [
{"role": "user", "content": "Summarize the attached PDF."},
{
"tool_call_id": "tooluse_pdf_img_1",
"role": "tool",
"name": "fetch_document",
"content": [
{
"type": "image_url",
"image_url": {
"url": f"data:application/pdf;base64,{pdf_b64}",
},
},
],
},
]
translated_msg = _bedrock_converse_messages_pt(
messages=messages, model="", llm_provider=""
)
tool_result = translated_msg[-1]["content"][-1]["toolResult"]
assert tool_result["toolUseId"] == "tooluse_pdf_img_1"
assert len(tool_result["content"]) == 1
block = tool_result["content"][0]
assert "document" in block, f"expected document block, got {block}"
assert block["document"]["format"] == "pdf"
assert block["document"]["source"]["bytes"] == pdf_b64
def test_bedrock_tool_message_image_url_png_still_becomes_image():
"""
Regression: image_url with an image mime type must continue to translate
to a Bedrock image block (not document). Locks in existing behavior after
the document-passthrough fix.
"""
png_b64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGNgYGBgAAAABQABXvMqOgAAAABJRU5ErkJggg=="
messages = [
{"role": "user", "content": "Describe the attached image."},
{
"tool_call_id": "tooluse_png_1",
"role": "tool",
"name": "fetch_image",
"content": [
{
"type": "image_url",
"image_url": {
"url": f"data:image/png;base64,{png_b64}",
},
},
],
},
]
translated_msg = _bedrock_converse_messages_pt(
messages=messages, model="", llm_provider=""
)
tool_result = translated_msg[-1]["content"][-1]["toolResult"]
assert len(tool_result["content"]) == 1
block = tool_result["content"][0]
assert "image" in block, f"expected image block, got {block}"
assert "document" not in block
assert block["image"]["format"] == "png"
assert block["image"]["source"]["bytes"] == png_b64
def test_base_aws_llm_get_credentials():
import time

View file

@ -2553,6 +2553,80 @@ def test_convert_to_anthropic_tool_result_image_url_pdf_data_uri_becomes_documen
assert block["source"]["data"] == pdf_b64
def test_convert_to_anthropic_tool_result_image_url_unsupported_mime_stays_image_path():
"""
An `image_url` data URI whose mime is neither application/pdf nor text/plain
(e.g. application/json) must NOT be routed through the document path. Anthropic
only accepts application/pdf and text/plain as base64 document media_types
anything else would produce a document block the API rejects. The old
(pre-fix) behavior was to wrap such data as an image block, which also
fails but stays on the image code path; preserve that failure mode rather
than switching to a document path that is equally broken.
"""
from litellm.litellm_core_utils.prompt_templates.factory import (
convert_to_anthropic_tool_result,
)
message = {
"tool_call_id": "toolu_json_1",
"role": "tool",
"name": "fetch_json",
"content": [
{
"type": "image_url",
"image_url": {
"url": "data:application/json;base64,eyJrIjoidiJ9",
},
},
],
}
result = convert_to_anthropic_tool_result(message)
content = result["content"]
assert isinstance(content, list) and len(content) == 1
block = content[0]
assert block["type"] == "image", (
f"unsupported mime {block.get('source', {}).get('media_type')!r} "
f"should not be routed to document path; got {block}"
)
def test_convert_to_anthropic_tool_result_image_url_text_plain_data_uri_becomes_document():
"""
text/plain is one of the two mimes Anthropic accepts as a base64 document
media_type. Confirm it routes through the document path so tightening the
gate to {application/pdf, text/plain} (not "application/*") covers both.
"""
from litellm.litellm_core_utils.prompt_templates.factory import (
convert_to_anthropic_tool_result,
)
txt_b64 = "aGVsbG8=" # "hello"
message = {
"tool_call_id": "toolu_txt_1",
"role": "tool",
"name": "fetch_text",
"content": [
{
"type": "image_url",
"image_url": {
"url": f"data:text/plain;base64,{txt_b64}",
},
},
],
}
result = convert_to_anthropic_tool_result(message)
content = result["content"]
assert isinstance(content, list) and len(content) == 1
block = content[0]
assert block["type"] == "document"
assert block["source"]["media_type"] == "text/plain"
assert block["source"]["data"] == txt_b64
def test_convert_to_anthropic_tool_result_image_url_png_still_becomes_image():
"""
Regression: image_url with a real image mime type must continue to translate

View file

@ -4234,6 +4234,97 @@ def test_bedrock_tool_message_image_url_pdf_data_uri_becomes_document():
assert block["document"]["source"]["bytes"] == pdf_b64
def test_bedrock_tool_message_file_id_http_url_becomes_document():
"""
OpenAI `file.file_id` is a server-side file reference. The Bedrock
user-message path (_process_file_message at factory.py:4796) accepts either
`file_data` or `file_id` and forwards to BedrockImageProcessor. The
tool-result path must match: when `file_id` is an http(s) PDF URL, it
should resolve to a Bedrock document block, not be silently dropped.
"""
from unittest.mock import patch
from litellm.litellm_core_utils.prompt_templates.factory import (
BedrockImageProcessor,
_bedrock_converse_messages_pt,
)
pdf_url = "https://example.com/whitepaper.pdf"
fake_document_block = {
"document": {
"format": "pdf",
"name": "fake_doc",
"source": {"bytes": "ZmFrZQ=="},
}
}
messages = [
{"role": "user", "content": "Summarize the attached PDF."},
{
"tool_call_id": "tooluse_fid_1",
"role": "tool",
"name": "fetch_document",
"content": [
{
"type": "file",
"file": {
"file_id": pdf_url,
"filename": "whitepaper.pdf",
},
},
],
},
]
with patch.object(
BedrockImageProcessor,
"process_image_sync",
return_value=fake_document_block,
) as mock_proc:
translated_msg = _bedrock_converse_messages_pt(
messages=messages, model="", llm_provider=""
)
mock_proc.assert_called_once()
assert mock_proc.call_args.kwargs["image_url"] == pdf_url
tool_result = translated_msg[-1]["content"][-1]["toolResult"]
assert len(tool_result["content"]) == 1
block = tool_result["content"][0]
assert "document" in block, f"expected document block, got {block}"
assert block["document"]["source"]["bytes"] == "ZmFrZQ=="
def test_bedrock_tool_message_file_without_data_or_id_raises():
"""
The user-message path raises BadRequestError when a `type: "file"` block
has neither `file_data` nor `file_id` (factory.py:4802-4809). The
tool-result path must match silently dropping the block makes the model
see an empty tool result and obscures the caller bug.
"""
import litellm
from litellm.litellm_core_utils.prompt_templates.factory import (
_bedrock_converse_messages_pt,
)
messages = [
{"role": "user", "content": "Summarize."},
{
"tool_call_id": "tooluse_bad_1",
"role": "tool",
"name": "fetch_document",
"content": [
{
"type": "file",
"file": {"filename": "nothing.pdf"},
},
],
},
]
with pytest.raises(litellm.BadRequestError):
_bedrock_converse_messages_pt(messages=messages, model="", llm_provider="")
def test_bedrock_tool_message_image_url_png_still_becomes_image():
"""
Regression: image_url with an image mime type must continue to translate