Skip to content

[chore] rm old performPlaywrightMethod #875

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 1 commit into from
Jul 14, 2025

Conversation

seanmcguire12
Copy link
Member

why

  • it is not good practice to maintain two separate implementations of the same function

what changed

  • removed a duplicate implementation of performPlaywrightMethod which was being used by the observe_simple_google_search eval
  • replaced the function call with a call to page.act(ObserveResult) which targets the implementation of performPlaywrightMethod that is actually being used (and is therefore the implementation that we should be testing

test plan

  • regression evals
  • observe evals

@seanmcguire12 seanmcguire12 added the observe These changes pertain to the observe function label Jul 14, 2025
Copy link

changeset-bot bot commented Jul 14, 2025

⚠️ No Changeset found

Latest commit: b96a5a3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR is a code cleanup that removes a duplicate implementation of performPlaywrightMethod from a11y/utils.ts and consolidates its usage in observe_simple_google_search.ts to use the primary implementation through stagehand.page.act(). The change simplifies the codebase by eliminating redundant code while maintaining the same functionality.

The PR aligns with good engineering practices by:

  1. Eliminating duplicate code that could lead to maintenance issues
  2. Ensuring consistent behavior by using a single implementation
  3. Making the codebase more maintainable

Confidence score: 5/5

  1. This PR is extremely safe to merge as it's a straightforward cleanup that doesn't change functionality
  2. The high score is due to comprehensive test coverage (regression + observe evals) and the mechanical nature of the change
  3. Files that need attention:
    • evals/tasks/observe_simple_google_search.ts - verify the new act() calls work as expected
    • lib/a11y/utils.ts - ensure no other code was depending on the removed implementation

2 files reviewed, no comments
Edit PR Review Bot Settings | Greptile

@seanmcguire12 seanmcguire12 merged commit 1d9b74c into main Jul 14, 2025
16 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
observe These changes pertain to the observe function
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants