fix: clean up /clear and /resume#22007
Conversation
|
Size Change: +3.37 kB (+0.01%) Total Size: 26.5 MB
ℹ️ View Unchanged
|
Summary of ChangesHello, 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 critical bugs in the Gemini CLI's session management, specifically preventing Highlights
Changelog
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
This pull request effectively addresses two critical bugs related to session management. The fix for the /clear command preventing session data from being overwritten is well-reasoned, correctly reordering session ID generation before the chat reset. The addition of uiTelemetryService.clear() on session resume is a clean solution to the metric aggregation issue. The related changes in ChatRecordingService, UiTelemetryService, and SessionContext are all necessary and correctly implemented to support these fixes. The test files have also been updated appropriately to cover the new logic. Overall, this is a solid contribution that improves the stability and correctness of the CLI.
mattKorwel
left a comment
There was a problem hiding this comment.
This change looks solid, thanks for adding. I think we need to follow on and figure out how we relate lifetime of terminal stats now that this is properly cleared out.
Summary
This PR fixes critical session management bugs in the Gemini CLI where
/clearwould inadvertently overwrite previous session data and/resumewould fail to reset UI telemetry, leading to aggregated metrics.New footer helped me noticed session was not updating during
/clearand that session was getting aggregated...clear.race.condition.mp4
After fix:
clear.fix.mp4
Details
/clearoverwriting sessions: InclearCommand.ts, the session ID generation was moved before the chat reset. This ensures that the newly instantiatedGeminiChat(and its internalChatRecordingService) uses the new session ID immediately, creating a distinct file on disk instead of overwriting the previous one./resume: Added auiTelemetryService.clear()call inuseSessionBrowser.ts. This triggers a full reset of token counts and updates the session ID in the React UI footer when an older session is loaded.ChatRecordingService: Updated to refresh its localsessionIdfrom the configuration during initialization.UiTelemetryService: Implemented aclear()andhydrate()method to reset accumulated metrics and broadcast the change. Hydrate repopulates metrics on resume so that/statsand footer field are loaded.SessionContext: Added an event listener to respond to telemetry clear events, ensuring the UI stays in sync.Related Issues
Fixes the reported issue where
/clearcaused old sessions to vanish from the/resumelist and token counts were aggregated across sessions.How to Validate
hello). Note the session ID and token count in the footer./clear./resume.Pre-Merge Checklist