Skip to content

fix(browser): add full_page parameter to browser_vision for viewport-only screenshots - #19647

Open
liuhao1024 wants to merge 3 commits into
NousResearch:mainfrom
liuhao1024:fix/issue-19620-browser-vision-full-page
Open

liuhao1024 wants to merge 3 commits into
NousResearch:mainfrom
liuhao1024:fix/issue-19620-browser-vision-full-page

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 4, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds a full_page parameter to browser_vision that controls whether the screenshot captures the full scrollable page or just the current viewport.

Root Cause

browser_vision unconditionally appended --full to the agent-browser screenshot command (line 2167 of tools/browser_tool.py). On long pages (e.g. bodyHeight ≈ 10,000 px), this produces enormous PNGs (~41k px height) that vision APIs downscale to unreadable smears. The model then reports "highly compressed, low-resolution, corrupted image" and refuses to extract data.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • See commit messages for detailed changes

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

…INSTALL_TIMEOUT

Increase the default npm install timeout for WhatsApp bridge from 60s
to 300s (5 minutes) to accommodate slower systems like Unraid NAS.
Make it configurable via WHATSAPP_NPM_INSTALL_TIMEOUT environment variable
for users who need even longer timeouts.

Closes NousResearch#14980
- Add 'path', 'old_string', 'new_string', and 'patch' to required list
- Update description to clarify mode-specific parameter requirements
- This addresses issue where LLMs would omit these parameters because
  they were not marked as required in the schema, even though they
  are required depending on the mode

Fixes NousResearch#15524
@alt-glitch alt-glitch added type/bug Something isn't working tool/browser Browser automation (CDP, Playwright) comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists labels May 4, 2026
@alt-glitch

Copy link
Copy Markdown

Closes #19620 per PR description. Related to #9775 (selector-based screenshots).

…only screenshots

browser_vision unconditionally passed --full to the screenshot command,
capturing the entire scrollable page. On long pages (e.g. 10,000+ px),
this produces enormous PNGs that vision APIs downscale to unreadable
smears.

Add a full_page parameter (default True for backward compatibility) that
controls whether --full is passed. Setting full_page=False captures only
the current viewport, producing crisp, manageable screenshots.

Fixes NousResearch#19620
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating a real browser-vision failure: current main still unconditionally appends --full on the normal agent-browser screenshot path (tools/browser_tool.py:4017-4022).

Problems

  • The PR also changes PATCH_SCHEMA to require every mode-specific field simultaneously. That makes either valid mode structurally incomplete. Current main deliberately uses description-level requirements with required == ["mode"] because conditional schema forms are not provider-safe (tools/file_tools.py:2051-2054,2092; tests/tools/test_file_tools.py:607-613).
  • The browser code has since been refactored: current browser_vision has Camofox (tools/browser_tool.py:3911-3913) and Lightpanda/Chrome fallback (tools/browser_tool.py:3961-3968) paths that the old-context patch does not cover.
  • The added tests swallow all exceptions and only assert inside if mock_cmd.called, so they can pass without exercising the screenshot call.

Suggested changes

  • Salvage the focused browser change against current tools/browser_tool.py, including its schema and registry handler; cover normal, Lightpanda fallback, and Camofox behavior explicitly.
  • Replace the tests with unconditional command-argument assertions using a valid temporary screenshot fixture.
  • Drop the unrelated WhatsApp and file-schema commits.

Automated hermes-sweeper review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/browser Browser automation (CDP, Playwright) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants