Simplify run status code: dedup color_if/abbreviate_home, fix Removing active state

- Replace duplicate `abbreviate_home` with existing `tilde_path`
- Include `Removing` in `is_active()` so removing runs aren't pruned
- Log warning on status.json write failure instead of silently discarding
- Extract `color_if` to cli/mod.rs, remove copies in runs.rs and rewind.rs
- Unify near-duplicate RunInfo construction in scan_runs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-15 16:53:17 -04:00
parent c5f658b450
commit f34a09bf1c
4 changed files with 45 additions and 64 deletions

View file

@ -279,6 +279,16 @@ pub fn relative_path(path: &Path) -> String {
tilde_path(path)
}
/// Return `Some(color)` when color is enabled, `None` otherwise.
/// Used with `cli_table`'s `.foreground_color()` which accepts `Option<Color>`.
pub(crate) fn color_if(use_color: bool, color: cli_table::Color) -> Option<cli_table::Color> {
if use_color {
Some(color)
} else {
None
}
}
/// Shorten an absolute path by replacing the home directory prefix with `~`.
pub fn tilde_path(path: &Path) -> String {
if let Some(home) = dirs::home_dir() {

View file

@ -283,7 +283,6 @@ pub fn print_timeline(
}
let use_color = styles.use_color;
let color_if = |color| if use_color { Some(color) } else { None };
let title = vec![
"@".cell().bold(true),
@ -313,11 +312,13 @@ pub fn print_timeline(
};
vec![
ordinal_str.cell().foreground_color(color_if(Color::Cyan)),
ordinal_str
.cell()
.foreground_color(super::color_if(use_color, Color::Cyan)),
entry.node_name.clone().cell(),
detail_str
.cell()
.foreground_color(color_if(Color::Ansi256(8))),
.foreground_color(super::color_if(use_color, Color::Ansi256(8))),
]
})
.collect();

View file

@ -155,47 +155,30 @@ pub fn scan_runs(base: &Path) -> Result<Vec<RunInfo>> {
.unwrap_or_else(|_| dir_name.clone());
let si = read_status(&path);
if matches!(si.status, RunStatus::Dead) {
// True orphan — no manifest, no status.json
runs.push(RunInfo {
run_id,
dir_name,
workflow_name: "[no manifest]".to_string(),
workflow_slug: None,
status: si.status,
status_reason: si.reason,
start_time: mtime,
labels: HashMap::new(),
duration_ms: None,
total_cost: None,
host_repo_path: None,
start_time_dt: mtime_dt,
end_time: None,
path,
goal: String::new(),
is_orphan: true,
});
} else {
// Has status.json → run is initializing, not an orphan
runs.push(RunInfo {
run_id,
dir_name,
workflow_name: "[starting]".to_string(),
workflow_slug: None,
status: si.status,
status_reason: si.reason,
start_time: mtime,
labels: HashMap::new(),
duration_ms: si.duration_ms,
total_cost: si.total_cost,
host_repo_path: None,
start_time_dt: mtime_dt,
end_time: si.end_time,
path,
goal: String::new(),
is_orphan: false,
});
}
let is_orphan = matches!(si.status, RunStatus::Dead);
runs.push(RunInfo {
run_id,
dir_name,
workflow_name: if is_orphan {
"[no manifest]"
} else {
"[starting]"
}
.to_string(),
workflow_slug: None,
status: si.status,
status_reason: si.reason,
start_time: mtime,
labels: HashMap::new(),
duration_ms: si.duration_ms,
total_cost: si.total_cost,
host_repo_path: None,
start_time_dt: mtime_dt,
end_time: si.end_time,
path,
goal: String::new(),
is_orphan,
});
}
}
@ -227,7 +210,9 @@ impl StatusInfo {
/// Write the run status to `status.json` (best-effort).
pub fn write_run_status(run_dir: &Path, status: RunStatus, reason: Option<StatusReason>) {
let record = RunStatusRecord::new(status, reason);
let _ = record.save(&run_dir.join("status.json"));
if let Err(e) = record.save(&run_dir.join("status.json")) {
warn!("failed to write status.json for {}: {e}", run_dir.display());
}
}
fn read_status(run_dir: &Path) -> StatusInfo {
@ -441,13 +426,7 @@ fn truncate_goal(goal: &str, max_len: usize) -> String {
format!("{truncated}...")
}
fn color_if(use_color: bool, color: Color) -> Option<Color> {
if use_color {
Some(color)
} else {
None
}
}
use super::color_if;
fn status_cell(status: &RunStatus, use_color: bool) -> CellStruct {
let text = status.to_string();
@ -464,15 +443,6 @@ fn status_cell(status: &RunStatus, use_color: bool) -> CellStruct {
.foreground_color(color_if(use_color, color.unwrap_or(Color::Ansi256(8))))
}
fn abbreviate_home(path: &str) -> String {
if let Some(home) = dirs::home_dir() {
if let Ok(rel) = Path::new(path).strip_prefix(&home) {
return format!("~/{}", rel.display());
}
}
path.to_string()
}
pub fn list_command(args: &RunsListArgs, styles: &Styles) -> Result<()> {
let base = default_runs_base();
let runs = scan_runs(&base)?;
@ -538,7 +508,7 @@ pub fn list_command(args: &RunsListArgs, styles: &Styles) -> Result<()> {
let dir_display = run
.host_repo_path
.as_deref()
.map(abbreviate_home)
.map(|p| super::tilde_path(Path::new(p)))
.unwrap_or_else(|| "-".to_string());
vec![
run_id_display

View file

@ -29,7 +29,7 @@ impl RunStatus {
pub fn is_active(self) -> bool {
matches!(
self,
Self::Submitted | Self::Starting | Self::Running | Self::Paused
Self::Submitted | Self::Starting | Self::Running | Self::Paused | Self::Removing
)
}
@ -198,7 +198,7 @@ mod tests {
assert!(RunStatus::Starting.is_active());
assert!(RunStatus::Running.is_active());
assert!(RunStatus::Paused.is_active());
assert!(!RunStatus::Removing.is_active());
assert!(RunStatus::Removing.is_active());
assert!(!RunStatus::Succeeded.is_active());
assert!(!RunStatus::Failed.is_active());
assert!(!RunStatus::Dead.is_active());