-
Notifications
You must be signed in to change notification settings - Fork 891
[fix] ensure selfHeal respects arguments #897
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
[fix] ensure selfHeal respects arguments #897
Conversation
🦋 Changeset detectedLatest commit: 31e36c4 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this 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 addresses a critical bug in Stagehand's self-healing functionality where action arguments were not being preserved during the healing process. The fix maintains the original method and arguments when retrying failed actions, only updating the selector through observation.
Key changes:
- Modified
actHandler.ts
to preserve original method and arguments during self-healing - Added
selfHeal: true
to default StagehandConfig - Added three new test cases to validate argument preservation:
- heal_scroll_50: Tests scroll position preservation
- heal_simple_google_search: Tests search text preservation
- heal_custom_dropdown: Tests click handling on custom dropdowns
PR Description Notes:
- The PR description template is completely empty. Please fill in the 'why', 'what changed', and 'test plan' sections.
Confidence score: 4/5
- This PR is safe to merge with proper testing as it fixes a bug rather than introducing new functionality.
- The changes are well-contained and include comprehensive test coverage for various interaction types.
- Special attention needed for:
- evals/tasks/heal_*.ts files to ensure test coverage is comprehensive
- lib/handlers/actHandler.ts to verify self-healing logic is correct
6 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
@@ -48,6 +48,7 @@ const StagehandConfig = { | |||
}, | |||
}, | |||
}, | |||
selfHeal: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seanmcguire12 👀, 👍 / 👎 ?
evals/tasks/heal_scroll_50.ts
Outdated
await stagehand.page.act({ | ||
description: "Scroll 50% down the page", | ||
selector: "/html/body/div/div/button", | ||
arguments: ["50"], | ||
method: "scrollTo", | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this make sense to self heal at all?
b3f01bc
to
048c119
Compare
const element: ObserveResult = observeResults[0]; | ||
await this._performPlaywrightMethod( | ||
// override previously provided method and arguments | ||
observe.method, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will also be overriden
why
On self healing we were finding the selector, but nor remembering the arguments previously passed; so either they were empty or the LLM was hallucinating them. This PR fixes this
what changed
Updated the logic from selfHealing to reuse the previously passed arguments and method
test plan
evals/tasks/heal_*.ts
)