Skip to content

Commit 5550f7b

Browse files
wolfibDevtools-frontend LUCI CQ
authored andcommitted
Add client_version to AIDA DoConversation requests
Send the Chrome version as metadata to the AIDA API. This allows sorting clients into different experiment groups based on Chrome version. `getChromeVersion` has been moved into `Runtime.ts` to avoid a circular dependency. Bug: 352823552 Change-Id: I8c18e1de3c0296db85392735cd6e2e74f894f724 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6218231 Commit-Queue: Alex Rudenko <[email protected]> Reviewed-by: Alex Rudenko <[email protected]> Auto-Submit: Wolfgang Beyer <[email protected]> Commit-Queue: Wolfgang Beyer <[email protected]>
1 parent d417160 commit 5550f7b

File tree

10 files changed

+102
-26
lines changed

10 files changed

+102
-26
lines changed

front_end/core/host/AidaClient.test.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,26 @@
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 {describeWithEnvironment, updateHostConfig} from '../../testing/EnvironmentHelpers.js';
5+
import {
6+
describeWithEnvironment,
7+
restoreUserAgentForTesting,
8+
setUserAgentForTesting,
9+
updateHostConfig
10+
} from '../../testing/EnvironmentHelpers.js';
611

712
import * as Host from './host.js';
813

914
const TEST_MODEL_ID = 'testModelId';
1015

1116
describeWithEnvironment('AidaClient', () => {
17+
beforeEach(() => {
18+
setUserAgentForTesting();
19+
});
20+
21+
afterEach(() => {
22+
restoreUserAgentForTesting();
23+
});
24+
1225
it('adds no model temperature if console insights is not enabled', () => {
1326
updateHostConfig({
1427
aidaAvailability: {
@@ -21,6 +34,10 @@ describeWithEnvironment('AidaClient', () => {
2134
client: 'CHROME_DEVTOOLS',
2235
client_feature: 1,
2336
functionality_type: 2,
37+
metadata: {
38+
disable_user_content_logging: false,
39+
client_version: 'unit_test',
40+
},
2441
});
2542
});
2643

@@ -43,6 +60,10 @@ describeWithEnvironment('AidaClient', () => {
4360
},
4461
client_feature: 1,
4562
functionality_type: 2,
63+
metadata: {
64+
disable_user_content_logging: false,
65+
client_version: 'unit_test',
66+
},
4667
});
4768
});
4869

@@ -65,6 +86,10 @@ describeWithEnvironment('AidaClient', () => {
6586
},
6687
client_feature: 1,
6788
functionality_type: 2,
89+
metadata: {
90+
disable_user_content_logging: false,
91+
client_version: 'unit_test',
92+
},
6893
});
6994
});
7095

@@ -84,6 +109,10 @@ describeWithEnvironment('AidaClient', () => {
84109
client: 'CHROME_DEVTOOLS',
85110
client_feature: 1,
86111
functionality_type: 2,
112+
metadata: {
113+
disable_user_content_logging: false,
114+
client_version: 'unit_test',
115+
},
87116
});
88117
});
89118

@@ -108,6 +137,10 @@ describeWithEnvironment('AidaClient', () => {
108137
},
109138
client_feature: 1,
110139
functionality_type: 2,
140+
metadata: {
141+
disable_user_content_logging: false,
142+
client_version: 'unit_test',
143+
},
111144
});
112145
});
113146

@@ -127,6 +160,7 @@ describeWithEnvironment('AidaClient', () => {
127160
client: 'CHROME_DEVTOOLS',
128161
metadata: {
129162
disable_user_content_logging: true,
163+
client_version: 'unit_test',
130164
},
131165
options: {
132166
temperature: 0.5,

front_end/core/host/AidaClient.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,12 @@ export interface AidaRequest {
159159
// eslint-disable-next-line @typescript-eslint/naming-convention
160160
model_id?: string,
161161
};
162-
metadata?: {
162+
metadata: {
163163
// eslint-disable-next-line @typescript-eslint/naming-convention
164164
disable_user_content_logging: boolean,
165165
// eslint-disable-next-line @typescript-eslint/naming-convention
166+
client_version: string,
167+
// eslint-disable-next-line @typescript-eslint/naming-convention
166168
string_session_id?: string,
167169
// eslint-disable-next-line @typescript-eslint/naming-convention
168170
user_tier?: UserTier,
@@ -264,12 +266,6 @@ export class AidaBlockError extends Error {}
264266

265267
export class AidaClient {
266268
static buildConsoleInsightsRequest(input: string): AidaRequest {
267-
const request: AidaRequest = {
268-
current_message: {parts: [{text: input}], role: Role.USER},
269-
client: CLIENT_NAME,
270-
functionality_type: FunctionalityType.EXPLAIN_ERROR,
271-
client_feature: ClientFeature.CHROME_CONSOLE_INSIGHTS,
272-
};
273269
const {hostConfig} = Root.Runtime;
274270
let temperature = -1;
275271
let modelId = '';
@@ -278,6 +274,20 @@ export class AidaClient {
278274
modelId = hostConfig.devToolsConsoleInsights.modelId || '';
279275
}
280276
const disallowLogging = hostConfig.aidaAvailability?.disallowLogging ?? true;
277+
const chromeVersion = Root.Runtime.getChromeVersion();
278+
if (!chromeVersion) {
279+
throw new Error('Cannot determine Chrome version');
280+
}
281+
const request: AidaRequest = {
282+
current_message: {parts: [{text: input}], role: Role.USER},
283+
client: CLIENT_NAME,
284+
functionality_type: FunctionalityType.EXPLAIN_ERROR,
285+
client_feature: ClientFeature.CHROME_CONSOLE_INSIGHTS,
286+
metadata: {
287+
disable_user_content_logging: disallowLogging,
288+
client_version: chromeVersion,
289+
},
290+
};
281291

282292
if (temperature >= 0) {
283293
request.options ??= {};
@@ -287,11 +297,6 @@ export class AidaClient {
287297
request.options ??= {};
288298
request.options.model_id = modelId;
289299
}
290-
if (disallowLogging) {
291-
request.metadata = {
292-
disable_user_content_logging: true,
293-
};
294-
}
295300
return request;
296301
}
297302

front_end/core/root/Runtime.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,8 @@ describe('Runtime', () => {
6161

6262
assert.deepEqual(experiments.map(experiment => experiment.name), ['example', 'configurable']);
6363
});
64+
65+
it('getChromeVersion result has the correct shape', () => {
66+
assert.isTrue(/^\d{3}\.0\.0\.0$/.test(Root.Runtime.getChromeVersion()));
67+
});
6468
});

front_end/core/root/Runtime.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ export function isNodeEntry(pathname: string): boolean {
3838
return nodeEntryPoints.some(component => pathname.includes(component));
3939
}
4040

41+
export const getChromeVersion = (): string => {
42+
const chromeRegex = /(?:^|\W)(?:Chrome|HeadlessChrome)\/(\S+)/;
43+
const chromeMatch = navigator.userAgent.match(chromeRegex);
44+
if (chromeMatch && chromeMatch.length > 1) {
45+
return chromeMatch[1];
46+
}
47+
return '';
48+
};
49+
4150
export class Runtime {
4251
private constructor() {
4352
}

front_end/core/sdk/NetworkManager.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,19 +1362,10 @@ export class MultitargetNetworkManager extends Common.ObjectWrapper.ObjectWrappe
13621362
multiTargetNetworkManagerInstance = null;
13631363
}
13641364

1365-
static getChromeVersion(): string {
1366-
const chromeRegex = /(?:^|\W)(?:Chrome|HeadlessChrome)\/(\S+)/;
1367-
const chromeMatch = navigator.userAgent.match(chromeRegex);
1368-
if (chromeMatch && chromeMatch.length > 1) {
1369-
return chromeMatch[1];
1370-
}
1371-
return '';
1372-
}
1373-
13741365
static patchUserAgentWithChromeVersion(uaString: string): string {
13751366
// Patches Chrome/ChrOS version from user #agent ("1.2.3.4" when user #agent is: "Chrome/1.2.3.4").
13761367
// Otherwise, ignore it. This assumes additional appVersions appear after the Chrome version.
1377-
const chromeVersion = MultitargetNetworkManager.getChromeVersion();
1368+
const chromeVersion = Root.Runtime.getChromeVersion();
13781369
if (chromeVersion.length > 0) {
13791370
// "1.2.3.4" becomes "1.0.100.0"
13801371
const additionalAppVersion = chromeVersion.split('.', 1)[0] + '.0.100.0';
@@ -1389,7 +1380,7 @@ export class MultitargetNetworkManager extends Common.ObjectWrapper.ObjectWrappe
13891380
if (!userAgentMetadata.brands) {
13901381
return;
13911382
}
1392-
const chromeVersion = MultitargetNetworkManager.getChromeVersion();
1383+
const chromeVersion = Root.Runtime.getChromeVersion();
13931384
if (chromeVersion.length === 0) {
13941385
return;
13951386
}

front_end/panels/ai_assistance/agents/AiAgent.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
import * as Host from '../../../core/host/host.js';
6+
import * as Root from '../../../core/root/root.js';
67
import type * as Lit from '../../../ui/lit/lit.js';
78
import {debugLog, isDebugMode} from '../debug.js';
89

@@ -290,6 +291,7 @@ export abstract class AiAgent<T> {
290291
disable_user_content_logging: !(this.#serverSideLoggingEnabled ?? false),
291292
string_session_id: this.#sessionId,
292293
user_tier: Host.AidaClient.convertToUserTierEnum(this.userTier),
294+
client_version: Root.Runtime.getChromeVersion(),
293295
},
294296

295297
functionality_type: declarations.length ? Host.AidaClient.FunctionalityType.AGENTIC_CHAT :

front_end/panels/ai_assistance/agents/FileAgent.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ import * as SDK from '../../../core/sdk/sdk.js';
77
import * as Bindings from '../../../models/bindings/bindings.js';
88
import * as Workspace from '../../../models/workspace/workspace.js';
99
import {createUISourceCode, mockAidaClient} from '../../../testing/AiAssistanceHelpers.js';
10-
import {updateHostConfig} from '../../../testing/EnvironmentHelpers.js';
10+
import {
11+
restoreUserAgentForTesting,
12+
setUserAgentForTesting,
13+
updateHostConfig,
14+
} from '../../../testing/EnvironmentHelpers.js';
1115
import {describeWithMockConnection} from '../../../testing/MockConnection.js';
1216
import {FileAgent, FileContext, ResponseType} from '../ai_assistance.js';
1317

@@ -74,6 +78,7 @@ describeWithMockConnection('FileAgent', () => {
7478
sinon.stub(agent, 'preamble').value('preamble');
7579
await Array.fromAsync(agent.run('question', {selected: null}));
7680

81+
setUserAgentForTesting();
7782
assert.deepEqual(
7883
agent.buildRequest({text: 'test input'}, Host.AidaClient.Role.USER),
7984
{
@@ -94,6 +99,7 @@ describeWithMockConnection('FileAgent', () => {
9499
disable_user_content_logging: false,
95100
string_session_id: 'sessionId',
96101
user_tier: 2,
102+
client_version: 'unit_test',
97103
},
98104
options: {
99105
model_id: 'test model',
@@ -103,6 +109,7 @@ describeWithMockConnection('FileAgent', () => {
103109
functionality_type: 1,
104110
},
105111
);
112+
restoreUserAgentForTesting();
106113
});
107114
});
108115

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@
55
import * as Host from '../../../core/host/host.js';
66
import * as Trace from '../../../models/trace/trace.js';
77
import {mockAidaClient} from '../../../testing/AiAssistanceHelpers.js';
8-
import {describeWithEnvironment, updateHostConfig} from '../../../testing/EnvironmentHelpers.js';
8+
import {
9+
describeWithEnvironment,
10+
restoreUserAgentForTesting,
11+
setUserAgentForTesting,
12+
updateHostConfig
13+
} from '../../../testing/EnvironmentHelpers.js';
914
import {TraceLoader} from '../../../testing/TraceLoader.js';
1015
import * as TimelineUtils from '../../timeline/utils/utils.js';
1116
import {CallTreeContext, PerformanceAgent, ResponseType} from '../ai_assistance.js';
@@ -83,6 +88,7 @@ describeWithEnvironment('PerformanceAgent', () => {
8388
sinon.stub(agent, 'preamble').value('preamble');
8489

8590
await Array.fromAsync(agent.run('question', {selected: null}));
91+
setUserAgentForTesting();
8692

8793
assert.deepEqual(
8894
agent.buildRequest(
@@ -108,6 +114,7 @@ describeWithEnvironment('PerformanceAgent', () => {
108114
disable_user_content_logging: false,
109115
string_session_id: 'sessionId',
110116
user_tier: 2,
117+
client_version: 'unit_test',
111118
},
112119
options: {
113120
model_id: 'test model',
@@ -117,6 +124,7 @@ describeWithEnvironment('PerformanceAgent', () => {
117124
functionality_type: 1,
118125
},
119126
);
127+
restoreUserAgentForTesting();
120128
});
121129
});
122130
describe('run', function() {

front_end/panels/ai_assistance/agents/StylingAgent.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import * as SDK from '../../../core/sdk/sdk.js';
88
import {mockAidaClient} from '../../../testing/AiAssistanceHelpers.js';
99
import {
1010
describeWithEnvironment,
11+
restoreUserAgentForTesting,
12+
setUserAgentForTesting,
1113
updateHostConfig,
1214
} from '../../../testing/EnvironmentHelpers.js';
1315
import * as AiAssistance from '../ai_assistance.js';
@@ -527,6 +529,7 @@ c`;
527529
sinon.stub(agent, 'preamble').value('preamble');
528530
await Array.fromAsync(agent.run('question', {selected: null}));
529531

532+
setUserAgentForTesting();
530533
assert.deepEqual(
531534
agent.buildRequest(
532535
{
@@ -551,6 +554,7 @@ c`;
551554
disable_user_content_logging: false,
552555
string_session_id: 'sessionId',
553556
user_tier: 2,
557+
client_version: 'unit_test',
554558
},
555559
options: {
556560
model_id: 'test model',
@@ -560,6 +564,7 @@ c`;
560564
functionality_type: 1,
561565
},
562566
);
567+
restoreUserAgentForTesting();
563568
});
564569

565570
it('builds a request with aborted query in history before a real request', async () => {

front_end/testing/EnvironmentHelpers.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,17 @@ export function expectConsoleLogs(expectedLogs: {warn?: string[], log?: string[]
501501
});
502502
}
503503

504+
let originalUserAgent: string;
505+
506+
export function setUserAgentForTesting(): void {
507+
originalUserAgent = window.navigator.userAgent;
508+
Object.defineProperty(window.navigator, 'userAgent', {value: 'Chrome/unit_test', configurable: true});
509+
}
510+
511+
export function restoreUserAgentForTesting(): void {
512+
Object.defineProperty(window.navigator, 'userAgent', {value: originalUserAgent});
513+
}
514+
504515
export function resetHostConfig() {
505516
for (const key of Object.keys(Root.Runtime.hostConfig)) {
506517
// @ts-expect-error

0 commit comments

Comments
 (0)