Skip to content

Commit 7374af9

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
PerfAI: pick no_navigation Insight set if it's the only choice
I noticed this bug when wondering why the AI had no idea about the interaction I was asking it to debug. The code to select the first insight assumed we'd find one with a navigation, but often for interactions that is not the case. This CL updates the logic to: 1. If there is only 1 InsightSet, use that. 2. If there are >1, prefer the first with a navigation attached, but otherwise fallback to the first set otherwise. This fixes the behaviour where for no-navigation traces the AI has no idea about any insights. (Long term we should still do the proper fixing where we tell the AI about all of them). [email protected] Fixed: 449113697 Change-Id: I806ca7fa7770845c08b0d29afa3b8f484cb2793a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7003546 Auto-Submit: Jack Franklin <[email protected]> Reviewed-by: Connor Clark <[email protected]> Commit-Queue: Connor Clark <[email protected]>
1 parent a8da482 commit 7374af9

File tree

3 files changed

+50
-4
lines changed

3 files changed

+50
-4
lines changed

front_end/models/ai_assistance/data_formatters/PerformanceTraceFormatter.snapshot.txt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,29 @@ Available insights:
731731
example question: Which third parties are having the largest impact on my page performance?
732732
=== end content
733733

734+
Title: PerformanceTraceFormatter formatTraceSummary includes INP insight when there is no navigation
735+
Content:
736+
URL: https://b2607f8b04800100000289cb1c0a82ba72253000000000000000001.proxy.googlers.com/long-interaction/index.html?x=35
737+
Bounds: {min: 337943614456, max: 337947260898}
738+
CPU throttling: none
739+
Network throttling: none
740+
Metrics (lab / observed):
741+
- INP: 139 ms, event: (eventKey: s-3347, ts: 337944870984)
742+
- CLS: 0.00
743+
Metrics (field / real users): n/a – no data for this page in CrUX
744+
Available insights:
745+
- insight name: INPBreakdown
746+
description: Start investigating with the longest subpart. [Delays can be minimized](https://web.dev/articles/optimize-inp#optimize_interactions). To reduce processing duration, [optimize the main-thread costs](https://web.dev/articles/optimize-long-tasks), often JS.
747+
relevant trace bounds: {min: 337944870984, max: 337945010222}
748+
example question: Suggest fixes for my longest interaction
749+
example question: Why is a large INP score problematic?
750+
example question: What's the biggest contributor to my longest interaction?
751+
- insight name: ThirdParties
752+
description: 3rd party code can significantly impact load performance. [Reduce and defer loading of 3rd party code](https://web.dev/articles/optimizing-content-efficiency-loading-third-party-javascript/) to prioritize your page's content.
753+
relevant trace bounds: {min: 337944792445, max: 337944809570}
754+
example question: Which third parties are having the largest impact on my page performance?
755+
=== end content
756+
734757
Title: PerformanceTraceFormatter formatNetworkRequests formats network requests that have redirects
735758
Content:
736759
## Network request: http://localhost:3000/redirect3

front_end/models/ai_assistance/data_formatters/PerformanceTraceFormatter.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ describeWithEnvironment('PerformanceTraceFormatter', () => {
5757
const output = formatter.formatTraceSummary();
5858
snapshotTester.assert(this, output);
5959
});
60+
61+
it('includes INP insight when there is no navigation', async function() {
62+
const {formatter} = await createFormatter(this, 'slow-interaction-button-click.json.gz');
63+
const output = formatter.formatTraceSummary();
64+
assert.include(output, 'INP: 139 ms');
65+
assert.include(output, 'insight name: INPBreakdown');
66+
snapshotTester.assert(this, output);
67+
});
6068
});
6169

6270
it('formatCriticalRequests', async function() {

front_end/models/ai_assistance/performance/AIContext.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,26 @@ interface AgentFocusData {
1616
insight: Trace.Insights.Types.InsightModel|null;
1717
}
1818

19+
/**
20+
* Gets the first, most relevant InsightSet to use, following the logic of:
21+
* 1. If there is only one InsightSet, use that.
22+
* 2. If there are more, prefer the first we find that has a navigation associated with it.
23+
* 3. If none with a navigation are found, fallback to the first one.
24+
* 4. Otherwise, return null.
25+
*
26+
* TODO(cjamcl): we should just give the agent the entire insight set, and give
27+
* summary detail about all of them + the ability to query each.
28+
*/
1929
function getFirstInsightSet(insights: Trace.Insights.Types.TraceInsightSets): Trace.Insights.Types.InsightSet|null {
20-
// Currently only support a single insight set. Pick the first one with a navigation.
21-
// TODO(cjamcl): we should just give the agent the entire insight set, and give
22-
// summary detail about all of them + the ability to query each.
23-
return [...insights.values()].filter(insightSet => insightSet.navigation).at(0) ?? null;
30+
const insightSets = Array.from(insights.values());
31+
if (insightSets.length === 0) {
32+
return null;
33+
}
34+
if (insightSets.length === 1) {
35+
return insightSets[0];
36+
}
37+
38+
return insightSets.filter(set => set.navigation).at(0) ?? insightSets.at(0) ?? null;
2439
}
2540

2641
export class AgentFocus {

0 commit comments

Comments
 (0)