-
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
Changes from 2 commits
020c0db
d38d106
fd10e18
e6190d9
bcd53b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); | ||
} | ||
} |
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; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -274,18 +274,35 @@ public void documentChanged(final DocumentEvent event) { | |
} | ||
String currentSuggestion = session.getCurrentSuggestion().getInsertText(); | ||
int currentOffset = widget.getCaretOffset(); | ||
|
||
if (input.isEmpty()) { | ||
Activator.getLogger().info("distance traversed is 1:" + distanceTraversed); | ||
if (distanceTraversed <= 0) { | ||
// discard all suggestions as caret position is less than request invocation position | ||
session.updateCompletionStates(new ArrayList<String>()); | ||
session.transitionToDecisionMade(); | ||
session.end(); | ||
return; | ||
} | ||
distanceTraversed = typeaheadProcessor.getNewDistanceTraversedOnDeleteAndUpdateBracketState( | ||
event.getLength(), distanceTraversed, brackets); | ||
Activator.getLogger().info("distance traversed is" + distanceTraversed); | ||
if (distanceTraversed < 0) { | ||
// discard all suggestions as caret position is less than request invocation position | ||
session.updateCompletionStates(new ArrayList<String>()); | ||
session.transitionToDecisionMade(); | ||
session.end(); | ||
} | ||
|
||
// note: distanceTraversed as 0 is currently understood to be when a user presses BS removing any typeahead | ||
if (distanceTraversed == 0) { | ||
// reset completion states for all suggestions when user reverts to request | ||
// invocation state | ||
session.resetCompletionStates(); | ||
// mark currently displayed suggestion as seen | ||
session.markSuggestionAsSeen(); | ||
} | ||
|
||
return; | ||
} | ||
|
||
|
@@ -328,12 +345,19 @@ public void documentChanged(final DocumentEvent event) { | |
session.transitionToDecisionMade(lineToUnsetIndent); | ||
Display.getCurrent().asyncExec(() -> { | ||
if (session.isActive()) { | ||
session.end(); | ||
// discard suggestions and end immediately as typeahead does not match | ||
session.updateCompletionStates(new ArrayList<String>()); | ||
session.endImmediately(); | ||
} | ||
}); | ||
return; | ||
} | ||
|
||
// discard all other suggestions except for current one as typeahead matches it | ||
// also mark current one as seen as it continues to be displayed | ||
session.updateCompletionStates(List.of(session.getCurrentSuggestion().getItemId())); | ||
session.markSuggestionAsSeen(); | ||
Comment on lines
+356
to
+357
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Should we perhaps wrap them in a check to see if the user has just started the typeahead? if (!session.hasBeenTypedahead()) {
session.updateCompletionStates(List.of(session.getCurrentSuggestion().getItemId()));
session.markSuggestionAsSeen();
} There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
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. |
||
|
||
// Here we perform "post closing bracket insertion caret correction", which | ||
// consists of the following: | ||
// - Check if the input is a closing bracket | ||
|
@@ -398,7 +422,7 @@ public void mouseDown(final MouseEvent e) { | |
int currentOffset = invocationOffset + distanceTraversed; | ||
int lastKnownLine = widget.getLineAtOffset(currentOffset); | ||
qInvocationSessionInstance.transitionToDecisionMade(lastKnownLine + 1); | ||
qInvocationSessionInstance.end(); | ||
qInvocationSessionInstance.endImmediately(); | ||
return; | ||
} | ||
|
||
|
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.
Not sure if we want to keep this in. This would get logged every time backspace is hit during a typeahead.
Same with the logging call made below.
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.
updated forgot to remove it