Skip to content

fix(responses): generalize tool call parsing for reasoning models#1799

Merged
jxnl merged 3 commits into567-labs:mainfrom
sapountzis:fix/parse_responses_tools_w_reasoning
Sep 5, 2025
Merged

fix(responses): generalize tool call parsing for reasoning models#1799
jxnl merged 3 commits into567-labs:mainfrom
sapountzis:fix/parse_responses_tools_w_reasoning

Conversation

@sapountzis
Copy link
Copy Markdown
Contributor

@sapountzis sapountzis commented Sep 1, 2025

Description

Fixes a bug in parse_responses_tools method that caused failures when using reasoning models where completion.output[0] may not exist or may not have the expected name attribute.

Problem

The previous implementation made unsafe assumptions about the response structure:

  1. Conditional complexity: Used using_inbuilt_tools parameter to handle different response types with branching logic
  2. Unsafe indexing: Directly accessed completion.output[0] without checking if it exists or has the expected properties
  3. Reasoning model incompatibility: Reasoning models can return responses where the first output element isn't the expected tool call

Solution

  • Unified approach: Removed the using_inbuilt_tools parameter and conditional logic
  • Safe iteration: Now iterates through all messages in completion.output to find the matching tool call
  • Robust error handling: Properly handles cases where no matching tool call is found

Changes

  • Removed using_inbuilt_tools parameter from parse_responses_tools method signature
  • Replaced unsafe completion.output[0] access with defensive iteration
  • Simplified the logic by removing the conditional branch for inbuilt tools
  • Updated error handling to be more explicit about missing tool calls

Important

Fixes parse_responses_tools to safely handle reasoning models by iterating over completion.output and removing unsafe assumptions.

  • Behavior:
    • Fixes bug in parse_responses_tools to handle reasoning models where completion.output[0] may not exist or lack name attribute.
    • Iterates through completion.output to find matching tool call, removing unsafe direct access.
    • Handles cases where no matching tool call is found with explicit error.
  • Code Changes:
    • Removes using_inbuilt_tools parameter from parse_responses_tools.
    • Simplifies logic by removing conditional branches for inbuilt tools.
    • Updates error handling for missing tool calls.

This description was created by Ellipsis for 9eca15c. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 9eca15c in 52 seconds. Click for details.
  • Reviewed 53 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. instructor/processing/function_calls.py:556
  • Draft comment:
    Removed the unused 'using_inbuilt_tools' parameter from the function signature. This simplification cleans up the interface and removes redundant branching logic.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 85% None
2. instructor/processing/function_calls.py:561
  • Draft comment:
    The loop now iterates safely over 'completion.output' to find the matching tool call. Consider using a for-else construct to make the intent even clearer.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 85% None
3. instructor/processing/function_calls.py:567
  • Draft comment:
    Enhanced error handling is now in place if no matching tool call is found. It might be beneficial to ensure that 'completion.output' is always a list to avoid unexpected errors.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 85% None

Workflow ID: wflow_xoFCSICcoQAyXZAS

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@jxnl
Copy link
Copy Markdown
Collaborator

jxnl commented Sep 2, 2025

@claude please review this

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Sep 2, 2025

Claude encountered an error —— View job

Failed with exit code 128

I'll analyze this and get back to you.

@sapountzis
Copy link
Copy Markdown
Contributor Author

@jxnl are the tests and claude expected to succeed? My changes are not related to the errors

@jxnl
Copy link
Copy Markdown
Collaborator

jxnl commented Sep 4, 2025

I just approved workflows to run. Since they cost credits, I try not to make every pull request run everything.

@jxnl jxnl merged commit 70e356f into 567-labs:main Sep 5, 2025
3 of 11 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.

2 participants