From 79895c5f0fe27bf3a205caeda48623006d2c1752 Mon Sep 17 00:00:00 2001 From: Fini Date: Thu, 4 Jun 2026 03:22:15 +0800 Subject: [PATCH] fix(ai): exit non-zero on failed benchmark output/render steps The headless benchmark paths reported success even when their output/render step failed: op-smoke still returned SUCCESS when the OPENPENCIL_SMOKE_OUT write failed, and --render-shots returned the GUI-skip 'handled' signal (exit 0) on an unreadable/unparsable .op, an empty page, or a node that failed to render. op-smoke now exits 4 when a requested save fails; --render-shots exits 1/2 on any read/parse/empty/partial-render failure and only exits 0 when every top-level node rendered. Verified: good .op -> 0, missing file -> 1. Found by Codex stop-time review. --- crates/op-host-desktop/src/render_cli.rs | 25 ++++++++++------ crates/op-smoke/src/main.rs | 38 +++++++++++++++++------- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/crates/op-host-desktop/src/render_cli.rs b/crates/op-host-desktop/src/render_cli.rs index b9b922039..a8773476c 100644 --- a/crates/op-host-desktop/src/render_cli.rs +++ b/crates/op-host-desktop/src/render_cli.rs @@ -12,9 +12,11 @@ use std::path::PathBuf; use crate::export::{export_node_raster, RasterFormat}; -/// When `--render-shots` is present on argv, render node PNGs and -/// return `true` so `main` exits; otherwise return `false` and let the -/// normal GUI path continue. +/// When `--render-shots` is present on argv, render node PNGs and return +/// `true` so `main` exits 0; otherwise return `false` and let the normal +/// GUI path continue. Any failure (bad args, unreadable / unparsable +/// `.op`, no nodes, or a node that fails to render) exits non-zero — a +/// benchmark driver must not read success when no PNG was produced. pub fn run_cli_if_requested() -> bool { let args: Vec = std::env::args().collect(); let Some(pos) = args.iter().position(|a| a == "--render-shots") else { @@ -22,7 +24,7 @@ pub fn run_cli_if_requested() -> bool { }; let (Some(file), Some(out_dir)) = (args.get(pos + 1), args.get(pos + 2)) else { eprintln!("usage: openpencil-desktop --render-shots [scale]"); - return true; + std::process::exit(2); }; let scale: f32 = args .get(pos + 3) @@ -31,21 +33,21 @@ pub fn run_cli_if_requested() -> bool { let out_dir = PathBuf::from(out_dir); if let Err(e) = std::fs::create_dir_all(&out_dir) { eprintln!("render-shots: mkdir {}: {e}", out_dir.display()); - return true; + std::process::exit(1); } let src = match std::fs::read_to_string(file) { Ok(s) => s, Err(e) => { eprintln!("render-shots: read {file}: {e}"); - return true; + std::process::exit(1); } }; let loaded = match jian_ops_schema::load_str(&src) { Ok(l) => l, Err(e) => { eprintln!("render-shots: parse {file}: {e}"); - return true; + std::process::exit(1); } }; let state = op_editor_core::EditorState::from_document(loaded.value); @@ -53,11 +55,11 @@ pub fn run_cli_if_requested() -> bool { let Some(page) = scene.active_page() else { eprintln!("render-shots: no active page"); - return true; + std::process::exit(1); }; if page.children.is_empty() { eprintln!("render-shots: active page has no nodes"); - return true; + std::process::exit(1); } let total = page.children.len(); @@ -73,6 +75,11 @@ pub fn run_cli_if_requested() -> bool { } } eprintln!("render-shots: {ok}/{total} node(s) → {}", out_dir.display()); + if ok < total { + // A benchmark driver must see a non-zero exit when any node failed + // to render, not a silent success. + std::process::exit(1); + } true } diff --git a/crates/op-smoke/src/main.rs b/crates/op-smoke/src/main.rs index 9f4611dd3..a2cf6a852 100644 --- a/crates/op-smoke/src/main.rs +++ b/crates/op-smoke/src/main.rs @@ -343,17 +343,28 @@ async fn main() -> std::process::ExitCode { // so the render / screenshot step can pick it up. Canonical // serde_json mirrors `persistence::save_to_path`. Saved regardless of // Ok/Err so a partial doc stays inspectable on failure. - if let Ok(out_path) = std::env::var("OPENPENCIL_SMOKE_OUT") { - if !out_path.is_empty() { - match serde_json::to_string_pretty(&sink.state.doc) { - Ok(json) => match std::fs::write(&out_path, json) { - Ok(()) => eprintln!("[SMOKE] saved doc → {out_path}"), - Err(e) => eprintln!("[SMOKE] save failed ({out_path}): {e}"), - }, - Err(e) => eprintln!("[SMOKE] serialize failed: {e}"), + // A requested save that FAILS forces a non-zero exit below — a + // benchmark driver must not read "success" when no `.op` was written. + let save_failed = match std::env::var("OPENPENCIL_SMOKE_OUT") { + Ok(out_path) if !out_path.is_empty() => match serde_json::to_string_pretty(&sink.state.doc) + { + Ok(json) => match std::fs::write(&out_path, json) { + Ok(()) => { + eprintln!("[SMOKE] saved doc → {out_path}"); + false + } + Err(e) => { + eprintln!("[SMOKE] save failed ({out_path}): {e}"); + true + } + }, + Err(e) => { + eprintln!("[SMOKE] serialize failed: {e}"); + true } - } - } + }, + _ => false, + }; match result { Ok(summary) => { @@ -372,7 +383,12 @@ async fn main() -> std::process::ExitCode { .unwrap_or_default() ); } - std::process::ExitCode::SUCCESS + if save_failed { + eprintln!("[FINAL] generation OK but OPENPENCIL_SMOKE_OUT write failed"); + std::process::ExitCode::from(4) + } else { + std::process::ExitCode::SUCCESS + } } Err(e) => { eprintln!("[FINAL] Err in {elapsed:?}: {e}");