From c69d59395b6c5b17b2335b1e71e5298d7f88ce34 Mon Sep 17 00:00:00 2001 From: Eric Zhang Date: Sat, 8 Aug 2026 02:39:19 -0400 Subject: [PATCH] [inkling] Render tool-result media instead of coercing content to str (#33898) Co-authored-by: Claude Opus 5 (1M context) --- python/sglang/srt/parser/inkling_renderer.py | 39 +++--- .../unit/parser/test_inkling_renderer.py | 128 ++++++++++++++++++ 2 files changed, 147 insertions(+), 20 deletions(-) diff --git a/python/sglang/srt/parser/inkling_renderer.py b/python/sglang/srt/parser/inkling_renderer.py index 1502cecb7..3e51698cd 100644 --- a/python/sglang/srt/parser/inkling_renderer.py +++ b/python/sglang/srt/parser/inkling_renderer.py @@ -99,17 +99,26 @@ def render_inkling_messages( append_effort() role = _expect_role(message) if role == "tool": - tool_name = message.get("name") or tool_call_id_to_name.get( - message.get("tool_call_id") or "", "" - ) - _append_message( - input_ids, - tokenizer, - "tool", - "text", - _expect_string_content(message.get("content", "")), - author_name=str(tool_name), + tool_name = str( + message.get("name") + or tool_call_id_to_name.get(message.get("tool_call_id") or "", "") ) + # The MM processor harvests media from tool messages, so coercing content + # to one string would drop images and desync the placeholder count. + tool_parts = list(_iter_render_parts(message.get("content", ""))) + if not tool_parts: + tool_parts = [("text", "")] # else the answered tool_call dangles + for kind, text in tool_parts: + if kind == "thinking": + raise ValueError("Inkling thinking parts require role='assistant'") + _append_message( + input_ids, + tokenizer, + "tool", + kind, + text, + author_name=tool_name, + ) continue parts = list(_iter_render_parts(message.get("content", ""))) @@ -249,16 +258,6 @@ def _format_reasoning_effort(reasoning_effort: float) -> str: return f"{round(value, 2):g}" -def _expect_string_content(content: Any) -> str: - if content is None: - return "" - if not isinstance(content, str): - raise TypeError( - f"message content must be a string for this Inkling role, got {type(content).__name__}" - ) - return content - - def _expect_role(message: Mapping[str, Any]) -> str: role = message.get("role") if role not in ROLE_MESSAGE_TOKENS: diff --git a/test/registered/unit/parser/test_inkling_renderer.py b/test/registered/unit/parser/test_inkling_renderer.py index 8a84701a6..c9c2c4781 100644 --- a/test/registered/unit/parser/test_inkling_renderer.py +++ b/test/registered/unit/parser/test_inkling_renderer.py @@ -3,15 +3,18 @@ import unittest from sglang.srt.entrypoints.openai.chat_encoding import encode_simple_chat from sglang.srt.parser.inkling_renderer import render_inkling_messages from sglang.srt.parser.inkling_tokenizer import ( + CONTENT_IMAGE, CONTENT_INVOKE_TOOL_JSON, CONTENT_MODEL_END_SAMPLING, CONTENT_TEXT, CONTENT_THINKING, CONTENT_XML, END_MESSAGE, + IMAGE_TOKEN_ID, INKLING_SPECIAL_TOKEN_IDS, MESSAGE_MODEL, MESSAGE_SYSTEM, + MESSAGE_TOOL, MESSAGE_USER, ) from sglang.test.ci.ci_register import register_cpu_ci @@ -184,6 +187,131 @@ class TestInklingRenderer(unittest.TestCase): ) self.assertNotIn(INKLING_SPECIAL_TOKEN_IDS[CONTENT_MODEL_END_SAMPLING], actual) + def test_tool_result_renders_image_parts_alongside_text(self): + """Bug regression: the tool branch coerced content to a string, so a + tool_result carrying an image (Claude Code screenshots / Read of a PNG) + raised TypeError and 500'd the request. Every part must render, and the + image must emit a placeholder for the MM processor to expand.""" + actual = render_inkling_messages( + [ + { + "role": "tool", + "tool_call_id": "call-1", + "name": "screenshot", + "content": [ + {"type": "text", "text": "captured"}, + { + "type": "image_url", + "image_url": {"url": "data:image/png;base64,AAAA"}, + }, + ], + } + ], + self.tokenizer, + ) + self.assertEqual( + actual, + _block(MESSAGE_SYSTEM, CONTENT_TEXT, "Thinking effort level: 0.9") + + _block(MESSAGE_TOOL, CONTENT_TEXT, "captured", author="screenshot") + + [ + INKLING_SPECIAL_TOKEN_IDS[MESSAGE_TOOL], + *_text("screenshot"), + INKLING_SPECIAL_TOKEN_IDS[CONTENT_IMAGE], + IMAGE_TOKEN_ID, + INKLING_SPECIAL_TOKEN_IDS[END_MESSAGE], + ], + ) + + def test_tool_result_placeholder_count_matches_image_parts(self): + """The MM processor harvests media from tool messages and expands one + placeholder per item, so the counts have to agree or the two passes + desync.""" + image = { + "type": "image_url", + "image_url": {"url": "data:image/png;base64,AAAA"}, + } + actual = render_inkling_messages( + [{"role": "tool", "name": "shot", "content": [image, image, image]}], + self.tokenizer, + ) + self.assertEqual(actual.count(IMAGE_TOKEN_ID), 3) + + def test_tool_result_with_multiple_text_blocks_renders_each(self): + """A tool_result with 2+ text blocks also arrives as a list and used to + raise, even with no image involved.""" + actual = render_inkling_messages( + [ + { + "role": "tool", + "name": "bash", + "content": [ + {"type": "text", "text": "stdout"}, + {"type": "text", "text": "stderr"}, + ], + } + ], + self.tokenizer, + ) + self.assertEqual( + actual, + _block(MESSAGE_SYSTEM, CONTENT_TEXT, "Thinking effort level: 0.9") + + _block(MESSAGE_TOOL, CONTENT_TEXT, "stdout", author="bash") + + _block(MESSAGE_TOOL, CONTENT_TEXT, "stderr", author="bash"), + ) + + def test_empty_tool_result_still_emits_a_block(self): + """An empty tool result must not vanish — the tool_call it answers + would be left dangling.""" + for content in ("", None, []): + with self.subTest(content=content): + actual = render_inkling_messages( + [{"role": "tool", "name": "noop", "content": content}], + self.tokenizer, + ) + self.assertEqual( + actual, + _block(MESSAGE_SYSTEM, CONTENT_TEXT, "Thinking effort level: 0.9") + + _block(MESSAGE_TOOL, CONTENT_TEXT, "", author="noop"), + ) + + def test_tool_result_author_falls_back_to_tool_call_id(self): + """String content still resolves the author from a prior tool_call.""" + actual = render_inkling_messages( + [ + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call-1", + "function": {"name": "weather", "arguments": "{}"}, + } + ], + }, + {"role": "tool", "tool_call_id": "call-1", "content": "sunny"}, + ], + self.tokenizer, + ) + self.assertEqual( + actual[ + -len(_block(MESSAGE_TOOL, CONTENT_TEXT, "sunny", author="weather")) : + ], + _block(MESSAGE_TOOL, CONTENT_TEXT, "sunny", author="weather"), + ) + + def test_tool_result_rejects_thinking_parts(self): + with self.assertRaisesRegex(ValueError, "require role='assistant'"): + render_inkling_messages( + [ + { + "role": "tool", + "name": "t", + "content": [{"type": "thinking", "thinking": "nope"}], + } + ], + self.tokenizer, + ) + def test_reasoning_content_cannot_reorder_thinking_parts(self): with self.assertRaisesRegex(ValueError, "cannot mix"): render_inkling_messages(