Fix Inkling tool-call parsing recovery, content handling, and streaming (#32861)
This commit is contained in:
Ke Bao
@@ -86,30 +86,32 @@ class TestInklingDetector(unittest.TestCase):
|
||||
self.assertEqual(name, "weather")
|
||||
self.assertEqual(json.loads(parameters), {"city": "SF"})
|
||||
|
||||
def test_mismatched_header_is_rejected(self):
|
||||
def test_header_name_is_ignored_and_payload_name_wins(self):
|
||||
"""The message header is author metadata, not a name check: a header
|
||||
that differs from the payload name still yields a call named by the
|
||||
payload."""
|
||||
detector = InklingDetector()
|
||||
source = (
|
||||
"<|message_model|>other<|content_invoke_tool_json|>"
|
||||
'{"name":"weather","args":{}}<|end_message|>'
|
||||
)
|
||||
result = detector.detect_and_parse(source, self.tools)
|
||||
self.assertEqual(len(result.calls), 1)
|
||||
self.assertEqual(result.calls[0].name, "weather")
|
||||
self.assertEqual(result.normal_text, "")
|
||||
|
||||
def test_raw_fallback_strips_protocol_tokens(self):
|
||||
"""When a payload cannot be parsed or recovered, the visible text is
|
||||
surfaced as content with the <|...|> special tokens stripped."""
|
||||
detector = InklingDetector()
|
||||
source = (
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>"
|
||||
"{not json at all<|end_message|>"
|
||||
)
|
||||
result = detector.detect_and_parse(source, self.tools)
|
||||
self.assertEqual(result.calls, [])
|
||||
|
||||
def test_rejected_call_does_not_leak_protocol_tokens(self):
|
||||
"""Bug regression: the no-surviving-calls path returned the RAW text,
|
||||
so a rejected call (e.g. header/payload mismatch) leaked <|...|>
|
||||
protocol tokens into user-visible content."""
|
||||
detector = InklingDetector()
|
||||
source = (
|
||||
"<|message_model|>other<|content_invoke_tool_json|>"
|
||||
'{"name":"weather","args":{}}<|end_message|>'
|
||||
)
|
||||
result = detector.detect_and_parse(source, self.tools)
|
||||
self.assertNotIn("<|", result.normal_text)
|
||||
# Framework parity: the rejected tool-call REGION is dropped entirely
|
||||
# (normal_text = content before the marker), like every other detector
|
||||
# — the JSON payload must not surface as visible content either.
|
||||
self.assertEqual(result.normal_text, "")
|
||||
self.assertIn("{not json at all", result.normal_text)
|
||||
|
||||
def test_headerless_legacy_tool_call_still_parses(self):
|
||||
"""Spec tolerance: a bare <|content_invoke_tool_json|> block with no
|
||||
@@ -189,11 +191,9 @@ class TestInklingDetector(unittest.TestCase):
|
||||
args = "".join(c.parameters for c in result.calls)
|
||||
self.assertEqual(json.loads(args), {"city": "SF"})
|
||||
|
||||
def test_streaming_rejected_middle_call_keeps_later_valid_call(self):
|
||||
"""Bug regression: a rejected call (header/name mismatch) cleared the
|
||||
whole buffer, discarding a later valid call that arrived in the same
|
||||
delta. Only the rejected call's span may be dropped; the drain must
|
||||
continue so the trailing valid call still streams."""
|
||||
def test_streaming_differing_headers_all_stream(self):
|
||||
"""The header is author metadata, not a name gate: three calls with
|
||||
differing headers all stream, indexed 0/1/2 by payload name."""
|
||||
detector = InklingDetector()
|
||||
source = (
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>"
|
||||
@@ -208,37 +208,38 @@ class TestInklingDetector(unittest.TestCase):
|
||||
args_by_index[call.tool_index] = (
|
||||
args_by_index.get(call.tool_index, "") + call.parameters
|
||||
)
|
||||
self.assertEqual(sorted(args_by_index), [0, 1])
|
||||
self.assertEqual(sorted(args_by_index), [0, 1, 2])
|
||||
self.assertEqual(json.loads(args_by_index[0]), {"city": "SF"})
|
||||
self.assertEqual(json.loads(args_by_index[1]), {"city": "NY"})
|
||||
self.assertEqual(json.loads(args_by_index[1]), {"city": "XX"})
|
||||
self.assertEqual(json.loads(args_by_index[2]), {"city": "NY"})
|
||||
|
||||
def test_streaming_rejection_does_not_collide_tool_indices(self):
|
||||
"""Bug regression: a rejected mid-stream call reset current_tool_id to
|
||||
-1, so the NEXT valid call re-announced as tool_index 0 — colliding
|
||||
with the first call's index and slicing its arguments against index
|
||||
0's already-streamed args."""
|
||||
def test_streaming_malformed_call_switches_to_raw_passthrough(self):
|
||||
"""A call that fails to frame switches the stream to raw passthrough:
|
||||
earlier calls stay emitted (streaming cannot un-emit), and everything
|
||||
after the failure is surfaced as content, never as further calls."""
|
||||
detector = InklingDetector()
|
||||
chunks = [
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>",
|
||||
'{"name":"weather","args":{"city":"SF"}}<|end_message|>',
|
||||
# header/payload mismatch -> rejected
|
||||
"<|message_model|>other<|content_invoke_tool_json|>",
|
||||
'{"name":"weather","args":{"city":"NY"}}<|end_message|>',
|
||||
# valid again
|
||||
# unrecoverable -> raw passthrough from here on
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>",
|
||||
"{not json at all<|end_message|>",
|
||||
# would-be call, now passthrough text
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>",
|
||||
'{"name":"weather","args":{"city":"LA"}}<|end_message|>',
|
||||
]
|
||||
args_by_index: dict = {}
|
||||
calls: list = []
|
||||
normal_text = ""
|
||||
for chunk in chunks:
|
||||
for call in detector.parse_streaming_increment(chunk, self.tools).calls:
|
||||
args_by_index[call.tool_index] = (
|
||||
args_by_index.get(call.tool_index, "") + call.parameters
|
||||
)
|
||||
self.assertEqual(json.loads(args_by_index[0]), {"city": "SF"})
|
||||
self.assertEqual(len(args_by_index), 2)
|
||||
second_index = max(args_by_index)
|
||||
self.assertGreater(second_index, 0)
|
||||
self.assertEqual(json.loads(args_by_index[second_index]), {"city": "LA"})
|
||||
result = detector.parse_streaming_increment(chunk, self.tools)
|
||||
normal_text += result.normal_text
|
||||
calls.extend(result.calls)
|
||||
self.assertEqual(len(calls), 1)
|
||||
self.assertEqual(calls[0].name, "weather")
|
||||
self.assertEqual(json.loads(calls[0].parameters), {"city": "SF"})
|
||||
self.assertNotIn("<|", normal_text)
|
||||
self.assertIn("{not json at all", normal_text)
|
||||
self.assertIn("LA", normal_text)
|
||||
|
||||
def test_undeclared_tool_name_is_surfaced(self):
|
||||
"""A call to a tool absent from the request's tool list surfaces as a
|
||||
@@ -275,8 +276,9 @@ class TestInklingDetector(unittest.TestCase):
|
||||
self.assertEqual(json.loads(parameters), {"query": "q"})
|
||||
self.assertNotIn("<|", normal_text)
|
||||
|
||||
def test_malformed_json_does_not_leak_protocol_tokens(self):
|
||||
"""Malformed JSON must drop the protocol region and its tool header."""
|
||||
def test_malformed_json_surfaces_as_raw_fallback(self):
|
||||
"""Malformed JSON that also fails recovery surfaces the visible payload
|
||||
as content (special tokens stripped), not a tool call."""
|
||||
detector = InklingDetector()
|
||||
source = (
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>"
|
||||
@@ -284,10 +286,12 @@ class TestInklingDetector(unittest.TestCase):
|
||||
)
|
||||
result = detector.detect_and_parse(source, self.tools)
|
||||
self.assertEqual(result.calls, [])
|
||||
self.assertEqual(result.normal_text, "")
|
||||
self.assertNotIn("<|", result.normal_text)
|
||||
self.assertIn("{not json at all", result.normal_text)
|
||||
|
||||
def test_parser_does_not_restore_malformed_tool_call_as_text(self):
|
||||
"""The parser wrapper must preserve the detector's sanitized fallback."""
|
||||
def test_parser_preserves_raw_fallback_text(self):
|
||||
"""The parser wrapper preserves the detector's raw fallback, so the
|
||||
visible prefix plus the failed payload reach the caller as content."""
|
||||
from sglang.srt.function_call.function_call_parser import FunctionCallParser
|
||||
|
||||
source = (
|
||||
@@ -298,8 +302,9 @@ class TestInklingDetector(unittest.TestCase):
|
||||
normal_text, calls = FunctionCallParser(self.tools, "inkling").parse_non_stream(
|
||||
source
|
||||
)
|
||||
self.assertEqual(normal_text, "Visible prefix.")
|
||||
self.assertEqual(calls, [])
|
||||
self.assertTrue(normal_text.startswith("Visible prefix."))
|
||||
self.assertIn("{not json at all", normal_text)
|
||||
|
||||
def test_parser_preserves_text_without_tool_call_marker(self):
|
||||
from sglang.srt.function_call.function_call_parser import FunctionCallParser
|
||||
@@ -311,7 +316,10 @@ class TestInklingDetector(unittest.TestCase):
|
||||
self.assertEqual(normal_text, source)
|
||||
self.assertEqual(calls, [])
|
||||
|
||||
def test_malformed_call_does_not_discard_an_earlier_valid_call(self):
|
||||
def test_one_malformed_call_fails_the_whole_batch(self):
|
||||
"""All-or-nothing: a single unrecoverable call fails canonical framing
|
||||
for the whole response, so even an earlier valid call is discarded and
|
||||
the visible text is surfaced as content."""
|
||||
source = (
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>"
|
||||
'{"name":"weather","args":{"city":"SF"}}<|end_message|>'
|
||||
@@ -319,10 +327,10 @@ class TestInklingDetector(unittest.TestCase):
|
||||
"{not json at all<|end_message|>"
|
||||
)
|
||||
result = InklingDetector().detect_and_parse(source, self.tools)
|
||||
self.assertEqual(result.normal_text, "")
|
||||
self.assertEqual(len(result.calls), 1)
|
||||
self.assertEqual(result.calls[0].name, "weather")
|
||||
self.assertEqual(json.loads(result.calls[0].parameters), {"city": "SF"})
|
||||
self.assertEqual(result.calls, [])
|
||||
self.assertNotIn("<|", result.normal_text)
|
||||
self.assertIn('{"name":"weather","args":{"city":"SF"}}', result.normal_text)
|
||||
self.assertIn("{not json at all", result.normal_text)
|
||||
|
||||
def test_clean_normal_text_strips_the_full_control_alphabet(self):
|
||||
"""Fall-through text is cleaned against the whole shared control-token
|
||||
@@ -341,6 +349,86 @@ class TestInklingDetector(unittest.TestCase):
|
||||
self.assertEqual(info.trigger, header)
|
||||
self.assertTrue(info.begin.startswith(header + '{"name":"weather"'))
|
||||
|
||||
def test_content_after_tool_call_is_preserved(self):
|
||||
"""A tool call followed by a text block returns both: the call plus the
|
||||
trailing visible content, not just the prefix before the marker."""
|
||||
source = (
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>"
|
||||
'{"name":"weather","args":{"city":"SF"}}<|end_message|>'
|
||||
"<|message_model|><|content_text|>Here you go.<|end_message|>"
|
||||
)
|
||||
result = InklingDetector().detect_and_parse(source, self.tools)
|
||||
self.assertEqual(len(result.calls), 1)
|
||||
self.assertEqual(result.calls[0].name, "weather")
|
||||
self.assertEqual(result.normal_text, "Here you go.")
|
||||
|
||||
def test_empty_name_is_allowed_on_the_canonical_path(self):
|
||||
source = "<|content_invoke_tool_json|>" '{"name":"","args":{}}<|end_message|>'
|
||||
result = InklingDetector().detect_and_parse(source, self.tools)
|
||||
self.assertEqual(len(result.calls), 1)
|
||||
self.assertEqual(result.calls[0].name, "")
|
||||
|
||||
def test_recovery_uses_only_the_last_marker(self):
|
||||
"""Canonical framing fails on the garbage payload; recovery reads only
|
||||
the payload after the LAST marker."""
|
||||
source = (
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>garbage"
|
||||
"<|content_invoke_tool_json|>"
|
||||
'{"name":"weather","args":{"city":"SF"}}<|end_message|>'
|
||||
)
|
||||
result = InklingDetector().detect_and_parse(source, self.tools)
|
||||
self.assertEqual(len(result.calls), 1)
|
||||
self.assertEqual(result.calls[0].name, "weather")
|
||||
self.assertEqual(json.loads(result.calls[0].parameters), {"city": "SF"})
|
||||
|
||||
def test_recovery_requires_a_nonempty_name(self):
|
||||
"""Recovery (unlike the canonical path) rejects an empty name, falling
|
||||
through to raw text."""
|
||||
source = (
|
||||
"<|message_model|>weather<|content_invoke_tool_json|>bad"
|
||||
'<|content_invoke_tool_json|>{"name":"","args":{}}<|end_message|>'
|
||||
)
|
||||
result = InklingDetector().detect_and_parse(source, self.tools)
|
||||
self.assertEqual(result.calls, [])
|
||||
self.assertNotIn("<|", result.normal_text)
|
||||
|
||||
def test_nonfinite_numbers_rejected_canonically_but_recovered(self):
|
||||
"""NaN/Infinity are not valid canonical JSON, so the strict pass fails;
|
||||
recovery accepts them."""
|
||||
source = (
|
||||
"<|content_invoke_tool_json|>"
|
||||
'{"name":"weather","args":{"v":NaN}}<|end_message|>'
|
||||
)
|
||||
result = InklingDetector().detect_and_parse(source, self.tools)
|
||||
self.assertEqual(len(result.calls), 1)
|
||||
self.assertEqual(result.calls[0].name, "weather")
|
||||
|
||||
def test_streaming_name_not_emitted_before_end_message(self):
|
||||
"""Atomicity: the tool name is withheld until the closing marker, so a
|
||||
call that never completes never leaks an orphan name delta."""
|
||||
detector = InklingDetector()
|
||||
pre = detector.parse_streaming_increment(
|
||||
'<|message_model|>weather<|content_invoke_tool_json|>{"name":"wea',
|
||||
self.tools,
|
||||
)
|
||||
self.assertEqual(pre.calls, [])
|
||||
post = detector.parse_streaming_increment(
|
||||
'ther","args":{"city":"SF"}}<|end_message|>', self.tools
|
||||
)
|
||||
self.assertEqual(len(post.calls), 1)
|
||||
self.assertEqual(post.calls[0].name, "weather")
|
||||
self.assertEqual(json.loads(post.calls[0].parameters), {"city": "SF"})
|
||||
|
||||
def test_raw_text_tool_invocation_surfaces_as_a_call(self):
|
||||
"""A headerless <|content_invoke_tool_text|> block reaches the tool loop
|
||||
as a call carrying the raw body, instead of being dropped."""
|
||||
source = "<|content_invoke_tool_text|>search the web<|end_message|>"
|
||||
result = InklingDetector().detect_and_parse(source, self.tools)
|
||||
self.assertEqual(len(result.calls), 1)
|
||||
self.assertEqual(
|
||||
json.loads(result.calls[0].parameters), {"text": "search the web"}
|
||||
)
|
||||
|
||||
|
||||
class TestPythonicDetector(unittest.TestCase):
|
||||
def setUp(self):
|
||||
|
||||
Reference in New Issue
Block a user