Skip to content

Commit 106b5d9

Browse files
mvanhornmvanhorn
committed
Move CrUX ratings into formatTraceFormatter per review
Replace regex post-processing of formatter output with direct access to CrUX field metrics via DevTools.TraceEngine.Insights.Common. Ratings are now computed from structured data and injected into the summary, avoiding fragile string parsing.
1 parent 5ce83fd commit 106b5d9

2 files changed

Lines changed: 122 additions & 113 deletions

File tree

‎src/trace-processing/parse.ts‎

Lines changed: 97 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,13 @@ ${DevTools.PerformanceTraceFormatter.networkDataFormatDescription}`;
7979
type Rating = 'good' | 'needs-improvement' | 'poor';
8080

8181
/**
82-
* Rate a Web Vitals metric value against its thresholds.
82+
* Rate a timing-based Web Vitals metric value (in ms) against its thresholds.
8383
* Thresholds are from https://web.dev/articles/vitals
8484
*/
85-
function rateMetric(metric: string, value: number): Rating | null {
85+
export function rateTimingMetric(
86+
metric: string,
87+
valueMs: number,
88+
): Rating | null {
8689
const thresholds: Record<string, {good: number; poor: number}> = {
8790
LCP: {good: 2500, poor: 4000},
8891
FCP: {good: 1800, poor: 3000},
@@ -94,16 +97,16 @@ function rateMetric(metric: string, value: number): Rating | null {
9497
if (!t) {
9598
return null;
9699
}
97-
if (value <= t.good) {
100+
if (valueMs <= t.good) {
98101
return 'good';
99102
}
100-
if (value >= t.poor) {
103+
if (valueMs >= t.poor) {
101104
return 'poor';
102105
}
103106
return 'needs-improvement';
104107
}
105108

106-
function rateCLS(value: number): Rating {
109+
export function rateCLS(value: number): Rating {
107110
if (value <= 0.1) {
108111
return 'good';
109112
}
@@ -114,49 +117,103 @@ function rateCLS(value: number): Rating {
114117
}
115118

116119
/**
117-
* Post-process the formatter output to append a rating to each CrUX field
118-
* metric line. Lines produced by the DevTools formatter look like:
119-
* - LCP: 2595 ms (scope: url)
120-
* - INP: 140 ms (scope: url)
121-
* - CLS: 0.06 (scope: url)
122-
* - TTFB: 1273 ms (scope: url)
123-
* - FCP: 2425 ms (scope: url)
120+
* Build a CrUX field metrics section with ratings included directly,
121+
* using the structured data from the trace insights rather than
122+
* regex post-processing.
124123
*/
125-
export function addRatingsToCruxMetrics(summary: string): string {
126-
const timingMetricRe =
127-
/^(\s+- (?:LCP|INP|FCP|TTFB): )(\d+) ms( \(scope: \w+\))$/;
128-
const clsMetricRe = /^(\s+- CLS: )(\d+\.\d+)( \(scope: \w+\))$/;
129-
130-
return summary
131-
.split('\n')
132-
.map(line => {
133-
const timingMatch = line.match(timingMetricRe);
134-
if (timingMatch) {
135-
const metric = timingMatch[1]
136-
.trim()
137-
.replace(/^- /, '')
138-
.replace(/:.*/, '');
139-
const value = Number(timingMatch[2]);
140-
const rating = rateMetric(metric, value);
141-
if (rating) {
142-
return `${timingMatch[1]}${timingMatch[2]} ms${timingMatch[3]} [${rating}]`;
143-
}
124+
function buildRatedCruxSection(result: TraceResult): string[] | null {
125+
const parsedTrace = result.parsedTrace;
126+
const insights = result.insights;
127+
if (!insights) {
128+
return null;
129+
}
130+
131+
// Find the first insight set with CrUX data.
132+
for (const insightSet of insights.values()) {
133+
try {
134+
const cruxScope =
135+
DevTools.CrUXManager.instance().getSelectedScope();
136+
const fieldMetrics =
137+
DevTools.TraceEngine.Insights.Common.getFieldMetricsForInsightSet(
138+
insightSet,
139+
parsedTrace.metadata,
140+
cruxScope,
141+
);
142+
143+
if (!fieldMetrics) {
144+
continue;
145+
}
146+
147+
const {lcp: fieldLcp, inp: fieldInp, cls: fieldCls} = fieldMetrics;
148+
if (!fieldLcp && !fieldInp && !fieldCls) {
149+
continue;
150+
}
151+
152+
const parts: string[] = [];
153+
parts.push('Metrics (field / real users):');
154+
155+
if (fieldLcp) {
156+
const ms = Math.round(fieldLcp.value / 1000);
157+
const rating = rateTimingMetric('LCP', ms);
158+
const ratingStr = rating ? ` [${rating}]` : '';
159+
parts.push(
160+
` - LCP: ${ms} ms (scope: ${fieldLcp.pageScope})${ratingStr}`,
161+
);
144162
}
145-
const clsMatch = line.match(clsMetricRe);
146-
if (clsMatch) {
147-
const value = Number(clsMatch[2]);
148-
const rating = rateCLS(value);
149-
return `${clsMatch[1]}${clsMatch[2]}${clsMatch[3]} [${rating}]`;
163+
if (fieldInp) {
164+
const ms = Math.round(fieldInp.value / 1000);
165+
const rating = rateTimingMetric('INP', ms);
166+
const ratingStr = rating ? ` [${rating}]` : '';
167+
parts.push(
168+
` - INP: ${ms} ms (scope: ${fieldInp.pageScope})${ratingStr}`,
169+
);
150170
}
151-
return line;
152-
})
153-
.join('\n');
171+
if (fieldCls) {
172+
const clsValue = fieldCls.value;
173+
const rating = rateCLS(clsValue);
174+
parts.push(
175+
` - CLS: ${clsValue.toFixed(2)} (scope: ${fieldCls.pageScope}) [${rating}]`,
176+
);
177+
}
178+
179+
return parts;
180+
} catch {
181+
continue;
182+
}
183+
}
184+
185+
return null;
154186
}
155187

156188
export function getTraceSummary(result: TraceResult): string {
157189
const focus = DevTools.AgentFocus.fromParsedTrace(result.parsedTrace);
158190
const formatter = new DevTools.PerformanceTraceFormatter(focus);
159-
const summaryText = addRatingsToCruxMetrics(formatter.formatTraceSummary());
191+
let summaryText = formatter.formatTraceSummary();
192+
193+
// Replace the CrUX section in the formatter output with our rated version.
194+
const ratedCrux = buildRatedCruxSection(result);
195+
if (ratedCrux) {
196+
const lines = summaryText.split('\n');
197+
const cruxHeaderIdx = lines.findIndex(l =>
198+
l.startsWith('Metrics (field / real users):'),
199+
);
200+
if (cruxHeaderIdx !== -1) {
201+
// Find the end of the CrUX section (next non-indented line or section header).
202+
let endIdx = cruxHeaderIdx + 1;
203+
while (
204+
endIdx < lines.length &&
205+
(lines[endIdx].startsWith(' - ') || lines[endIdx].startsWith(' - '))
206+
) {
207+
endIdx++;
208+
}
209+
lines.splice(
210+
cruxHeaderIdx,
211+
endIdx - cruxHeaderIdx,
212+
...ratedCrux,
213+
);
214+
summaryText = lines.join('\n');
215+
}
216+
}
160217
return `## Summary of Performance trace findings:
161218
${summaryText}
162219

‎tests/trace-processing/parse.test.ts‎

Lines changed: 25 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ import assert from 'node:assert';
88
import {describe, it} from 'node:test';
99

1010
import {
11-
addRatingsToCruxMetrics,
1211
getTraceSummary,
1312
parseRawTraceBuffer,
13+
rateCLS,
14+
rateTimingMetric,
1415
} from '../../src/trace-processing/parse.js';
1516

1617
import '../../src/DevtoolsUtils.js';
@@ -41,96 +42,47 @@ describe('Trace parsing', async () => {
4142
t.assert.snapshot?.(output);
4243
});
4344

44-
describe('addRatingsToCruxMetrics', () => {
45-
it('adds good rating for fast LCP', () => {
46-
const input = ' - LCP: 1500 ms (scope: url)';
47-
assert.strictEqual(
48-
addRatingsToCruxMetrics(input),
49-
' - LCP: 1500 ms (scope: url) [good]',
50-
);
45+
describe('rateTimingMetric', () => {
46+
it('rates fast LCP as good', () => {
47+
assert.strictEqual(rateTimingMetric('LCP', 1500), 'good');
5148
});
5249

53-
it('adds needs-improvement rating for moderate LCP', () => {
54-
const input = ' - LCP: 3000 ms (scope: url)';
55-
assert.strictEqual(
56-
addRatingsToCruxMetrics(input),
57-
' - LCP: 3000 ms (scope: url) [needs-improvement]',
58-
);
50+
it('rates moderate LCP as needs-improvement', () => {
51+
assert.strictEqual(rateTimingMetric('LCP', 3000), 'needs-improvement');
5952
});
6053

61-
it('adds poor rating for slow LCP', () => {
62-
const input = ' - LCP: 5000 ms (scope: url)';
63-
assert.strictEqual(
64-
addRatingsToCruxMetrics(input),
65-
' - LCP: 5000 ms (scope: url) [poor]',
66-
);
54+
it('rates slow LCP as poor', () => {
55+
assert.strictEqual(rateTimingMetric('LCP', 5000), 'poor');
6756
});
6857

69-
it('adds good rating for fast INP', () => {
70-
const input = ' - INP: 100 ms (scope: url)';
71-
assert.strictEqual(
72-
addRatingsToCruxMetrics(input),
73-
' - INP: 100 ms (scope: url) [good]',
74-
);
58+
it('rates fast INP as good', () => {
59+
assert.strictEqual(rateTimingMetric('INP', 100), 'good');
7560
});
7661

77-
it('adds good rating for low CLS', () => {
78-
const input = ' - CLS: 0.05 (scope: url)';
79-
assert.strictEqual(
80-
addRatingsToCruxMetrics(input),
81-
' - CLS: 0.05 (scope: url) [good]',
82-
);
62+
it('rates FCP at boundary as needs-improvement', () => {
63+
assert.strictEqual(rateTimingMetric('FCP', 2500), 'needs-improvement');
8364
});
8465

85-
it('adds poor rating for high CLS', () => {
86-
const input = ' - CLS: 0.30 (scope: url)';
87-
assert.strictEqual(
88-
addRatingsToCruxMetrics(input),
89-
' - CLS: 0.30 (scope: url) [poor]',
90-
);
66+
it('rates fast TTFB as good', () => {
67+
assert.strictEqual(rateTimingMetric('TTFB', 500), 'good');
9168
});
9269

93-
it('adds rating for FCP', () => {
94-
const input = ' - FCP: 2500 ms (scope: origin)';
95-
assert.strictEqual(
96-
addRatingsToCruxMetrics(input),
97-
' - FCP: 2500 ms (scope: origin) [needs-improvement]',
98-
);
70+
it('returns null for unknown metrics', () => {
71+
assert.strictEqual(rateTimingMetric('UNKNOWN', 100), null);
9972
});
73+
});
10074

101-
it('adds rating for TTFB', () => {
102-
const input = ' - TTFB: 500 ms (scope: url)';
103-
assert.strictEqual(
104-
addRatingsToCruxMetrics(input),
105-
' - TTFB: 500 ms (scope: url) [good]',
106-
);
75+
describe('rateCLS', () => {
76+
it('rates low CLS as good', () => {
77+
assert.strictEqual(rateCLS(0.05), 'good');
10778
});
10879

109-
it('does not modify non-CrUX lines', () => {
110-
const input = ' - LCP: 1500 ms, event: (eventKey: 1, ts: 123)';
111-
assert.strictEqual(addRatingsToCruxMetrics(input), input);
80+
it('rates moderate CLS as needs-improvement', () => {
81+
assert.strictEqual(rateCLS(0.15), 'needs-improvement');
11282
});
11383

114-
it('handles multi-line summary with mixed content', () => {
115-
const input = [
116-
'Metrics (field / real users):',
117-
' - LCP: 2595 ms (scope: url)',
118-
' - LCP breakdown:',
119-
' - TTFB: 1273 ms (scope: url)',
120-
' - INP: 140 ms (scope: url)',
121-
' - CLS: 0.06 (scope: url)',
122-
' - The above data is from CrUX',
123-
].join('\n');
124-
const expected = [
125-
'Metrics (field / real users):',
126-
' - LCP: 2595 ms (scope: url) [needs-improvement]',
127-
' - LCP breakdown:',
128-
' - TTFB: 1273 ms (scope: url) [needs-improvement]',
129-
' - INP: 140 ms (scope: url) [good]',
130-
' - CLS: 0.06 (scope: url) [good]',
131-
' - The above data is from CrUX',
132-
].join('\n');
133-
assert.strictEqual(addRatingsToCruxMetrics(input), expected);
84+
it('rates high CLS as poor', () => {
85+
assert.strictEqual(rateCLS(0.30), 'poor');
13486
});
13587
});
13688

0 commit comments

Comments
 (0)