Chat: allow large file with range in @-mentions #3619

abeatrix · 2024-03-29T18:30:56Z

CLOSE: #3589 (comment)
PART OF: https://github.com/sourcegraph/cody-issues/issues/9

Demo

updated.mov

This pull request updates the prompt editor to allow large files to be added to the context when they have a specified range. Previously, large files were not allowed to be added to the context file list. Now, large files with ranges can be included in the context.

##Changes

Update contextItemMentionNodeDisplayText in ContextItemMentionNode.ts:

Remove the condition that prevents large files from being added to the context.
Always display the file path and range (if available) for files.

Update Item component in OptionsList.tsx:

Modify the condition for showing the "File too large" warning to only display when the file is large and doesn't have a range specified.

Update MentionsPlugin in atMentions.tsx:

Differentiate between file selections and other selections.
For file selections, replace the node with the selected file path and range (if available).
If the selected file is large and doesn't have a range, append a : after the file path to allow adding line range.
Create a mention node for the selected item and insert it into the document.

Limitation

Users will not see the large file warning on large file with range input because we will need to get the range numbers first before we can check the token usage, which should be part of the works when we start working on improving our context window logic.

Tracking issue: https://github.com/sourcegraph/cody-issues/issues/9

Test plan

Search for a large file via @-mentions:

Press tab, verify : is added to the end of the file path. And then add file range:

Hit tab / space again, and the large file is now added as a context item token:

Verify that selecting files that are not too large should automatically add to input box as token.

kalanchan

edit: uploading proper video

kalanchan

The @ file with line numbers still got excluded in the context, did I test this correctly?

also, it feels a bit weird to backspace after hitting tab/enter to enter in the line numbers, would it be better for large files to not have that empty space and let the user start selecting the line numbers right afterwards?

large-file-excluded.mp4

abeatrix · 2024-03-29T22:01:21Z

The @ file with line numbers still got excluded in the context, did I test this correctly?

also, it feels a bit weird to backspace after hitting tab/enter to enter in the line numbers, would it be better for large files to not have that empty space and let the user start selecting the line numbers right afterwards?

large-file-excluded.mp4

I don't think that's related to my changes in this PR, since we can see the file was appended to the chat correctly, but got excluded by the chat context window since i suspect it's because you are using the smaller claude 3 model? Can you try Claude 3 Opus?

Mentioned in the Limitation section about more work need to be on the range for large files as well, which we can look into when we start the works on context windows? We did add a UI to display when an @-file is excluded tho

kalanchan · 2024-03-29T22:16:11Z

I think the current limitation is a bit confusing for the user, I don't know who to trust if I'm able to @ file a large file with range but the UI says it got excluded from context 😓

@toolmantim @chillatom what do you all think? I think we need to address the limitations section either in the same PR or a follow up right afterwards

abeatrix · 2024-03-29T22:29:34Z

I don't know who to trust if I'm able to @ file a large file with range but the UI says it got excluded from context

It was part of @toolmantim 's design and the engineering work on "We should not silently exclude a file that has been explicitly @ mentioned" 😅 We also have users asking to enable to @ line numbers again after we have disabled it on large files: #3523 & https://discord.com/channels/969688426372825169/1222845844563296317

@toolmantim @chillatom @kalanchan I'll need more information on how we want the context window works before we can address this limitation too. I've sent you all an invite for next week so we can discuss live!

Edit: I've also added some new issues to this tracking issues https://github.com/sourcegraph/cody-issues/issues/9

abeatrix · 2024-04-01T16:56:46Z

@kalanchan You were right there was a bug with the display logic where used @-mention items are showing up as excluded when the file was originally too large. That's why in your screenshot Cody was still able to answer the question.

I've fixed that so that all used files shouldn't be marked as large file:

Also updated to include :1-10 when user try to include a large file to give them a head start:

Also added warning to let them know files will still be excluded if the context from the selected range is too large for now.

abeatrix · 2024-04-02T04:02:16Z

@toolmantim @chillatom updated the PR so that it matches the behavior we discussed earlier for Wednesday release: selecting a large file from the option list will add the file path to the input as text so that users can choose to add range after the auto-filled file path or remove it. Selecting files that are not large file will automatically turn it into a token.

Follow-up from this would be allowing users to add selection from editor as @-mentions via right click menu. (Related issue: #3612)

Let me know if I'm missing something here, thanks!

toolmantim

Looking good! I found a few things:

Items shouldn't look disabled now they're clickable. Can we bring back up the opacity and cursor etc?
When you select an item, can we complete the path, add : to the end, make sure it's the only item in the menu, and give them some hint for how to type the range? At the moment you need to manually type the : and then it starts doing a line-range to all items:
When you add : can we replace the "File is too large ..." file subtext with something more helpful like "Type a line range to include, e.g. 1-42":
Let's remove the "File will be excluded if context range exceeds limit" message. Could we implement a maximum number of lines you can do, and only if they exceed it we show the message, rather than warn everyone ahead of time?
If you type a line range (e.g. :1-42) and remove the lines, leaving only :, it seems to show an old cached value:

umpox · 2024-04-02T12:58:20Z

Agree with Tim's comments about the disabled text.

Noticed that this doesn't work with the Edit input right now, is it supposed to?

We can do this in a follow up but we should not show the same warning if so:

abeatrix · 2024-04-02T15:41:48Z

@toolmantim @umpox Thanks for the review! I've updated the PR to address the following changes:

Items shouldn't look disabled now they're clickable. we bring back up the opacity and cursor etc?
- Opacity updated from 0.5 to 0.8. Let me know if you think we should just remove it.
When you select an item, can we complete the path, add : to the end,
make sure it's the only item in the menu
give them some hint for how to type the range
- Updated title to “Type a line range to include, e.g. 1-42”
When you add : can we replace the "File is too large ..." file subtext with something more helpful like "Type a line range to include, e.g. 1-42":
- Updated title to “Type a line range to include, e.g. 1-42”
Let's remove the "File will be excluded if context range exceeds limit" message.
If you type a line range (e.g. :1-42) and remove the lines, leaving only :, it seems to show an old cached value:
Let's remove the "File will be excluded if context range exceeds limit" message
Could we implement a maximum number of lines you can do, and only if they exceed it we show the message, rather than warn everyone ahead of time?
- This would be in the scope for the works we are starting in this sprint right? To find the right number?

Noticed that this doesn't work with the Edit input right now, is it supposed to?

@umpox This is only updating the UI for chat, so I don't suppose the changes in this PR will work with Edit which uses a different UI. Will create an issue for Edit

Demo

updated.mov

sqs

Code LGTM with comments, have not tried this yet but I see other people have

vscode/CHANGELOG.md

vscode/src/edit/input/get-input.ts

vscode/webviews/promptEditor/plugins/atMentions/OptionsList.tsx

Co-authored-by: Quinn Slack <[email protected]>

vscode/src/edit/input/get-input.ts

approved

toolmantim · 2024-04-02T23:34:38Z

vscode/webviews/promptEditor/plugins/atMentions/OptionsList.module.css

@@ -72,7 +72,7 @@
    background-color: var(--vscode-list-hoverBackground);
 }
 .option-item.disabled {
-    opacity: 0.5;


Yeah, let's not change the opacity at all, as we have the label and don't want it to feel disabled. We could create a custom icon if we really wanted, but I think the label is good.

We could add a right chevron to the right hand side ">" to show that you don't be in fact selecting the item, but "going into it"

@toolmantim Thanks! removing opacity in #3665

hpn1975 · 2024-04-06T06:39:03Z

I am sorry.. but for a paid member keep getting this error is annoying... the file has only 75 lines of code.

I am not sure what you guys are fixing or what is fixed. And having to enter the line number is so annoying..

abeatrix added 7 commits March 28, 2024 17:20

Chat: fix @-mention range title on input

5f2e4b4

add autocomplete for context item mentions

a7ba056

clean up

26b482d

fix tests 1

18a7cf8

remove .only

5796d46

@mentions: allow large file with range as context

09942dc

fix test

28ceb45

abeatrix changed the base branch from main to bee/at-range March 29, 2024 18:34

Merge branch 'bee/at-range' into bee/allow-range

6f94c74

kalanchan approved these changes Mar 29, 2024

View reviewed changes

kalanchan previously requested changes Mar 29, 2024

View reviewed changes

Base automatically changed from bee/at-range to main April 1, 2024 15:10

abeatrix added 2 commits April 1, 2024 08:15

Merge branch 'main' into bee/allow-range

56ce87e

Fix excluded file UI bug and add range tips

99fa7b9

abeatrix added 2 commits April 1, 2024 14:42

double tab required for large file only

6d488dc

update e2e test to remove double tab

7d98a22

abeatrix mentioned this pull request Apr 1, 2024

Chat: remove double tab token #3651

Closed

remove example range

5ecce76

abeatrix requested review from kalanchan and a team April 1, 2024 23:57

abeatrix requested a review from umpox April 2, 2024 11:33

toolmantim reviewed Apr 2, 2024

View reviewed changes

update from feedback

560a7a7

abeatrix requested a review from toolmantim April 2, 2024 15:43

abeatrix added 3 commits April 2, 2024 08:49

update

15a908e

clean up

2e8f56f

update changelog

343d5dd

sqs approved these changes Apr 2, 2024

View reviewed changes

vscode/CHANGELOG.md Outdated Show resolved Hide resolved

vscode/src/edit/input/get-input.ts Outdated Show resolved Hide resolved

vscode/webviews/promptEditor/plugins/atMentions/OptionsList.tsx Outdated Show resolved Hide resolved

abeatrix and others added 2 commits April 2, 2024 15:00

Apply suggestions from code review

87cb975

Co-authored-by: Quinn Slack <[email protected]>

typo

5517d09

dominiccooney reviewed Apr 2, 2024

View reviewed changes

vscode/src/edit/input/get-input.ts Outdated Show resolved Hide resolved

Terminate string literal to run CI

8ef5ded

abeatrix merged commit c700d67 into main Apr 2, 2024
20 checks passed

abeatrix deleted the bee/allow-range branch April 2, 2024 23:28

toolmantim reviewed Apr 2, 2024

View reviewed changes

This was referenced Apr 2, 2024

remove opacity from optionlist #3665

Merged

Chat: add ability to send editor selection to chat as @-mention #3713

Merged

This was referenced Apr 10, 2024

Edit: display warning for large @-mentions #3767

Merged

feedback: feature request: Right click to mention line range in chat #3612

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chat: allow large file with range in @-mentions #3619

Chat: allow large file with range in @-mentions #3619

abeatrix commented Mar 29, 2024 •

edited

Loading

kalanchan left a comment •

edited

Loading

kalanchan left a comment •

edited

Loading

abeatrix commented Mar 29, 2024 •

edited

Loading

kalanchan commented Mar 29, 2024

abeatrix commented Mar 29, 2024 •

edited

Loading

abeatrix commented Apr 1, 2024 •

edited

Loading

abeatrix commented Apr 2, 2024 •

edited

Loading

toolmantim left a comment

umpox commented Apr 2, 2024

abeatrix commented Apr 2, 2024 •

edited

Loading

sqs left a comment

toolmantim Apr 2, 2024

abeatrix Apr 2, 2024

hpn1975 commented Apr 6, 2024

Chat: allow large file with range in @-mentions #3619

Chat: allow large file with range in @-mentions #3619

Conversation

abeatrix commented Mar 29, 2024 • edited Loading

Demo

Limitation

Test plan

kalanchan left a comment • edited Loading

Choose a reason for hiding this comment

kalanchan left a comment • edited Loading

Choose a reason for hiding this comment

abeatrix commented Mar 29, 2024 • edited Loading

kalanchan commented Mar 29, 2024

abeatrix commented Mar 29, 2024 • edited Loading

abeatrix commented Apr 1, 2024 • edited Loading

abeatrix commented Apr 2, 2024 • edited Loading

toolmantim left a comment

Choose a reason for hiding this comment

umpox commented Apr 2, 2024

abeatrix commented Apr 2, 2024 • edited Loading

Demo

sqs left a comment

Choose a reason for hiding this comment

toolmantim Apr 2, 2024

Choose a reason for hiding this comment

abeatrix Apr 2, 2024

Choose a reason for hiding this comment

hpn1975 commented Apr 6, 2024

abeatrix commented Mar 29, 2024 •

edited

Loading

kalanchan left a comment •

edited

Loading

kalanchan left a comment •

edited

Loading

abeatrix commented Mar 29, 2024 •

edited

Loading

abeatrix commented Mar 29, 2024 •

edited

Loading

abeatrix commented Apr 1, 2024 •

edited

Loading

abeatrix commented Apr 2, 2024 •

edited

Loading

abeatrix commented Apr 2, 2024 •

edited

Loading