Show notice on first autocomplete #1071

toolmantim · 2023-09-15T04:03:11Z

Adds a notice in the chat view when the user first accepts an autocomplete, helping to show them that an autocomplete has happened, and nudge them towards the next step of onboarding.

Test plan

Automated tests
Launched VS Code in separate instance, ran first autocomplete, verify shown
Dismiss, reload, verify not shown

toolmantim · 2023-09-15T04:09:52Z

Resurrected 1100db6 as this PR, and added generic notice dismissal tracking support using local storage. The only missing piece of updating the state when the autocomplete is accepted for the first time.

Screen.Recording.2023-09-15.at.1.56.42.pm.mov

DanTup · 2023-09-23T13:10:38Z

@toolmantim I pushed changes to trigger this from inline completion instead of the timeout. I also added a test to ensure the event is only triggered for the first one. There aren't any tests that the event actually triggers the notice though - do you have any pointers for how this could be done?

toolmantim · 2023-09-25T12:24:58Z

@philipp-spiess @abeatrix any pointers?

philipp-spiess · 2023-09-25T18:43:40Z

Hmm two ideas:

We don't have an E2E test for any inline completion right now. If we set that up, we can probably assert on the notice too. It might be a bit more work but will also add a valuable smoke test for autocomplete. Code pointers for this:
- Existing E2E tests: https://github.com/sourcegraph/cody/blob/main/vscode/test/e2e/inline-assist.test.ts
Alternatively it's possible to use the integration test setup and use an action for this (since we can directly invoke an action in that setup). There aren't good ways to do assertions on the UI content in this case though (you only have access to a JS thread in the extension host process)

philipp-spiess · 2023-09-25T18:45:30Z

vscode/src/completions/vscodeInlineCompletionItemProvider.ts

@@ -39,6 +41,7 @@ export interface CodyCompletionItemProviderConfig {
    tracer?: ProvideInlineCompletionItemsTracer | null
    contextFetcher?: (options: GetContextOptions) => Promise<GetContextResult>
    featureFlagProvider: FeatureFlagProvider
+    sidebarChatProvider: ChatViewProvider | null


Instead of the hard dependency on the chat view provider, what do you think about just passing a triggerNotice callback here? Should make testing a bit simpler too since we don't need to mock the class

DanTup · 2023-09-29T15:53:20Z

@philipp-spiess

We don't have an E2E test for any inline completion right now. If we set that up, we can probably assert on the notice too. It might be a bit more work but will also add a valuable smoke test for autocomplete. Code pointers for this:
Existing E2E tests: https://github.com/sourcegraph/cody/blob/main/vscode/test/e2e/inline-assist.test.ts

Thanks! I've added a new e2e test that works similar to that one. A lot of this is new to me so I don't know if I've taken the right approach - feel free to nitpick if there are things that can be improved :-)

I also merged main and resolved the conflicts - one in Notice.tsx because of a new text parameter, and one in the inline completion because I inserted the trigger code at the same location as some new cache code. I don't know if you have preferences for rebase vs merge, but I figured a rebase would rewrite history that also exists on Tim's machine.

philipp-spiess · 2023-09-29T17:31:18Z

@DanTup The test works great! Thank you so much for it. Do you know how to reset the local storage locally? I was trying to see the notice in my dev build but wasn't able to. I tried to remove the /Users/philipp/Library/Application Support/Code/Local Storage directory but it still doesn't show up.

The thing I wanted to see is how this works when you trigger the first autocomplete request when the sidebar is hidden.

DanTup · 2023-09-29T17:47:04Z

Do you know how to reset the local storage locally?

Heh, no I don't - I was doing it by just rigging the code when running it. Seems like there should be a way, but I'm not sure where it's stored - I'll have a dig.

The thing I wanted to see is how this works when you trigger the first autocomplete request when the sidebar is hidden.

Good question - I hadn't thought about that. What's the desired behaviour - automatically show the sidebar, do nothing (and leave the notice there until the user switches and sees it), or don't trigger the notification at all?

philipp-spiess · 2023-09-29T18:20:46Z

Good question - I hadn't thought about that. What's the desired behaviour - automatically show the sidebar, do nothing (and leave the notice there until the user switches and sees it), or don't trigger the notification at all?

I think that's for @toolmantim to decide. :)

philipp-spiess

Noice

DanTup · 2023-09-30T10:33:03Z

Do you know how to reset the local storage locally?

For me (Windows), deleting C:\Users\danny\AppData\Roaming\Code\User\globalStorage works (because the LocalStorage is backed by extensions globalState):

cody/vscode/src/main.ts

Line 43 in 57dd0b0

localStorage.setStorage(context.globalState)

However this will clear a load of stuff from your VS Code instance for all extensions.

A better option will be to use the "separate instance" launch config:

This gives you a clean session each time (because it uses --profile-temp) - though means you'll need to login again etc.

DanTup · 2023-09-30T10:37:19Z

Seems like the tests on the bot are stalling, but I'm not sure why:

2023-09-29T20:20:57.0293019Z  �[32m✓�[39m �[35m|vscode|�[39m src/non-stop/utils.test.ts �[2m (�[22m�[2m3 tests�[22m�[2m)�[22m�[90m 4�[2mms�[22m�[39m
2023-09-29T20:20:57.3129730Z  �[32m✓�[39m �[33m|web|�[39m src/sample.test.ts �[2m (�[22m�[2m1 test�[22m�[2m)�[22m�[90m 3�[2mms�[22m�[39m
2023-09-29T20:20:58.1299490Z  �[32m✓�[39m �[33m|shared|�[39m src/chat/transcript/transcript.test.ts �[2m (�[22m�[2m10 tests�[22m�[2m)�[22m�[90m 12�[2mms�[22m�[39m
2023-09-29T20:20:58.8132373Z  �[32m✓�[39m �[33m|shared|�[39m src/chat/bot-response-multiplexer.test.ts �[2m (�[22m�[2m9 tests�[22m�[2m)�[22m�[90m 6�[2mms�[22m�[39m
2023-09-29T20:20:59.7273860Z  �[32m✓�[39m �[33m|shared|�[39m src/common/markdown/markdown.test.ts �[2m (�[22m�[2m12 tests�[22m�[2m)�[22m�[90m 99�[2mms�[22m�[39m
2023-09-29T20:24:36.9864178Z ##[error]The operation was canceled.
2023-09-29T20:24:36.9920322Z Post job cleanup.
2023-09-29T20:24:37.1222550Z Pruning is unnecessary.
2023-09-29T20:24:37.1274916Z Post job cleanup.
2023-09-29T20:24:37.3697007Z [command]/usr/bin/git version
2023-09-29T20:24:37.3697309Z git version 2.42.0

Running pnpm test:unit locally works fine for me (besides some Windows failures I'm working on in another PR). The last thing in the log before "cancellation" (which I suspect is because of it taking too long) was a completed test markdown.test.ts. That seems to be the last test to run for me locally so I'm not sure what's up - is it stalling in some cleanup somewhere? (it doesn't seem like this is failing on main).

toolmantim · 2023-10-01T23:47:49Z

Good question - I hadn't thought about that. What's the desired behaviour - automatically show the sidebar, do nothing (and leave the notice there until the user switches and sees it), or don't trigger the notification at all?

@DanTup @philipp-spiess automatically show the sidebar for now. Given this is only once and it's onboarding, I think it's okay to drag people back in. But if we test this out and it's annoying or problematic let's look at switching to a native notification w/ a button (to bring people back into the next onboarding task)

toolmantim · 2023-10-02T04:02:53Z

Thank you so much @DanTup for getting this ready, and @philipp-spiess for reviewing.

I gave it a test locally and works perfectly. I added showing the chat (in a way that doesn't steal focus), and a changelog entry. This is good to go!

philipp-spiess · 2023-10-02T12:32:20Z

This works great now! Amazing work to both of you 🙇

toolmantim added 2 commits September 15, 2023 14:12

Add autocomplete onboarding notice

ab500d4

Add autocomplete notice and generic dismissal support

8acb345

toolmantim force-pushed the show-notice-on-first-autocomplete branch from 5ad00df to 8acb345 Compare September 15, 2023 04:12

toolmantim mentioned this pull request Sep 15, 2023

Make it obvious in-editor when Cody has provided an autocomplete #667

Closed

beyang assigned DanTup Sep 15, 2023

Trigger notice on first inline completion acceptance

f885670

philipp-spiess reviewed Sep 25, 2023

View reviewed changes

DanTup added 3 commits September 29, 2023 13:22

Switch sidebarChatProvider to just a triggerNotice function

bab706e

Add a basic e2e test for inline completion onboarding notice

53559f0

Merge branch 'main' into show-notice-on-first-autocomplete

254e2e8

DanTup added 2 commits September 29, 2023 16:59

Fix errors from merge

f44f80c

Use valid request params for test

179bc40

DanTup marked this pull request as ready for review September 29, 2023 16:07

philipp-spiess approved these changes Sep 29, 2023

View reviewed changes

Fix quote lints

671fdf4

toolmantim added 4 commits October 2, 2023 14:48

Bring chat view on first autocomplete

21ea6c8

Merge branch 'main' into show-notice-on-first-autocomplete

3676c5d

Changelog

4ecf3ea

Don't steal focus

6c63bb8

Lint

7439c23

toolmantim merged commit b13136f into main Oct 2, 2023
12 checks passed

toolmantim deleted the show-notice-on-first-autocomplete branch October 2, 2023 04:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show notice on first autocomplete #1071

Show notice on first autocomplete #1071

toolmantim commented Sep 15, 2023 •

edited

Loading

toolmantim commented Sep 15, 2023 •

edited

Loading

DanTup commented Sep 23, 2023 •

edited

Loading

toolmantim commented Sep 25, 2023

philipp-spiess commented Sep 25, 2023

philipp-spiess Sep 25, 2023

DanTup Sep 29, 2023

DanTup commented Sep 29, 2023

philipp-spiess commented Sep 29, 2023

DanTup commented Sep 29, 2023

philipp-spiess commented Sep 29, 2023

philipp-spiess left a comment

DanTup commented Sep 30, 2023 •

edited

Loading

DanTup commented Sep 30, 2023

toolmantim commented Oct 1, 2023 •

edited

Loading

toolmantim commented Oct 2, 2023

philipp-spiess commented Oct 2, 2023

Show notice on first autocomplete #1071

Show notice on first autocomplete #1071

Conversation

toolmantim commented Sep 15, 2023 • edited Loading

Test plan

toolmantim commented Sep 15, 2023 • edited Loading

DanTup commented Sep 23, 2023 • edited Loading

toolmantim commented Sep 25, 2023

philipp-spiess commented Sep 25, 2023

philipp-spiess Sep 25, 2023

Choose a reason for hiding this comment

DanTup Sep 29, 2023

Choose a reason for hiding this comment

DanTup commented Sep 29, 2023

philipp-spiess commented Sep 29, 2023

DanTup commented Sep 29, 2023

philipp-spiess commented Sep 29, 2023

philipp-spiess left a comment

Choose a reason for hiding this comment

DanTup commented Sep 30, 2023 • edited Loading

DanTup commented Sep 30, 2023

toolmantim commented Oct 1, 2023 • edited Loading

toolmantim commented Oct 2, 2023

philipp-spiess commented Oct 2, 2023

toolmantim commented Sep 15, 2023 •

edited

Loading

toolmantim commented Sep 15, 2023 •

edited

Loading

DanTup commented Sep 23, 2023 •

edited

Loading

DanTup commented Sep 30, 2023 •

edited

Loading

toolmantim commented Oct 1, 2023 •

edited

Loading