Skip to content

Fix vlm_web_browser.py example #410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 30, 2025
Merged

Fix vlm_web_browser.py example #410

merged 3 commits into from
Jan 30, 2025

Conversation

abidlabs
Copy link
Member

@abidlabs abidlabs commented Jan 29, 2025

A typo which meant that the older images were never actually being removed

A typo which meant that the images were never being remove for lean processing
@abidlabs abidlabs requested a review from merveenoyan January 29, 2025 02:39
@abidlabs abidlabs changed the title Update vlm_web_browser.py example Fix vlm_web_browser.py example Jan 29, 2025
@@ -48,7 +48,7 @@ def save_screenshot(step_log: ActionStep, agent: CodeAgent) -> None:
current_step = step_log.step_number
if driver is not None:
for step_logs in agent.logs: # Remove previous screenshots from logs for lean processing
if isinstance(step_log, ActionStep) and step_log.step_number <= current_step - 2:
if isinstance(step_logs, ActionStep) and step_logs.step_number <= current_step - 2:
Copy link
Member Author

Choose a reason for hiding this comment

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

Would be better to just rename this to avoid potential issues with step_logs <> step_log but in the interest of keeping the diff minimal, left the PR like this

@abidlabs
Copy link
Member Author

abidlabs commented Jan 29, 2025

Note: I am also pretty sure there's a typo on line 60:

step_log.observations = url_info if step_logs.observations is None else step_log.observations + "\n" + url_info

should be:

step_log.observations = url_info if step_log.observations is None else step_log.observations + "\n" + url_info

but wasn't 100% sure I understood the rationale for this line so I didn't make the change in this PR

@aymeric-roucher aymeric-roucher merged commit c0abd21 into main Jan 30, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants