Revert "feat(aria): only include changed chunks into snapshot diff (microsoft#37988)"

This reverts commit 7249e20.

Author: agg23

packages/injected/src/ariaSnapshot.ts

@@ -519,83 +519,42 @@ function buildByRefMap(root: AriaNode | undefined, map: Map<string | undefined,
 
 function compareSnapshots(ariaSnapshot: AriaSnapshot, previousSnapshot: AriaSnapshot | undefined): Map<AriaNode, 'skip' | 'same' | 'changed'> {
   const previousByRef = buildByRefMap(previousSnapshot?.root);
-  const result = new Map<AriaNode, 'skip' | 'same' | 'changed'>();
+  const result = new Map<AriaNode, 'same' | 'changed'>();
 
   // Returns whether ariaNode is the same as previousNode.
   const visit = (ariaNode: AriaNode, previousNode: AriaNode | undefined): boolean => {
     let same: boolean = ariaNode.children.length === previousNode?.children.length && ariaNodesEqual(ariaNode, previousNode);
     if (ariaNode.role === 'iframe')
       same = false;
-    let canBeSkipped = same;
 
     for (let childIndex = 0 ; childIndex < ariaNode.children.length; childIndex++) {
       const child = ariaNode.children[childIndex];
       const previousChild = previousNode?.children[childIndex];
       if (typeof child === 'string') {
         same &&= child === previousChild;
-        canBeSkipped &&= child === previousChild;
       } else {
         let previous = typeof previousChild !== 'string' ? previousChild : undefined;
         if (child.ref)
           previous = previousByRef.get(child.ref);
         const sameChild = visit(child, previous);
-        // New child, different order of children, or changed child with no ref -
-        // we have to include this node to list children in the right order.
-        if (!previous || (!sameChild && !child.ref) || (previous !== previousChild))
-          canBeSkipped = false;
         same &&= (sameChild && previous === previousChild);
       }
     }
 
-    result.set(ariaNode, same ? 'same' : (canBeSkipped ? 'skip' : 'changed'));
+    result.set(ariaNode, same ? 'same' : 'changed');
     return same;
   };
 
   visit(ariaSnapshot.root, previousByRef.get(previousSnapshot?.root?.ref));
   return result;
 }
 
-// Chooses only the changed parts of the snapshot and returns them as new roots.
-function filterSnapshotDiff(nodes: (AriaNode | string)[], statusMap: Map<AriaNode, 'skip' | 'same' | 'changed'>): (AriaNode | string)[] {
-  const result: (AriaNode | string)[] = [];
-
-  const visit = (ariaNode: AriaNode) => {
-    const status = statusMap.get(ariaNode);
-    if (status === 'same') {
-      // No need to render unchanged root at all.
-    } else if (status === 'skip') {
-      // Only render changed children.
-      for (const child of ariaNode.children) {
-        if (typeof child !== 'string')
-          visit(child);
-      }
-    } else {
-      // Render this node's subtree.
-      result.push(ariaNode);
-    }
-  };
-
-  for (const node of nodes) {
-    if (typeof node === 'string')
-      result.push(node);
-    else
-      visit(node);
-  }
-  return result;
-}
-
 export function renderAriaTree(ariaSnapshot: AriaSnapshot, publicOptions: AriaTreeOptions, previousSnapshot?: AriaSnapshot): string {
   const options = toInternalOptions(publicOptions);
   const lines: string[] = [];
   const includeText = options.renderStringsAsRegex ? textContributesInfo : () => true;
   const renderString = options.renderStringsAsRegex ? convertToBestGuessRegex : (str: string) => str;
-
-  // Do not render the root fragment, just its children.
-  let nodesToRender = ariaSnapshot.root.role === 'fragment' ? ariaSnapshot.root.children : [ariaSnapshot.root];
-
   const statusMap = compareSnapshots(ariaSnapshot, previousSnapshot);
-  if (previousSnapshot)
-    nodesToRender = filterSnapshotDiff(nodesToRender, statusMap);
 
   const visitText = (text: string, indent: string) => {
     const escaped = yamlEscapeValueIfNeeded(renderString(text));
@@ -645,15 +604,15 @@ export function renderAriaTree(ariaSnapshot: AriaSnapshot, publicOptions: AriaTr
   };
 
   const visit = (ariaNode: AriaNode, indent: string, renderCursorPointer: boolean) => {
+    const status = statusMap.get(ariaNode);
+
     // Replace the whole subtree with a single reference when possible.
-    if (statusMap.get(ariaNode) === 'same' && ariaNode.ref) {
+    if (status === 'same' && ariaNode.ref) {
       lines.push(indent + `- ref=${ariaNode.ref} [unchanged]`);
       return;
     }
 
-    // When producing a diff, add <changed> marker to all diff roots.
-    const isDiffRoot = !!previousSnapshot && !indent;
-    const escapedKey = indent + '- ' + (isDiffRoot ? '<changed> ' : '') + yamlEscapeKeyIfNeeded(createKey(ariaNode, renderCursorPointer));
+    const escapedKey = indent + '- ' + yamlEscapeKeyIfNeeded(createKey(ariaNode, renderCursorPointer));
     const singleInlinedTextChild = getSingleInlinedTextChild(ariaNode);
 
     if (!ariaNode.children.length && !Object.keys(ariaNode.props).length) {
@@ -671,18 +630,22 @@ export function renderAriaTree(ariaSnapshot: AriaSnapshot, publicOptions: AriaTr
       lines.push(escapedKey + ':');
       for (const [name, value] of Object.entries(ariaNode.props))
         lines.push(indent + '  - /' + name + ': ' + yamlEscapeValueIfNeeded(value));
+    }
 
-      const childIndent = indent + '  ';
+    indent += '  ';
+    if (singleInlinedTextChild === undefined) {
       const inCursorPointer = !!ariaNode.ref && renderCursorPointer && hasPointerCursor(ariaNode);
       for (const child of ariaNode.children) {
         if (typeof child === 'string')
-          visitText(includeText(ariaNode, child) ? child : '', childIndent);
+          visitText(includeText(ariaNode, child) ? child : '', indent);
         else
-          visit(child, childIndent, renderCursorPointer && !inCursorPointer);
+          visit(child, indent, renderCursorPointer && !inCursorPointer);
       }
     }
   };
 
+  // Do not render the root fragment, just its children.
+  const nodesToRender = ariaSnapshot.root.role === 'fragment' ? ariaSnapshot.root.children : [ariaSnapshot.root];
   for (const nodeToRender of nodesToRender) {
     if (typeof nodeToRender === 'string')
       visitText(nodeToRender, '');

packages/playwright-core/src/server/page.ts

@@ -1046,7 +1046,7 @@ async function snapshotFrameForAI(progress: Progress, frame: frames.Frame, optio
         if (!node)
           return true;
         return injected.ariaSnapshot(node, { mode: 'ai', ...options });
-      }, { refPrefix: frame.seq ? 'f' + frame.seq : '', incremental: options.mode === 'incremental' && !frame.parentFrame(), track: options.track }));
+      }, { refPrefix: frame.seq ? 'f' + frame.seq : '', incremental: options.mode === 'incremental', track: options.track }));
       if (snapshotOrRetry === true)
         return continuePolling;
       return snapshotOrRetry;
@@ -1060,7 +1060,7 @@ async function snapshotFrameForAI(progress: Progress, frame: frames.Frame, optio
   const lines = snapshot.split('\n');
   const result = [];
   for (const line of lines) {
-    const match = line.match(/^(\s*)-(?: <changed>)? iframe (?:\[active\] )?\[ref=([^\]]*)\]/);
+    const match = line.match(/^(\s*)- iframe (?:\[active\] )?\[ref=([^\]]*)\]/);
     if (!match) {
       result.push(line);
       continue;

packages/playwright/src/mcp/browser/response.ts

@@ -192,8 +192,7 @@ function renderTabSnapshot(tabSnapshot: TabSnapshot, options: { omitSnapshot?: b
   lines.push(`- Page Title: ${tabSnapshot.title}`);
   lines.push(`- Page Snapshot:`);
   lines.push('```yaml');
-  // TODO: perhaps not render page state when there are no changes?
-  lines.push(options.omitSnapshot ? '<snapshot>' : (tabSnapshot.ariaSnapshot || '<no changes>'));
+  lines.push(options.omitSnapshot ? '<snapshot>' : tabSnapshot.ariaSnapshot);
   lines.push('```');
 
   return lines.join('\n');

tests/mcp/click.spec.ts

@@ -41,7 +41,7 @@ test('browser_click', async ({ client, server }) => {
     },
   })).toHaveResponse({
     code: `await page.getByRole('button', { name: 'Submit' }).click();`,
-    pageState: expect.stringContaining(`button "Submit" [active] [ref=e2]`),
+    pageState: expect.stringContaining(`- button "Submit" [active] [ref=e2]`),
   });
 });
 
@@ -70,7 +70,7 @@ test('browser_click (double)', async ({ client, server }) => {
     },
   })).toHaveResponse({
     code: `await page.getByRole('heading', { name: 'Click me' }).dblclick();`,
-    pageState: expect.stringContaining(`heading "Double clicked" [level=1] [ref=e3]`),
+    pageState: expect.stringContaining(`- heading "Double clicked" [level=1] [ref=e3]`),
   });
 });
 
@@ -100,7 +100,7 @@ test('browser_click (right)', async ({ client, server }) => {
   });
   expect(result).toHaveResponse({
     code: `await page.getByRole('button', { name: 'Menu' }).click({ button: 'right' });`,
-    pageState: expect.stringContaining(`button "Right clicked"`),
+    pageState: expect.stringContaining(`- button "Right clicked"`),
   });
 });
 
@@ -131,7 +131,7 @@ test('browser_click (modifiers)', async ({ client, server, mcpBrowser }) => {
       },
     })).toHaveResponse({
       code: `await page.getByRole('button', { name: 'Submit' }).click({ modifiers: ['Control'] });`,
-      pageState: expect.stringContaining(`generic [ref=e3]: ctrlKey:true metaKey:false shiftKey:false altKey:false`),
+      pageState: expect.stringContaining(`- generic [ref=e3]: ctrlKey:true metaKey:false shiftKey:false altKey:false`),
     });
   }
 
@@ -144,7 +144,7 @@ test('browser_click (modifiers)', async ({ client, server, mcpBrowser }) => {
     },
   })).toHaveResponse({
     code: `await page.getByRole('button', { name: 'Submit' }).click({ modifiers: ['Shift'] });`,
-    pageState: expect.stringContaining(`generic [ref=e3]: ctrlKey:false metaKey:false shiftKey:true altKey:false`),
+    pageState: expect.stringContaining(`- generic [ref=e3]: ctrlKey:false metaKey:false shiftKey:true altKey:false`),
   });
 
   expect(await client.callTool({
@@ -156,7 +156,7 @@ test('browser_click (modifiers)', async ({ client, server, mcpBrowser }) => {
     },
   })).toHaveResponse({
     code: `await page.getByRole('button', { name: 'Submit' }).click({ modifiers: ['Shift', 'Alt'] });`,
-    pageState: expect.stringContaining(`generic [ref=e3]: ctrlKey:false metaKey:false shiftKey:true altKey:true`),
+    pageState: expect.stringContaining(`- generic [ref=e3]: ctrlKey:false metaKey:false shiftKey:true altKey:true`),
   });
 });
 

tests/mcp/core.spec.ts

@@ -58,7 +58,7 @@ test('browser_select_option', async ({ client, server }) => {
 - Page Title: Title
 - Page Snapshot:
 \`\`\`yaml
-- <changed> combobox [ref=e2]:
+- combobox [ref=e2]:
   - option "Foo"
   - option "Bar" [selected]
 \`\`\``,
@@ -90,8 +90,8 @@ test('browser_select_option (multiple)', async ({ client, server }) => {
   })).toHaveResponse({
     code: `await page.getByRole('listbox').selectOption(['bar', 'baz']);`,
     pageState: expect.stringContaining(`
-- <changed> option "Bar" [selected] [ref=e4]
-- <changed> option "Baz" [selected] [ref=e5]`),
+  - option "Bar" [selected] [ref=e4]
+  - option "Baz" [selected] [ref=e5]`),
   });
 });
 

tests/mcp/dialogs.spec.ts

@@ -139,7 +139,7 @@ test('confirm dialog (true)', async ({ client, server }) => {
     },
   })).toHaveResponse({
     modalState: undefined,
-    pageState: expect.stringContaining(`generic [active] [ref=e1]: "true"`),
+    pageState: expect.stringContaining(`- generic [active] [ref=e1]: "true"`),
   });
 });
 
@@ -175,7 +175,7 @@ test('confirm dialog (false)', async ({ client, server }) => {
     },
   })).toHaveResponse({
     modalState: undefined,
-    pageState: expect.stringContaining(`generic [active] [ref=e1]: "false"`),
+    pageState: expect.stringContaining(`- generic [active] [ref=e1]: "false"`),
   });
 });
 
@@ -213,7 +213,7 @@ test('prompt dialog', async ({ client, server }) => {
   });
 
   expect(result).toHaveResponse({
-    pageState: expect.stringContaining(`generic [active] [ref=e1]: Answer`),
+    pageState: expect.stringContaining(`- generic [active] [ref=e1]: Answer`),
   });
 });
 

tests/mcp/secrets.spec.ts

@@ -53,7 +53,7 @@ test('browser_type', async ({ startClient, server }) => {
     expect(response).toHaveResponse({
       code: `await page.getByRole('textbox').fill(process.env['X-PASSWORD']);
 await page.getByRole('textbox').press('Enter');`,
-      pageState: expect.stringMatching(/textbox (\[active\] )?\[ref=e2\]: <secret>X-PASSWORD<\/secret>/),
+      pageState: expect.stringContaining(`- textbox`),
     });
   }
 

tests/mcp/snapshot-diff.spec.ts

@@ -65,7 +65,7 @@ test('should return aria snapshot diff', async ({ client, server }) => {
   })).toHaveResponse({
     pageState: expect.stringContaining(`Page Snapshot:
 \`\`\`yaml
-<no changes>
+- ref=e1 [unchanged]
 \`\`\``),
   });
 
@@ -78,7 +78,7 @@ test('should return aria snapshot diff', async ({ client, server }) => {
   })).toHaveResponse({
     pageState: expect.stringContaining(`Page Snapshot:
 \`\`\`yaml
-- <changed> generic [ref=e1]:
+- generic [ref=e1]:
   - button "Button 1" [active] [ref=e2]
   - button "Button 2new text" [ref=e105]
   - ref=e4 [unchanged]

tests/mcp/type.spec.ts

@@ -44,7 +44,7 @@ test('browser_type', async ({ client, server }) => {
     expect(response).toHaveResponse({
       code: `await page.getByRole('textbox').fill('Hi!');
 await page.getByRole('textbox').press('Enter');`,
-      pageState: expect.stringMatching(/textbox (\[active\] )?\[ref=e2\]: Hi!/),
+      pageState: expect.stringContaining(`- textbox`),
     });
   }
 
@@ -79,7 +79,7 @@ test('browser_type (slowly)', async ({ client, server }) => {
 
     expect(response).toHaveResponse({
       code: `await page.getByRole('textbox').pressSequentially('Hi!');`,
-      pageState: expect.stringMatching(/textbox (\[active\] )?\[ref=e2\]: Hi!/),
+      pageState: expect.stringContaining(`- textbox`),
     });
   }
   const response = await client.callTool({