fix: flush debounce on Tab/Enter to prevent stale autocomplete#1661
fix: flush debounce on Tab/Enter to prevent stale autocomplete#1661cameronhotchkies wants to merge 3 commits into
Conversation
When a user types '@timp' quickly and presses Tab before the 120ms debounce fires, the mention query still reflects the stale empty-string state from the initial '@' keystroke. This causes the autocomplete to commit suggestions[0] from the full unfiltered member list — often the current user themselves. Fix: when Tab or Enter is pressed while a debounce timer is pending, cancel the timer, re-run detectPrefixQuery synchronously against the already-stashed latest editor value and cursor (latestValueRef / latestCursorRef), then re-derive the ranked suggestions inline via rankMentionCandidates. The top-ranked candidate for the fresh query is returned as the suggestion, closing the race window completely. This is Option 1 (flush + re-derive inline) — the user gets the correct completion on the first Tab press with no extra interaction required. Extracted flushMentionDebounce into its own module to keep useMentions.ts under the 1000-line file-size limit. Co-authored-by: Cameron Hotchkies <chotchkies@block.xyz> Signed-off-by: Cameron Hotchkies <chotchkies@block.xyz> Co-authored-by: Goose <opensource@block.xyz> Ai-assisted: true
wesbillman
left a comment
There was a problem hiding this comment.
Reviewed the diff plus the surrounding useMentions.ts / MessageComposer.tsx call path. The root-cause analysis is right (stale suggestions from the pre-debounce query committed on Tab/Enter), the flush-and-re-derive approach is the correct shape, the fix covers both composers (MessageComposer and ForumComposer share handleMentionKeyDown), the added callback deps are complete, and all desktop CI suites are green. One substantive issue before merge:
1. Fresh suggestion is paired with a stale replace range (mentionStartIndex)
flushMentionDebounce re-runs detectPrefixQuery and gets a fresh mention.startIndex — then discards it. The flushed suggestion is handed back to the composer, which calls insertMention(suggestion, cursor), and insertMention builds the edit from the stale state mentionStartIndex (useMentions.ts:699) — the value set by whichever debounce last fired, i.e. possibly a different @ run than the one just flushed.
Repro: type @al, pause >120ms so the dropdown opens (stale mentionStartIndex = offset of first @), then quickly type @b and press Tab within the debounce window. Flush resolves the suggestion for query b, but the edit replaces from the first @ through the cursor — wiping @al from the composer.
The pre-existing code had a related staleness (stale suggestion + stale range), but this PR introduces the mismatched pair (fresh suggestion + stale range). Since the flush already computes the correct startIndex, the fix is small: return it alongside the suggestion (e.g. { suggestion, startIndex }) and either stash it in a ref that insertMention reads, or thread it through to the insert. Calling setMentionStartIndex alone won't work — the insert runs synchronously in the same event handler, before the state commit.
2. No-match fall-through re-commits the stale suggestion (non-blocking)
When the flush finds a fresh query but rankMentionCandidates returns zero matches (@zzzq + fast Tab), the code falls through to suggestions[mentionSelectedIndex] — committing a suggestion ranked for the stale query, which is the original bug's behavior for the no-match subcase. Not a regression (pre-PR did the same), but since you're already in the flush path, returning { handled: true } with no suggestion (and closing the dropdown) seems truer to intent. Note the fall-through is correct for the query.length === 0 case (plain @ + Tab), so only the ranked.length === 0 branch would change.
3. Nit: the new module is unit-testable but untested
flushMentionDebounce is nearly pure (refs in, suggestion out) and this race plus both edge cases above are cheap to pin down with fake refs — mentionRanking.test.mjs next door is the precedent. A regression test here would keep the race from quietly reopening.
4. Nit: duplicated suggestion mapping
The ranked-candidate → MentionSuggestion mapping duplicates the matchingSuggestions memo minus the profile-map avatar fallback and ownerLabel. Harmless today (the insert path ignores both fields), but the two copies can drift — a shared mapper would prevent that. Fine to leave as-is.
Happy to re-review once #1 is addressed — it should be a ~10-line change.
Co-authored-by: npub1m0vvn9qm5md0a080p27qzkm9uaw49e699ukwfq7fc0756xq0y5zqhzhdk2 <dbd8c9941ba6dafebcef0abc015b65e75d52e7452f2ce483c9c3fd4d180f2504@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1m0vvn9qm5md0a080p27qzkm9uaw49e699ukwfq7fc0756xq0y5zqhzhdk2 <dbd8c9941ba6dafebcef0abc015b65e75d52e7452f2ce483c9c3fd4d180f2504@sprout-oss.stage.blox.sqprod.co>
Share candidate-to-suggestion mapping between the rendered autocomplete list and the synchronous flush path, so avatar and owner metadata cannot drift. Also make the no-match flush case explicit: close the dropdown without committing a stale suggestion while preserving the plain bare-@ fallthrough. Co-authored-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1x4hk035p3p9q39a3fcrd2fe30lpkrhr5dwe0cqzzjphxyyh8m0gsq4vqap <356f67c681884a0897b14e06d527317fc361dc746bb2fc0042906e6212e7dbd1@sprout-oss.stage.blox.sqprod.co>
cameronhotchkies
left a comment
There was a problem hiding this comment.
🤖 Updated by Grumplestiltzkin: pushed 5885487a; inline notes call out how the review points were addressed.
| profiles, | ||
| }); | ||
| if (flushed?.type === "match") { | ||
| flushedMentionStartIndexRef.current = flushed.startIndex; |
There was a problem hiding this comment.
🤖 Updated by Grumplestiltzkin: addresses the blocking stale-range issue. The flush result carries the fresh startIndex, and this ref lets insertMention consume it synchronously instead of reading stale React state.
| setMentionQuery(null); // reset so dropdown closes | ||
| return { handled: true, suggestion: flushed.suggestion }; | ||
| } | ||
| if (flushed?.type === "no-match") { |
There was a problem hiding this comment.
🤖 Updated by Grumplestiltzkin: addresses the no-match fall-through. A flushed query with zero ranked matches now closes the dropdown and returns handled without committing a stale suggestion; the bare-@ case still falls through as requested.
| assert.equal(flushed?.startIndex, 7); | ||
| }); | ||
|
|
||
| test("flushMentionDebounce returns no-match for a fresh query with no matches", () => { |
There was a problem hiding this comment.
🤖 Updated by Grumplestiltzkin: covers the helper behavior the review called out: the stale-range repro returns the fresh suggestion/start index, no-match returns an explicit no-match result, and empty query remains the plain-@ fall-through.
| ownerPubkey?: string | null; | ||
| }; | ||
|
|
||
| export function mapMentionCandidateToSuggestion(opts: { |
There was a problem hiding this comment.
🤖 Updated by Grumplestiltzkin: addresses the duplicated mapping nit. The rendered autocomplete list and synchronous flush path now share this candidate-to-suggestion mapper, so avatar fallback, owner label, and channel-membership metadata cannot drift.
|
You called? |
agh! sorry for the rogue @ mention. The underlying bug was happening to a controller agent persona (named 'Timpani' to provide a steady beat) ... it didn't occur to me the PR descriptions would tag unrelated github users. I'll be more careful in the future. |
When a user types '@timp' (an arbitrary name) quickly and presses Tab before the 120ms debounce fires, the mention query still reflects the stale empty-string state from the initial '@' keystroke. This causes the autocomplete to commit suggestions[0] from the full unfiltered member list — often the current user themselves.
Fix: when Tab or Enter is pressed while a debounce timer is pending, cancel the timer, re-run detectPrefixQuery synchronously against the already-stashed latest editor value and cursor (latestValueRef / latestCursorRef), then re-derive the ranked suggestions inline via rankMentionCandidates. The top-ranked candidate for the fresh query is returned as the suggestion, closing the race window completely.
With this option (flush + re-derive inline) the user gets the correct completion on the first Tab press with no extra interaction required.
Extracted flushMentionDebounce into its own module to keep useMentions.ts under the 1000-line file-size limit.
Fixes: #1660
Ai-assisted: true