feat(tui): expose stream chunk timeout config#2778
Conversation
Harvested from PR #2507 by @cyq1017. Reported by @mserrano11 in #2365. Co-authored-by: cyq1017 <61975706+cyq1017@users.noreply.github.com>
There was a problem hiding this comment.
Hmbown has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Code Review
This pull request introduces the [tui].stream_chunk_timeout_secs configuration option and a corresponding /config stream_chunk_timeout_secs command, allowing users to customize the SSE stream idle timeout. It maintains backward compatibility with the legacy DEEPSEEK_STREAM_IDLE_TIMEOUT_SECS environment variable and adds persistence support, UI updates, and documentation. Feedback highlights two key issues: first, live updates to the timeout via /config are not propagated to the active DeepSeekClient instance, rendering session-only updates ineffective; second, the setting is incorrectly marked as ConfigScope::Session instead of ConfigScope::Saved in the TUI view, which prevents users from persisting it through the TUI configuration editor.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| Op::SetStreamChunkTimeout { timeout_secs } => { | ||
| self.config.stream_chunk_timeout = Duration::from_secs(timeout_secs); | ||
| let _ = self | ||
| .tx_event | ||
| .send(Event::status(format!( | ||
| "Stream chunk timeout set to {timeout_secs}s" | ||
| ))) | ||
| .await; | ||
| } |
There was a problem hiding this comment.
Live Update Not Propagated to DeepSeekClient
When the /config stream_chunk_timeout_secs <value> command is executed, it sends Op::SetStreamChunkTimeout to the engine, which correctly updates self.config.stream_chunk_timeout. However, the active DeepSeekClient instance (which actually performs the network requests and handles the SSE stream) is not updated with this new timeout.
Because DeepSeekClient is constructed with a fixed stream_idle_timeout at startup (or whenever it is instantiated using the global Config), any live session-only changes to the timeout will not affect the inner SSE byte-stream idle timeout (used in crates/tui/src/client/chat.rs).
For example, if a user increases the timeout to 600 seconds to accommodate a slow local server, the inner SSE stream will still timeout and fail after the default 300 seconds, rendering the live session update ineffective.
Suggested Fix
Consider one of the following approaches:
- Pass the timeout dynamically from the engine's
self.config.stream_chunk_timeoutas an argument to the streaming/chat request methods onDeepSeekClientinstead of storing it as a fixed field in the client. - Introduce interior mutability (e.g.,
Arc<std::sync::atomic::AtomicU64>or a thread-safe setter) toDeepSeekClientso that its timeout can be updated dynamically whenOp::SetStreamChunkTimeoutis processed.
| section: ConfigSection::Network, | ||
| key: "stream_chunk_timeout_secs".to_string(), | ||
| value: app.stream_chunk_timeout_secs.to_string(), | ||
| editable: true, | ||
| scope: ConfigScope::Session, | ||
| }, |
There was a problem hiding this comment.
Inconsistent Config Scope in TUI View
The stream_chunk_timeout_secs setting is defined here with scope: ConfigScope::Session. However, this is a standard persistent configuration option in config.toml (under [tui]), and the /config command fully supports persisting it to disk via --save (using persist_tui_integer_key).
Marking it as ConfigScope::Session in the TUI config view prevents it from being saved to the persistent configuration file when the user saves their settings from the TUI config editor interface. It should be changed to ConfigScope::Saved to allow full persistence support from the TUI.
section: ConfigSection::Network,
key: "stream_chunk_timeout_secs".to_string(),
value: app.stream_chunk_timeout_secs.to_string(),
editable: true,
scope: ConfigScope::Saved,
},
Summary
[tui].stream_chunk_timeout_secsplus/config stream_chunk_timeout_secs, including live session updates,--save,0-> default handling, and a Network row in the config viewEngineConfigandDeepSeekClientso both the outer turn-loop timeout and inner SSE byte-stream timeout use the same value, while preservingDEEPSEEK_STREAM_IDLE_TIMEOUT_SECSas a fallbackCredit
Harvested from PR #2507 by @cyq1017.
Reported by @mserrano11 in #2365.
Verification
cargo test -p codewhale-tui --bin codewhale-tui --locked stream_chunk_timeout -- --nocapturecargo test -p codewhale-tui --bin codewhale-tui --locked client_stream_idle_timeout_uses_tui_config -- --nocapturecargo test -p codewhale-tui --bin codewhale-tui --locked config_view -- --nocapturecargo fmt --all -- --checkgit diff --check./scripts/release/check-versions.shpython3 scripts/check-coauthor-trailers.py --range codex/v0.9.0-stewardship..HEAD --check-authors