Revert Change CallId to allow for more data. (#793)#880
Revert Change CallId to allow for more data. (#793)#880jgindin wants to merge 1 commit intogoogle:mainfrom
Conversation
After further discussion, we decided that the use case for passing the agent could be solved in a better way. We're adding the `call` property to the `functionResponse` as we believe that will be useful for logging purposes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reverts the structure of CallId from an object back to a string, simplifying the specification. It also introduces a new optional call property to functionResponse for better logging. The changes to the specification and the updates to existing test cases to reflect the CallId change are well-executed. My only feedback is regarding test coverage for the newly added call property, as detailed in my comment.
| "call": { | ||
| "type": "string", | ||
| "description": "The name of the function which was called. MUST be copied verbatim from the function invocation. Useful for logging." | ||
| }, |
There was a problem hiding this comment.
The new call property is a useful addition for logging. However, the test suite for functionResponse messages in specification/v0_10/test/cases/function_response.json has not been updated to include cases for this new property. Per the repository's style guide, new code changes should be accompanied by tests. Please add a test case for a valid functionResponse containing the call property to ensure it is handled correctly.
References
- If there are code changes, code should have tests. (link)
1 participant
Description
After further discussion, we decided that the use case for passing the agent could be solved in a better way.
We're adding the
callproperty to thefunctionResponseas we believe that will be useful for logging purposes.Pre-launch Checklist