fix: improve Ollama sequential tool execution to prevent redundant calls #966
Conversation
- Add early stopping logic for Ollama after first successful tool execution - Enhance tool summary generation to create more natural, OpenAI-like responses - Apply fix to both sync and async execution paths - Prevent multiple tool calls when execution is complete - Maintains full backward compatibility Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
Warning Rate limit exceeded@github-actions[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@claude review this pull request and do a detailed analysis and fix this if the existing code doesn't have the solution implemented. Making sure it has backward compatibility, no existing features removed. After making those changes, again review the applied changes. Use @web to search if you dont know any information or to find the latest documentation or to find the latest version. Run the code if you think you need to run it to test it. Minimal code change to start with if required any changes. |
|
Claude finished @MervinPraison's task —— View job ✅ PR #966 Review & Fix CompleteTodo List:
🔍 Review SummaryOriginal Issue: Ollama made redundant tool calls (3 instead of 2) and returned "Response: None" instead of natural responses like OpenAI. PR Changes Reviewed:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @MervinPraison, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses an issue specific to the Ollama LLM provider, where it was making redundant tool calls and returning empty responses during sequential tool execution. The core changes introduce an early stopping mechanism and enhance the generation of tool summaries, aiming to provide a more natural and efficient interaction experience for Ollama, similar to OpenAI's behavior.
Highlights
- Ollama Sequential Execution Fix: Implemented early stopping logic for the Ollama LLM provider to prevent redundant tool calls after a successful tool execution, particularly when
Response: Nonewas previously returned. This ensures a cleaner and more efficient interaction flow. - Enhanced Tool Summary Generation: Improved the
_generate_ollama_tool_summarymethod to create more natural and coherent responses from tool execution results, mirroring OpenAI's output style. This includes specific handling for single versus multiple tool results to produce concise summaries. - Sync and Async Path Consistency: Applied the new early stopping and tool summary enhancements consistently across both synchronous (
get_response) and asynchronous (get_response_async) execution paths, ensuring uniform behavior regardless of the call method. - Backward Compatibility: The changes maintain full backward compatibility, ensuring no disruption to existing functionalities or integrations.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request aims to fix an issue with Ollama's tool-calling behavior where it returns empty responses, leading to infinite loops or incomplete results. The changes introduce early-stopping logic and improve tool result summarization.
While the improved summary generation is a good step, the placement of the new early-stopping logic in both the synchronous and asynchronous get_response methods is flawed. The fix doesn't address the primary scenario, and the original bug will likely persist. There's also a minor issue with unreachable code.
| if self._is_ollama_provider() and tool_results and iteration_count >= 1: | ||
| # Generate coherent response from tool results | ||
| tool_summary = self._generate_ollama_tool_summary(accumulated_tool_results, response_text) | ||
| if tool_summary: | ||
| final_response_text = tool_summary | ||
| break |
There was a problem hiding this comment.
The placement of this early-stopping logic is incorrect. It's inside the if tool_calls: block, meaning it only runs if the LLM returns another tool call.
The goal of this PR is to fix the scenario where Ollama returns an empty response with no tool calls after a tool execution. This new logic won't trigger in that case. The execution jumps to the else block at line 1223, causing the loop to break and likely return an empty string.
Consider moving this check to the else block at line 1223, when tool_calls is None but accumulated_tool_results has items.
| if self._is_ollama_provider() and tool_results and iteration_count >= 1: | ||
| # Generate coherent response from tool results | ||
| tool_summary = self._generate_ollama_tool_summary(accumulated_tool_results, response_text) | ||
| if tool_summary: | ||
| final_response_text = tool_summary | ||
| break |
There was a problem hiding this comment.
This logic has the same flaw as in the synchronous get_response method. It's inside the if tool_calls: block (see line 1820). It won't trigger when Ollama returns an empty response without tool calls.
Consider moving the check to handle the case where the current response has no tool calls, but previous iterations have produced tool results.
| return "Based on the tool execution: " + ". ".join(summary_lines) + "." | ||
|
|
||
| return "\n".join(summary_lines) | ||
| return None |
There was a problem hiding this comment.
This return None statement appears to be unreachable. The if/else logic starting at line 333 covers all cases when tool_results is not empty. Since this function is called when tool_results is not empty, this line will likely never be executed and can be removed for code clarity.
…ool execution - Only trigger early stopping when no new tool calls are present in current response - Prevents premature termination during sequential tool calls like get_stock_price -> multiply - Maintains backward compatibility while fixing issue #964 - Applied to both sync and async execution paths 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
@claude review if the changes made in the above branch are as per the requirements and make changes to the above branch if issues still exist. |
|
Claude finished @MervinPraison's task —— View job ✅ PR #966 Review & Fix CompleteTodo List:
🔍 Review SummaryOriginal Issue: Ollama made redundant tool calls (3 instead of 2) and returned "Response: None" instead of natural responses like OpenAI. PR Changes Reviewed:
|
…l execution Fixes critical issues identified in PR review: - Move early stopping logic from inside ''if tool_calls:'' block to ''else:'' block - Fix logic placement in both sync and async execution paths - Remove unreachable code in _generate_ollama_tool_summary() - Ensure early stopping triggers correctly when Ollama returns empty responses after tool execution This ensures the fix properly handles the Ollama edge case where it returns empty responses after successful tool execution, preventing infinite loops while allowing proper sequential tool calls. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Fixes #964
This PR addresses the issue where Ollama LLM provider was making redundant tool calls and returning
Response: Nonecompared to OpenAI’s clean sequential execution.Changes
Testing
🤖 Generated with Claude Code