-
Notifications
You must be signed in to change notification settings - Fork 4
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
Determine and send inline completion session result #323
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
020c0db
Determine and send inline completion session result
shruti0085 d38d106
fix test
shruti0085 fd10e18
removed extra logs
shruti0085 e6190d9
Merge branch 'main' into shruti0085/statesSuggestion
shruti0085 bcd53b8
Merge branch 'main' into shruti0085/statesSuggestion
breedloj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/model/InlineCompletionStates.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package software.aws.toolkits.eclipse.amazonq.lsp.model; | ||
|
||
public final class InlineCompletionStates { | ||
// Indicates if suggestion has been seen by the user in the UI | ||
private boolean seen; | ||
// Indicates if suggestion accepted | ||
private boolean accepted; | ||
// Indicates if suggestion was filtered out on the client-side and marked as | ||
// discarded. | ||
private boolean discarded; | ||
|
||
public boolean isSeen() { | ||
return seen; | ||
} | ||
|
||
public void setSeen(final boolean seen) { | ||
this.seen = seen; | ||
} | ||
|
||
public boolean isAccepted() { | ||
return accepted; | ||
} | ||
|
||
public void setAccepted(final boolean accepted) { | ||
this.accepted = accepted; | ||
} | ||
|
||
public boolean isDiscarded() { | ||
return discarded; | ||
} | ||
|
||
public void setDiscarded(final boolean discarded) { | ||
this.discarded = discarded; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return String.format("{accepted=%b, seen=%b, discarded=%b}", accepted, seen, discarded); | ||
} | ||
} |
63 changes: 63 additions & 0 deletions
63
...tware/aws/toolkits/eclipse/amazonq/lsp/model/LogInlineCompletionSessionResultsParams.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package software.aws.toolkits.eclipse.amazonq.lsp.model; | ||
|
||
import java.util.concurrent.ConcurrentHashMap; | ||
|
||
import com.fasterxml.jackson.annotation.JsonInclude; | ||
|
||
public final class LogInlineCompletionSessionResultsParams { | ||
// Session Id attached to get completion items response | ||
private final String sessionId; | ||
// Map with results of interaction with completion items/suggestions in the UI | ||
private final ConcurrentHashMap<String, InlineCompletionStates> completionSessionResult; | ||
|
||
// Total time when items from this suggestion session were visible in UI | ||
@JsonInclude(JsonInclude.Include.NON_NULL) | ||
private Long totalSessionDisplayTime; | ||
// Time from request invocation start to rendering of the first suggestion in the UI. | ||
@JsonInclude(JsonInclude.Include.NON_NULL) | ||
private Long firstCompletionDisplayLatency; | ||
// Length of additional characters inputed by user from when the trigger happens to when the first suggestion is about to be shown in UI | ||
@JsonInclude(JsonInclude.Include.NON_NULL) | ||
private Integer typeaheadLength; | ||
|
||
public LogInlineCompletionSessionResultsParams(final String sessionId, final ConcurrentHashMap<String, InlineCompletionStates> completionSessionResult) { | ||
this.sessionId = sessionId; | ||
this.completionSessionResult = completionSessionResult; | ||
} | ||
|
||
public Long getTotalSessionDisplayTime() { | ||
return totalSessionDisplayTime; | ||
} | ||
|
||
public void setTotalSessionDisplayTime(final Long totalSessionDisplayTime) { | ||
this.totalSessionDisplayTime = totalSessionDisplayTime; | ||
} | ||
|
||
public Long getFirstCompletionDisplayLatency() { | ||
return firstCompletionDisplayLatency; | ||
} | ||
|
||
public void setFirstCompletionDisplayLatency(final Long firstCompletionDisplayLatency) { | ||
this.firstCompletionDisplayLatency = firstCompletionDisplayLatency; | ||
} | ||
|
||
public Integer getTypeaheadLength() { | ||
return typeaheadLength; | ||
} | ||
|
||
public void setTypeaheadLength(final Integer typeaheadLength) { | ||
this.typeaheadLength = typeaheadLength; | ||
} | ||
|
||
public ConcurrentHashMap<String, InlineCompletionStates> getCompletionSessionResult() { | ||
return completionSessionResult; | ||
} | ||
|
||
public String getSessionId() { | ||
return sessionId; | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When these two lines get run, it means that the user has typed ahead with a character that matches.
This means
updateCompletionStates
andmarkSuggestionAsSeen
are going to be called for every correct character typed in a typeahead session, with the same suggestion.Should we perhaps wrap them in a check to see if the user has just started the typeahead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line would do the opposite of what we want to do here which is to mark when suggestions have been filtered because typeahead matches.
The above logic would do the opposite because it says if there is no typeahead then consider suggsestions as filtered and discard other suggestions except for current one as being filtered out.
Also wouldn't this always return true if we hit this line of code in this block(L349)
session.hasBeenTypedahead()
since this would only process when there is A typeahead?That is correct. Since this documentChange event is raised for any update, based on user's actions it could have been a backspace marking things not-discarded unseen in the previous stroke or any of the other checks before this line. Ideally the whole list of suggestions should be filtered down to match the typeahead prefix. However due to limitations in the current filtering logic we only show the current suggestion instead of other valid suggestions from the list. I think it still makes sense to have this because unless we can retain the information of the typedPrefix among other stuff this class does it is hard to know if typeahead actually diverged or not.