fix: allow negative POINT coordinates so off-screen elements still point - #129
Open
TUARAN wants to merge 1 commit into
Open
fix: allow negative POINT coordinates so off-screen elements still point#129TUARAN wants to merge 1 commit into
TUARAN wants to merge 1 commit into
Conversation
Claude occasionally returns slightly negative coordinates for elements near the screen edge, and ElementLocationDetector already documents that out-of-range coordinates occur. The parser's \d+ groups rejected them, so pointing was skipped and the raw [POINT:...] tag leaked into the spoken text. Accept an optional minus sign so the existing clamp path handles the bounds.
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
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
parsePointingCoordinatesnow accepts an optional minus sign on the x/y values in[POINT:x,y:label:screenN]tags, so slightly negative coordinates from Claude no longer make the parser reject the tag and skip pointing.Issue
Fixes #128 — "Cursor pointing is skipped when Claude returns a slightly negative coordinate".
Root cause
The pattern in
leanring-buddy/CompanionManager.swiftused\d+for both coordinates. Claude occasionally returns slightly negative coordinates (e.g.[POINT:-1,42:button]) for elements near the screen edge — a caseElementLocationDetectoralready documents. The\d+groups cannot match a leading-, so the whole tag fails to parse, the raw tag is left in the spoken text, and pointing is skipped even though the downstream code already clamps out-of-bounds coordinates to the screenshot bounds.Verification
Behavior verified with the same regex engine the app uses (
NSRegularExpression/ ICU) via a standalone harness, before vs after the one-line change:[POINT:1100,42:color inspector][POINT:-1,42:button][POINT:10,-5:button:screen2][POINT:-3,-7:menu][POINT:none]swiftc -parse leanring-buddy/CompanionManager.swiftexits 0 after the change.Before/after behavior
[POINT:-1,42:button]→parsePointingCoordinatesreturnscoordinate: nil, the raw tag stays in the spoken text, and no cursor flight is scheduled.coordinate: (-1, 42), tag removed from spoken text, and the existing clamp (max(0, min(pointCoordinate.x, screenshotWidth))inCompanionManager.swift) maps the negative value to the screenshot edge.Risk and scope
One-line regex change. No behavior change for non-negative coordinates,
[POINT:none], or responses without a tag. No new dependencies.Not tested
Full app run (requires a real macOS UI with screen-recording/audio permissions;
xcodebuildis intentionally avoided per the repo instructions).