From 3e35d689bab5ef33fc8bf34f94887d56a5693638 Mon Sep 17 00:00:00 2001 From: Leo Date: Wed, 11 Mar 2026 12:43:46 +0100 Subject: [PATCH 1/2] fix: agent fallback validator --- .../src/notte_agent/common/validator.py | 5 + .../src/notte_agent/falco/perception.py | 7 +- .../src/notte_browser/session.py | 5 + .../src/notte_core/browser/observation.py | 1 + tests/agent/test_validator.py | 203 ++++++++++++++++++ tests/integration/sdk/test_agent_fallback.py | 11 + 6 files changed, 230 insertions(+), 2 deletions(-) diff --git a/packages/notte-agent/src/notte_agent/common/validator.py b/packages/notte-agent/src/notte_agent/common/validator.py index 3c71432b5..bc58af956 100644 --- a/packages/notte-agent/src/notte_agent/common/validator.py +++ b/packages/notte-agent/src/notte_agent/common/validator.py @@ -17,6 +17,11 @@ But if something is missing or the image does not show what was requested dont let it pass. Try to understand the page and help the model with suggestions like scroll, do x, ... to get the solution right. +IMPORTANT: The agent navigates through multiple pages. Actions in the history were executed +on previous pages (check the URL in each action). The current page observation shows the +RESULT of navigation, not where actions were performed. Element IDs (L1, L2, etc.) reset +on each page - do not assume IDs in action history match current page elements. + Task to validate: {{task}}. Return a JSON object with 2 keys: `is_valid` and `reason`: diff --git a/packages/notte-agent/src/notte_agent/falco/perception.py b/packages/notte-agent/src/notte_agent/falco/perception.py index aac7c399c..aa5f04706 100644 --- a/packages/notte-agent/src/notte_agent/falco/perception.py +++ b/packages/notte-agent/src/notte_agent/falco/perception.py @@ -103,10 +103,13 @@ def perceive_action_result( include_data: bool = True, ) -> str: id_str = f" with id={result.action.id}" if include_ids and isinstance(result.action, InteractionAction) else "" + url_str = f" (on {result.url})" if result.url else "" if not result.success: err_msg = trim_message(result.message) - return f"❌ action '{result.action.name()}'{id_str} failed with error: {err_msg}" - success_msg = f"✅ action '{result.action.name()}'{id_str} succeeded: '{result.action.execution_message()}'" + return f"❌ action '{result.action.name()}'{id_str} failed{url_str}: {err_msg}" + success_msg = ( + f"✅ action '{result.action.name()}'{id_str} succeeded{url_str}: '{result.action.execution_message()}'" + ) if include_data: return f"{success_msg}{self.perceive_data(result.data, only_structured=True)}" return success_msg diff --git a/packages/notte-browser/src/notte_browser/session.py b/packages/notte-browser/src/notte_browser/session.py index eea017bd1..6b1f25e20 100644 --- a/packages/notte-browser/src/notte_browser/session.py +++ b/packages/notte-browser/src/notte_browser/session.py @@ -575,6 +575,7 @@ async def _aexecute_impl( exception = None scraped_data = None resolved_action = None + action_url = self.page.url # Capture URL BEFORE action execution with TimedSpan.capture() as span: try: @@ -707,6 +708,7 @@ async def _aexecute_impl( action=resolved_action, success=success, message=message, + url=action_url, data=scraped_data, exception=exception, started_at=span.started_at, @@ -921,6 +923,7 @@ async def ascrape( exception: Exception | None = None data: DataSpace | None = None + scrape_url = self.page.url # Capture URL BEFORE scrape execution with TimedSpan.capture() as span: try: data = await self._ascrape(**params) @@ -931,6 +934,7 @@ async def ascrape( action=scrape_action, success=False, message=scrape_action.execution_message(), + url=scrape_url, data=None, exception=exception, started_at=span.started_at, @@ -957,6 +961,7 @@ async def ascrape( # success is True if structured_scrape_failed is False, otherwise False success=not data.structured_scrape_failed if is_structured_scrape else True, message=scrape_action.execution_message(), + url=scrape_url, data=data, exception=data.structured_scrape_exception if is_structured_scrape else None, started_at=span.started_at, diff --git a/packages/notte-core/src/notte_core/browser/observation.py b/packages/notte-core/src/notte_core/browser/observation.py index ac3781e7d..521b144cb 100644 --- a/packages/notte-core/src/notte_core/browser/observation.py +++ b/packages/notte-core/src/notte_core/browser/observation.py @@ -320,6 +320,7 @@ class ExecutionResult(FilledTimedSpan): action: ActionUnion success: bool message: str + url: str | None = None # URL at time of action execution data: DataSpace | None = None exception: NotteBaseError | Exception | None = Field(default=None) diff --git a/tests/agent/test_validator.py b/tests/agent/test_validator.py index 1a0941ab8..1ce8bc099 100644 --- a/tests/agent/test_validator.py +++ b/tests/agent/test_validator.py @@ -124,3 +124,206 @@ def test_agent_with_schema(): ) assert valid.success, f"Failed to validate output: {valid.answer}" _ = Product.model_validate_json(valid.answer) + + +def test_execution_result_includes_url(): + """Test that ExecutionResult now includes URL context for actions.""" + from notte_core.actions import ClickAction + from notte_core.browser.observation import ExecutionResult, TimedSpan + + span = TimedSpan.empty() + + # Test with URL + result_with_url = ExecutionResult( + action=ClickAction(id="L1"), + success=True, + message="Clicked on element", + url="https://example.com", + started_at=span.started_at, + ended_at=span.ended_at, + ) + assert result_with_url.url == "https://example.com" + + # Test without URL (backwards compatible) + result_without_url = ExecutionResult( + action=ClickAction(id="L1"), + success=True, + message="Clicked on element", + started_at=span.started_at, + ended_at=span.ended_at, + ) + assert result_without_url.url is None + + +def test_perceive_action_result_includes_url(): + """Test that perceive_action_result includes URL in the output string.""" + from notte_agent.falco.perception import FalcoPerception + from notte_core.actions import ClickAction + from notte_core.browser.observation import ExecutionResult, TimedSpan + + perception = FalcoPerception() + span = TimedSpan.empty() + + # Test success with URL + result_success = ExecutionResult( + action=ClickAction(id="L1"), + success=True, + message="Clicked on element with text label: Learn more", + url="https://example.com", + started_at=span.started_at, + ended_at=span.ended_at, + ) + output_success = perception.perceive_action_result(result_success) + assert "(on https://example.com)" in output_success + assert "succeeded" in output_success + + # Test failure with URL + result_failure = ExecutionResult( + action=ClickAction(id="L1"), + success=False, + message="Element not found", + url="https://example.com/page", + started_at=span.started_at, + ended_at=span.ended_at, + ) + output_failure = perception.perceive_action_result(result_failure) + assert "(on https://example.com/page)" in output_failure + assert "failed" in output_failure + + # Test without URL (backwards compatible) + result_no_url = ExecutionResult( + action=ClickAction(id="L1"), + success=True, + message="Clicked", + started_at=span.started_at, + ended_at=span.ended_at, + ) + output_no_url = perception.perceive_action_result(result_no_url) + assert "(on " not in output_no_url # No URL prefix should appear + + +def test_validator_system_prompt_has_multipage_context(): + """Test that the validator system prompt includes multi-page context explanation.""" + from notte_agent.common.validator import system_rules + + # Check that the system prompt contains the multi-page context explanation + assert "IMPORTANT:" in system_rules + assert "multiple pages" in system_rules.lower() + assert "Element IDs" in system_rules or "element ids" in system_rules.lower() + assert "reset" in system_rules.lower() + + +def test_validator_receives_url_in_action_history(): + """ + Integration test: Verify that when the validator receives action history, + each action includes the URL where it was executed. + + This tests the fix for the bug where the validator would incorrectly reject + valid completions because actions executed on previous pages appeared to + reference non-existent elements on the current page. + """ + from unittest.mock import MagicMock + + from notte_agent.common.validator import CompletionValidator + from notte_agent.falco.perception import FalcoPerception + from notte_core.actions import ClickAction, CompletionAction + from notte_core.browser.observation import ExecutionResult, Observation, TimedSpan, TrajectoryProgress + from notte_core.trajectory import Trajectory + + # Create a mock trajectory that simulates: + # 1. Click on "Learn more" link on example.com + # 2. Page navigates to iana.org + span = TimedSpan.empty() + + # Simulate click action that was executed on example.com + click_result = ExecutionResult( + action=ClickAction(id="L1"), + success=True, + message="Clicked on element with text label: Learn more", + url="https://example.com", # KEY: This URL shows where the action was executed + started_at=span.started_at, + ended_at=span.ended_at, + ) + + trajectory = Trajectory() + + import asyncio + + asyncio.run(trajectory.append(click_result)) + + # Create perception and verify it includes URL in output + perception = FalcoPerception() + action_result_str = perception.perceive_action_result(click_result) + + # Verify URL is included in the action result string + assert "(on https://example.com)" in action_result_str, ( + f"URL should be included in action result. Got: {action_result_str}" + ) + + # Verify the format shows what happened + assert "succeeded" in action_result_str + assert "click" in action_result_str.lower() + + # Create validation message and verify URL context is present + # Use a mock LLM since we only need to test validation_message() method + mock_llm = MagicMock() + validator = CompletionValidator(llm=mock_llm, perception=perception, use_vision=False) + + completion = CompletionAction(success=True, answer="Successfully clicked the Learn more link") + + # Create a minimal observation (simulating we're now on iana.org) + progress = TrajectoryProgress(current_step=2, max_steps=5) + mock_obs = Observation.empty() + + validation_msg = validator.validation_message(completion, trajectory, progress, mock_obs) + + # The validation message should contain the URL context + assert "https://example.com" in validation_msg, ( + f"Validation message should include the URL where the action was executed. Got: {validation_msg}" + ) + + +def test_validator_accepts_completion_after_page_navigation(): + """ + End-to-end test with real LLM and AgentFallback: Verify that the validator + correctly accepts a completion after the agent navigated away from the page + where actions were executed. + + This is the exact scenario that was broken before the fix: + 1. Agent on example.com, user triggers invalid action + 2. AgentFallback kicks in and clicks "Learn more" link (L1) + 3. Page navigates to iana.org + 4. Agent tries to complete: "Successfully clicked the Learn more link" + 5. OLD behavior: Validator rejects because iana.org has no "Learn more" element + 6. NEW behavior: Validator accepts because action history shows the click + happened on example.com (via URL context) + """ + with notte.Session(headless=True) as session: + # Step 1: Navigate to example.com + session.execute(type="goto", url="https://example.com") + obs = session.observe() + assert "example" in obs.metadata.url.lower() + + # Step 2: Use AgentFallback - trigger with invalid action, agent should click the link + with notte.AgentFallback( + session, + task="Click on the 'Learn more' link", + max_steps=3, + use_vision=False, + ) as fallback: + # Trigger fallback with invalid action + session.execute(type="click", id="B999999", raise_on_failure=False) + + # Verify we navigated to iana.org (the click worked) + obs_after = session.observe() + assert "iana" in obs_after.metadata.url.lower(), ( + f"Expected to navigate to iana.org, but on: {obs_after.metadata.url}" + ) + + # The key assertion: AgentFallback should have succeeded + # This was the bug - the validator would reject because it saw iana.org elements + # but the action history said "clicked Learn more" without URL context + assert fallback.success, ( + "AgentFallback should succeed. Validator should accept completion because " + "action history now includes URL context showing click was on example.com." + ) diff --git a/tests/integration/sdk/test_agent_fallback.py b/tests/integration/sdk/test_agent_fallback.py index ccb357d8e..2265a2c49 100644 --- a/tests/integration/sdk/test_agent_fallback.py +++ b/tests/integration/sdk/test_agent_fallback.py @@ -57,3 +57,14 @@ def test_agent_fallback_scrape_should_raise_error(): with pytest.raises(ValueError): with client.AgentFallback(session, task="add the Cap product to cart", max_steps=1): _ = session.scrape() + + +def test_agent_fallback_validator_with_page_navigation(): + """ + Test that the validator correctly handles page navigation scenarios. + + NOTE: This test uses the remote SDK client. The fix for this bug is in the local + notte-agent package. Use test_agent_fallback_validator_with_page_navigation_local + for testing the local fix. + """ + pytest.skip("This test uses remote SDK - use the local test instead for validating the fix") From 584fde636e9137ef0fe42bd1dd7234d78f962031 Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 17 Mar 2026 10:55:31 +0100 Subject: [PATCH 2/2] fix: address PR review comments for agent fallback validator - Convert test_validator_receives_url_in_action_history to async to avoid asyncio.run() conflicts with pytest-asyncio - Add @pytest.mark.integration to E2E test requiring browser/LLM/network - Fix docstring reference to correct test name in SDK test - Register 'integration' marker in pyproject.toml Co-Authored-By: Claude Opus 4.5 --- pyproject.toml | 3 +++ tests/agent/test_validator.py | 9 ++++----- tests/integration/sdk/test_agent_fallback.py | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e84989391..0cf661788 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,6 +84,9 @@ timeout_method = "thread" asyncio_mode = "strict" log_cli = true log_cli_level = "INFO" +markers = [ + "integration: marks tests as integration tests (requires browser, LLM, network access)", +] filterwarnings = [ "ignore::DeprecationWarning:sklearn.utils.fixes:", "ignore::DeprecationWarning:pandas.core.common:", diff --git a/tests/agent/test_validator.py b/tests/agent/test_validator.py index 1ce8bc099..322a33565 100644 --- a/tests/agent/test_validator.py +++ b/tests/agent/test_validator.py @@ -213,7 +213,8 @@ def test_validator_system_prompt_has_multipage_context(): assert "reset" in system_rules.lower() -def test_validator_receives_url_in_action_history(): +@pytest.mark.asyncio +async def test_validator_receives_url_in_action_history(): """ Integration test: Verify that when the validator receives action history, each action includes the URL where it was executed. @@ -246,10 +247,7 @@ def test_validator_receives_url_in_action_history(): ) trajectory = Trajectory() - - import asyncio - - asyncio.run(trajectory.append(click_result)) + await trajectory.append(click_result) # Create perception and verify it includes URL in output perception = FalcoPerception() @@ -283,6 +281,7 @@ def test_validator_receives_url_in_action_history(): ) +@pytest.mark.integration def test_validator_accepts_completion_after_page_navigation(): """ End-to-end test with real LLM and AgentFallback: Verify that the validator diff --git a/tests/integration/sdk/test_agent_fallback.py b/tests/integration/sdk/test_agent_fallback.py index 2265a2c49..b3549b8fd 100644 --- a/tests/integration/sdk/test_agent_fallback.py +++ b/tests/integration/sdk/test_agent_fallback.py @@ -64,7 +64,7 @@ def test_agent_fallback_validator_with_page_navigation(): Test that the validator correctly handles page navigation scenarios. NOTE: This test uses the remote SDK client. The fix for this bug is in the local - notte-agent package. Use test_agent_fallback_validator_with_page_navigation_local - for testing the local fix. + notte-agent package. Use test_validator_accepts_completion_after_page_navigation + in tests/agent/test_validator.py for testing the local fix. """ pytest.skip("This test uses remote SDK - use the local test instead for validating the fix")