Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(chat): remove current selection from initial mention #7161

Merged
merged 3 commits into from
Feb 20, 2025

Conversation

abeatrix
Copy link
Contributor

@abeatrix abeatrix commented Feb 20, 2025

This commit fixes a regression from #7115, which aims to remove the current selection from being added as part of initial mentions chips in first message, but removed the option completely by mistake.

  • Adding the current text selection as a context item provided to the chat webview, but removes it from the initial at-mention context chips

Test plan

  1. Core Functionality Tests:
  • Select code in the editor and start a new chat
    • Expected: Selection should NOT appear in initial @-mention chips
    • Expected: Selection should be available as context in chat webview
  • Verify selection content is properly passed to backend
    • Expected: Selection data exists in context payload
  1. Regression Prevention:
  • Start chat without any selection
    • Expected: Chat should initialize normally
    • Expected: @-mention chips should work as normal
  1. Edge Cases:
  • Test with large selections (>1000 characters)
  • Test with multi-line code selections
  • Test selections containing special characters/unicode
  • Test with selections across different file types
  1. Integration Testing:
  • Verify selection context works with other context sources
  • Check selection persistence across chat sessions
  • Verify selection can be referenced in follow-up messages
  1. UI Verification:
  • Verify chat UI displays context correctly
  • Check @-mention chips visual appearance
  • Verify selection context is accessible but not in initial chips

This commit fixes a regression from #7115, which aims to remove the current selection from being added as part of initial mentions chips in first message, but removed the option completely by mistake.

-   Adding the current text selection as a context item provided to the chat webview, but removes it from the initial at-mention context chips

## Test Plan

Selection Behavior Tests
Select code in the editor and start a new chat
Verify the selection is NOT shown in the initial @-mention chips
Verify the selection IS available as context in the chat webview
Regression Prevention Tests
Start a chat without any selection
Start a chat with multiple selections
Verify @-mention chips work correctly in both cases
Integration Tests
Test interaction between selection and other context sources
Verify selection context is properly passed to the chat backend
Edge Cases
Test with very large selections
Test with multi-line selections
Test with selections containing special characters
UI Verification
Verify the chat UI correctly displays the context without the initial selection in chips
Verify the selection can still be referenced in follow-up messages
@abeatrix abeatrix requested review from arafatkatze, a team and julialeex February 20, 2025 15:49
@abeatrix abeatrix enabled auto-merge (squash) February 20, 2025 17:07
Copy link
Contributor

@arafatkatze arafatkatze left a comment

Choose a reason for hiding this comment

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

The selection not being present in the new chat behavior has been verified.

NOTE: when you have an existing chat and if you click on Opt+L it would toggle rather than start a new chat. IF that's the expected behavior(instead of new chat) then this PR is okay.

@abeatrix abeatrix merged commit ad4753a into main Feb 20, 2025
23 checks passed
@abeatrix abeatrix deleted the bee/fix-selection-context branch February 20, 2025 17:57
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