Skip to content

Commit d2147d3

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
AI: define unit formatters in models/ai_assistance
So we can begin to not rely on the i18n module. DISABLE_SPELLCHECKER=picking up errors in external dataset we do not control Bug: 443038315, 442509324 Change-Id: I9230f2fedef456bd3fc6c205c5cc34a91ee61dd5 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6915541 Reviewed-by: Nikolay Vitkov <[email protected]> Commit-Queue: Alex Rudenko <[email protected]> Auto-Submit: Jack Franklin <[email protected]> Reviewed-by: Mathias Bynens <[email protected]>
1 parent 71794bb commit d2147d3

20 files changed

+606
-407
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1033,7 +1033,7 @@ grd_files_unbundled_sources = [
10331033
"front_end/models/ai_assistance/data_formatters/NetworkRequestFormatter.js",
10341034
"front_end/models/ai_assistance/data_formatters/PerformanceInsightFormatter.js",
10351035
"front_end/models/ai_assistance/data_formatters/PerformanceTraceFormatter.js",
1036-
"front_end/models/ai_assistance/data_formatters/Types.js",
1036+
"front_end/models/ai_assistance/data_formatters/UnitFormatters.js",
10371037
"front_end/models/ai_assistance/debug.js",
10381038
"front_end/models/ai_assistance/injected.js",
10391039
"front_end/models/ai_code_completion/AiCodeCompletion.js",

front_end/core/i18n/NumberFormatter.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ export function defineFormatter(options: Intl.NumberFormatOptions): NumberFormat
3939
* We don't like that, so when there is no space literal we inject the provided separator manually.
4040
*/
4141
function formatAndEnsureSpace(formatter: Intl.NumberFormat, value: number, separator = '\xA0'): string {
42+
// TODO(crbug.com/443038315): this method is defined in
43+
// models/ai_assistance in the UnitFormatters file. We can't currently
44+
// re-use it because various models depend on i18n and that creates a
45+
// circular build.
46+
// We should move the unit formatters to their own model so it can be
47+
// used across the codebase.
48+
4249
const parts = formatter.formatToParts(value);
4350

4451
let hasSpace = false;

front_end/models/ai_assistance/BUILD.gn

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ devtools_module("ai_assistance") {
2727
"data_formatters/NetworkRequestFormatter.ts",
2828
"data_formatters/PerformanceInsightFormatter.ts",
2929
"data_formatters/PerformanceTraceFormatter.ts",
30-
"data_formatters/Types.ts",
30+
"data_formatters/UnitFormatters.ts",
3131
"debug.ts",
3232
"injected.ts",
3333
]
@@ -90,6 +90,7 @@ ts_library("unittests") {
9090
"data_formatters/NetworkRequestFormatter.test.ts",
9191
"data_formatters/PerformanceInsightFormatter.test.ts",
9292
"data_formatters/PerformanceTraceFormatter.test.ts",
93+
"data_formatters/UnitFormatters.test.ts",
9394
]
9495

9596
deps = [

front_end/models/ai_assistance/ConversationHandler.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ describeWithMockConnection('ConversationHandler', () => {
203203
},
204204
{
205205
text:
206-
'Queued at (timestamp): 0 μs\nStarted at (timestamp): 0 μs\nConnection start (stalled) (duration): -\nDuration (duration): -',
206+
'Queued at (timestamp): 0 s\nStarted at (timestamp): 0 s\nConnection start (stalled) (duration): -\nDuration (duration): -',
207207
title: 'Timing',
208208
},
209209
{

front_end/models/ai_assistance/agents/NetworkAgent.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ describeWithMockConnection('NetworkAgent', () => {
160160
{
161161
title: 'Timing',
162162
text:
163-
'Queued at (timestamp): 0 μs\nStarted at (timestamp): 8.4 min\nQueueing (duration): 8.4 min\nConnection start (stalled) (duration): 800.00 ms\nRequest sent (duration): 100.00 ms\nDuration (duration): 8.4 min',
163+
'Queued at (timestamp): 0 s\nStarted at (timestamp): 501 s\nQueueing (duration): 501 s\nConnection start (stalled) (duration): 800 ms\nRequest sent (duration): 100 ms\nDuration (duration): 501 s',
164164
},
165165
{
166166
title: 'Request initiator chain',
@@ -197,12 +197,12 @@ x-forwarded-for: bar3
197197
198198
Response status: 200 \n
199199
Request timing:
200-
Queued at (timestamp): 0 μs
201-
Started at (timestamp): 8.4 min
202-
Queueing (duration): 8.4 min
203-
Connection start (stalled) (duration): 800.00 ms
204-
Request sent (duration): 100.00 ms
205-
Duration (duration): 8.4 min
200+
Queued at (timestamp): 0 s
201+
Started at (timestamp): 501 s
202+
Queueing (duration): 501 s
203+
Connection start (stalled) (duration): 800 ms
204+
Request sent (duration): 100 ms
205+
Duration (duration): 501 s
206206
207207
Request initiator chain:
208208
- URL: <redacted cross-origin initiator URL>

front_end/models/ai_assistance/agents/PerformanceAgent.test.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import * as Trace from '../../trace/trace.js';
2020
import {
2121
type ActionResponse,
2222
ConversationType,
23-
PERF_AGENT_UNIT_FORMATTERS,
2423
PerformanceAgent,
2524
PerformanceTraceContext,
2625
PerformanceTraceFormatter,
@@ -353,8 +352,7 @@ code
353352
});
354353

355354
const expectedDetailText =
356-
new PerformanceTraceFormatter(
357-
PERF_AGENT_UNIT_FORMATTERS, context.getItem(), new Trace.EventsSerializer.EventsSerializer())
355+
new PerformanceTraceFormatter(context.getItem(), new Trace.EventsSerializer.EventsSerializer())
358356
.formatTraceSummary();
359357

360358
const responses = await Array.fromAsync(agent.run('test', {selected: context}));
@@ -469,8 +467,7 @@ Help me understand?`;
469467
assert.isOk(match, `no request found for ${url}`);
470468
});
471469

472-
const formatter = new PerformanceTraceFormatter(
473-
PERF_AGENT_UNIT_FORMATTERS, context.getItem(), new Trace.EventsSerializer.EventsSerializer());
470+
const formatter = new PerformanceTraceFormatter(context.getItem(), new Trace.EventsSerializer.EventsSerializer());
474471
const expectedRequestsOutput = formatter.formatNetworkTrackSummary(bounds);
475472

476473
const expectedBytesSize = Platform.StringUtilities.countWtf8Bytes(expectedRequestsOutput);
@@ -517,8 +514,7 @@ Help me understand?`;
517514
const action = responses.find(response => response.type === ResponseType.ACTION);
518515
assert.exists(action);
519516

520-
const formatter = new PerformanceTraceFormatter(
521-
PERF_AGENT_UNIT_FORMATTERS, context.getItem(), new Trace.EventsSerializer.EventsSerializer());
517+
const formatter = new PerformanceTraceFormatter(context.getItem(), new Trace.EventsSerializer.EventsSerializer());
522518
const summary = formatter.formatMainThreadTrackSummary(bounds);
523519
assert.isOk(summary);
524520

front_end/models/ai_assistance/agents/PerformanceAgent.ts

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
TraceEventFormatter,
1919
} from '../data_formatters/PerformanceInsightFormatter.js';
2020
import {PerformanceTraceFormatter} from '../data_formatters/PerformanceTraceFormatter.js';
21-
import type {UnitFormatters} from '../data_formatters/Types.js';
2221
import {debugLog} from '../debug.js';
2322

2423
import {
@@ -220,19 +219,6 @@ enum ScorePriority {
220219
DEFAULT = 1,
221220
}
222221

223-
export const PERF_AGENT_UNIT_FORMATTERS: UnitFormatters = {
224-
micros(x) {
225-
const milli = Trace.Helpers.Timing.microToMilli(x as Trace.Types.Timing.Micro);
226-
return PERF_AGENT_UNIT_FORMATTERS.millis(milli);
227-
},
228-
millis(x) {
229-
return i18n.TimeUtilities.preciseMillisToString(x, 1, ' ');
230-
},
231-
bytes(x) {
232-
return i18n.ByteUtilities.bytesToString(x);
233-
},
234-
};
235-
236222
export class PerformanceTraceContext extends ConversationContext<TimelineUtils.AIContext.AgentFocus> {
237223
static full(
238224
parsedTrace: Trace.Handlers.Types.ParsedTrace, insights: Trace.Insights.Types.TraceInsightSets,
@@ -330,8 +316,7 @@ export class PerformanceTraceContext extends ConversationContext<TimelineUtils.A
330316
return;
331317
}
332318

333-
return new PerformanceInsightFormatter(PERF_AGENT_UNIT_FORMATTERS, focus.parsedTrace, focus.insight)
334-
.getSuggestions();
319+
return new PerformanceInsightFormatter(focus.parsedTrace, focus.insight).getSuggestions();
335320
}
336321
}
337322

@@ -672,7 +657,7 @@ export class PerformanceAgent extends AiAgent<TimelineUtils.AIContext.AgentFocus
672657
this.addFact(this.#networkDataDescriptionFact);
673658

674659
if (!this.#traceFacts.length) {
675-
this.#formatter = new PerformanceTraceFormatter(PERF_AGENT_UNIT_FORMATTERS, focus, this.#eventsSerializer);
660+
this.#formatter = new PerformanceTraceFormatter(focus, this.#eventsSerializer);
676661
this.#createFactForTraceSummary(focus);
677662
this.#createFactForCriticalRequests();
678663
this.#createFactForMainThreadBottomUpSummary();
@@ -738,8 +723,7 @@ export class PerformanceAgent extends AiAgent<TimelineUtils.AIContext.AgentFocus
738723
return {error: 'No insight available'};
739724
}
740725

741-
const details =
742-
new PerformanceInsightFormatter(PERF_AGENT_UNIT_FORMATTERS, parsedTrace, insight).formatInsight();
726+
const details = new PerformanceInsightFormatter(parsedTrace, insight).formatInsight();
743727

744728
const key = `getInsightDetails('${params.insightName}')`;
745729
this.#cacheFunctionResult(focus, key, details);

front_end/models/ai_assistance/ai_assistance.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,6 @@ export * from './data_formatters/FileFormatter.js';
2020
export * from './data_formatters/NetworkRequestFormatter.js';
2121
export * from './data_formatters/PerformanceInsightFormatter.js';
2222
export * from './data_formatters/PerformanceTraceFormatter.js';
23-
export * from './data_formatters/Types.js';
23+
export * from './data_formatters/UnitFormatters.js';
2424
export * from './ConversationHandler.js';
2525
export * from './injected.js';

front_end/models/ai_assistance/data_formatters/NetworkRequestFormatter.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import * as i18n from '../../../core/i18n/i18n.js';
65
import type * as SDK from '../../../core/sdk/sdk.js';
76
import * as Logs from '../../logs/logs.js';
87
import * as NetworkTimeCalculator from '../../network_time_calculator/network_time_calculator.js';
98

9+
import {seconds} from './UnitFormatters.js';
10+
1011
const MAX_HEADERS_SIZE = 1000;
1112

1213
/**
@@ -104,22 +105,22 @@ Request initiator chain:\n${this.formatRequestInitiatorChain()}`;
104105
formatNetworkRequestTiming(): string {
105106
const results = NetworkTimeCalculator.calculateRequestTimeRanges(this.#request, this.#calculator.minimumBoundary());
106107

107-
function getDuration(name: string): string|undefined {
108+
const getDuration = (name: string): string|undefined => {
108109
const result = results.find(r => r.name === name);
109110
if (!result) {
110111
return;
111112
}
112-
return i18n.TimeUtilities.secondsToString(result.end - result.start, true);
113-
}
113+
return seconds(result.end - result.start);
114+
};
114115

115116
const labels = [
116117
{
117118
label: 'Queued at (timestamp)',
118-
value: this.#calculator.formatValue(this.#request.issueTime(), 2),
119+
value: seconds(this.#request.issueTime() - this.#calculator.zeroTime()),
119120
},
120121
{
121122
label: 'Started at (timestamp)',
122-
value: this.#calculator.formatValue(this.#request.startTime, 2),
123+
value: seconds(this.#request.startTime - this.#calculator.zeroTime()),
123124
},
124125
{
125126
label: 'Queueing (duration)',

0 commit comments

Comments
 (0)