Fix terminal chat auto-expand for commands without visible output#290482
Fix terminal chat auto-expand for commands without visible output#290482meganrogge merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where terminal chat incorrectly auto-expands output for commands with no visible output (like sleep 3). The fix adjusts the threshold for detecting "real" terminal output from more than 1 data event to more than 4 data events.
Changes:
- Increased the data event threshold from > 1 to > 4 to distinguish between shell integration sequences (which fire ~4 events) and actual command output
- Updated comments to clarify that shell integration sequences fire multiple times per command (PromptStart, CommandStart, CommandExecuted, CommandFinished, etc.)
| return receivedDataCount > 1; | ||
| // Shell integration sequences fire multiple times per command (PromptStart, CommandStart, | ||
| // CommandExecuted, CommandFinished, etc.), so we need a higher threshold | ||
| return receivedDataCount > 4; |
There was a problem hiding this comment.
Consider extracting the magic number 4 into a named constant with documentation. The threshold value is empirically determined based on shell integration sequences firing (PromptStart, CommandStart, CommandExecuted, CommandFinished), but this isn't immediately obvious from the code. A constant like SHELL_INTEGRATION_SEQUENCE_COUNT = 4 with a comment explaining why this specific value was chosen would make the code more maintainable and prevent future changes from inadvertently breaking this logic.
fix #290480
Shell integration sequences (PromptStart, CommandStart, CommandExecuted, CommandFinished) fire multiple data events even for commands with no visible output. The old threshold of > 1 was too low, causing
hasRealOutputto return true for sleep 2 since it receives ~4 SI sequence events. Raising the threshold to > 4 means only commands with actual ongoing output (like progress bars) will trigger the fallback, while silent long-running commands like sleep won't falsely auto-expand.cc @Tyriar