-
Notifications
You must be signed in to change notification settings - Fork 7.9k
fix(tui): increase paste burst char interval on Windows to 30ms #9348
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
fix(tui): increase paste burst char interval on Windows to 30ms #9348
Conversation
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51881cce7b
ℹ️ 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".
f39d1fa to
a2a64f2
Compare
|
Addressed the review feedback: Fix: Applied the same Windows-specific timing to Now both
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ 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". |
840a379 to
9a6853e
Compare
|
Rebased on latest main - tui2 paste_burst.rs was removed upstream, so changes now only apply to tui. @codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ 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". |
|
@yuvrajangadsingh, looks like there are some merge conflicts that need to be resolved. |
db61009 to
6129831
Compare
|
Rebased on latest main. Conflicts resolved. @codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ 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". |
|
@etraut-openai Conflicts resolved, rebased on latest main. CI passing, Codex review clean. Ready for review. |
d879bf7 to
c47627e
Compare
|
Gentle ping - users are waiting for this fix. See #2137 (comment) The PR is rebased and ready for review. |
Windows terminals (especially VS Code integrated terminal) deliver paste key events more slowly than native terminals. The previous 8ms threshold caused paste detection to fail, resulting in Enter submitting the message instead of inserting a newline during multi-line pastes. This follows the existing pattern of Windows-specific timing for PASTE_BURST_ACTIVE_IDLE_TIMEOUT (60ms vs 8ms). Fixes openai#2137
a1259fc to
b72fc2f
Compare
…adsingh/codex into fix/windows-paste-burst-timing
joshka-oai
left a comment
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.
Thanks for the PR and apologies for the delay in getting it over the line and the churn getting there.
| #[cfg(not(windows))] | ||
| const PASTE_BURST_CHAR_INTERVAL: Duration = Duration::from_millis(8); | ||
| #[cfg(windows)] | ||
| const PASTE_BURST_CHAR_INTERVAL: Duration = Duration::from_millis(30); |
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.
Note that this is probably right on the threshold where we might from time to time detect actual typing as a paste for very fast typers typing common digraphs like 'th', 'er' etc. https://chatgpt.com/share/e/698b7f8e-4c10-8009-aa40-5e421aace79f
Not a blocker - just a note for if we do need to update this down a bit - 25ms would be a good target point (unless there's data to suggest that the pastes are taking this long on your system) If it becomes a bigger issue then we'd want this to be runtime configurable, but we can hold off on that for now.
|
Working out the bazel problems with this so we can merge it. |
|
@joshka-oai, we can ignore the bazel issues. Those actions will always fail for external contributions. I'll override and merge. |
Summary
PASTE_BURST_CHAR_INTERVALfrom 8ms to 30ms on Windows to fix multi-line paste issues in VS Code integrated terminalPASTE_BURST_ACTIVE_IDLE_TIMEOUT)Problem
When pasting multi-line text in Codex CLI on Windows (especially VS Code integrated terminal), only the first portion is captured before auto-submit. The rest arrives as a separate message.
Root cause: VS Code's terminal emulation adds latency (~10-15ms per character) between key events. The 8ms
PASTE_BURST_CHAR_INTERVALthreshold is too tight - characters arrive slower than expected, so burst detection fails and Enter submits instead of inserting a newline.Solution
Use Windows-specific timing (30ms) for
PASTE_BURST_CHAR_INTERVAL, following the same pattern already used forPASTE_BURST_ACTIVE_IDLE_TIMEOUT(60ms on Windows vs 8ms on Unix).30ms is still fast enough to distinguish paste from typing (humans type ~200ms between keystrokes).
Test plan
Fixes #2137