Skip to content
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

introduce complete verification #1201

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Conversation

LawyZheng
Copy link
Collaborator

@LawyZheng LawyZheng commented Nov 15, 2024

Important

Introduces a complete verification mechanism for user goals using CompleteVerifyResult and updates related functions and prompts.

  • Behavior:
    • Introduces complete_verify() in agent.py to verify user goals using CompleteVerifyResult.
    • Updates check_user_goal_complete() in agent.py to use complete_verify().
    • Modifies handle_complete_action() in handler.py to verify actions using complete_verify().
  • Models:
    • Adds CompleteVerifyResult class in actions.py to encapsulate verification results.
    • Updates CompleteAction in actions.py to include a verified attribute.
  • Prompts:
    • Updates check-user-goal.j2 to include page_info in the JSON response format.

This description was created by Ellipsis for 34c74f8. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 34c74f8 in 2 minutes and 49 seconds

More details
  • Looked at 220 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. skyvern/forge/agent.py:950
  • Draft comment:
    model_validate is not a standard method in Pydantic's BaseModel. Consider using parse_obj or validate instead.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is about a change in the diff, specifically the use of model_validate. The comment seems to be suggesting a code change, which aligns with the rules for keeping comments. However, I need to verify if model_validate is indeed not a standard method in Pydantic's BaseModel.
    I might be missing the fact that model_validate could be a custom method defined elsewhere in the codebase or a newer addition to Pydantic that the comment is not aware of.
    Given the rules, I should only keep the comment if there is strong evidence that model_validate is incorrect. Without verifying the existence of model_validate, I should err on the side of caution and consider deleting the comment.
    Delete the comment unless there is strong evidence that model_validate is incorrect. Without verification, assume the comment is not useful.
2. skyvern/webeye/actions/actions.py:74
  • Draft comment:
    The __repr__ method uses CompleteVerifyResponse instead of CompleteVerifyResult. Consider updating for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The CompleteVerifyResult class in actions.py has a __repr__ method that uses CompleteVerifyResponse instead of CompleteVerifyResult. This is a minor naming inconsistency.

Workflow ID: wflow_2MbA9X0Y3iA3s0Bq


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

skyvern/webeye/actions/handler.py Show resolved Hide resolved
@LawyZheng LawyZheng merged commit e505671 into main Nov 15, 2024
2 checks passed
@LawyZheng LawyZheng deleted the lawy/introduce-complete-verification branch November 15, 2024 06:03
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.

1 participant