fix(mcp): print the path shared by browser_find matches once - #42127
fix(mcp): print the path shared by browser_find matches once#42127Ishaan (guptaishaan) wants to merge 1 commit into
Conversation
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: microsoft#42077
There was a problem hiding this comment.
🟢 Ready to approve
The change is localized, matches the stated intent, and is covered by a new regression test plus existing find-related tests.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR reduces browser_find output verbosity in the MCP backend by rendering all match context windows as a single combined tree, so shared ancestor paths are printed once (instead of once per snippet), aligning find output closer to being a cheaper alternative to full snapshots.
Changes:
- Merge all match windows’ line indices (including required ancestor lines) into one sorted set and render a single tree, removing per-snippet repetition and the
----separator. - Preserve the existing ellipsis rule to mark gaps between non-path context windows within the combined output.
- Add a regression test ensuring a shared path for multiple matches is printed once and that the gap between windows is ellipsized.
File summaries
| File | Description |
|---|---|
packages/playwright-core/src/tools/backend/find.ts |
Combines multiple context windows into one rendered tree so shared ancestor paths aren’t repeated. |
tests/mcp/find.spec.ts |
Adds coverage for multiple matches sharing a deep ancestor path and verifies the combined output behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Test results for "MCP"1 failed 8062 passed, 1284 skipped Merge workflow run. |
Fixes #42077
browser_findrendered every context window as an independent snippet and recomputedancestorIndices()for each one, so the path from the root was re-emitted once per match. On a pagewhere several matches sit under a common deep ancestor, that made
findproduce more output thanthe full snapshot it is supposed to replace: on a 6-card job board fixture,
browser_snapshotwas59 lines and
browser_findwas 84.This collects the ancestor paths and context windows of all matches into one sorted set of line
indices and renders them as a single tree. A shared path is printed once, and the existing ellipsis
rule now marks the gap between windows. Same fixture after the change: 54 lines. The
----separator between snippets is gone, since there is only one tree now.
Verified on Ubuntu 20.04 x64, node 22.14, chromium r1237 and firefox 1539. New test in
tests/mcp/find.spec.tsfails before the change and passes after. All existing find and cli-findtests pass on chromium and firefox. I could not run webkit, its binaries do not install on this
host, so if webkit numbers refs differently the new test may need the same adjustment the existing
ref-asserting tests would.
Two things in the issue are deliberately not addressed here. The full ancestor chain for a single
match is unchanged, that is the behavior added in #41654 and it is a product decision rather than a
bug. There is still no
--max-resultsonfind. The issue's third point, no depth limit onsnapshot, is already fixed on main -browser_snapshothonorsdepthboth page-wide and relativeto a
targetroot.Thanks to David Condrey (@dcondrey) for the report.