Skip to content

fix: stop returning snapshot filter errors to the model as page content - #174

Open
karaposu wants to merge 1 commit into
brightdata:mainfrom
karaposu:fix/snapshot-filter-fallback
Open

fix: stop returning snapshot filter errors to the model as page content#174
karaposu wants to merge 1 commit into
brightdata:mainfrom
karaposu:fix/snapshot-filter-fallback

Conversation

@karaposu

Copy link
Copy Markdown

Problem

When the filtered ARIA snapshot path fails to parse, Aria_snapshot_filter.filter_snapshot's catch
returns Error filtering snapshot: <message>\n<stack> in the same slot as the page's interactive
elements. The snapshot tool prints that string verbatim under its Interactive Elements: header — so
the model receives a JavaScript stack trace presented as the page, with no signal it's an error, and
reasons about garbage. The realistic trigger is Playwright's ariaSnapshot returning a non-string
shape (the same API drift #158/#160 dealt with).

Fix (two changes, atomic by design)

  • filter_snapshot no longer catches: a parse failure throws. The
    'No interactive elements found' empty case stays — that is a value, not an error.
  • capture_snapshot wraps only the filter call. On a throw it logs the full stack to stderr and
    returns the unfiltered snapshot — real content already in hand — prefixed with a model-visible
    [note: snapshot filtering failed (…)] line, in the same result shape as filtered:false, so the
    tool renders it with no changes. Refs from the full snapshot still resolve through ref_locator's
    aria-ref branch, so the click/type workflow keeps working after a fallback. A non-string snapshot
    shows (no snapshot text available) instead of the word undefined.

Why this matters for the filtered:true default

This also makes flipping the snapshot tool's filtered default to true (as #163's bb37007 does)
safe: with the default on, every snapshot rides the filter path, and a filter bug would otherwise sit
on the main road. Merge note: bb37007 edits the same capture_snapshot function (DOM-ref
renumbering) — the changes are logically independent.

Tests

test/aria-snapshot-filter.test.js: the parser/format matrix (verified against the real parser
before being written), the throw case (fails on the pre-fix code, which returned the error string),
and a fallback wiring test using an injected fake page — asserting note + full content, no DOM pass,
and an intact result shape.

When the filtered ARIA snapshot path failed to parse, filter_snapshot's
catch returned "Error filtering snapshot: <message>\n<stack>" in the same
slot as the page's interactive elements. The snapshot tool prints that
string verbatim under an "Interactive Elements:" header, so the model
received a JavaScript stack trace presented as the page, with no signal it
was an error. The realistic trigger is Playwright's ariaSnapshot returning
a non-string shape (the same API drift brightdata#158/brightdata#160 dealt with).

Two changes, atomic by design:

- filter_snapshot no longer catches: a parse failure throws. The
  "No interactive elements found" empty case stays -- that is a value (a
  parsed page with nothing interactive), not an error.
- capture_snapshot wraps only the filter call. On a throw it logs the full
  stack to stderr and returns the unfiltered snapshot -- real content
  already in hand -- prefixed with a model-visible
  "[note: snapshot filtering failed (...)]" line, in the same result shape
  as filtered:false, so the tool renders it with no changes. The DOM pass
  is skipped on the degraded path; refs from the full snapshot still
  resolve through ref_locator's aria-ref branch, so the click/type
  workflow keeps working after a fallback. A non-string snapshot shows
  "(no snapshot text available)" instead of the word "undefined".

This also makes flipping the tool's filtered default to true (as PR brightdata#163's
bb37007 does on dev) safe: with the default on, every snapshot rides the
filter path, and a filter bug would otherwise sit on the main road. Note
for merge ordering: bb37007 edits the same capture_snapshot function
(DOM-ref renumbering); the changes are logically independent.

Adds test/aria-snapshot-filter.test.js: the parser/format matrix (verified
against the real parser before the fix), the throw case (fails on the
pre-fix code, which returned the error string), and a fallback wiring test
using an injected fake page -- asserting note + full content, no DOM pass,
and an intact result shape.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant