Skip to content

Commit 65fe683

Browse files
theahuranori-agent
andauthored
perf(tui): stop polling nori-skillsets; cache version (#444)
## Summary - `spawn_system_info_worker` in `codex-rs/tui/src/app/mod.rs` was firing a full `SystemInfo` refresh every 5 seconds via `recv_timeout` fall-through, which in turn spawned `nori-skillsets --version` and `nori-skillsets list-active` as Node.js subprocesses. With several concurrent `nori` sessions this produced sustained CPU load just to paint the footer (see discovery notes below). Switch to blocking `recv()` so refreshes only happen on explicit `request_system_info_refresh()` calls. - Cache `nori-skillsets --version` in a process-wide `OnceLock` (`NORI_VERSION_CACHE`) in `codex-rs/tui/src/system_info.rs`. The installed CLI version is stable for the lifetime of the TUI process, so the subprocess runs at most once per session. `list-active` is still re-invoked on every refresh. - Document the new event-driven refresh model and version caching in `codex-rs/tui/docs.md`. Refreshes are still triggered on: startup, user message submit, task completion, effective cwd changes (debounced 500ms by `EffectiveCwdTracker`), and skillset install/switch success. ### Tradeoff An external `nori-skillsets switch` run in another terminal while the TUI is idle will not be reflected in the footer until the user does something that triggers a refresh. Discussed and accepted. ### Impact - Per-session baseline load: ~0.4 `nori-skillsets` spawns/sec → **0** when idle. - Per-refresh cost: 2 subprocesses → 1. ## Test plan - [x] `cargo test -p nori-tui` — 1199 passed, 0 failed - [x] `just fmt` + `just fix -p nori-tui` — clean - [x] `cargo build --bin nori` — success - [x] `cargo test -p tui-pty-e2e` — all suites pass - [x] End-to-end: launched `nori --agent elizacp` under tmux; TUI rendered, `›` prompt appeared, footer populated with active skillset and cached version string; user submit triggered a refresh that updated the footer. No panics. Co-authored-by: Nori <contact@tilework.tech>
1 parent 959dedf commit 65fe683

3 files changed

Lines changed: 40 additions & 25 deletions

File tree

codex-rs/tui/docs.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,22 @@ Two collection methods are provided:
227227

228228
The first-message is obtained from `ChatWidget::first_prompt_text()`, which stores the text of the first submitted prompt. This flows through `SystemInfoRefreshRequest` to the background worker, enabling accurate transcript matching when multiple sessions exist in the same project directory.
229229

230+
**Refresh model:**
231+
232+
`spawn_system_info_worker` runs a background thread that blocks on its request channel: a refresh happens only when a `SystemInfoRefreshRequest` is sent via `request_system_info_refresh()`. There is no periodic polling. Refreshes are triggered on:
233+
234+
1. Startup (explicit initial refresh in `App::run()`)
235+
2. User message submit (`chatwidget/user_input.rs`)
236+
3. Task completion (`chatwidget/event_handlers.rs`)
237+
4. Effective cwd change observed from tool-call directories or file-change paths (debounced 500ms by `EffectiveCwdTracker`)
238+
5. Successful skillset install or switch (`app/event_handling.rs`)
239+
240+
This means an external change (e.g., the user runs `nori-skillsets switch` in another terminal) will not be reflected in the footer until the next event-driven refresh. Footer staleness is bounded by user activity, not by wall-clock time.
241+
242+
**Version caching:**
243+
244+
`get_nori_version()` shells out to `nori-skillsets --version` (or `nori-ai --version` as a legacy fallback) and caches the result in a process-wide `OnceLock` (`NORI_VERSION_CACHE`). The installed CLI version is stable for the lifetime of a TUI process, so the subprocess runs at most once per session. Only `nori-skillsets list-active` is re-invoked on every refresh.
245+
230246
**`/diff` Slash Command** (`get_git_diff.rs`):
231247

232248
The `/diff` handler in `key_handling.rs` resolves the effective CWD from the `effective_cwd_tracker` (falling back to `config.cwd`) and passes it to `get_git_diff()`. This ensures `/diff` works correctly in git worktrees and directories different from the process launch directory. All git commands in `get_git_diff.rs` use `.current_dir()` when a directory is provided.

codex-rs/tui/src/app/mod.rs

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -351,13 +351,8 @@ impl App {
351351
let upgrade_version = crate::updates::get_upgrade_version(&config);
352352

353353
let (system_info_tx, system_info_rx) = mpsc::channel();
354-
let _system_info_worker = Self::spawn_system_info_worker(
355-
system_info_rx,
356-
app_event_tx.clone(),
357-
config.cwd.clone(),
358-
config.model.clone(),
359-
initial_prompt.clone(),
360-
);
354+
let _system_info_worker =
355+
Self::spawn_system_info_worker(system_info_rx, app_event_tx.clone());
361356

362357
let mut app = Self {
363358
app_event_tx,
@@ -556,31 +551,22 @@ impl App {
556551
fn spawn_system_info_worker(
557552
system_info_rx: mpsc::Receiver<SystemInfoRefreshRequest>,
558553
app_event_tx: AppEventSender,
559-
initial_dir: PathBuf,
560-
initial_model: String,
561-
initial_first_message: Option<String>,
562554
) -> thread::JoinHandle<()> {
563555
thread::spawn(move || {
564-
let mut last_request = SystemInfoRefreshRequest {
565-
dir: initial_dir,
566-
model: Some(initial_model),
567-
first_message: initial_first_message,
568-
};
569-
loop {
570-
match system_info_rx.recv_timeout(Duration::from_secs(5)) {
571-
Ok(request) => last_request = request,
572-
Err(mpsc::RecvTimeoutError::Timeout) => {}
573-
Err(mpsc::RecvTimeoutError::Disconnected) => break,
574-
}
575-
576-
let agent_kind = last_request
556+
// Refresh only when a request arrives. The initial refresh is
557+
// scheduled explicitly via `request_system_info_refresh` at
558+
// startup, and subsequent refreshes are driven by user actions
559+
// (message submit, task completion, tool-call cwd changes,
560+
// skillset install/switch). No periodic polling.
561+
while let Ok(request) = system_info_rx.recv() {
562+
let agent_kind = request
577563
.model
578564
.as_ref()
579565
.and_then(|model| codex_acp::AgentKind::from_slug(model));
580566
let info = crate::system_info::SystemInfo::collect_for_directory_with_message(
581-
&last_request.dir,
567+
&request.dir,
582568
agent_kind,
583-
last_request.first_message.as_deref(),
569+
request.first_message.as_deref(),
584570
);
585571
app_event_tx.send(AppEvent::SystemInfoRefreshed(info));
586572
}

codex-rs/tui/src/system_info.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use codex_acp::AgentKind;
22
use codex_acp::TranscriptLocation;
33
use std::env;
44
use std::process::Command;
5+
use std::sync::OnceLock;
56

67
/// Indicates which command was used to detect the Nori version.
78
/// This affects the UI display text.
@@ -161,7 +162,19 @@ fn discover_transcript(
161162
})
162163
}
163164

165+
/// Cached result of `nori-skillsets --version` / `nori-ai --version`.
166+
///
167+
/// The installed CLI version is stable for the lifetime of a TUI process, so
168+
/// we detect it once and reuse the result on every subsequent `SystemInfo`
169+
/// refresh. This avoids spawning a heavyweight Node.js subprocess on every
170+
/// footer update.
171+
static NORI_VERSION_CACHE: OnceLock<(Option<String>, Option<NoriVersionSource>)> = OnceLock::new();
172+
164173
fn get_nori_version() -> (Option<String>, Option<NoriVersionSource>) {
174+
NORI_VERSION_CACHE.get_or_init(detect_nori_version).clone()
175+
}
176+
177+
fn detect_nori_version() -> (Option<String>, Option<NoriVersionSource>) {
165178
// Try nori-skillsets first (new installer)
166179
if let Ok(output) = Command::new("nori-skillsets").arg("--version").output()
167180
&& output.status.success()

0 commit comments

Comments
 (0)