Add snapshot options, browser_dom_query, and unit test coverage - #4
Conversation
- extractPageSnapshot accepts maxDepth/maxChildren/selector; snapshot tools expose them - PageSnapshotNode uses attributes map + hiddenChildCount/hiddenAttrCount instead of role/name - New browser_dom_query returns element presence, count, and state without waiting - buildNode unit-tested by calling the page.evaluate callback in Node.js with a stubbed document
|
@etairl Happy to make any changes needed or remove anything that isn't wanted. |
There was a problem hiding this comment.
The big one: removing .first() from generateLocatorCandidates is a regression, not an upgrade. Playwright's Locator.evaluate runs in strict mode and throws on multi-match — for "all elements" you'd need evaluateAll. The unit test passes because the mock doesn't enforce strict mode, but browser_generate_locator exists specifically to disambiguate broad selectors like button or .btn, which will now throw instead of picking the first match. Please restore .first().
Otherwise: top-level node truncation when no selector is set silently drops children without reporting hiddenChildCount (worth fixing while you're in there), and extractPageSnapshot does await evaluate() then await page.title() sequentially — Promise.all them. The buildNode test infra and the attribute-priority/cap work are great; happy to merge once the .first() thing is sorted.
- Capture url inside page.evaluate so it's atomic with title/tree - Hard cap total nodes at 500 with hiddenByNodeCap reporting - Replace hiddenChildCount with hiddenByDepth and hiddenByCount so formatted output tells the caller which param to increase
|
Addressed the .first() regression and tried to look for other concerns in the changes. |
etairl
left a comment
There was a problem hiding this comment.
Nailed it — and the URL race catch plus the 500-node cap and the split hidden-reason hints are real improvements I didn't ask for. Merging.
Summary
extractPageSnapshotnow acceptsmaxDepth,maxChildren, andselectoroptions;browser_snapshotandbrowser_get_page_structureexpose these as optional inputs viasnapshotSchemaPageSnapshotNodereplaces therole/namefields with a genericattributesmap;hiddenChildCountandhiddenAttrCountsignal truncation to callersformatNode(used byformatPageStructure) updated accordingly: attributes are now rendered askey="value"for every entry in the map, with a trailing[…N more attrs]token when the cap is hit and a[…N more children]line appended when child nodes were omitted — previously onlyroleandnamewere surfacedgenerateLocatorCandidatesremoves the.first()call so the evaluate callback runs against all matching elements rather than only the firstbrowser_dom_querytool returns element presence, count, and state (visible,enabled,checked,value) without waiting — useful for conditional logic without implicit timeoutsbuildNodeis now unit-tested by having thepage.evaluatemock call the serialised callback directly in Node.js with avi.stubGlobaldocument, covering SKIP_TAGS filtering, depth/child capping, attribute capping and priority ordering, text truncation, whitespace normalisation,innerTextfallback, and selector scopingTest plan
All 41 tests pass (
pnpm test). Manual testing ofbrowser_snapshotparams andbrowser_dom_querybehaviour confirmed locally.