fix coords tagging in agent_xray.py #242
Conversation
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the coordinate validation in tag_screenshot_with_action so that the function accepts both 2D and 3D coordinate formats.
- Update validation logic to allow coordinate arrays of length 2 or 3.
- Maintain error handling for coordinate formats that do not meet these requirements.
| coords = [c.strip() for c in coords] | ||
| if len(coords) != 2: | ||
| if len(coords) not in [2, 3]: | ||
| raise ValueError(f"Invalid coordinate format: {coords}") |
There was a problem hiding this comment.
The updated validation now accepts 3 coordinates, yet the subsequent code only processes the first two values. Consider handling the third coordinate appropriately or adding documentation to clarify its intended use.
| raise ValueError(f"Invalid coordinate format: {coords}") | |
| raise ValueError(f"Invalid coordinate format: {coords}") | |
| # Only the first two coordinates (x, y) are used. The third coordinate, if present, is ignored. |
![korbit-ai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Missing parentheses validation ▹ view | 🧠 Incorrect |
 |
Docstring coordinate format mismatch ▹ view | 🧠 Not in scope |
Files scanned
| File Path | Reviewed |
|---|---|
| src/agentlab/analyze/agent_xray.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| if len(coords) not in [2, 3]: | ||
| raise ValueError(f"Invalid coordinate format: {coords}") |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| try: | ||
| coords = action[action.index("(") + 1 : action.index(")")].split(",") |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Description by Korbit AI
What change is being made?
Modify the coordinate validation logic in
agent_xray.pyto accept both 2 and 3 coordinate formats.Why are these changes being made?
The update was made to accommodate an additional coordinate format necessary for certain actions, resolving issues where actions could be improperly formatted with three coordinates rather than two. This change ensures that valid input formats are accepted without generating errors.