refactor: improve code quality and reduce duplication

- Rename slurm_job_id to scheduler_job_id for scheduler-agnostic naming
- Add tracing crate and replace eprintln! with structured logging
- Use once_cell::Lazy for pre-compiled regex patterns in types.rs
- Extract shared duration formatting functions to charmer-parsers
- Remove redundant parser implementations in failure modules
- Consolidate time parsing with parse_duration_secs()

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: jayhesselberth

Cargo.toml

@@ -35,6 +35,7 @@ camino = { version = "1.1", features = ["serde1"] }
 
 # Regex for parsing
 regex = "1.11"
+once_cell = "1.20"
 
 # Base64 for snakemake filenames
 base64 = "0.22"
@@ -43,6 +44,9 @@ base64 = "0.22"
 thiserror = "2.0"
 miette = { version = "7.4", features = ["fancy"] }
 
+# Logging
+tracing = "0.1"
+
 # Clipboard
 arboard = "3"
 

crates/charmer-core/Cargo.toml

@@ -11,3 +11,4 @@ base64.workspace = true
 thiserror.workspace = true
 camino.workspace = true
 chrono.workspace = true
+tracing.workspace = true

crates/charmer-core/src/metadata.rs

@@ -138,7 +138,7 @@ pub fn scan_metadata_dir(working_dir: &Utf8Path) -> Result<Vec<SnakemakeJob>, Me
             Ok(job) => jobs.push(job),
             Err(e) => {
                 // Log but continue on parse errors
-                eprintln!("Warning: Failed to parse metadata file {}: {}", path, e);
+                tracing::warn!("Failed to parse metadata file {}: {}", path, e);
             }
         }
     }

crates/charmer-lsf/Cargo.toml

@@ -9,3 +9,4 @@ charmer-parsers.workspace = true
 chrono.workspace = true
 thiserror.workspace = true
 tokio.workspace = true
+tracing.workspace = true

crates/charmer-lsf/src/bjobs.rs

@@ -87,7 +87,7 @@ pub async fn query_bjobs(job_name_filter: Option<&str>) -> Result<Vec<LsfJob>, B
         }
         match parse_bjobs_line(line) {
             Ok(job) => jobs.push(job),
-            Err(e) => eprintln!("Warning: {}", e),
+            Err(e) => tracing::warn!("Failed to parse bjobs line: {}", e),
         }
     }
 

crates/charmer-lsf/src/failure.rs

@@ -2,7 +2,10 @@
 //!
 //! Query detailed failure information and provide actionable suggestions.
 
-use charmer_parsers::run_command_allow_failure;
+use charmer_parsers::{
+    format_duration, format_duration_lsf, parse_duration_secs, parse_memory_mb,
+    run_command_allow_failure, MemoryFormat,
+};
 use thiserror::Error;
 use tokio::process::Command;
 
@@ -193,19 +196,19 @@ fn parse_bhist_output(job_id: &str, output: &str) -> Result<FailureAnalysis, Fai
 
         // Look for memory info
         if line.contains("MAX MEM:") {
-            max_mem_mb = parse_lsf_memory(line, "MAX MEM:");
+            max_mem_mb = parse_lsf_memory_from_line(line, "MAX MEM:");
         }
         if line.contains("MEMLIMIT") || line.contains("MEM LIMIT:") {
-            mem_limit_mb =
-                parse_lsf_memory(line, "MEMLIMIT").or_else(|| parse_lsf_memory(line, "MEM LIMIT:"));
+            mem_limit_mb = parse_lsf_memory_from_line(line, "MEMLIMIT")
+                .or_else(|| parse_lsf_memory_from_line(line, "MEM LIMIT:"));
         }
 
         // Look for runtime info
         if line.contains("Run time:") || line.contains("RUN_TIME:") {
-            run_time_seconds = parse_lsf_time(line);
+            run_time_seconds = parse_lsf_time_from_line(line);
         }
         if line.contains("RUNLIMIT") || line.contains("RUN LIMIT:") {
-            run_limit_seconds = parse_lsf_time(line);
+            run_limit_seconds = parse_lsf_time_from_line(line);
         }
     }
 
@@ -285,99 +288,61 @@ fn extract_number(s: &str, prefix: &str) -> Option<u64> {
     }
 }
 
-/// Parse LSF memory value (e.g., "4.5 Gbytes", "1024 Mbytes").
-fn parse_lsf_memory(line: &str, prefix: &str) -> Option<u64> {
+/// Parse LSF memory value from a line with a prefix (e.g., "MAX MEM: 4.5 Gbytes").
+fn parse_lsf_memory_from_line(line: &str, prefix: &str) -> Option<u64> {
     if let Some(idx) = line.find(prefix) {
-        let after = &line[idx + prefix.len()..];
-        // Find the number
-        let parts: Vec<&str> = after.split_whitespace().collect();
-        if parts.len() >= 2 {
-            let value: f64 = parts[0].parse().ok()?;
-            let unit = parts[1].to_lowercase();
-            return Some(if unit.starts_with('g') {
-                (value * 1024.0) as u64
-            } else if unit.starts_with('m') {
-                value as u64
-            } else if unit.starts_with('k') {
-                (value / 1024.0) as u64
-            } else {
-                value as u64
-            });
-        }
+        let after = &line[idx + prefix.len()..].trim();
+        // Extract just "4.5 Gbytes" part for the shared parser
+        let mem_str: String = after
+            .split_whitespace()
+            .take(2)
+            .collect::<Vec<_>>()
+            .join(" ");
+        parse_memory_mb(&mem_str, MemoryFormat::Lsf)
+    } else {
+        None
     }
-    None
 }
 
-/// Parse LSF time value (e.g., "01:30:00", "1800 seconds").
-fn parse_lsf_time(line: &str) -> Option<u64> {
-    // Look for HH:MM:SS pattern
+/// Parse LSF time value from a line (e.g., "Run time: 01:30:00").
+fn parse_lsf_time_from_line(line: &str) -> Option<u64> {
+    // Look for HH:MM:SS pattern in the line
     for word in line.split_whitespace() {
-        if word.contains(':') {
-            let parts: Vec<&str> = word.split(':').collect();
-            if parts.len() == 3 {
-                let hours: u64 = parts[0].parse().ok()?;
-                let mins: u64 = parts[1].parse().ok()?;
-                let secs: u64 = parts[2].parse().ok()?;
-                return Some(hours * 3600 + mins * 60 + secs);
-            }
+        if word.contains(':') && word.chars().filter(|c| *c == ':').count() == 2 {
+            return parse_duration_secs(word);
         }
     }
     // Look for "N seconds" pattern
     if let Some(idx) = line.find("seconds") {
         let before = line[..idx].trim();
-        // Get the last number token before "seconds"
         if let Some(num_str) = before.split_whitespace().last() {
-            if let Ok(secs) = num_str.parse::<f64>() {
-                return Some(secs as u64);
+            if let Ok(secs) = num_str.parse::<u64>() {
+                return Some(secs);
             }
         }
     }
     None
 }
 
-/// Format seconds as human-readable duration.
-fn format_duration(seconds: u64) -> String {
-    let hours = seconds / 3600;
-    let mins = (seconds % 3600) / 60;
-    let secs = seconds % 60;
-
-    if hours > 24 {
-        let days = hours / 24;
-        let hours = hours % 24;
-        format!("{}d {:02}:{:02}:{:02}", days, hours, mins, secs)
-    } else if hours > 0 {
-        format!("{:02}:{:02}:{:02}", hours, mins, secs)
-    } else {
-        format!("{:02}:{:02}", mins, secs)
-    }
-}
-
-/// Format seconds as LSF duration format (HH:MM).
-fn format_duration_lsf(seconds: u64) -> String {
-    let hours = seconds / 3600;
-    let mins = (seconds % 3600) / 60;
-    format!("{}:{:02}", hours, mins)
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
 
     #[test]
-    fn test_parse_lsf_memory() {
+    fn test_parse_lsf_memory_from_line() {
         assert_eq!(
-            parse_lsf_memory("MAX MEM: 4.5 Gbytes", "MAX MEM:"),
+            parse_lsf_memory_from_line("MAX MEM: 4.5 GB", "MAX MEM:"),
             Some(4608)
         );
         assert_eq!(
-            parse_lsf_memory("MEMLIMIT 8192 Mbytes", "MEMLIMIT"),
+            parse_lsf_memory_from_line("MEMLIMIT 8192 MB", "MEMLIMIT"),
             Some(8192)
         );
     }
 
     #[test]
-    fn test_parse_lsf_time() {
-        assert_eq!(parse_lsf_time("Run time: 01:30:00"), Some(5400));
-        assert_eq!(parse_lsf_time("1800 seconds"), Some(1800));
+    fn test_parse_lsf_time_from_line() {
+        assert_eq!(parse_lsf_time_from_line("Run time: 01:30:00"), Some(5400));
+        assert_eq!(parse_lsf_time_from_line("1800 seconds"), Some(1800));
     }
 }

crates/charmer-monitor/src/app.rs

@@ -353,8 +353,8 @@ impl App {
             }
         }
 
-        // Try SLURM log path format: .snakemake/slurm_logs/rule_{rule}/{slurm_job_id}.log
-        if let Some(ref slurm_id) = job.slurm_job_id {
+        // Try SLURM log path format: .snakemake/slurm_logs/rule_{rule}/{scheduler_job_id}.log
+        if let Some(ref slurm_id) = job.scheduler_job_id {
             let slurm_log = working_dir
                 .join(".snakemake")
                 .join("slurm_logs")

crates/charmer-monitor/src/components/job_detail.rs

@@ -319,8 +319,8 @@ fn build_detail_lines(job: &Job, command_expanded: bool) -> Vec<Line<'static>> {
         ),
     ]));
 
-    // SLURM/LSF Job ID
-    if let Some(ref slurm_id) = job.slurm_job_id {
+    // Scheduler Job ID (SLURM/LSF)
+    if let Some(ref slurm_id) = job.scheduler_job_id {
         lines.push(Line::from(vec![
             Span::styled("Job ID: ", Style::default().fg(Color::Gray)),
             Span::styled(slurm_id.clone(), Style::default().fg(Color::Cyan)),

crates/charmer-parsers/src/lib.rs

@@ -9,7 +9,10 @@ pub mod time;
 
 pub use command::{run_command, run_command_allow_failure, CommandError};
 pub use memory::{parse_memory_mb, MemoryFormat};
-pub use time::{parse_duration, parse_exit_code, parse_lsf_timestamp, parse_slurm_timestamp};
+pub use time::{
+    format_duration, format_duration_lsf, format_duration_slurm, parse_duration,
+    parse_duration_secs, parse_exit_code, parse_lsf_timestamp, parse_slurm_timestamp,
+};
 
 /// Filter helper for optional string fields.
 /// Returns None if the string is empty or a placeholder value.

crates/charmer-parsers/src/time.rs

@@ -87,6 +87,51 @@ pub fn parse_exit_code(s: &str) -> i32 {
         .unwrap_or(0)
 }
 
+/// Parse duration to seconds from various formats.
+///
+/// Like `parse_duration` but returns seconds as u64 instead of Duration.
+pub fn parse_duration_secs(s: &str) -> Option<u64> {
+    parse_duration(s).map(|d| d.as_secs())
+}
+
+/// Format seconds as human-readable duration (e.g., "1d 02:30:00", "01:30:00", "05:30").
+pub fn format_duration(seconds: u64) -> String {
+    let hours = seconds / 3600;
+    let mins = (seconds % 3600) / 60;
+    let secs = seconds % 60;
+
+    if hours > 24 {
+        let days = hours / 24;
+        let hours = hours % 24;
+        format!("{}d {:02}:{:02}:{:02}", days, hours, mins, secs)
+    } else if hours > 0 {
+        format!("{:02}:{:02}:{:02}", hours, mins, secs)
+    } else {
+        format!("{:02}:{:02}", mins, secs)
+    }
+}
+
+/// Format seconds as LSF duration format (HH:MM for resource limits).
+pub fn format_duration_lsf(seconds: u64) -> String {
+    let hours = seconds / 3600;
+    let mins = (seconds % 3600) / 60;
+    format!("{}:{:02}", hours, mins)
+}
+
+/// Format seconds as SLURM duration format (D-HH:MM:SS).
+pub fn format_duration_slurm(seconds: u64) -> String {
+    let days = seconds / 86400;
+    let hours = (seconds % 86400) / 3600;
+    let mins = (seconds % 3600) / 60;
+    let secs = seconds % 60;
+
+    if days > 0 {
+        format!("{}-{:02}:{:02}:{:02}", days, hours, mins, secs)
+    } else {
+        format!("{:02}:{:02}:{:02}", hours, mins, secs)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;