fix(amazonq): ensure summary.md + icons saved, populate job history sooner, fix minor styling issues #4928
Conversation
There was a problem hiding this comment.
The author of this PR: #4914
was unable to test his work, so putting that work here.
| @@ -28,7 +28,7 @@ export class TabDataGenerator { | |||
| ['unknown', 'Ask a question or enter "/" for quick actions'], | |||
| ['cwc', 'Ask a question or enter "/" for quick actions'], | |||
| ['featuredev', 'Briefly describe a task or issue'], | |||
| ['gumby', 'Chat is disabled during Code Transformation.'], | |||
| ['gumby', 'Open a new tab to chat with Q'], | |||
There was a problem hiding this comment.
This exact text was approved by our technical writer.
| @@ -8,7 +8,6 @@ import { randomUUID } from '../../common/crypto' | |||
| interface ICodeTransformTelemetryState { | |||
| sessionId: string | |||
| sessionStartTime: number | |||
| resultStatus: string | |||
There was a problem hiding this comment.
Redundant; we already store job status.
| return getIcon('vscode-play') | ||
| default: | ||
| return getIcon('vscode-stop-circle') | ||
| } |
There was a problem hiding this comment.
Both of these above were unused.
| @@ -0,0 +1,105 @@ | |||
| @keyframes spin { | |||
There was a problem hiding this comment.
The author of this PR (#4914) was unable to test it so adding his work here.
| @@ -56,6 +56,18 @@ export function throwIfCancelled() { | |||
| } | |||
| } | |||
|
|
|||
| export async function updateJobHistory() { | |||
There was a problem hiding this comment.
Yeah this one has no need to be, can change it back.
| @@ -210,7 +214,6 @@ export async function startTransformationJob(uploadId: string) { | |||
| } | |||
| throw new Error('Start job failed') | |||
| } | |||
| transformByQState.setJobId(encodeHTML(jobId)) | |||
There was a problem hiding this comment.
why remove jobId from transform state? This can cause some metrics to miss jobId?
There was a problem hiding this comment.
Oh I didn't remove, I just moved it to happen sooner. I set the jobId right after the upload completes so that it is populated in the job history tab sooner.
| transformByQState.resetPlanSteps() | ||
| transformByQState.resetSessionJobHistory() | ||
| transformByQState.setJobId('') // so that details for last job are not overwritten when running one job after another |
There was a problem hiding this comment.
The purpose (or the comment) of not overwritten the last job's ID was not very clear to me, since a back-to-back job should ideally be persisting the context for the new job? Shouldn't JobId here already persist the new Job's ID (followup question to the above for removing setJobId() )
There was a problem hiding this comment.
The sessionJobHistory maps jobIds to details about the job, and in the beginning of a 2nd job, if the previous job's ID is not cleared, the updateHistory function running on an interval could put that ID back in the map, but this time with outdated/inaccurate info (info from the current job).
| transformByQState.setToCancelled() | ||
| transformByQState.setPolledJobStatus('CANCELLED') |
There was a problem hiding this comment.
Yes, in the job history panel we display the polledJobStatus so need this set too for the text/display purposes.
| @@ -226,8 +226,12 @@ export async function pollTransformationStatusUntilPlanReady(jobId: string) { | |||
| await pollTransformationJob(jobId, CodeWhispererConstants.validStatesForPlanGenerated) | |||
| } catch (error) { | |||
| getLogger().error(`CodeTransformation: ${CodeWhispererConstants.failedToCompleteJobNotification}`, error) | |||
| transformByQState.setJobFailureErrorNotification(CodeWhispererConstants.failedToCompleteJobNotification) | |||
| transformByQState.setJobFailureErrorChatMessage(CodeWhispererConstants.failedToCompleteJobChatMessage) | |||
| if (!transformByQState.getJobFailureErrorNotification()) { | |||
There was a problem hiding this comment.
To prevent the error notification / chat message from being overwritten if they were already set by our pollTransformationJob function -- pollTransformationJob will set these if our backend returns a specific error reason, in which case we want to show that to user, not these generic messages.
Problem
Several minor issues from GA launch: summary.md + icons not actually being saved when users accept changes, job history info not shown until the job terminates, and issues with white text / blue highlighting / grey text in transformation plan not visible with certain background colors.
Solution
Since the summary.md and icons are in their own new directories, ensure those are created before saving the files. Populate the job history tab once job ID is available and allow users to switch back and forth between plan progress and job history throughout the transformation. Use default text color which will adjust by background, make blue highlighting look better for dark backgrounds, use a lighter grey font color.
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.