Enhance draft input persistence for remote chat sessions#311312
Enhance draft input persistence for remote chat sessions#311312DonJayamanne wants to merge 1 commit intomainfrom
Conversation
Screenshot ChangesBase: Changed (5)
|
| Before | After |
|---|---|
 |
 |
editor/inlineCompletions/other/JumpToHint/Dark
| Before | After |
|---|---|
 |
 |
agentSessionsViewer/CompletedUnread/Dark
| Before | After |
|---|---|
 |
 |
agentSessionsViewer/CompletedUnread/Light
| Before | After |
|---|---|
 |
 |
agentSessionsViewer/ApprovalRowLongLabel/Light
| Before | After |
|---|---|
 |
 |
There was a problem hiding this comment.
Pull request overview
This PR improves draft (unsent) chat input persistence for remote/external chat sessions so that switching away from a session and back restores the user’s pending input state. It also adds a regression test validating draft restoration after disposing/reloading a remote session.
Changes:
- Persist serialized draft
inputStatefor external sessions in session metadata and restore it when loading remote sessions. - Persist external session metadata on dispose when there are requests or an unsent draft.
- Add a unit test covering draft restoration for a remote session after disposal/reload.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/test/common/chatService/chatService.test.ts | Adds a test to ensure a remote session’s draft input text is restored after dispose/reload. |
| src/vs/workbench/contrib/chat/common/model/chatSessionStore.ts | Extends session metadata to include serialized draft inputState for external sessions. |
| src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts | Persists metadata for external sessions when there’s a draft; restores draft from stored metadata on remote session load. |
| src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts | Adds a public method to flush view input state into the bound input model. |
| src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts | Calls the flush helper before dropping the model reference (currently only in the setModel(undefined) path). |
Copilot's findings
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts:210
- The dispose-time condition
model.getRequests().length > 0 || hasDraftInput(model)can leave stalemetadata.inputStatebehind for empty external sessions. Example: type a draft (persisted), reopen, clear the draft back to empty, switch away ->hasDraftInputbecomes false and metadata is no longer written, so the old draft will be restored again next open. Consider persisting metadata for external sessions when there is any existing storedinputStateto clear, or basing the decision on whetherinputModel.toJSON()has changed rather than only whether it’s currently non-empty.
} else if (!localSessionId && (model.getRequests().length > 0 || hasDraftInput(model))) {
// External sessions: persist metadata when there are requests, OR when the
// user has typed/attached unsent input we need to restore on next open.
await this._chatSessionStore.storeSessionsMetadataOnly([model]);
}
- Files reviewed: 5/5 changed files
- Comments generated: 3
| if (!model) { | ||
| // Flush any unsent draft to the outgoing input model before we drop our | ||
| // reference to it, so the host's `willDisposeModel` persistence sees it. | ||
| this.inputPart.flushInputStateToModel(); | ||
| if (this.viewModel?.editing) { |
There was a problem hiding this comment.
setModel() flushes draft input only when called with undefined. During normal session switches (old model -> new model), callers pass the new model directly, so the outgoing draft may never be synced to the model before the reference is released and willDisposeModel runs. Consider flushing whenever the session resource is changing (i.e. before switching away from an existing viewModel), not only when clearing the model.
| /** | ||
| * True when the user has typed text or attached non-trivial context to the input | ||
| * but not yet sent it. Used to decide whether an external session needs metadata | ||
| * persisted on dispose so the draft survives switching sessions. | ||
| */ | ||
| function hasDraftInput(model: ChatModel): boolean { | ||
| const state = model.inputModel.state.get(); | ||
| if (!state) { | ||
| return false; | ||
| } | ||
| if (state.inputText.trim().length > 0) { | ||
| return true; | ||
| } | ||
| return state.attachments.length > 0; | ||
| } |
There was a problem hiding this comment.
hasDraftInput() only considers non-whitespace text and attachments. If the user changes other input state (e.g. mode, selected model, permission level, or contrib state) without text/attachments, the session can still have meaningful draft state but metadata won’t be persisted for external sessions with no requests, so those selections can be lost when switching sessions.
This issue also appears on line 206 of the same file.
| const isExternal = session instanceof ChatModel && !LocalChatSessionUri.parseLocalSessionId(session.sessionResource); | ||
| // Persist draft input state only for external sessions; local sessions already | ||
| // have their full state serialized via storeSessions, so duplicating here would | ||
| // be wasteful and risk drift between the two locations. | ||
| const inputState = isExternal ? (session as ChatModel).inputModel.toJSON() : undefined; | ||
|
|
There was a problem hiding this comment.
inputState is stored in the session index (storageService) for external sessions. Since InputModel.toJSON() exports attachments via IChatRequestVariableEntry.toExport, this can include base64-encoded Uint8Array payloads (e.g. pasted images), significantly increasing the size of the index entry. External sessions are also excluded from trimEntries(), so this data can accumulate without bound. Consider filtering/limiting binary attachment data for metadata storage, or storing draft input state in a separate, size-managed location.
Improve the handling of unsent draft inputs in remote chat sessions. Ensure that draft inputs are preserved when switching sessions, allowing users to retain their input state. Add tests to verify that draft inputs are correctly restored after disposing and reloading a session.
Fixes #311310