-
Notifications
You must be signed in to change notification settings - Fork 38
fix: flush debounce on Tab/Enter to prevent stale autocomplete #1661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import assert from "node:assert/strict"; | ||
| import test from "node:test"; | ||
|
|
||
| import { flushMentionDebounce } from "./flushMentionDebounce.ts"; | ||
|
|
||
| function ref(current) { | ||
| return { current }; | ||
| } | ||
|
|
||
| function candidate(overrides = {}) { | ||
| return { | ||
| kind: "identity", | ||
| displayName: "Beta", | ||
| isAgent: false, | ||
| isMember: true, | ||
| pubkey: "b".repeat(64), | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| test("flushMentionDebounce returns the fresh suggestion with its fresh start index", () => { | ||
| const debounceTimerRef = ref(setTimeout(() => {}, 1000)); | ||
|
|
||
| const flushed = flushMentionDebounce({ | ||
| debounceTimerRef, | ||
| latestValueRef: ref("@Alpha @be"), | ||
| latestCursorRef: ref("@Alpha @be".length), | ||
| searchableNamesLowerRef: ref(["alpha", "beta"]), | ||
| candidates: [ | ||
| candidate({ displayName: "Alpha", pubkey: "a".repeat(64) }), | ||
| candidate({ displayName: "Beta", pubkey: "b".repeat(64) }), | ||
| ], | ||
| activePersonaIds: new Set(), | ||
| channelType: "group", | ||
| }); | ||
|
|
||
| assert.equal(debounceTimerRef.current, null); | ||
| assert.equal(flushed?.type, "match"); | ||
| assert.equal(flushed?.suggestion.displayName, "Beta"); | ||
| assert.equal(flushed?.startIndex, 7); | ||
| }); | ||
|
|
||
| test("flushMentionDebounce returns no-match for a fresh query with no matches", () => { | ||
| const flushed = flushMentionDebounce({ | ||
| debounceTimerRef: ref(setTimeout(() => {}, 1000)), | ||
| latestValueRef: ref("@Alpha @zzzq"), | ||
| latestCursorRef: ref("@Alpha @zzzq".length), | ||
| searchableNamesLowerRef: ref(["alpha", "beta"]), | ||
| candidates: [candidate()], | ||
| activePersonaIds: new Set(), | ||
| channelType: "group", | ||
| }); | ||
|
|
||
| assert.deepEqual(flushed, { type: "no-match" }); | ||
| }); | ||
|
|
||
| test("flushMentionDebounce returns null for an empty fresh query", () => { | ||
| const flushed = flushMentionDebounce({ | ||
| debounceTimerRef: ref(setTimeout(() => {}, 1000)), | ||
| latestValueRef: ref("@"), | ||
| latestCursorRef: ref("@".length), | ||
| searchableNamesLowerRef: ref(["alpha", "beta"]), | ||
| candidates: [candidate()], | ||
| activePersonaIds: new Set(), | ||
| channelType: "group", | ||
| }); | ||
|
|
||
| assert.equal(flushed, null); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| /** | ||
| * Synchronously flush a pending mention debounce and resolve the correct | ||
| * top-ranked suggestion. Used by handleMentionKeyDown to close the race | ||
| * window where Tab/Enter fires before the debounce catches up to typed text. | ||
| */ | ||
| import type { MentionSuggestion } from "@/features/messages/ui/MentionAutocomplete"; | ||
| import type { UserProfileLookup } from "@/features/profile/lib/identity"; | ||
| import type { ChannelType } from "@/shared/api/types"; | ||
| import { detectPrefixQuery } from "@/shared/lib/detectPrefixQuery"; | ||
| import { | ||
| type MentionCandidateForRanking, | ||
| rankMentionCandidates, | ||
| } from "./mentionRanking"; | ||
| import { | ||
| mapMentionCandidateToSuggestion, | ||
| type MentionSuggestionCandidate, | ||
| } from "./mentionSuggestionMapping"; | ||
|
|
||
| type MentionCandidateWithUI = MentionCandidateForRanking & | ||
| MentionSuggestionCandidate; | ||
|
|
||
| export type FlushMentionDebounceResult = | ||
| | { type: "match"; suggestion: MentionSuggestion; startIndex: number } | ||
| | { type: "no-match" }; | ||
|
|
||
| /** | ||
| * Cancel the pending debounce timer, re-detect the prefix query from the | ||
| * latest editor state, rank candidates, and return the top suggestion — or | ||
| * null if no valid match is found. | ||
| */ | ||
| export function flushMentionDebounce<T extends MentionCandidateWithUI>(opts: { | ||
| debounceTimerRef: React.MutableRefObject<ReturnType< | ||
| typeof setTimeout | ||
| > | null>; | ||
| latestValueRef: React.RefObject<string>; | ||
| latestCursorRef: React.RefObject<number>; | ||
| searchableNamesLowerRef: React.RefObject<string[]>; | ||
| candidates: readonly T[]; | ||
| activePersonaIds: ReadonlySet<string>; | ||
| channelType?: ChannelType | null; | ||
| currentPubkey?: string | null; | ||
| ownerProfiles?: UserProfileLookup; | ||
| profiles?: UserProfileLookup; | ||
| }): FlushMentionDebounceResult | null { | ||
| if (opts.debounceTimerRef.current !== null) { | ||
| clearTimeout(opts.debounceTimerRef.current); | ||
| } | ||
| opts.debounceTimerRef.current = null; | ||
|
|
||
| const mention = detectPrefixQuery( | ||
| "@", | ||
| opts.latestValueRef.current, | ||
| opts.latestCursorRef.current, | ||
| opts.searchableNamesLowerRef.current, | ||
| ); | ||
|
|
||
| if (!mention || mention.query.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| const ranked = rankMentionCandidates( | ||
| opts.candidates, | ||
| mention.query, | ||
| opts.activePersonaIds, | ||
| ); | ||
|
|
||
| if (ranked.length === 0) { | ||
| return { type: "no-match" }; | ||
| } | ||
|
|
||
| const { candidate, label } = ranked[0]; | ||
| return { | ||
| type: "match", | ||
| suggestion: mapMentionCandidateToSuggestion({ | ||
| candidate, | ||
| label, | ||
| channelType: opts.channelType, | ||
| currentPubkey: opts.currentPubkey, | ||
| ownerProfiles: opts.ownerProfiles, | ||
| profiles: opts.profiles, | ||
| }), | ||
| startIndex: mention.startIndex, | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import type { MentionSuggestion } from "@/features/messages/ui/MentionAutocomplete"; | ||
| import type { UserProfileLookup } from "@/features/profile/lib/identity"; | ||
| import { formatOwnerLabel } from "@/features/profile/lib/identity"; | ||
| import type { ChannelRole, ChannelType } from "@/shared/api/types"; | ||
| import { normalizePubkey } from "@/shared/lib/pubkey"; | ||
|
|
||
| export type MentionSuggestionCandidate = { | ||
| kind: "identity" | "persona"; | ||
| pubkey?: string; | ||
| personaId?: string | null; | ||
| avatarUrl?: string | null; | ||
| isAgent: boolean; | ||
| isMember: boolean; | ||
| role?: ChannelRole | null; | ||
| ownerPubkey?: string | null; | ||
| }; | ||
|
|
||
| export function mapMentionCandidateToSuggestion(opts: { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 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. |
||
| candidate: MentionSuggestionCandidate; | ||
| label: string; | ||
| channelType?: ChannelType | null; | ||
| currentPubkey?: string | null; | ||
| ownerProfiles?: UserProfileLookup; | ||
| profiles?: UserProfileLookup; | ||
| }): MentionSuggestion { | ||
| const { | ||
| candidate, | ||
| channelType, | ||
| currentPubkey, | ||
| label, | ||
| ownerProfiles, | ||
| profiles, | ||
| } = opts; | ||
| const ownerLabel = candidate.isAgent | ||
| ? formatOwnerLabel(candidate.ownerPubkey, currentPubkey, ownerProfiles) | ||
| : null; | ||
|
|
||
| return { | ||
| pubkey: candidate.pubkey, | ||
| personaId: candidate.personaId ?? undefined, | ||
| kind: candidate.kind, | ||
| displayName: label, | ||
| avatarUrl: | ||
| candidate.avatarUrl ?? | ||
| (candidate.pubkey | ||
| ? profiles?.[normalizePubkey(candidate.pubkey)]?.avatarUrl | ||
| : null) ?? | ||
| null, | ||
| isAgent: candidate.isAgent, | ||
| notInChannel: channelType !== "dm" && candidate.isMember === false, | ||
| ownerLabel, | ||
| role: !candidate.isAgent && candidate.role === "admin" ? "admin" : null, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,12 +32,13 @@ import type { | |
| UserSearchResult, | ||
| } from "@/shared/api/types"; | ||
| import type { UserProfileLookup } from "@/features/profile/lib/identity"; | ||
| import { formatOwnerLabel } from "@/features/profile/lib/identity"; | ||
| import { detectPrefixQuery } from "@/shared/lib/detectPrefixQuery"; | ||
| import { normalizePubkey, truncatePubkey } from "@/shared/lib/pubkey"; | ||
| import { trimMapToSize } from "@/shared/lib/trimMapToSize"; | ||
| import { flushMentionDebounce } from "./flushMentionDebounce"; | ||
| import { hasMention } from "./hasMention"; | ||
| import { rankMentionCandidates } from "./mentionRanking"; | ||
| import { mapMentionCandidateToSuggestion } from "./mentionSuggestionMapping"; | ||
|
|
||
| const MENTION_DEBOUNCE_MS = 120; | ||
| const MENTION_SUGGESTION_LIMIT = 50; | ||
|
|
@@ -531,6 +532,7 @@ export function useMentions( | |
| ); | ||
| const latestValueRef = React.useRef<string>(""); | ||
| const latestCursorRef = React.useRef<number>(0); | ||
| const flushedMentionStartIndexRef = React.useRef<number | null>(null); | ||
| const searchableNamesLowerRef = React.useRef<string[]>(searchableNamesLower); | ||
|
|
||
| // Keep the known-names ref in sync so the debounced callback never reads stale data. | ||
|
|
@@ -558,35 +560,16 @@ export function useMentions( | |
| activePersonaIds, | ||
| ) | ||
| .slice(0, Math.max(MENTION_SUGGESTION_LIMIT, mentionCandidates.length)) | ||
| .map(({ candidate, label }) => { | ||
| const ownerLabel = candidate.isAgent | ||
| ? formatOwnerLabel( | ||
| candidate.ownerPubkey, | ||
| currentPubkey, | ||
| ownerProfilesQuery.data?.profiles, | ||
| ) | ||
| : null; | ||
| const notInChannel = | ||
| options?.channelType !== "dm" && candidate.isMember === false; | ||
|
|
||
| return { | ||
| pubkey: candidate.pubkey, | ||
| personaId: candidate.personaId, | ||
| kind: candidate.kind, | ||
| displayName: label, | ||
| avatarUrl: | ||
| candidate.avatarUrl ?? | ||
| (candidate.pubkey | ||
| ? profiles?.[normalizePubkey(candidate.pubkey)]?.avatarUrl | ||
| : null) ?? | ||
| null, | ||
| isAgent: candidate.isAgent, | ||
| notInChannel, | ||
| ownerLabel, | ||
| role: | ||
| !candidate.isAgent && candidate.role === "admin" ? "admin" : null, | ||
| }; | ||
| }); | ||
| .map(({ candidate, label }) => | ||
| mapMentionCandidateToSuggestion({ | ||
| candidate, | ||
| label, | ||
| channelType: options?.channelType, | ||
| currentPubkey, | ||
| ownerProfiles: ownerProfilesQuery.data?.profiles, | ||
| profiles, | ||
| }), | ||
| ); | ||
| }, [ | ||
| activePersonaIds, | ||
| currentPubkey, | ||
|
|
@@ -695,8 +678,11 @@ export function useMentions( | |
| setMentionQuery(null); | ||
| setMentionSelectedIndex(0); | ||
|
|
||
| const startIndex = | ||
| flushedMentionStartIndexRef.current ?? mentionStartIndex; | ||
| flushedMentionStartIndexRef.current = null; | ||
| return { | ||
| replaceFromOffset: mentionStartIndex, | ||
| replaceFromOffset: startIndex, | ||
| replaceToOffset: selectionEnd, | ||
| insertText, | ||
| }; | ||
|
|
@@ -917,6 +903,34 @@ export function useMentions( | |
| !event.shiftKey) | ||
| ) { | ||
| event.preventDefault(); | ||
|
|
||
| // If a debounce is pending, the suggestions array reflects a stale query. | ||
| // Flush: re-detect synchronously and re-derive the correct suggestion. | ||
| if (debounceTimerRef.current !== null) { | ||
| const flushed = flushMentionDebounce({ | ||
| debounceTimerRef, | ||
| latestValueRef, | ||
| latestCursorRef, | ||
| searchableNamesLowerRef, | ||
| candidates: mentionCandidates, | ||
| activePersonaIds, | ||
| channelType: options?.channelType, | ||
| currentPubkey, | ||
| ownerProfiles: ownerProfilesQuery.data?.profiles, | ||
| profiles, | ||
| }); | ||
| if (flushed?.type === "match") { | ||
| flushedMentionStartIndexRef.current = flushed.startIndex; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Updated by Grumplestiltzkin: addresses the blocking stale-range issue. The flush result carries the fresh |
||
| setMentionQuery(null); // reset so dropdown closes | ||
| return { handled: true, suggestion: flushed.suggestion }; | ||
| } | ||
| if (flushed?.type === "no-match") { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 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- |
||
| setMentionQuery(null); | ||
| return { handled: true }; | ||
| } | ||
| // Plain `@` after flush intentionally falls through to existing suggestions. | ||
| } | ||
|
|
||
| return { handled: true, suggestion: suggestions[mentionSelectedIndex] }; | ||
| } | ||
|
|
||
|
|
@@ -928,7 +942,17 @@ export function useMentions( | |
|
|
||
| return { handled: false }; | ||
| }, | ||
| [isMentionOpen, mentionSelectedIndex, suggestions], | ||
| [ | ||
| activePersonaIds, | ||
| currentPubkey, | ||
| isMentionOpen, | ||
| mentionCandidates, | ||
| mentionSelectedIndex, | ||
| options?.channelType, | ||
| ownerProfilesQuery.data?.profiles, | ||
| profiles, | ||
| suggestions, | ||
| ], | ||
| ); | ||
|
|
||
| return { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 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.