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.

Sign up for GitHub

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

Jump to bottom

Incorrect language detection leads to annoying recommendation #143901

Closed
sbatten opened this issue Feb 24, 2022 · 16 comments
Closed

Incorrect language detection leads to annoying recommendation #143901

sbatten opened this issue Feb 24, 2022 · 16 comments
Assignees
@JacksonKearl
Labels
bug Issue identified by VS Code Team member as probable bug languages-guessing Language guessing issues verified Verification succeeded
Milestone
March 2022

Comments

@sbatten
Copy link
Member

sbatten commented Feb 24, 2022

Testing #143576

Typing out manually

class Foo {
	private foo: Foo
	constructor() {
		this.foo = ''
	}
}

image
@TylerLeonhardt
Copy link
Member

Related to this, I also get C++ guessed if I open a new untitled file and type:

<

yes just one character. I was attempting to type <html which does guess correctly, but I feel like those first few characters that guess C++ is a little much.

For my issue, maybe we wait for like 5 characters at least?

@TylerLeonhardt TylerLeonhardt mentioned this issue Feb 24, 2022
Closed
@TylerLeonhardt
Copy link
Member

I put my comment in a new issue since I think they're not going to require the same fix.

@JacksonKearl
Copy link
Contributor

Added a minimum amount of text used when guessing which might help here. Getting a confidence score out of the model is a better fix.

I wonder if we should revisit how we prompt for ext recommendations in untitled files. A "This Looks Like C++, Would you like to install relevant extensions? Yes / No / Not C++ " would be good for analysis of model performance as well.

@JacksonKearl JacksonKearl added bug Issue identified by VS Code Team member as probable bug languages-guessing Language guessing issues labels Feb 28, 2022
@JacksonKearl JacksonKearl added this to the March 2022 milestone Feb 28, 2022
@sbatten
Copy link
Member Author

sbatten commented Feb 28, 2022

I wonder if we should be prompting at all in this case. If the user isn't even saving the file yet, do we really think they want to install extensions?

@JacksonKearl
Copy link
Contributor

Yeah I thought about that as well, I'd be fine not showing ext installs for Untitled files. @digitarald what do you think?

@digitarald
Copy link
Contributor

Agreed, not showing it for untitled buffers makes sense – until they saved it.

@TylerLeonhardt
Copy link
Member

I don't know... I feel conflicted. As a new user to VS Code, I might:

  • Click "New File" which gives me an untitled file
  • Paste in my C code from my homework assignment
  • wonder why all my friends say to use VS Code because all it did was made the code look pretty

The language detection was to help new users be on that happy path and if we take away that notification, I'm afraid they won't get guided enough down that path.

Now maybe turning the notification into something less aggressive... or perhaps delaying the notification for some amount of time... that might be a happy medium.

@digitarald
Copy link
Contributor

Usertesting showed a lot of confusion with untitled buffers and not getting rich language features, so I agree with @TylerLeonhardt's argument.

As far as I understand: The model does bias against recently used file types ↪ Detected file types are likely to have been opened before ↪ So the get=the-extension=for-this-type recommendation might not be that common. Is that right?

@sbatten
Copy link
Member Author

sbatten commented Mar 1, 2022

@digitarald @TylerLeonhardt I'm skeptical, but only because I don't think I have all the information. Not getting rich language features in an untitled buffer where you've never saved a single cpp file seems like a reasonable scenario. Not getting rich language features when you have used c++ in the past and installed the extension but we didn't detect the language is more confusing.

If this is a new user doing their homework, they are going to need to save this file to run it, at least without some extension installed which implies they already know a bit about how things work in VS Code. I feel like save is the right signal for showing extension recommendations.

@TylerLeonhardt
Copy link
Member

If this is a new user doing their homework, they are going to need to save this file to run it

About ~1 yr ago, @isidorn added a "save on run" feature. The intent (my understanding) is that this was added because a lot of new users were "running" untitled files without saving them to an actual concrete file and extensions were having to handle this case (and some weren't causing confusion to the user). This was mostly the case because new users weren't quite grasping that an untitled file wasn't a real file on disk.

Maybe @isidorn has some thoughts on this

@JacksonKearl
Copy link
Contributor

I think save on run is equally valid for files on disk -- if you don't have autosave enabled it's easy to make a change and run without saving in between then get stuck in a undesirable "why aren't my changes doing anything" state.

@JacksonKearl
Copy link
Contributor

That being said, I'm looking into gauging model confidence this iteration, so I could see a situation where we only show popups when the model is very confident (or the language was explicitly selected).

@sbatten
Copy link
Member Author

sbatten commented Mar 1, 2022

The save on run feature already handles this...
image

@TylerLeonhardt
Copy link
Member

I don't know if we can assume they get that far.

@isidorn
Copy link
Contributor

isidorn commented Mar 2, 2022 •
edited
Loading

@TylerLeonhardt correct, we added a sev on run for what you pointed out.

I like the flow of untitled file -> language is auto detected -> recommendation is shown. I think it is smooth for new users.
Can we just try to fine tune the model confidence this iteration as @JacksonKearl says? So that we detect more strictly. That could fix this scenario.
I do not like the idea of splitting the behaviour based on model confidence (could be confusing). IMHO model confidence should only influence if we auto detect or not.

@kieferrm kieferrm mentioned this issue Mar 14, 2022
Closed
87 tasks
@JacksonKearl
Copy link
Contributor

The model now has some better confidence heuristics built in.

@rzhao271 rzhao271 added the verified Verification succeeded label Mar 24, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@JacksonKearl JacksonKearl

Labels
bug Issue identified by VS Code Team member as probable bug languages-guessing Language guessing issues verified Verification succeeded
Projects
None yet
Milestone
March 2022
Development

No branches or pull requests
6 participants
@digitarald @isidorn @TylerLeonhardt @sbatten @rzhao271 @JacksonKearl