Skip to content

fix(auto-parser): support bare bracket tool calls#305

Merged
Thump604 merged 1 commit intowaybarrios:mainfrom
Thump604:codex/issue146-bare-bracket-tools
Apr 18, 2026
Merged

fix(auto-parser): support bare bracket tool calls#305
Thump604 merged 1 commit intowaybarrios:mainfrom
Thump604:codex/issue146-bare-bracket-tools

Conversation

@Thump604
Copy link
Copy Markdown
Collaborator

Summary

  • teach the auto tool parser to recognize bare bracket tool calls like [read({"file_path": "/tmp/test.py"})]
  • wake the streaming server fast-path up on bare bracket tool markup so tool calls do not leak as raw text
  • add regression coverage for parser extraction and streaming chat completions

Test plan

  • PYTHONPATH=/tmp/vllm-mlx-issue146 /opt/ai-runtime/venv-live/bin/python -m pytest /tmp/vllm-mlx-issue146/tests/test_tool_parsers.py -q
  • PYTHONPATH=/tmp/vllm-mlx-issue146 /opt/ai-runtime/venv-live/bin/python -m pytest /tmp/vllm-mlx-issue146/tests/test_server.py -q
  • /opt/ai-runtime/venv-live/bin/python -m black --check --fast /tmp/vllm-mlx-issue146/vllm_mlx/tool_parsers/auto_tool_parser.py /tmp/vllm-mlx-issue146/vllm_mlx/server.py /tmp/vllm-mlx-issue146/tests/test_tool_parsers.py /tmp/vllm-mlx-issue146/tests/test_server.py

Closes #146.

Copy link
Copy Markdown
Collaborator

@janhilgard janhilgard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix(auto-parser): support bare bracket tool calls

Overall: Correct feature addition with good test coverage, but the streaming detection logic has a concerning complexity issue.

What this does

Adds support for the [func_name({...})] bare bracket tool call format to the AutoToolParser. This format is used by some models that do not prefix with [Calling tool: or use XML/Mistral-style markup.

Strengths

  1. Clean pattern addition in extract_tool_calls. The BARE_BRACKET_PATTERN is inserted at the right position in the priority chain (after Qwen bracket, before Nemotron). The pattern \[(\w+)\((\{.*?\})\)\] correctly captures the function name and JSON arguments.

  2. Good test coverage. Three new tests:

    • test_detects_bare_bracket for non-streaming extraction
    • test_auto_streaming_bare_bracket for incremental streaming
    • test_auto_parser_streams_bare_bracket_tool_calls for end-to-end server streaming
  3. Server-side streaming detection. _streaming_tool_markup_possible is added to detect bare bracket patterns in accumulated text, replacing the previous "<" in content check.

Issues

  1. The has_marker logic in extract_tool_calls_streaming is convoluted and likely buggy.

    has_marker = any(m in current_text for m in markers) and (
        self.BARE_BRACKET_PARTIAL_PATTERN.search(current_text) is not None
        or self.BARE_BRACKET_PATTERN.search(current_text) is not None
        or "[Calling tool:" in current_text
        or self.MISTRAL_TOKEN in current_text
        or "<" in current_text
    )

    The markers list now includes "[" which matches nearly any text containing a square bracket (e.g. JSON arrays in content, markdown links). The second condition then acts as a guard, but the overall logic is hard to reason about.

    Specific bug: A model output like "See [this link](url)" would match "[" in markers, then the second conjunction falls through to "<" in current_text which would be false (no angle brackets), but if any of the other conditions happen to match, it would incorrectly trigger tool parsing. The intent seems right but the expression is fragile.

  2. BARE_BRACKET_PARTIAL_PATTERN early return can suppress legitimate tool detections.

    if (
        self.BARE_BRACKET_PARTIAL_PATTERN.search(current_text) is not None
        and self.BARE_BRACKET_PATTERN.search(current_text) is None
    ):
        return None

    This returns None (meaning "not content, not tool call -- buffer") when a partial bare bracket is detected but the full pattern is not complete. This is correct for actual bare bracket tool calls, but could incorrectly suppress streaming output if the model produces text like [read( followed by something that is not a JSON object.

  3. Ordering concern with Qwen bracket pattern. QWEN_BRACKET_PATTERN is \[Calling tool:\s*(\w+)\((\{.*?\})\)\] and BARE_BRACKET_PATTERN is \[(\w+)\((\{.*?\})\)\]. Since Qwen bracket is checked first (step 3) and bare bracket second (step 4), this is fine. But the bare bracket pattern would also match Qwen bracket format text, so if the ordering ever changes, there would be double-counting.

  4. The _streaming_tool_markup_possible in server.py overlaps with PR #304. Both PRs modify the same streaming fast-path check in server.py. These will conflict on merge. Consider coordinating the merge order.

Minor

  • The renumbering of steps (4 -> 5, 5 -> 6, etc.) in extract_tool_calls is correct and helpful for readability.
  • The _STREAMING_BARE_BRACKET_PARTIAL regex could be more precise -- \[\w+\($ only anchors at end of string, which is correct for streaming but could miss patterns mid-text in non-streaming contexts.

The core feature is useful and the non-streaming path is clean. The streaming detection logic needs simplification -- the compound boolean with "[" as a marker is the main concern.

Teach the auto tool parser to recognize bare bracket tool calls like
[read({"file_path": "/tmp/test.py"})]. Add _streaming_tool_markup_possible()
to server.py so the streaming fast-path wakes on bare bracket markup
instead of only on '<'.

Closes waybarrios#146.
@Thump604 Thump604 force-pushed the codex/issue146-bare-bracket-tools branch from 86d11f2 to cd79ecf Compare April 18, 2026 02:05
@Thump604 Thump604 merged commit 3b91f9d into waybarrios:main Apr 18, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auto tool parser: bare bracket format [func({...})] not recognized

2 participants