Fix tool calls not shown until there is some text in message#15
Fix tool calls not shown until there is some text in message#15fr33z00 wants to merge 10 commits intomoeru-ai:mainfrom
Conversation
Add reasoning data extraction
Reasoning extraction
Few more fixes
reverted reference
Summary of ChangesHello @fr33z00, 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 addresses and resolves an issue where tool calls were not immediately visible in the user interface until some text content was present in the message. The changes introduce a new mechanism for displaying intermediate reasoning steps and ensure that message content is properly initialized when tool calls begin. This is supported by a significant upgrade of core 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
|
![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 introduces support for streaming 'reasoning' steps and fixes an issue where tool calls would not be displayed until some text was present in the message. The changes are mostly in call-api.ts to handle the new 'reasoning-delta' event and to correctly initialize the message content for tool calls. I've found a few areas in the new logic that could be made more type-safe and robust to prevent potential runtime errors.
| if (part && !shouldNewReasoningNext) { | ||
| part.reasoning += event.text | ||
| } | ||
| else { | ||
| parts.push({ reasoning: event.text, type: 'reasoning' }) | ||
| } |
There was a problem hiding this comment.
The current implementation is not type-safe. part is of type UIMessagePart | undefined, and accessing part.reasoning directly can lead to a runtime error if part is not a UIMessageReasoningPart. Although findLast filters by type, TypeScript's control flow analysis doesn't automatically narrow the type here. You can add an explicit check for part.type to ensure type safety.
| if (part && !shouldNewReasoningNext) { | |
| part.reasoning += event.text | |
| } | |
| else { | |
| parts.push({ reasoning: event.text, type: 'reasoning' }) | |
| } | |
| if (part && part.type === 'reasoning' && !shouldNewReasoningNext) { | |
| part.reasoning += event.text; | |
| } | |
| else { | |
| parts.push({ reasoning: event.text, type: 'reasoning' }); | |
| } |
| parts.push({ reasoning: event.text, type: 'reasoning' }) | ||
| } | ||
|
|
||
| message.content = (message.content as string ?? '') + event.text |
There was a problem hiding this comment.
The type assertion message.content as string is unsafe. If message.content is an array of parts, it will be coerced to a string like "[object Object]" at runtime, which is likely not the intended behavior. To safely handle this, you should check if message.content is a string before concatenating. This same issue exists on line 40 for text-delta and should be addressed there as well, although it's outside the current diff.
| message.content = (message.content as string ?? '') + event.text | |
| message.content = (typeof message.content === 'string' ? message.content : '') + event.text |
| }, | ||
| type: 'tool-call', | ||
| }) | ||
| message.content = message.content as string ?? '' |
There was a problem hiding this comment.
The type assertion message.content as string is unsafe here. If message.content is an array of parts, it will not be converted to an empty string as intended. A safer approach is to check the type of message.content to ensure it's a string, and initialize to an empty string otherwise. This correctly implements the fix for showing tool calls without prior text content.
| message.content = message.content as string ?? '' | |
| message.content = typeof message.content === 'string' ? message.content : '' |
|
Hello. As said in the title, but not commented because I was tricked one more time by github that I don't use much, This PR intends to fix the fact that no part of the message other than user prompt is available when LLM decides to start with tool calls, and this, until it writes text. I only changed one line, in this one, but code base is my previous PR. |
| "name": "@xsai-use/shared", | ||
| "type": "module", | ||
| "version": "0.0.2", | ||
| "version": "0.0.2-reasoning", |
There was a problem hiding this comment.
| "version": "0.0.2-reasoning", | |
| "version": "0.0.2", |
Perhaps you should wait until the previous PR is merged before submitting a new one. |
|
This PR could/should have been done on the base code, because it is valid independently of the previous PR. |
Sure. |
2 participants
No description provided.