From 3fab37cbcefad0cb6a4ee8840a1b595a009434bf Mon Sep 17 00:00:00 2001 From: Ferrol Aderholdt Date: Tue, 21 Jul 2026 14:55:57 -0700 Subject: [PATCH 1/4] Fix false ERROR state from screen-scraping 'error' - Anchor generic_error regex to line-start structural shapes only (^error[:[] ^failed[:[] etc.) instead of bare substring matches. - Scope error detection to BaseKind::Other; agent sessions (Claude, Codex, LocalAgent) no longer flip to ERROR from screen text. - Add tests for anchored patterns and kind-scoped behavior. --- src/patterns.rs | 50 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/src/patterns.rs b/src/patterns.rs index b307296..896c92c 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -57,7 +57,9 @@ impl PatternMatcher { ) .unwrap(), generic_waiting: Regex::new(r"\[y/n\]|\[Y/n\]|\(yes/no\)|Press Enter").unwrap(), - generic_error: Regex::new(r"(?i)error:|failed:|panic!|fatal:|command not found") + generic_error: Regex::new( + r"(?i)^error[:\[]|^failed[:\[]|^panic!|^[Ff]atal:|command not found", + ) .unwrap(), nctx_re: Regex::new(r"\bn_ctx\s*=\s*(\d+)").unwrap(), @@ -96,7 +98,9 @@ impl PatternMatcher { if self.generic_waiting.is_match(line) { return Some(SessionState::Waiting); } - if self.generic_error.is_match(line) { + // Generic error detection only for shell/custom sessions — agents + // report ERROR via process exit, JSONL records, or IPC. + if matches!(base, BaseKind::Other) && self.generic_error.is_match(line) { return Some(SessionState::Error); } match base { @@ -294,16 +298,50 @@ mod tests { fn generic_waiting_and_error_take_precedence_for_all_session_kinds() { let matcher = PatternMatcher::new(); + // Generic waiting applies to all kinds for base in [BaseKind::Claude, BaseKind::Codex, BaseKind::Other] { assert_eq!( matcher.infer_state("Press Enter to continue", base), Some(SessionState::Waiting) ); - assert_eq!( - matcher.infer_state("fatal: command not found", base), - Some(SessionState::Error) - ); } + + // Generic error detection only applies to Other (shell/custom) + assert_eq!( + matcher.infer_state("fatal: command not found", BaseKind::Other), + Some(SessionState::Error) + ); + + // Agents do NOT trigger Error from screen-scraped "error" text + assert_ne!( + matcher.infer_state("fatal: command not found", BaseKind::Claude), + Some(SessionState::Error) + ); + assert_ne!( + matcher.infer_state("fatal: command not found", BaseKind::Codex), + Some(SessionState::Error) + ); + + // Codex discussing an error file is just Running, not Error + assert_eq!( + matcher.infer_state( + "fixed the error in ucp_tag_send.c", + BaseKind::Codex + ), + Some(SessionState::Running) + ); + + // Shell session: anchored "Error:" at start of line -> Error + assert_eq!( + matcher.infer_state("Error: connection refused", BaseKind::Other), + Some(SessionState::Error) + ); + + // Shell session: bare "error" mid-line does NOT match + assert_ne!( + matcher.infer_state("no error here", BaseKind::Other), + Some(SessionState::Error) + ); } #[test] From 003c313dc3f196186275f08c717c9e4d61e702b1 Mon Sep 17 00:00:00 2001 From: Ferrol Aderholdt Date: Tue, 21 Jul 2026 15:00:42 -0700 Subject: [PATCH 2/4] Fix Codex token count inflation - Report fresh input tokens (total minus cached) instead of raw cumulative, matching claude_log.rs behavior where cache reads are excluded from the displayed input count. Cache still prices into cost correctly. - Align tail() with claude_log accumulation pattern: track latest cumulative values in local acc_* variables and send one consolidated SessionStats per poll cycle instead of dispatching per-event. --- src/codex_log.rs | 89 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 80 insertions(+), 9 deletions(-) diff --git a/src/codex_log.rs b/src/codex_log.rs index 97d0eb0..2039e5b 100644 --- a/src/codex_log.rs +++ b/src/codex_log.rs @@ -139,8 +139,9 @@ fn parse_token_count(v: &serde_json::Value, model: &str, config: &Config) -> Opt + (cached_input_tokens as f64 / 1_000_000.0) * rate.cache_read + (output_tokens as f64 / 1_000_000.0) * rate.output; + let fresh_input = input_tokens.saturating_sub(cached_input_tokens); Some(TokenStats { - input_tokens, + input_tokens: fresh_input, output_tokens, context_tokens, total_cost_usd, @@ -155,6 +156,10 @@ async fn tail( ) { let mut offset: u64 = 0; let mut model = "unknown".to_string(); + let mut acc_input: u64 = 0; + let mut acc_output: u64 = 0; + let mut acc_cost: f64 = 0.0; + let mut context_tokens: u64 = 0; // Scan the first 20 lines (session_meta / first turn_context) for the model. if let Ok(content) = tokio::fs::read_to_string(path).await { @@ -181,6 +186,8 @@ async fn tail( break; } + let mut new_stats = false; + if let Ok(file) = tokio::fs::File::open(path).await { let mut file = file; if file.seek(std::io::SeekFrom::Start(offset)).await.is_ok() { @@ -220,13 +227,11 @@ async fn tail( } } if let Some(stats) = parse_token_count(&v, &model, config) { - if tx - .send(AppEvent::SessionStats { session_id, stats }) - .await - .is_err() - { - return; - } + acc_input = stats.input_tokens; + acc_output = stats.output_tokens; + acc_cost = stats.total_cost_usd; + context_tokens = stats.context_tokens; + new_stats = true; } } Err(_) => break, @@ -235,6 +240,18 @@ async fn tail( } } + if new_stats { + let stats = TokenStats { + input_tokens: acc_input, + output_tokens: acc_output, + total_cost_usd: acc_cost, + context_tokens, + }; + if tx.send(AppEvent::SessionStats { session_id, stats }).await.is_err() { + return; + } + } + sleep(Duration::from_millis(500)).await; } } @@ -299,7 +316,7 @@ mod tests { let config = crate::config::Config::default(); let stats = parse_token_count(&v, "unknown", &config).unwrap(); - assert_eq!(stats.input_tokens, 99975); + assert_eq!(stats.input_tokens, 41223); // fresh input: 99975 - 58752 cached assert_eq!(stats.output_tokens, 1358); assert_eq!(stats.context_tokens, 31619); // "unknown" model rate is 0.0, so cost should be 0. @@ -435,4 +452,58 @@ mod tests { let dir = sessions_dir(Some("/opt/codex-personal")).unwrap(); assert_eq!(dir, PathBuf::from("/opt/codex-personal/sessions")); } + + fn make_codex_token_event(input: u64, cached: u64, output: u64) -> serde_json::Value { + serde_json::json!({ + "type": "event_msg", + "payload": { + "type": "token_count", + "info": { + "total_token_usage": { + "input_tokens": input, + "cached_input_tokens": cached, + "output_tokens": output + }, + "last_token_usage": { + "input_tokens": input - cached + } + } + } + }) + } + + #[test] + fn cumulative_token_counts_not_inflated_by_naive_sum() { + let entry1 = make_codex_token_event(10_000, 2_000, 3_000); + let entry2 = make_codex_token_event(30_000, 8_000, 8_000); + let entry3 = make_codex_token_event(50_000, 15_000, 12_000); + + let config = Config::default(); + + let stats1 = parse_token_count(&entry1, "gpt-5", &config).unwrap(); + let _stats2 = parse_token_count(&entry2, "gpt-5", &config).unwrap(); + let stats3 = parse_token_count(&entry3, "gpt-5", &config).unwrap(); + + assert_eq!(stats1.input_tokens, 8_000); // 10k - 2k cached = 8k fresh + assert_eq!(stats3.input_tokens, 35_000); // 50k - 15k cached = 35k fresh + assert_eq!(stats3.output_tokens, 12_000); + + let naive_sum_input = stats1.input_tokens + _stats2.input_tokens + stats3.input_tokens; + assert!(naive_sum_input > stats3.input_tokens, "naive sum ({}) exceeds correct total", naive_sum_input); + } + + #[test] + fn cached_input_excluded_from_fresh_input_count() { + let v = make_codex_token_event(100_000, 60_000, 5_000); + let config = Config::default(); + let stats = parse_token_count(&v, "gpt-5.4-mini", &config).unwrap(); + + assert_eq!(stats.input_tokens, 40_000); // only fresh (non-cached) input + assert_eq!(stats.output_tokens, 5_000); + assert!(stats.total_cost_usd > 0.0); // cost still includes cache pricing + + let v2 = make_codex_token_event(10_000, 0, 500); + let stats2 = parse_token_count(&v2, "gpt-5.4-mini", &config).unwrap(); + assert_eq!(stats2.input_tokens, 10_000); + } } From ec5f75f44cd7d3b5acb543f9a1b768e4c4615b25 Mon Sep 17 00:00:00 2001 From: Ferrol Aderholdt Date: Tue, 21 Jul 2026 15:05:29 -0700 Subject: [PATCH 3/4] Fix Codex session scrollback in alternate screen mode When the TUI is on the alternate screen and content scrolls upward, capture the line leaving the top of the visible grid into output_lines for PageUp/Shift-Up scrollback. Gated to Claude/Codex sessions via captures_alt_scrollback() predicate; shell sessions unaffected. Dedup identical top lines to tolerate TUI repaints. --- src/session.rs | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/session.rs b/src/session.rs index 7f4428e..6e90597 100644 --- a/src/session.rs +++ b/src/session.rs @@ -92,6 +92,12 @@ impl SessionKind { .unwrap_or(false) } + /// True for TUI-based agent sessions whose transcript is useful as scrollback. + /// Full-repaint dashboards (htop, btop) would spew garbage if enabled here. + pub fn captures_alt_scrollback(&self) -> bool { + matches!(self, SessionKind::Claude | SessionKind::Codex) + } + pub(crate) fn custom_base_name_pub(&self) -> Option<&str> { self.custom_base_name() } @@ -360,8 +366,45 @@ impl Session { /// for full-screen TUIs that repaint with byte-identical output. pub fn process_bytes(&mut self, data: &[u8]) -> bool { use std::hash::{Hash, Hasher}; + + // Capture top-line snapshot BEFORE processing, if this session kind + // supports alt-screen scrollback capture and we're on the alternate screen. + let prev_top = if self.kind.captures_alt_scrollback() + && self.screen.screen().alternate_screen() + { + self.screen.screen().contents().lines().next().map(|s| { + s.trim_end().trim_end_matches('\t').to_string() + }) + } else { + None + }; + self.bytes_since_last_tick += data.len(); self.screen.process(data); + + // After processing, check if content scrolled (top row changed) + if let Some(ref prev) = prev_top { + let current_top: Option = self.screen.screen().contents().lines().next().map(|s| { + s.trim_end().trim_end_matches('\t').to_string() + }); + + // If the top line changed, content scrolled upward. The old top row + // was pushed off-screen — capture it for scrollback. + if let Some(cur) = current_top { + if cur != *prev && !prev.is_empty() { + // Dedupe: skip if identical to last appended line + let dominated = self.output_lines.back().is_some_and(|last| *last == *prev); + if !dominated { + self.push_output_line(prev.clone()); + } + } + } else { + // Screen is now empty (left alternate screen?), capture the old top + if !prev.is_empty() && !self.output_lines.back().is_some_and(|last| *last == *prev) { + self.push_output_line(prev.clone()); + } + } + } // contents_formatted is exactly what the display path renders from, so // an unchanged hash means the next frame would be pixel-identical. let mut hasher = std::collections::hash_map::DefaultHasher::new(); @@ -965,4 +1008,40 @@ mod tests { ); } } + + #[test] + fn alt_screen_scrolled_lines_captured_in_output_lines() { + let mut s = Session::new(1, "codex".into(), SessionKind::Codex, "/tmp".into(), 5, 80, 100); + s.process_bytes(b"\x1b[?1049h\x1b[HLine A\r\nLine B\r\nLine C\r\nLine D\r\nLine E"); + assert!(s.output_lines.is_empty()); + + // Scroll up by 1: Line A leaves the top, everything shifts up + s.process_bytes(b"\x1b[1S"); + + assert_eq!(s.output_lines.len(), 1); + assert_eq!(s.output_lines.front().unwrap(), "Line A"); + + // Scroll up by 1 more: Line B leaves the top + s.process_bytes(b"\x1b[1S"); + + assert_eq!(s.output_lines.len(), 2); + assert_eq!(s.output_lines.get(1).unwrap(), "Line B"); + } + + #[test] + fn alt_screen_scrollback_deduplicates_identical_repaint() { + let mut s = Session::new(0, "codex".into(), SessionKind::Codex, "/tmp".into(), 3, 80, 100); + s.process_bytes(b"\x1b[?1049h\x1b[HHeader\r\nBody 1\r\nFooter"); + + // Repaint with same top line (TUI re-rendering) — should NOT add to output_lines + s.process_bytes(b"\x1b[HHeader\r\nBody 1\r\nFooter"); + assert!(s.output_lines.is_empty()); + } + + #[test] + fn shell_session_no_alt_scrollback() { + let mut s = Session::new(0, "shell".into(), SessionKind::Shell, "/tmp".into(), 3, 80, 100); + s.process_bytes(b"hello world\n"); + assert!(s.output_lines.is_empty()); + } } From 559e02353c456e84912f11cc1bb00ead801abc30 Mon Sep 17 00:00:00 2001 From: Ferrol Aderholdt Date: Tue, 21 Jul 2026 15:20:38 -0700 Subject: [PATCH 4/4] formatting --- src/codex_log.rs | 12 ++++++++-- src/patterns.rs | 5 +--- src/session.rs | 62 +++++++++++++++++++++++++++++++++++------------- 3 files changed, 57 insertions(+), 22 deletions(-) diff --git a/src/codex_log.rs b/src/codex_log.rs index 2039e5b..97522c8 100644 --- a/src/codex_log.rs +++ b/src/codex_log.rs @@ -247,7 +247,11 @@ async fn tail( total_cost_usd: acc_cost, context_tokens, }; - if tx.send(AppEvent::SessionStats { session_id, stats }).await.is_err() { + if tx + .send(AppEvent::SessionStats { session_id, stats }) + .await + .is_err() + { return; } } @@ -489,7 +493,11 @@ mod tests { assert_eq!(stats3.output_tokens, 12_000); let naive_sum_input = stats1.input_tokens + _stats2.input_tokens + stats3.input_tokens; - assert!(naive_sum_input > stats3.input_tokens, "naive sum ({}) exceeds correct total", naive_sum_input); + assert!( + naive_sum_input > stats3.input_tokens, + "naive sum ({}) exceeds correct total", + naive_sum_input + ); } #[test] diff --git a/src/patterns.rs b/src/patterns.rs index 896c92c..7bd2c5d 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -324,10 +324,7 @@ mod tests { // Codex discussing an error file is just Running, not Error assert_eq!( - matcher.infer_state( - "fixed the error in ucp_tag_send.c", - BaseKind::Codex - ), + matcher.infer_state("fixed the error in ucp_tag_send.c", BaseKind::Codex), Some(SessionState::Running) ); diff --git a/src/session.rs b/src/session.rs index 6e90597..7bf238b 100644 --- a/src/session.rs +++ b/src/session.rs @@ -369,24 +369,30 @@ impl Session { // Capture top-line snapshot BEFORE processing, if this session kind // supports alt-screen scrollback capture and we're on the alternate screen. - let prev_top = if self.kind.captures_alt_scrollback() - && self.screen.screen().alternate_screen() - { - self.screen.screen().contents().lines().next().map(|s| { - s.trim_end().trim_end_matches('\t').to_string() - }) - } else { - None - }; + let prev_top = + if self.kind.captures_alt_scrollback() && self.screen.screen().alternate_screen() { + self.screen + .screen() + .contents() + .lines() + .next() + .map(|s| s.trim_end().trim_end_matches('\t').to_string()) + } else { + None + }; self.bytes_since_last_tick += data.len(); self.screen.process(data); // After processing, check if content scrolled (top row changed) if let Some(ref prev) = prev_top { - let current_top: Option = self.screen.screen().contents().lines().next().map(|s| { - s.trim_end().trim_end_matches('\t').to_string() - }); + let current_top: Option = self + .screen + .screen() + .contents() + .lines() + .next() + .map(|s| s.trim_end().trim_end_matches('\t').to_string()); // If the top line changed, content scrolled upward. The old top row // was pushed off-screen — capture it for scrollback. @@ -400,7 +406,7 @@ impl Session { } } else { // Screen is now empty (left alternate screen?), capture the old top - if !prev.is_empty() && !self.output_lines.back().is_some_and(|last| *last == *prev) { + if !prev.is_empty() && self.output_lines.back().is_none_or(|last| *last != *prev) { self.push_output_line(prev.clone()); } } @@ -1011,7 +1017,15 @@ mod tests { #[test] fn alt_screen_scrolled_lines_captured_in_output_lines() { - let mut s = Session::new(1, "codex".into(), SessionKind::Codex, "/tmp".into(), 5, 80, 100); + let mut s = Session::new( + 1, + "codex".into(), + SessionKind::Codex, + "/tmp".into(), + 5, + 80, + 100, + ); s.process_bytes(b"\x1b[?1049h\x1b[HLine A\r\nLine B\r\nLine C\r\nLine D\r\nLine E"); assert!(s.output_lines.is_empty()); @@ -1030,7 +1044,15 @@ mod tests { #[test] fn alt_screen_scrollback_deduplicates_identical_repaint() { - let mut s = Session::new(0, "codex".into(), SessionKind::Codex, "/tmp".into(), 3, 80, 100); + let mut s = Session::new( + 0, + "codex".into(), + SessionKind::Codex, + "/tmp".into(), + 3, + 80, + 100, + ); s.process_bytes(b"\x1b[?1049h\x1b[HHeader\r\nBody 1\r\nFooter"); // Repaint with same top line (TUI re-rendering) — should NOT add to output_lines @@ -1040,7 +1062,15 @@ mod tests { #[test] fn shell_session_no_alt_scrollback() { - let mut s = Session::new(0, "shell".into(), SessionKind::Shell, "/tmp".into(), 3, 80, 100); + let mut s = Session::new( + 0, + "shell".into(), + SessionKind::Shell, + "/tmp".into(), + 3, + 80, + 100, + ); s.process_bytes(b"hello world\n"); assert!(s.output_lines.is_empty()); }