Skip to content

fix tip dismissal#296058

Merged
meganrogge merged 5 commits intomainfrom
merogge/dismiss-tips
Feb 18, 2026
Merged

fix tip dismissal#296058
meganrogge merged 5 commits intomainfrom
merogge/dismiss-tips

Conversation

@meganrogge
Copy link
Collaborator

fixes #296057

Dismissed tips were stored as a plain array, so duplicate IDs built up. A recovery check then treated “too many dismissed” as “none dismissed,” and a fallback always picked a tip, so previously dismissed tips could reappear after re-enabling tips.

We now dedupe persisted dismissals, only keep valid tip IDs, remove the reset-to-empty behavior, and return no tip when all are dismissed. That keeps dismissals persistent across disable/re-enable, and we added a regression test to lock this behavior.

Copilot AI review requested due to automatic review settings February 18, 2026 17:45
@vs-code-engineering vs-code-engineering bot added this to the February 2026 milestone Feb 18, 2026
@meganrogge meganrogge marked this pull request as draft February 18, 2026 17:48
Copy link
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 fixes a regression where dismissed tips would reappear after users re-enabled the tips feature. The issue was caused by duplicate tip IDs accumulating in storage (since dismissals were stored as a plain array), combined with a "safety valve" that reset dismissals when all tips were marked as dismissed, and a fallback that always returned a tip.

Changes:

  • Added deduplication using Set when storing and retrieving dismissed tip IDs
  • Added validation to only keep valid tip IDs (those in TIP_CATALOG)
  • Removed the "safety valve" that reset dismissals to empty when all tips were dismissed
  • Changed behavior to return undefined instead of a fallback tip when all tips are dismissed
  • Added regression test to verify dismissed tips stay dismissed across disable/re-enable cycles

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/vs/workbench/contrib/chat/browser/chatTipService.ts Implemented deduplication in dismissTip() and _getDismissedTipIds(), removed reset logic and fallback tip selection
src/vs/workbench/contrib/chat/test/browser/chatTipService.test.ts Added regression test verifying dismissed tips persist after disabling and re-enabling tips

@meganrogge meganrogge marked this pull request as ready for review February 18, 2026 18:32
@meganrogge meganrogge requested a review from Copilot February 18, 2026 18:32
@meganrogge meganrogge enabled auto-merge (squash) February 18, 2026 18:32
Copy link
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

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

@meganrogge
Copy link
Collaborator Author

Woah, something went wrong here 😅

@meganrogge meganrogge closed this Feb 18, 2026
auto-merge was automatically disabled February 18, 2026 19:17

Pull request was closed

@meganrogge meganrogge reopened this Feb 18, 2026
@meganrogge meganrogge merged commit 67c89aa into main Feb 18, 2026
22 of 33 checks passed
@meganrogge meganrogge deleted the merogge/dismiss-tips branch February 18, 2026 19:37
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.

Tips toolbar: Dismissed tips are not staying dismissed when feature is re-enabled

2 participants

Comments