Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 6 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="workflows/workflow_use/healing/selector_generator.py">
<violation number="1" location="workflows/workflow_use/healing/selector_generator.py:67">
Defaulting max_total_strategies to 2 truncates the selector list to only two strategies, removing critical fallbacks like XPath and causing element healing to fail once the first strategies miss.</violation>
</file>
<file name="workflows/workflow_use/healing/xpath_optimizer.py">
<violation number="1" location="workflows/workflow_use/healing/xpath_optimizer.py:138">
When max_alternatives is 1 or less, the function still keeps one non-fallback alternative and then appends the absolute path, returning two entries even though the limit was set to 1; callers expecting the documented cap can get more selectors than requested.</violation>
</file>
<file name="workflows/cli.py">
<violation number="1" location="workflows/cli.py:1228">
Optional workflow inputs without defaults can no longer be supplied: the new check treats "optional" as equivalent to "has a default", skips the entire input prompt, and silently drops those parameters whenever every required field has a default.</violation>
<violation number="2" location="workflows/cli.py:2505">
run-stored-workflow skips prompting for optional parameters with no defaults whenever the required parameters already have defaults, so those values cannot be provided anymore.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| 10. Direct CSS/xpath (fallback) | ||
| """ | ||
|
|
||
| def __init__(self, enable_xpath_optimization: bool = True, max_xpath_alternatives: int = 2, max_total_strategies: int = 2): |
Contributor
There was a problem hiding this comment.
Defaulting max_total_strategies to 2 truncates the selector list to only two strategies, removing critical fallbacks like XPath and causing element healing to fail once the first strategies miss.
Prompt for AI agents
Address the following comment on workflows/workflow_use/healing/selector_generator.py at line 67:
<comment>Defaulting max_total_strategies to 2 truncates the selector list to only two strategies, removing critical fallbacks like XPath and causing element healing to fail once the first strategies miss.</comment>
<file context>
@@ -62,6 +64,23 @@ class SelectorGenerator:
10. Direct CSS/xpath (fallback)
"""
+ def __init__(self, enable_xpath_optimization: bool = True, max_xpath_alternatives: int = 2, max_total_strategies: int = 2):
+ """
+ Initialize the SelectorGenerator.
</file context>
Suggested change
| def __init__(self, enable_xpath_optimization: bool = True, max_xpath_alternatives: int = 2, max_total_strategies: int = 2): | |
| def __init__(self, enable_xpath_optimization: bool = True, max_xpath_alternatives: int = 2, max_total_strategies: Optional[int] = None): |
| if max_alternatives > 1: | ||
| unique_alternatives = unique_alternatives[: max_alternatives - 1] | ||
| else: | ||
| unique_alternatives = unique_alternatives[:1] |
Contributor
There was a problem hiding this comment.
When max_alternatives is 1 or less, the function still keeps one non-fallback alternative and then appends the absolute path, returning two entries even though the limit was set to 1; callers expecting the documented cap can get more selectors than requested.
Prompt for AI agents
Address the following comment on workflows/workflow_use/healing/xpath_optimizer.py at line 138:
<comment>When max_alternatives is 1 or less, the function still keeps one non-fallback alternative and then appends the absolute path, returning two entries even though the limit was set to 1; callers expecting the documented cap can get more selectors than requested.</comment>
<file context>
@@ -117,17 +119,29 @@ def optimize_xpath(self, absolute_xpath: str, element_info: Optional[Dict] = Non
+ if max_alternatives > 1:
+ unique_alternatives = unique_alternatives[: max_alternatives - 1]
+ else:
+ unique_alternatives = unique_alternatives[:1]
+
+ # Always add original absolute path as last resort
</file context>
Suggested change
| unique_alternatives = unique_alternatives[:1] | |
| unique_alternatives = [] |
✅ Addressed in 42a6294
| f', default: {typer.style(str(default_value), fg=typer.colors.BLUE)}' if default_value is not None else '' | ||
| # Check if all inputs have defaults (can run without user input) | ||
| all_inputs_have_defaults = all( | ||
| getattr(inp, 'default', None) is not None or not inp.required for inp in workflow_definition.input_schema |
Contributor
There was a problem hiding this comment.
run-stored-workflow skips prompting for optional parameters with no defaults whenever the required parameters already have defaults, so those values cannot be provided anymore.
Prompt for AI agents
Address the following comment on workflows/cli.py at line 2505:
<comment>run-stored-workflow skips prompting for optional parameters with no defaults whenever the required parameters already have defaults, so those values cannot be provided anymore.</comment>
<file context>
@@ -2460,24 +2500,64 @@ def run_stored_workflow(
- f', default: {typer.style(str(default_value), fg=typer.colors.BLUE)}' if default_value is not None else ''
+ # Check if all inputs have defaults (can run without user input)
+ all_inputs_have_defaults = all(
+ getattr(inp, 'default', None) is not None or not inp.required for inp in workflow_definition.input_schema
+ )
+
</file context>
Suggested change
| getattr(inp, 'default', None) is not None or not inp.required for inp in workflow_definition.input_schema | |
| getattr(inp, 'default', None) is not None for inp in workflow_definition.input_schema |
| default_value = getattr(input_def, 'default', None) | ||
| # Check if all inputs have defaults (can skip prompting) | ||
| all_inputs_have_defaults = all( | ||
| getattr(input_def, 'default', None) is not None or not input_def.required for input_def in input_definitions |
Contributor
There was a problem hiding this comment.
Optional workflow inputs without defaults can no longer be supplied: the new check treats "optional" as equivalent to "has a default", skips the entire input prompt, and silently drops those parameters whenever every required field has a default.
Prompt for AI agents
Address the following comment on workflows/cli.py at line 1228:
<comment>Optional workflow inputs without defaults can no longer be supplied: the new check treats "optional" as equivalent to "has a default", skips the entire input prompt, and silently drops those parameters whenever every required field has a default.</comment>
<file context>
@@ -1223,58 +1223,78 @@ async def _run_workflow():
- default_value = getattr(input_def, 'default', None)
+ # Check if all inputs have defaults (can skip prompting)
+ all_inputs_have_defaults = all(
+ getattr(input_def, 'default', None) is not None or not input_def.required for input_def in input_definitions
+ )
</file context>
Suggested change
| getattr(input_def, 'default', None) is not None or not input_def.required for input_def in input_definitions | |
| getattr(input_def, 'default', None) is not None for input_def in input_definitions |
Contributor
There was a problem hiding this comment.
3 issues found across 4 files (reviewed changes from recent commits).
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="workflows/test_max_alternatives_bug_fix.py">
<violation number="1" location="workflows/test_max_alternatives_bug_fix.py:34">
The max_alternatives=1 check only asserts the count of returned XPaths, so the regression test would still pass even if the optimizer returned a single (wrong) optimized XPath instead of the required absolute fallback.</violation>
<violation number="2" location="workflows/test_max_alternatives_bug_fix.py:50">
The max_alternatives=2 test only verifies the number of XPaths, so it would miss regressions where the absolute fallback is absent or duplicated, defeating the stated expectation of “1 optimized + 1 absolute”.</violation>
<violation number="3" location="workflows/test_max_alternatives_bug_fix.py:66">
The max_alternatives=3 regression test asserts only the total count, so it would not detect cases where the absolute fallback is missing or where fewer than two optimized selectors are produced, contradicting the expectation printed to the user.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| is_absolute = xpath == absolute_xpath | ||
| print(f' {i}. {xpath[:60]}{"..." if len(xpath) > 60 else ""} {"(absolute)" if is_absolute else "(optimized)"}') | ||
|
|
||
| if len(result_3) == 3: |
Contributor
There was a problem hiding this comment.
The max_alternatives=3 regression test asserts only the total count, so it would not detect cases where the absolute fallback is missing or where fewer than two optimized selectors are produced, contradicting the expectation printed to the user.
Prompt for AI agents
Address the following comment on workflows/test_max_alternatives_bug_fix.py at line 66:
<comment>The max_alternatives=3 regression test asserts only the total count, so it would not detect cases where the absolute fallback is missing or where fewer than two optimized selectors are produced, contradicting the expectation printed to the user.</comment>
<file context>
@@ -0,0 +1,78 @@
+ is_absolute = xpath == absolute_xpath
+ print(f' {i}. {xpath[:60]}{"..." if len(xpath) > 60 else ""} {"(absolute)" if is_absolute else "(optimized)"}')
+
+if len(result_3) == 3:
+ print(' ✅ PASS: Exactly 3 XPaths returned')
+else:
</file context>
✅ Addressed in f5fdca1
| is_absolute = xpath == absolute_xpath | ||
| print(f' {i}. {xpath[:60]}{"..." if len(xpath) > 60 else ""} {"(absolute)" if is_absolute else "(optimized)"}') | ||
|
|
||
| if len(result_2) == 2: |
Contributor
There was a problem hiding this comment.
The max_alternatives=2 test only verifies the number of XPaths, so it would miss regressions where the absolute fallback is absent or duplicated, defeating the stated expectation of “1 optimized + 1 absolute”.
Prompt for AI agents
Address the following comment on workflows/test_max_alternatives_bug_fix.py at line 50:
<comment>The max_alternatives=2 test only verifies the number of XPaths, so it would miss regressions where the absolute fallback is absent or duplicated, defeating the stated expectation of “1 optimized + 1 absolute”.</comment>
<file context>
@@ -0,0 +1,78 @@
+ is_absolute = xpath == absolute_xpath
+ print(f' {i}. {xpath[:60]}{"..." if len(xpath) > 60 else ""} {"(absolute)" if is_absolute else "(optimized)"}')
+
+if len(result_2) == 2:
+ print(' ✅ PASS: Exactly 2 XPaths returned')
+else:
</file context>
✅ Addressed in f5fdca1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Adds an XPath selection optimizer that generates a small set of robust, prioritized XPath alternatives (plus an absolute fallback) to improve selector healing stability and reduce brittle paths. Also fixes the alternative limit to return only the absolute XPath when max_alternatives=1.
New Features
Bug Fixes
Written for commit 371ab17. Summary will update automatically on new commits.