fix(core): ensure chat compression summary persists across session resumes#21345
fix(core): ensure chat compression summary persists across session resumes#21345Abhijit-2592 wants to merge 3 commits intomainfrom
Conversation
…sumes - Modified ChatRecordingService.initialize to accept an optional initialHistory parameter. - When initialHistory is provided during session resumption (e.g., after chat compression), it now overwrites the messages in the session file on disk. - Updated GeminiChat constructor to pass the history to ChatRecordingService.initialize. - Implemented apiContentToMessageRecords helper to convert API Content objects to storage-compatible MessageRecord objects. - This ensures that the compressed chat history (the summary) is immediately synced to disk, preventing it from being lost when the session is closed and resumed. - Added a unit test in chatRecordingService.test.ts to verify the new overwrite behavior. Fixes #21335
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 a critical issue where chat compression summaries were not being properly persisted across user sessions. By modifying the chat recording service to accept and save an initial history upon session initialization, it ensures that compressed chat states are correctly maintained, significantly improving the reliability of the 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
|
![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 addresses a bug where chat history compression summaries were not being persisted across sessions. The fix involves updating the ChatRecordingService to accept an initial history, which is then used to overwrite the session file on disk. The changes are logical, and a new unit test has been added to validate the fix. My review includes one suggestion to improve type safety in a new helper function by removing an unsafe type assertion.
|
Size Change: +1.59 kB (+0.01%) Total Size: 26 MB
ℹ️ View Unchanged
|
gemini-cli
bot
added
area/agent
joshualitt
dismissed
their stale review
More offline discussion required, will revisit soon.
… data loss - Modified ChatRecordingService.initialize to accept an explicit overwriteHistory flag. - When overwriteHistory is true (e.g., after chat compression), it overwrites disk messages; otherwise, it preserves existing historical metadata (IDs/timestamps). - Updated GeminiChat and GeminiClient.startChat to propagate this flag, ensuring it is only true during the compression flow. - Refactored apiContentToMessageRecords for full type safety by removing unsafe type assertions and adding part fallbacks, as suggested in PR review. - Updated unit tests in chatRecordingService.test.ts and client.test.ts to verify the new behavior and fix regression. - Verified all workspace and integration tests pass via preflight. Fixes #21335
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
From /review-frontend:
Summary
While this PR successfully fixes the bug where chat compression summaries weren't persisting across session resumes (Fixes #21335), the current implementation introduces a critical data-loss regression that severely breaks the CLI frontend UI.
By passing overwriteHistory=true, the ChatRecordingService re-synthesizes the entire preserved history from raw Gemini Content[] objects back into MessageRecord[]s via the newly added apiContentToMessageRecords function. This approach is fundamentally lossy and strips critical metadata required by the React frontend.
Findings
🔴 Critical UX/Frontend Regressions
- Breaks the Expandable Tool UI (
HistoryItemToolGroup): TheapiContentToMessageRecordsfunction entirely strips thetoolCallsarray (and its metadata likedisplayName,renderOutputAsMarkdown, etc.) from the preserved portion of the history (historyToKeepTruncated).- Frontend Impact: When the CLI UI resumes the session,
convertSessionToHistoryFormatsrelies onmsg.toolCallsto render the beautiful, expandableHistoryItemToolGroupcomponents. Without this metadata, tool invocations will fall back to rendering literal raw strings (e.g.,[Function Call: list_directory]and[Function Response: list_directory]) in standard message bubbles.
- Frontend Impact: When the CLI UI resumes the session,
- Breaks the "Thinking..." and Token UI: Similar to
toolCalls, thethoughtsarray andtokensobject are dropped from the preserved history.- Frontend Impact: The frontend will no longer be able to render the "Thinking..." dropdown or token usage stats for any historical messages preserved after compression.
- Destroys the Session Timeline:
apiContentToMessageRecordsloops through the retained history and assignsnew Date().toISOString()to every single message.- Frontend Impact: The entire preserved history will incorrectly appear in the UI as having occurred at the exact moment the compression happened, breaking the display of elapsed time and chronological flow.
- Role Mapping Mismatch: If a
Contentobject contains afunctionResponse(which usesrole: 'function'or sometimesrole: 'user'depending on normalization), it falls into theelseblock and is mapped totype: 'user'. The frontend specifically checks that tool calls aremsg.type !== 'user', further breaking the rendering pipeline.
Conclusion: Request Changes
The approach of rebuilding the session file from Content[] is fundamentally lossy because the Gemini SDK's Content objects do not carry the rich metadata that ChatRecordingService and the CLI UI require.
Instead of ChatRecordingService blindly overwriting the entire file using apiContentToMessageRecords, I recommend one of the following approaches:
- Slice the existing
MessageRecord[](Recommended): HaveChatCompressionServicecalculate how many messages were truncated/compressed. Then, pass that split index toChatRecordingService, which can slice its existingconversation.messagesarray (preserving all IDs, timestamps, and tool/thought metadata) and prepend a single, newly constructedMessageRecordfor the summary text. - Pass a Summary Event: Instead of a full overwrite, emit a "HistoryCompressed" event to the
ChatRecordingServicecontaining just the summary text and the number of messages to retain, allowing it to modify the JSON safely in place without losing the tail end of the history.
3 participants
Summary
This PR fixes a bug where the
/compresscommand's summary was not persistent across session exits and resumes.Details
The issue was that while
/compressupdated the in-memory chat history, it failed to sync this change to the session file on disk during the transition to a new chat session.Key changes:
ChatRecordingService.initialize: Updated to accept an optionalinitialHistoryparameter. When provided during session resumption (which happens internally after compression), it overwrites themessagesarray in the session file with the new history.GeminiChatConstructor: Updated to pass itshistoryto theChatRecordingService.initializecall, ensuring immediate persistence of the starting history (e.g., the compression summary).apiContentToMessageRecords: Added a private helper inChatRecordingServiceto convert APIContentobjects into storage-compatibleMessageRecordobjects.Related Issues
Fixes #21335
How to Validate
gemini./compressand wait for the "Chat history compressed" message.ctrl+dor/exit).gemini --resume latest.Alternatively, run the new unit test:
npm test -w @google/gemini-cli-core -- src/services/chatRecordingService.test.tsPre-Merge Checklist