From 731c051733b0f359ee0de0128ff8e5eec3fa97d2 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sat, 8 Aug 2026 16:27:03 +0200 Subject: [PATCH] feat(pi-shell/minimizer): implemented length threshold and empty preservation - Add minimum character threshold to bypass minimization for short outputs. - Add preserve-if-empty configuration and pipeline support for filters. - Update test fixtures and integration tests to meet length thresholds. --- crates/pi-shell/src/minimizer.rs | 2 +- crates/pi-shell/src/minimizer/defs/rustc.toml | 18 +- crates/pi-shell/src/minimizer/engine.rs | 170 ++++++++++++------ crates/pi-shell/src/minimizer/filters/mod.rs | 32 +++- crates/pi-shell/src/minimizer/pipeline.rs | 36 +++- crates/pi-shell/src/shell.rs | 70 +++----- crates/pi-shell/tests/minimizer_fixtures.rs | 80 ++++----- packages/coding-agent/CHANGELOG.md | 1 + 8 files changed, 252 insertions(+), 157 deletions(-) diff --git a/crates/pi-shell/src/minimizer.rs b/crates/pi-shell/src/minimizer.rs index dd0879b39..89dfcc09f 100644 --- a/crates/pi-shell/src/minimizer.rs +++ b/crates/pi-shell/src/minimizer.rs @@ -46,7 +46,7 @@ pub struct MinimizerOutput { /// Label for the dispatch path that produced this output (e.g. `"git"`, /// `"pipeline:gradle"`, or `"passthrough"`). For non-rewrite misses, this /// carries the reason label (e.g. `"compound"`, `"piped"`, `"parse-error"`, - /// `"too-large"`, `"disabled"`, `"unknown"`, `"unsupported"`, + /// `"too-short"`, `"too-large"`, `"disabled"`, `"unknown"`, `"unsupported"`, /// `"pipeline-noop"`). pub filter: &'static str, /// Original (un-minimized) capture, surfaced only when the filter diff --git a/crates/pi-shell/src/minimizer/defs/rustc.toml b/crates/pi-shell/src/minimizer/defs/rustc.toml index 222f2bc3e..5112e2889 100644 --- a/crates/pi-shell/src/minimizer/defs/rustc.toml +++ b/crates/pi-shell/src/minimizer/defs/rustc.toml @@ -10,17 +10,20 @@ # The intervening snippet/caret/label lines (` |`, ` 3 | …`, ` = note:`) are # dropped: they are recoverable context, not signal, and the kept `-->` pointer # already localizes the diagnostic. +# Query/help output is not a diagnostic stream. Preserve it when no diagnostic +# line matches, rather than letting the engine turn the meaningful response into +# a generic success sentinel. # -# DELIBERATELY no on_empty: the on_empty stage is NOT exit-gated -# (pipeline.rs:330-335), so a failed compile whose error text happened to miss -# the keep-set would otherwise collapse to a fake "rustc: ok". The engine's -# ensure_success_visible prints "OK" only on exit 0, which is the correct, -# exit-gated success sentinel. +# DELIBERATELY no on_empty: that stage is not exit-gated, so a failed compile +# whose error text happened to miss the keep-set would otherwise collapse to a +# fake "rustc: ok". The engine's ensure_success_visible prints "OK" only on +# exit 0, which is the correct, exit-gated success sentinel. [filters.rustc] description = "Compact rustc output — keep diagnostics, ICE/panic, and failure trailers" match_command = "^rustc$" strip_ansi = true +preserve_if_empty = true keep_lines_matching = [ "^error", "^warning", @@ -85,3 +88,8 @@ warning: unused variable: `x` warning: 1 warning emitted """ expected = "warning: unused variable: `x`\n --> src/lib.rs:2:9\nwarning: 1 warning emitted\n" + +[[tests.rustc]] +name = "query output is preserved instead of collapsed to ok" +input = "/opt/rust/lib/rustlib\n" +expected = "/opt/rust/lib/rustlib\n" diff --git a/crates/pi-shell/src/minimizer/engine.rs b/crates/pi-shell/src/minimizer/engine.rs index 6337db1da..ce24e1f97 100644 --- a/crates/pi-shell/src/minimizer/engine.rs +++ b/crates/pi-shell/src/minimizer/engine.rs @@ -13,6 +13,11 @@ use crate::minimizer::{ pipeline::{self, CompiledPipeline, PipelineRegistry}, plan, }; +/// Captured outputs shorter than this are returned verbatim without filtering. +pub const MIN_MINIMIZE_CHARS: usize = 1_000; +fn is_below_minimize_threshold(captured: &str) -> bool { + captured.chars().take(MIN_MINIMIZE_CHARS).count() < MIN_MINIMIZE_CHARS +} /// Minimization strategy for a shell command. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -305,6 +310,9 @@ fn apply_identity( let subcommand = identity.subcommand.as_deref(); if filters::supports(&identity.program, subcommand) { + if is_below_minimize_threshold(captured) { + return MinimizerOutput::passthrough(captured).labeled("too-short"); + } let ctx = MinimizerCtx { program: &identity.program, subcommand, command, config }; let Ok(rust_output) = catch_unwind(AssertUnwindSafe(|| filters::filter(&ctx, captured, exit_code))) @@ -329,6 +337,9 @@ fn apply_identity( if pipeline.skipped_by_exit(exit_code) { return MinimizerOutput::passthrough(captured).labeled("exit-skip"); } + if is_below_minimize_threshold(captured) { + return MinimizerOutput::passthrough(captured).labeled("too-short"); + } let text = catch_unwind(AssertUnwindSafe(|| pipeline.apply(captured).into_owned())) .unwrap_or_else(|_| captured.to_string()); if text == captured { @@ -525,6 +536,15 @@ pub fn verify_builtin_filters() -> Vec { pipeline::run_tests(builtin_pipelines()) } +#[cfg(test)] +fn minimizable_input(input: &str) -> String { + let mut output = input.to_string(); + while output.chars().count() < MIN_MINIMIZE_CHARS { + output.push('\n'); + } + output +} + #[cfg(test)] mod tests { use std::{ @@ -550,6 +570,29 @@ mod tests { let _ = fs::remove_file(path); cfg } + + #[test] + fn output_below_minimum_is_not_minimized() { + let cfg = config_from_settings( + r#" +schema_version = 1 +[filters.printf] +match_command = "^printf$" +replace = [{ pattern = "x", replacement = "y" }] +"#, + ); + let short = "x".repeat(MIN_MINIMIZE_CHARS - 1); + let exact = "x".repeat(MIN_MINIMIZE_CHARS); + + let short_result = apply("printf", &short, 0, &cfg); + assert_eq!(short_result.text, short); + assert!(!short_result.changed); + assert_eq!(short_result.filter, "too-short"); + + let exact_result = apply("printf", &exact, 0, &cfg); + assert!(exact_result.changed); + assert_ne!(exact_result.text, exact); + } #[test] fn disabled_config_does_not_minimize() { let cfg = MinimizerConfig::default(); @@ -594,16 +637,17 @@ on_empty = "OVERLAY" only_on_exit = [0] "#, ); - let diff_input = "diff --git a/file.rs b/file.rs\n@@\n-old\n+new\n"; - let diff = apply("git diff", diff_input, 0, &cfg); + let diff_input = minimizable_input("diff --git a/file.rs b/file.rs\n@@\n-old\n+new\n"); + let diff = apply("git diff", &diff_input, 0, &cfg); assert_eq!(diff.filter, "pipeline+builtin"); assert_eq!(diff.text, "OVERLAY"); - let status = apply("git status", "## main\n M file.rs\n", 0, &cfg); + let status_input = minimizable_input("## main\n M file.rs\n"); + let status = apply("git status", &status_input, 0, &cfg); assert_ne!(status.filter, "pipeline+builtin"); assert!(status.text.contains("unstaged 1")); - let failed = apply("git diff", diff_input, 1, &cfg); + let failed = apply("git diff", &diff_input, 1, &cfg); assert_ne!(failed.filter, "pipeline+builtin"); assert!(failed.text.contains("file changed")); } @@ -620,9 +664,11 @@ only_on_exit = [0] // and strip the NX banner; the bare npx def must not claim subcommand // `nx`. let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let input = "\n > NX Running target build for 3 projects\n\n> nx run app:build\nError: \ - build failed\n\n > NX Ran target build for 3 projects (2s)\n"; - let out = apply("npx nx build", input, 0, &cfg); + let input = minimizable_input( + "\n > NX Running target build for 3 projects\n\n> nx run app:build\nError: build \ + failed\n\n > NX Ran target build for 3 projects (2s)\n", + ); + let out = apply("npx nx build", &input, 0, &cfg); assert!( !out.text.contains("NX Running target"), "npx def shadowed nx-wrapped; NX banner survived: {:?}", @@ -662,9 +708,11 @@ only_on_exit = [0] // for genuinely UNKNOWN tools (its whole purpose). cowsay is not routed // or owned by any other def, so the npx pipeline claims it. let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let input = "Need to install the following packages:\ncowsay@1.6.0\nOk to proceed? \ - (y)\n\nnpm warn deprecated foo@1.0.0: use bar instead\n< Hello >\n"; - let out = apply("npx cowsay hello", input, 0, &cfg); + let input = minimizable_input( + "Need to install the following packages:\ncowsay@1.6.0\nOk to proceed? (y)\n\nnpm warn \ + deprecated foo@1.0.0: use bar instead\n< Hello >\n", + ); + let out = apply("npx cowsay hello", &input, 0, &cfg); assert!(out.changed); assert!(!out.text.contains("Need to install")); assert!(!out.text.contains("Ok to proceed")); @@ -676,7 +724,8 @@ only_on_exit = [0] fn enabled_known_filter_minimizes() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; assert!(should_minimize("git diff", &cfg)); - let out = apply("git diff", "diff --git a/file.rs b/file.rs\n@@\n-old\n+new\n", 0, &cfg); + let input = minimizable_input("diff --git a/file.rs b/file.rs\n@@\n-old\n+new\n"); + let out = apply("git diff", &input, 0, &cfg); assert!(out.changed); assert!(out.text.contains("file changed")); } @@ -685,8 +734,8 @@ only_on_exit = [0] fn enabled_config_minimizes_git_status() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; assert!(should_minimize("git status", &cfg)); - let input = "## main\n M file.rs\n"; - let out = apply("git status", input, 0, &cfg); + let input = minimizable_input("## main\n M file.rs\n"); + let out = apply("git status", &input, 0, &cfg); assert!(out.changed); assert!(out.text.contains("unstaged 1")); assert_eq!(out.filter, "git"); @@ -695,13 +744,11 @@ only_on_exit = [0] #[test] fn successful_minimization_keeps_visible_ok_when_filter_removes_all_lines() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let out = apply( - "cargo build", - " Compiling app v0.1.0\n Finished `dev` profile [unoptimized + debuginfo] target(s) \ - in 1.23s\n", - 0, - &cfg, + let input = format!( + "{} Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.23s\n", + " Compiling app v0.1.0\n".repeat(200), ); + let out = apply("cargo build", &input, 0, &cfg); assert!(out.changed); assert_eq!(out.text, "OK\n"); @@ -709,6 +756,16 @@ only_on_exit = [0] assert!(out.original_text.is_some()); } + #[test] + fn rustc_query_output_is_not_collapsed_to_ok() { + let cfg = MinimizerConfig { enabled: true, ..Default::default() }; + let input = "/opt/rust/lib/rustlib\n"; + let out = apply("rustc --print sysroot", input, 0, &cfg); + + assert_eq!(out.text, input); + assert!(!out.changed); + } + #[test] fn successful_user_pipeline_empty_output_returns_visible_ok() { let cfg = config_from_settings( @@ -720,20 +777,21 @@ strip_lines_matching = [".*"] "#, ); - assert!(should_minimize("printf done", &cfg)); - let out = apply("printf done", "drop me\n", 0, &cfg); + let input = minimizable_input("drop me\n"); + let out = apply("printf done", &input, 0, &cfg); assert!(out.changed); assert_eq!(out.text, "OK\n"); assert_eq!(out.filter, "pipeline"); assert_eq!(out.output_bytes, out.text.len()); - assert_eq!(out.original_text.as_deref(), Some("drop me\n")); + assert_eq!(out.original_text.as_deref(), Some(input.as_str())); } #[test] fn failed_minimization_does_not_invent_ok_for_empty_output() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let out = apply("cargo build", " Compiling app v0.1.0\n", 1, &cfg); + let input = " Compiling app v0.1.0\n".repeat(200); + let out = apply("cargo build", &input, 1, &cfg); assert!(out.changed); assert_eq!(out.text, ""); @@ -797,25 +855,21 @@ strip_lines_matching = [".*"] assert!(should_minimize("ctest --output-on-failure", &cfg)); assert!(should_minimize("./build/foo_test --gtest_filter=Foo.*", &cfg)); - let ctest = apply( - "ctest --output-on-failure", + let ctest_input = minimizable_input( "Test project /tmp/build\n1/2 Test #1: ok ........ Passed 0.01 sec\n2/2 Test #2: \ bad .......***Failed 0.02 sec\nThe following tests FAILED:\n", - 8, - &cfg, ); + let ctest = apply("ctest --output-on-failure", &ctest_input, 8, &cfg); assert!(ctest.changed); assert_eq!(ctest.filter, "ctest"); assert!(!ctest.text.contains("Test #1")); assert!(ctest.text.contains("Test #2: bad")); - let gtest = apply( - "./build/foo_test", + let gtest_input = minimizable_input( "[ RUN ] Foo.Pass\n[ OK ] Foo.Pass (0 ms)\nfoo_test.cc:42: Failure\nExpected: \ 1\n[ FAILED ] Foo.Fails\n", - 1, - &cfg, ); + let gtest = apply("./build/foo_test", >est_input, 1, &cfg); assert!(gtest.changed); assert_eq!(gtest.filter, "gtest"); assert!(!gtest.text.contains("Foo.Pass")); @@ -1037,10 +1091,12 @@ strip_lines_matching = [".*"] #[test] fn rails_db_migrate_routes_to_standalone_def() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let input = "== 20240115 CreateUsers: migrating\n-- create_table(:users)\n -> 0.0234s\n== \ - 20240115 CreateUsers: migrated\n\n== 20240116 AddIndexToOrders: migrating\n-- \ - add_index(:orders)\n -> 0.0123s\n== 20240116 AddIndexToOrders: migrated\n"; - let out = apply("rails db:migrate", input, 0, &cfg); + let input = minimizable_input( + "== 20240115 CreateUsers: migrating\n-- create_table(:users)\n -> 0.0234s\n== 20240115 \ + CreateUsers: migrated\n\n== 20240116 AddIndexToOrders: migrating\n-- \ + add_index(:orders)\n -> 0.0123s\n== 20240116 AddIndexToOrders: migrated\n", + ); + let out = apply("rails db:migrate", &input, 0, &cfg); assert!(out.changed); assert!(out.text.contains("✓ CreateUsers")); assert!(out.text.contains("✓ AddIndexToOrders")); @@ -1053,8 +1109,10 @@ strip_lines_matching = [".*"] #[test] fn rails_routes_routes_to_standalone_def() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let input = " Prefix Verb URI Pattern Controller#Action\n root GET / home#index\n"; - let out = apply("rails routes", input, 0, &cfg); + let input = minimizable_input( + " Prefix Verb URI Pattern Controller#Action\n root GET / home#index\n", + ); + let out = apply("rails routes", &input, 0, &cfg); assert!(out.changed); assert!(!out.text.contains("Prefix")); assert!(out.text.contains("root GET")); @@ -1064,8 +1122,10 @@ strip_lines_matching = [".*"] #[test] fn rake_routes_regression_unminimized() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let input = " Prefix Verb URI Pattern Controller#Action\n root GET / home#index\n"; - let out = apply("rake routes", input, 0, &cfg); + let input = minimizable_input( + " Prefix Verb URI Pattern Controller#Action\n root GET / home#index\n", + ); + let out = apply("rake routes", &input, 0, &cfg); assert!(out.changed, "rake routes should be minimized by the rails-routes def"); assert!(!out.text.contains("Prefix")); assert!(out.text.contains("root GET")); @@ -1074,9 +1134,11 @@ strip_lines_matching = [".*"] #[test] fn bundle_exec_rails_db_migrate_reaches_def() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let input = "== 20240115 CreateUsers: migrating\n-- create_table(:users)\n -> 0.0234s\n== \ - 20240115 CreateUsers: migrated\n"; - let out = apply("bundle exec rails db:migrate", input, 0, &cfg); + let input = minimizable_input( + "== 20240115 CreateUsers: migrating\n-- create_table(:users)\n -> 0.0234s\n== 20240115 \ + CreateUsers: migrated\n", + ); + let out = apply("bundle exec rails db:migrate", &input, 0, &cfg); assert!(out.changed); assert!(out.text.contains("CreateUsers")); assert!(!out.text.contains("-- create_table")); @@ -1085,8 +1147,10 @@ strip_lines_matching = [".*"] #[test] fn bundle_exec_rails_routes_reaches_def() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let input = " Prefix Verb URI Pattern Controller#Action\n root GET / home#index\n"; - let out = apply("bundle exec rails routes", input, 0, &cfg); + let input = minimizable_input( + " Prefix Verb URI Pattern Controller#Action\n root GET / home#index\n", + ); + let out = apply("bundle exec rails routes", &input, 0, &cfg); assert!(out.changed); assert!(!out.text.contains("Prefix")); assert!(out.text.contains("root GET")); @@ -1119,11 +1183,13 @@ strip_lines_matching = [".*"] #[test] fn rails_db_migrate_keyword_in_name_not_dropped() { let cfg = MinimizerConfig { enabled: true, ..Default::default() }; - let input = "== 20240115 CreateTestResults: migrating\n-- create_table(:test_results)\n \ - -> 0.0234s\n== 20240115 CreateTestResults: migrated\n\n== 20240116 \ - AddIndexToOrders: migrating\n-- add_index(:orders)\n -> 0.0123s\n== 20240116 \ - AddIndexToOrders: migrated\n"; - let out = apply("rails db:migrate", input, 0, &cfg); + let input = minimizable_input( + "== 20240115 CreateTestResults: migrating\n-- create_table(:test_results)\n -> \ + 0.0234s\n== 20240115 CreateTestResults: migrated\n\n== 20240116 AddIndexToOrders: \ + migrating\n-- add_index(:orders)\n -> 0.0123s\n== 20240116 AddIndexToOrders: \ + migrated\n", + ); + let out = apply("rails db:migrate", &input, 0, &cfg); assert!(out.changed); // Both migrations must survive; the old overlay-on-rake would keep only // lines containing "test" and silently drop AddIndexToOrders. @@ -1209,12 +1275,10 @@ mod pipeline_integration_tests { enabled: Some(true), ..Default::default() }); - let out = apply( - "gradle build", + let input = minimizable_input( "> Task :app:compileJava UP-TO-DATE\n> Task :app:test\nBUILD SUCCESSFUL in 8s\n", - 0, - &cfg, ); + let out = apply("gradle build", &input, 0, &cfg); // gradle.toml is deleted; gradle now dispatches to the Rust jvm Build // filter, which strips `> Task :…UP-TO-DATE` task-progress noise (the // carried-over defs/gradle.toml behaviour, now as a Rust filter — NOT a diff --git a/crates/pi-shell/src/minimizer/filters/mod.rs b/crates/pi-shell/src/minimizer/filters/mod.rs index 3a0e21489..13d3bbe16 100644 --- a/crates/pi-shell/src/minimizer/filters/mod.rs +++ b/crates/pi-shell/src/minimizer/filters/mod.rs @@ -488,7 +488,7 @@ fn wrapper_invoked_tool<'a>(ctx: &'a MinimizerCtx<'_>, tools: &[&'a str]) -> Opt #[cfg(test)] mod tests { use super::*; - use crate::minimizer::MinimizerConfig; + use crate::minimizer::{MinimizerConfig, engine::MIN_MINIMIZE_CHARS}; fn ctx<'a>( program: &'a str, @@ -499,6 +499,14 @@ mod tests { MinimizerCtx { program, subcommand, command, config } } + fn minimizable_input(input: &str) -> String { + let mut output = input.to_string(); + while output.chars().count() < MIN_MINIMIZE_CHARS { + output.push('\n'); + } + output + } + #[test] fn npx_test_tools_route_to_node_test_filter() { let config = MinimizerConfig::default(); @@ -975,9 +983,11 @@ mod tests { fn bundle_exec_rails_db_migrate_routes_to_def() { let config = MinimizerConfig { enabled: true, ..Default::default() }; let context = ctx("bundle", Some("exec"), "bundle exec rails db:migrate", &config); - let input = "== 20240115 CreateUsers: migrating\n-- create_table(:users)\n -> 0.0234s\n== \ - 20240115 CreateUsers: migrated\n"; - let out = filter(&context, input, 0); + let input = minimizable_input( + "== 20240115 CreateUsers: migrating\n-- create_table(:users)\n -> 0.0234s\n== 20240115 \ + CreateUsers: migrated\n", + ); + let out = filter(&context, &input, 0); assert!(out.changed); assert!(out.text.contains("CreateUsers")); assert!(!out.text.contains("-- create_table")); @@ -987,8 +997,10 @@ mod tests { fn bundle_exec_rails_routes_routes_to_def() { let config = MinimizerConfig { enabled: true, ..Default::default() }; let context = ctx("bundle", Some("exec"), "bundle exec rails routes", &config); - let input = " Prefix Verb URI Pattern Controller#Action\n root GET / home#index\n"; - let out = filter(&context, input, 0); + let input = minimizable_input( + " Prefix Verb URI Pattern Controller#Action\n root GET / home#index\n", + ); + let out = filter(&context, &input, 0); assert!(out.changed); assert!(!out.text.contains("Prefix")); assert!(out.text.contains("root GET")); @@ -998,9 +1010,11 @@ mod tests { fn bundle_exec_rspec_routes_to_ruby_filter() { let config = MinimizerConfig { enabled: true, ..Default::default() }; let context = ctx("bundle", Some("exec"), "bundle exec rspec", &config); - let input = "Randomized with seed 12345\n\nUserController\n GET /users\n returns a list \ - of users\n\nFinished in 0.45 seconds\n5 examples, 0 failures\n"; - let out = filter(&context, input, 0); + let input = minimizable_input( + "Randomized with seed 12345\n\nUserController\n GET /users\n returns a list of \ + users\n\nFinished in 0.45 seconds\n5 examples, 0 failures\n", + ); + let out = filter(&context, &input, 0); assert!(out.changed); assert!(out.text.contains("5 examples, 0 failures")); assert!(!out.text.contains("returns a list of users")); diff --git a/crates/pi-shell/src/minimizer/pipeline.rs b/crates/pi-shell/src/minimizer/pipeline.rs index 0a2e8904e..d0756b737 100644 --- a/crates/pi-shell/src/minimizer/pipeline.rs +++ b/crates/pi-shell/src/minimizer/pipeline.rs @@ -20,7 +20,9 @@ //! 6. `truncate_lines_at` — per-line Unicode-safe char cap //! 7. `head_lines` / `tail_lines` — keep first/last N lines with a marker //! 8. `max_lines` — hard cap after head/tail -//! 9. `on_empty` — replace an empty result with a sentinel +//! 9. `preserve_if_empty` — keep the original input when filtering removes +//! every line +//! 10. `on_empty` — replace an empty result with a sentinel //! //! Pipelines never panic for the caller: regex compilation errors are //! surfaced when the pipeline is loaded, and runtime application is total. @@ -66,6 +68,11 @@ pub struct PipelineDef { pub tail_lines: Option, pub max_lines: Option, pub on_empty: Option, + /// Return the original input unchanged when all filtering stages remove it. + /// Useful for diagnostic filters that must not discard an unrecognized + /// successful output, such as a compiler query response. + #[serde(default)] + pub preserve_if_empty: bool, /// Apply only when the command exit code is in this list. Empty = always. #[serde(default)] pub only_on_exit: Vec, @@ -146,6 +153,7 @@ pub struct CompiledPipeline { pub tail_lines: Option, pub max_lines: Option, pub on_empty: Option, + pub preserve_if_empty: bool, pub only_on_exit: Vec, pub except_on_exit: Vec, } @@ -225,6 +233,7 @@ pub fn compile(name: String, def: PipelineDef) -> Result(&self, input: &'a str) -> Cow<'a, str> { // Stage 1: strip_ansi @@ -330,7 +339,14 @@ impl CompiledPipeline { stage7 }; - // Stage 9: on_empty + // Stage 9: preserve the source when a keep-only filter removed every line. + // This prevents a successful query/diagnostic command's meaningful output + // from being replaced by the engine's generic "OK" sentinel. + if self.preserve_if_empty && stage8.trim().is_empty() { + return Cow::Borrowed(input); + } + + // Stage 10: on_empty if let Some(msg) = self.on_empty.as_deref() && stage8.trim().is_empty() { @@ -592,6 +608,20 @@ replacement = "" assert_eq!(out.as_ref(), "file.rs\n"); } + #[test] + fn preserve_if_empty_returns_original_when_filter_drops_every_line() { + let src = r#" +schema_version = 1 +[filters.keep] +match_command = "^keep$" +keep_lines_matching = ["^diagnostic:"] +preserve_if_empty = true +"#; + let pipeline = compile_one(src); + let input = "query result\n"; + assert_eq!(pipeline.apply(input).as_ref(), input); + } + #[test] fn on_empty_fires_when_output_is_blank() { let src = r#" diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index 65fbae2c3..510338621 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -4499,20 +4499,17 @@ replace = [{ pattern = "hello", replacement = "HI" }] async fn segmented_false_semicolon_printf_continues_and_returns_last_code() { let root = unique_temp_dir("false-semi"); let minimizer = printf_minimizer(&root.join("minimizer.toml"), None); - let (result, output) = run_command_capture( - "false ; printf 'hello\n'", - None, - Some(minimizer), - CancelToken::default(), - ) - .await; + let expected = "hello\n".repeat(200); + let command = format!("false ; printf '{}'", "hello\\n".repeat(200)); + let (result, output) = + run_command_capture(&command, None, Some(minimizer), CancelToken::default()).await; let _ = std::fs::remove_dir_all(&root); - let minimized = result.minimized.expect("minimized result"); + let minimized = result.minimized.expect("long output should be minimized"); assert_eq!(result.exit_code, Some(0)); - assert_eq!(output, "hello\n"); + assert_eq!(output, expected); assert_eq!(minimized.filter, "chain"); - assert_eq!(minimized.original_text, "hello\n"); - assert_eq!(minimized.text, "HI\n"); + assert_eq!(minimized.original_text, expected); + assert_eq!(minimized.text, "HI\n".repeat(200)); } #[cfg(unix)] @@ -4544,12 +4541,9 @@ replace = [{ pattern = "^.+$", replacement = "PWD" }] run_command_capture("cd tmp && pwd", Some(&root), Some(minimizer), CancelToken::default()) .await; let _ = std::fs::remove_dir_all(&root); - let minimized = result.minimized.expect("minimized result"); + assert!(result.minimized.is_none(), "short pwd output must not be minimized"); assert_eq!(result.exit_code, Some(0)); assert_eq!(output, expected); - assert_eq!(minimized.filter, "chain"); - assert_eq!(minimized.text, "PWD\n"); - assert_eq!(minimized.original_text, expected); } #[cfg(unix)] @@ -4575,22 +4569,21 @@ replace = [{ pattern = "^.+$", replacement = "PWD" }] async fn segmented_printf_chain_preserves_raw_original_text() { let root = unique_temp_dir("minimizer"); let minimizer = printf_minimizer(&root.join("minimizer.toml"), None); - let (result, output) = run_command_capture( - "printf 'hello\n' ; printf 'world\n'", - None, - Some(minimizer), - CancelToken::default(), - ) - .await; + let hello = "hello\n".repeat(200); + let world = "world\n".repeat(200); + let command = + format!("printf '{}' ; printf '{}'", "hello\\n".repeat(200), "world\\n".repeat(200)); + let (result, output) = + run_command_capture(&command, None, Some(minimizer), CancelToken::default()).await; let _ = std::fs::remove_dir_all(&root); - let minimized = result.minimized.expect("minimized result"); + let minimized = result.minimized.expect("long output should be minimized"); assert_eq!(result.exit_code, Some(0)); - assert_eq!(output, "hello\nworld\n"); + assert_eq!(output, format!("{hello}{world}")); assert_eq!(minimized.filter, "chain"); - assert_eq!(minimized.original_text, "hello\nworld\n"); - assert_eq!(minimized.text, "HI\nworld\n"); - assert_eq!(minimized.input_bytes, 12); - assert_eq!(minimized.output_bytes, 9); + assert_eq!(minimized.original_text, format!("{hello}{world}")); + assert_eq!(minimized.text, format!("{}{}", "HI\n".repeat(200), world)); + assert_eq!(minimized.input_bytes, (hello.len() + world.len()) as u32); + assert_eq!(minimized.output_bytes, ("HI\n".repeat(200).len() + world.len()) as u32); } /// Regression: a quoted here-doc followed by another command must execute @@ -4659,24 +4652,19 @@ replace = [{ pattern = "^.+$", replacement = "PWD" }] async fn segmented_chain_with_redirect_executes_correctly() { let root = unique_temp_dir("redirect-chain"); let minimizer = printf_minimizer(&root.join("minimizer.toml"), None); - let (result, output) = run_command_capture( - "echo hidden >/dev/null && printf 'hello\\n'", - None, - Some(minimizer), - CancelToken::default(), - ) - .await; + let expected = "hello\n".repeat(200); + let command = format!("echo hidden >/dev/null && printf '{}'", "hello\\n".repeat(200)); + let (result, output) = + run_command_capture(&command, None, Some(minimizer), CancelToken::default()).await; let _ = std::fs::remove_dir_all(&root); assert_eq!(result.exit_code, Some(0)); // The redirect survived reconstruction: segment 1's stdout went to // /dev/null, so only segment 2's output is captured. assert!(!output.contains("hidden"), "redirect must suppress segment-1 stdout"); - assert_eq!(output, "hello\n"); - let minimized = result - .minimized - .expect("redirect chain should be minimized"); - assert_eq!(minimized.original_text, "hello\n"); - assert_eq!(minimized.text, "HI\n"); + assert_eq!(output, expected); + let minimized = result.minimized.expect("long output should be minimized"); + assert_eq!(minimized.original_text, expected); + assert_eq!(minimized.text, "HI\n".repeat(200)); assert!(!output.contains("syntax error")); } diff --git a/crates/pi-shell/tests/minimizer_fixtures.rs b/crates/pi-shell/tests/minimizer_fixtures.rs index 315e54ca1..950337d84 100644 --- a/crates/pi-shell/tests/minimizer_fixtures.rs +++ b/crates/pi-shell/tests/minimizer_fixtures.rs @@ -9,32 +9,27 @@ //! - `.exit` — integer exit code (optional; defaults to 0). //! - `.min` — expected minimized snapshot (optional, see gate below). //! -//! Gate per fixture (`raw` measured in bytes): +//! Gate per fixture: //! -//! - `raw.len() >= 500`: assert `minimized.len() <= 0.40 * raw.len()` — the -//! savings gate. A short output is not worth a filter round-trip, so the gate -//! only applies to buffers large enough to matter. -//! - `raw.len() < 500`: a `.min` snapshot is REQUIRED and must match exactly. -//! Small buffers cannot meaningfully clear a ratio gate, so they are pinned -//! by an exact snapshot instead. -//! - `.min` present alongside a `>= 500`-byte raw: assert BOTH the savings gate -//! and the exact snapshot. +//! - `raw.chars().count() < MIN_MINIMIZE_CHARS`: the minimizer must return the +//! raw output unchanged; short output is never filtered. +//! - `raw.chars().count() >= MIN_MINIMIZE_CHARS`: assert `minimized.len() <= +//! 0.40 * raw.len()` — the savings gate, measured in bytes because the +//! output-size budget is byte-based. +//! - `.min` present alongside an eligible raw buffer: assert BOTH the savings +//! gate and the exact snapshot. //! //! All fixture failures are collected before the harness panics so a single run //! reports every regression, not just the first. use std::{fmt::Write as _, fs, path::Path}; -use pi_shell::minimizer::{self, MinimizerConfig}; +use pi_shell::minimizer::{self, MinimizerConfig, engine::MIN_MINIMIZE_CHARS}; /// Byte-savings gate: minimized output must be at most this fraction of the raw /// input for buffers large enough to be worth filtering. const SAVINGS_RATIO: f64 = 0.40; -/// Raw buffers below this byte length are pinned by an exact `.min` snapshot -/// instead of the ratio gate. -const GATE_MIN_BYTES: usize = 500; - /// A single discovered fixture: the `.cmd`/`.raw` pair plus its optional /// `.exit` and `.min` companions. struct Fixture { @@ -85,9 +80,20 @@ fn minimizer_fixtures_clear_savings_gate() { fn check_fixture(fixture: &Fixture, cfg: &MinimizerConfig) -> Result<(), String> { let out = minimizer::apply(&fixture.command, &fixture.raw, fixture.exit, cfg); let minimized = out.text.as_str(); - + let raw_chars = fixture.raw.chars().count(); let raw_len = fixture.raw.len(); let min_len = minimized.len(); + + if raw_chars < MIN_MINIMIZE_CHARS { + if minimized == fixture.raw { + return Ok(()); + } + return Err(format!( + "[{}] cmd={:?} exit={} raw={} chars: short output was modified", + fixture.name, fixture.command, fixture.exit, raw_chars, + )); + } + let ratio = if raw_len == 0 { 0.0 } else { @@ -95,36 +101,20 @@ fn check_fixture(fixture: &Fixture, cfg: &MinimizerConfig) -> Result<(), String> }; let mut problems: Vec = Vec::new(); - - if raw_len >= GATE_MIN_BYTES { - let budget = (SAVINGS_RATIO * raw_len as f64).floor() as usize; - if min_len > budget { - problems.push(format!( - "savings gate: minimized {min_len} B > {budget} B budget ({:.1}% of {raw_len} B raw, \ - limit {:.0}%)", - ratio * 100.0, - SAVINGS_RATIO * 100.0 - )); - } - // A `.min` alongside a large raw pins the exact shape too. - if let Some(expected) = &fixture.expected - && expected != minimized - { - problems.push(format!("snapshot mismatch:\n{}", diff_excerpt(expected, minimized))); - } - } else { - // Small buffers cannot meaningfully clear a ratio gate; require an exact - // snapshot instead. - match &fixture.expected { - None => problems.push(format!( - "raw is {raw_len} B (< {GATE_MIN_BYTES} B): a `.min` snapshot is required for \ - sub-threshold fixtures" - )), - Some(expected) if expected != minimized => { - problems.push(format!("snapshot mismatch:\n{}", diff_excerpt(expected, minimized))); - }, - Some(_) => {}, - } + let budget = (SAVINGS_RATIO * raw_len as f64).floor() as usize; + if min_len > budget { + problems.push(format!( + "savings gate: minimized {min_len} B > {budget} B budget ({:.1}% of {raw_len} B raw, \ + limit {:.0}%)", + ratio * 100.0, + SAVINGS_RATIO * 100.0 + )); + } + // A `.min` alongside an eligible raw buffer pins the exact shape too. + if let Some(expected) = &fixture.expected + && expected != minimized + { + problems.push(format!("snapshot mismatch:\n{}", diff_excerpt(expected, minimized))); } if problems.is_empty() { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4d54fafd0..a08456035 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed shell minimization replacing meaningful `rustc --print` output with `OK`. +- Fixed shell minimization altering outputs shorter than 1,000 characters; these now pass through unchanged. ## [17.2.11] - 2026-08-07