chore(llm): Hardening Fallback Tool Call

[justin-tahara]

Description

Building upon on the previous PR that was merged in here: #8290

There was an issue with nested tool calls which was addressed. This PR aims to harden this so we don't cause more issues in the future around how we are parsing the individual tool calls.

Previously extract_tool_calls_from_response_text would treat the nested objects as separate tool calls.

How Has This Been Tested?

Added more tests and ran against existing tests.

Additional Options

• [Required] I have considered whether this PR needs to be cherry-picked to the latest beta branch.
• [Optional] Override Linear Check

---

Summary by cubic

Hardens tool-call parsing to stop nested argument objects from being treated as separate calls. Prevents duplicate invocations while keeping intentional repeats.

• Bug Fixes
  • Detect and skip nested arguments duplicates by comparing the current JSON with the previous tool’s extracted args.
  • Preserve intentional duplicate tool calls.
  • Added tests for nested duplicates, intentional duplicates, and non-duplicate sequences.

^{Written for commit 0769fd5. Summary will update on new commits.}

[greptile-apps]

Greptile Overview

Greptile Summary

Hardens the fallback tool-call parser to correctly handle nested argument objects. The previous implementation used simple deduplication that would collapse any consecutive identical tool calls, including intentional duplicates. This PR refines the logic to only skip nested argument objects (e.g., when find_all_json_objects extracts both {"name":"search","arguments":{"q":"x"}} and its nested {"q":"x"} separately) while preserving intentional duplicate tool calls.

Key Changes:

• Replaced post-processing deduplication with inline duplicate detection that checks if the current JSON object is actually the nested arguments from the previous tool call
• Added _is_nested_arguments_duplicate to verify that duplicates are truly artifacts of nested extraction
• Added _extract_nested_arguments_obj to extract the arguments object from various tool call formats
• Tracks both prev_json_obj and prev_tool_call to enable proper comparison
• Renamed test from test_collapses_exact_duplicated_sequence to test_collapses_nested_arguments_duplicate to better reflect its purpose
• Added test_keeps_intentional_duplicate_tool_calls to ensure legitimate duplicate calls aren't filtered

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The implementation is well-designed with clear logic for distinguishing between nested duplicates and intentional duplicates. The new helper functions are focused and correct. Test coverage is comprehensive, including edge cases for nested duplicates, non-duplicates, and intentional duplicates. The refactoring improves code clarity and correctness without introducing complexity.
• No files require special attention

Important Files Changed

Filename	Overview
backend/onyx/chat/llm_step.py	Improved tool call parsing to correctly filter nested argument objects while preserving intentional duplicate tool calls
backend/tests/unit/onyx/chat/test_llm_step.py	Updated test name and added comprehensive test for intentional duplicate tool calls

Sequence Diagram

sequenceDiagram
    participant LLM as LLM Response
    participant Parser as extract_tool_calls_from_response_text
    participant JSONFinder as find_all_json_objects
    participant Matcher as _try_match_json_to_tool
    participant DupCheck as _is_nested_arguments_duplicate
    participant Extractor as _extract_nested_arguments_obj

    LLM->>Parser: response_text with tool calls
    Parser->>JSONFinder: Extract all JSON objects
    JSONFinder-->>Parser: [json_obj1, json_obj2, ...]
    
    loop For each json_obj
        Parser->>Matcher: Try to match to tool
        Matcher-->>Parser: (tool_name, tool_args) or None
        
        alt Has previous tool call
            Parser->>Parser: Check if matched_tool_call == prev_tool_call
            alt Tool calls match
                Parser->>DupCheck: Is current_json_obj nested duplicate?
                DupCheck->>Extractor: Extract args from prev_json_obj
                Extractor-->>DupCheck: nested_args or None
                DupCheck->>DupCheck: Compare current_json_obj == nested_args
                DupCheck-->>Parser: True (skip) or False (keep)
            end
        end
        
        alt Not a nested duplicate
            Parser->>Parser: Add to matched_tool_calls
            Parser->>Parser: Update prev_json_obj & prev_tool_call
        end
    end
    
    Parser-->>LLM: List of ToolCallKickoff objects

Loading

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="backend/onyx/chat/llm_step.py">

<violation number="1" location="backend/onyx/chat/llm_step.py:350">
P2: The strict equality check `matched_tool_call == prev_tool_call` prevents deduplication when `_try_match_json_to_tool` produces different results for the outer tool call versus the inner arguments object. 

This happens because Format 1 (direct tool call) returns arguments as-is (including extra keys), while Format 4 (parameter schema match) filters arguments to only known properties. If an extra argument is present, the equality check fails, and the nested argument object is incorrectly added as a duplicate tool call. Since `_is_nested_arguments_duplicate` already verifies that the current object is structurally identical to the previous tool's arguments, the equality check is unnecessary and harmful.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P2: The strict equality check matched_tool_call == prev_tool_call prevents deduplication when _try_match_json_to_tool produces different results for the outer tool call versus the inner arguments object.

This happens because Format 1 (direct tool call) returns arguments as-is (including extra keys), while Format 4 (parameter schema match) filters arguments to only known properties. If an extra argument is present, the equality check fails, and the nested argument object is incorrectly added as a duplicate tool call. Since _is_nested_arguments_duplicate already verifies that the current object is structurally identical to the previous tool's arguments, the equality check is unnecessary and harmful.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/onyx/chat/llm_step.py, line 350:

<comment>The strict equality check `matched_tool_call == prev_tool_call` prevents deduplication when `_try_match_json_to_tool` produces different results for the outer tool call versus the inner arguments object. 

This happens because Format 1 (direct tool call) returns arguments as-is (including extra keys), while Format 4 (parameter schema match) filters arguments to only known properties. If an extra argument is present, the equality check fails, and the nested argument object is incorrectly added as a duplicate tool call. Since `_is_nested_arguments_duplicate` already verifies that the current object is structurally identical to the previous tool's arguments, the equality check is unnecessary and harmful.</comment>

<file context>
@@ -333,20 +333,32 @@ def extract_tool_calls_from_response_text(
+        if (
+            prev_json_obj is not None
+            and prev_tool_call is not None
+            and matched_tool_call == prev_tool_call
+            and _is_nested_arguments_duplicate(
+                previous_json_obj=prev_json_obj,
</file context>