Fix autocomplete targeting between mentions - #30463
Closed
charliemarsh-oai wants to merge 7 commits into
Closed
Conversation
charliemarsh-oai
force-pushed
the
charlie/fix-skill-autocomplete
branch
3 times, most recently
from
July 5, 2026 19:11
1035ec9 to
6df1532
Compare
This was referenced Jul 5, 2026
charliemarsh-oai
changed the base branch from
main
to
charlie/autocomplete-separators
July 5, 2026 19:11
This was referenced Jul 6, 2026
charliemarsh-oai
force-pushed
the
charlie/autocomplete-separators
branch
from
July 6, 2026 21:46
a8c0044 to
b629fce
Compare
charliemarsh-oai
added a commit
that referenced
this pull request
Jul 6, 2026
## Summary Autocomplete popup synchronization already identifies the active token's range and query, but accepting a result previously discarded that range and independently recomputed token boundaries around the cursor. Those calculations could disagree at ambiguous cursor positions. For example, in `@first| @second` (with `|` marking the cursor at the intervening space), the popup targets `@second`, while the old acceptance path replaces `@first`. This threads the active range through legacy file search, skill mentions, and mentions-v2 completion. File, image, and mention insertion now replace the same token that supplied the popup query, and image completion reuses the shared file insertion path. ## Stack This is PR 1 of 3. #31191 builds separator and dismissal behavior on these explicit replacement ranges, and #30463 then fixes token affinity between adjacent mentions.
charliemarsh-oai
force-pushed
the
charlie/autocomplete-separators
branch
from
July 6, 2026 21:59
b629fce to
f183e69
Compare
charliemarsh-oai
force-pushed
the
charlie/fix-skill-autocomplete
branch
from
July 6, 2026 21:59
6df1532 to
6920861
Compare
charliemarsh-oai
force-pushed
the
charlie/autocomplete-separators
branch
from
July 6, 2026 22:01
f183e69 to
e804544
Compare
charliemarsh-oai
force-pushed
the
charlie/fix-skill-autocomplete
branch
2 times, most recently
from
July 6, 2026 23:58
97becb0 to
156cf1e
Compare
charliemarsh-oai
force-pushed
the
charlie/autocomplete-separators
branch
from
July 7, 2026 00:13
e44e267 to
304ad36
Compare
charliemarsh-oai
force-pushed
the
charlie/fix-skill-autocomplete
branch
from
July 7, 2026 00:13
156cf1e to
6c75899
Compare
charliemarsh-oai
added a commit
that referenced
this pull request
Jul 7, 2026
## Summary Autocomplete completion previously inserted a new space even when a separator was already present, which could leave redundant whitespace around neighboring text. Popup dismissal also tracked only the query string, so dismissing one token could suppress a different occurrence with the same text. This gives completions one horizontal-separator policy across files, images, skills, and mentions. Existing separators are reused when possible, ordinary suffix text remains separated, and line breaks stay intact. Dismissal now identifies the complete whitespace-delimited token occurrence, so offset-only edits preserve dismissal while later identical tokens remain independent. Newly completed values beginning with `$` or `@` can also remain closed when the next PR's affinity rule recognizes the token to the left of the cursor. ## Examples The examples below use `|` to represent the cursor. ### Reuse existing separators Starting with an existing two-space gap: ```text @ma| next ``` After accepting `src/main.rs` and typing `foo`, completion previously added a third separator and left both original spaces before `next`: ```text src/main.rs foo| next ``` After this PR, completion reuses the first existing separator as the insertion point and preserves the second between the new text and `next`: ```text src/main.rs foo| next ``` ### Keep a completed sigil-prefixed value closed Starting before a line break: ```text @ma| next ``` and accepting a path whose result is itself prefixed with `@` produces: ```text @scope/main.rs | next ``` Without this PR's completion dismissal, the affinity rule in #30463 rediscovers the completed token to the left of the cursor and reopens its popup: ```text @scope/main.rs | ^^^^^^^^^^^^^^^ popup reopens next ``` With this PR, the inserted occurrence is dismissed and the popup remains closed: ```text @scope/main.rs | ^^^^^^^^^^^^^^^ popup remains closed next ``` ### Do not dismiss an identical later occurrence Given two identical tokens: ```text @scope/main.rs| @scope/main.rs ``` after dismissing the first popup with Escape and moving to the second token, query-only dismissal previously suppressed the second popup too: ```text @scope/main.rs @scope/main.rs| ^^^^^^^^^^^^^ popup remains closed ``` After this PR, dismissal also matches the token's ordinal among complete tokens, so the second occurrence opens normally: ```text @scope/main.rs @scope/main.rs| ^^^^^^^^^^^^^ popup opens ``` ### Keep dismissal across offset-only edits After dismissing `@ma`, moving to its start, and pasting an email-like token: ```text email@ma.com @ma| ``` the `@ma` bytes embedded in `email@ma.com` do not count as another autocomplete token. The original `@ma` keeps its dismissal even though its byte range moved. ## Stack This is PR 2 of 3, stacked on #31190. It relies on the explicit replacement ranges introduced there and provides the completion lifecycle used by the targeting fix in #30463.
charliemarsh-oai
force-pushed
the
charlie/fix-skill-autocomplete
branch
from
July 7, 2026 22:11
6c75899 to
2d81ad6
Compare
charliemarsh-oai
marked this pull request as ready for review
July 7, 2026 22:12
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d81ad6cbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
charliemarsh-oai
marked this pull request as draft
July 7, 2026 22:43
charliemarsh-oai
marked this pull request as ready for review
July 7, 2026 23:18
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ae71105ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
@codex review |
charliemarsh-oai
marked this pull request as draft
July 7, 2026 23:59
Contributor
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
charliemarsh-oai
marked this pull request as ready for review
July 8, 2026 00:34
This was referenced Jul 11, 2026
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
When the cursor is placed between an unbound skill mention and an already bound mention, we previously preferred the token to the right. For example, with
|representing the cursor:The popup should target
$unbound-skill, but it instead targets$bound-skill, whose text is already an atomic element.Here's an example. I've entered
$simplify-code, then I move the cursor to the start of the line to add$rustdoc. But the completion sticks on the right-most token, and hittingTaborEnteradds a duplicate copy:Tab.mov
And after:
Untitled.mov
Stack
This is PR 3 of 3, stacked on #31191. #31190 threads exact autocomplete target ranges through insertion, and #31191 defines separator and occurrence-scoped dismissal behavior before this PR changes token affinity.