Repository navigation
Conversation
|
Hi @Laurianti, Thank you for your contribution and for taking the time to submit this pull request. Our team is currently reviewing your changes and we will reach out if we need any further information. Thank you. |
There was a problem hiding this comment.
Right now, nothing checks that the error code makes the difference. If you drop errorCode().isPresent() from the new condition, every test in EventTest still passes, because none verify that an error-free function-response event is not final.
Could you port test_is_final_response_with_function_response_is_not_final and test_is_final_response_with_trailing_code_result_is_not_final from adk-python? It would also be great to add a test showing that an error event ending with a code execution result is final.
There was a problem hiding this comment.
Fixed in 617dff4: ported test_is_final_response_with_function_response_is_not_final and test_is_final_response_with_trailing_code_result_is_not_final, and added finalResponse_isTrueForErrorEventWithTrailingCodeExecutionResult. Without errorCode().isPresent() the first two now fail.
dosadczuk
added
waiting on reporter
…esponse Matches adk-python: a complete event carrying an error code ends the turn unless the model still asked for tools, even when it holds a function response.
4ca024e to
617dff4
Compare
dosadczuk
left a comment
dosadczuk
left a comment
There was a problem hiding this comment.
The PR description lost its line breaks: it's a single line, so GitHub renders all of it as one heading and the checkboxes don't show. Could you restore the template's line breaks?
Also, "The other cases are unchanged" isn't quite right: an error event ending in a code execution result now becomes final too (your finalResponse_isTrueForErrorEventWithTrailingCodeExecutionResult covers it).
|
Thanks, both fixed:
|
3 participants
Link to Issue or Description of Change
2. Or, if no issue exists, describe the change:
Ports the adk-python fix google/adk-python@75ae2db to
Event.finalResponse().Problem:
adk-python treats a complete event that carries an error code as a final response unless it contains function calls. In Java, an error event that holds a function response is not a final response, so callers that wait for the final response do not stop on it.
Solution:
Same rule as adk-python:
finalResponse()returns true whenerrorCode()is present, the event is not partial and it has no function calls. Two kinds of error events become final responses: one that holds a function response and one that ends in a code execution result. Events without an error code are unchanged.Testing Plan
Unit Tests:
The four adk-python error-event tests are ported to
EventTest: error event with text (final), with a function response (final), with a function call (not final), partial (not final). Also portedtest_is_final_response_with_function_response_is_not_finalandtest_is_final_response_with_trailing_code_result_is_not_final, and added an error event ending with a code execution result (final). Without the fix the error event with a function response fails; without theerrorCode().isPresent()check the two ported non-error tests fail.mvn -pl core test: 1890 tests, 0 failures, 0 errors, 24 skipped.Manual End-to-End (E2E) Tests:
Not needed: the change is limited to
Event.finalResponse().Checklist