Skip to content

sessions: make navigation history size observable#315391

Merged
sandy081 merged 1 commit into
mainfrom
sandy081/sessions-nav-historysize
May 8, 2026
Merged

sessions: make navigation history size observable#315391
sandy081 merged 1 commit into
mainfrom
sandy081/sessions-nav-historysize

Conversation

@sandy081
Copy link
Copy Markdown
Member

@sandy081 sandy081 commented May 8, 2026

Follow-up to #315382.

Addresses two review comments from Copilot:

  1. _history.length is not observable_canGoForward derived read this._history.length (a plain array), so mutations that don't change _currentIndex (e.g. session removal at a position after the current index) could leave the derived stale. Fix: add a _historySize observable, update it in all mutation paths (_pushEntry, _navigateTo, _removeEntriesMatching), and read it in the _canGoForward derived.

  2. Misleading comment_navigateTo said "remove it and try again" but the code only removes the missing entry without retrying. Updated the comment to match the actual behavior.

- Add _historySize observable so _canGoForward derived recomputes when
  history is mutated without changing _currentIndex (e.g. session removal)
- Update _historySize in all mutation paths: _pushEntry, _navigateTo,
  _removeEntriesMatching
- Fix misleading 'remove and try again' comment in _navigateTo — the code
  only removes the missing entry without retrying

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 8, 2026 22:40
@sandy081 sandy081 enabled auto-merge (squash) May 8, 2026 22:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR follows up on the Agents window session navigation history implementation by making the history size observable (so canGoForward updates correctly when the backing array changes) and by correcting an inaccurate comment in the missing-session navigation path.

Changes:

  • Introduces _historySize as an observable and uses it in the _canGoForward derived.
  • Updates all history mutation paths to keep _historySize in sync.
  • Adjusts a comment in _navigateTo to match the actual behavior when a session no longer exists.
Show a summary per file
File Description
src/vs/sessions/services/sessions/browser/sessionNavigation.ts Adds an observable for history length to keep forward-navigation context state correct and updates comments around missing-session handling.

Copilot's findings

Comments suppressed due to low confidence (1)

src/vs/sessions/services/sessions/browser/sessionNavigation.ts:220

  • _removeEntriesMatching can set _currentIndex to 0 even when all history entries are removed (because newCurrentIdx can become -1 but is clamped via Math.max(0, ...)). This makes the index point to a non-existent entry and can incorrectly keep Back enabled if _beyondHistory is true. Handle the empty-history case explicitly (e.g. set _currentIndex to -1 and clear _beyondHistory when this._history.length === 0).
		if (newCurrentIdx !== currentIdx) {
			this._currentIndex.set(Math.max(0, newCurrentIdx), undefined);
		}
		this._historySize.set(this._history.length, undefined);
	}
  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment thread src/vs/sessions/services/sessions/browser/sessionNavigation.ts
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

blocks-ci screenshots changed

Replace the contents of test/componentFixtures/blocks-ci-screenshots.md with:

Updated blocks-ci-screenshots.md
<!-- auto-generated by CI — do not edit manually -->

#### editor/codeEditor/CodeEditor/Dark
![screenshot](https://hediet-screenshots.azurewebsites.net/images/67bfb687fd2818bd53771a60660541b9ed6f38b80d37da0aac15d267ecaeacec)

#### editor/codeEditor/CodeEditor/Light
![screenshot](https://hediet-screenshots.azurewebsites.net/images/0469dd8d0a587d94a1eaec514c79917b93b9a38694ef2b767bb1892819ae0a55)

#### editor/inlineChatZoneWidget/InlineChatZoneWidget/Dark
![screenshot](https://hediet-screenshots.azurewebsites.net/images/97162fc53c861ee13dc78a18e41fe3a25a42f62dc52a560510ebf084a418e1c3)

#### editor/inlineChatZoneWidget/InlineChatZoneWidget/Light
![screenshot](https://hediet-screenshots.azurewebsites.net/images/3b7e2eb5cc9ba727e2bc1c5113c3e17d8e9a6ce9a37b77519be3716ceb9a9afa)

#### editor/inlineChatZoneWidget/InlineChatZoneWidgetTerminated/Dark
![screenshot](https://hediet-screenshots.azurewebsites.net/images/97162fc53c861ee13dc78a18e41fe3a25a42f62dc52a560510ebf084a418e1c3)

#### editor/inlineChatZoneWidget/InlineChatZoneWidgetTerminated/Light
![screenshot](https://hediet-screenshots.azurewebsites.net/images/3b7e2eb5cc9ba727e2bc1c5113c3e17d8e9a6ce9a37b77519be3716ceb9a9afa)

@sandy081 sandy081 merged commit 767fc35 into main May 8, 2026
28 of 29 checks passed
@sandy081 sandy081 deleted the sandy081/sessions-nav-historysize branch May 8, 2026 22:58
@vs-code-engineering vs-code-engineering Bot added this to the 1.120.0 milestone May 8, 2026
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.

3 participants