Skip to content

Commit 7fd331c

Browse files
roagaharshithadurai
roaga
authored and
harshithadurai
committed
feat(autofix): Prompt rethink on step completion (#81052)
To make it clear to users that they can dive deeper or correct unsatisfactory results, when they reach a proposed-but-unconfirmed root cause or code change, we highlight the rethink buttons and show a message as a reminder. This PR also updates the placeholder text to make it more clear what you can do with the rethink feature. <img width="668" alt="Screenshot 2024-11-20 at 7 58 44 AM" src="https://github.com/user-attachments/assets/89086600-0ae4-42f0-9410-dbae11b3a5f6">
1 parent bced14f commit 7fd331c

File tree

6 files changed

+127
-31
lines changed

6 files changed

+127
-31
lines changed

‎static/app/components/events/autofix/autofixInsightCards.spec.tsx‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,17 +164,29 @@ describe('AutofixInsightCards', () => {
164164
renderComponent();
165165
const rethinkButton = screen.getByRole('button', {name: 'Rethink from here'});
166166
await userEvent.click(rethinkButton);
167-
expect(screen.getByPlaceholderText('Say something...')).toBeInTheDocument();
167+
expect(
168+
screen.getByPlaceholderText(
169+
'You should know X... Dive deeper into Y... Look at Z...'
170+
)
171+
).toBeInTheDocument();
168172
});
169173

170174
it('hides rethink input overlay when clicked outside', async () => {
171175
renderComponent();
172176
const rethinkButton = screen.getByRole('button', {name: 'Rethink from here'});
173177
await userEvent.click(rethinkButton);
174-
expect(screen.getByPlaceholderText('Say something...')).toBeInTheDocument();
178+
expect(
179+
screen.getByPlaceholderText(
180+
'You should know X... Dive deeper into Y... Look at Z...'
181+
)
182+
).toBeInTheDocument();
175183

176184
await userEvent.click(document.body);
177-
expect(screen.queryByPlaceholderText('Say something...')).not.toBeInTheDocument();
185+
expect(
186+
screen.queryByPlaceholderText(
187+
'You should know X... Dive deeper into Y... Look at Z...'
188+
)
189+
).not.toBeInTheDocument();
178190
});
179191

180192
it('submits rethink request when form is submitted', async () => {
@@ -187,7 +199,9 @@ describe('AutofixInsightCards', () => {
187199
const rethinkButton = screen.getByRole('button', {name: 'Rethink from here'});
188200
await userEvent.click(rethinkButton);
189201

190-
const input = screen.getByPlaceholderText('Say something...');
202+
const input = screen.getByPlaceholderText(
203+
'You should know X... Dive deeper into Y... Look at Z...'
204+
);
191205
await userEvent.type(input, 'Rethink this part');
192206

193207
const submitButton = screen.getByLabelText(
@@ -222,7 +236,9 @@ describe('AutofixInsightCards', () => {
222236
const rethinkButton = screen.getByRole('button', {name: 'Rethink from here'});
223237
await userEvent.click(rethinkButton);
224238

225-
const input = screen.getByPlaceholderText('Say something...');
239+
const input = screen.getByPlaceholderText(
240+
'You should know X... Dive deeper into Y... Look at Z...'
241+
);
226242
await userEvent.type(input, 'Rethink this part');
227243

228244
const submitButton = screen.getByLabelText(
@@ -246,7 +262,9 @@ describe('AutofixInsightCards', () => {
246262
const rethinkButton = screen.getByRole('button', {name: 'Rethink from here'});
247263
await userEvent.click(rethinkButton);
248264

249-
const input = screen.getByPlaceholderText('Say something...');
265+
const input = screen.getByPlaceholderText(
266+
'You should know X... Dive deeper into Y... Look at Z...'
267+
);
250268
await userEvent.type(input, 'Rethink this part');
251269

252270
const submitButton = screen.getByLabelText(

‎static/app/components/events/autofix/autofixInsightCards.tsx‎

Lines changed: 80 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ interface AutofixInsightCardProps {
135135
repos: AutofixRepository[];
136136
runId: string;
137137
stepIndex: number;
138+
isLastInsightInStep?: boolean;
139+
shouldHighlightRethink?: boolean;
138140
}
139141

140142
function AutofixInsightCard({
@@ -146,6 +148,8 @@ function AutofixInsightCard({
146148
stepIndex,
147149
groupId,
148150
runId,
151+
shouldHighlightRethink,
152+
isLastInsightInStep,
149153
}: AutofixInsightCardProps) {
150154
const isUserMessage = insight.justification === 'USER';
151155

@@ -165,6 +169,7 @@ function AutofixInsightCard({
165169
stepIndex={stepIndex}
166170
groupId={groupId}
167171
runId={runId}
172+
isHighlighted={shouldHighlightRethink}
168173
/>
169174
)}
170175
{!isUserMessage && (
@@ -288,6 +293,8 @@ function AutofixInsightCard({
288293
stepIndex={stepIndex}
289294
groupId={groupId}
290295
runId={runId}
296+
isHighlighted={shouldHighlightRethink}
297+
isLastCard={isLastInsightInStep}
291298
/>
292299
)}
293300
</AnimationWrapper>
@@ -304,6 +311,7 @@ interface AutofixInsightCardsProps {
304311
repos: AutofixRepository[];
305312
runId: string;
306313
stepIndex: number;
314+
shouldHighlightRethink?: boolean;
307315
}
308316

309317
function AutofixInsightCards({
@@ -314,6 +322,7 @@ function AutofixInsightCards({
314322
stepIndex,
315323
groupId,
316324
runId,
325+
shouldHighlightRethink,
317326
}: AutofixInsightCardsProps) {
318327
return (
319328
<InsightsContainer>
@@ -330,6 +339,8 @@ function AutofixInsightCards({
330339
stepIndex={stepIndex}
331340
groupId={groupId}
332341
runId={runId}
342+
isLastInsightInStep={index === insights.length - 1}
343+
shouldHighlightRethink={shouldHighlightRethink}
333344
/>
334345
)
335346
)
@@ -350,6 +361,8 @@ function AutofixInsightCards({
350361
stepIndex={stepIndex}
351362
groupId={groupId}
352363
runId={runId}
364+
isHighlighted={shouldHighlightRethink}
365+
isLastCard
353366
/>
354367
</EmptyResultsContainer>
355368
) : null}
@@ -393,11 +406,15 @@ function ChainLink({
393406
runId,
394407
stepIndex,
395408
insightCardAboveIndex,
409+
isHighlighted,
410+
isLastCard,
396411
}: {
397412
groupId: string;
398413
insightCardAboveIndex: number | null;
399414
runId: string;
400415
stepIndex: number;
416+
isHighlighted?: boolean;
417+
isLastCard?: boolean;
401418
}) {
402419
const [showOverlay, setShowOverlay] = useState(false);
403420
const [referenceElement, setReferenceElement] = useState<
@@ -452,15 +469,30 @@ function ChainLink({
452469
return (
453470
<ArrowContainer>
454471
<IconArrow direction={'down'} className="arrow-icon" />
455-
<RethinkButton
456-
ref={setReferenceElement}
457-
icon={<IconRefresh size="xs" />}
458-
size="zero"
459-
className="rethink-button"
460-
title={t('Rethink from here')}
461-
aria-label={t('Rethink from here')}
462-
onClick={() => setShowOverlay(true)}
463-
/>
472+
<RethinkButtonContainer className="rethink-button-container">
473+
<AnimatePresence>
474+
{isLastCard && isHighlighted && (
475+
<RethinkMessage
476+
initial={{opacity: 0, x: 20}}
477+
animate={{opacity: 1, x: 0}}
478+
exit={{opacity: 0, x: 20}}
479+
transition={{duration: 0.4}}
480+
>
481+
Not satisfied?
482+
</RethinkMessage>
483+
)}
484+
</AnimatePresence>
485+
<RethinkButton
486+
ref={setReferenceElement}
487+
icon={<IconRefresh size="xs" />}
488+
size="zero"
489+
className="rethink-button"
490+
title={t('Rethink from here')}
491+
aria-label={t('Rethink from here')}
492+
onClick={() => setShowOverlay(true)}
493+
isHighlighted={isHighlighted}
494+
/>
495+
</RethinkButtonContainer>
464496

465497
{showOverlay &&
466498
createPortal(
@@ -488,7 +520,7 @@ function ChainLink({
488520
>
489521
<Input
490522
type="text"
491-
placeholder="Say something..."
523+
placeholder="You should know X... Dive deeper into Y... Look at Z..."
492524
value={comment}
493525
onChange={e => setComment(e.target.value)}
494526
size="md"
@@ -591,25 +623,55 @@ const ArrowContainer = styled('div')`
591623
align-self: center;
592624
}
593625
594-
.rethink-button {
626+
.rethink-button-container {
595627
grid-column: 3 / 4;
596628
justify-self: end;
597629
align-self: center;
630+
position: relative;
598631
}
632+
`;
599633

600-
&:hover {
601-
.rethink-button {
602-
color: ${p => p.theme.subText};
603-
}
604-
}
634+
const RethinkButtonContainer = styled('div')`
635+
position: relative;
636+
`;
637+
638+
const RethinkMessage = styled(motion.div)`
639+
color: ${p => p.theme.active};
640+
font-size: ${p => p.theme.fontSizeSmall};
641+
position: absolute;
642+
right: calc(100% + ${space(1)});
643+
margin-top: 1px;
644+
white-space: nowrap;
605645
`;
606646

607-
const RethinkButton = styled(Button)`
647+
const RethinkButton = styled(Button)<{isHighlighted?: boolean}>`
608648
font-weight: normal;
609649
font-size: small;
610650
border: none;
611651
color: ${p => p.theme.subText};
612-
transition: color 0.2s ease-in-out;
652+
transition: all 0.4s ease-in-out;
653+
position: relative;
654+
655+
${p =>
656+
p.isHighlighted &&
657+
`
658+
color: ${p.theme.button.primary.backgroundActive};
659+
background: ${p.theme.purple100};
660+
border-radius: ${p.theme.borderRadius};
661+
662+
&:hover {
663+
color: ${p.theme.activeHover};
664+
background: ${p.theme.purple200};
665+
}
666+
`}
667+
668+
&:hover {
669+
transform: scale(1.05);
670+
}
671+
672+
&:active {
673+
transform: scale(0.95);
674+
}
613675
`;
614676

615677
const RethinkInput = styled('div')`

‎static/app/components/events/autofix/autofixMessageBox.spec.tsx‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,17 @@ describe('AutofixMessageBox', () => {
8080
render(<AutofixMessageBox {...defaultProps} />);
8181

8282
expect(screen.getByText('Test display text')).toBeInTheDocument();
83-
expect(screen.getByPlaceholderText('Say something...')).toBeInTheDocument();
83+
expect(
84+
screen.getByPlaceholderText('Share helpful context or feedback...')
85+
).toBeInTheDocument();
8486
expect(screen.getByRole('button', {name: 'Send'})).toBeInTheDocument();
8587
});
8688

8789
it('calls onSend when provided and button is clicked', async () => {
8890
const onSendMock = jest.fn();
8991
render(<AutofixMessageBox {...defaultProps} onSend={onSendMock} />);
9092

91-
const input = screen.getByPlaceholderText('Say something...');
93+
const input = screen.getByPlaceholderText('Share helpful context or feedback...');
9294
await userEvent.type(input, 'Test message');
9395
await userEvent.click(screen.getByRole('button', {name: 'Send'}));
9496

@@ -104,7 +106,7 @@ describe('AutofixMessageBox', () => {
104106

105107
render(<AutofixMessageBox {...defaultProps} />);
106108

107-
const input = screen.getByPlaceholderText('Say something...');
109+
const input = screen.getByPlaceholderText('Share helpful context or feedback...');
108110
await userEvent.type(input, 'Test message');
109111
await userEvent.click(screen.getByRole('button', {name: 'Send'}));
110112

@@ -125,7 +127,7 @@ describe('AutofixMessageBox', () => {
125127

126128
render(<AutofixMessageBox {...defaultProps} />);
127129

128-
const input = screen.getByPlaceholderText('Say something...');
130+
const input = screen.getByPlaceholderText('Share helpful context or feedback...');
129131
await userEvent.type(input, 'Test message');
130132
await userEvent.click(screen.getByRole('button', {name: 'Send'}));
131133

@@ -196,7 +198,9 @@ describe('AutofixMessageBox', () => {
196198
it('shows feedback input when "Give feedback" is selected', () => {
197199
render(<AutofixMessageBox {...changesStepProps} />);
198200

199-
expect(screen.getByPlaceholderText('Say something...')).toBeInTheDocument();
201+
expect(
202+
screen.getByPlaceholderText('Share helpful context or feedback...')
203+
).toBeInTheDocument();
200204
expect(screen.getByRole('button', {name: 'Send'})).toBeInTheDocument();
201205
});
202206

‎static/app/components/events/autofix/autofixMessageBox.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ function AutofixMessageBox({
409409
onChange={e => setMessage(e.target.value)}
410410
placeholder={
411411
!isRootCauseSelectionStep
412-
? 'Say something...'
412+
? 'Share helpful context or feedback...'
413413
: rootCauseMode === 'suggested_root_cause'
414414
? '(Optional) Provide any instructions for the fix...'
415415
: 'Propose your own root cause...'

‎static/app/components/events/autofix/autofixSteps.spec.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ describe('AutofixSteps', () => {
190190

191191
render(<AutofixSteps {...propsWithChanges} />);
192192

193-
const input = screen.getByPlaceholderText('Say something...');
193+
const input = screen.getByPlaceholderText('Share helpful context or feedback...');
194194
await userEvent.type(input, 'Feedback on changes');
195195
await userEvent.click(screen.getByRole('button', {name: 'Send'}));
196196

‎static/app/components/events/autofix/autofixSteps.tsx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ interface StepProps {
3434
hasStepBelow: boolean;
3535
repos: AutofixRepository[];
3636
runId: string;
37+
shouldHighlightRethink: boolean;
3738
step: AutofixStep;
3839
}
3940

@@ -67,6 +68,7 @@ export function Step({
6768
hasStepBelow,
6869
hasStepAbove,
6970
hasErroredStepBefore,
71+
shouldHighlightRethink,
7072
}: StepProps) {
7173
return (
7274
<StepCard>
@@ -90,6 +92,7 @@ export function Step({
9092
stepIndex={step.index}
9193
groupId={groupId}
9294
runId={runId}
95+
shouldHighlightRethink={shouldHighlightRethink}
9396
/>
9497
)}
9598
{step.type === AutofixStepType.ROOT_CAUSE_ANALYSIS && (
@@ -249,6 +252,14 @@ export function AutofixSteps({data, groupId, runId}: AutofixStepsProps) {
249252
nextStep?.status === 'PROCESSING' &&
250253
nextStep?.insights?.length === 0;
251254

255+
const isNextStepLastStep = index === steps.length - 2;
256+
const shouldHighlightRethink =
257+
(nextStep?.type === AutofixStepType.ROOT_CAUSE_ANALYSIS &&
258+
isNextStepLastStep) ||
259+
(nextStep?.type === AutofixStepType.CHANGES &&
260+
nextStep.changes.length > 0 &&
261+
!nextStep.changes.every(change => change.pull_request));
262+
252263
return (
253264
<div ref={el => (stepsRef.current[index] = el)} key={step.id}>
254265
{twoNonDefaultStepsInARow && <br />}
@@ -264,6 +275,7 @@ export function AutofixSteps({data, groupId, runId}: AutofixStepsProps) {
264275
runId={runId}
265276
repos={repos}
266277
hasErroredStepBefore={previousStepErrored}
278+
shouldHighlightRethink={shouldHighlightRethink}
267279
/>
268280
</div>
269281
);

0 commit comments

Comments
 (0)