Fix tool output fragmentation by encapsulating content in functionResponse - #13082
Conversation
Summary of ChangesHello @abhipatel12, 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 introduces a significant refactoring to how tool outputs are structured within the system. The primary goal is to eliminate fragmentation by ensuring that complex outputs, such as arrays of parts or binary data, are always encapsulated within a single Highlights
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 refactors convertToFunctionResponse to consistently wrap complex tool outputs into a single functionResponse part, which is a great improvement for handling fragmented responses. The changes are logical and well-tested. I've found one critical issue where the identity (name and ID) of a nested functionResponse is lost during transformation, which could lead to incorrect output attribution. I've provided a suggestion to preserve this information.
|
Size Change: +650 B (0%) Total Size: 21.6 MB ℹ️ View Unchanged
|
cc5ff91 to
04288bc
Compare
04288bc to
0fba3f9
Compare
0fba3f9 to
8ac50af
Compare
…ponse
Refactors `convertToFunctionResponse` to ensure all tool outputs—specifically arrays and binary data—are consistently wrapped within the `content` field of a single `functionResponse` part. This fixes an issue where certain tools returned fragmented responses across multiple parts.
Details:
- Modified `convertToFunctionResponse` in `packages/core/src/core/coreToolScheduler.ts` to wrap `Part[]` and `PartListUnion` outputs into `response: { content: [...] }`.
- Introduced a `toParts` helper to consistently convert various input types into a `Part[]`.
- Simple string outputs continue to use the legacy `response: { output: ... }` format.
- Updated `coreToolScheduler.test.ts` and `nonInteractiveToolExecutor.test.ts`.
8ac50af to
8c70761
Compare
…ing content in functionResponse (google-gemini#13082)
Summary
Refactors
convertToFunctionResponseto ensure all tool outputs—specifically arrays and binary data—are consistently wrapped within thecontentfield of a singlefunctionResponsepart. This fixes an issue where certain tools (likeReadManyFilesToolandDiscoveredMCPTool) returned fragmented responses across multiple parts.Details
convertToFunctionResponseinpackages/core/src/core/coreToolScheduler.tsto wrapPart[]andPartListUnionoutputs intoresponse: { content: [...] }.response: { output: ... }format to minimize disruption for simple tools.coreToolScheduler.test.tsandnonInteractiveToolExecutor.test.tsto match the new output structure, ensuring binary data and mixed content are correctly asserted as being insidefunctionResponse.How to Validate
Run Core Scheduler Tests:
npm test src/core/coreToolScheduler.test.tsVerify that all tests pass, especially the new cases for "PartListUnion array" and "inlineData".
Run Executor Tests:
npm test src/core/nonInteractiveToolExecutor.test.tsConfirm that
should correctly format llmContent with inlineDatapasses with the new expectation.Manual Verification (Optional):
ReadFileToolto read an image or PDF (ask the model to do this)functionResponsepart containing the binary data withincontent, rather than afunctionResponsefollowed by a separateinlineDatapart (via/chat share foo.json)Pre-Merge Checklist