fix: ensure balanced tool lifecycle callbacks for hallucinated tools#4847
fix: ensure balanced tool lifecycle callbacks for hallucinated tools#4847weiguangli-io wants to merge 1 commit intogoogle:mainfrom
Conversation
When the LLM hallucinates a non-existent tool name, the ValueError handler was calling on_tool_error_callback directly without first invoking before_tool_callback or entering the tracer span context. This broke the push/pop invariant that plugins (e.g., BigQueryAgentAnalyticsPlugin) rely on for TraceManager span stack management, causing stack corruption. Move the hallucinated-tool error handling inside the traced lifecycle path (_run_with_trace) so that: 1. The tracer span context (start_as_current_span) is entered first 2. before_tool_callback runs before on_tool_error_callback 3. after_tool_callback / trace_tool_call run in the finally block Applied to both _execute_single_function_call_async and _execute_single_function_call_live. Fixes google#4775
Summary of ChangesHello, 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 resolves an issue where the tool lifecycle callbacks were not consistently balanced when an LLM attempted to call a non-existent tool. By relocating the error handling for hallucinated tools within the established tracing context, the change ensures that all necessary callbacks are invoked in the correct sequence, preventing potential corruption of trace spans and maintaining the integrity of plugin operations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. 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
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of unbalanced tool lifecycle callbacks when the LLM hallucinates a non-existent tool. By refactoring the error handling to occur within the traced lifecycle path, the before_tool_callback is now correctly invoked before on_tool_error_callback, ensuring proper span stack management and preventing corruption. The addition of test_hallucinated_tool_calls_before_tool_callback provides excellent validation for this fix, confirming the intended callback order. The changes are well-implemented and directly resolve the reported bug.
Summary
Fixes #4775
When the LLM hallucinates a non-existent tool name, the
ValueErrorhandler in_execute_single_function_call_async(and its_livecounterpart) was callingon_tool_error_callbackdirectly without first invokingbefore_tool_callbackor entering thetracer.start_as_current_span()block. This broke the push/pop invariant that plugins (e.g.,BigQueryAgentAnalyticsPlugin) rely on forTraceManagerspan stack management, causing stack corruption.Root cause: The hallucinated-tool error path short-circuited before entering
_run_with_trace(), skippingbefore_tool_callbackand the tracer span context.Fix: Move the hallucinated-tool error handling inside the traced lifecycle path so that:
start_as_current_span) is entered firstbefore_tool_callbackruns beforeon_tool_error_callbackafter_tool_callback/trace_tool_callrun in thefinallyblockApplied to both
_execute_single_function_call_asyncand_execute_single_function_call_live.Test plan
test_hallucinated_tool_calls_before_tool_callbackthat verifiesbefore_tool_callbackis called beforeon_tool_error_callbackfor hallucinated toolsautoformat.shwith no changes needed