Skip to content

fix(browser): default non-finite vision config - #51981

Open
lin-hongkuan wants to merge 1 commit into
NousResearch:mainfrom
lin-hongkuan:codex/fix-browser-vision-nonfinite-config
Open

lin-hongkuan wants to merge 1 commit into
NousResearch:mainfrom
lin-hongkuan:codex/fix-browser-vision-nonfinite-config

Conversation

@lin-hongkuan

Copy link
Copy Markdown
Contributor

Summary

  • reject non-finite browser vision timeout and temperature config values
  • fall back to the existing 120s / 0.1 defaults for inf and nan
  • add regression coverage for the regular browser vision path

Tests

  • python -m pytest tests/tools/test_browser_console.py::TestBrowserVisionConfig::test_browser_vision_defaults_nonfinite_config -q
  • python -m pytest tests/tools/test_browser_console.py::TestBrowserVisionConfig -q
  • python -m pytest tests/tools/test_browser_console.py -q (fails on Windows due existing default-encoding reads of SKILL.md in TestDogfoodSkill; targeted browser vision tests pass)
  • python -m py_compile tools/browser_tool.py
  • python scripts/check-windows-footguns.py --all

@alt-glitch alt-glitch added type/bug Something isn't working tool/browser Browser automation (CDP, Playwright) P3 Low — cosmetic, nice to have labels Jun 24, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the non-finite configuration case. The browser_vision change itself matches the current defect at tools/browser_tool.py:4118-4121 and the new test exercises the intended fallback.

Problems

  • The same auxiliary.vision timeout/temperature parsing remains unguarded in image vision at tools/vision_tools.py:1236-1239 and video vision at tools/vision_tools.py:1739-1742. Those paths pass the converted values to their LLM calls, so nan/inf remains accepted outside browser_vision.

Suggested changes

  • Apply finite-value validation to both tools/vision_tools.py paths, retaining their existing defaults, and add matching regression coverage.

Automated hermes-sweeper review.

Comment thread tools/browser_tool.py
@@ -113,6 +114,15 @@

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extend this finite-value handling to the sibling auxiliary.vision consumers in tools/vision_tools.py (1236-1239 for image analysis and 1739-1742 for video analysis). They still pass nan/inf through direct float(...) conversions.

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

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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