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

convert css shape to string #1092

Merged
merged 1 commit into from
Oct 30, 2024
Merged

convert css shape to string #1092

merged 1 commit into from
Oct 30, 2024

Conversation

LawyZheng
Copy link
Collaborator

@LawyZheng LawyZheng commented Oct 30, 2024

Important

Add functionality to convert CSS shapes to string descriptions using LLM, with caching and logging support.

  • Behavior:
    • Add _convert_css_shape_to_string in agent_functions.py to convert CSS shapes to string descriptions using LLM.
    • Update cleanup_element_tree_func in agent_functions.py to include CSS shape conversion.
    • Add css-shape-convert.j2 for LLM prompt to convert CSS shapes.
  • Caching:
    • Implement _get_shape_cache_key in agent_functions.py for caching CSS shape conversions.
    • Use cache to store and retrieve CSS shape descriptions.
  • Logging:
    • Add detailed logging for CSS shape conversion attempts and failures in agent_functions.py.
  • Utilities:
    • Add checkDisabledFromStyle in domUtils.js to determine if an element is disabled based on style.
    • Update buildElementObject in domUtils.js to include disabled attribute based on style.
  • Imports:
    • Import Frame in agent_functions.py and handler.py to support frame operations.

This description was created by Ellipsis for 33f0b42. 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.

👍 Looks good to me! Reviewed everything up to 33f0b42 in 1 minute and 15 seconds

More details
  • Looked at 431 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. skyvern/forge/agent_functions.py:170
  • Draft comment:
    Consider renaming _convert_css_shape_to_string to _convert_element_shape_to_string for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The function name _convert_css_shape_to_string is misleading as it suggests a conversion of CSS to a string, but it actually converts the shape of an element to a string description. The name should reflect the purpose more clearly.
2. skyvern/forge/agent_functions.py:31
  • Draft comment:
    The logic in _should_css_shape_convert can be simplified by combining conditions for span and i tags. Consider refactoring for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The function _should_css_shape_convert is checking for specific tags and conditions to decide if a shape conversion is needed. However, the logic is a bit scattered and could be simplified for better readability.
3. skyvern/forge/agent_functions.py:70
  • Draft comment:
    Consider using a set for USELESS_SHAPE_ATTRIBUTE to optimize the lookup time when removing attributes in _remove_skyvern_attributes.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The function _remove_skyvern_attributes is using a deep copy of the element, which is necessary to avoid modifying the original element. However, the logic for removing attributes can be optimized.
4. skyvern/forge/prompts/skyvern/css-shape-convert.j2:6
  • Draft comment:
    Remove comments from JSON in css-shape-convert.j2 to ensure valid JSON syntax.
  • Reason this comment was not posted:
    Based on historical feedback, this comment is too similar to comments previously marked by users as bad.
5. skyvern/webeye/scraper/domUtils.js:689
  • Draft comment:
    Consider expanding checkDisabledFromStyle to account for other styles that may indicate a disabled state.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The function buildElementObject in domUtils.js is checking for disabled elements by style but does not account for other potential styles that could indicate a disabled state.

Workflow ID: wflow_IfB3c6n5VNUz2iVR


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@LawyZheng LawyZheng merged commit 8762865 into main Oct 30, 2024
2 checks passed
@LawyZheng LawyZheng deleted the lawy/support-parse-css-shape branch October 30, 2024 16:12
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