From 11b564d3849507a617bebb9ffaacc84c774449b5 Mon Sep 17 00:00:00 2001 From: tharun_varshan_s Date: Fri, 9 Oct 2026 10:50:16 +0530 Subject: [PATCH 1/3] Fix program_html evaluator race condition and empty string handling (fixes #407) --- .../src/browsergym/webarenalite/evaluators.py | 15 ++- .../webarena/test_webarenalite_evaluators.py | 120 ++++++++++++++++++ 2 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 tests/webarena/test_webarenalite_evaluators.py diff --git a/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py b/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py index 12d4de191..3276ed8b1 100644 --- a/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py +++ b/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py @@ -264,7 +264,14 @@ def __call__( # navigate to that url if target_url != "last": page.goto(target_url) - time.sleep(3) # TODO [shuyanzh]: fix this hard-coded sleep + try: + # networkidle avoids evaluating while the page is still loading resources. + # We tolerate timeouts because some target websites have background + # activity that never truly reaches networkidle. In these cases, we + # fall through and attempt evaluation anyway. + page.wait_for_load_state("networkidle", timeout=3000) + except playwright.sync_api.TimeoutError: + pass # empty, use the full page if not locator.strip(): @@ -282,8 +289,8 @@ def __call__( if not selected_element: selected_element = "" except Exception: - # the page is wrong, return empty - selected_element = "" + # JavaScript evaluation failed; distinguish evaluator failure from a valid empty result. + selected_element = None elif locator.startswith("lambda:"): try: locator = locator.lstrip("lambda:") @@ -291,7 +298,7 @@ def __call__( if not selected_element: selected_element = None except Exception: - # the page is wrong, return empty + # JavaScript evaluation failed; distinguish evaluator failure from a valid empty result. selected_element = None # run program to call API elif locator.startswith("func:"): # a helper function diff --git a/tests/webarena/test_webarenalite_evaluators.py b/tests/webarena/test_webarenalite_evaluators.py new file mode 100644 index 000000000..a6cc318d0 --- /dev/null +++ b/tests/webarena/test_webarenalite_evaluators.py @@ -0,0 +1,120 @@ +import pytest +from unittest.mock import MagicMock, patch +import json +import tempfile +import playwright.sync_api +import importlib.util + +# Skip test if webarenalite is not installed or importable in this environment +webarenalite_installed = importlib.util.find_spec("browsergym.webarenalite") is not None +if webarenalite_installed: + from browsergym.webarenalite.evaluators import evaluator_router + + +@pytest.fixture +def mock_page(): + page = MagicMock() + page.url = "http://example.com" + return page + + +@pytest.mark.skipif(not webarenalite_installed, reason="browsergym.webarenalite not installed") +class TestWebArenaLiteEvaluators: + def _create_evaluator(self, config_dict): + with tempfile.NamedTemporaryFile("w+", delete=False) as f: + json.dump(config_dict, f) + filename = f.name + return evaluator_router(filename) + + def test_successful_locator_normal_scoring(self, mock_page): + # 1. Locator returns normal non-empty text. + config = { + "eval": { + "eval_types": ["program_html"], + "program_html": [ + { + "url": "last", + "locator": "document.querySelector('foo').innerText", + "required_contents": {"exact_match": "Hello World"}, + } + ], + } + } + mock_page.evaluate.return_value = "Hello World" + + evaluator = self._create_evaluator(config) + score = evaluator(trajectory=[{"action_type": "stop"}], config_file="", page=mock_page) + + assert score == 1.0 + mock_page.evaluate.assert_called_with("() => document.querySelector('foo').innerText") + + def test_legitimate_empty_value_normal_scoring(self, mock_page): + # 2. Locator returns a legitimate empty string. + config = { + "eval": { + "eval_types": ["program_html"], + "program_html": [ + { + "url": "last", + "locator": "document.querySelector('empty').innerText", + "required_contents": {"exact_match": ""}, + } + ], + } + } + mock_page.evaluate.return_value = "" + + evaluator = self._create_evaluator(config) + score = evaluator(trajectory=[{"action_type": "stop"}], config_file="", page=mock_page) + + assert score == 1.0 + + def test_page_evaluate_exception_evaluator_failure(self, mock_page): + # 4. page.evaluate() raises an exception (e.g. element missing). + config = { + "eval": { + "eval_types": ["program_html"], + "program_html": [ + { + "url": "last", + "locator": "document.querySelector('missing').innerText", + "required_contents": {"exact_match": ""}, + } + ], + } + } + mock_page.evaluate.side_effect = Exception("Element not found") + + evaluator = self._create_evaluator(config) + score = evaluator(trajectory=[{"action_type": "stop"}], config_file="", page=mock_page) + + # Missing element results in score 0.0 because it's caught and guarded as None + assert score == 0.0 + + def test_slow_navigation_waits_appropriately(self, mock_page): + # 5, 6, 8. Check that navigation waits for networkidle and tolerates timeouts. + config = { + "eval": { + "eval_types": ["program_html"], + "program_html": [ + { + "url": "http://example.com/target", + "locator": "document.querySelector('foo').innerText", + "required_contents": {"exact_match": "Hello World"}, + } + ], + } + } + + # Simulate wait_for_load_state throwing a TimeoutError + mock_page.wait_for_load_state.side_effect = playwright.sync_api.TimeoutError( + "Timeout exceeded" + ) + mock_page.evaluate.return_value = "Hello World" + + evaluator = self._create_evaluator(config) + score = evaluator(trajectory=[{"action_type": "stop"}], config_file="", page=mock_page) + + mock_page.goto.assert_called_with("http://example.com/target") + mock_page.wait_for_load_state.assert_called_with("networkidle", timeout=3000) + assert score == 1.0 # The timeout was tolerated and evaluation proceeded From 16ec4ef0a03bea16b4b9e7ec7894c7bcea606aac Mon Sep 17 00:00:00 2001 From: tharun_varshan_s Date: Sat, 17 Oct 2026 12:02:07 +0530 Subject: [PATCH 2/3] fix: import TimeoutError from playwright.sync_api in evaluators --- .../webarenalite/src/browsergym/webarenalite/evaluators.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py b/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py index 3276ed8b1..74e3dc753 100644 --- a/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py +++ b/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py @@ -15,7 +15,7 @@ from beartype import beartype from nltk.tokenize import word_tokenize # type: ignore -from playwright.sync_api import CDPSession, Page +from playwright.sync_api import CDPSession, Page, TimeoutError from .helper_functions import ( PseudoPage, gitlab_get_project_memeber_role, @@ -270,7 +270,7 @@ def __call__( # activity that never truly reaches networkidle. In these cases, we # fall through and attempt evaluation anyway. page.wait_for_load_state("networkidle", timeout=3000) - except playwright.sync_api.TimeoutError: + except TimeoutError: pass # empty, use the full page From 34a64afd63171e3b17ed74bbd80e73e8bd34f905 Mon Sep 17 00:00:00 2001 From: tharun_varshan_s Date: Tue, 20 Oct 2026 09:24:39 +0530 Subject: [PATCH 3/3] test: pass config_file to evaluator, mock Page with spec, and document evaluation provenance --- .../src/browsergym/webarenalite/evaluators.py | 14 ++- .../webarena/test_webarenalite_evaluators.py | 102 ++++++++++++++---- 2 files changed, 92 insertions(+), 24 deletions(-) diff --git a/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py b/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py index 74e3dc753..9bd229b49 100644 --- a/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py +++ b/browsergym/webarenalite/src/browsergym/webarenalite/evaluators.py @@ -230,9 +230,15 @@ def parse_urls( class HTMLContentEvaluator(Evaluator): - """Check whether the contents appear in the page""" - - """Check whether the contents appear in the page""" + """Check whether the contents appear in the page. + + Note on evaluation provenance / error handling: + When JavaScript locator evaluation raises an exception (e.g. element not found + or syntax error), `selected_element` is set to `None`, resulting in a score of 0.0. + The evaluator returns the same numeric score (0.0) for a locator exception as it does + for a genuine content mismatch. This limitation preserves standard benchmark scoring + semantics while distinguishing locator exceptions from legitimate empty string results. + """ @staticmethod @beartype @@ -290,6 +296,7 @@ def __call__( selected_element = "" except Exception: # JavaScript evaluation failed; distinguish evaluator failure from a valid empty result. + # Note: yields score 0.0, matching a content mismatch (evaluation-provenance limitation). selected_element = None elif locator.startswith("lambda:"): try: @@ -299,6 +306,7 @@ def __call__( selected_element = None except Exception: # JavaScript evaluation failed; distinguish evaluator failure from a valid empty result. + # Note: yields score 0.0, matching a content mismatch (evaluation-provenance limitation). selected_element = None # run program to call API elif locator.startswith("func:"): # a helper function diff --git a/tests/webarena/test_webarenalite_evaluators.py b/tests/webarena/test_webarenalite_evaluators.py index a6cc318d0..aea473f77 100644 --- a/tests/webarena/test_webarenalite_evaluators.py +++ b/tests/webarena/test_webarenalite_evaluators.py @@ -1,9 +1,16 @@ -import pytest -from unittest.mock import MagicMock, patch import json +import os +import pathlib import tempfile -import playwright.sync_api +from unittest.mock import MagicMock, patch + import importlib.util +import pytest +import playwright.sync_api + +# Set mock WebArena environment variables before importing webarenalite if not already present +for var in ["REDDIT", "SHOPPING", "SHOPPING_ADMIN", "GITLAB", "WIKIPEDIA", "MAP", "HOMEPAGE"]: + os.environ.setdefault(var, "http://localhost") # Skip test if webarenalite is not installed or importable in this environment webarenalite_installed = importlib.util.find_spec("browsergym.webarenalite") is not None @@ -13,20 +20,20 @@ @pytest.fixture def mock_page(): - page = MagicMock() + # Use spec=playwright.sync_api.Page to satisfy @beartype validation on EvaluatorComb.__call__ + page = MagicMock(spec=playwright.sync_api.Page) page.url = "http://example.com" return page @pytest.mark.skipif(not webarenalite_installed, reason="browsergym.webarenalite not installed") class TestWebArenaLiteEvaluators: - def _create_evaluator(self, config_dict): - with tempfile.NamedTemporaryFile("w+", delete=False) as f: - json.dump(config_dict, f) - filename = f.name - return evaluator_router(filename) + def _create_evaluator(self, config_dict, tmp_path): + config_file = tmp_path / "eval_config.json" + config_file.write_text(json.dumps(config_dict)) + return evaluator_router(str(config_file)), str(config_file) - def test_successful_locator_normal_scoring(self, mock_page): + def test_successful_locator_normal_scoring(self, mock_page, tmp_path): # 1. Locator returns normal non-empty text. config = { "eval": { @@ -42,13 +49,17 @@ def test_successful_locator_normal_scoring(self, mock_page): } mock_page.evaluate.return_value = "Hello World" - evaluator = self._create_evaluator(config) - score = evaluator(trajectory=[{"action_type": "stop"}], config_file="", page=mock_page) + evaluator, config_file = self._create_evaluator(config, tmp_path) + score = evaluator( + trajectory=[{"action_type": "stop"}], + config_file=config_file, + page=mock_page, + ) assert score == 1.0 mock_page.evaluate.assert_called_with("() => document.querySelector('foo').innerText") - def test_legitimate_empty_value_normal_scoring(self, mock_page): + def test_legitimate_empty_value_normal_scoring(self, mock_page, tmp_path): # 2. Locator returns a legitimate empty string. config = { "eval": { @@ -64,12 +75,16 @@ def test_legitimate_empty_value_normal_scoring(self, mock_page): } mock_page.evaluate.return_value = "" - evaluator = self._create_evaluator(config) - score = evaluator(trajectory=[{"action_type": "stop"}], config_file="", page=mock_page) + evaluator, config_file = self._create_evaluator(config, tmp_path) + score = evaluator( + trajectory=[{"action_type": "stop"}], + config_file=config_file, + page=mock_page, + ) assert score == 1.0 - def test_page_evaluate_exception_evaluator_failure(self, mock_page): + def test_page_evaluate_exception_evaluator_failure(self, mock_page, tmp_path): # 4. page.evaluate() raises an exception (e.g. element missing). config = { "eval": { @@ -85,13 +100,17 @@ def test_page_evaluate_exception_evaluator_failure(self, mock_page): } mock_page.evaluate.side_effect = Exception("Element not found") - evaluator = self._create_evaluator(config) - score = evaluator(trajectory=[{"action_type": "stop"}], config_file="", page=mock_page) + evaluator, config_file = self._create_evaluator(config, tmp_path) + score = evaluator( + trajectory=[{"action_type": "stop"}], + config_file=config_file, + page=mock_page, + ) # Missing element results in score 0.0 because it's caught and guarded as None assert score == 0.0 - def test_slow_navigation_waits_appropriately(self, mock_page): + def test_slow_navigation_waits_appropriately(self, mock_page, tmp_path): # 5, 6, 8. Check that navigation waits for networkidle and tolerates timeouts. config = { "eval": { @@ -112,9 +131,50 @@ def test_slow_navigation_waits_appropriately(self, mock_page): ) mock_page.evaluate.return_value = "Hello World" - evaluator = self._create_evaluator(config) - score = evaluator(trajectory=[{"action_type": "stop"}], config_file="", page=mock_page) + evaluator, config_file = self._create_evaluator(config, tmp_path) + score = evaluator( + trajectory=[{"action_type": "stop"}], + config_file=config_file, + page=mock_page, + ) mock_page.goto.assert_called_with("http://example.com/target") mock_page.wait_for_load_state.assert_called_with("networkidle", timeout=3000) assert score == 1.0 # The timeout was tolerated and evaluation proceeded + + def test_locator_exception_vs_content_mismatch_score(self, mock_page, tmp_path): + # Documents evaluation-provenance limitation: both locator exception and + # content mismatch evaluate to 0.0. + config = { + "eval": { + "eval_types": ["program_html"], + "program_html": [ + { + "url": "last", + "locator": "document.querySelector('foo').innerText", + "required_contents": {"exact_match": "Expected Content"}, + } + ], + } + } + + evaluator, config_file = self._create_evaluator(config, tmp_path) + + # Case A: Content mismatch + mock_page.evaluate.side_effect = None + mock_page.evaluate.return_value = "Different Content" + score_mismatch = evaluator( + trajectory=[{"action_type": "stop"}], + config_file=config_file, + page=mock_page, + ) + assert score_mismatch == 0.0 + + # Case B: Locator exception + mock_page.evaluate.side_effect = Exception("Locator query failed") + score_exception = evaluator( + trajectory=[{"action_type": "stop"}], + config_file=config_file, + page=mock_page, + ) + assert score_exception == 0.0