Skip to content

Commit 558dda6

Browse files
committed
fix(mcp): print the path shared by browser_find matches once
browser_find rendered every context window as its own snippet and recomputed the ancestor path for each one, so the path from the root was re-emitted once per match. With several matches under a common deep ancestor this made find produce more output than the full snapshot it is meant to replace: on a 6-card job board fixture browser_snapshot was 59 lines and browser_find was 84. Collect the ancestor paths and context windows of all matches into one sorted set of line indices and render them as a single tree. The shared path is printed once and the existing ellipsis rule marks the gap between windows. Same fixture is now 54 lines. The ---- separator between snippets is gone, there is only one tree. Fixes: #42077
1 parent 15b1aec commit 558dda6

2 files changed

Lines changed: 63 additions & 13 deletions

File tree

packages/playwright-core/src/tools/backend/find.ts

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,26 @@ const find = defineTabTool({
9292
path.add(ancestor);
9393
}
9494

95-
const snippets = windows.map(window => {
96-
const indices = ancestorIndices(lines, indents, window.start);
95+
// Render all the windows into a single tree, so that a path shared by several matches is
96+
// printed once instead of being repeated for every one of them.
97+
const included = new Set<number>();
98+
for (const window of windows) {
99+
for (const ancestor of ancestorIndices(lines, indents, window.start))
100+
included.add(ancestor);
97101
for (let i = window.start; i <= window.end; i++)
98-
indices.push(i);
99-
const out: string[] = [];
100-
for (let i = 0; i < indices.length; i++) {
101-
const index = indices[i];
102-
if (i > 0 && index > indices[i - 1] + 1 && !path.has(index) && !path.has(indices[i - 1]))
103-
out.push(' '.repeat(indents[index]) + '...');
104-
out.push(lines[index]);
105-
}
106-
return out.join('\n');
107-
});
102+
included.add(i);
103+
}
104+
105+
const indices = [...included].sort((a, b) => a - b);
106+
const out: string[] = [];
107+
for (let i = 0; i < indices.length; i++) {
108+
const index = indices[i];
109+
if (i > 0 && index > indices[i - 1] + 1 && !path.has(index) && !path.has(indices[i - 1]))
110+
out.push(' '.repeat(indents[index]) + '...');
111+
out.push(lines[index]);
112+
}
108113
const matchWord = matchedLines.length === 1 ? 'match' : 'matches';
109-
response.addTextResult(`Found ${matchedLines.length} ${matchWord} for ${query}:\n\n${snippets.join('\n\n----\n\n')}`);
114+
response.addTextResult(`Found ${matchedLines.length} ${matchWord} for ${query}:\n\n${out.join('\n')}`);
110115
},
111116
});
112117

tests/mcp/find.spec.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,51 @@ test('browser_find marks gaps within off-path context with an ellipsis', async (
132132
});
133133
});
134134

135+
const repeatedPage = `
136+
<main>
137+
<section aria-label="Sidebar">
138+
<nav aria-label="Primary">
139+
<ul>
140+
<li><a href="/a">Target A</a></li>
141+
<li>filler one</li>
142+
<li>filler two</li>
143+
<li>filler three</li>
144+
<li>filler four</li>
145+
<li>filler five</li>
146+
<li><a href="/b">Target B</a></li>
147+
</ul>
148+
</nav>
149+
</section>
150+
</main>
151+
`;
152+
153+
test('browser_find prints the path shared by several matches once', async ({ client, server }) => {
154+
server.setContent('/', repeatedPage, 'text/html');
155+
await client.callTool({ name: 'browser_navigate', arguments: { url: server.PREFIX } });
156+
157+
const response = await client.callTool({
158+
name: 'browser_find',
159+
arguments: { text: 'Target' },
160+
});
161+
162+
expect(response).toHaveResponse({
163+
result: expect.stringContaining(`Found 2 matches for "Target":
164+
165+
- main [ref=e2]:
166+
- region "Sidebar" [ref=e3]:
167+
- navigation "Primary" [ref=e4]:
168+
- list [ref=e5]:
169+
- listitem [ref=e6]:`),
170+
});
171+
// Both matches live in the same tree, so the path above them is printed once and the gap
172+
// between the two context windows is marked with an ellipsis.
173+
expect(response).toHaveResponse({
174+
result: expect.stringContaining(` - listitem [ref=e9]: filler two
175+
...
176+
- listitem [ref=e11]: filler four`),
177+
});
178+
});
179+
135180
test('browser_find is case-insensitive for text', async ({ client, server }) => {
136181
server.setContent('/', listPage, 'text/html');
137182
await client.callTool({ name: 'browser_navigate', arguments: { url: server.PREFIX } });

0 commit comments

Comments
 (0)