Skip to content

Conversation

@AdsQnn
Copy link

@AdsQnn AdsQnn commented Oct 25, 2025

Summary

This PR simplifies the paste burst detection API and fixes an issue with OSC color query responses.

Changes

  1. Simplified PasteBurst API

    • Replaced FlushResult enum with Option<String> for cleaner API
    • Updated chat_composer.rs to handle the simplified return type
    • Distinguishes paste vs typed input by string length instead of enum variants
  2. Improved paste detection reliability

    • Increased PASTE_BURST_CHAR_INTERVAL from 8ms to 16ms
    • Added better heuristics: now considers retro_chars >= 2 as paste-like
    • Improved recommended_flush_delay() with guard band for timer jitter
  3. Fixed color query issue

    • Disabled requery_default_colors() on terminal focus gain
    • Prevents OSC color query responses from bleeding into input stream

before

Zrzut ekranu z 2025-10-25 22-00-52

Here you can see how it sends right after pasting.
Zrzut ekranu z 2025-10-21 03-55-01

after
Zrzut ekranu z 2025-10-25 22-02-22

Problem with EAGAIN and reading from stdin:

What is EAGAIN?

  • EAGAIN = Unix/Linux error: "Try AGAIN" — meaning “no data available right now, try later”.
  • It occurs with non-blocking I/O when you attempt to read but the buffer is currently empty.
  • This mechanism allows you to drain all available input in a single loop without blocking.

How it was supposed to work for paste burst:

loop {
    match read(stdin_fd, &mut buf) {
        Ok(n) => all_data.extend(&buf[..n]),  // Keep reading available bytes
        Err(EAGAIN) => break,                 // Done! No more data right now
    }
}

Why it's tricky on Windows:

  1. Windows doesn’t have EAGAIN

    • Uses a completely different I/O model: Overlapped I/O / IOCP (I/O Completion Ports)
    • Similar errors: ERROR_IO_PENDING, ERROR_NO_DATA instead of EAGAIN.
  2. Different console API

    • Unix: uses read() on file descriptor 0 (stdin)
    • Windows: uses ReadConsoleInput() or ReadFile() — totally different mechanisms
  3. Non-blocking mode is handled differently

    • Unix: fcntl(fd, F_SETFL, O_NONBLOCK)
    • Windows: SetNamedPipeHandleState() or overlapped async operations
  4. Crossterm already handles this

    • The crossterm crate provides cross-platform event handling
    • No need to reimplement EAGAIN-like logic manually
    • Prefer using crossterm::event::EventStream for unified async input

Why you can't drain stdin on Windows:

Console input is an event queue, not a byte stream

Unix: stdin is a file descriptor—you read bytes until EAGAIN
Windows: ReadConsoleInput() returns events (key, mouse, resize) from a queue
No simple "read everything available"

No non-blocking read for console

Unix: O_NONBLOCK + read() = returns EAGAIN immediately
Windows: ReadConsoleInput() blocks or returns 0 events (unclear if empty or waiting)
PeekConsoleInput() only peeks, doesn't consume

Raw stdin ≠ console on Windows

ReadFile(GetStdHandle(STD_INPUT_HANDLE)) works for pipes/files
For console you need ReadConsoleW()—different function!
Non-blocking ReadFile() requires overlapped I/O (async callbacks)

Timing problem

Can't tell if event queue is empty because NO input OR data still incoming
No equivalent of "EAGAIN = end of currently available data"

Conclusion: Windows requires async/event-based approach (IOCP), not sync drain loop. That's why they use crossterm.
Therefore, paste bursts on Windows won’t work when there’s no support for bracketed paste — and the same applies to some other systems as well.
That means you could try using async fd, but it's unclear how it would work.
However, bracketed paste is supported by most modern terminals, so this isn’t a significant issue.

If anyone’s interested, I can share an example snippet that handles EAGAIN on an async file descriptor.

In summary, at the current stage everything works correctly — no duplicate input, no unexpected sends, fully stable behavior.
The root cause of the issue was the requery_default_colors() call.

@dylan-hurd-oai

I’ve compiled and run it — everything works as expected.
I’m leaving the notes above in case someone finds them useful in the future.

Paste burst works analogously to before, and regarding EAGAIN I'm speaking generally—the problem existed earlier. At the moment as I was writing it doesn't pose a problem because bracketed paste works virtually everywhere. But educationally speaking, the system depending on load, without bracketed paste, in addition to sending byte-by-byte, could also send packets in 1, 2, 4, 8 KB sizes, etc. Event stream ini poll can't be drained that way to pure bytes.

So currently paste burst with bracketed paste disabled won't work correctly on any system, not just Windows, precisely because of the 1, 2, 4, 8 KB packet sizing. For short pastes it will work relatively fine, but for long ones it will depend on how much you paste at once—the buffer might catch the whole 8k characters at once and it will work, or it might break it into 1k chunks, and then only subsequent clicks will fill in the next packets. So if the terminal doesn't support bracketed paste, the only advice for now is don't paste long text :)

Using async fd will solve the problem on Unix systems, but how it will work on Windows is a mystery.

So now it will work on Windows and any other system, as long as the terminal supports bracketed paste. If it doesn’t, then on any system it might work sometimes and sometimes not.

Url

@AdsQnn
Copy link
Author

AdsQnn commented Oct 27, 2025

Okay, I’ll add some conditional flag instead of commenting it out.
I think I won’t remove or comment out the whole thing — you can decide yourselves what to do with it next ?

I’ll set it to false for now, since I’m not sure whether you’ll want to completely remove this function or use it in some other way.

- Replace FlushResult enum with Option<String> in paste_burst.rs
- Update chat_composer to use Option<String> instead of FlushResult
- Increase PASTE_BURST_CHAR_INTERVAL from 8ms to 16ms for better reliability
- Disable terminal color requery on focus to prevent OSC responses bleeding into input stream
- Add improved heuristics for paste detection (retro_chars >= 2)
@AdsQnn AdsQnn force-pushed the fix/paste-burst-option-and-disable-color-query-clean branch from 0ad7ae6 to d967912 Compare October 27, 2025 08:51
@AdsQnn
Copy link
Author

AdsQnn commented Oct 27, 2025

How the flag and optimization work

Runtime code:

const ENABLE_COLOR_REQUERY: bool = false;  // Value known at compile-time

pub fn requery_default_colors() {
    if ENABLE_COLOR_REQUERY {  // Compiler sees: if false
        imp::requery_default_colors();  // This code NEVER runs
    }
}

What the Rust compiler does:

  1. Non-used code elimination – It sees if false { ... }
  2. Removes the entire block inside the if
  3. What’s left is an empty function:
pub fn requery_default_colors() {
    // nothing – empty function
}
  1. Inlining – It can further optimize:

// tui.rs calls:
crate::terminal_palette::requery_default_colors();

// Compiler replaces it with an empty function → the call disappears


Final effect

  • ✅ Zero overhead – as if the function didn’t exist
  • ✅ Code will not run – removed at compile-time
  • ✅ No “unused code” warnings – function “exists” in the code
  • ✅ Easy to toggle – just set false → true and it works

💡 Summary

This is better than commenting code – the compiler guarantees that it won’t run,
and you don’t have to comment/uncomment things in multiple places!

Ah, now I get why I sent just the paste burst earlier :) — I thought you’d be handling the PR with fixes and adjusting the whole structure yourselves. I figured my part was just to get the credit and that it would later be merged into the main branch automatically, so I didn’t want to interfere or add anything on my own :)
Because I got a message saying that the changes from the previous PR were too large, and that I should only send the paste burst to get the credit — I just misinterpreted that.

I was thinking I’d get some initial feedback on how to handle the commented-out line — I didn’t want to add any logic on my own to avoid introducing confusion.
At the moment, I can’t fully engage in the project and only work on it whenever I have some spare time.
So, apologies for any complications — it’s just that what might seem obvious to you may not be so obvious to me.
I also didn’t want to make any abrupt changes, since most of my edits are more like suggestions that still need testing and feedback.
I’m also not entirely sure what’s planned for future implementation and what implications it might have.

I’m hoping it finally goes through and you can be done with it once and for all :)

@AdsQnn
Copy link
Author

AdsQnn commented Oct 27, 2025

Ah, the tests passed successfully earlier, but I updated the branch to check another issue — it needs to be run again now.
I won’t touch anything else

@dylan-hurd-oai
Copy link
Collaborator

Going to cc @joshka-oai here since he is coordinating a refactor of related logic here

@AdsQnn
Copy link
Author

AdsQnn commented Oct 28, 2025

That's very good — so things don't get messy in the long run :)

Though here it’s mainly about blocking this function. The paste burst itself behaved similarly in both cases — with bracketed paste disabled, there wasn’t any significant difference. Only the API.

Practically speaking: without bracketed paste mode, reliable cross-platform handling of large paste operations is extremely difficult — especially when dealing with both Windows and Unix together.
Without it, paste bursts work only “by luck” — in small data cases or very simple scenarios. In reality, almost no popular CLI handles this well across platforms without bracketed paste support.

Reason: without bracketed paste, it’s nearly impossible to reliably distinguish a paste burst from normal user input across different systems (e.g., Unix uses EAGAIN, while Windows doesn’t have an equivalent).

In the mode where you have DisableBracketedPaste enabled and you’re reading raw bytes (which is required to make it work reliably and prevent the buffer from hanging), you take full responsibility for everything the terminal/OS normally handles: decoding, escape sequence parsing, event mapping, and signal processing.

I’m writing this as a note for the future, in case anyone else decides to wrestle with this problem.

@joshka-oai
Copy link
Collaborator

joshka-oai commented Oct 31, 2025

There's an alternate fix for the color queries that addresses this at the crossterm level over in #5935. The goal there is to move this down a level with some other similar things. This might be complementary to that (as changing the timing of when these queries happen is a good idea), so it's likely not wasted effort. I probably won't get to looking at this PR until later next week (or the week after if other priorities take my time), but it seems at a high level to possibly be complementary.

@AdsQnn
Copy link
Author

AdsQnn commented Oct 31, 2025

Ok, no need to approve it — that’s not the point. Maybe you have a different idea on how to approach this. I just wanted to show where the cause might be, so you can verify that it works. I also wanted to report as many observations as possible that I noticed during this process. I hope I didn’t cause more confusion than help :)

I tried to do this without touching the code — just to point out the possible cause, so you could decide how to proceed yourselves :)

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