feat(cli): add model token usage indicator to footer and fix stream handling#19646
feat(cli): add model token usage indicator to footer and fix stream handling#19646christianmerkwirth wants to merge 5 commits intogoogle-gemini:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @christianmerkwirth, 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 enhances the CLI's user experience by introducing a visible token usage indicator in the footer, allowing users to monitor their model interactions more effectively. Concurrently, it significantly improves the stability and error handling of the Highlights
Changelog
Ignored Files
Activity
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
The pull request introduces a model token usage indicator in the CLI footer and improves the robustness of stream handling in CodeAssistServer. The changes are well-structured and include comprehensive tests. However, there are a few critical issues in the manual SSE parsing logic and resource management that should be addressed to ensure correctness and prevent potential memory leaks.
|
Hi @christianmerkwirth thanks for the PR 😄 Quick note, we try and have PRs follow the separation of concerns pattern. i.e. they should fix one thing and one thing only. That way there is a clean history and if we need to revert something we can do it easily without having to pull out pieces of logic. For this PR the stream handling fix should be fully separate from the footer indicator as they are unrelated to one another. My personal rule of thumb is that whenever you start adding "and" to a PR title it should most likely be split into two PRs 😁. Also I have a PR up to add Thus the So I think the best course of action is to remove the footer piece entirely from this PR and have it focus on the stream handling issue. Hope this makes sense. Thanks a ton for the contribution! |
Summary
Add a model token usage indicator to the CLI footer and fix a crash in
CodeAssistServerrelated to non-Node stream responses.Details
getShortDisplayStringinpackages/coreto provide compact model abbreviations (e.g., '3P', '2.5F').TokenUsageIndicatorcomponent inpackages/cli.ui.footer.showTokenUsagesetting (default:true) to allow users to toggle the display.CodeAssistServerto robustly handle stream responses by wrapping async iterables (like WebReadableStream) into Node.jsReadablestreams.input.on is not a functioncrash.contentGenerator.ts.Related Issues
input.on is not a functioncrash.How to Validate
npm start.3P: 1.2k)./model proor/model flashand verify usage updates./stats modelto confirm the numbers match.settings set ui.footer.showTokenUsage false.Pre-Merge Checklist