Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdded URL context propagation across the agent/browser pipeline: a new optional Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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, ( |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High test
Copilot Autofix
AI about 1 month ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/agent/test_validator.py (1)
286-329: E2E test relies on external sites.This test depends on
example.comandiana.orgbeing available and maintaining their current link structure. While this is a reasonable assumption forexample.com(it's specifically designed as a stable example domain), consider adding a brief comment noting this dependency, or ensure the test is marked appropriately for CI environments where external network access may be restricted.💡 Optional: Add network dependency marker
+@pytest.mark.network def test_validator_accepts_completion_after_page_navigation(): """ End-to-end test with real LLM and AgentFallback: Verify that the validatorThis allows CI configurations to skip network-dependent tests if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/agent/test_validator.py` around lines 286 - 329, The test test_validator_accepts_completion_after_page_navigation relies on external sites (example.com and iana.org); update the test to either document this dependency with a brief inline comment near the test name or add a pytest marker so CI can skip it when network access is restricted (e.g., add `@pytest.mark.network` or `@pytest.mark.integration` or a `@pytest.mark.skipif` network-check before the test), and ensure references to notte.Session and notte.AgentFallback remain unchanged so the test behavior is preserved when it runs under network-enabled CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/sdk/test_agent_fallback.py`:
- Around line 62-70: Docstring in
test_agent_fallback_validator_with_page_navigation references a non-existent
local test name; update the docstring to reference the actual local test name
test_validator_accepts_completion_after_page_navigation (or the correct local
test) so the comment is accurate, i.e., edit the triple-quoted string inside
test_agent_fallback_validator_with_page_navigation to replace
test_agent_fallback_validator_with_page_navigation_local with
test_validator_accepts_completion_after_page_navigation.
---
Nitpick comments:
In `@tests/agent/test_validator.py`:
- Around line 286-329: The test
test_validator_accepts_completion_after_page_navigation relies on external sites
(example.com and iana.org); update the test to either document this dependency
with a brief inline comment near the test name or add a pytest marker so CI can
skip it when network access is restricted (e.g., add `@pytest.mark.network` or
`@pytest.mark.integration` or a `@pytest.mark.skipif` network-check before the
test), and ensure references to notte.Session and notte.AgentFallback remain
unchanged so the test behavior is preserved when it runs under network-enabled
CI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1ca4efc-90c1-45c6-8c81-1a5c7c1e80d8
📒 Files selected for processing (6)
packages/notte-agent/src/notte_agent/common/validator.pypackages/notte-agent/src/notte_agent/falco/perception.pypackages/notte-browser/src/notte_browser/session.pypackages/notte-core/src/notte_core/browser/observation.pytests/agent/test_validator.pytests/integration/sdk/test_agent_fallback.py
- 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 <noreply@anthropic.com>
2 participants
Summary by CodeRabbit
New Features
Tests
Chores
Greptile Summary
This PR fixes a bug where the
CompletionValidatorwould incorrectly reject a valid agent completion after page navigation. When an agent clicked a link that navigated to a new page, the validator saw the new page's elements but the action history referenced elements from the previous page — leading to false negatives. The fix threads the page URL at time of execution through the data model (ExecutionResult.url), surfaces it in the agent's perception layer (FalcoPerception.perceive_action_result), and adds a clear multi-page context explanation to the validator system prompt.Key changes:
observation.py— adds an optionalurlfield toExecutionResult(backward compatible, defaults toNone).session.py— capturesself.page.urlbefore both action execution and scrape execution, and passes it to every newly createdExecutionResult.perception.py— appends(on <url>)to the formatted action-result string when a URL is available, giving the LLM provenance for each action in the history.validator.py— extends the system prompt with anIMPORTANTblock explaining that element IDs reset per page and that action history URLs reflect where actions were actually performed.ExecutionResultURL population,perceive_action_resultoutput format, system prompt content, and end-to-end validation after page navigation.Concerns:
test_validator_accepts_completion_after_page_navigationintests/agent/test_validator.pyis a full end-to-end test (real browser, LLM, and external network calls toexample.com/iana.org) with nopytest.mark.integrationguard or skip condition. It will run with the unit-test suite in any environment and fail in offline/resource-constrained CI.test_validator_receives_url_in_action_history,asyncio.run()is called inside a synchronous test to append to a trajectory. This can conflict with an already-running event loop in some pytest configurations.Confidence Score: 4/5
urlfield withNonedefault) is fully backward compatible. URL capture happens at exactly the right moment (before execution) in both the action and scrape code paths. The perception and validator prompt changes are additive. The only risks are a fragile end-to-end test placed outside the integration folder (nopytest.markguard) and potentialasyncio.runconflicts in the test suite — neither affects production behaviour.test_validator_accepts_completion_after_page_navigationshould be guarded withpytest.mark.integrationor moved totests/integration/.Important Files Changed
self.page.urlbefore both action execution and scrape execution, then passes it to all createdExecutionResultobjects via the newurlfield. Correctly covers both the action path and the scrape path.(on <url>)to the success and failure action-result strings when a URL is present, providing the LLM with provenance context for each action in the history.test_validator_accepts_completion_after_page_navigation) that requires a real browser, LLM, and external network access; this test is placed in the non-integration test folder without a marker and without a skip guard, making it potentially fragile in offline/CI environments.pytest.skip, redirecting testers to the local test intest_validator.py. Clean placeholder with a useful message.Last reviewed commit: 3e35d68