Skip to content

Commit c8af4ee

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
RPP: only default to Insights agent if insight is expanded
Without this change the AI Assistance panel just shows the Insight agent at all times if you have it enabled. Long term there is some work to be done to figure out the exact flow + when we should start a new conversation based on the selection + what is changed, but this fix is a start. Fixed: 400356881 Change-Id: Ia383eca3e997a71d53f07d100dd172f94612ccbb Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6316748 Auto-Submit: Jack Franklin <[email protected]> Reviewed-by: Nikolay Vitkov <[email protected]> Commit-Queue: Jack Franklin <[email protected]>
1 parent 8016252 commit c8af4ee

File tree

5 files changed

+94
-45
lines changed

5 files changed

+94
-45
lines changed

front_end/panels/ai_assistance/AiAssistancePanel.test.ts

Lines changed: 65 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import * as UI from '../../ui/legacy/legacy.js';
2323
import * as Elements from '../elements/elements.js';
2424
import * as Network from '../network/network.js';
2525
import * as Sources from '../sources/sources.js';
26+
import type * as TimelineComponents from '../timeline/components/components.js';
2627
import * as Timeline from '../timeline/timeline.js';
2728
import * as TimelineUtils from '../timeline/utils/utils.js';
2829

@@ -448,49 +449,54 @@ describeWithMockConnection('AI Assistance Panel', () => {
448449
assert.deepEqual(updatedViewInputAfterNewChat.conversationType, AiAssistance.ConversationType.STYLING);
449450
});
450451

451-
it('should select the performance insights agent if it is enabled', async () => {
452-
updateHostConfig({
453-
devToolsAiAssistancePerformanceAgent: {
454-
enabled: true,
455-
insightsEnabled: true,
456-
},
457-
});
458-
const {panel, expectViewUpdate} =
459-
await createAiAssistancePanel({aidaClient: mockAidaClient([[{explanation: 'test'}]])});
460-
461-
const updatedViewInput = await expectViewUpdate(() => {
462-
panel.handleAction('freestyler.elements-floating-button');
463-
});
452+
it('should select the performance insights agent if it is enabled and the user has expanded an insight',
453+
async () => {
454+
updateHostConfig({
455+
devToolsAiAssistancePerformanceAgent: {
456+
enabled: true,
457+
insightsEnabled: true,
458+
},
459+
});
460+
const {panel, expectViewUpdate} =
461+
await createAiAssistancePanel({aidaClient: mockAidaClient([[{explanation: 'test'}]])});
464462

465-
const updatedViewInputAfterMessage = await expectViewUpdate(() => {
466-
updatedViewInput.onTextSubmit('test');
467-
});
463+
const updatedViewInput = await expectViewUpdate(() => {
464+
panel.handleAction('freestyler.elements-floating-button');
465+
});
468466

469-
UI.Context.Context.instance().setFlavor(
470-
Timeline.TimelinePanel.TimelinePanel, sinon.createStubInstance(Timeline.TimelinePanel.TimelinePanel));
467+
const updatedViewInputAfterMessage = await expectViewUpdate(() => {
468+
updatedViewInput.onTextSubmit('test');
469+
});
471470

472-
assert.deepEqual(updatedViewInputAfterMessage.messages, [
473-
{
474-
entity: AiAssistance.ChatMessageEntity.USER,
475-
text: 'test',
476-
imageInput: undefined,
477-
},
478-
{
479-
answer: 'test',
480-
entity: AiAssistance.ChatMessageEntity.MODEL,
481-
rpcId: undefined,
482-
suggestions: undefined,
483-
steps: [],
484-
},
485-
]);
486-
const updatedViewInputAfterNewChat = await expectViewUpdate(() => {
487-
updatedViewInputAfterMessage.onNewChatClick();
488-
});
471+
UI.Context.Context.instance().setFlavor(
472+
Timeline.TimelinePanel.TimelinePanel, sinon.createStubInstance(Timeline.TimelinePanel.TimelinePanel));
473+
474+
UI.Context.Context.instance().setFlavor(
475+
Timeline.TimelinePanel.SelectedInsight,
476+
new Timeline.TimelinePanel.SelectedInsight({} as unknown as TimelineComponents.Sidebar.ActiveInsight));
477+
478+
assert.deepEqual(updatedViewInputAfterMessage.messages, [
479+
{
480+
entity: AiAssistance.ChatMessageEntity.USER,
481+
text: 'test',
482+
imageInput: undefined,
483+
},
484+
{
485+
answer: 'test',
486+
entity: AiAssistance.ChatMessageEntity.MODEL,
487+
rpcId: undefined,
488+
suggestions: undefined,
489+
steps: [],
490+
},
491+
]);
492+
const updatedViewInputAfterNewChat = await expectViewUpdate(() => {
493+
updatedViewInputAfterMessage.onNewChatClick();
494+
});
489495

490-
assert.deepEqual(updatedViewInputAfterNewChat.messages, []);
491-
assert.deepEqual(
492-
updatedViewInputAfterNewChat.conversationType, AiAssistance.ConversationType.PERFORMANCE_INSIGHT);
493-
});
496+
assert.deepEqual(updatedViewInputAfterNewChat.messages, []);
497+
assert.deepEqual(
498+
updatedViewInputAfterNewChat.conversationType, AiAssistance.ConversationType.PERFORMANCE_INSIGHT);
499+
});
494500

495501
it('should select the Dr Jones performance agent if insights are not enabled', async () => {
496502
updateHostConfig({
@@ -1068,7 +1074,7 @@ describeWithMockConnection('AI Assistance Panel', () => {
10681074
}
10691075

10701076
describe('Performance Insight agent', () => {
1071-
it('should select the PERFORMANCE_INSIGHT agent when the performance panel is open and insights are enabled',
1077+
it('should select the PERFORMANCE_INSIGHT agent when the performance panel is open and insights are enabled and an insight is expanded',
10721078
async () => {
10731079
updateHostConfig({
10741080
devToolsAiAssistancePerformanceAgent: {
@@ -1078,10 +1084,29 @@ describeWithMockConnection('AI Assistance Panel', () => {
10781084
});
10791085
UI.Context.Context.instance().setFlavor(
10801086
Timeline.TimelinePanel.TimelinePanel, sinon.createStubInstance(Timeline.TimelinePanel.TimelinePanel));
1087+
UI.Context.Context.instance().setFlavor(
1088+
Timeline.TimelinePanel.SelectedInsight,
1089+
new Timeline.TimelinePanel.SelectedInsight({} as unknown as TimelineComponents.Sidebar.ActiveInsight));
10811090
const {initialViewInput} = await createAiAssistancePanel();
10821091

10831092
assert.strictEqual(initialViewInput.conversationType, AiAssistance.ConversationType.PERFORMANCE_INSIGHT);
10841093
});
1094+
1095+
it('should select the PERFORMANCE agent when the performance panel is open and insights are enabled but the user has not selected an insight',
1096+
async () => {
1097+
updateHostConfig({
1098+
devToolsAiAssistancePerformanceAgent: {
1099+
enabled: true,
1100+
insightsEnabled: true,
1101+
},
1102+
});
1103+
UI.Context.Context.instance().setFlavor(
1104+
Timeline.TimelinePanel.TimelinePanel, sinon.createStubInstance(Timeline.TimelinePanel.TimelinePanel));
1105+
UI.Context.Context.instance().setFlavor(Timeline.TimelinePanel.SelectedInsight, null);
1106+
1107+
const {initialViewInput} = await createAiAssistancePanel();
1108+
assert.strictEqual(initialViewInput.conversationType, AiAssistance.ConversationType.PERFORMANCE);
1109+
});
10851110
});
10861111
});
10871112

front_end/panels/ai_assistance/AiAssistancePanel.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,15 @@ export class AiAssistancePanel extends UI.Panel.Panel {
558558
const isPerformancePanelVisible =
559559
Boolean(UI.Context.Context.instance().flavor(TimelinePanel.TimelinePanel.TimelinePanel));
560560

561+
// Check if the user has an insight expanded in the performance panel sidebar.
562+
// If they have, we default to the Insights agent; otherwise we fallback to
563+
// the regular Performance agent.
564+
// Note that we do not listen to this flavor changing; this code is here to
565+
// ensure that by default we do not pick the Insights agent if the user has
566+
// just imported a trace and not done anything else. It doesn't make sense
567+
// to select the Insights AI agent in that case.
568+
const userHasExpandedPerfInsight =
569+
Boolean(UI.Context.Context.instance().flavor(TimelinePanel.TimelinePanel.SelectedInsight));
561570
let targetConversationType: ConversationType|undefined = undefined;
562571
if (isElementsPanelVisible && hostConfig.devToolsFreestyler?.enabled) {
563572
targetConversationType = ConversationType.STYLING;
@@ -567,7 +576,7 @@ export class AiAssistancePanel extends UI.Panel.Panel {
567576
targetConversationType = ConversationType.FILE;
568577
} else if (
569578
isPerformancePanelVisible && hostConfig.devToolsAiAssistancePerformanceAgent?.enabled &&
570-
hostConfig.devToolsAiAssistancePerformanceAgent?.insightsEnabled) {
579+
hostConfig.devToolsAiAssistancePerformanceAgent?.insightsEnabled && userHasExpandedPerfInsight) {
571580
targetConversationType = ConversationType.PERFORMANCE_INSIGHT;
572581
} else if (isPerformancePanelVisible && hostConfig.devToolsAiAssistancePerformanceAgent?.enabled) {
573582
targetConversationType = ConversationType.PERFORMANCE;

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,13 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
733733
}
734734
this.#sideBar.setActiveInsight(insight);
735735
this.flameChart.setActiveInsight(insight);
736+
737+
if (insight) {
738+
const selectedInsight = new SelectedInsight(insight);
739+
UI.Context.Context.instance().setFlavor(SelectedInsight, selectedInsight);
740+
} else {
741+
UI.Context.Context.instance().setFlavor(SelectedInsight, null);
742+
}
736743
}
737744

738745
/**
@@ -3040,3 +3047,13 @@ export class ActionDelegate implements UI.ActionRegistration.ActionDelegate {
30403047
return false;
30413048
}
30423049
}
3050+
3051+
/**
3052+
* Used to set the UI.Context when the user expands an Insight. This is only
3053+
* relied upon in the AI Agent code to know which agent to pick by default based
3054+
* on the context of the panel.
3055+
*/
3056+
export class SelectedInsight {
3057+
constructor(public insight: TimelineComponents.Sidebar.ActiveInsight) {
3058+
}
3059+
}

front_end/panels/timeline/components/insights/BaseInsightComponent.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export abstract class BaseInsightComponent<T extends InsightModel<{}, {}>> exten
156156
return;
157157
}
158158

159-
if (!this.data.insightSetKey) {
159+
if (!this.data.insightSetKey || !this.model) {
160160
// Shouldn't happen, but needed to satisfy TS.
161161
return;
162162
}

front_end/panels/timeline/components/insights/SidebarInsight.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ export interface InsightDetails {
1818
export class InsightActivated extends Event {
1919
static readonly eventName = 'insightactivated';
2020

21-
constructor(
22-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
23-
public model: InsightModel<any, any>, public insightSetKey: string) {
21+
constructor(public model: InsightModel<{}, {}>, public insightSetKey: string) {
2422
super(InsightActivated.eventName, {bubbles: true, composed: true});
2523
}
2624
}

0 commit comments

Comments
 (0)