Nikolay Vitkov (Gerrit)

unread,

Oct 30, 2025, 9:40:18 AM (10 days ago) Oct 30

to Nicholas Roscino, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Nicholas Roscino

Nikolay Vitkov added 1 comment

File front_end/models/ai_assistance/injected.ts

Line 93, Patchset 6 (Latest):export const pageBindings = ['setElementStyles'];

Nikolay Vitkov . unresolved

Note: this is not `PageBinding` in the terms we usually use for CDP.
See FREESTYLER_BINDING_NAME and CDP `Runtime.addBinding` for more context.

Open in Gerrit

Related details

Attention is currently required from:

Nicholas Roscino

Submit Requirements:

Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Nicholas Roscino (Gerrit)

unread,

Nov 3, 2025, 7:22:55 AM (6 days ago) Nov 3

to Nikolay Vitkov, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Nikolay Vitkov

Nicholas Roscino voted and added 1 comment

Votes added by Nicholas Roscino

Auto-Submit	+1
Commit-Queue	+2

1 comment

File front_end/models/ai_assistance/injected.ts

Line 93, Patchset 6:export const pageBindings = ['setElementStyles'];

Nikolay Vitkov . resolved

Note: this is not `PageBinding` in the terms we usually use for CDP.
See FREESTYLER_BINDING_NAME and CDP `Runtime.addBinding` for more context.

Nicholas Roscino

Done

Open in Gerrit

Related details

Attention is currently required from:

Nikolay Vitkov

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Nicholas Roscino (Gerrit)

unread,

Nov 3, 2025, 9:42:31 AM (6 days ago) Nov 3

to Alex Rudenko, Nikolay Vitkov, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Alex Rudenko and Nikolay Vitkov

Nicholas Roscino voted

Auto-Submit	+1
Commit-Queue	+2

Open in Gerrit

Related details

Attention is currently required from:

Alex Rudenko
Nikolay Vitkov

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Alex Rudenko (Gerrit)

unread,

Nov 3, 2025, 9:48:19 AM (6 days ago) Nov 3

to Nicholas Roscino, Nikolay Vitkov, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Nicholas Roscino and Nikolay Vitkov

Alex Rudenko added 1 comment

File front_end/models/ai_assistance/EvaluateAction.ts

Line 16, Patchset 9 (Latest):export function stringifyErrorOnThePage(this: Error, executedCode: string[], bindingFunctions: string[]): string {

Alex Rudenko . unresolved

let's only return `this.stack` from this function and handle the stack trace parsing in the DevTools context (stringifyErrorOnThePage runs in the page context).

Open in Gerrit

Related details

Attention is currently required from:

Nicholas Roscino
Nikolay Vitkov

Submit Requirements:

Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Nicholas Roscino (Gerrit)

unread,

Nov 4, 2025, 10:14:47 AM (5 days ago) Nov 4

to Alex Rudenko, Nikolay Vitkov, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Alex Rudenko and Nikolay Vitkov

Nicholas Roscino voted and added 1 comment

Votes added by Nicholas Roscino

Auto-Submit

+0

1 comment

File front_end/models/ai_assistance/EvaluateAction.ts

Line 16, Patchset 9:export function stringifyErrorOnThePage(this: Error, executedCode: string[], bindingFunctions: string[]): string {

Alex Rudenko . resolved

let's only return `this.stack` from this function and handle the stack trace parsing in the DevTools context (stringifyErrorOnThePage runs in the page context).

Nicholas Roscino

Marked as resolved.

Open in Gerrit

Related details

Attention is currently required from:

Alex Rudenko
Nikolay Vitkov

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Alex Rudenko (Gerrit)

unread,

Nov 4, 2025, 10:18:47 AM (5 days ago) Nov 4

to Nicholas Roscino, Nikolay Vitkov, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Nicholas Roscino and Nikolay Vitkov

Alex Rudenko voted and added 2 comments

Votes added by Alex Rudenko

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Alex Rudenko . resolved

LGTM thanks!

File front_end/models/ai_assistance/EvaluateAction.ts

Line 22, Patchset 10 (Latest): if (!this.stack) {

Alex Rudenko . unresolved

I think this if is redundant. If stack is not there, it is not be serialised to json if it is undefined.

Open in Gerrit

Related details

Attention is currently required from:

Nicholas Roscino
Nikolay Vitkov

Submit Requirements:

Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Nicholas Roscino (Gerrit)

unread,

Nov 4, 2025, 11:34:00 AM (5 days ago) Nov 4

to Alex Rudenko, Nikolay Vitkov, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Nikolay Vitkov

Nicholas Roscino added 1 comment

File front_end/models/ai_assistance/EvaluateAction.ts

Line 22, Patchset 10: if (!this.stack) {

Alex Rudenko . resolved

I think this if is redundant. If stack is not there, it is not be serialised to json if it is undefined.

Nicholas Roscino

Right! Removed

Open in Gerrit

Related details

Attention is currently required from:

Nikolay Vitkov

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Nicholas Roscino (Gerrit)

unread,

Nov 4, 2025, 11:52:02 AM (5 days ago) Nov 4

to Alex Rudenko, Nikolay Vitkov, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Nikolay Vitkov

Nicholas Roscino voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Nikolay Vitkov

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Nikolay Vitkov (Gerrit)

unread,

Nov 4, 2025, 12:13:59 PM (5 days ago) Nov 4

to Nicholas Roscino, Alex Rudenko, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Attention needed from Nicholas Roscino

Nikolay Vitkov voted and added 10 comments

Votes added by Nikolay Vitkov

Code-Review

+1

10 comments

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Nikolay Vitkov . resolved

LGTM, with some comments and nits.

File front_end/models/ai_assistance/EvaluateAction.ts

Line 27, Patchset 11 (Latest): if (this instanceof Error) {

Nikolay Vitkov . unresolved

Do we need this, now what we branch out the error?

Line 75, Patchset 11 (Latest): let res;

Nikolay Vitkov . unresolved

I don't think we need to use a let here, each branch returns.

Line 146, Patchset 11 (Latest): const stackLines = lines.map(curr => curr.trim()).filter((trimmedLine: string) => {

Nikolay Vitkov . unresolved

Unnecessary type cast `trimmedLine: string` as it is already inferred.

Line 152, Patchset 11 (Latest): // based on the filter above we can assume we have at least 2 elements in splittedStackLine at this point

Nikolay Vitkov . unresolved

Do we need to do that in the above filter, or we can move it to this find with return false?

Line 193, Patchset 11 (Latest): const functionLines = functionDeclaration?.split('\n');

Nikolay Vitkov . unresolved

Nit: Unnecessary optional chaining

Line 195, Patchset 11 (Latest): const executedCode = functionLines[lineNum];

Nikolay Vitkov . unresolved

I think here we mean error line?

File front_end/models/ai_assistance/injected.ts

Line 59, Patchset 11 (Latest): freestyler.id, {args: JSON.stringify(args), element: args.element, resolve, reject, error: args.error});

Nikolay Vitkov . unresolved

Super nit: Comma after args.error to keep the diff minimal.

Line 93, Patchset 11 (Latest):export const pageExposedFunctions = ['setElementStyles'];

Nikolay Vitkov . unresolved

Nit: This is const and should follow naming convention - capital letters, _ as separator.

Line 145, Patchset 11 (Latest): {method: 'setElementStyles', selector, className, styles, element: el, error: bindingError});

Nikolay Vitkov . unresolved

Super Nit: you can keep as smaller diff if you add `,` after bindingError

Open in Gerrit

Related details

Attention is currently required from:

Nicholas Roscino

Submit Requirements:

Code-Owners
Code-Review

No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Nicholas Roscino (Gerrit)

unread,

Nov 5, 2025, 5:19:17 AM (4 days ago) Nov 5

to Nikolay Vitkov, Alex Rudenko, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Nicholas Roscino added 9 comments

File front_end/models/ai_assistance/EvaluateAction.ts

Line 27, Patchset 11 (Latest): if (this instanceof Error) {

Nikolay Vitkov . resolved

Do we need this, now what we branch out the error?

Nicholas Roscino

Probably not, removed!

Line 75, Patchset 11 (Latest): let res;

Nikolay Vitkov . resolved

I don't think we need to use a let here, each branch returns.

Nicholas Roscino

Done

Line 146, Patchset 11 (Latest): const stackLines = lines.map(curr => curr.trim()).filter((trimmedLine: string) => {

Nikolay Vitkov . resolved

Unnecessary type cast `trimmedLine: string` as it is already inferred.

Nicholas Roscino

Done

Line 152, Patchset 11 (Latest): // based on the filter above we can assume we have at least 2 elements in splittedStackLine at this point

Nikolay Vitkov . resolved

Do we need to do that in the above filter, or we can move it to this find with return false?

Nicholas Roscino

We can move it without problems I believe!

Line 193, Patchset 11 (Latest): const functionLines = functionDeclaration?.split('\n');

Nikolay Vitkov . resolved

Nit: Unnecessary optional chaining

Nicholas Roscino

Done

Line 195, Patchset 11 (Latest): const executedCode = functionLines[lineNum];

Nikolay Vitkov . resolved

I think here we mean error line?

Nicholas Roscino

Marked as resolved.

File front_end/models/ai_assistance/injected.ts

Line 59, Patchset 11 (Latest): freestyler.id, {args: JSON.stringify(args), element: args.element, resolve, reject, error: args.error});

Nikolay Vitkov . resolved

Super nit: Comma after args.error to keep the diff minimal.

Nicholas Roscino

Done

Line 93, Patchset 11 (Latest):export const pageExposedFunctions = ['setElementStyles'];

Nikolay Vitkov . resolved

Nit: This is const and should follow naming convention - capital letters, _ as separator.

Nicholas Roscino

Acknowledged

Line 145, Patchset 11 (Latest): {method: 'setElementStyles', selector, className, styles, element: el, error: bindingError});

Nikolay Vitkov . resolved

Super Nit: you can keep as smaller diff if you add `,` after bindingError

Nicholas Roscino

Done

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Nicholas Roscino (Gerrit)

unread,

Nov 5, 2025, 5:25:30 AM (4 days ago) Nov 5

to Nikolay Vitkov, Alex Rudenko, Devtools-frontend LUCI CQ, devtools-rev...@chromium.org

Nicholas Roscino voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Devtools-frontend LUCI CQ (Gerrit)

unread,

Nov 5, 2025, 6:35:02 AM (4 days ago) Nov 5

to Nicholas Roscino, Nikolay Vitkov, Alex Rudenko, devtools-rev...@chromium.org

Devtools-frontend LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

11 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: front_end/models/ai_assistance/EvaluateAction.ts
Insertions: 14, Deletions: 15.

@@ -5,7 +5,7 @@
 import * as SDK from '../../core/sdk/sdk.js';
 import * as Protocol from '../../generated/protocol.js';
 
-import {pageExposedFunctions} from './injected.js';
+import {PAGE_EXPOSED_FUNCTIONS} from './injected.js';
 
 export function formatError(message: string): string {
   return `Error: ${message}`;
@@ -24,9 +24,6 @@
 
 /* istanbul ignore next */
 export function stringifyObjectOnThePage(this: unknown): string {
-  if (this instanceof Error) {
-    return `Error: ${this.message}`;
-  }
   const seenBefore = new WeakMap();
   return JSON.stringify(this, function replacer(this: unknown, key: string, value: unknown) {
     if (typeof value === 'object' && value !== null) {
@@ -72,9 +69,8 @@
     case Protocol.Runtime.RemoteObjectType.Function:
       return `${object.description}`;
     case Protocol.Runtime.RemoteObjectType.Object: {
-      let res;
       if (object.subtype === 'error') {
-        res = await object.callFunctionJSON(getErrorStackOnThePage, []);
+        const res = await object.callFunctionJSON(getErrorStackOnThePage, []);
 
         if (!res) {
           throw new Error('Could not stringify the object' + object);
@@ -82,7 +78,7 @@
 
         return EvaluateAction.stringifyError(res, functionDeclaration);
       }
-      res = await object.callFunction(stringifyObjectOnThePage);
+      const res = await object.callFunction(stringifyObjectOnThePage);
 
       if (!res.object || res.object.type !== Protocol.Runtime.RemoteObjectType.String) {
         throw new Error('Could not stringify the object' + object);
@@ -143,13 +139,16 @@
   static getExecutedLineFromStack(stack: string, pageExposedFunctions: string[]): number|null {
     const lines = stack.split('\n');
 
-    const stackLines = lines.map(curr => curr.trim()).filter((trimmedLine: string) => {
-      return trimmedLine.startsWith('at') && trimmedLine.split(' ').length >= 2;
+    const stackLines = lines.map(curr => curr.trim()).filter(trimmedLine => {
+      return trimmedLine.startsWith('at');
     });
 
     const selectedStack = stackLines.find(stackLine => {
       const splittedStackLine = stackLine.split(' ');
-      // based on the filter above we can assume we have at least 2 elements in splittedStackLine at this point
+
+      if (splittedStackLine.length < 2) {
+        return false;
+      }
 
       const signature = splittedStackLine[1] === 'async' ?
           splittedStackLine[2] :  // if the stack line contains async the function name is the next element
@@ -185,18 +184,18 @@
       return `Error: ${result.message}`;
     }
 
-    const lineNum = EvaluateAction.getExecutedLineFromStack(result.stack, pageExposedFunctions);
+    const lineNum = EvaluateAction.getExecutedLineFromStack(result.stack, PAGE_EXPOSED_FUNCTIONS);
     if (!lineNum) {
       return `Error: ${result.message}`;
     }
 
-    const functionLines = functionDeclaration?.split('\n');
+    const functionLines = functionDeclaration.split('\n');
 
-    const executedCode = functionLines[lineNum];
-    if (!executedCode) {
+    const errorLine = functionLines[lineNum];
+    if (!errorLine) {
       return `Error: ${result.message}`;
     }
 
-    return `Error: executing the line "${executedCode.trim()}" failed with the following error:\n${result.message}`;
+    return `Error: executing the line "${errorLine.trim()}" failed with the following error:\n${result.message}`;
   }
 }
```
```
The name of the file: front_end/models/ai_assistance/injected.ts
Insertions: 16, Deletions: 5.

@@ -55,8 +55,13 @@
   if (!global.freestyler) {
     const freestyler = (args: FreestyleCallbackArgs): Promise<string> => {
       const {resolve, reject, promise} = Promise.withResolvers<string>();
-      freestyler.callbacks.set(
-          freestyler.id, {args: JSON.stringify(args), element: args.element, resolve, reject, error: args.error});
+      freestyler.callbacks.set(freestyler.id, {
+        args: JSON.stringify(args),
+        element: args.element,
+        resolve,
+        reject,
+        error: args.error,
+      });
       // @ts-expect-error this is binding added though CDP
       globalThis[bindingName](String(freestyler.id));
       freestyler.id++;
@@ -90,7 +95,7 @@
 
 export const freestylerBinding = `(${String(freestylerBindingFunc)})('${FREESTYLER_BINDING_NAME}')`;
 
-export const pageExposedFunctions = ['setElementStyles'];
+export const PAGE_EXPOSED_FUNCTIONS = ['setElementStyles'];
 
 /**
  * Please see fileoverview
@@ -141,8 +146,14 @@
 
     const bindingError = new Error();
 
-    const result = await global.freestyler(
-        {method: 'setElementStyles', selector, className, styles, element: el, error: bindingError});
+    const result = await global.freestyler({
+      method: 'setElementStyles',
+      selector,
+      className,
+      styles,
+      element: el,
+      error: bindingError,
+    });
 
     const rootNode = el.getRootNode();
     if (rootNode instanceof ShadowRoot) {
```

Change information

Commit message:

Adds failing line to the error for JS evaluations executed by CSS Agent

Fixed: 389034694

Change-Id: Ic302adae2e149f2263a692d4ebe70eef5bf1cf07

Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7079119

Reviewed-by: Alex Rudenko <alexr...@chromium.org>

Reviewed-by: Nikolay Vitkov <nvi...@chromium.org>

Commit-Queue: Nicholas Roscino <nros...@chromium.org>

Files:

M front_end/core/sdk/RemoteObject.test.ts
M front_end/core/sdk/RemoteObject.ts
M front_end/models/ai_assistance/EvaluateAction.test.ts
M front_end/models/ai_assistance/EvaluateAction.ts
M front_end/models/ai_assistance/injected.ts
M test/e2e_non_hosted/ai_assistance/ai_assistance_test.ts

Change size: L

Delta: 6 files changed, 352 insertions(+), 89 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Nikolay Vitkov, +1 by Alex Rudenko

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Adds failing line to the error for JS evaluations executed by CSS Agent [devtools/devtools-frontend : main]

Nikolay Vitkov (Gerrit)

Nikolay Vitkov added 1 comment

Related details

Nicholas Roscino (Gerrit)

Nicholas Roscino voted and added 1 comment

Votes added by Nicholas Roscino

1 comment

Related details

Nicholas Roscino (Gerrit)

Nicholas Roscino voted

Related details

Alex Rudenko (Gerrit)

Alex Rudenko added 1 comment

Related details

Nicholas Roscino (Gerrit)

Nicholas Roscino voted and added 1 comment

Votes added by Nicholas Roscino

1 comment

Related details

Alex Rudenko (Gerrit)

Alex Rudenko voted and added 2 comments

Votes added by Alex Rudenko

2 comments

Related details

Nicholas Roscino (Gerrit)

Nicholas Roscino added 1 comment

Related details

Nicholas Roscino (Gerrit)

Nicholas Roscino voted Commit-Queue+2

Related details

Nikolay Vitkov (Gerrit)

Nikolay Vitkov voted and added 10 comments

Votes added by Nikolay Vitkov

10 comments

Related details

Nicholas Roscino (Gerrit)

Nicholas Roscino added 9 comments

Related details

Nicholas Roscino (Gerrit)

Nicholas Roscino voted Commit-Queue+2

Related details

Devtools-frontend LUCI CQ (Gerrit)

Devtools-frontend LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information