-
Notifications
You must be signed in to change notification settings - Fork 272
Track SessionContext instead of InvocationContext #4916
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
Changes from all commits
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 |
---|---|---|
|
@@ -4,25 +4,33 @@ | |
package software.aws.toolkits.jetbrains.services.codewhisperer.model | ||
|
||
import com.intellij.openapi.Disposable | ||
import com.intellij.openapi.editor.Editor | ||
import com.intellij.openapi.editor.VisualPosition | ||
import com.intellij.openapi.editor.markup.RangeHighlighter | ||
import com.intellij.openapi.project.Project | ||
import com.intellij.openapi.ui.popup.JBPopup | ||
import com.intellij.openapi.util.Disposer | ||
import com.intellij.openapi.vfs.VirtualFile | ||
import com.intellij.util.concurrency.annotations.RequiresEdt | ||
import software.amazon.awssdk.services.codewhispererruntime.model.Completion | ||
import software.amazon.awssdk.services.codewhispererruntime.model.GenerateCompletionsResponse | ||
import software.aws.toolkits.jetbrains.core.credentials.ToolkitConnection | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.codescan.sessionconfig.PayloadContext | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.language.CodeWhispererProgrammingLanguage | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.service.CodeWhispererAutomatedTriggerType | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.service.CodeWhispererInvocationStatus | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.service.RequestContext | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.service.ResponseContext | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.telemetry.CodeWhispererTelemetryService | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CodeWhispererConstants | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CrossFileStrategy | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.util.SupplementalContextStrategy | ||
import software.aws.toolkits.jetbrains.services.codewhisperer.util.UtgStrategy | ||
import software.aws.toolkits.telemetry.CodewhispererCompletionType | ||
import software.aws.toolkits.telemetry.CodewhispererTriggerType | ||
import software.aws.toolkits.telemetry.Result | ||
import java.time.Duration | ||
import java.time.Instant | ||
import java.util.concurrent.TimeUnit | ||
|
||
data class Chunk( | ||
|
@@ -81,10 +89,19 @@ data class SupplementalContextInfo( | |
} | ||
|
||
data class RecommendationContext( | ||
val details: List<DetailContext>, | ||
val details: MutableList<DetailContext>, | ||
val userInputOriginal: String, | ||
val userInputSinceInvocation: String, | ||
val position: VisualPosition, | ||
val jobId: Int, | ||
var typeahead: String = "", | ||
) | ||
|
||
data class PreviewContext( | ||
val jobId: Int, | ||
val detail: DetailContext, | ||
val userInput: String, | ||
val typeahead: String, | ||
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. dumb q, what's the difference between these 2 typeaheads 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. previously we have typeahead and typeaheadoriginal for the purpose of accounting leading spaces in the typeahead, Now we trigger more so we have more display oppotunities, I feel like this becomes a bit unnecessary and makes the logic complicated, so remove typeahead original to make it no longer accomodate leading spaces. |
||
) | ||
|
||
data class DetailContext( | ||
|
@@ -95,17 +112,47 @@ data class DetailContext( | |
val isTruncatedOnRight: Boolean, | ||
val rightOverlap: String = "", | ||
val completionType: CodewhispererCompletionType, | ||
var hasSeen: Boolean = false, | ||
var isAccepted: Boolean = false | ||
) | ||
|
||
data class SessionContext( | ||
val typeahead: String = "", | ||
val typeaheadOriginal: String = "", | ||
val selectedIndex: Int = 0, | ||
val project: Project, | ||
val editor: Editor, | ||
var popup: JBPopup? = null, | ||
var selectedIndex: Int = -1, | ||
val seen: MutableSet<Int> = mutableSetOf(), | ||
val isFirstTimeShowingPopup: Boolean = true, | ||
var isFirstTimeShowingPopup: Boolean = true, | ||
var toBeRemovedHighlighter: RangeHighlighter? = null, | ||
var insertEndOffset: Int = -1, | ||
) | ||
var popupOffset: Int = -1, | ||
val latencyContext: LatencyContext, | ||
var hasAccepted: Boolean = false | ||
) : Disposable { | ||
private var isDisposed = false | ||
|
||
@RequiresEdt | ||
override fun dispose() { | ||
CodeWhispererTelemetryService.getInstance().sendUserDecisionEventForAll( | ||
this, | ||
hasAccepted, | ||
CodeWhispererInvocationStatus.getInstance().popupStartTimestamp?.let { Duration.between(it, Instant.now()) } | ||
) | ||
CodeWhispererInvocationStatus.getInstance().setDisplaySessionActive(false) | ||
|
||
if (hasAccepted) { | ||
popup?.closeOk(null) | ||
} else { | ||
popup?.cancel() | ||
} | ||
popup?.let { Disposer.dispose(it) } | ||
popup = null | ||
Comment on lines
+143
to
+149
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. popup manager and sessioncontext are so tied together that it feels like it should just be handled together 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. popup now becomes a state of a session (1 session 1 popup) so I put it as a field of
separately. |
||
CodeWhispererInvocationStatus.getInstance().finishInvocation() | ||
isDisposed = true | ||
} | ||
|
||
fun isDisposed() = isDisposed | ||
} | ||
|
||
data class RecommendationChunk( | ||
val text: String, | ||
|
@@ -124,16 +171,21 @@ data class InvocationContext( | |
val requestContext: RequestContext, | ||
val responseContext: ResponseContext, | ||
val recommendationContext: RecommendationContext, | ||
val popup: JBPopup, | ||
) : Disposable { | ||
override fun dispose() {} | ||
private var isDisposed = false | ||
|
||
@RequiresEdt | ||
override fun dispose() { | ||
isDisposed = true | ||
} | ||
|
||
fun isDisposed() = isDisposed | ||
} | ||
|
||
data class WorkerContext( | ||
val requestContext: RequestContext, | ||
val responseContext: ResponseContext, | ||
val response: GenerateCompletionsResponse, | ||
val popup: JBPopup, | ||
) | ||
|
||
data class CodeScanTelemetryEvent( | ||
|
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 seeing why need mutable
Uh oh!
There was an error while loading. Please reload this page.
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.
Now each trigger will only have one state
InvocationContext
and is stored inongoingRequests
, when subsequent suggestions sent back to client, they will be added to this mutable details, and other parts ofInvocationContext
remains the same.otherwise we will have to replace
InvocationContext
which I feel is a bit unnecessary.