mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-08 22:21:45 +00:00
Polish CLI run output: cleaner layout and less noise
- Increase indent from 2 to 4 spaces for progress UI lines - Add blank line separators after Sandbox/Setup and before Retro output - Dim Sandbox/Setup and workflow metadata lines - Rename "Parsed workflow:" to "Workflow:" - Remove redundant inform() calls for run/stage start/complete - Remove Notes: and Logs: lines from end-of-run summary Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
a1adbe89ca
commit
81841d28eb
4 changed files with 38 additions and 161 deletions
|
|
@ -29,21 +29,21 @@ macro_rules! cached_style {
|
|||
|
||||
cached_style!(
|
||||
style_header_running,
|
||||
" {spinner:.dim} {wide_msg} {elapsed:.dim}"
|
||||
" {spinner:.dim} {wide_msg} {elapsed:.dim}"
|
||||
);
|
||||
cached_style!(style_header_done, " {wide_msg} {prefix:.dim}");
|
||||
cached_style!(style_header_done, " {wide_msg:.dim} {prefix:.dim}");
|
||||
cached_style!(
|
||||
style_stage_running,
|
||||
" {spinner:.cyan} {wide_msg} {elapsed:.dim}"
|
||||
" {spinner:.cyan} {wide_msg} {elapsed:.dim}"
|
||||
);
|
||||
cached_style!(style_stage_done, " {wide_msg} {prefix:.dim}");
|
||||
cached_style!(style_stage_done, " {wide_msg} {prefix:.dim}");
|
||||
cached_style!(
|
||||
style_tool_running,
|
||||
" {spinner:.dim} {wide_msg} {elapsed:.dim}"
|
||||
" {spinner:.dim} {wide_msg} {elapsed:.dim}"
|
||||
);
|
||||
cached_style!(style_tool_done, " {wide_msg}");
|
||||
cached_style!(style_static_dim, " {wide_msg:.dim}");
|
||||
cached_style!(style_empty, "");
|
||||
cached_style!(style_tool_done, " {wide_msg}");
|
||||
cached_style!(style_static_dim, " {wide_msg:.dim}");
|
||||
cached_style!(style_empty, " ");
|
||||
|
||||
// ── Cached glyphs ───────────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -136,6 +136,7 @@ pub struct ProgressUI {
|
|||
setup_command_count: usize,
|
||||
sandbox_bar: Option<ProgressBar>,
|
||||
setup_bar: Option<ProgressBar>,
|
||||
any_stage_started: bool,
|
||||
}
|
||||
|
||||
impl ProgressUI {
|
||||
|
|
@ -153,6 +154,7 @@ impl ProgressUI {
|
|||
setup_command_count: 0,
|
||||
sandbox_bar: None,
|
||||
setup_bar: None,
|
||||
any_stage_started: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -165,7 +167,7 @@ impl ProgressUI {
|
|||
});
|
||||
}
|
||||
|
||||
/// Clear all active bars before the summary block is printed.
|
||||
/// Clear all active bars and release the terminal for normal stderr output.
|
||||
pub fn finish(&mut self) {
|
||||
for (_id, stage) in self.active_stages.drain() {
|
||||
for entry in &stage.tool_calls {
|
||||
|
|
@ -173,6 +175,13 @@ impl ProgressUI {
|
|||
}
|
||||
stage.spinner.finish_and_clear();
|
||||
}
|
||||
if let ProgressRenderer::Tty(tty) = &self.renderer {
|
||||
// Add a trailing blank line through indicatif so it survives the final redraw
|
||||
let sep = tty.multi.add(ProgressBar::new_spinner());
|
||||
sep.set_style(style_empty());
|
||||
sep.finish();
|
||||
tty.multi.set_draw_target(ProgressDrawTarget::hidden());
|
||||
}
|
||||
}
|
||||
|
||||
// ── Event dispatch ──────────────────────────────────────────────────
|
||||
|
|
@ -257,7 +266,7 @@ impl ProgressUI {
|
|||
}
|
||||
}
|
||||
ProgressRenderer::Plain => {
|
||||
eprintln!(" Sandbox: {provider} (ready in {dur})");
|
||||
eprintln!(" Sandbox: {provider} (ready in {dur})");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -294,7 +303,7 @@ impl ProgressUI {
|
|||
}
|
||||
}
|
||||
ProgressRenderer::Plain => {
|
||||
eprintln!(" Setup: {count} command{suffix} ({dur})");
|
||||
eprintln!(" Setup: {count} command{suffix} ({dur})");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -308,12 +317,9 @@ impl ProgressUI {
|
|||
let bar = tty.multi.add(ProgressBar::new_spinner());
|
||||
bar.set_style(style_static_dim());
|
||||
bar.finish_with_message(format!("Logs: {path_str}"));
|
||||
let sep = tty.multi.add(ProgressBar::new_spinner());
|
||||
sep.set_style(style_empty());
|
||||
sep.finish();
|
||||
}
|
||||
ProgressRenderer::Plain => {
|
||||
eprintln!(" Logs: {path_str}");
|
||||
eprintln!(" Logs: {path_str}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -322,6 +328,12 @@ impl ProgressUI {
|
|||
|
||||
fn on_stage_started(&mut self, node_id: &str, name: &str) {
|
||||
if let ProgressRenderer::Tty(tty) = &self.renderer {
|
||||
if !self.any_stage_started {
|
||||
self.any_stage_started = true;
|
||||
let sep = tty.multi.add(ProgressBar::new_spinner());
|
||||
sep.set_style(style_empty());
|
||||
sep.finish();
|
||||
}
|
||||
let bar = tty.multi.add(ProgressBar::new_spinner());
|
||||
bar.set_style(style_stage_running());
|
||||
bar.set_message(name.to_string());
|
||||
|
|
@ -353,9 +365,9 @@ impl ProgressUI {
|
|||
}
|
||||
ProgressRenderer::Plain => {
|
||||
if prefix.is_empty() {
|
||||
eprintln!(" {glyph} {name}");
|
||||
eprintln!(" {glyph} {name}");
|
||||
} else {
|
||||
eprintln!(" {glyph} {name} {prefix}");
|
||||
eprintln!(" {glyph} {name} {prefix}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -207,11 +207,11 @@ pub async fn run_command(
|
|||
let (graph, diagnostics) = WorkflowBuilder::new().prepare(&source)?;
|
||||
|
||||
eprintln!(
|
||||
"{} {} ({})",
|
||||
styles.bold.apply_to("Parsed workflow:"),
|
||||
"{} {} {}",
|
||||
styles.bold.apply_to("Workflow:"),
|
||||
graph.name,
|
||||
styles.dim.apply_to(format!(
|
||||
"{} nodes, {} edges",
|
||||
"({} nodes, {} edges)",
|
||||
graph.nodes.len(),
|
||||
graph.edges.len()
|
||||
)),
|
||||
|
|
@ -655,6 +655,11 @@ pub async fn run_command(
|
|||
}
|
||||
}
|
||||
|
||||
// Finish progress bars before printing summary
|
||||
if !args.verbose {
|
||||
progress_ui.lock().expect("progress lock poisoned").finish();
|
||||
}
|
||||
|
||||
// Auto-derive retro (always, cheap) and optionally run retro agent
|
||||
{
|
||||
let (failed, failure_reason) = match &engine_result {
|
||||
|
|
@ -684,11 +689,6 @@ pub async fn run_command(
|
|||
|
||||
let outcome = engine_result?;
|
||||
|
||||
// Finish progress bars before printing summary
|
||||
if !args.verbose {
|
||||
progress_ui.lock().expect("progress lock poisoned").finish();
|
||||
}
|
||||
|
||||
// 8. Print result
|
||||
eprintln!("\n{}", styles.bold.apply_to("=== Run Result ==="),);
|
||||
|
||||
|
|
@ -737,17 +737,9 @@ pub async fn run_command(
|
|||
}
|
||||
drop(acc);
|
||||
|
||||
if let Some(notes) = &outcome.notes {
|
||||
eprintln!("Notes: {notes}");
|
||||
}
|
||||
if let Some(failure) = outcome.failure_reason() {
|
||||
eprintln!("{}", styles.red.apply_to(format!("Failure: {failure}")),);
|
||||
}
|
||||
eprintln!(
|
||||
"{} {}",
|
||||
styles.dim.apply_to("Logs:"),
|
||||
styles.underline.apply_to(logs_dir.display()),
|
||||
);
|
||||
|
||||
// 9. Exit code
|
||||
match outcome.status {
|
||||
|
|
@ -1023,11 +1015,6 @@ async fn run_from_branch(
|
|||
"Duration: {}",
|
||||
HumanDuration(Duration::from_millis(run_duration_ms))
|
||||
);
|
||||
eprintln!(
|
||||
"{} {}",
|
||||
styles.dim.apply_to("Logs:"),
|
||||
styles.underline.apply_to(logs_dir.display()),
|
||||
);
|
||||
|
||||
match outcome.status {
|
||||
StageStatus::Success | StageStatus::PartialSuccess => Ok(()),
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ pub fn validate_command(args: &ValidateArgs, styles: &Styles) -> anyhow::Result<
|
|||
"{} ({} nodes, {} edges)",
|
||||
styles
|
||||
.bold
|
||||
.apply_to(format!("Parsed workflow: {}", graph.name)),
|
||||
.apply_to(format!("Workflow: {}", graph.name)),
|
||||
graph.nodes.len(),
|
||||
graph.edges.len(),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -776,19 +776,6 @@ impl WorkflowRunEngine {
|
|||
}
|
||||
}
|
||||
|
||||
/// Call inform on the interviewer, if one is configured.
|
||||
fn inform(&self, message: &str, stage: &str) {
|
||||
if let Some(ref interviewer) = self.interviewer {
|
||||
// inform is async but we fire-and-forget since it's informational
|
||||
let interviewer = Arc::clone(interviewer);
|
||||
let message = message.to_string();
|
||||
let stage = stage.to_string();
|
||||
tokio::spawn(async move {
|
||||
interviewer.inform(&message, &stage).await;
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// Mirror graph-level attributes into the context.
|
||||
fn mirror_graph_attributes(graph: &Graph, context: &Context) {
|
||||
if !graph.goal().is_empty() {
|
||||
|
|
@ -1084,8 +1071,6 @@ impl WorkflowRunEngine {
|
|||
_ => None,
|
||||
},
|
||||
});
|
||||
self.inform(&format!("Run started: {}", graph.name), "run");
|
||||
|
||||
// Write manifest.json (spec 5.6)
|
||||
let manifest = write_manifest(&config.logs_root, graph, config);
|
||||
|
||||
|
|
@ -1329,9 +1314,6 @@ impl WorkflowRunEngine {
|
|||
attempt: 1,
|
||||
max_attempts: usize::try_from(retry_policy.max_attempts).unwrap_or(usize::MAX),
|
||||
});
|
||||
if node.handler_type() != Some("wait.human") {
|
||||
self.inform(&format!("Stage started: {}", node.label()), &node.id);
|
||||
}
|
||||
let stage_start = Instant::now();
|
||||
|
||||
let (mut outcome, attempts_used) = if let Some((ref token, _)) = stall_token {
|
||||
|
|
@ -1440,7 +1422,6 @@ impl WorkflowRunEngine {
|
|||
max_attempts: usize::try_from(retry_policy.max_attempts)
|
||||
.unwrap_or(usize::MAX),
|
||||
});
|
||||
self.inform(&format!("Stage completed: {}", node.label()), &node.id);
|
||||
}
|
||||
|
||||
// Write per-node status.json (spec 5.6)
|
||||
|
|
@ -3274,109 +3255,6 @@ mod tests {
|
|||
|
||||
// --- Gap #15: Interviewer.inform() tests ---
|
||||
|
||||
/// Mock interviewer that records `inform()` calls.
|
||||
struct RecordingInformer {
|
||||
messages: std::sync::Mutex<Vec<(String, String)>>,
|
||||
}
|
||||
|
||||
impl RecordingInformer {
|
||||
fn new() -> Self {
|
||||
Self {
|
||||
messages: std::sync::Mutex::new(Vec::new()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl crate::interviewer::Interviewer for RecordingInformer {
|
||||
async fn ask(&self, _question: crate::interviewer::Question) -> crate::interviewer::Answer {
|
||||
crate::interviewer::Answer::yes()
|
||||
}
|
||||
|
||||
async fn inform(&self, message: &str, stage: &str) {
|
||||
self.messages
|
||||
.lock()
|
||||
.unwrap()
|
||||
.push((message.to_string(), stage.to_string()));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn engine_calls_inform_on_run_start() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let g = simple_graph();
|
||||
let informer = Arc::new(RecordingInformer::new());
|
||||
let engine = WorkflowRunEngine::with_interviewer(
|
||||
make_registry(),
|
||||
Arc::new(EventEmitter::new()),
|
||||
Arc::clone(&informer) as Arc<dyn crate::interviewer::Interviewer>,
|
||||
local_env(),
|
||||
);
|
||||
let config = RunConfig {
|
||||
logs_root: dir.path().to_path_buf(),
|
||||
cancel_token: None,
|
||||
dry_run: false,
|
||||
run_id: "test-run".into(),
|
||||
git_checkpoint: None,
|
||||
base_sha: None,
|
||||
run_branch: None,
|
||||
meta_branch: None,
|
||||
labels: HashMap::new(),
|
||||
};
|
||||
engine.run(&g, &config).await.unwrap();
|
||||
// Give spawned inform tasks time to complete
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
|
||||
let messages = informer.messages.lock().unwrap();
|
||||
assert!(
|
||||
messages
|
||||
.iter()
|
||||
.any(|(msg, stage)| msg.contains("Run started") && stage == "run"),
|
||||
"expected 'Run started' inform call, got: {messages:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn engine_calls_inform_on_stage_start_and_complete() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let g = simple_graph();
|
||||
let informer = Arc::new(RecordingInformer::new());
|
||||
let engine = WorkflowRunEngine::with_interviewer(
|
||||
make_registry(),
|
||||
Arc::new(EventEmitter::new()),
|
||||
Arc::clone(&informer) as Arc<dyn crate::interviewer::Interviewer>,
|
||||
local_env(),
|
||||
);
|
||||
let config = RunConfig {
|
||||
logs_root: dir.path().to_path_buf(),
|
||||
cancel_token: None,
|
||||
dry_run: false,
|
||||
run_id: "test-run".into(),
|
||||
git_checkpoint: None,
|
||||
base_sha: None,
|
||||
run_branch: None,
|
||||
meta_branch: None,
|
||||
labels: HashMap::new(),
|
||||
};
|
||||
engine.run(&g, &config).await.unwrap();
|
||||
// Give spawned inform tasks time to complete
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
|
||||
let messages = informer.messages.lock().unwrap();
|
||||
assert!(
|
||||
messages
|
||||
.iter()
|
||||
.any(|(msg, _)| msg.contains("Stage started")),
|
||||
"expected 'Stage started' inform call, got: {messages:?}"
|
||||
);
|
||||
assert!(
|
||||
messages
|
||||
.iter()
|
||||
.any(|(msg, _)| msg.contains("Stage completed")),
|
||||
"expected 'Stage completed' inform call, got: {messages:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn engine_without_interviewer_runs_normally() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue