Skip to content

chat: preserve transcript order when refreshing history - #339458

Open
Osvaldo Ortega (osortega) wants to merge 2 commits into
mainfrom
osortega/agents/preserve-chat-history-order
Open

Osvaldo Ortega (osortega) wants to merge 2 commits into
mainfrom
osortega/agents/preserve-chat-history-order

Conversation

@osortega

Copy link
Copy Markdown
Contributor

Summary

Refreshing an older response could move it below a newer locally sent message. The history merger removed and rebuilt the changed suffix, kept local requests in place, and appended the rebuilt turns at the end. Even an elapsed-time or usage update could change the visible conversation order.

  • Match history turns by request ID and replace changed restored turns in their existing model/view position.
  • Insert newly loaded history before its next known turn, preserving unchanged and locally created request/response objects.
  • Keep drafts and the existing deferral while a response is streaming; repeated snapshots do not duplicate requests.
  • Dispose superseded response listeners and refresh last-request state and session cost after history removals.
  • Add regressions using the real chat service, model, and view model for metadata/content updates, older history, local echoes, missing IDs, streaming, and removals.

This is independent of the authentication-recovery changes in #339454. It does not change the protocol client, authentication, or host-side persistence.

Validation

  • Confirmed the ordering regressions fail before the fix: [README, hi] becomes [hi, README] despite correctly ordered incoming history.
  • Reproduced and fixed both review-found removal edge cases; follow-up review found no remaining blocking issues.
  • 319 related tests pass in Electron.
  • 318 browser-applicable tests pass in each of Chromium, Firefox, and WebKit.
  • Targeted TypeScript diagnostics, ESLint, whitespace checks, and pre-commit hygiene pass.
  • The maintainer reported that the earlier combined history/authentication test snapshot no longer reproduced the issue. The final removal-only corrections have automated validation, not a second manual retest.

Manual verification

Open a restored remote chat, send and complete a new message, then trigger a history refresh (for example, return after switching apps). Confirm earlier turns remain above the newer message, response text stays visible, and the unsent draft is preserved. Also load older history and verify it is inserted before the existing turns without duplication.

A turn also missing from another client's history is not proof of a display-order bug; this PR does not claim to repair missing host-side history.

Reconcile refreshed history by request identity and replace changed turns in their existing model and view positions instead of appending them after local messages. Preserve drafts, local responses, and streaming deferral while supporting older history insertion.

Dispose superseded response listeners and publish removals after updating the request list so last-request state and session cost remain current. Add real service/model/view regressions for ordering, identity, streaming, and history removals.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 3, 2026 07:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

ID-less history prepends can reassign generated identities to the wrong turns.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Preserves chat transcript ordering during passive history refreshes while retaining local turns and drafts.

Changes:

  • Reconciles history by request ID and inserts turns at stable positions.
  • Updates model/view replacement and disposal handling.
  • Adds regression coverage for ordering, streaming, removals, and metadata updates.
File Description
chatServiceImpl.ts Reconciles refreshed history turns.
chatModel.ts Supports indexed request replacement.
chatViewModel.ts Maintains view order and listener lifetimes.
chatInputPart.ts Refreshes cost state and response listeners.
chatService.test.ts Adds history-refresh regressions.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts Outdated
@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Screenshot Changes

Base: 253b7648 Current: 486c5d52

Changed (2)

chat/aiCustomizations/aiCustomizationManagementEditor/DiscoverInfiniteScroll/Light
Before After
before after
chat/aiCustomizations/aiCustomizationManagementEditor/DiscoverPluginsLoadingMore/Light
Before After
before after

2 insignificant change(s) omitted (≤20 px, Δ≤2). See CI logs for details.

Replace positional fallback with unique request-content matching in both the previous and incoming history. Preserve provider IDs, avoid assigning ambiguous duplicates another turn's local identity, and keep identical history refreshes as no-ops.

Cover ID-less history prepends, removals, and one-to-many and many-to-one ambiguous matches with real model/view regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants