Skip to content

fix(code_executors): emit no empty event from BuiltInCodeExecutor - #7879

Open
adityasingh2400 wants to merge 1 commit into
google:mainfrom
adityasingh2400:fix/builtin-code-executor-empty-event
Open

adityasingh2400 wants to merge 1 commit into
google:mainfrom
adityasingh2400:fix/builtin-code-executor-empty-event

Conversation

@adityasingh2400

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
For BuiltInCodeExecutor, the code execution response processor always yielded an Event(actions=event_actions). When the response had no image to save, that event had no content and empty actions. is_final_response() still returned True for it, and it was stored in the session.

So every reply came with an empty "final" event in front of it. A run loop that stops at the first final response, as is_final_response() is documented to support, got nothing back (#4138).

Solution:
The event is only yielded when event_actions.artifact_delta is non-empty, i.e. when an image was actually saved as an artifact. The image path is unchanged:

  • artifacts are saved;
  • the parts are rewritten to "Saved as artifact: …";
  • the event carries the artifact_delta.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added test_builtin_code_executor_emits_no_event_without_images to tests/unittests/flows/llm_flows/extensions/test_code_execution.py, next to test_builtin_code_executor_image_artifact_creation. A text-only response yields no events, saves no artifact, and leaves the parts unchanged.

# On main (before the fix)
AssertionError: assert [Event(model_...)] == []
  Left contains one more item: Event(model_version=None, content=None, ...)

# With the fix
$ pytest tests/unittests/flows
1197 passed

pre-commit run passes on the changed files.

Manual End-to-End (E2E) Tests:

I ran InMemoryRunner with LlmAgent(code_executor=BuiltInCodeExecutor()) and a mock model that answers "The answer is 42.":

=== BEFORE (main) ===
'a' content= None actions= {'state_delta': {}} final= True
'a' content= parts=[Part(text='The answer is 42.')] ... final= True
session events: 3 [('user', True), ('a', False), ('a', True)]

=== AFTER (this PR) ===
'a' content= parts=[Part(text='The answer is 42.')] ... final= True
session events: 2 [('user', True), ('a', True)]

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

#4138 was closed after the reporter worked around this, but the empty event is still emitted on main.

The built-in branch of the code execution response processor yielded an
event even when no image was saved. That event has no content and no
actions, yet is_final_response() is True, so every reply was preceded by
an empty "final" event, and a run loop that stops at the first final
response (google#4138) never saw the model's answer.

Only yield the event when it carries an artifact_delta.
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.

BuiltInCodeExecutor emits an empty final event before every model reply

2 participants