Skip to content

actionWidget: fix: resume interrupted IME searches - #335578

Open
Ulugbek Abdullaev (ulugbekna) wants to merge 2 commits into
mainfrom
ulugbekna/actionlist-ime-retry
Open

actionWidget: fix: resume interrupted IME searches#335578
Ulugbek Abdullaev (ulugbekna) wants to merge 2 commits into
mainfrom
ulugbekna/actionlist-ime-retry

Conversation

@ulugbekna

Copy link
Copy Markdown
Contributor

Summary

Resume an asynchronous ActionList search that IME composition interrupted, even when composition ends with the original query. Do not repeat searches that already completed or failed, and do not let a stale request's cleanup disturb a newer request.

This is a focused follow-up to #328269. The shared-component fixes are separated from #335018 so the automation changes can be reviewed and landed independently.

Only two files change:

  • actionList.ts: the existing filter-input guard and asynchronous request cleanup.
  • actionList.test.ts: four regression cases using the existing widget helpers and deferred promises.

Background and failure

ActionList can filter an existing list locally or delegate filtering to an asynchronous provider through onFilter. For asynchronous filtering, it owns the current request's cancellation token source in a MutableDisposable.

The existing IME handling deliberately defers filtering during composition. Intermediate input can contain uncommitted text, and rebuilding or laying out the result list can disrupt the IME candidate window. Starting a composition also cancels an outstanding search so its results cannot update the list during composition.

Both compositionend and the accompanying input event eventually use the same value-change handler. That handler originally skipped every unchanged query to deduplicate those events.

The failure occurs when:

  1. The user searches for release, starting an asynchronous request.
  2. IME composition starts before the request finishes, cancelling that request.
  3. Composition ends with release again, either after intermediate text or without a committed change.
  4. The unchanged-query guard suppresses a replacement request, while the original request's results are correctly ignored because it was cancelled.

The pending query therefore never supplies fresh results until the user changes the search text again.

How the fix works

1. An unchanged query can restart an interrupted request

The input handler still ignores all changes while composition is active. Outside composition, it skips an unchanged query only when there is no cancelled current request.

This lets compositionend restart the search that composition interrupted. The restart immediately replaces the cancelled token source with a live one, so the trailing input event for the same text does not issue another request. Existing checks continue to reject results from the cancelled request.

2. Settled requests stop looking like pending work

Relaxing the guard alone would introduce an unnecessary retry: previously the current token source remained stored after a search completed or failed. A later compositionstart could cancel that already-settled source and make the unchanged query look interrupted.

The asynchronous chain now has a finally handler that clears the token source only when:

  • it is still the widget's current request; and
  • it has not been cancelled.

The same cleanup runs after success or failure. Clearing through the existing MutableDisposable disposes the source without cancelling an already-settled token, so later composition cannot mark that search as interrupted.

The identity check is essential: an older request can settle after a newer request has replaced it. Its cleanup must not clear the newer request's cancellation state. Otherwise composition could neither reliably cancel nor restart the newer request.

A cancelled current request intentionally remains available as the retry signal, even if its promise has already settled. The existing replacement, hide, and disposal paths own its cleanup; no additional collection or long-lived listener is introduced.

Scope and preserved behavior

  • Local/static filtering remains unchanged.
  • Composing text still does not drive filtering or intermediate list updates.
  • Searches whose committed query changes still follow the existing request path.
  • Completed and failed searches are not retried solely because composition leaves the query unchanged.
  • Superseded results remain ignored, and superseded completions cannot clear newer request state.
  • The existing error-handling behavior is preserved; this does not introduce a retry policy for failed searches.
  • No public API, settings, commands, keybindings, labels, ARIA roles, styling, or layout rules change.
  • No automation, Agents Window, or chat-picker production code is included.

The interaction remains focused and consistent: the IME retains ownership during composition, and only the interrupted committed query resumes afterward. This is not a visual redesign or a new interactive surface.

Validation

Standalone validation against the main-based branch:

  • npm run transpile-client succeeded and generated fresh output containing both fixes.
  • 192 tests passed across the five modules below.
  • Both commits passed the normal pre-commit hygiene hook.
  • git diff --check passed.
./scripts/test.sh \
  --run src/vs/platform/actionWidget/test/browser/actionList.test.ts \
  --run src/vs/platform/actionWidget/test/browser/tabbedActionListWidget.test.ts \
  --run src/vs/platform/actionWidget/test/browser/actionWidgetDropdown.test.ts \
  --run src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostSessionConfigPicker.test.ts \
  --run src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts \
  --reporter dot

The new regression cases cover:

  1. An interrupted search is restarted once when composition leaves the query unchanged; stale results stay hidden and the replacement results appear.
  2. A successfully completed search is not retried or marked cancelled by a later unchanged composition.
  3. A failed search is not retried or marked cancelled by a later unchanged composition.
  4. Settling an older, superseded request does not lose the pending request's ability to be cancelled and retried by composition.

The tests explicitly send the trailing input event to verify deduplication. The existing suites also cover composition suppression, stale-result handling, keyboard and combobox behavior, disposable cleanup, and the two dynamic consumers.

Validation used synthetic browser composition events in the Electron unit runner. A native operating-system IME session and manual screen-reader pass were not rerun for this extraction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f
Release settled filter cancellation state without disturbing newer requests. Preserve retries for interrupted composition while avoiding repeat searches after completed success or failure.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues remain, and the focused regression coverage addresses the revised behavior.

Review tier: Lite
Findings: None

What changed in this PR

Fixes interrupted asynchronous ActionList searches after IME composition while preventing redundant retries and stale cleanup.

Changes:

  • Restarts cancelled searches when the query is unchanged.
  • Safely clears settled request state.
  • Adds regression tests for request lifecycle scenarios.
File Description
src/​vs/​platform/​actionWidget/​test/​browser/​actionList.test.ts Adds asynchronous filtering regression tests.
src/​vs/​platform/​actionWidget/​browser/​actionList.ts Updates IME retry and request cleanup handling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants