feat(tui): expose stream chunk timeout config

[cyq1017]

Problem

• Slow local or compatible model servers can go longer than the current stream chunk idle timeout before sending another SSE chunk. When that happens, CodeWhale closes the turn even though the server may still be working.

Change

• Add [tui].stream_chunk_timeout_secs with the existing 300 second default.
• Wire the value into the engine stream loop through the shared timeout environment.
• Expose the setting through /config stream_chunk_timeout_secs, persisted config, and the config view.
• Clamp accepted values to 1..=3600 seconds, with 0 meaning the default.

Verification

• cargo test -p codewhale-tui stream_chunk --all-features --locked -- --nocapture
• cargo check -p codewhale-tui --all-features --locked
• cargo fmt --all -- --check
• git diff --check origin/main..HEAD

Closes #2365

Greptile Summary

This PR exposes the existing SSE stream-chunk idle timeout as a user-configurable setting ([tui].stream_chunk_timeout_secs, default 300 s, clamped to 1–3600 s). The timeout is wired from config → App → EngineConfig → turn loop at startup, and can be updated live via /config stream_chunk_timeout_secs or the config view without a restart.

• The old ad-hoc stream_chunk_timeout_secs() env-reader in streaming.rs is removed; the value now travels through EngineConfig.stream_chunk_timeout and is mutable via the new Op::SetStreamChunkTimeout op.
• A new ConfigSection::Network variant is added to the config view so the setting lands in a logical group rather than under Display.
• The show_single_setting path for this key reloads config from disk instead of reading app.stream_chunk_timeout_secs, causing /config stream_chunk_timeout_secs (query) to report a stale value after a session-only change.

Confidence Score: 4/5

The core timeout wiring is correct and the new setting takes effect immediately for subsequent turns. One path is wrong: querying the setting after a session-only change shows the on-disk value, not the live value.

All the plumbing — EngineConfig, Op dispatch, App state initialization, config view, and persist path — is correct. The one defect is in show_single_setting: it reloads Config from disk instead of reading app.stream_chunk_timeout_secs, so the query output contradicts the session-only confirmation the user just saw.

crates/tui/src/commands/config.rs — the show_single_setting branch for stream_chunk_timeout_secs reads from disk instead of live app state

Important Files Changed

Filename	Overview
crates/tui/src/commands/config.rs	Adds stream_chunk_timeout_secs get/set/persist command handling; show_single_setting reads from disk instead of app state, causing stale query results after session-only changes
crates/tui/src/config.rs	Adds DEFAULT/MIN/MAX/ENV constants and stream_chunk_timeout_secs() resolver with clamping, zero-maps-to-default, and env fallback; well-tested
crates/tui/src/core/engine.rs	Adds stream_chunk_timeout field to EngineConfig and handles Op::SetStreamChunkTimeout to live-update the engine's idle timeout
crates/tui/src/core/engine/turn_loop.rs	Replaces the inline env-read with stream_chunk_timeout_budget() that reads from EngineConfig; straightforward wiring
crates/tui/src/tui/app.rs	Adds stream_chunk_timeout_secs to App struct and UpdateStreamChunkTimeout(u64) action variant; correctly initialized from config at startup
crates/tui/src/tui/views/mod.rs	Adds a new ConfigSection::Network variant and places stream_chunk_timeout_secs there in the config view, with hint text

Sequence Diagram

sequenceDiagram
    participant User
    participant ConfigCmd as /config command
    participant App
    participant UI as ui.rs
    participant Engine
    participant TurnLoop as turn_loop

    User->>ConfigCmd: /config stream_chunk_timeout_secs 90
    ConfigCmd->>App: "app.stream_chunk_timeout_secs = 90"
    ConfigCmd-->>UI: AppAction::UpdateStreamChunkTimeout(90)
    UI->>Engine: "Op::SetStreamChunkTimeout { timeout_secs: 90 }"
    Engine->>Engine: "config.stream_chunk_timeout = Duration::from_secs(90)"

    Note over TurnLoop: Next turn uses updated value
    TurnLoop->>Engine: "stream_chunk_timeout_budget(&self.config)"
    Engine-->>TurnLoop: (90, Duration::from_secs(90))

    User->>ConfigCmd: /config stream_chunk_timeout_secs (query)
    Note over ConfigCmd: reads Config::load() from disk,<br/>not app.stream_chunk_timeout_secs,<br/>shows 300 instead of 90

Loading

_{Reviews (4): Last reviewed commit: "fix(tui): keep config sections grouped" | Re-trigger Greptile}

[gemini-code-assist]

Code Review

This pull request introduces a new stream_chunk_timeout_secs configuration option to manage SSE stream stall timeouts, including support for persisting, setting, and displaying this value in the TUI. Feedback on these changes highlights critical safety and performance concerns: mutating the environment dynamically with std::env::set_var in a multi-threaded context is unsafe and should be replaced by passing the timeout directly; casting u64 to i64 using as risks silent overflow and should use safe conversion; and loading the configuration synchronously on the UI thread can cause lag, so the configuration should be loaded once and reused.

[gemini-code-assist]

Calling std::env::set_var in a multi-threaded environment is inherently unsafe and can lead to undefined behavior or crashes (such as segmentation faults) if other threads are concurrently reading the environment (e.g., via std::env::var or standard library/C library calls).

Since this is a TUI application running a background engine on a multi-threaded Tokio runtime, mutating the environment at runtime (especially when the user dynamically changes settings or when spawning the engine) is highly risky.

Recommendation:
Instead of passing the timeout configuration via an environment variable, refactor the engine/SSE client to accept the timeout value directly from the Config or EngineConfig struct. This avoids the need for unsafe environment mutation entirely.

[gemini-code-assist]

Calling std::env::set_var in a multi-threaded environment is inherently unsafe and can lead to undefined behavior or crashes (such as segmentation faults) if other threads are concurrently reading the environment (e.g., via std::env::var or standard library/C library calls).

Since this is a TUI application running a background engine on a multi-threaded Tokio runtime, mutating the environment at runtime (especially when the user dynamically changes settings or when spawning the engine) is highly risky.

Recommendation:
Instead of passing the timeout configuration via an environment variable, refactor the engine/SSE client to accept the timeout value directly from the Config or EngineConfig struct. This avoids the need for unsafe environment mutation entirely.

[gemini-code-assist]

Casting value from u64 to i64 using as i64 can silently overflow and result in a negative integer if value exceeds i64::MAX. Since persist_tui_integer_key is a public helper function, it could be reused in the future for other configuration keys where the value might exceed i64::MAX.

Consider using try_into() to safely convert the value, or adding a check to ensure it fits within i64 bounds.

    let val_i64 = i64::try_from(value).context("value exceeds maximum supported integer size")?;
    tui_table.insert(key.to_string(), toml::Value::Integer(val_i64));

[gemini-code-assist]

Calling Config::load synchronously inside the initialization of a single ConfigRow performs disk I/O on the UI thread. If multiple rows in the ConfigView load the configuration independently, this can cause noticeable lag or stuttering in the TUI.

Recommendation:
Load the Config once at the beginning of ConfigView::new_for_app (or the relevant view constructor) and reuse that loaded instance across all rows that need to read from the saved configuration.

[Hmbown]

Thanks @cyq1017. I reviewed this during the v0.8.50 triage pass and am not harvesting it yet. The user-facing knob is plausible, but this needs one more design pass before it is release-safe.

Concrete next steps:

• Avoid mutating DEEPSEEK_STREAM_IDLE_TIMEOUT_SECS from live TUI/config paths. Thread the resolved timeout through config/engine/streaming state instead of changing process env in a multi-threaded runtime.
• Avoid synchronous Config::load(...) from the config view row rendering path; load once or reuse already-held app/config state so opening settings cannot hitch on disk I/O.
• Replace the value as i64 TOML integer cast with a checked conversion or a range-limited type before serialization.
• Add a focused runtime test that proves the configured timeout is the one used by the SSE idle-timeout path, not just that it appears in config/UI.

Once those are addressed, this could be a good follow-up for #2365 / stalled stream tuning. For v0.8.50 I am keeping it out because #2487 is already broad and we should not add a new timeout-control surface with these unresolved runtime risks.

[Hmbown]

Thanks for the quick follow-up, @cyq1017. I rechecked the new a71a6b6 commit. This is much closer: the timeout now threads through EngineConfig / Op::SetStreamChunkTimeout instead of mutating process env at runtime, the TOML integer write uses a checked conversion, and there is a runtime-budget test for the SSE idle path.

I am still keeping it out of #2504 for now because a couple of release-polish items remain:

• The config view still does synchronous Config::load(...) while constructing the stream_chunk_timeout_secs row. Please use app.stream_chunk_timeout_secs or a config value loaded once for the view instead of doing disk I/O from row construction.
• When the user enters 0, the command response still says stream_chunk_timeout_secs = 0, while the active value becomes the resolved default (300). Please show the effective value, or make the message explicit like 0 (default 300).
• If you touch the view row, please also move this out of Display; it is a stream/network timeout rather than a display setting.

Once those are cleaned up and Windows CI is green, this looks like a solid follow-up for #2365.

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.