Recover tool calls emitted as XML text (ROB-558) - #2349
Open
naomi-robusta wants to merge 1 commit into
Open
Conversation
Alert-triage relies on the model calling relay's update_ai_triage_metadata
tool via a structured tool_calls response. On some model routes (Holmes
litellm -> relay litellm -> provider) the model instead narrates the call in
the Anthropic tool-use XML dialect it was trained on:
<function_calls>
<invoke name="update_ai_triage_metadata">
<parameter name="team">...</parameter>
...
inside message.content, with no structured tool_calls. litellm's own XML->
tool_calls parser uses strict ET.fromstring and breaks whenever a parameter
value contains markdown / unescaped <, >, & or code fences, so the raw XML
leaks into the content field. Holmes then treats it as the final answer: the
tool never runs (team stays "pending AI investigation") and the raw tags show
up in the user-facing triage result.
Fix: in DefaultLLM.completion() — the single, tool-list-aware choke point that
the agentic loop always calls with stream=False — when a response has no
structured tool_calls but its content holds tool-call XML for a tool that was
actually offered, leniently recover the structured tool call(s) with a
regex-based parser (never ET.fromstring, tolerant of mismatched/absent closing
tags and special chars in values) and strip the XML from the content. Gated on
the recovered tool name matching an offered tool, so ordinary prose that merely
mentions <invoke> is never disturbed. Provider-agnostic.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019TBGvpjmk6npFcvivReBp1
Signed-off-by: Claude <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
WalkthroughThe change adds XML tool-call recovery for non-streaming LLM responses. It parses supported invocation formats, validates tool names against offered tools, creates structured calls, cleans response text, and preserves existing structured calls or ordinary answers. ChangesXML tool-call recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LiteLLM
participant DefaultLLM
participant recover_tool_calls_from_text
LiteLLM->>DefaultLLM: non-streaming ModelResponse
DefaultLLM->>recover_tool_calls_from_text: response content and offered tool names
recover_tool_calls_from_text-->>DefaultLLM: cleaned content and generated tool calls
DefaultLLM-->>LiteLLM: updated ModelResponse
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
✅ Docker images ready for
Use these tags to pull the images for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:f3addd68e
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:f3addd68e me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:f3addd68e
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:f3addd68e
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:f3addd68e
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:f3addd68e me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:f3addd68e
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:f3addd68ePatch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:f3addd68e \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:f3addd68eRobusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:f3addd68e \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:f3addd68e |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
holmes/core/tool_call_recovery.py (1)
75-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParenthesize the mixed
and/orexpression.Ruff flags Line 79 for
and/orchaining without parentheses (RUF021). The current precedence happens to produce the correct result, but the intent is not obvious from the code. Add parentheses to make precedence explicit.🔧 Proposed fix
def _maybe_json(value: str) -> Any: """Mirror LiteLLM's parse_xml_params: decode a value as JSON when it looks like a JSON scalar/array/object, otherwise keep the raw (stripped) string.""" stripped = value.strip() - if stripped and stripped[0] in "[{" or stripped in ("true", "false", "null"): + if (stripped and stripped[0] in "[{") or stripped in ("true", "false", "null"): try: return json.loads(stripped) except (ValueError, TypeError): return value return valueAs per coding guidelines, "Use Ruff for formatting and linting (configured in pyproject.toml)", and this is flagged by the Ruff RUF021 static analysis hint.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holmes/core/tool_call_recovery.py` around lines 75 - 84, Update the condition in _maybe_json to parenthesize the stripped[0] membership check before combining it with the true/false/null check, preserving the existing evaluation behavior while making the and/or precedence explicit.Sources: Coding guidelines, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@holmes/core/tool_call_recovery.py`:
- Around line 75-84: Update the condition in _maybe_json to parenthesize the
stripped[0] membership check before combining it with the true/false/null check,
preserving the existing evaluation behavior while making the and/or precedence
explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 19160050-0901-407e-839e-726210bfe938
📒 Files selected for processing (3)
holmes/core/llm.pyholmes/core/tool_call_recovery.pytests/core/test_tool_call_recovery.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes ROB-558 by recovering tool calls that models (particularly Claude) emit as literal XML text instead of structured
tool_callsfields. This happens when LiteLLM's prompt-based tool-calling fallback is used for models it doesn't recognize as function-calling-capable. LiteLLM's strict XML parser breaks on unescaped markdown/code in parameter values, causing the raw XML to leak into message content and preventing tool execution.Key Changes
New module
holmes/core/tool_call_recovery.py: Implements lenient regex-based parsing to recover tool calls from XML textname="..."attribute) and older<tool_name>tag syntax<,>,&in values)<invoke>)Integration in
holmes/core/llm.py:_offered_tool_names()helper to extract tool names from the tools parameter_recover_text_tool_calls()function that mutates the LLM response to recover XML tool callsDefaultLLM.completion()after receiving the response from LiteLLMtool_callsand content contains tool-call XMLComprehensive test suite
tests/core/test_tool_call_recovery.py:<tool_name>dialectDefaultLLM.completion()Implementation Details
name="..."attribute or nested<tool_name>tag<parameter name="...">markers and slicing to the next marker</parameter>,</key>, or other variants)https://claude.ai/code/session_019TBGvpjmk6npFcvivReBp1
Summary by CodeRabbit