first regression test #88
Conversation
| @@ -46,6 +47,29 @@ async def act(self, options: Union[ActOptions, ObserveResult]) -> ActResult: | |||
| options, self.stagehand.dom_settle_timeout_ms | |||
| ) | |||
|
|
|||
| # Extract timeout_ms from options (check both snake_case and camelCase) | |||
| timeout_ms = options.get("timeout_ms") or options.get("timeoutMs") | |||
There was a problem hiding this comment.
I needed to touch this since the regression tests revealed we were not handling act timeout locally
There was a problem hiding this comment.
@miguelg719 i am not sure how you want to handle the aliases mapping for local execution ? Since we need to convert to CamelCase for the API, but in Python for local execution it should be snake_case? Should we not use the Alias mapper for local execution
from LLM:
Alternative Solutions
We could have avoided this by:
- Not using aliases in ActHandler: Change StagehandPage.act() to serialize without aliases for local execution
- Converting back to snake_case: In ActHandler, convert camelCase back to snake_case
- Using the Pydantic object directly: Pass the ActOptions object instead of a serialized dict
But the current approach is the safest since it maintains compatibility with both the API format and handles the current codebase structure without major refactoring.
There was a problem hiding this comment.
I am open to a different solution, but the regression tests should pass locally
|
|
||
| result = await stagehand.page.extract(extract_options) | ||
|
|
||
| # TODO - how to unify the extract result handling between LOCAL and BROWSERBASE? |
There was a problem hiding this comment.
@miguelg719 pls lmk what you think
There was a problem hiding this comment.
resolved with new PR on extract API pydantic return, will merge into here when merged there
|
|
||
| result = await stagehand.page.extract(extract_options) | ||
|
|
||
| # TODO - how to unify the extract result handling between LOCAL and BROWSERBASE? |
There was a problem hiding this comment.
@miguelg719 pls lmk what you think
There was a problem hiding this comment.
resolved with new PR on extract API pydantic return, will merge into here when merged there
| ) | ||
|
|
||
| result = await stagehand.page.extract(extract_options) | ||
| #TODO - how to unify the extract result handling between LOCAL and BROWSERBASE? |
There was a problem hiding this comment.
@miguelg719 pls lmk what you think
There was a problem hiding this comment.
resolved with new PR on extract API pydantic return, will merge into here when merged there
|
|
||
| result = await stagehand.page.extract(extract_options) | ||
|
|
||
| #TODO - how to unify the extract result handling between LOCAL and BROWSERBASE? |
There was a problem hiding this comment.
@miguelg719 pls lmk what you think
There was a problem hiding this comment.
resolved with new PR on extract API pydantic return, will merge into here when merged there
| # If timeout is specified, wrap the entire act operation with asyncio.wait_for | ||
| if timeout_ms: | ||
| try: | ||
| return await asyncio.wait_for( |
There was a problem hiding this comment.
To confirm based on a concern raised in standup yesterday, I am not too concerned about race conditions here as this implementation mirrors TS.
why
what changed
test plan