diff --git a/browsergym/core/src/browsergym/utils/obs.py b/browsergym/core/src/browsergym/utils/obs.py index db5b8ce03..dc02349cb 100644 --- a/browsergym/core/src/browsergym/utils/obs.py +++ b/browsergym/core/src/browsergym/utils/obs.py @@ -41,7 +41,7 @@ def flatten_dom_to_str( filter_with_bid_only: bool = False, filter_som_only: bool = False, coord_decimals: int = 0, - hide_bid_if_invisible: int = False, + hide_bid_if_invisible: bool = False, hide_all_bids: bool = False, ) -> str: """Formats a DOM snapshot into a string text""" @@ -131,18 +131,23 @@ def dfs(node_idx: int, parent_node_skipped: bool) -> str: filter_with_bid_only=filter_with_bid_only, filter_som_only=filter_som_only, coord_decimals=coord_decimals, + hide_bid_if_invisible=hide_bid_if_invisible, ) # insert extra attributes before regular attributes attributes = extra_attributes_to_print + attributes + # a bid with no recorded visibility is kept, the way _process_bid keeps it + bid_visibility = (extra_properties or {}).get(bid, {}).get("visibility") + # insert bid as first attribute if not ( hide_all_bids or bid is None or ( hide_bid_if_invisible - and extra_properties.get(bid, {}).get("visibility", 0) < 0.5 + and bid_visibility is not None + and bid_visibility < 0.5 ) ): attributes.insert(0, f'bid="{bid}"') @@ -205,6 +210,7 @@ def _process_bid( filter_with_bid_only: bool = False, filter_som_only: bool = False, coord_decimals: int = 0, + hide_bid_if_invisible: bool = False, ): """ Process extra attributes and attribute-based filters, for the element with the given bid. @@ -225,6 +231,7 @@ def _process_bid( filter_visible_only, filter_with_bid_only, filter_som_only, + hide_bid_if_invisible, ) ): raise ValueError("extra_properties argument required") @@ -371,6 +378,7 @@ def dfs(node_idx: int, depth: int, parent_node_filtered: bool, parent_node_name: filter_with_bid_only=filter_with_bid_only, filter_som_only=filter_som_only, coord_decimals=coord_decimals, + hide_bid_if_invisible=hide_bid_if_invisible, ) # if either is True, skip the node @@ -386,12 +394,16 @@ def dfs(node_idx: int, depth: int, parent_node_filtered: bool, parent_node_name: else: node_str = f"{node_role} {repr(node_name.strip())}" + # a bid with no recorded visibility is kept, the way _process_bid keeps it + bid_visibility = (extra_properties or {}).get(bid, {}).get("visibility") + if not ( hide_all_bids or bid is None or ( hide_bid_if_invisible - and extra_properties.get(bid, {}).get("visibility", 0) < 0.5 + and bid_visibility is not None + and bid_visibility < 0.5 ) ): node_str = f"[{bid}] " + node_str diff --git a/tests/core/test_obs_bid_visibility.py b/tests/core/test_obs_bid_visibility.py new file mode 100644 index 000000000..bbcc427a2 --- /dev/null +++ b/tests/core/test_obs_bid_visibility.py @@ -0,0 +1,75 @@ +"""Tests for how the observation formatters treat the hide_bid_if_invisible flag. + +These run on hand-built snapshots rather than a live page, so they need no browser. +""" + +import pytest + +from browsergym.core.constants import BROWSERGYM_ID_ATTRIBUTE as BID_ATTR +from browsergym.utils.obs import flatten_axtree_to_str, flatten_dom_to_str + + +def _axtree(bid: str) -> dict: + return { + "nodes": [ + { + "nodeId": "1", + "role": {"value": "RootWebArea"}, + "name": {"value": "a page"}, + "childIds": ["2"], + }, + { + "nodeId": "2", + "role": {"value": "button"}, + "name": {"value": "Submit"}, + "childIds": [], + "browsergym_id": bid, + }, + ] + } + + +def _dom_snapshot(bid: str) -> dict: + strings = ["#document", "html", "body", "button", BID_ATTR, bid] + return { + "strings": strings, + "documents": [ + { + "nodes": { + "nodeType": [9, 1, 1, 1], + "nodeName": [0, 1, 2, 3], + "nodeValue": [-1, -1, -1, -1], + "parentIndex": [-1, 0, 1, 2], + "attributes": [[], [], [], [4, 5]], + "contentDocumentIndex": {"index": [], "value": []}, + }, + } + ], + } + + +@pytest.mark.parametrize( + ("flatten", "build"), + [(flatten_axtree_to_str, _axtree), (flatten_dom_to_str, _dom_snapshot)], + ids=["axtree", "dom"], +) +def test_hide_bid_if_invisible_without_extra_properties_is_an_error(flatten, build): + """The flag cannot be honoured without visibility data, so say so, as the other flags do.""" + with pytest.raises(ValueError, match="extra_properties argument required"): + flatten(build("42"), hide_bid_if_invisible=True) + + +@pytest.mark.parametrize( + ("flatten", "build"), + [(flatten_axtree_to_str, _axtree), (flatten_dom_to_str, _dom_snapshot)], + ids=["axtree", "dom"], +) +def test_hide_bid_if_invisible_hides_only_a_recorded_invisible_bid(flatten, build): + """A bid with no recorded visibility stays, matching how _process_bid treats one.""" + visible = {"42": {"visibility": 1.0, "bbox": None, "clickable": False, "set_of_marks": False}} + invisible = {"42": {"visibility": 0.0, "bbox": None, "clickable": False, "set_of_marks": False}} + + assert "42" in flatten(build("42"), extra_properties=visible, hide_bid_if_invisible=True) + assert "42" not in flatten(build("42"), extra_properties=invisible, hide_bid_if_invisible=True) + # no entry at all: nothing says it is invisible, so the bid is kept + assert "42" in flatten(build("42"), extra_properties={}, hide_bid_if_invisible=True)