Feature Flag: Hover Commands #3585

abeatrix · 2024-03-27T20:02:30Z

Description

This pull request introduces a new feature to display Cody commands in a pop-up window when hovering over code. The feature is controlled by a new feature flag cody-hover-commands and can be enabled/disabled via user configuration.

The feature is currently controlled by a new feature flag, cody-hover-commands on DotCom; however, users with the feature flag enabled can turn off this feature via VS Code configuration with:

"cody.experimental.hoverCommands": false,

NOTE: Configuring this setting alone will not enable the feature as it depends on the feature flag.

New events:

CodyVSCodeExtension:experiment:hoverCommands:enrolled
- First the the hover command is displayed
- Log once per users' lifetime
CodyVSCodeExtension:hoverCommands:visible
- When a hover command is visible
CodyVSCodeExtension:hoverCommands:clicked
- When a hover command is clicked
All commands invoked by clicking on the hover commands will have event source set to hover.

Test plan

Connect to DotCom in Cody with your Sourcegraph account
In Cody, open a file and hover over some code to see the Cody Commands showing up in the pop-up

Expected Behavior

When hovers over a top-level symbol, the Doc and Explain commands will show up.
When hovers over a highlighted block of code with multiple lines, or variables, the Chat and Edit command will show up.
When hovers over code with problem, the Explain Code command will show up.

Configurable for users with feature flag enabled

Users without the feature flag shouldn't be able to see this option.

Ghost Text

Users with the hover commands feature flag will have Ghost Text disabled.

Telemetry Events

CodyVSCodeExtension:experiment:hoverCommands:enrolled

Requirement: "They should be fired as soon as possible for the user and ideally once in the user's lifetime."

All users will log this event the first time they initiate the extension that has this feature built-in.
Users who are part of the experiment are logged as the "treatment" variant (v.s. "control")

█ logEvent (telemetry disabled): CodyVSCodeExtension:experiment:hoverCommands:enrolled  {
  "details": "VSCode",
  "properties": {
    "variant": "treatment"
  },
  "opts": {
    "hasV2Event": true
  }
}
█ telemetry-v2: recordEvent: cody.experiment.hoverCommands/enrolled  {
  "parameters": {
    "version": 0,
    "privateMetadata": {
      "variant": "treatment"
    }
  },
  "timestamp": "2024-03-27T21:44:48.753Z"
}

umpox

This is an awesome idea, and I totally agree that there's a lot of value to be unlocked by catering for mouse-only users.

I have left some comments, I think as this is a really high-visibility change we should be super careful to nail the UX so it's not annoying for users.

WDYT about starting v1 of the experiment for just "Document" and then we can add enhancements onto it? I think we'll need a couple of tree-sitter changes to support some of the other cases without it possibly being annoying for users (e.g. explain)

Excited to see where this goes and happy to pair on this to help!

vscode/src/main.ts

vscode/src/commands/HoverCommandsProvider.ts

umpox · 2024-03-28T10:48:38Z

vscode/src/commands/HoverCommandsProvider.ts

+        // Display Chat & Edit commands when the current position is empty
+        commandsOnHovers.chat.enabled = true
+        commandsOnHovers.edit.enabled = true


Hmm, this means that the user will see this all the time. It feels quite aggressive compared to other cases where we only show if we think there's a good chance that the user will find value in it.

Should we consider removing this for V1 of this experiment? I worry that it could be annoying for users, especially as they'll immediately just see our logo!

I'm actually more inclined to be more aggressive on v1 so we can get more data first before we make any adjustments like we did with ghost text v.1.

Moreover, the hover provider shows up when users specifically hover on a piece of code or symbol, so I think the hover actions is much more intentional here.

I hear your concerns though! we can update it so that it will only show on hover with a delay (if user has hovered over the code for x seconds vs showing up on any mouse moves), but I'd like to keep the commands to show up on non-symbol codes to make the commands more discoverable and accessible.

And maybe for v1 we can just roll out to a smaller group of users (10%?), and add an option to turn off the feature right on the hover, Wdyt?

Cc @chillatom @chenkc805 does this sound ok to you all? (I'll move the discussion to slack)

+1 we chatted about this live. Making this more aggressive helps test the discoverability hypothesis, so I'm supportive of it.

For power users, let's include a checkbox in the user settings where users can turn this off if they want.

@chenkc805 We chatted about this with @chillatom too. We agreed to show this where there are existing hovers for now, rather than add lots of new ones from Cody. This will still be aggressive and get loads of visibility, but has less chance of being annoying to users. We can look at the idea of showing them everywhere as a follow up

vscode/src/commands/HoverCommandsProvider.ts

PriNova · 2024-03-28T16:54:19Z

When hovering over multiple lines and choosing 'New Chat' will these lines

be included in the Chat window as an @-mention context?
if a chat window already has focus included as @-mention context?

One potential downside could be, that users use the web version of VSCode (and if it is a non-PWA) that hovering will mostly not work.

Co-authored-by: Tom Ross <tom@umpox.com>

…bee/hover

toolmantim · 2024-03-28T20:32:08Z

I’d love a chance to properly review this.

At a glance from the description, don’t experimental settings get filtered out by the production build? (so there’d be no visible way for users to disable this in settings if they didn’t like it)

abeatrix · 2024-03-28T20:38:56Z

I’d love a chance to properly review this.

At a glance from the description, don’t experimental settings get filtered out by the production build? (so there’d be no visible way for users to disable this in settings if they didn’t like it)

The setting is undocumented so they can still set it in their User settings JSON file to disable it. I've tested and confirmed that'd work 👍

After discussing with @umpox @chenkc805 @chillatom , we have agreed to show the commands on existing hovers so that it'd be less likely for users to find it distracting and want to disable it. They still can though!

toolmantim · 2024-03-28T20:41:51Z

Users should be able to see it in settings so they can disable it if they wish. Can we make sure they have a visible way to disable (in user settings)?

I’d still like a chance to review this please!

toolmantim · 2024-03-28T21:13:36Z

(Given its feature flagged, can review on main before a user rollout too)

umpox · 2024-03-28T21:15:12Z

@toolmantim @abeatrix Let’s catch up next week and discuss, I think we’re getting to a good place but would be good to get your eyes from a design POV. We can discuss how this can work alongside the ghost text too

abeatrix · 2024-03-28T21:52:38Z

@toolmantim @umpox @chenkc805 Yea i'll also start a thread on Slack so we can do this async since i'm not sure if we can find a time that would work for everyone. I'll make sure not to enable the feature flag for users until then.

In the meantime, we can still roll this out to main so that we can test this internally (I've enabled this feature flag for all Sourcegraph teammates).

@umpox At least for phase 1 (v1), IMO this experiment should be treated independently from ghost text, as any additional changes to existing features would add new variables to the outcome, and the main goal of this experiment is to see how users behave with mouse v.s. keyboard shortcuts. We should then use the data we get back from phase 1 to make further improvements and adjustments to the experiment and features accordingly if that makes sense.
I just added a new Phases section to the doc so we can discuss further there if anyone disagrees 😃

abeatrix · 2024-03-29T00:51:48Z

@umpox ive updated the behavior to match what we discussed earlier:

display doc and explain commands on top level symbols on hover (matches ghost text)
display chat and edit commands on the rest of document symbols on hover
display chat and edit commands on any of the highlighted code

On telemetry side:

log enrollment event for first hover event

Let me know if I missed anything 😃

abeatrix · 2024-03-29T04:45:13Z

@toolmantim added status bar toggle

toolmantim · 2024-04-02T03:16:20Z

Similarly to how Opt-k works now, can we make sure not to mess with people's selections when they run Edit Code or New Chat? Currently it changes your selection, and doesn't restore when you hit escape, whereas Opt-K doesn't seem to change it.

CleanShot.2024-04-02.at.14.13.53.mp4

abeatrix · 2024-04-02T03:42:39Z

Similarly to how Opt-k works now, can we make sure not to mess with people's selections when they run Edit Code or New Chat? Currently it changes your selection, and doesn't restore when you hit escape, whereas Opt-K doesn't seem to change it.

CleanShot.2024-04-02.at.14.13.53.mp4

@toolmantim updated.

umpox · 2024-04-02T07:58:14Z

Can we include "New Chat | Edit Code" on all hovers, I think this makes sense given these actions are always available, whereas documentation and explain is dependent on the active symbol. I immediately noticed that I went to edit a function but this action wasn't available

There seems to be a bug with "Edit code". I think we may need a wrapper command here that calls executeEdit with something like:

range: new vscode.Range(hoverPosition.start, hoverPosition.start)

EditCode.mov

umpox · 2024-04-02T09:18:09Z

BusyDoc.mov

@abeatrix @toolmantim This can get pretty busy with GitLens and other extensions 🤔 Maybe not an issue for the experiment but possibly something to consider for the future

umpox

Feels good! Left some comments I think we should address

vscode/src/commands/HoverCommandsProvider.ts

umpox · 2024-04-02T09:38:20Z

vscode/src/commands/HoverCommandsProvider.ts

+        const activeSymbolRange = document.getWordRangeAtPosition(position)
+        const activeSymbol = activeSymbolRange && document.getText(activeSymbolRange)
+        if (activeSymbolRange && this.symbolStore.some(s => s.name === activeSymbol)) {
+            this.currentHoverSymbol = activeSymbol
+            commandsOnHovers.chat.enabled = true
+            commandsOnHovers.edit.enabled = true
+            return Object.values(commandsOnHovers)
+        }


I don't think we should use name as an identifier for a symbol, as it's not unique for symbols. E.g. you can have const name in two different functions in JS, and they are not the same.

Can we just use the range instead?

Suggested change

const activeSymbolRange = document.getWordRangeAtPosition(position)

const activeSymbol = activeSymbolRange && document.getText(activeSymbolRange)

if (activeSymbolRange && this.symbolStore.some(s => s.name === activeSymbol)) {

this.currentHoverSymbol = activeSymbol

commandsOnHovers.chat.enabled = true

commandsOnHovers.edit.enabled = true

return Object.values(commandsOnHovers)

}

if (this.symbolStore.some(s => s.range.contains(position))) {

commandsOnHovers.chat.enabled = true

commandsOnHovers.edit.enabled = true

return Object.values(commandsOnHovers)

}

It looks like "currentHoverSymbol" is only used for the "Explain symbolName" on click of "New Chat". But if we make "New Chat" behaviour the same as it is for when there is a selection (just opens a chat), then we might not need this?

Can we make "New Chat" start a new chat with the message input containing the line range? e.g. "@some-file.ts:42-43" if you clicked "New Chat" from a const statement on line 42

I won't be able to get it to work in v1, but i added a workaround for now where we just start a new chat and say I have a question about @file so that users can just ask questions after that.

vscode/src/commands/HoverCommandsProvider.ts

vscode/src/services/LocalStorageProvider.ts

vscode/src/commands/HoverCommandsProvider.ts

Co-authored-by: Tom Ross <tom@umpox.com>

vscode/src/commands/HoverCommandsProvider.ts

vscode/src/commands/execute/doc.ts

umpox · 2024-04-03T09:10:32Z

Just re-adding my comment from above:

Can we include "New Chat | Edit Code" on all hovers, I think this makes sense given these actions are always available, whereas documentation and explain is dependent on the active symbol. I immediately noticed that I went to edit a function but this action wasn't available

cc @toolmantim for thoughts here

abeatrix · 2024-04-03T14:00:16Z

I think this makes sense given these actions are always available, whereas documentation and explain is dependent on the active symbol. I immediately noticed that I went to edit a function but this action wasn't available

I personally would find that helpful but I believe they want us to be less aggressive/spammy for the first round which is understandable 😆 I think we should add Edit Code for all nodes with show hint tho:

Skipping New Chat since it's basically doing the same thing as Explain?

…ags (#3662) Co-authored-by: Tom Ross <tom@umpox.com>

umpox

Looks great, thank you for being so thorough with these changes!

umpox · 2024-04-03T15:21:39Z

🚀

abeatrix · 2024-04-03T15:32:03Z

@umpox thank you for sharing your feedback and for your through review as well <3

Commands: new feature flag - hover commands

a41674b

abeatrix requested review from chillatom, umpox and a team March 27, 2024 20:02

abeatrix added 2 commits March 27, 2024 13:54

Merge branch 'main' into bee/hover

b1daf84

add throttle and logFirstEnrollmentEvent

a8a9adc

umpox mentioned this pull request Mar 28, 2024

Doc: Hide hints when there's already a docstring attached to a symbol #3593

Closed

umpox requested changes Mar 28, 2024

View reviewed changes

abeatrix and others added 4 commits March 28, 2024 10:36

move feature flag check to provider

b71a65c

Apply suggestions from code review

dc2154a

Co-authored-by: Tom Ross <tom@umpox.com>

Merge branch 'bee/hover' of https://github.com/sourcegraph/cody into …

bae434d

…bee/hover

apply discussed

927eb49

kalanchan mentioned this pull request Mar 28, 2024

Retention Experiment: Hover Commands #3605

Closed

abeatrix added 3 commits March 28, 2024 19:33

add comments

12b94c5

simplify logic

dbe85b0

add config to status bar toggle

e0d581f

abeatrix requested a review from umpox March 29, 2024 18:56

abeatrix added 3 commits April 1, 2024 16:36

fix register()

1e54eb1

update display logic

734ddef

update Ask Cody to Explain to Explain Code

c5f0897

undo selection change

6242c1b

umpox mentioned this pull request Apr 2, 2024

Edit: Show clickable command labels on ghost text #3655

Closed

umpox reviewed Apr 2, 2024

View reviewed changes

vscode/src/commands/HoverCommandsProvider.ts Outdated Show resolved Hide resolved

abeatrix and others added 7 commits April 2, 2024 07:42

accept range and uri for commands

eb221d1

add filter

45f3748

Update vscode/src/commands/HoverCommandsProvider.ts

dc5d73e

Co-authored-by: Tom Ross <tom@umpox.com>

clean up

0e1dc7a

clean up

bc36958

clean up prompt

7267098

Add Explain Error

98c8f80

umpox reviewed Apr 3, 2024

View reviewed changes

vscode/src/commands/HoverCommandsProvider.ts Outdated Show resolved Hide resolved

umpox reviewed Apr 3, 2024

View reviewed changes

vscode/src/commands/execute/doc.ts Outdated Show resolved Hide resolved

update doc command to use args.url and args.range

1be878b

abeatrix and others added 3 commits April 3, 2024 07:21

Add menu click

97c46e7

update

0534d76

Hover Commands: enable and disable ghost text and hover on feature fl…

5ec99d1

…ags (#3662) Co-authored-by: Tom Ross <tom@umpox.com>

abeatrix requested a review from umpox April 3, 2024 15:18

umpox approved these changes Apr 3, 2024

View reviewed changes

abeatrix merged commit 80de929 into main Apr 3, 2024
20 checks passed

abeatrix deleted the bee/hover branch April 3, 2024 15:32

abeatrix mentioned this pull request Apr 3, 2024

Add missing changelog entry #3676

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature Flag: Hover Commands #3585

Feature Flag: Hover Commands #3585

abeatrix commented Mar 27, 2024 •

edited

Loading

umpox left a comment

umpox Mar 28, 2024

abeatrix Mar 28, 2024

chenkc805 Mar 28, 2024

umpox Mar 28, 2024

PriNova commented Mar 28, 2024 •

edited

Loading

toolmantim commented Mar 28, 2024

abeatrix commented Mar 28, 2024 •

edited

Loading

toolmantim commented Mar 28, 2024

toolmantim commented Mar 28, 2024

umpox commented Mar 28, 2024

abeatrix commented Mar 28, 2024

abeatrix commented Mar 29, 2024

abeatrix commented Mar 29, 2024

toolmantim commented Apr 2, 2024

abeatrix commented Apr 2, 2024

umpox commented Apr 2, 2024

umpox commented Apr 2, 2024

umpox left a comment

umpox Apr 2, 2024

toolmantim Apr 2, 2024

abeatrix Apr 2, 2024

umpox commented Apr 3, 2024

abeatrix commented Apr 3, 2024

umpox left a comment

umpox commented Apr 3, 2024

abeatrix commented Apr 3, 2024

Feature Flag: Hover Commands #3585

Feature Flag: Hover Commands #3585

Conversation

abeatrix commented Mar 27, 2024 • edited Loading

Description

Test plan

Expected Behavior

Configurable for users with feature flag enabled

Ghost Text

Telemetry Events

CodyVSCodeExtension:experiment:hoverCommands:enrolled

umpox left a comment

Choose a reason for hiding this comment

umpox Mar 28, 2024

Choose a reason for hiding this comment

abeatrix Mar 28, 2024

Choose a reason for hiding this comment

chenkc805 Mar 28, 2024

Choose a reason for hiding this comment

umpox Mar 28, 2024

Choose a reason for hiding this comment

PriNova commented Mar 28, 2024 • edited Loading

toolmantim commented Mar 28, 2024

abeatrix commented Mar 28, 2024 • edited Loading

toolmantim commented Mar 28, 2024

toolmantim commented Mar 28, 2024

umpox commented Mar 28, 2024

abeatrix commented Mar 28, 2024

abeatrix commented Mar 29, 2024

abeatrix commented Mar 29, 2024

toolmantim commented Apr 2, 2024

abeatrix commented Apr 2, 2024

umpox commented Apr 2, 2024

umpox commented Apr 2, 2024

umpox left a comment

Choose a reason for hiding this comment

umpox Apr 2, 2024

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

umpox commented Apr 3, 2024

abeatrix commented Apr 3, 2024

umpox left a comment

Choose a reason for hiding this comment

umpox commented Apr 3, 2024

abeatrix commented Apr 3, 2024

abeatrix commented Mar 27, 2024 •

edited

Loading

PriNova commented Mar 28, 2024 •

edited

Loading

abeatrix commented Mar 28, 2024 •

edited

Loading