Skip to content

Use prompt_toolkit's complete_in_thread option#1548

Merged
rolandwalker merged 1 commit intomainfrom
RW/prompt-toolkit-complete-in-thread
Feb 16, 2026
Merged

Use prompt_toolkit's complete_in_thread option#1548
rolandwalker merged 1 commit intomainfrom
RW/prompt-toolkit-complete-in-thread

Conversation

@rolandwalker
Copy link
Contributor

Description

This makes the interface more responsive by not blocking typing.

This is intended to combine with #1547 for more fluid typing.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

Copy link
Contributor

@scottnemes scottnemes left a comment

Choose a reason for hiding this comment

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

Nice, seems a little more snappy

@rolandwalker
Copy link
Contributor Author

@scottnemes the difference is most noticeable on a very long statement, since we are parsing the SQL to produce the completions.

which makes the interface more responsive by not blocking typing.
@rolandwalker rolandwalker force-pushed the RW/prompt-toolkit-complete-in-thread branch from 4e74c41 to b96732d Compare February 16, 2026 09:00
@github-actions
Copy link

Review summary: Small change, but it alters threading behavior in completions. I see two correctness risks that should be addressed before merge.

  1. Potential incompatibility with minimum prompt_toolkit version (hard failure).
    PromptSession will raise TypeError if the installed prompt_toolkit does not support complete_in_thread. The minimum version is prompt_toolkit>=3.0.6, but it’s unclear if complete_in_thread exists that far back. If it was introduced later, this PR breaks users pinned near the minimum.
    Action: confirm the option exists in 3.0.6. If not, either bump the minimum version or gate the argument based on prompt_toolkit.__version__.
    Refs: mycli/main.py:1184, pyproject.toml:15

  2. Possible data race between background completion and completer mutation.
    With complete_in_thread=True, SQLCompleter.get_completions() runs on a background thread. But the completer is mutated/reset under a lock (reset_completions()), and the prompt toolkit thread does not acquire that lock. This can lead to inconsistent reads or transient errors when reset_completions() runs during a completion request.
    Action: wrap the completer with a thread-safe adapter (e.g., a custom Completer that acquires _completer_lock around get_completions), or ensure reset/swap are done in a thread-safe, immutable fashion.
    Refs: mycli/main.py:1181-1188, mycli/main.py:1388-1416, mycli/sqlcompleter.py:953-972

Tests: I didn’t run tests. There’s no test coverage for threaded completion behavior; consider adding at least a concurrency smoke test around reset_completions() + get_completions() if you keep background completion.

If you want, I can draft the locking wrapper or the version-guard patch.

@rolandwalker rolandwalker merged commit ec6605a into main Feb 16, 2026
10 checks passed
@rolandwalker rolandwalker deleted the RW/prompt-toolkit-complete-in-thread branch February 16, 2026 09:04
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.

2 participants