From fe0602b7847a499db89c8901f8254d95eb981478 Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Wed, 9 Sep 2026 15:27:03 +0200 Subject: [PATCH 1/8] feat(mcp): crush unbounded listing outputs to row budget updates, list_refhosts and openqa_overview keep first-40 + last-10 + all anomaly rows (exact-deduped, hard cap 100) with a narrowing-hint notice; --json stays a valid array with the notice trailing. openqa --export still writes the full overview. --- CHANGELOG.md | 11 ++ Cargo.lock | 1 + .../mtui-core/src/commands/list_refhosts.rs | 152 +++++++++++++++++- crates/mtui-core/src/commands/mod.rs | 1 + .../mtui-core/src/commands/openqa_overview.rs | 143 +++++++++++++++- crates/mtui-core/src/commands/row_budget.rs | 131 +++++++++++++++ crates/mtui-core/src/commands/updates.rs | 129 ++++++++++++++- crates/mtui-mcp/Cargo.toml | 1 + crates/mtui-mcp/tests/it.rs | 2 + crates/mtui-mcp/tests/json_crush.rs | 133 +++++++++++++++ 10 files changed, 690 insertions(+), 14 deletions(-) create mode 100644 crates/mtui-core/src/commands/row_budget.rs create mode 100644 crates/mtui-mcp/tests/json_crush.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index fcef8116..829ae470 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,17 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht path) are refused, and endpoint URLs print userinfo-stripped. The REPL prints everything verbatim as before. No tool schema changed. `config set` of an endpoint URL acknowledges with the userinfo-stripped value. +- Unbounded listings now crush to a row budget — first-40 + last-10 + all + anomaly rows, exact-deduped, hard cap 100 — instead of dumping thousands of + rows into the client's context: `updates` (anomaly: non-`testing` status; + narrow with `--limit/--field/-G`), `list_refhosts` (anomaly: non-`free` lock + or pool claim; narrow with `--name/--arch/--product/--version/--addon`), and + `openqa_overview` (anomaly: non-`passed` version rows, build checks with + matches; display only, `--export` still writes the full overview; narrow with + `--no-aggregated/--aggregated-groups/--days/--test-pattern`). `--json` still + prints a valid JSON array (truncated) with the + `…[truncated N of M rows; …]` notice on a trailing line — strip it before + parsing. MCP tool names/schemas unchanged. ### Deprecated diff --git a/Cargo.lock b/Cargo.lock index ebcb8e75..b8a9ec78 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2594,6 +2594,7 @@ dependencies = [ "tokio-util", "tracing", "tracing-subscriber", + "wiremock", ] [[package]] diff --git a/crates/mtui-core/src/commands/list_refhosts.rs b/crates/mtui-core/src/commands/list_refhosts.rs index cc916982..9b8568af 100644 --- a/crates/mtui-core/src/commands/list_refhosts.rs +++ b/crates/mtui-core/src/commands/list_refhosts.rs @@ -30,6 +30,16 @@ use crate::command::{Command, Scope}; use crate::error::{CommandError, CommandResult}; use crate::session::Session; +use super::row_budget::{crush, row_notice}; + +/// Narrowing flags named in the row-budget notice. +const REFHOSTS_HINT: &str = "--name/--arch/--product/--version/--addon"; + +/// Non-`free` or pool-claimed rows survive the crush: the actionable lock signal. +fn is_anomaly_record(r: &Record) -> bool { + !matches!(r.lock.as_deref(), None | Some("free")) || r.pool.is_some() +} + /// One matched refhost, rendered as a table row or a JSON object. #[derive(Debug, Clone)] pub struct Record { @@ -381,17 +391,43 @@ impl Command for ListRefhosts { probe_locks(&ProbeConfig::new(&config), &mut records).await; } + // Row budget backstops the whole-inventory dump: head+tail+anomalies, exact-deduped. + let crushed = crush( + records, + |r| { + ( + r.name.clone(), + r.arch.clone(), + r.product.clone(), + r.version.clone(), + r.addons.clone(), + r.slot.clone(), + r.lock.clone(), + r.pool.clone(), + ) + }, + is_anomaly_record, + ); + // --json emits the truncated array (valid JSON) plus an optional trailing notice line; strip the notice before parsing. + let notice = (crushed.truncated > 0) + .then(|| row_notice(crushed.truncated, crushed.total, REFHOSTS_HINT)); if as_json { - session.display.println(&render_json(&records)); + session.display.println(&render_json(&crushed.kept)); + if let Some(notice) = notice { + session.display.println(¬ice); + } return Ok(()); } - if records.is_empty() { + if crushed.kept.is_empty() { session.display.println("no refhosts match"); return Ok(()); } session .display - .println(&render_table(&records, pool, free, verbose)); + .println(&render_table(&crushed.kept, pool, free, verbose)); + if let Some(notice) = notice { + session.display.println(¬ice); + } Ok(()) } } @@ -968,4 +1004,114 @@ default: ListRefhosts.call(&mut session, &args).await.unwrap(); assert!(buf.contents().contains("no refhosts match")); } + + // ------------------------------------------------------------ row budget + + /// 150-record inventory with one mid-list `locked` anomaly and exact duplicates. + fn crush_records() -> Vec { + let mut recs: Vec = (0..150) + .map(|i| Record { + name: format!("host-{i:03}"), + arch: "x86_64".to_owned(), + product: "sles".to_owned(), + version: "15-6".to_owned(), + addons: vec![], + slot: None, + lock: Some("free".to_owned()), + pool: None, + }) + .collect(); + recs[100].name = "host-anomaly".to_owned(); + recs[100].lock = Some("locked".to_owned()); + recs.push(recs[0].clone()); + recs.push(recs[1].clone()); + recs + } + + #[test] + fn row_budget_crushes_inventory_and_keeps_locked_anomaly() { + use super::super::row_budget::{ROW_CAP, crush}; + let out = crush( + crush_records(), + |r| { + ( + r.name.clone(), + r.arch.clone(), + r.product.clone(), + r.version.clone(), + r.addons.clone(), + r.slot.clone(), + r.lock.clone(), + r.pool.clone(), + ) + }, + is_anomaly_record, + ); + assert_eq!(out.total, 150, "deduped total"); + assert!(out.kept.len() <= ROW_CAP, "{}", out.kept.len()); + assert!(out.kept.iter().any(|r| r.name == "host-anomaly")); + assert!(!out.kept.iter().any(|r| r.name == "host-060")); + assert!(out.truncated > 0); + } + + #[test] + fn row_budget_json_stays_parseable_with_trailing_notice() { + use super::super::row_budget::{crush, row_notice}; + let out = crush( + crush_records(), + |r| { + ( + r.name.clone(), + r.arch.clone(), + r.product.clone(), + r.version.clone(), + r.addons.clone(), + r.slot.clone(), + r.lock.clone(), + r.pool.clone(), + ) + }, + is_anomaly_record, + ); + let mut text = render_json(&out.kept); + text.push('\n'); + text.push_str(&row_notice(out.truncated, out.total, REFHOSTS_HINT)); + assert!( + text.contains("--name/--arch/--product/--version/--addon"), + "{text}" + ); + let json_part: String = text + .lines() + .filter(|l| !l.contains("[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + let arr = parsed.as_array().unwrap(); + assert!(arr.iter().any(|r| r["name"] == "host-anomaly")); + } + + #[tokio::test] + async fn call_crushes_large_inventory_with_notice() { + use crate::commands::testkit::matches; + let mut yaml = String::from("default:\n"); + for i in 0..150 { + yaml.push_str(&format!( + " - name: host-{i:03}\n arch: x86_64\n product:\n name: sles\n version:\n major: 15\n minor: 6\n" + )); + } + let (mut session, buf, _dir) = session_with_refhosts_file(&yaml); + let args = matches(&ListRefhosts, &[]); + ListRefhosts.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + assert!(out.contains("[truncated"), "{out}"); + assert!( + out.contains("--name/--arch/--product/--version/--addon"), + "{out}" + ); + assert!( + out.contains("host-000") && out.contains("host-149"), + "{out}" + ); + assert!(!out.contains("host-060"), "middle row dropped: {out}"); + } } diff --git a/crates/mtui-core/src/commands/mod.rs b/crates/mtui-core/src/commands/mod.rs index f9e01da5..82ef3dcb 100644 --- a/crates/mtui-core/src/commands/mod.rs +++ b/crates/mtui-core/src/commands/mod.rs @@ -43,6 +43,7 @@ mod openqa_overview; mod regenerate; mod reload_openqa; mod request_review; +pub(crate) mod row_budget; mod simpleset; mod updates; diff --git a/crates/mtui-core/src/commands/openqa_overview.rs b/crates/mtui-core/src/commands/openqa_overview.rs index 68b3138b..0ab0e51a 100644 --- a/crates/mtui-core/src/commands/openqa_overview.rs +++ b/crates/mtui-core/src/commands/openqa_overview.rs @@ -10,6 +10,21 @@ use crate::commands::support::{require_update, template_completion}; use crate::error::{CommandError, CommandResult}; use crate::session::Session; +use super::row_budget::{crush, row_notice}; + +/// Narrowing flags named in the row-budget notices. +const OPENQA_HINT: &str = "--no-aggregated/--aggregated-groups/--days/--test-pattern"; + +/// Non-`passed` rows survive the crush: the actionable openQA signal. +fn is_anomaly_version(row: &oqa::VersionResult) -> bool { + row.status != "passed" +} + +/// Build checks with extracted matches survive the crush. +fn is_anomaly_build(entry: &oqa::BuildCheckResult) -> bool { + !entry.matches.is_empty() +} + /// The aggregated-update job groups offered for tab completion. const AGGREGATED_GROUP_CHOICES: &[&str] = &["core", "containers", "yast", "security"]; @@ -239,9 +254,29 @@ impl Command for OpenQAOverview { session .display .println(&session.display.blue("Single incidents - Core")); - for row in &single_incidents { + // Row budget backstops many-version incidents: head+tail+anomalies. + let single = crush( + single_incidents.clone(), + |r| { + ( + r.version.clone(), + r.url.clone(), + r.status.clone(), + r.failed_count, + r.running_count, + r.note.clone(), + ) + }, + is_anomaly_version, + ); + for row in &single.kept { print_version_row(session, row); } + if single.truncated > 0 { + session + .display + .println(&row_notice(single.truncated, single.total, OPENQA_HINT)); + } if !no_aggregated { session.display.println("-------"); @@ -250,9 +285,30 @@ impl Command for OpenQAOverview { "\nAggregated updates - {}", title_case(&group.group) ))); - for row in &group.versions { + let versions = crush( + group.versions.clone(), + |r| { + ( + r.version.clone(), + r.url.clone(), + r.status.clone(), + r.failed_count, + r.running_count, + r.note.clone(), + ) + }, + is_anomaly_version, + ); + for row in &versions.kept { print_version_row(session, row); } + if versions.truncated > 0 { + session.display.println(&row_notice( + versions.truncated, + versions.total, + OPENQA_HINT, + )); + } } if aggregated.is_empty() { let msg = session @@ -279,9 +335,19 @@ impl Command for OpenQAOverview { if build_checks.is_empty() { session.display.println("No build checks for this incident"); } else { - for entry in &build_checks { + let checks = crush( + build_checks.clone(), + |e| (e.url.clone(), e.matches.clone(), e.summary.clone()), + is_anomaly_build, + ); + for entry in &checks.kept { print_build_check(session, entry); } + if checks.truncated > 0 { + session + .display + .println(&row_notice(checks.truncated, checks.total, OPENQA_HINT)); + } } if args.get_flag("export") { @@ -691,4 +757,75 @@ mod tests { buf.contents() ); } + + // ------------------------------------------------------------ row budget + + /// 150 version rows with one mid-list `failed` anomaly and exact duplicates. + fn crush_versions() -> Vec { + let mut rows: Vec = (0..150) + .map(|i| oqa::VersionResult { + version: format!("15-SP{i:03}"), + url: format!("http://oqa/{i}"), + status: "passed".to_owned(), + ..Default::default() + }) + .collect(); + rows[100].status = "failed".to_owned(); + rows[100].failed_count = 3; + rows.push(rows[0].clone()); + rows + } + + #[test] + fn row_budget_crushes_versions_and_keeps_failed_anomaly() { + use super::super::row_budget::{ROW_CAP, crush}; + let out = crush( + crush_versions(), + |r| { + ( + r.version.clone(), + r.url.clone(), + r.status.clone(), + r.failed_count, + r.running_count, + r.note.clone(), + ) + }, + is_anomaly_version, + ); + assert_eq!(out.total, 150, "deduped total"); + assert!(out.kept.len() <= ROW_CAP, "{}", out.kept.len()); + assert!(out.kept.iter().any(|r| r.status == "failed")); + assert!(!out.kept.iter().any(|r| r.version == "15-SP060")); + assert!(out.truncated > 0); + } + + #[test] + fn row_budget_crushes_build_checks_and_keeps_matches() { + use super::super::row_budget::crush; + let mut entries: Vec = (0..150) + .map(|i| oqa::BuildCheckResult { + url: format!("http://qam/{i}.log"), + ..Default::default() + }) + .collect(); + entries[100].matches = vec!["FAIL line".to_owned()]; + let out = crush( + entries, + |e| (e.url.clone(), e.matches.clone(), e.summary.clone()), + is_anomaly_build, + ); + assert!(out.kept.iter().any(|e| e.url == "http://qam/100.log")); + assert!(!out.kept.iter().any(|e| e.url == "http://qam/60.log")); + } + + #[test] + fn row_budget_notice_names_narrowing_flags() { + use super::super::row_budget::row_notice; + let n = row_notice(90, 150, OPENQA_HINT); + assert!( + n.contains("--no-aggregated/--aggregated-groups/--days"), + "{n}" + ); + } } diff --git a/crates/mtui-core/src/commands/row_budget.rs b/crates/mtui-core/src/commands/row_budget.rs new file mode 100644 index 00000000..673095ab --- /dev/null +++ b/crates/mtui-core/src/commands/row_budget.rs @@ -0,0 +1,131 @@ +//! Row-budget crush for unbounded listing outputs (SmartCrusher-lite). +//! +//! Row budget: keep first-40 + last-10 + all anomaly rows, exact-dedup identical rows, hard cap 100. + +use std::collections::HashSet; +use std::hash::Hash; + +/// Head rows always kept. +pub(crate) const ROW_HEAD: usize = 40; +/// Tail rows always kept. +pub(crate) const ROW_TAIL: usize = 10; +/// Hard cap on kept rows, anomalies included. +pub(crate) const ROW_CAP: usize = 100; + +/// Outcome of [`crush`]: the kept items plus truncation counts for the notice. +pub(crate) struct CrushOutcome { + /// Kept items in original order. + pub kept: Vec, + /// Post-dedup total the kept subset was drawn from. + pub total: usize, + /// `total - kept.len()`; zero means nothing was dropped. + pub truncated: usize, +} + +/// Crush `items` to budget, preserving order. +/// +/// Exact-dedups on `key`, then keeps head + tail + all middle anomalies up to [`ROW_CAP`]. +pub(crate) fn crush( + items: Vec, + mut key_of: impl FnMut(&T) -> K, + mut is_anomaly: impl FnMut(&T) -> bool, +) -> CrushOutcome { + // Dedup first so identical rows never consume budget twice. + let mut seen = HashSet::new(); + let mut items: Vec = items + .into_iter() + .filter(|it| seen.insert(key_of(it))) + .collect(); + let total = items.len(); + if total <= ROW_CAP { + return CrushOutcome { + kept: items, + total, + truncated: 0, + }; + } + let tail_start = total - ROW_TAIL; + let mut anomaly_idx: Vec = (ROW_HEAD..tail_start) + .filter(|&i| is_anomaly(&items[i])) + .collect(); + // Cap anomalies to what fits between head and tail. + anomaly_idx.truncate(ROW_CAP - ROW_HEAD - ROW_TAIL); + let keep: HashSet = (0..ROW_HEAD) + .chain(anomaly_idx) + .chain(tail_start..total) + .collect(); + let mut kept = Vec::with_capacity(keep.len()); + for (i, it) in items.drain(..).enumerate() { + if keep.contains(&i) { + kept.push(it); + } + } + let truncated = total - kept.len(); + CrushOutcome { + kept, + total, + truncated, + } +} + +/// Human/JSON trailing notice naming the narrowing flags. +#[must_use] +pub(crate) fn row_notice(truncated: usize, total: usize, hint: &str) -> String { + format!("…[truncated {truncated} of {total} rows; narrow with {hint}]") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn under_cap_passes_through_with_no_truncation() { + let out = crush(vec![1, 2, 3], |v| *v, |_| false); + assert_eq!(out.kept, vec![1, 2, 3]); + assert_eq!(out.truncated, 0); + } + + #[test] + fn over_cap_keeps_head_tail_and_all_middle_anomalies() { + // 0..150, anomalies at 50 and 140 (tail) plus 100. + let items: Vec = (0..150).collect(); + let out = crush(items, |v| *v, |v| *v == 50 || *v == 100 || *v == 140); + assert_eq!(out.total, 150); + // Head 0..40, anomalies 50+100, tail 140..150 (140 already in tail). + assert!(out.kept.contains(&0) && out.kept.contains(&39)); + assert!(out.kept.contains(&50) && out.kept.contains(&100)); + assert!(out.kept.contains(&140) && out.kept.contains(&149)); + assert!(!out.kept.contains(&60), "non-anomaly middle row dropped"); + assert_eq!(out.kept.len(), ROW_HEAD + ROW_TAIL + 2); + assert_eq!(out.truncated, 150 - out.kept.len()); + // Order preserved. + let mut sorted = out.kept.clone(); + sorted.sort_unstable(); + assert_eq!(sorted, out.kept); + } + + #[test] + fn anomaly_overflow_truncates_middle_first_deterministically() { + // Every middle row anomalous: only the first fitting anomalies survive. + let items: Vec = (0..300).collect(); + let out = crush(items, |v| *v, |v| *v >= ROW_HEAD); + assert_eq!(out.kept.len(), ROW_CAP); + assert!(out.kept.contains(&0)); + assert!(out.kept.contains(&299)); + } + + #[test] + fn exact_duplicates_consume_no_budget() { + let items = vec![7, 7, 7, 8, 8, 9]; + let out = crush(items, |v| *v, |_| false); + assert_eq!(out.kept, vec![7, 8, 9]); + assert_eq!(out.truncated, 0); + } + + #[test] + fn notice_names_narrowing_flags() { + let n = row_notice(90, 150, "--limit/--field/-G"); + assert!(n.contains("[truncated 90 of 150"), "{n}"); + assert!(n.contains("--limit/--field/-G"), "{n}"); + } +} diff --git a/crates/mtui-core/src/commands/updates.rs b/crates/mtui-core/src/commands/updates.rs index b6a01af2..8b3aa138 100644 --- a/crates/mtui-core/src/commands/updates.rs +++ b/crates/mtui-core/src/commands/updates.rs @@ -13,6 +13,18 @@ use crate::commands::apicall::teregen_client; use crate::error::{CommandError, CommandResult}; use crate::session::Session; +use super::row_budget::{crush, row_notice}; + +/// Narrowing flags named in the row-budget notice. +const UPDATES_HINT: &str = "--limit/--field/-G"; + +/// Non-`testing` rows survive the crush: the actionable signal under `--status all`. +fn is_anomaly_row(v: &Value) -> bool { + v.get("status") + .and_then(Value::as_str) + .is_some_and(|s| s != "testing") +} + /// The `--status` value that widens the queue to every status. const STATUS_ALL: &str = "all"; @@ -320,26 +332,45 @@ impl Command for Updates { return Ok(()); } - let shown: &[Value] = if limit > 0 && limit < rows.len() { - &rows[..limit] + let limited: Vec = if limit > 0 && limit < rows.len() { + rows.into_iter().take(limit).collect() } else { - &rows + rows }; + // Row budget backstops `--limit 0=all`: head+tail+anomalies, exact-deduped. + let crushed = crush( + limited, + |v| serde_json::to_string(v).unwrap_or_default(), + is_anomaly_row, + ); + let shown = &crushed.kept; + // --json emits the truncated array (valid JSON) plus an optional trailing notice line; strip the notice before parsing. + let notice = (crushed.truncated > 0) + .then(|| row_notice(crushed.truncated, crushed.total, UPDATES_HINT)); if as_json { - // The raw rows, nothing discarded, and no count header: stdout is - // the JSON document. let doc = Value::Array(shown.to_vec()); session.display.println( &serde_json::to_string_pretty(&doc) .expect("serialising a serde_json::Value is infallible"), ); + if let Some(notice) = notice { + session.display.println(¬ice); + } return Ok(()); } - session - .display - .println(&format!("Update queue ({}):", shown.len())); + if crushed.truncated > 0 { + session.display.println(&format!( + "Update queue ({} of {}):", + shown.len(), + crushed.total + )); + } else { + session + .display + .println(&format!("Update queue ({}):", shown.len())); + } if specs.is_empty() { for u in shown { session.display.println(&render_row(u, want_assignment)); @@ -352,6 +383,9 @@ impl Command for Updates { session.display.println(&render_fields(u, &specs)); } } + if let Some(notice) = notice { + session.display.println(¬ice); + } Ok(()) } } @@ -1598,4 +1632,83 @@ mod tests { ); assert!(cmd.try_get_matches_from(["--json"]).is_ok()); } + + // ------------------------------------------------------------ row budget + + /// 150-row queue with one mid-queue non-`testing` anomaly and exact duplicates. + fn crush_fixture() -> Vec { + let mut rows: Vec = (0..150) + .map(|i| { + serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", + "id": format!("row-{i:03}"), + }) + }) + .collect(); + rows[100] = serde_json::json!({ + "priority": 1, "status": "failed", "kind": "Maintenance", + "id": "row-anomaly", + }); + rows.push(rows[0].clone()); + rows.push(rows[1].clone()); + rows + } + + #[tokio::test] + async fn row_budget_crushes_human_queue_and_keeps_anomaly() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({"updates": crush_fixture()})), + ) + .mount(&server) + .await; + let (mut session, buf) = teregen_session(&server); + let args = matches(&Updates, &["--status", "all"]); + Updates.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + assert!(out.contains("row-anomaly"), "anomaly must survive: {out}"); + assert!(!out.contains("row-060"), "middle normal row dropped: {out}"); + assert!(out.contains("row-000") && out.contains("row-149"), "{out}"); + assert!(out.contains("[truncated"), "{out}"); + assert!(out.contains("--limit/--field/-G"), "{out}"); + assert!( + out.contains(" of 150"), + "deduped total in notice/header: {out}" + ); + } + + #[tokio::test] + async fn row_budget_json_stays_parseable_with_trailing_notice() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({"updates": crush_fixture()})), + ) + .mount(&server) + .await; + let (mut session, buf) = teregen_session(&server); + let args = matches(&Updates, &["--status", "all", "--json"]); + Updates.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + assert!(out.contains("[truncated"), "{out}"); + assert!(out.contains("--limit/--field/-G"), "{out}"); + let json_part: String = out + .lines() + .filter(|l| !l.contains("[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + let rows = parsed.as_array().unwrap(); + assert!( + rows.len() <= super::super::row_budget::ROW_CAP, + "{}", + rows.len() + ); + assert!(rows.iter().any(|r| r["id"] == "row-anomaly"), "{out}"); + } } diff --git a/crates/mtui-mcp/Cargo.toml b/crates/mtui-mcp/Cargo.toml index e11a95df..b5ec575a 100644 --- a/crates/mtui-mcp/Cargo.toml +++ b/crates/mtui-mcp/Cargo.toml @@ -90,6 +90,7 @@ mtui-types.workspace = true tokio-util = { workspace = true } futures.workspace = true tempfile = "3" +wiremock.workspace = true # Serialises the `$RUST_LOG` mutations in the `startup_filter` tests: the whole # crate's tests share one process, so the env var is a process-global. serial_test.workspace = true diff --git a/crates/mtui-mcp/tests/it.rs b/crates/mtui-mcp/tests/it.rs index fbc4224f..dfd1db3e 100644 --- a/crates/mtui-mcp/tests/it.rs +++ b/crates/mtui-mcp/tests/it.rs @@ -12,6 +12,8 @@ mod ambiguous_template; mod http_body_limit; #[path = "http_isolation.rs"] mod http_isolation; +#[path = "json_crush.rs"] +mod json_crush; #[path = "mcp_jobs.rs"] mod mcp_jobs; #[path = "nonempty_success.rs"] diff --git a/crates/mtui-mcp/tests/json_crush.rs b/crates/mtui-mcp/tests/json_crush.rs new file mode 100644 index 00000000..a43f9131 --- /dev/null +++ b/crates/mtui-mcp/tests/json_crush.rs @@ -0,0 +1,133 @@ +//! Row-budget crush reaches the MCP client intact (STEP 1). +//! +//! Drives the real `updates` / `list_refhosts` commands through +//! [`McpSession::run_command`] with unbounded mocked backends: the JSON output +//! still parses (after stripping the trailing notice) and the notice names the +//! narrowing flags. Also pins that this step changed no tool schemas. + +#![cfg(feature = "mcp")] + +use mtui_config::Config; +use mtui_core::register_all; +use mtui_mcp::McpSession; + +/// 150 TeReGen rows with one mid-queue non-`testing` anomaly. +fn queue_fixture() -> serde_json::Value { + let mut rows: Vec = (0..150) + .map(|i| { + serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", + "id": format!("row-{i:03}"), + }) + }) + .collect(); + rows[100] = serde_json::json!({ + "priority": 1, "status": "failed", "kind": "Maintenance", + "id": "row-anomaly", + }); + serde_json::json!({"updates": rows}) +} + +/// `updates --json` over an unbounded queue: valid JSON, anomaly kept, notice names flags. +#[tokio::test] +async fn updates_json_crush_parses_and_names_flags() { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with(ResponseTemplate::new(200).set_body_json(queue_fixture())) + .mount(&server) + .await; + + let mut config = Config::default(); + config.teregen_api = server.uri(); + let sess = McpSession::new(config); + let registry = register_all(); + + let argv = ["--status", "all", "--json"] + .iter() + .map(|s| (*s).to_owned()) + .collect::>(); + let out = sess + .run_command(®istry, "updates", &argv) + .await + .expect("updates succeeds"); + assert!(out.contains("[truncated"), "{out}"); + assert!(out.contains("--limit/--field/-G"), "{out}"); + let json_part: String = out + .lines() + .filter(|l| !l.contains("[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + let rows = parsed.as_array().unwrap(); + assert!(rows.len() <= 100, "row budget holds: {}", rows.len()); + assert!(rows.iter().any(|r| r["id"] == "row-anomaly"), "{out}"); + assert!(!rows.iter().any(|r| r["id"] == "row-060"), "{out}"); +} + +/// `list_refhosts` over a 150-host inventory: head+tail kept, notice names filters. +#[tokio::test] +async fn list_refhosts_crush_notifies_with_narrowing_flags() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("refhosts.yml"); + let mut yaml = String::from("default:\n"); + for i in 0..150 { + yaml.push_str(&format!( + " - name: host-{i:03}\n arch: x86_64\n product:\n name: sles\n version:\n major: 15\n minor: 6\n" + )); + } + std::fs::write(&path, yaml).unwrap(); + + let mut config = Config::default(); + config.refhosts_resolvers = "path".to_owned(); + config.refhosts_path = path; + let sess = McpSession::new(config); + let registry = register_all(); + + let out = sess + .run_command(®istry, "list_refhosts", &[]) + .await + .expect("list_refhosts succeeds"); + assert!(out.contains("[truncated"), "{out}"); + assert!( + out.contains("--name/--arch/--product/--version/--addon"), + "{out}" + ); + assert!( + out.contains("host-000") && out.contains("host-149"), + "{out}" + ); + assert!(!out.contains("host-060"), "middle row dropped: {out}"); +} + +/// This step is output-only: the three crushed tools keep their schemas. +#[test] +fn crushed_tool_schemas_unchanged() { + use std::collections::HashMap; + + use mtui_mcp::build_tools; + + let tools: HashMap> = build_tools(®ister_all()) + .into_iter() + .map(|d| { + let props = d + .input_schema + .get("properties") + .and_then(|v| v.as_object()) + .map(|m| m.keys().cloned().collect::>()) + .unwrap_or_default(); + (d.name.clone(), props) + }) + .collect(); + for name in ["updates", "list_refhosts", "openqa_overview"] { + assert!(tools.contains_key(name), "tool {name} renamed?"); + } + // No budget flags were added: `updates` keeps its `--limit`, the other two + // gain none. + assert!(tools["updates"].contains(&"limit".to_owned())); + assert!(!tools["list_refhosts"].contains(&"limit".to_owned())); + assert!(!tools["openqa_overview"].contains(&"limit".to_owned())); +} From b511524673dcf15939e99c19153446ea8787d7e6 Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Wed, 9 Sep 2026 17:37:54 +0200 Subject: [PATCH 2/8] fix(mcp): document truncation, page middle rows, keep unknown status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --json help states the over-cap protocol (valid array + trailing …[truncated line, strip prefix). updates gains --offset, list_refhosts gains --limit/--offset (pre-crush paging, fits --limit plumbing). Unknown/missing/null status is anomaly-keep; row-cap != byte-cap noted (~600 openqa total); export still full. --- CHANGELOG.md | 17 +- .../mtui-core/src/commands/list_refhosts.rs | 99 +++++++- .../mtui-core/src/commands/openqa_overview.rs | 48 ++++ crates/mtui-core/src/commands/row_budget.rs | 22 +- crates/mtui-core/src/commands/updates.rs | 226 ++++++++++++++++-- crates/mtui-mcp/tests/json_crush.rs | 115 ++++++++- ...slimmed_command_tool_schemas_snapshot.snap | 21 +- docs/src/cli.md | 21 +- 8 files changed, 516 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 829ae470..b19e1fe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,15 +48,22 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht endpoint URL acknowledges with the userinfo-stripped value. - Unbounded listings now crush to a row budget — first-40 + last-10 + all anomaly rows, exact-deduped, hard cap 100 — instead of dumping thousands of - rows into the client's context: `updates` (anomaly: non-`testing` status; - narrow with `--limit/--field/-G`), `list_refhosts` (anomaly: non-`free` lock - or pool claim; narrow with `--name/--arch/--product/--version/--addon`), and + rows into the client's context: `updates` (anomaly: non-`testing` status, + unknown/missing/null status kept; narrow with `--limit/--offset/--field/-G`), + `list_refhosts` (anomaly: non-`free` lock or pool claim; narrow with + `--limit/--offset/--name/--arch/--product/--version/--addon`), and `openqa_overview` (anomaly: non-`passed` version rows, build checks with matches; display only, `--export` still writes the full overview; narrow with `--no-aggregated/--aggregated-groups/--days/--test-pattern`). `--json` still prints a valid JSON array (truncated) with the - `…[truncated N of M rows; …]` notice on a trailing line — strip it before - parsing. MCP tool names/schemas unchanged. + `…[truncated N of M rows; …]` notice on a trailing line — strip lines starting + with that prefix before parsing (also in `--json` help). Any middle slice is + recoverable via pre-crush `--offset`/`--limit` paging (chosen over an + explicit-window notice as it fits the existing `--limit` plumbing). + Row-cap is not byte-cap: MCP `max_output_bytes` can still cut mid-array on + huge rows; `openqa_overview` per-section caps sum to ~600 rows total. + **MCP schema note:** additive only — `updates` gains `offset`, `list_refhosts` + gains `limit`/`offset`; no renames/removals. ### Deprecated diff --git a/crates/mtui-core/src/commands/list_refhosts.rs b/crates/mtui-core/src/commands/list_refhosts.rs index 9b8568af..858e37b8 100644 --- a/crates/mtui-core/src/commands/list_refhosts.rs +++ b/crates/mtui-core/src/commands/list_refhosts.rs @@ -33,7 +33,7 @@ use crate::session::Session; use super::row_budget::{crush, row_notice}; /// Narrowing flags named in the row-budget notice. -const REFHOSTS_HINT: &str = "--name/--arch/--product/--version/--addon"; +const REFHOSTS_HINT: &str = "--limit/--offset/--name/--arch/--product/--version/--addon"; /// Non-`free` or pool-claimed rows survive the crush: the actionable lock signal. fn is_anomaly_record(r: &Record) -> bool { @@ -303,7 +303,26 @@ impl Command for ListRefhosts { Arg::new("json") .long("json") .action(ArgAction::SetTrue) - .help("emit JSON"), + .help( + "emit JSON array of kept rows; over-cap output adds a trailing `…[truncated …` \ + notice line — strip lines starting with that prefix before parsing", + ), + ) + .arg( + Arg::new("limit") + .long("limit") + .value_name("N") + .value_parser(clap::value_parser!(usize)) + .default_value("0") + .help("cap the number of rows after --offset (0 = all)"), + ) + .arg( + Arg::new("offset") + .long("offset") + .value_name("N") + .value_parser(clap::value_parser!(usize)) + .default_value("0") + .help("skip the first N rows (0 = from the start); with --limit, page any middle slice"), ) .arg( Arg::new("free") @@ -334,6 +353,8 @@ impl Command for ListRefhosts { &["--addon"], &["--pool"], &["--json"], + &["--limit"], + &["--offset"], &["--free"], &["-v", "--verbose"], ], @@ -374,6 +395,8 @@ impl Command for ListRefhosts { let free = args.get_flag("free"); let verbose = args.get_flag("verbose"); let as_json = args.get_flag("json"); + let limit = args.get_one::("limit").copied().unwrap_or(0); + let offset = args.get_one::("offset").copied().unwrap_or(0); let filters = Filters { testplatform: args.get_one::("testplatform").map(String::as_str), @@ -391,9 +414,20 @@ impl Command for ListRefhosts { probe_locks(&ProbeConfig::new(&config), &mut records).await; } + // Paging via --offset (pre-crush) makes any middle slice recoverable; chosen over an + // explicit-window notice as it fits the existing --limit plumbing. + let windowed: Vec = { + let skipped = offset.min(records.len()); + let mut v: Vec = records.into_iter().skip(skipped).collect(); + if limit > 0 && limit < v.len() { + v.truncate(limit); + } + v + }; // Row budget backstops the whole-inventory dump: head+tail+anomalies, exact-deduped. + // Row-cap is not byte-cap: MCP max_output_bytes can still cut mid-array on huge rows. let crushed = crush( - records, + windowed, |r| { ( r.name.clone(), @@ -875,6 +909,8 @@ mod tests { "--addon", "--pool", "--json", + "--limit", + "--offset", "--free", "-v", "--verbose", @@ -909,6 +945,10 @@ mod tests { "sdk", "--pool", "--json", + "--limit", + "10", + "--offset", + "5", "--free", "-v", ], @@ -931,6 +971,8 @@ mod tests { assert!(args.get_flag("json")); assert!(args.get_flag("free")); assert!(args.get_flag("verbose")); + assert_eq!(args.get_one::("limit").copied(), Some(10)); + assert_eq!(args.get_one::("offset").copied(), Some(5)); } /// A session resolving refhosts from a local `path` file, plus the temp dir @@ -1077,12 +1119,18 @@ default: text.push('\n'); text.push_str(&row_notice(out.truncated, out.total, REFHOSTS_HINT)); assert!( - text.contains("--name/--arch/--product/--version/--addon"), + text.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{text}" + ); + assert!( + text.contains("--limit/--offset/--name/--arch/--product/--version/--addon"), "{text}" ); let json_part: String = text .lines() - .filter(|l| !l.contains("[truncated")) + .filter(|l| !l.starts_with("…[truncated")) .collect::>() .join("\n"); let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); @@ -1103,9 +1151,14 @@ default: let args = matches(&ListRefhosts, &[]); ListRefhosts.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - assert!(out.contains("[truncated"), "{out}"); assert!( - out.contains("--name/--arch/--product/--version/--addon"), + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + assert!( + out.contains("--limit/--offset/--name/--arch/--product/--version/--addon"), "{out}" ); assert!( @@ -1114,4 +1167,36 @@ default: ); assert!(!out.contains("host-060"), "middle row dropped: {out}"); } + + #[test] + fn json_help_mentions_truncation() { + let base = clap::Command::new("list_refhosts").no_binary_name(true); + let mut cmd = ListRefhosts.configure(base); + let help = cmd.render_help().to_string(); + assert!(help.contains("…[truncated"), "{help}"); + assert!(help.contains("strip lines starting with"), "{help}"); + } + + #[tokio::test] + async fn middle_row_recoverable_via_offset_limit() { + use crate::commands::testkit::matches; + let mut yaml = String::from("default:\n"); + for i in 0..150 { + yaml.push_str(&format!( + " - name: host-{i:03}\n arch: x86_64\n product:\n name: sles\n version:\n major: 15\n minor: 6\n" + )); + } + let (mut session, buf, _dir) = session_with_refhosts_file(&yaml); + let args = matches(&ListRefhosts, &["--offset", "50", "--limit", "50"]); + ListRefhosts.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + assert!( + out.contains("host-060"), + "paged middle row must appear: {out}" + ); + assert!( + !out.contains("…[truncated"), + "50-row window fits budget: {out}" + ); + } } diff --git a/crates/mtui-core/src/commands/openqa_overview.rs b/crates/mtui-core/src/commands/openqa_overview.rs index 0ab0e51a..146eb2c8 100644 --- a/crates/mtui-core/src/commands/openqa_overview.rs +++ b/crates/mtui-core/src/commands/openqa_overview.rs @@ -13,6 +13,9 @@ use crate::session::Session; use super::row_budget::{crush, row_notice}; /// Narrowing flags named in the row-budget notices. +// Per-section caps sum to ~600 rows total (single 100 + up to 4 aggregated groups + +// build checks 100); row-cap is not byte-cap: MCP max_output_bytes can still cut +// mid-display on huge rows, while --export always writes the full overview. const OPENQA_HINT: &str = "--no-aggregated/--aggregated-groups/--days/--test-pattern"; /// Non-`passed` rows survive the crush: the actionable openQA signal. @@ -823,9 +826,54 @@ mod tests { fn row_budget_notice_names_narrowing_flags() { use super::super::row_budget::row_notice; let n = row_notice(90, 150, OPENQA_HINT); + assert!(n.starts_with("…[truncated"), "{n}"); assert!( n.contains("--no-aggregated/--aggregated-groups/--days"), "{n}" ); } + + #[tokio::test] + async fn export_bypasses_crush_keeps_dropped_middle() { + // 150 passed rows: crush would drop the middle, but --export writes the full overview. + let versions: Vec = (0..150) + .map(|i| oqa::VersionResult { + version: format!("15-SP{i:03}"), + url: format!("http://oqa/{i}"), + status: "passed".to_owned(), + ..Default::default() + }) + .collect(); + let crushed = super::super::row_budget::crush( + versions.clone(), + |r| { + ( + r.version.clone(), + r.url.clone(), + r.status.clone(), + r.failed_count, + r.running_count, + r.note.clone(), + ) + }, + is_anomaly_version, + ); + assert!(!crushed.kept.iter().any(|r| r.version == "15-SP060")); + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("log"); + std::fs::write( + &log, + "comment: hi\n\nregression tests:\n-----------------\n\n", + ) + .unwrap(); + let (mut session, _buf) = session_with_hosts("SUSE:Maintenance:1:1", &["h1"], "ok"); + session.metadata_mut().base_mut().path = Some(log.clone()); + export_to_testreport(&mut session, &versions, &[], &[], true).unwrap(); + let written = std::fs::read_to_string(&log).unwrap(); + assert!(written.contains("15-SP060"), "{written}"); + assert!( + written.contains("15-SP000") && written.contains("15-SP149"), + "{written}" + ); + } } diff --git a/crates/mtui-core/src/commands/row_budget.rs b/crates/mtui-core/src/commands/row_budget.rs index 673095ab..30d9571c 100644 --- a/crates/mtui-core/src/commands/row_budget.rs +++ b/crates/mtui-core/src/commands/row_budget.rs @@ -1,6 +1,7 @@ //! Row-budget crush for unbounded listing outputs (SmartCrusher-lite). //! //! Row budget: keep first-40 + last-10 + all anomaly rows, exact-dedup identical rows, hard cap 100. +//! Row-cap is not byte-cap: MCP `max_output_bytes` can still cut mid-array on huge rows. use std::collections::HashSet; use std::hash::Hash; @@ -106,12 +107,26 @@ mod tests { #[test] fn anomaly_overflow_truncates_middle_first_deterministically() { - // Every middle row anomalous: only the first fitting anomalies survive. + // Every middle row anomalous: only the first fitting anomalies survive, in index order. let items: Vec = (0..300).collect(); let out = crush(items, |v| *v, |v| *v >= ROW_HEAD); assert_eq!(out.kept.len(), ROW_CAP); - assert!(out.kept.contains(&0)); - assert!(out.kept.contains(&299)); + let expected: Vec = (0..ROW_HEAD) + .chain(ROW_HEAD..ROW_HEAD + (ROW_CAP - ROW_HEAD - ROW_TAIL)) + .chain(300 - ROW_TAIL..300) + .collect(); + assert_eq!(out.kept, expected); + } + + #[test] + fn boundary_100_passes_101_crushes() { + let out100 = crush((0..100).collect::>(), |v| *v, |_| false); + assert_eq!(out100.truncated, 0); + assert_eq!(out100.kept.len(), 100); + let out101 = crush((0..101).collect::>(), |v| *v, |_| false); + assert_eq!(out101.total, 101); + assert_eq!(out101.kept.len(), ROW_HEAD + ROW_TAIL); + assert_eq!(out101.truncated, 101 - (ROW_HEAD + ROW_TAIL)); } #[test] @@ -125,6 +140,7 @@ mod tests { #[test] fn notice_names_narrowing_flags() { let n = row_notice(90, 150, "--limit/--field/-G"); + assert!(n.starts_with("…[truncated"), "{n}"); assert!(n.contains("[truncated 90 of 150"), "{n}"); assert!(n.contains("--limit/--field/-G"), "{n}"); } diff --git a/crates/mtui-core/src/commands/updates.rs b/crates/mtui-core/src/commands/updates.rs index 8b3aa138..33095989 100644 --- a/crates/mtui-core/src/commands/updates.rs +++ b/crates/mtui-core/src/commands/updates.rs @@ -16,13 +16,11 @@ use crate::session::Session; use super::row_budget::{crush, row_notice}; /// Narrowing flags named in the row-budget notice. -const UPDATES_HINT: &str = "--limit/--field/-G"; +const UPDATES_HINT: &str = "--limit/--offset/--field/-G"; -/// Non-`testing` rows survive the crush: the actionable signal under `--status all`. +/// Non-`testing` rows survive the crush; unknown/missing/null status keeps too (safe direction). fn is_anomaly_row(v: &Value) -> bool { - v.get("status") - .and_then(Value::as_str) - .is_some_and(|s| s != "testing") + v.get("status").and_then(Value::as_str) != Some("testing") } /// The `--status` value that widens the queue to every status. @@ -91,10 +89,11 @@ impl Command for Updates { .action(ArgAction::SetTrue) .conflicts_with("field") .help( - "print the raw TeReGen rows as a pretty-printed JSON array \ - (--limit-capped, each row emitted whole, unlike -F; not \ - combinable with -F); an empty queue prints []; narrow large \ - queues with --limit", + "print the raw TeReGen rows as a JSON array (each row \ + emitted whole, unlike -F; honours --limit/--offset; not combinable \ + with -F); an empty queue prints []; over-cap output is a valid JSON array \ + of kept rows plus a trailing `…[truncated …` notice line — strip lines \ + starting with that prefix before parsing", ), ) .arg( @@ -110,7 +109,15 @@ impl Command for Updates { .value_name("N") .value_parser(clap::value_parser!(usize)) .default_value("0") - .help("cap the number of rows (0 = all)"), + .help("cap the number of rows after --offset (0 = all)"), + ) + .arg( + Arg::new("offset") + .long("offset") + .value_name("N") + .value_parser(clap::value_parser!(usize)) + .default_value("0") + .help("skip the first N rows (0 = from the start); with --limit, page any middle slice"), ) .arg( Arg::new("assignee") @@ -169,6 +176,7 @@ impl Command for Updates { &[ &["--status"], &["--limit"], + &["--offset"], &["--assignee"], &["--mine"], &["--all-assignees"], @@ -202,6 +210,7 @@ impl Command for Updates { .cloned() .unwrap_or_else(|| "testing".to_owned()); let limit = args.get_one::("limit").copied().unwrap_or(0); + let offset = args.get_one::("offset").copied().unwrap_or(0); let mine = args.get_flag("mine"); let all_assignees = args.get_flag("all_assignees"); @@ -332,15 +341,21 @@ impl Command for Updates { return Ok(()); } - let limited: Vec = if limit > 0 && limit < rows.len() { - rows.into_iter().take(limit).collect() - } else { - rows + // Paging via --offset (pre-crush) makes any middle slice recoverable; chosen over an + // explicit-window notice as it fits the existing --limit plumbing. + let windowed: Vec = { + let skipped = offset.min(rows.len()); + let mut v: Vec = rows.into_iter().skip(skipped).collect(); + if limit > 0 && limit < v.len() { + v.truncate(limit); + } + v }; // Row budget backstops `--limit 0=all`: head+tail+anomalies, exact-deduped. + // Row-cap is not byte-cap: MCP max_output_bytes can still cut mid-array on huge rows. let crushed = crush( - limited, - |v| serde_json::to_string(v).unwrap_or_default(), + windowed, + |v| serde_json::to_string(v).expect("serialising a serde_json::Value is infallible"), is_anomaly_row, ); let shown = &crushed.kept; @@ -741,6 +756,7 @@ mod tests { "-G", "--status", "--limit", + "--offset", "--assignee", "--mine", "--all-assignees", @@ -1672,8 +1688,13 @@ mod tests { assert!(out.contains("row-anomaly"), "anomaly must survive: {out}"); assert!(!out.contains("row-060"), "middle normal row dropped: {out}"); assert!(out.contains("row-000") && out.contains("row-149"), "{out}"); - assert!(out.contains("[truncated"), "{out}"); - assert!(out.contains("--limit/--field/-G"), "{out}"); + assert!( + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); assert!( out.contains(" of 150"), "deduped total in notice/header: {out}" @@ -1695,11 +1716,16 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - assert!(out.contains("[truncated"), "{out}"); - assert!(out.contains("--limit/--field/-G"), "{out}"); + assert!( + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); let json_part: String = out .lines() - .filter(|l| !l.contains("[truncated")) + .filter(|l| !l.starts_with("…[truncated")) .collect::>() .join("\n"); let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); @@ -1711,4 +1737,162 @@ mod tests { ); assert!(rows.iter().any(|r| r["id"] == "row-anomaly"), "{out}"); } + + #[test] + fn json_help_mentions_truncation() { + let base = clap::Command::new("updates").no_binary_name(true); + let mut cmd = Updates.configure(base); + let help = cmd.render_help().to_string(); + assert!(help.contains("…[truncated"), "{help}"); + assert!(help.contains("strip lines starting with"), "{help}"); + } + + #[test] + fn unknown_status_is_anomaly_keep() { + for row in [ + serde_json::json!({"id": "a"}), + serde_json::json!({"id": "b", "status": null}), + serde_json::json!({"id": "c", "status": "weird"}), + serde_json::json!({"id": "d", "status": 5}), + ] { + assert!(is_anomaly_row(&row), "{row}"); + } + assert!(!is_anomaly_row(&serde_json::json!({"status": "testing"}))); + assert!(is_anomaly_row(&serde_json::json!({"status": "failed"}))); + } + + #[tokio::test] + async fn unknown_status_rows_survive_crush() { + let mut rows: Vec = (0..150) + .map(|i| { + serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", + "id": format!("row-{i:03}"), + }) + }) + .collect(); + rows[100] = serde_json::json!({"priority": 1, "kind": "Maintenance", "id": "row-nostatus"}); + rows[101] = serde_json::json!({"priority": 1, "status": null, "kind": "Maintenance", "id": "row-null"}); + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({"updates": rows})), + ) + .mount(&server) + .await; + let (mut session, buf) = teregen_session(&server); + let args = matches(&Updates, &["--status", "all", "--json"]); + Updates.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + let kept: Vec<_> = parsed + .as_array() + .unwrap() + .iter() + .map(|r| r["id"].as_str().unwrap_or("?")) + .collect(); + assert!(kept.contains(&"row-nostatus"), "{out}"); + assert!(kept.contains(&"row-null"), "{out}"); + } + + #[tokio::test] + async fn middle_row_recoverable_via_offset() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({"updates": crush_fixture()})), + ) + .mount(&server) + .await; + let (mut session, buf) = teregen_session(&server); + let args = matches( + &Updates, + &["--status", "all", "--offset", "50", "--limit", "50"], + ); + Updates.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + assert!( + out.contains("row-060"), + "paged middle row must appear: {out}" + ); + assert!( + !out.contains("…[truncated"), + "50-row window fits budget: {out}" + ); + } + + #[tokio::test] + async fn boundary_100_no_notice_101_truncated() { + for (n, expect_notice) in [(100, false), (101, true)] { + let rows: Vec = (0..n) + .map(|i| { + serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", + "id": format!("row-{i:03}"), + }) + }) + .collect(); + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({"updates": rows})), + ) + .mount(&server) + .await; + let (mut session, buf) = teregen_session(&server); + let args = matches(&Updates, &["--status", "all", "--json"]); + Updates.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + let has_notice = out + .lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")); + assert_eq!(has_notice, expect_notice, "n={n}: {out}"); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + assert!(parsed.is_array(), "n={n}: {out}"); + } + } + + #[tokio::test] + async fn json_filter_keeps_data_line_containing_truncated() { + // A data line containing "[truncated" must not be mistaken for the notice: + // only lines starting with the `…[truncated` prefix are stripped. + let rows = vec![serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", + "id": "row-[truncated]-fake", + })]; + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({"updates": rows})), + ) + .mount(&server) + .await; + let (mut session, buf) = teregen_session(&server); + let args = matches(&Updates, &["--status", "all", "--json"]); + Updates.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + assert_eq!(parsed[0]["id"], "row-[truncated]-fake", "{out}"); + } } diff --git a/crates/mtui-mcp/tests/json_crush.rs b/crates/mtui-mcp/tests/json_crush.rs index a43f9131..85bf86f5 100644 --- a/crates/mtui-mcp/tests/json_crush.rs +++ b/crates/mtui-mcp/tests/json_crush.rs @@ -1,9 +1,10 @@ -//! Row-budget crush reaches the MCP client intact (STEP 1). +//! Row-budget crush reaches the MCP client intact. //! //! Drives the real `updates` / `list_refhosts` commands through //! [`McpSession::run_command`] with unbounded mocked backends: the JSON output //! still parses (after stripping the trailing notice) and the notice names the -//! narrowing flags. Also pins that this step changed no tool schemas. +//! narrowing flags. Also pins the additive paging flags and that row-cap is not +//! byte-cap. #![cfg(feature = "mcp")] @@ -54,11 +55,16 @@ async fn updates_json_crush_parses_and_names_flags() { .run_command(®istry, "updates", &argv) .await .expect("updates succeeds"); - assert!(out.contains("[truncated"), "{out}"); - assert!(out.contains("--limit/--field/-G"), "{out}"); + assert!( + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); let json_part: String = out .lines() - .filter(|l| !l.contains("[truncated")) + .filter(|l| !l.starts_with("…[truncated")) .collect::>() .join("\n"); let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); @@ -91,9 +97,14 @@ async fn list_refhosts_crush_notifies_with_narrowing_flags() { .run_command(®istry, "list_refhosts", &[]) .await .expect("list_refhosts succeeds"); - assert!(out.contains("[truncated"), "{out}"); assert!( - out.contains("--name/--arch/--product/--version/--addon"), + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + assert!( + out.contains("--limit/--offset/--name/--arch/--product/--version/--addon"), "{out}" ); assert!( @@ -103,7 +114,87 @@ async fn list_refhosts_crush_notifies_with_narrowing_flags() { assert!(!out.contains("host-060"), "middle row dropped: {out}"); } -/// This step is output-only: the three crushed tools keep their schemas. +/// Paging recovers a dropped middle row through MCP. +#[tokio::test] +async fn updates_offset_recovers_middle_row() { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with(ResponseTemplate::new(200).set_body_json(queue_fixture())) + .mount(&server) + .await; + + let mut config = Config::default(); + config.teregen_api = server.uri(); + let sess = McpSession::new(config); + let registry = register_all(); + + let argv = [ + "--status", "all", "--json", "--offset", "50", "--limit", "50", + ] + .iter() + .map(|s| (*s).to_owned()) + .collect::>(); + let out = sess + .run_command(®istry, "updates", &argv) + .await + .expect("updates succeeds"); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + let rows = parsed.as_array().unwrap(); + assert!(rows.iter().any(|r| r["id"] == "row-060"), "{out}"); +} + +/// Row-cap is not byte-cap: fat rows still hit max_output_bytes after crush. +#[tokio::test] +async fn fat_rows_hit_byte_cap_after_crush() { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let filler = "x".repeat(1024); + let rows: Vec = (0..150) + .map(|i| { + serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", + "id": format!("row-{i:03}"), "title": filler, + }) + }) + .collect(); + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/updates")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({"updates": rows})), + ) + .mount(&server) + .await; + + let mut config = Config::default(); + config.teregen_api = server.uri(); + config.mcp_max_output_bytes = 2000; + let sess = McpSession::new(config); + let registry = register_all(); + + let argv = ["--status", "all", "--json"] + .iter() + .map(|s| (*s).to_owned()) + .collect::>(); + let out = sess + .run_command(®istry, "updates", &argv) + .await + .expect("updates succeeds"); + assert!(out.contains("max_output_bytes=2000"), "{out}"); + assert!(out.contains("bytes"), "{out}"); +} + +/// Additive paging flags only: no tool renames/removals. #[test] fn crushed_tool_schemas_unchanged() { use std::collections::HashMap; @@ -125,9 +216,11 @@ fn crushed_tool_schemas_unchanged() { for name in ["updates", "list_refhosts", "openqa_overview"] { assert!(tools.contains_key(name), "tool {name} renamed?"); } - // No budget flags were added: `updates` keeps its `--limit`, the other two - // gain none. + // Paging is additive: `updates` gains `--offset`, `list_refhosts` gains + // `--limit`/`--offset`, `openqa_overview` gains none. assert!(tools["updates"].contains(&"limit".to_owned())); - assert!(!tools["list_refhosts"].contains(&"limit".to_owned())); + assert!(tools["updates"].contains(&"offset".to_owned())); + assert!(tools["list_refhosts"].contains(&"limit".to_owned())); + assert!(tools["list_refhosts"].contains(&"offset".to_owned())); assert!(!tools["openqa_overview"].contains(&"limit".to_owned())); } diff --git a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap index 905ceace..9836ccbc 100644 --- a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap +++ b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap @@ -606,13 +606,23 @@ expression: pretty }, "json": { "default": false, - "description": "emit JSON", + "description": "emit JSON array of kept rows; over-cap output adds a trailing `…[truncated …` notice line — strip lines starting with that prefix before parsing", "type": "boolean" }, + "limit": { + "default": 0, + "description": "cap the number of rows after --offset (0 = all)", + "type": "integer" + }, "name": { "description": "hostname glob, e.g. 'whale-*' or '*.qam.suse.cz'", "type": "string" }, + "offset": { + "default": 0, + "description": "skip the first N rows (0 = from the start); with --limit, page any middle slice", + "type": "integer" + }, "pool": { "default": false, "description": "group by test-target slot (product+version+arch+addons)", @@ -1659,12 +1669,12 @@ expression: pretty }, "json": { "default": false, - "description": "print the raw TeReGen rows as a pretty-printed JSON array (--limit-capped, each row emitted whole, unlike -F; not combinable with -F); an empty queue prints []; narrow large queues with --limit", + "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap output is a valid JSON array of kept rows plus a trailing `…[truncated …` notice line — strip lines starting with that prefix before parsing", "type": "boolean" }, "limit": { "default": 0, - "description": "cap the number of rows (0 = all)", + "description": "cap the number of rows after --offset (0 = all)", "type": "integer" }, "mine": { @@ -1672,6 +1682,11 @@ expression: pretty "description": "filter to updates assigned to the current session user", "type": "boolean" }, + "offset": { + "default": 0, + "description": "skip the first N rows (0 = from the start); with --limit, page any middle slice", + "type": "integer" + }, "review_group": { "description": "filter by review group as the bare group name, e.g. qam-sle (not the '-review' login form, which classic rows lack); repeatable — groups are OR-ed (one server query per group)", "items": { diff --git a/docs/src/cli.md b/docs/src/cli.md index c05779e3..08209d7c 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -710,7 +710,7 @@ Options: select output fields by osc-qam name (e.g. -F Rating -F 'Assigned Roles'); repeatable, rendered as one block per update; names match case-insensitively, ignoring spaces/hyphens/underscores; narrow large queues with --limit --json - print the raw TeReGen rows as a pretty-printed JSON array (--limit-capped, each row emitted whole, unlike -F; not combinable with -F); an empty queue prints []; narrow large queues with --limit + print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap output is a valid JSON array of kept rows plus a trailing `…[truncated …` notice line — strip lines starting with that prefix before parsing --status filter by status (default: testing); use 'all' for every status @@ -718,7 +718,12 @@ Options: [default: testing] --limit - cap the number of rows (0 = all) + cap the number of rows after --offset (0 = all) + + [default: 0] + + --offset + skip the first N rows (0 = from the start); with --limit, page any middle slice [default: 0] @@ -1069,7 +1074,17 @@ Options: group by test-target slot (product+version+arch+addons) --json - emit JSON + emit JSON array of kept rows; over-cap output adds a trailing `…[truncated …` notice line — strip lines starting with that prefix before parsing + + --limit + cap the number of rows after --offset (0 = all) + + [default: 0] + + --offset + skip the first N rows (0 = from the start); with --limit, page any middle slice + + [default: 0] --free also probe live operation-lock and pool-claim state (connects to each matched host) From 2d98adaaf5085fccffeee37e76cf4df4fa86d7af Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Thu, 10 Sep 2026 09:12:38 +0200 Subject: [PATCH 3/8] fix(mcp): keep --json valid on truncation, borrow crush keys --json truncation notice goes to stderr so stdout stays a valid JSON array; updates dedups on (id, status, priority); openqa crush borrows slices instead of cloning bulk vecs. --- CHANGELOG.md | 12 +- .../mtui-core/src/commands/list_refhosts.rs | 75 ++++++------- .../mtui-core/src/commands/openqa_overview.rs | 77 +++++++------ crates/mtui-core/src/commands/row_budget.rs | 69 +++++++++++- crates/mtui-core/src/commands/updates.rs | 105 ++++++++++-------- crates/mtui-mcp/tests/json_crush.rs | 35 ++---- ...slimmed_command_tool_schemas_snapshot.snap | 4 +- docs/src/cli.md | 4 +- 8 files changed, 218 insertions(+), 163 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b19e1fe0..031d62c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,12 +52,12 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht unknown/missing/null status kept; narrow with `--limit/--offset/--field/-G`), `list_refhosts` (anomaly: non-`free` lock or pool claim; narrow with `--limit/--offset/--name/--arch/--product/--version/--addon`), and - `openqa_overview` (anomaly: non-`passed` version rows, build checks with - matches; display only, `--export` still writes the full overview; narrow with - `--no-aggregated/--aggregated-groups/--days/--test-pattern`). `--json` still - prints a valid JSON array (truncated) with the - `…[truncated N of M rows; …]` notice on a trailing line — strip lines starting - with that prefix before parsing (also in `--json` help). Any middle slice is + `openqa_overview` (anomaly: non-`passed` version rows, build checks with + matches; display only, `--export` still writes the full overview; narrow with + `--no-aggregated/--aggregated-groups/--days/--test-pattern`). `--json` + over-cap stdout is still a valid JSON array of kept rows; the + `…[truncated N of M rows; …]` notice goes to stderr (human-readable output + keeps it inline; also in `--json` help). Any middle slice is recoverable via pre-crush `--offset`/`--limit` paging (chosen over an explicit-window notice as it fits the existing `--limit` plumbing). Row-cap is not byte-cap: MCP `max_output_bytes` can still cut mid-array on diff --git a/crates/mtui-core/src/commands/list_refhosts.rs b/crates/mtui-core/src/commands/list_refhosts.rs index 858e37b8..74c2ed8f 100644 --- a/crates/mtui-core/src/commands/list_refhosts.rs +++ b/crates/mtui-core/src/commands/list_refhosts.rs @@ -304,8 +304,8 @@ impl Command for ListRefhosts { .long("json") .action(ArgAction::SetTrue) .help( - "emit JSON array of kept rows; over-cap output adds a trailing `…[truncated …` \ - notice line — strip lines starting with that prefix before parsing", + "emit a JSON array of kept rows; over-cap stdout is still a \ + valid JSON array, the `…[truncated …` notice goes to stderr", ), ) .arg( @@ -442,13 +442,14 @@ impl Command for ListRefhosts { }, is_anomaly_record, ); - // --json emits the truncated array (valid JSON) plus an optional trailing notice line; strip the notice before parsing. + // Human output keeps the notice inline; --json routes it to stderr. let notice = (crushed.truncated > 0) .then(|| row_notice(crushed.truncated, crushed.total, REFHOSTS_HINT)); if as_json { session.display.println(&render_json(&crushed.kept)); + // Stdout stays strictly valid JSON; the notice goes to stderr. if let Some(notice) = notice { - session.display.println(¬ice); + eprintln!("{notice}"); } return Ok(()); } @@ -1096,46 +1097,31 @@ default: assert!(out.truncated > 0); } - #[test] - fn row_budget_json_stays_parseable_with_trailing_notice() { - use super::super::row_budget::{crush, row_notice}; - let out = crush( - crush_records(), - |r| { - ( - r.name.clone(), - r.arch.clone(), - r.product.clone(), - r.version.clone(), - r.addons.clone(), - r.slot.clone(), - r.lock.clone(), - r.pool.clone(), - ) - }, - is_anomaly_record, - ); - let mut text = render_json(&out.kept); - text.push('\n'); - text.push_str(&row_notice(out.truncated, out.total, REFHOSTS_HINT)); - assert!( - text.lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")), - "{text}" - ); + #[tokio::test] + async fn json_over_cap_stdout_stays_valid_with_notice_on_stderr() { + use crate::commands::testkit::matches; + let mut yaml = String::from("default:\n"); + for i in 0..150 { + yaml.push_str(&format!( + " - name: host-{i:03}\n arch: x86_64\n product:\n name: sles\n version:\n major: 15\n minor: 6\n" + )); + } + let (mut session, buf, _dir) = session_with_refhosts_file(&yaml); + let args = matches(&ListRefhosts, &["--json"]); + ListRefhosts.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + // The notice went to stderr: stdout parses as-is, middle dropped. + assert!(!out.contains("…[truncated"), "{out}"); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let arr = parsed.as_array().unwrap(); assert!( - text.contains("--limit/--offset/--name/--arch/--product/--version/--addon"), - "{text}" + arr.len() <= super::super::row_budget::ROW_CAP, + "{}", + arr.len() ); - let json_part: String = text - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); - let arr = parsed.as_array().unwrap(); - assert!(arr.iter().any(|r| r["name"] == "host-anomaly")); + assert!(arr.iter().any(|r| r["name"] == "host-000"), "{out}"); + assert!(arr.iter().any(|r| r["name"] == "host-149"), "{out}"); + assert!(!arr.iter().any(|r| r["name"] == "host-060"), "{out}"); } #[tokio::test] @@ -1169,12 +1155,13 @@ default: } #[test] - fn json_help_mentions_truncation() { + fn json_help_routes_notice_to_stderr() { let base = clap::Command::new("list_refhosts").no_binary_name(true); let mut cmd = ListRefhosts.configure(base); let help = cmd.render_help().to_string(); assert!(help.contains("…[truncated"), "{help}"); - assert!(help.contains("strip lines starting with"), "{help}"); + assert!(help.contains("valid JSON array"), "{help}"); + assert!(help.contains("stderr"), "{help}"); } #[tokio::test] diff --git a/crates/mtui-core/src/commands/openqa_overview.rs b/crates/mtui-core/src/commands/openqa_overview.rs index 146eb2c8..13f06eac 100644 --- a/crates/mtui-core/src/commands/openqa_overview.rs +++ b/crates/mtui-core/src/commands/openqa_overview.rs @@ -10,7 +10,7 @@ use crate::commands::support::{require_update, template_completion}; use crate::error::{CommandError, CommandResult}; use crate::session::Session; -use super::row_budget::{crush, row_notice}; +use super::row_budget::{crush_slice, row_notice}; /// Narrowing flags named in the row-budget notices. // Per-section caps sum to ~600 rows total (single 100 + up to 4 aggregated groups + @@ -258,16 +258,16 @@ impl Command for OpenQAOverview { .display .println(&session.display.blue("Single incidents - Core")); // Row budget backstops many-version incidents: head+tail+anomalies. - let single = crush( - single_incidents.clone(), - |r| { + let single = crush_slice( + &single_incidents, + |r: &oqa::VersionResult| { ( - r.version.clone(), - r.url.clone(), - r.status.clone(), + r.version.as_str(), + r.url.as_str(), + r.status.as_str(), r.failed_count, r.running_count, - r.note.clone(), + r.note.as_str(), ) }, is_anomaly_version, @@ -288,16 +288,16 @@ impl Command for OpenQAOverview { "\nAggregated updates - {}", title_case(&group.group) ))); - let versions = crush( - group.versions.clone(), - |r| { + let versions = crush_slice( + &group.versions, + |r: &oqa::VersionResult| { ( - r.version.clone(), - r.url.clone(), - r.status.clone(), + r.version.as_str(), + r.url.as_str(), + r.status.as_str(), r.failed_count, r.running_count, - r.note.clone(), + r.note.as_str(), ) }, is_anomaly_version, @@ -338,9 +338,11 @@ impl Command for OpenQAOverview { if build_checks.is_empty() { session.display.println("No build checks for this incident"); } else { - let checks = crush( - build_checks.clone(), - |e| (e.url.clone(), e.matches.clone(), e.summary.clone()), + let checks = crush_slice( + &build_checks, + |e: &oqa::BuildCheckResult| { + (e.url.as_str(), e.matches.as_slice(), e.summary.as_str()) + }, is_anomaly_build, ); for entry in &checks.kept { @@ -781,17 +783,18 @@ mod tests { #[test] fn row_budget_crushes_versions_and_keeps_failed_anomaly() { - use super::super::row_budget::{ROW_CAP, crush}; - let out = crush( - crush_versions(), - |r| { + use super::super::row_budget::{ROW_CAP, crush_slice}; + let rows = crush_versions(); + let out = crush_slice( + &rows, + |r: &oqa::VersionResult| { ( - r.version.clone(), - r.url.clone(), - r.status.clone(), + r.version.as_str(), + r.url.as_str(), + r.status.as_str(), r.failed_count, r.running_count, - r.note.clone(), + r.note.as_str(), ) }, is_anomaly_version, @@ -805,7 +808,7 @@ mod tests { #[test] fn row_budget_crushes_build_checks_and_keeps_matches() { - use super::super::row_budget::crush; + use super::super::row_budget::crush_slice; let mut entries: Vec = (0..150) .map(|i| oqa::BuildCheckResult { url: format!("http://qam/{i}.log"), @@ -813,9 +816,9 @@ mod tests { }) .collect(); entries[100].matches = vec!["FAIL line".to_owned()]; - let out = crush( - entries, - |e| (e.url.clone(), e.matches.clone(), e.summary.clone()), + let out = crush_slice( + &entries, + |e: &oqa::BuildCheckResult| (e.url.as_str(), e.matches.as_slice(), e.summary.as_str()), is_anomaly_build, ); assert!(out.kept.iter().any(|e| e.url == "http://qam/100.log")); @@ -844,16 +847,16 @@ mod tests { ..Default::default() }) .collect(); - let crushed = super::super::row_budget::crush( - versions.clone(), - |r| { + let crushed = super::super::row_budget::crush_slice( + &versions, + |r: &oqa::VersionResult| { ( - r.version.clone(), - r.url.clone(), - r.status.clone(), + r.version.as_str(), + r.url.as_str(), + r.status.as_str(), r.failed_count, r.running_count, - r.note.clone(), + r.note.as_str(), ) }, is_anomaly_version, diff --git a/crates/mtui-core/src/commands/row_budget.rs b/crates/mtui-core/src/commands/row_budget.rs index 30d9571c..77291fda 100644 --- a/crates/mtui-core/src/commands/row_budget.rs +++ b/crates/mtui-core/src/commands/row_budget.rs @@ -33,7 +33,7 @@ pub(crate) fn crush( ) -> CrushOutcome { // Dedup first so identical rows never consume budget twice. let mut seen = HashSet::new(); - let mut items: Vec = items + let items: Vec = items .into_iter() .filter(|it| seen.insert(key_of(it))) .collect(); @@ -46,9 +46,50 @@ pub(crate) fn crush( }; } let tail_start = total - ROW_TAIL; - let mut anomaly_idx: Vec = (ROW_HEAD..tail_start) + let anomaly_idx: Vec = (ROW_HEAD..tail_start) .filter(|&i| is_anomaly(&items[i])) .collect(); + keep_head_anomaly_tail(items, anomaly_idx, tail_start, total) +} + +/// Crush `items` to budget, preserving order. +/// +/// Exact-dedups on `key`, then keeps head + tail + all middle anomalies up to [`ROW_CAP`]. +/// Borrowed twin for already-owned data (openQA overview): same keep, no bulk clone. +pub(crate) fn crush_slice<'a, T, K: Eq + Hash>( + items: &'a [T], + mut key_of: impl FnMut(&'a T) -> K, + mut is_anomaly: impl FnMut(&'a T) -> bool, +) -> CrushOutcome<&'a T> { + let mut seen = HashSet::new(); + let mut uniq: Vec<&'a T> = Vec::new(); + for it in items { + if seen.insert(key_of(it)) { + uniq.push(it); + } + } + let total = uniq.len(); + if total <= ROW_CAP { + return CrushOutcome { + kept: uniq, + total, + truncated: 0, + }; + } + let tail_start = total - ROW_TAIL; + let anomaly_idx: Vec = (ROW_HEAD..tail_start) + .filter(|&i| is_anomaly(uniq[i])) + .collect(); + keep_head_anomaly_tail(uniq, anomaly_idx, tail_start, total) +} + +/// Shared head + capped-anomaly + tail keep, order-preserving. +fn keep_head_anomaly_tail( + mut items: Vec, + mut anomaly_idx: Vec, + tail_start: usize, + total: usize, +) -> CrushOutcome { // Cap anomalies to what fits between head and tail. anomaly_idx.truncate(ROW_CAP - ROW_HEAD - ROW_TAIL); let keep: HashSet = (0..ROW_HEAD) @@ -144,4 +185,28 @@ mod tests { assert!(n.contains("[truncated 90 of 150"), "{n}"); assert!(n.contains("--limit/--field/-G"), "{n}"); } + + #[test] + fn slice_matches_owned_keep_without_cloning() { + // Borrowed keys prove the no-clone path compiles and keeps identically. + let items: Vec = (0..150).collect(); + let owned = crush(items.clone(), |v| *v, |v| *v == 100); + let borrowed = crush_slice(&items, |v| *v, |v| *v == 100); + assert_eq!(borrowed.total, owned.total); + assert_eq!(borrowed.truncated, owned.truncated); + assert_eq!(borrowed.kept, owned.kept.iter().collect::>()); + assert!(borrowed.kept.contains(&&100)); + assert!(!borrowed.kept.contains(&&60)); + } + + #[test] + fn slice_borrows_string_keys_and_dedups() { + let rows: Vec<(String, String)> = (0..150) + .map(|i| (format!("v-{i:03}"), "passed".to_owned())) + .collect(); + let out = crush_slice(&rows, |(v, s)| (v.as_str(), s.as_str()), |_| false); + assert_eq!(out.total, 150); + assert_eq!(out.kept.len(), ROW_HEAD + ROW_TAIL); + assert_eq!(out.kept[0].0, "v-000"); + } } diff --git a/crates/mtui-core/src/commands/updates.rs b/crates/mtui-core/src/commands/updates.rs index 33095989..5395d466 100644 --- a/crates/mtui-core/src/commands/updates.rs +++ b/crates/mtui-core/src/commands/updates.rs @@ -23,6 +23,22 @@ fn is_anomaly_row(v: &Value) -> bool { v.get("status").and_then(Value::as_str) != Some("testing") } +/// Lightweight dedup key: id/status/priority only, not the whole serialised row. +fn update_key( + v: &Value, +) -> ( + Option, + Option, + Option, + Option, +) { + let field = |k: &str| v.get(k).map(ToString::to_string); + let id = field("id"); + // Id-less rows have no stable identity: fall back to the full row there. + let rest = id.is_none().then(|| v.to_string()); + (id, field("status"), field("priority"), rest) +} + /// The `--status` value that widens the queue to every status. const STATUS_ALL: &str = "all"; @@ -91,9 +107,9 @@ impl Command for Updates { .help( "print the raw TeReGen rows as a JSON array (each row \ emitted whole, unlike -F; honours --limit/--offset; not combinable \ - with -F); an empty queue prints []; over-cap output is a valid JSON array \ - of kept rows plus a trailing `…[truncated …` notice line — strip lines \ - starting with that prefix before parsing", + with -F); an empty queue prints []; over-cap stdout is still a \ + valid JSON array of kept rows, the `…[truncated …` notice goes \ + to stderr", ), ) .arg( @@ -353,13 +369,9 @@ impl Command for Updates { }; // Row budget backstops `--limit 0=all`: head+tail+anomalies, exact-deduped. // Row-cap is not byte-cap: MCP max_output_bytes can still cut mid-array on huge rows. - let crushed = crush( - windowed, - |v| serde_json::to_string(v).expect("serialising a serde_json::Value is infallible"), - is_anomaly_row, - ); + let crushed = crush(windowed, update_key, is_anomaly_row); let shown = &crushed.kept; - // --json emits the truncated array (valid JSON) plus an optional trailing notice line; strip the notice before parsing. + // Human output keeps the notice inline; --json routes it to stderr. let notice = (crushed.truncated > 0) .then(|| row_notice(crushed.truncated, crushed.total, UPDATES_HINT)); @@ -369,8 +381,9 @@ impl Command for Updates { &serde_json::to_string_pretty(&doc) .expect("serialising a serde_json::Value is infallible"), ); + // Stdout stays strictly valid JSON; the notice goes to stderr. if let Some(notice) = notice { - session.display.println(¬ice); + eprintln!("{notice}"); } return Ok(()); } @@ -1702,7 +1715,7 @@ mod tests { } #[tokio::test] - async fn row_budget_json_stays_parseable_with_trailing_notice() { + async fn row_budget_json_stdout_stays_valid_with_notice_on_stderr() { let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/updates")) @@ -1716,19 +1729,9 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - assert!( - out.lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")), - "{out}" - ); - assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + // The notice went to stderr: stdout parses as-is and names no flags. + assert!(!out.contains("…[truncated"), "{out}"); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); let rows = parsed.as_array().unwrap(); assert!( rows.len() <= super::super::row_budget::ROW_CAP, @@ -1736,15 +1739,33 @@ mod tests { rows.len() ); assert!(rows.iter().any(|r| r["id"] == "row-anomaly"), "{out}"); + assert!(!rows.iter().any(|r| r["id"] == "row-060"), "{out}"); } #[test] - fn json_help_mentions_truncation() { + fn json_help_routes_notice_to_stderr() { let base = clap::Command::new("updates").no_binary_name(true); let mut cmd = Updates.configure(base); let help = cmd.render_help().to_string(); assert!(help.contains("…[truncated"), "{help}"); - assert!(help.contains("strip lines starting with"), "{help}"); + assert!(help.contains("valid JSON array"), "{help}"); + assert!(help.contains("stderr"), "{help}"); + } + + #[test] + fn update_key_is_id_status_priority_with_idless_fallback() { + // Same identity fields dedup even when the rest differs (coarser than + // full-row serialisation, by design: one id is one update). + let a = serde_json::json!({"id": "x", "status": "testing", "priority": 1, "title": "one"}); + let b = serde_json::json!({"id": "x", "status": "testing", "priority": 1, "title": "two"}); + assert_eq!(update_key(&a), update_key(&b)); + let c = serde_json::json!({"id": "y", "status": "testing", "priority": 1}); + assert_ne!(update_key(&a), update_key(&c)); + // Id-less rows keep exact-dedup: distinct rows stay distinct. + let u1 = serde_json::json!({"status": "testing", "priority": 1, "title": "one"}); + let u2 = serde_json::json!({"status": "testing", "priority": 1, "title": "two"}); + assert_ne!(update_key(&u1), update_key(&u2)); + assert_eq!(update_key(&u1), update_key(&u1.clone())); } #[test] @@ -1785,12 +1806,9 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + // Notice on stderr: stdout parses as-is. + assert!(!out.contains("…[truncated"), "{out}"); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); let kept: Vec<_> = parsed .as_array() .unwrap() @@ -1830,8 +1848,8 @@ mod tests { } #[tokio::test] - async fn boundary_100_no_notice_101_truncated() { - for (n, expect_notice) in [(100, false), (101, true)] { + async fn boundary_100_101_json_stdout_stays_valid() { + for (n, expect_kept) in [(100, 100), (101, 50)] { let rows: Vec = (0..n) .map(|i| { serde_json::json!({ @@ -1852,18 +1870,15 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - let has_notice = out - .lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")); - assert_eq!(has_notice, expect_notice, "n={n}: {out}"); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + // --json never carries the notice, truncated or not: it is on stderr. + assert!(!out.contains("…[truncated"), "n={n}: {out}"); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); assert!(parsed.is_array(), "n={n}: {out}"); + assert_eq!( + parsed.as_array().unwrap().len(), + expect_kept, + "n={n}: {out}" + ); } } diff --git a/crates/mtui-mcp/tests/json_crush.rs b/crates/mtui-mcp/tests/json_crush.rs index 85bf86f5..8e4898e2 100644 --- a/crates/mtui-mcp/tests/json_crush.rs +++ b/crates/mtui-mcp/tests/json_crush.rs @@ -1,10 +1,10 @@ //! Row-budget crush reaches the MCP client intact. //! //! Drives the real `updates` / `list_refhosts` commands through -//! [`McpSession::run_command`] with unbounded mocked backends: the JSON output -//! still parses (after stripping the trailing notice) and the notice names the -//! narrowing flags. Also pins the additive paging flags and that row-cap is not -//! byte-cap. +//! [`McpSession::run_command`] with unbounded mocked backends: `--json` tool +//! output parses as-is (the truncation notice goes to stderr, keeping stdout +//! valid JSON) while human output keeps the notice naming the narrowing flags. +//! Also pins the additive paging flags and that row-cap is not byte-cap. #![cfg(feature = "mcp")] @@ -29,9 +29,9 @@ fn queue_fixture() -> serde_json::Value { serde_json::json!({"updates": rows}) } -/// `updates --json` over an unbounded queue: valid JSON, anomaly kept, notice names flags. +/// `updates --json` over an unbounded queue: stdout stays valid JSON, anomaly kept. #[tokio::test] -async fn updates_json_crush_parses_and_names_flags() { +async fn updates_json_crush_stays_valid() { use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -55,19 +55,9 @@ async fn updates_json_crush_parses_and_names_flags() { .run_command(®istry, "updates", &argv) .await .expect("updates succeeds"); - assert!( - out.lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")), - "{out}" - ); - assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + // Notice on stderr: tool output parses as-is with no trailing notice line. + assert!(!out.contains("…[truncated"), "{out}"); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); let rows = parsed.as_array().unwrap(); assert!(rows.len() <= 100, "row budget holds: {}", rows.len()); assert!(rows.iter().any(|r| r["id"] == "row-anomaly"), "{out}"); @@ -142,12 +132,7 @@ async fn updates_offset_recovers_middle_row() { .run_command(®istry, "updates", &argv) .await .expect("updates succeeds"); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); let rows = parsed.as_array().unwrap(); assert!(rows.iter().any(|r| r["id"] == "row-060"), "{out}"); } diff --git a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap index 9836ccbc..5e40d217 100644 --- a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap +++ b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap @@ -606,7 +606,7 @@ expression: pretty }, "json": { "default": false, - "description": "emit JSON array of kept rows; over-cap output adds a trailing `…[truncated …` notice line — strip lines starting with that prefix before parsing", + "description": "emit a JSON array of kept rows; over-cap stdout is still a valid JSON array, the `…[truncated …` notice goes to stderr", "type": "boolean" }, "limit": { @@ -1669,7 +1669,7 @@ expression: pretty }, "json": { "default": false, - "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap output is a valid JSON array of kept rows plus a trailing `…[truncated …` notice line — strip lines starting with that prefix before parsing", + "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is still a valid JSON array of kept rows, the `…[truncated …` notice goes to stderr", "type": "boolean" }, "limit": { diff --git a/docs/src/cli.md b/docs/src/cli.md index 08209d7c..6b2df507 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -710,7 +710,7 @@ Options: select output fields by osc-qam name (e.g. -F Rating -F 'Assigned Roles'); repeatable, rendered as one block per update; names match case-insensitively, ignoring spaces/hyphens/underscores; narrow large queues with --limit --json - print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap output is a valid JSON array of kept rows plus a trailing `…[truncated …` notice line — strip lines starting with that prefix before parsing + print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is still a valid JSON array of kept rows, the `…[truncated …` notice goes to stderr --status filter by status (default: testing); use 'all' for every status @@ -1074,7 +1074,7 @@ Options: group by test-target slot (product+version+arch+addons) --json - emit JSON array of kept rows; over-cap output adds a trailing `…[truncated …` notice line — strip lines starting with that prefix before parsing + emit a JSON array of kept rows; over-cap stdout is still a valid JSON array, the `…[truncated …` notice goes to stderr --limit cap the number of rows after --offset (0 = all) From c13ca9070b44108d8827b608f0c74484d44d30d4 Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Thu, 10 Sep 2026 10:17:48 +0200 Subject: [PATCH 4/8] fix(mcp): document silent MCP --json truncation, drop strip-protocol test Both --json helps now state MCP results carry no truncation signal and MUST page with --limit/--offset; stderr clause kept CLI-only. CHANGELOG, cli.md, slimmed snapshot regen. Vestigial strip test now parses stdout as-is with fake row preserved. --- CHANGELOG.md | 6 ++- .../mtui-core/src/commands/list_refhosts.rs | 9 +++- crates/mtui-core/src/commands/updates.rs | 45 +++++++++++++------ ...slimmed_command_tool_schemas_snapshot.snap | 4 +- docs/src/cli.md | 4 +- 5 files changed, 47 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 031d62c4..c4260c99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,8 +56,10 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht matches; display only, `--export` still writes the full overview; narrow with `--no-aggregated/--aggregated-groups/--days/--test-pattern`). `--json` over-cap stdout is still a valid JSON array of kept rows; the - `…[truncated N of M rows; …]` notice goes to stderr (human-readable output - keeps it inline; also in `--json` help). Any middle slice is + `…[truncated N of M rows; …]` notice goes to stderr on the CLI (human-readable output + keeps it inline; also in `--json` help) but MCP tool results capture stdout only + and carry no truncation signal — MUST page with `--limit`/`--offset` to be sure + of completeness. Any middle slice is recoverable via pre-crush `--offset`/`--limit` paging (chosen over an explicit-window notice as it fits the existing `--limit` plumbing). Row-cap is not byte-cap: MCP `max_output_bytes` can still cut mid-array on diff --git a/crates/mtui-core/src/commands/list_refhosts.rs b/crates/mtui-core/src/commands/list_refhosts.rs index 74c2ed8f..35395ac5 100644 --- a/crates/mtui-core/src/commands/list_refhosts.rs +++ b/crates/mtui-core/src/commands/list_refhosts.rs @@ -305,7 +305,9 @@ impl Command for ListRefhosts { .action(ArgAction::SetTrue) .help( "emit a JSON array of kept rows; over-cap stdout is still a \ - valid JSON array, the `…[truncated …` notice goes to stderr", + valid JSON array, the `…[truncated …` notice goes to stderr on \ + the CLI; MCP results carry no truncation signal, MUST page with \ + --limit/--offset to be sure of completeness", ), ) .arg( @@ -1162,6 +1164,11 @@ default: assert!(help.contains("…[truncated"), "{help}"); assert!(help.contains("valid JSON array"), "{help}"); assert!(help.contains("stderr"), "{help}"); + assert!( + help.contains("MCP results carry no truncation signal"), + "{help}" + ); + assert!(help.contains("MUST page with --limit/--offset"), "{help}"); } #[tokio::test] diff --git a/crates/mtui-core/src/commands/updates.rs b/crates/mtui-core/src/commands/updates.rs index 5395d466..58fd593a 100644 --- a/crates/mtui-core/src/commands/updates.rs +++ b/crates/mtui-core/src/commands/updates.rs @@ -109,7 +109,8 @@ impl Command for Updates { emitted whole, unlike -F; honours --limit/--offset; not combinable \ with -F); an empty queue prints []; over-cap stdout is still a \ valid JSON array of kept rows, the `…[truncated …` notice goes \ - to stderr", + to stderr on the CLI; MCP results carry no truncation signal, \ + MUST page with --limit/--offset to be sure of completeness", ), ) .arg( @@ -1750,6 +1751,11 @@ mod tests { assert!(help.contains("…[truncated"), "{help}"); assert!(help.contains("valid JSON array"), "{help}"); assert!(help.contains("stderr"), "{help}"); + assert!( + help.contains("MCP results carry no truncation signal"), + "{help}" + ); + assert!(help.contains("MUST page with --limit/--offset"), "{help}"); } #[test] @@ -1883,13 +1889,21 @@ mod tests { } #[tokio::test] - async fn json_filter_keeps_data_line_containing_truncated() { - // A data line containing "[truncated" must not be mistaken for the notice: - // only lines starting with the `…[truncated` prefix are stripped. - let rows = vec![serde_json::json!({ - "priority": 1, "status": "testing", "kind": "Maintenance", + async fn json_data_row_containing_truncated_parses_as_is() { + // No strip protocol: --json stdout parses as-is, so a data id + // containing "[truncated" survives verbatim with no notice line on stdout. + let mut rows: Vec = (0..150) + .map(|i| { + serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", + "id": format!("row-{i:03}"), + }) + }) + .collect(); + rows[100] = serde_json::json!({ + "priority": 1, "status": "failed", "kind": "Maintenance", "id": "row-[truncated]-fake", - })]; + }); let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/updates")) @@ -1902,12 +1916,15 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); - assert_eq!(parsed[0]["id"], "row-[truncated]-fake", "{out}"); + assert!(!out.contains("…[truncated"), "{out}"); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + assert!( + parsed + .as_array() + .unwrap() + .iter() + .any(|r| r["id"] == "row-[truncated]-fake"), + "{out}" + ); } } diff --git a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap index 5e40d217..1dd029d9 100644 --- a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap +++ b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap @@ -606,7 +606,7 @@ expression: pretty }, "json": { "default": false, - "description": "emit a JSON array of kept rows; over-cap stdout is still a valid JSON array, the `…[truncated …` notice goes to stderr", + "description": "emit a JSON array of kept rows; over-cap stdout is still a valid JSON array, the `…[truncated …` notice goes to stderr on the CLI; MCP results carry no truncation signal, MUST page with --limit/--offset to be sure of completeness", "type": "boolean" }, "limit": { @@ -1669,7 +1669,7 @@ expression: pretty }, "json": { "default": false, - "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is still a valid JSON array of kept rows, the `…[truncated …` notice goes to stderr", + "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is still a valid JSON array of kept rows, the `…[truncated …` notice goes to stderr on the CLI; MCP results carry no truncation signal, MUST page with --limit/--offset to be sure of completeness", "type": "boolean" }, "limit": { diff --git a/docs/src/cli.md b/docs/src/cli.md index 6b2df507..40fbe2e4 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -710,7 +710,7 @@ Options: select output fields by osc-qam name (e.g. -F Rating -F 'Assigned Roles'); repeatable, rendered as one block per update; names match case-insensitively, ignoring spaces/hyphens/underscores; narrow large queues with --limit --json - print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is still a valid JSON array of kept rows, the `…[truncated …` notice goes to stderr + print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is still a valid JSON array of kept rows, the `…[truncated …` notice goes to stderr on the CLI; MCP results carry no truncation signal, MUST page with --limit/--offset to be sure of completeness --status filter by status (default: testing); use 'all' for every status @@ -1074,7 +1074,7 @@ Options: group by test-target slot (product+version+arch+addons) --json - emit a JSON array of kept rows; over-cap stdout is still a valid JSON array, the `…[truncated …` notice goes to stderr + emit a JSON array of kept rows; over-cap stdout is still a valid JSON array, the `…[truncated …` notice goes to stderr on the CLI; MCP results carry no truncation signal, MUST page with --limit/--offset to be sure of completeness --limit cap the number of rows after --offset (0 = all) From ddb38c2716cf3b23e4b5b14dda3046ab2d50dcda Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Sat, 12 Sep 2026 13:47:20 +0200 Subject: [PATCH 5/8] fix(mcp): carry --json row-budget notice in-band for CLI+MCP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Over-cap --json stdout is a JSON array plus a trailing …[truncated notice line, like the byte-cap convention, so MCP captures the signal too; naive parse fails loudly, strip …[truncated lines. Keeps crush/offset/anomaly, tuple keys, slice path. --- CHANGELOG.md | 10 +- .../mtui-core/src/commands/list_refhosts.rs | 44 ++++--- crates/mtui-core/src/commands/updates.rs | 122 ++++++++++-------- crates/mtui-mcp/tests/json_crush.rs | 37 ++++-- ...slimmed_command_tool_schemas_snapshot.snap | 4 +- docs/src/cli.md | 4 +- 6 files changed, 133 insertions(+), 88 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4260c99..e2ffcec3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,11 +55,11 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht `openqa_overview` (anomaly: non-`passed` version rows, build checks with matches; display only, `--export` still writes the full overview; narrow with `--no-aggregated/--aggregated-groups/--days/--test-pattern`). `--json` - over-cap stdout is still a valid JSON array of kept rows; the - `…[truncated N of M rows; …]` notice goes to stderr on the CLI (human-readable output - keeps it inline; also in `--json` help) but MCP tool results capture stdout only - and carry no truncation signal — MUST page with `--limit`/`--offset` to be sure - of completeness. Any middle slice is + over-cap stdout is a JSON array of kept rows plus a trailing + `…[truncated N of M rows; …]` notice line in-band (CLI and MCP alike, like + the byte-cap convention; also in `--json` help) — naive parse of full + stdout fails loudly, strip lines starting with that prefix before parsing. + Any middle slice is recoverable via pre-crush `--offset`/`--limit` paging (chosen over an explicit-window notice as it fits the existing `--limit` plumbing). Row-cap is not byte-cap: MCP `max_output_bytes` can still cut mid-array on diff --git a/crates/mtui-core/src/commands/list_refhosts.rs b/crates/mtui-core/src/commands/list_refhosts.rs index 35395ac5..12ca7815 100644 --- a/crates/mtui-core/src/commands/list_refhosts.rs +++ b/crates/mtui-core/src/commands/list_refhosts.rs @@ -304,10 +304,9 @@ impl Command for ListRefhosts { .long("json") .action(ArgAction::SetTrue) .help( - "emit a JSON array of kept rows; over-cap stdout is still a \ - valid JSON array, the `…[truncated …` notice goes to stderr on \ - the CLI; MCP results carry no truncation signal, MUST page with \ - --limit/--offset to be sure of completeness", + "emit a JSON array of kept rows; over-cap stdout adds a trailing \ + `…[truncated …` notice line — naive parse of full stdout fails, \ + strip lines starting with that prefix before parsing", ), ) .arg( @@ -444,14 +443,14 @@ impl Command for ListRefhosts { }, is_anomaly_record, ); - // Human output keeps the notice inline; --json routes it to stderr. + // Over-cap --json stays in-band like the byte-cap convention: JSON array + // plus a trailing notice line, so MCP captures the signal too. let notice = (crushed.truncated > 0) .then(|| row_notice(crushed.truncated, crushed.total, REFHOSTS_HINT)); if as_json { session.display.println(&render_json(&crushed.kept)); - // Stdout stays strictly valid JSON; the notice goes to stderr. if let Some(notice) = notice { - eprintln!("{notice}"); + session.display.println(¬ice); } return Ok(()); } @@ -1100,7 +1099,7 @@ default: } #[tokio::test] - async fn json_over_cap_stdout_stays_valid_with_notice_on_stderr() { + async fn row_budget_json_stays_parseable_with_trailing_notice() { use crate::commands::testkit::matches; let mut yaml = String::from("default:\n"); for i in 0..150 { @@ -1112,9 +1111,22 @@ default: let args = matches(&ListRefhosts, &["--json"]); ListRefhosts.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - // The notice went to stderr: stdout parses as-is, middle dropped. - assert!(!out.contains("…[truncated"), "{out}"); - let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + assert!( + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + assert!( + serde_json::from_str::(out.trim()).is_err(), + "naive parse of full stdout must fail loudly: {out}" + ); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); let arr = parsed.as_array().unwrap(); assert!( arr.len() <= super::super::row_budget::ROW_CAP, @@ -1157,18 +1169,12 @@ default: } #[test] - fn json_help_routes_notice_to_stderr() { + fn json_help_mentions_truncation() { let base = clap::Command::new("list_refhosts").no_binary_name(true); let mut cmd = ListRefhosts.configure(base); let help = cmd.render_help().to_string(); assert!(help.contains("…[truncated"), "{help}"); - assert!(help.contains("valid JSON array"), "{help}"); - assert!(help.contains("stderr"), "{help}"); - assert!( - help.contains("MCP results carry no truncation signal"), - "{help}" - ); - assert!(help.contains("MUST page with --limit/--offset"), "{help}"); + assert!(help.contains("strip lines starting with"), "{help}"); } #[tokio::test] diff --git a/crates/mtui-core/src/commands/updates.rs b/crates/mtui-core/src/commands/updates.rs index 58fd593a..c30abb4a 100644 --- a/crates/mtui-core/src/commands/updates.rs +++ b/crates/mtui-core/src/commands/updates.rs @@ -107,10 +107,10 @@ impl Command for Updates { .help( "print the raw TeReGen rows as a JSON array (each row \ emitted whole, unlike -F; honours --limit/--offset; not combinable \ - with -F); an empty queue prints []; over-cap stdout is still a \ - valid JSON array of kept rows, the `…[truncated …` notice goes \ - to stderr on the CLI; MCP results carry no truncation signal, \ - MUST page with --limit/--offset to be sure of completeness", + with -F); an empty queue prints []; over-cap stdout is a JSON \ + array of kept rows plus a trailing `…[truncated …` notice line — \ + naive parse of full stdout fails, strip lines starting with that \ + prefix before parsing", ), ) .arg( @@ -372,7 +372,8 @@ impl Command for Updates { // Row-cap is not byte-cap: MCP max_output_bytes can still cut mid-array on huge rows. let crushed = crush(windowed, update_key, is_anomaly_row); let shown = &crushed.kept; - // Human output keeps the notice inline; --json routes it to stderr. + // Over-cap --json stays in-band like the byte-cap convention: JSON array + // plus a trailing notice line, so MCP captures the signal too. let notice = (crushed.truncated > 0) .then(|| row_notice(crushed.truncated, crushed.total, UPDATES_HINT)); @@ -382,9 +383,8 @@ impl Command for Updates { &serde_json::to_string_pretty(&doc) .expect("serialising a serde_json::Value is infallible"), ); - // Stdout stays strictly valid JSON; the notice goes to stderr. if let Some(notice) = notice { - eprintln!("{notice}"); + session.display.println(¬ice); } return Ok(()); } @@ -1716,7 +1716,7 @@ mod tests { } #[tokio::test] - async fn row_budget_json_stdout_stays_valid_with_notice_on_stderr() { + async fn row_budget_json_stays_parseable_with_trailing_notice() { let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/updates")) @@ -1730,9 +1730,23 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - // The notice went to stderr: stdout parses as-is and names no flags. - assert!(!out.contains("…[truncated"), "{out}"); - let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + assert!( + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); + assert!( + serde_json::from_str::(out.trim()).is_err(), + "naive parse of full stdout must fail loudly: {out}" + ); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); let rows = parsed.as_array().unwrap(); assert!( rows.len() <= super::super::row_budget::ROW_CAP, @@ -1744,18 +1758,12 @@ mod tests { } #[test] - fn json_help_routes_notice_to_stderr() { + fn json_help_mentions_truncation() { let base = clap::Command::new("updates").no_binary_name(true); let mut cmd = Updates.configure(base); let help = cmd.render_help().to_string(); assert!(help.contains("…[truncated"), "{help}"); - assert!(help.contains("valid JSON array"), "{help}"); - assert!(help.contains("stderr"), "{help}"); - assert!( - help.contains("MCP results carry no truncation signal"), - "{help}" - ); - assert!(help.contains("MUST page with --limit/--offset"), "{help}"); + assert!(help.contains("strip lines starting with"), "{help}"); } #[test] @@ -1812,9 +1820,18 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - // Notice on stderr: stdout parses as-is. - assert!(!out.contains("…[truncated"), "{out}"); - let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + assert!( + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); let kept: Vec<_> = parsed .as_array() .unwrap() @@ -1854,8 +1871,8 @@ mod tests { } #[tokio::test] - async fn boundary_100_101_json_stdout_stays_valid() { - for (n, expect_kept) in [(100, 100), (101, 50)] { + async fn boundary_100_no_notice_101_truncated() { + for (n, expect_notice, expect_kept) in [(100, false, 100), (101, true, 50)] { let rows: Vec = (0..n) .map(|i| { serde_json::json!({ @@ -1876,9 +1893,23 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - // --json never carries the notice, truncated or not: it is on stderr. - assert!(!out.contains("…[truncated"), "n={n}: {out}"); - let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let has_notice = out + .lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")); + assert_eq!(has_notice, expect_notice, "n={n}: {out}"); + if expect_notice { + assert!( + serde_json::from_str::(out.trim()).is_err(), + "naive parse must fail when notice present: n={n}: {out}" + ); + } + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); assert!(parsed.is_array(), "n={n}: {out}"); assert_eq!( parsed.as_array().unwrap().len(), @@ -1889,21 +1920,13 @@ mod tests { } #[tokio::test] - async fn json_data_row_containing_truncated_parses_as_is() { - // No strip protocol: --json stdout parses as-is, so a data id - // containing "[truncated" survives verbatim with no notice line on stdout. - let mut rows: Vec = (0..150) - .map(|i| { - serde_json::json!({ - "priority": 1, "status": "testing", "kind": "Maintenance", - "id": format!("row-{i:03}"), - }) - }) - .collect(); - rows[100] = serde_json::json!({ - "priority": 1, "status": "failed", "kind": "Maintenance", + async fn json_filter_keeps_data_line_containing_truncated() { + // A data line containing "[truncated" must not be mistaken for the notice: + // only lines starting with the `…[truncated` prefix are stripped. + let rows = vec![serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", "id": "row-[truncated]-fake", - }); + })]; let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/updates")) @@ -1916,15 +1939,12 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - assert!(!out.contains("…[truncated"), "{out}"); - let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); - assert!( - parsed - .as_array() - .unwrap() - .iter() - .any(|r| r["id"] == "row-[truncated]-fake"), - "{out}" - ); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + assert_eq!(parsed[0]["id"], "row-[truncated]-fake", "{out}"); } } diff --git a/crates/mtui-mcp/tests/json_crush.rs b/crates/mtui-mcp/tests/json_crush.rs index 8e4898e2..4e26a3e8 100644 --- a/crates/mtui-mcp/tests/json_crush.rs +++ b/crates/mtui-mcp/tests/json_crush.rs @@ -1,10 +1,11 @@ //! Row-budget crush reaches the MCP client intact. //! //! Drives the real `updates` / `list_refhosts` commands through -//! [`McpSession::run_command`] with unbounded mocked backends: `--json` tool -//! output parses as-is (the truncation notice goes to stderr, keeping stdout -//! valid JSON) while human output keeps the notice naming the narrowing flags. -//! Also pins the additive paging flags and that row-cap is not byte-cap. +//! [`McpSession::run_command`] with unbounded mocked backends: over-cap `--json` +//! tool output carries the truncation notice in-band after the JSON array (like +//! the byte-cap convention), so naive parse fails loudly and clients strip +//! `…[truncated` lines; under-cap stays pure JSON. Also pins the additive +//! paging flags and that row-cap is not byte-cap. #![cfg(feature = "mcp")] @@ -29,9 +30,9 @@ fn queue_fixture() -> serde_json::Value { serde_json::json!({"updates": rows}) } -/// `updates --json` over an unbounded queue: stdout stays valid JSON, anomaly kept. +/// `updates --json` over an unbounded queue: notice in-band, anomaly kept. #[tokio::test] -async fn updates_json_crush_stays_valid() { +async fn updates_json_crush_carries_notice_in_band() { use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -55,9 +56,23 @@ async fn updates_json_crush_stays_valid() { .run_command(®istry, "updates", &argv) .await .expect("updates succeeds"); - // Notice on stderr: tool output parses as-is with no trailing notice line. - assert!(!out.contains("…[truncated"), "{out}"); - let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + assert!( + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "{out}" + ); + assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); + assert!( + serde_json::from_str::(out.trim()).is_err(), + "naive parse of full stdout must fail loudly: {out}" + ); + let json_part: String = out + .lines() + .filter(|l| !l.starts_with("…[truncated")) + .collect::>() + .join("\n"); + let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); let rows = parsed.as_array().unwrap(); assert!(rows.len() <= 100, "row budget holds: {}", rows.len()); assert!(rows.iter().any(|r| r["id"] == "row-anomaly"), "{out}"); @@ -132,6 +147,10 @@ async fn updates_offset_recovers_middle_row() { .run_command(®istry, "updates", &argv) .await .expect("updates succeeds"); + assert!( + !out.contains("…[truncated"), + "50-row window fits budget: {out}" + ); let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); let rows = parsed.as_array().unwrap(); assert!(rows.iter().any(|r| r["id"] == "row-060"), "{out}"); diff --git a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap index 1dd029d9..22efa450 100644 --- a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap +++ b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap @@ -606,7 +606,7 @@ expression: pretty }, "json": { "default": false, - "description": "emit a JSON array of kept rows; over-cap stdout is still a valid JSON array, the `…[truncated …` notice goes to stderr on the CLI; MCP results carry no truncation signal, MUST page with --limit/--offset to be sure of completeness", + "description": "emit a JSON array of kept rows; over-cap stdout adds a trailing `…[truncated …` notice line — naive parse of full stdout fails, strip lines starting with that prefix before parsing", "type": "boolean" }, "limit": { @@ -1669,7 +1669,7 @@ expression: pretty }, "json": { "default": false, - "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is still a valid JSON array of kept rows, the `…[truncated …` notice goes to stderr on the CLI; MCP results carry no truncation signal, MUST page with --limit/--offset to be sure of completeness", + "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is a JSON array of kept rows plus a trailing `…[truncated …` notice line — naive parse of full stdout fails, strip lines starting with that prefix before parsing", "type": "boolean" }, "limit": { diff --git a/docs/src/cli.md b/docs/src/cli.md index 40fbe2e4..9b70310a 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -710,7 +710,7 @@ Options: select output fields by osc-qam name (e.g. -F Rating -F 'Assigned Roles'); repeatable, rendered as one block per update; names match case-insensitively, ignoring spaces/hyphens/underscores; narrow large queues with --limit --json - print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is still a valid JSON array of kept rows, the `…[truncated …` notice goes to stderr on the CLI; MCP results carry no truncation signal, MUST page with --limit/--offset to be sure of completeness + print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is a JSON array of kept rows plus a trailing `…[truncated …` notice line — naive parse of full stdout fails, strip lines starting with that prefix before parsing --status filter by status (default: testing); use 'all' for every status @@ -1074,7 +1074,7 @@ Options: group by test-target slot (product+version+arch+addons) --json - emit a JSON array of kept rows; over-cap stdout is still a valid JSON array, the `…[truncated …` notice goes to stderr on the CLI; MCP results carry no truncation signal, MUST page with --limit/--offset to be sure of completeness + emit a JSON array of kept rows; over-cap stdout adds a trailing `…[truncated …` notice line — naive parse of full stdout fails, strip lines starting with that prefix before parsing --limit cap the number of rows after --offset (0 = all) From 1ad8fd7ced7b67a78d3b9560aaed75d1e53f4f87 Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Sat, 12 Sep 2026 15:00:39 +0200 Subject: [PATCH 6/8] fix(mcp): severity-ordered anomalies, window-first free probes Anomaly overflow keeps severe rows first (updates failed/blocked, refhosts locked/claimed, openQA failed) with kept/total counts in the notice; list_refhosts --free windows before probing so paging cuts SSH cost, notice/footer stay pre-window. --- CHANGELOG.md | 22 +- .../mtui-core/src/commands/list_refhosts.rs | 203 +++++++++++++++--- .../mtui-core/src/commands/openqa_overview.rs | 95 ++++++-- crates/mtui-core/src/commands/row_budget.rs | 181 +++++++++++++--- crates/mtui-core/src/commands/updates.rs | 77 ++++++- ...slimmed_command_tool_schemas_snapshot.snap | 3 +- docs/src/cli.md | 2 +- 7 files changed, 489 insertions(+), 94 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2ffcec3..83eec51c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,22 +46,28 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht path) are refused, and endpoint URLs print userinfo-stripped. The REPL prints everything verbatim as before. No tool schema changed. `config set` of an endpoint URL acknowledges with the userinfo-stripped value. -- Unbounded listings now crush to a row budget — first-40 + last-10 + all +- Unbounded listings now crush to a row budget — first-40 + last-10 + top-severity anomaly rows, exact-deduped, hard cap 100 — instead of dumping thousands of rows into the client's context: `updates` (anomaly: non-`testing` status, - unknown/missing/null status kept; narrow with `--limit/--offset/--field/-G`), - `list_refhosts` (anomaly: non-`free` lock or pool claim; narrow with + `failed`/`blocked` outrank the rest, unknown/missing/null status kept; narrow + with `--limit/--offset/--field/-G`), `list_refhosts` (anomaly: non-`free` lock + or pool claim, `locked`/claimed outrank the rest; narrow with `--limit/--offset/--name/--arch/--product/--version/--addon`), and - `openqa_overview` (anomaly: non-`passed` version rows, build checks with - matches; display only, `--export` still writes the full overview; narrow with + `openqa_overview` (anomaly: non-`passed` version rows, `failed` outranks the + rest, build checks with matches; display only, `--export` still writes the + full overview; narrow with `--no-aggregated/--aggregated-groups/--days/--test-pattern`). `--json` over-cap stdout is a JSON array of kept rows plus a trailing - `…[truncated N of M rows; …]` notice line in-band (CLI and MCP alike, like - the byte-cap convention; also in `--json` help) — naive parse of full - stdout fails loudly, strip lines starting with that prefix before parsing. + `…[truncated N of M rows (K/L anomalies kept); …]` notice line in-band (CLI + and MCP alike, like the byte-cap convention; also in `--json` help) — naive + parse of full stdout fails loudly, strip lines starting with that prefix + before parsing. Any middle slice is recoverable via pre-crush `--offset`/`--limit` paging (chosen over an explicit-window notice as it fits the existing `--limit` plumbing). + `list_refhosts --free` windows before probing, so paging reduces the SSH + probe cost; windowing order is the matched inventory order and the + notice/footer stay pre-window totals. Row-cap is not byte-cap: MCP `max_output_bytes` can still cut mid-array on huge rows; `openqa_overview` per-section caps sum to ~600 rows total. **MCP schema note:** additive only — `updates` gains `offset`, `list_refhosts` diff --git a/crates/mtui-core/src/commands/list_refhosts.rs b/crates/mtui-core/src/commands/list_refhosts.rs index 12ca7815..55d63f3d 100644 --- a/crates/mtui-core/src/commands/list_refhosts.rs +++ b/crates/mtui-core/src/commands/list_refhosts.rs @@ -36,8 +36,15 @@ use super::row_budget::{crush, row_notice}; const REFHOSTS_HINT: &str = "--limit/--offset/--name/--arch/--product/--version/--addon"; /// Non-`free` or pool-claimed rows survive the crush: the actionable lock signal. -fn is_anomaly_record(r: &Record) -> bool { - !matches!(r.lock.as_deref(), None | Some("free")) || r.pool.is_some() +/// Severity: `locked` or pool-claimed (2) outrank other states (1). +fn anomaly_severity(r: &Record) -> u8 { + if matches!(r.lock.as_deref(), Some("locked")) || r.pool.is_some() { + 2 + } else if !matches!(r.lock.as_deref(), None | Some("free")) { + 1 + } else { + 0 + } } /// One matched refhost, rendered as a table row or a JSON object. @@ -171,9 +178,16 @@ fn lock_label(lock: Option<&str>, pool: Option<&str>) -> String { } /// Render `records` as one aligned multi-line table, grouped by slot when -/// `pool`. +/// `pool`. `total` is the pre-window matched count; the footer shows +/// `kept of total` when windowing/crush dropped rows. #[must_use] -pub fn render_table(records: &[Record], pool: bool, free: bool, verbose: bool) -> String { +pub fn render_table( + records: &[Record], + pool: bool, + free: bool, + verbose: bool, + total: usize, +) -> String { let fmt = |r: &Record| -> String { let prod = format!("{} {}", r.product, r.version); let prod = prod.trim(); @@ -217,7 +231,11 @@ pub fn render_table(records: &[Record], pool: bool, free: bool, verbose: bool) - out.push('\n'); } } - out.push_str(&format!("\n{} refhost(s)", records.len())); + if records.len() == total { + out.push_str(&format!("\n{} refhost(s)", records.len())); + } else { + out.push_str(&format!("\n{} of {total} refhost(s)", records.len())); + } out } @@ -326,13 +344,13 @@ impl Command for ListRefhosts { .help("skip the first N rows (0 = from the start); with --limit, page any middle slice"), ) .arg( - Arg::new("free") - .long("free") - .action(ArgAction::SetTrue) - .help( - "also probe live operation-lock and pool-claim state \ - (connects to each matched host)", - ), + Arg::new("free") + .long("free") + .action(ArgAction::SetTrue) + .help( + "also probe live operation-lock and pool-claim state \ + (connects to each shown host; --offset/--limit window first)", + ), ) .arg( Arg::new("verbose") @@ -409,15 +427,14 @@ impl Command for ListRefhosts { pool, }; - let mut records = gather(&store, &filters); - - if free && !records.is_empty() { - probe_locks(&ProbeConfig::new(&config), &mut records).await; - } + let records = gather(&store, &filters); + let matched_total = records.len(); - // Paging via --offset (pre-crush) makes any middle slice recoverable; chosen over an - // explicit-window notice as it fits the existing --limit plumbing. - let windowed: Vec = { + // Windowing BEFORE the --free probes so paging reduces SSH cost: only + // the windowed subset is probed. Order is the deterministic matched + // inventory order (refhosts.yml file order after filters); the + // notice/footer below still report pre-window totals. + let mut windowed: Vec = { let skipped = offset.min(records.len()); let mut v: Vec = records.into_iter().skip(skipped).collect(); if limit > 0 && limit < v.len() { @@ -425,6 +442,12 @@ impl Command for ListRefhosts { } v }; + let window_dropped = matched_total.saturating_sub(windowed.len()); + + if free && !windowed.is_empty() { + probe_locks(&ProbeConfig::new(&config), &mut windowed).await; + } + // Row budget backstops the whole-inventory dump: head+tail+anomalies, exact-deduped. // Row-cap is not byte-cap: MCP max_output_bytes can still cut mid-array on huge rows. let crushed = crush( @@ -441,12 +464,21 @@ impl Command for ListRefhosts { r.pool.clone(), ) }, - is_anomaly_record, + anomaly_severity, ); - // Over-cap --json stays in-band like the byte-cap convention: JSON array - // plus a trailing notice line, so MCP captures the signal too. - let notice = (crushed.truncated > 0) - .then(|| row_notice(crushed.truncated, crushed.total, REFHOSTS_HINT)); + // Totals stay pre-window so a probed window never masquerades as the + // whole inventory; --free windows always notice since unprobed hosts + // hide lock state, plain windows only when the crush itself dropped. + let overall_truncated = window_dropped.saturating_add(crushed.truncated); + let notice = (crushed.truncated > 0 || (free && window_dropped > 0)).then(|| { + row_notice( + overall_truncated, + matched_total, + crushed.anomaly_kept, + crushed.anomaly_total, + REFHOSTS_HINT, + ) + }); if as_json { session.display.println(&render_json(&crushed.kept)); if let Some(notice) = notice { @@ -458,9 +490,13 @@ impl Command for ListRefhosts { session.display.println("no refhosts match"); return Ok(()); } - session - .display - .println(&render_table(&crushed.kept, pool, free, verbose)); + session.display.println(&render_table( + &crushed.kept, + pool, + free, + verbose, + matched_total, + )); if let Some(notice) = notice { session.display.println(¬ice); } @@ -665,7 +701,8 @@ mod tests { #[test] fn render_table_plain_lists_and_counts() { let recs = gather(&store(), &Filters::default()); - let out = render_table(&recs, false, false, false); + let total = recs.len(); + let out = render_table(&recs, false, false, false, total); assert!(out.contains("whale-01")); assert!(out.contains("sles 15-6")); assert!(out.ends_with("3 refhost(s)")); @@ -676,7 +713,8 @@ mod tests { #[test] fn render_table_verbose_shows_addons() { let recs = gather(&store(), &Filters::default()); - let out = render_table(&recs, false, false, true); + let total = recs.len(); + let out = render_table(&recs, false, false, true, total); assert!(out.contains("sdk")); } @@ -684,7 +722,8 @@ mod tests { fn render_table_free_column_present() { let mut recs = gather(&store(), &Filters::default()); recs[0].lock = Some("locked".to_owned()); - let out = render_table(&recs, false, true, false); + let total = recs.len(); + let out = render_table(&recs, false, true, false, total); assert!(out.contains("locked")); } @@ -719,7 +758,8 @@ mod tests { ..Default::default() }; let recs = gather(&store(), &f); - let out = render_table(&recs, true, false, false); + let total = recs.len(); + let out = render_table(&recs, true, false, false, total); assert!(out.contains("== sles-15-5 x86_64 ==")); assert!(out.contains("== sles-15-6 x86_64 ==")); assert!(out.contains(" whale-01")); @@ -1089,13 +1129,74 @@ default: r.pool.clone(), ) }, - is_anomaly_record, + anomaly_severity, ); assert_eq!(out.total, 150, "deduped total"); assert!(out.kept.len() <= ROW_CAP, "{}", out.kept.len()); assert!(out.kept.iter().any(|r| r.name == "host-anomaly")); assert!(!out.kept.iter().any(|r| r.name == "host-060")); assert!(out.truncated > 0); + assert_eq!((out.anomaly_kept, out.anomaly_total), (1, 1)); + } + + #[test] + fn locked_outranks_unreachable_when_overflowing() { + use super::super::row_budget::crush; + // 300 probed rows: mild `unreachable` everywhere middle except one + // severe `locked` at 250 — only 50 middle anomalies fit. + let recs: Vec = (0..300) + .map(|i| Record { + name: format!("host-{i:03}"), + arch: "x86_64".to_owned(), + product: "sles".to_owned(), + version: "15-6".to_owned(), + addons: vec![], + slot: None, + lock: Some(if i == 250 { + "locked".to_owned() + } else if (super::super::row_budget::ROW_HEAD + ..300 - super::super::row_budget::ROW_TAIL) + .contains(&i) + { + "unreachable".to_owned() + } else { + "free".to_owned() + }), + pool: None, + }) + .collect(); + assert_eq!(anomaly_severity(&recs[250]), 2); + assert_eq!(anomaly_severity(&recs[41]), 1); + assert_eq!(anomaly_severity(&recs[0]), 0); + let out = crush( + recs, + |r| { + ( + r.name.clone(), + r.arch.clone(), + r.product.clone(), + r.version.clone(), + r.addons.clone(), + r.slot.clone(), + r.lock.clone(), + r.pool.clone(), + ) + }, + anomaly_severity, + ); + let names: Vec<_> = out.kept.iter().map(|r| r.name.as_str()).collect(); + assert!(names.contains(&"host-250"), "severe locked survives"); + assert!(!names.contains(&"host-200"), "mild unreachable drops"); + assert_eq!((out.anomaly_kept, out.anomaly_total), (50, 250)); + } + + #[test] + fn footer_shows_pre_window_total_when_paged() { + let recs = crush_records(); + let total = recs.len(); + let windowed = recs[50..100].to_vec(); + let out = render_table(&windowed, false, false, false, total); + assert!(out.ends_with("50 of 152 refhost(s)"), "{out}"); } #[tokio::test] @@ -1198,5 +1299,41 @@ default: !out.contains("…[truncated"), "50-row window fits budget: {out}" ); + assert!(out.contains("50 of 150 refhost(s)"), "{out}"); + } + + #[tokio::test] + async fn free_window_reports_pre_window_totals() { + use crate::commands::testkit::matches; + // Tiny inventory so the --free probes fail fast (unresolvable names); + // windowing must happen before probing and the notice/footer must + // still name the pre-window matched total. + let mut yaml = String::from("default:\n"); + for i in 0..5 { + yaml.push_str(&format!( + " - name: host-{i:03}\n arch: x86_64\n product:\n name: sles\n version:\n major: 15\n minor: 6\n" + )); + } + let (mut session, buf, _dir) = session_with_refhosts_file(&yaml); + let args = matches(&ListRefhosts, &["--free", "--offset", "1", "--limit", "2"]); + ListRefhosts.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + assert!( + out.contains("host-001") && out.contains("host-002"), + "{out}" + ); + assert!( + !out.contains("host-000") && !out.contains("host-004"), + "{out}" + ); + assert!(out.contains("2 of 5 refhost(s)"), "{out}"); + assert!( + out.lines() + .last() + .is_some_and(|l| l.starts_with("…[truncated")), + "probed window still notices the unprobed remainder: {out}" + ); + assert!(out.contains("3 of 5 rows"), "{out}"); + assert!(out.contains("2/2 anomalies kept"), "{out}"); } } diff --git a/crates/mtui-core/src/commands/openqa_overview.rs b/crates/mtui-core/src/commands/openqa_overview.rs index 13f06eac..babdd9bb 100644 --- a/crates/mtui-core/src/commands/openqa_overview.rs +++ b/crates/mtui-core/src/commands/openqa_overview.rs @@ -19,13 +19,18 @@ use super::row_budget::{crush_slice, row_notice}; const OPENQA_HINT: &str = "--no-aggregated/--aggregated-groups/--days/--test-pattern"; /// Non-`passed` rows survive the crush: the actionable openQA signal. -fn is_anomaly_version(row: &oqa::VersionResult) -> bool { - row.status != "passed" +/// Severity: `failed` (2) outranks other non-`passed` (1). +fn anomaly_severity_version(row: &oqa::VersionResult) -> u8 { + match row.status.as_str() { + "passed" => 0, + "failed" => 2, + _ => 1, + } } /// Build checks with extracted matches survive the crush. -fn is_anomaly_build(entry: &oqa::BuildCheckResult) -> bool { - !entry.matches.is_empty() +fn anomaly_severity_build(entry: &oqa::BuildCheckResult) -> u8 { + u8::from(!entry.matches.is_empty()) } /// The aggregated-update job groups offered for tab completion. @@ -270,15 +275,19 @@ impl Command for OpenQAOverview { r.note.as_str(), ) }, - is_anomaly_version, + anomaly_severity_version, ); for row in &single.kept { print_version_row(session, row); } if single.truncated > 0 { - session - .display - .println(&row_notice(single.truncated, single.total, OPENQA_HINT)); + session.display.println(&row_notice( + single.truncated, + single.total, + single.anomaly_kept, + single.anomaly_total, + OPENQA_HINT, + )); } if !no_aggregated { @@ -300,7 +309,7 @@ impl Command for OpenQAOverview { r.note.as_str(), ) }, - is_anomaly_version, + anomaly_severity_version, ); for row in &versions.kept { print_version_row(session, row); @@ -309,6 +318,8 @@ impl Command for OpenQAOverview { session.display.println(&row_notice( versions.truncated, versions.total, + versions.anomaly_kept, + versions.anomaly_total, OPENQA_HINT, )); } @@ -343,15 +354,19 @@ impl Command for OpenQAOverview { |e: &oqa::BuildCheckResult| { (e.url.as_str(), e.matches.as_slice(), e.summary.as_str()) }, - is_anomaly_build, + anomaly_severity_build, ); for entry in &checks.kept { print_build_check(session, entry); } if checks.truncated > 0 { - session - .display - .println(&row_notice(checks.truncated, checks.total, OPENQA_HINT)); + session.display.println(&row_notice( + checks.truncated, + checks.total, + checks.anomaly_kept, + checks.anomaly_total, + OPENQA_HINT, + )); } } @@ -797,13 +812,58 @@ mod tests { r.note.as_str(), ) }, - is_anomaly_version, + anomaly_severity_version, ); assert_eq!(out.total, 150, "deduped total"); assert!(out.kept.len() <= ROW_CAP, "{}", out.kept.len()); assert!(out.kept.iter().any(|r| r.status == "failed")); assert!(!out.kept.iter().any(|r| r.version == "15-SP060")); assert!(out.truncated > 0); + assert_eq!((out.anomaly_kept, out.anomaly_total), (1, 1)); + } + + #[test] + fn failed_version_outranks_running_when_overflowing() { + use super::super::row_budget::crush_slice; + // 300 rows: mild `running` everywhere middle except severe `failed` + // at 250 — only 50 middle anomalies fit. + let rows: Vec = (0..300) + .map(|i| oqa::VersionResult { + version: format!("15-SP{i:03}"), + url: format!("http://oqa/{i}"), + status: if i == 250 { + "failed".to_owned() + } else if (super::super::row_budget::ROW_HEAD + ..300 - super::super::row_budget::ROW_TAIL) + .contains(&i) + { + "running".to_owned() + } else { + "passed".to_owned() + }, + ..Default::default() + }) + .collect(); + assert_eq!(anomaly_severity_version(&rows[250]), 2); + assert_eq!(anomaly_severity_version(&rows[41]), 1); + assert_eq!(anomaly_severity_version(&rows[0]), 0); + let out = crush_slice( + &rows, + |r: &oqa::VersionResult| { + ( + r.version.as_str(), + r.url.as_str(), + r.status.as_str(), + r.failed_count, + r.running_count, + r.note.as_str(), + ) + }, + anomaly_severity_version, + ); + assert!(out.kept.iter().any(|r| r.version == "15-SP250")); + assert!(!out.kept.iter().any(|r| r.version == "15-SP200")); + assert_eq!((out.anomaly_kept, out.anomaly_total), (50, 250)); } #[test] @@ -819,7 +879,7 @@ mod tests { let out = crush_slice( &entries, |e: &oqa::BuildCheckResult| (e.url.as_str(), e.matches.as_slice(), e.summary.as_str()), - is_anomaly_build, + anomaly_severity_build, ); assert!(out.kept.iter().any(|e| e.url == "http://qam/100.log")); assert!(!out.kept.iter().any(|e| e.url == "http://qam/60.log")); @@ -828,8 +888,9 @@ mod tests { #[test] fn row_budget_notice_names_narrowing_flags() { use super::super::row_budget::row_notice; - let n = row_notice(90, 150, OPENQA_HINT); + let n = row_notice(90, 150, 5, 60, OPENQA_HINT); assert!(n.starts_with("…[truncated"), "{n}"); + assert!(n.contains("5/60 anomalies kept"), "{n}"); assert!( n.contains("--no-aggregated/--aggregated-groups/--days"), "{n}" @@ -859,7 +920,7 @@ mod tests { r.note.as_str(), ) }, - is_anomaly_version, + anomaly_severity_version, ); assert!(!crushed.kept.iter().any(|r| r.version == "15-SP060")); let dir = tempfile::tempdir().unwrap(); diff --git a/crates/mtui-core/src/commands/row_budget.rs b/crates/mtui-core/src/commands/row_budget.rs index 77291fda..7b3db498 100644 --- a/crates/mtui-core/src/commands/row_budget.rs +++ b/crates/mtui-core/src/commands/row_budget.rs @@ -12,6 +12,7 @@ pub(crate) const ROW_HEAD: usize = 40; pub(crate) const ROW_TAIL: usize = 10; /// Hard cap on kept rows, anomalies included. pub(crate) const ROW_CAP: usize = 100; +const _: () = assert!(ROW_HEAD + ROW_TAIL <= ROW_CAP); /// Outcome of [`crush`]: the kept items plus truncation counts for the notice. pub(crate) struct CrushOutcome { @@ -21,15 +22,21 @@ pub(crate) struct CrushOutcome { pub total: usize, /// `total - kept.len()`; zero means nothing was dropped. pub truncated: usize, + /// Middle anomalies kept (severity-selected, capped). + pub anomaly_kept: usize, + /// Middle anomalies found pre-cap; head/tail always kept, uncounted. + pub anomaly_total: usize, } /// Crush `items` to budget, preserving order. /// -/// Exact-dedups on `key`, then keeps head + tail + all middle anomalies up to [`ROW_CAP`]. +/// Exact-dedups on `key`, then keeps head + tail + top-severity middle +/// anomalies up to [`ROW_CAP`]. `severity_of` returns 0 for routine rows, +/// higher for more severe anomalies; ties keep file order (stable). pub(crate) fn crush( items: Vec, mut key_of: impl FnMut(&T) -> K, - mut is_anomaly: impl FnMut(&T) -> bool, + mut severity_of: impl FnMut(&T) -> u8, ) -> CrushOutcome { // Dedup first so identical rows never consume budget twice. let mut seen = HashSet::new(); @@ -39,27 +46,45 @@ pub(crate) fn crush( .collect(); let total = items.len(); if total <= ROW_CAP { + let n = items.iter().filter(|it| severity_of(it) > 0).count(); return CrushOutcome { kept: items, total, truncated: 0, + anomaly_kept: n, + anomaly_total: n, }; } let tail_start = total - ROW_TAIL; - let anomaly_idx: Vec = (ROW_HEAD..tail_start) - .filter(|&i| is_anomaly(&items[i])) + let mut scored: Vec<(u8, usize)> = (ROW_HEAD..tail_start) + .map(|i| (severity_of(&items[i]), i)) + .filter(|(s, _)| *s > 0) .collect(); - keep_head_anomaly_tail(items, anomaly_idx, tail_start, total) + let anomaly_total = scored.len(); + // Stable severity-desc: severe survives, ties keep positional order. + scored.sort_by_key(|(s, _)| std::cmp::Reverse(*s)); + scored.truncate(ROW_CAP - ROW_HEAD - ROW_TAIL); + let anomaly_kept = scored.len(); + let anomaly_idx: Vec = scored.into_iter().map(|(_, i)| i).collect(); + keep_head_anomaly_tail( + items, + anomaly_idx, + tail_start, + total, + anomaly_kept, + anomaly_total, + ) } /// Crush `items` to budget, preserving order. /// -/// Exact-dedups on `key`, then keeps head + tail + all middle anomalies up to [`ROW_CAP`]. +/// Exact-dedups on `key`, then keeps head + tail + top-severity middle +/// anomalies up to [`ROW_CAP`]. /// Borrowed twin for already-owned data (openQA overview): same keep, no bulk clone. pub(crate) fn crush_slice<'a, T, K: Eq + Hash>( items: &'a [T], mut key_of: impl FnMut(&'a T) -> K, - mut is_anomaly: impl FnMut(&'a T) -> bool, + mut severity_of: impl FnMut(&'a T) -> u8, ) -> CrushOutcome<&'a T> { let mut seen = HashSet::new(); let mut uniq: Vec<&'a T> = Vec::new(); @@ -70,28 +95,44 @@ pub(crate) fn crush_slice<'a, T, K: Eq + Hash>( } let total = uniq.len(); if total <= ROW_CAP { + let n = uniq.iter().filter(|it| severity_of(it) > 0).count(); return CrushOutcome { kept: uniq, total, truncated: 0, + anomaly_kept: n, + anomaly_total: n, }; } let tail_start = total - ROW_TAIL; - let anomaly_idx: Vec = (ROW_HEAD..tail_start) - .filter(|&i| is_anomaly(uniq[i])) + let mut scored: Vec<(u8, usize)> = (ROW_HEAD..tail_start) + .map(|i| (severity_of(uniq[i]), i)) + .filter(|(s, _)| *s > 0) .collect(); - keep_head_anomaly_tail(uniq, anomaly_idx, tail_start, total) + let anomaly_total = scored.len(); + scored.sort_by_key(|(s, _)| std::cmp::Reverse(*s)); + scored.truncate(ROW_CAP - ROW_HEAD - ROW_TAIL); + let anomaly_kept = scored.len(); + let anomaly_idx: Vec = scored.into_iter().map(|(_, i)| i).collect(); + keep_head_anomaly_tail( + uniq, + anomaly_idx, + tail_start, + total, + anomaly_kept, + anomaly_total, + ) } /// Shared head + capped-anomaly + tail keep, order-preserving. fn keep_head_anomaly_tail( mut items: Vec, - mut anomaly_idx: Vec, + anomaly_idx: Vec, tail_start: usize, total: usize, + anomaly_kept: usize, + anomaly_total: usize, ) -> CrushOutcome { - // Cap anomalies to what fits between head and tail. - anomaly_idx.truncate(ROW_CAP - ROW_HEAD - ROW_TAIL); let keep: HashSet = (0..ROW_HEAD) .chain(anomaly_idx) .chain(tail_start..total) @@ -107,13 +148,23 @@ fn keep_head_anomaly_tail( kept, total, truncated, + anomaly_kept, + anomaly_total, } } /// Human/JSON trailing notice naming the narrowing flags. #[must_use] -pub(crate) fn row_notice(truncated: usize, total: usize, hint: &str) -> String { - format!("…[truncated {truncated} of {total} rows; narrow with {hint}]") +pub(crate) fn row_notice( + truncated: usize, + total: usize, + anomaly_kept: usize, + anomaly_total: usize, + hint: &str, +) -> String { + format!( + "…[truncated {truncated} of {total} rows ({anomaly_kept}/{anomaly_total} anomalies kept); narrow with {hint}]" + ) } #[cfg(test)] @@ -122,7 +173,7 @@ mod tests { #[test] fn under_cap_passes_through_with_no_truncation() { - let out = crush(vec![1, 2, 3], |v| *v, |_| false); + let out = crush(vec![1, 2, 3], |v| *v, |_| 0); assert_eq!(out.kept, vec![1, 2, 3]); assert_eq!(out.truncated, 0); } @@ -131,26 +182,71 @@ mod tests { fn over_cap_keeps_head_tail_and_all_middle_anomalies() { // 0..150, anomalies at 50 and 140 (tail) plus 100. let items: Vec = (0..150).collect(); - let out = crush(items, |v| *v, |v| *v == 50 || *v == 100 || *v == 140); + let out = crush(items, |v| *v, |v| u8::from(*v == 50 || *v == 100)); assert_eq!(out.total, 150); - // Head 0..40, anomalies 50+100, tail 140..150 (140 already in tail). + // Head 0..40, anomalies 50+100, tail 140..150. assert!(out.kept.contains(&0) && out.kept.contains(&39)); assert!(out.kept.contains(&50) && out.kept.contains(&100)); assert!(out.kept.contains(&140) && out.kept.contains(&149)); assert!(!out.kept.contains(&60), "non-anomaly middle row dropped"); assert_eq!(out.kept.len(), ROW_HEAD + ROW_TAIL + 2); assert_eq!(out.truncated, 150 - out.kept.len()); + assert_eq!((out.anomaly_kept, out.anomaly_total), (2, 2)); // Order preserved. let mut sorted = out.kept.clone(); sorted.sort_unstable(); assert_eq!(sorted, out.kept); } + #[test] + fn severe_anomaly_outranks_mild_when_overflowing() { + // 300 rows: every middle row anomalous, one severe at the far end. + // Severity-desc selection must keep the severe tail-middle row and + // drop a mild head-middle one, restoring display in index order. + let items: Vec = (0..300).collect(); + let out = crush( + items, + |v| *v, + |v| { + if *v == 250 { + 2 + } else if *v >= ROW_HEAD { + 1 + } else { + 0 + } + }, + ); + assert_eq!(out.kept.len(), ROW_CAP); + assert_eq!((out.anomaly_kept, out.anomaly_total), (50, 250)); + assert!( + out.kept.contains(&250), + "severe middle row survives: {:?}", + &out.kept[35..55] + ); + assert!( + !out.kept.contains(&89), + "mild middle row drops once severe outranks it" + ); + let n = row_notice( + out.truncated, + out.total, + out.anomaly_kept, + out.anomaly_total, + "--limit", + ); + assert!(n.contains("50/250 anomalies kept"), "{n}"); + // Display stays in original order despite severity selection. + let mut sorted = out.kept.clone(); + sorted.sort_unstable(); + assert_eq!(sorted, out.kept); + } + #[test] fn anomaly_overflow_truncates_middle_first_deterministically() { // Every middle row anomalous: only the first fitting anomalies survive, in index order. let items: Vec = (0..300).collect(); - let out = crush(items, |v| *v, |v| *v >= ROW_HEAD); + let out = crush(items, |v| *v, |v| u8::from(*v >= ROW_HEAD)); assert_eq!(out.kept.len(), ROW_CAP); let expected: Vec = (0..ROW_HEAD) .chain(ROW_HEAD..ROW_HEAD + (ROW_CAP - ROW_HEAD - ROW_TAIL)) @@ -161,10 +257,10 @@ mod tests { #[test] fn boundary_100_passes_101_crushes() { - let out100 = crush((0..100).collect::>(), |v| *v, |_| false); + let out100 = crush((0..100).collect::>(), |v| *v, |_| 0); assert_eq!(out100.truncated, 0); assert_eq!(out100.kept.len(), 100); - let out101 = crush((0..101).collect::>(), |v| *v, |_| false); + let out101 = crush((0..101).collect::>(), |v| *v, |_| 0); assert_eq!(out101.total, 101); assert_eq!(out101.kept.len(), ROW_HEAD + ROW_TAIL); assert_eq!(out101.truncated, 101 - (ROW_HEAD + ROW_TAIL)); @@ -173,16 +269,17 @@ mod tests { #[test] fn exact_duplicates_consume_no_budget() { let items = vec![7, 7, 7, 8, 8, 9]; - let out = crush(items, |v| *v, |_| false); + let out = crush(items, |v| *v, |_| 0); assert_eq!(out.kept, vec![7, 8, 9]); assert_eq!(out.truncated, 0); } #[test] fn notice_names_narrowing_flags() { - let n = row_notice(90, 150, "--limit/--field/-G"); + let n = row_notice(90, 150, 5, 60, "--limit/--field/-G"); assert!(n.starts_with("…[truncated"), "{n}"); assert!(n.contains("[truncated 90 of 150"), "{n}"); + assert!(n.contains("5/60 anomalies kept"), "{n}"); assert!(n.contains("--limit/--field/-G"), "{n}"); } @@ -190,21 +287,53 @@ mod tests { fn slice_matches_owned_keep_without_cloning() { // Borrowed keys prove the no-clone path compiles and keeps identically. let items: Vec = (0..150).collect(); - let owned = crush(items.clone(), |v| *v, |v| *v == 100); - let borrowed = crush_slice(&items, |v| *v, |v| *v == 100); + let owned = crush(items.clone(), |v| *v, |v| u8::from(*v == 100)); + let borrowed = crush_slice(&items, |v| *v, |v| u8::from(*v == 100)); assert_eq!(borrowed.total, owned.total); assert_eq!(borrowed.truncated, owned.truncated); + assert_eq!( + (borrowed.anomaly_kept, borrowed.anomaly_total), + (owned.anomaly_kept, owned.anomaly_total) + ); assert_eq!(borrowed.kept, owned.kept.iter().collect::>()); assert!(borrowed.kept.contains(&&100)); assert!(!borrowed.kept.contains(&&60)); } + #[test] + fn slice_severity_prefers_failed_over_running() { + // 150 borrowed rows, every middle row mild except one severe at 130: + // severity selection keeps 130 and drops a mild (e.g. 41). + let rows: Vec<(String, u8)> = (0..150) + .map(|i| { + let s = if i == 130 { + 2 + } else if (ROW_HEAD..150 - ROW_TAIL).contains(&i) { + 1 + } else { + 0 + }; + (format!("v-{i:03}"), s) + }) + .collect(); + let out = crush_slice(&rows, |(v, _)| v.as_str(), |(_, s)| *s); + assert_eq!((out.anomaly_kept, out.anomaly_total), (50, 100)); + assert!( + out.kept.iter().any(|(v, _)| v == "v-130"), + "severe middle row survives" + ); + assert!( + !out.kept.iter().any(|(v, _)| v == "v-100"), + "mild middle row drops once severe outranks it" + ); + } + #[test] fn slice_borrows_string_keys_and_dedups() { let rows: Vec<(String, String)> = (0..150) .map(|i| (format!("v-{i:03}"), "passed".to_owned())) .collect(); - let out = crush_slice(&rows, |(v, s)| (v.as_str(), s.as_str()), |_| false); + let out = crush_slice(&rows, |(v, s)| (v.as_str(), s.as_str()), |_| 0); assert_eq!(out.total, 150); assert_eq!(out.kept.len(), ROW_HEAD + ROW_TAIL); assert_eq!(out.kept[0].0, "v-000"); diff --git a/crates/mtui-core/src/commands/updates.rs b/crates/mtui-core/src/commands/updates.rs index c30abb4a..c9c58ec3 100644 --- a/crates/mtui-core/src/commands/updates.rs +++ b/crates/mtui-core/src/commands/updates.rs @@ -19,8 +19,13 @@ use super::row_budget::{crush, row_notice}; const UPDATES_HINT: &str = "--limit/--offset/--field/-G"; /// Non-`testing` rows survive the crush; unknown/missing/null status keeps too (safe direction). -fn is_anomaly_row(v: &Value) -> bool { - v.get("status").and_then(Value::as_str) != Some("testing") +/// Severity: `failed`/`blocked` (2) outrank other non-`testing` (1). +fn anomaly_severity(v: &Value) -> u8 { + match v.get("status").and_then(Value::as_str) { + Some("testing") => 0, + Some("failed" | "blocked") => 2, + _ => 1, + } } /// Lightweight dedup key: id/status/priority only, not the whole serialised row. @@ -370,12 +375,19 @@ impl Command for Updates { }; // Row budget backstops `--limit 0=all`: head+tail+anomalies, exact-deduped. // Row-cap is not byte-cap: MCP max_output_bytes can still cut mid-array on huge rows. - let crushed = crush(windowed, update_key, is_anomaly_row); + let crushed = crush(windowed, update_key, anomaly_severity); let shown = &crushed.kept; // Over-cap --json stays in-band like the byte-cap convention: JSON array // plus a trailing notice line, so MCP captures the signal too. - let notice = (crushed.truncated > 0) - .then(|| row_notice(crushed.truncated, crushed.total, UPDATES_HINT)); + let notice = (crushed.truncated > 0).then(|| { + row_notice( + crushed.truncated, + crushed.total, + crushed.anomaly_kept, + crushed.anomaly_total, + UPDATES_HINT, + ) + }); if as_json { let doc = Value::Array(shown.to_vec()); @@ -1790,10 +1802,59 @@ mod tests { serde_json::json!({"id": "c", "status": "weird"}), serde_json::json!({"id": "d", "status": 5}), ] { - assert!(is_anomaly_row(&row), "{row}"); + assert_eq!(anomaly_severity(&row), 1, "{row}"); + } + assert_eq!( + anomaly_severity(&serde_json::json!({"status": "testing"})), + 0 + ); + assert_eq!( + anomaly_severity(&serde_json::json!({"status": "failed"})), + 2 + ); + assert_eq!( + anomaly_severity(&serde_json::json!({"status": "blocked"})), + 2 + ); + } + + #[test] + fn severe_update_outranks_mild_when_overflowing() { + // 300 testing rows except a mild ("weird") early-middle and a severe + // ("failed") late-middle: only 50 middle anomalies fit, so the severe + // survives and the mild drops. + let mut rows: Vec = (0..300) + .map(|i| { + serde_json::json!({ + "priority": 1, "status": "testing", "kind": "Maintenance", + "id": format!("row-{i:03}"), + }) + }) + .collect(); + rows[280] = serde_json::json!({ + "priority": 1, "status": "weird", "kind": "Maintenance", "id": "row-mild", + }); + rows[250] = serde_json::json!({ + "priority": 1, "status": "failed", "kind": "Maintenance", "id": "row-severe", + }); + for (i, row) in rows.iter_mut().enumerate().take(292).skip(41) { + if i == 250 || i == 280 { + continue; + } + *row = serde_json::json!({ + "priority": 1, "status": "weird", "kind": "Maintenance", + "id": format!("row-{i:03}"), + }); } - assert!(!is_anomaly_row(&serde_json::json!({"status": "testing"}))); - assert!(is_anomaly_row(&serde_json::json!({"status": "failed"}))); + let out = crush(rows, update_key, anomaly_severity); + let ids: Vec<_> = out + .kept + .iter() + .filter_map(|r| r.get("id").and_then(Value::as_str)) + .collect(); + assert!(ids.contains(&"row-severe"), "severe survives: {ids:?}"); + assert!(!ids.contains(&"row-mild"), "mild drops: {ids:?}"); + assert_eq!((out.anomaly_kept, out.anomaly_total), (50, 249)); } #[tokio::test] diff --git a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap index 22efa450..5a4c9fb7 100644 --- a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap +++ b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap @@ -1,5 +1,6 @@ --- source: crates/mtui-mcp/tests/slim_profile.rs +assertion_line: 40 expression: pretty --- [ @@ -601,7 +602,7 @@ expression: pretty }, "free": { "default": false, - "description": "also probe live operation-lock and pool-claim state (connects to each matched host)", + "description": "also probe live operation-lock and pool-claim state (connects to each shown host; --offset/--limit window first)", "type": "boolean" }, "json": { diff --git a/docs/src/cli.md b/docs/src/cli.md index 9b70310a..da8c310f 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -1087,7 +1087,7 @@ Options: [default: 0] --free - also probe live operation-lock and pool-claim state (connects to each matched host) + also probe live operation-lock and pool-claim state (connects to each shown host; --offset/--limit window first) -v, --verbose include addons in the output From 4620c952b34f44a3ee77188e3063f9419ff5d581 Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Sun, 13 Sep 2026 09:12:44 +0200 Subject: [PATCH 7/8] fix(mcp): structured JSON envelope, decoupled probe note, full anomaly counts R4-P0a: list_refhosts emits truncation only on real row-budget cut; --free windowing gets decoupled probe note/envelope, no fabricated K/K. R4-P0b: update_key filters null ids to full-row fallback. R4-P1: anomaly counts cover 0..total incl. head/tail. R4-P2a: --json over-cap is rows+truncation(/probe) envelope, stdout stays valid JSON. R4-P2b: row_notice omits (K/L) when L==0. R2-P2s: insta snapshots for notice/probe/envelope. R3-D: shared truncation-notice convention in row_budget docs. --- CHANGELOG.md | 13 +- .../mtui-core/src/commands/list_refhosts.rs | 138 +++++++++----- crates/mtui-core/src/commands/row_budget.rs | 176 ++++++++++++++++-- ...__tests__snapshot_json_envelope_shape.snap | 25 +++ ...sts__snapshot_notice_and_probe_shapes.snap | 7 + crates/mtui-core/src/commands/updates.rs | 163 ++++++++-------- crates/mtui-mcp/tests/json_crush.rs | 31 +-- ...slimmed_command_tool_schemas_snapshot.snap | 5 +- docs/src/cli.md | 4 +- 9 files changed, 392 insertions(+), 170 deletions(-) create mode 100644 crates/mtui-core/src/commands/snapshots/mtui_core__commands__row_budget__tests__snapshot_json_envelope_shape.snap create mode 100644 crates/mtui-core/src/commands/snapshots/mtui_core__commands__row_budget__tests__snapshot_notice_and_probe_shapes.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 83eec51c..3221cd95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,17 +57,18 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht rest, build checks with matches; display only, `--export` still writes the full overview; narrow with `--no-aggregated/--aggregated-groups/--days/--test-pattern`). `--json` - over-cap stdout is a JSON array of kept rows plus a trailing - `…[truncated N of M rows (K/L anomalies kept); …]` notice line in-band (CLI - and MCP alike, like the byte-cap convention; also in `--json` help) — naive - parse of full stdout fails loudly, strip lines starting with that prefix - before parsing. + over-cap stdout is a JSON envelope `{"rows": [...], "truncation": {...}}` + so stdout stays valid JSON (CLI and MCP alike; also in `--json` help). + Anomaly counts cover the full dataset and the notice omits + `(K/L anomalies kept)` when none exist. Any middle slice is recoverable via pre-crush `--offset`/`--limit` paging (chosen over an explicit-window notice as it fits the existing `--limit` plumbing). `list_refhosts --free` windows before probing, so paging reduces the SSH probe cost; windowing order is the matched inventory order and the - notice/footer stay pre-window totals. + footer stays pre-window totals. A pure window emits only a probe note + (`…[probed X of Y hosts; …]` human, `"probe"` object in JSON), never a + truncation notice with anomaly counts over unprobed hosts. Row-cap is not byte-cap: MCP `max_output_bytes` can still cut mid-array on huge rows; `openqa_overview` per-section caps sum to ~600 rows total. **MCP schema note:** additive only — `updates` gains `offset`, `list_refhosts` diff --git a/crates/mtui-core/src/commands/list_refhosts.rs b/crates/mtui-core/src/commands/list_refhosts.rs index 55d63f3d..01f939ad 100644 --- a/crates/mtui-core/src/commands/list_refhosts.rs +++ b/crates/mtui-core/src/commands/list_refhosts.rs @@ -30,7 +30,9 @@ use crate::command::{Command, Scope}; use crate::error::{CommandError, CommandResult}; use crate::session::Session; -use super::row_budget::{crush, row_notice}; +use super::row_budget::{ + crush, json_envelope, probe_meta, probe_notice, row_notice, truncation_meta, +}; /// Narrowing flags named in the row-budget notice. const REFHOSTS_HINT: &str = "--limit/--offset/--name/--arch/--product/--version/--addon"; @@ -322,9 +324,9 @@ impl Command for ListRefhosts { .long("json") .action(ArgAction::SetTrue) .help( - "emit a JSON array of kept rows; over-cap stdout adds a trailing \ - `…[truncated …` notice line — naive parse of full stdout fails, \ - strip lines starting with that prefix before parsing", + "emit a JSON array of kept rows; over-cap stdout is a JSON envelope \ + {\"rows\": [...], \"truncation\": {...}} so stdout stays valid JSON \ + (--free windowing adds a \"probe\" object)", ), ) .arg( @@ -450,6 +452,7 @@ impl Command for ListRefhosts { // Row budget backstops the whole-inventory dump: head+tail+anomalies, exact-deduped. // Row-cap is not byte-cap: MCP max_output_bytes can still cut mid-array on huge rows. + let probed_len = windowed.len(); let crushed = crush( windowed, |r| { @@ -466,23 +469,37 @@ impl Command for ListRefhosts { }, anomaly_severity, ); - // Totals stay pre-window so a probed window never masquerades as the - // whole inventory; --free windows always notice since unprobed hosts - // hide lock state, plain windows only when the crush itself dropped. - let overall_truncated = window_dropped.saturating_add(crushed.truncated); - let notice = (crushed.truncated > 0 || (free && window_dropped > 0)).then(|| { + // Truncation notices only a real row-budget cut; --free windowing gets + // its own probe note so anomaly counts never cover unprobed hosts. + let trunc_notice = (crushed.truncated > 0).then(|| { row_notice( - overall_truncated, - matched_total, + crushed.truncated, + crushed.total, crushed.anomaly_kept, crushed.anomaly_total, REFHOSTS_HINT, ) }); + let probe_needed = free && window_dropped > 0; + let probe_note = probe_needed.then(|| probe_notice(probed_len, matched_total)); if as_json { - session.display.println(&render_json(&crushed.kept)); - if let Some(notice) = notice { - session.display.println(¬ice); + let trunc_meta = (crushed.truncated > 0).then(|| { + truncation_meta( + crushed.truncated, + crushed.total, + crushed.anomaly_kept, + crushed.anomaly_total, + REFHOSTS_HINT, + ) + }); + let probe_meta_opt = probe_needed.then(|| probe_meta(probed_len, matched_total)); + if trunc_meta.is_some() || probe_meta_opt.is_some() { + let rows: Vec = crushed.kept.iter().map(Record::to_json).collect(); + session + .display + .println(&json_envelope(rows, trunc_meta, probe_meta_opt)); + } else { + session.display.println(&render_json(&crushed.kept)); } return Ok(()); } @@ -497,9 +514,12 @@ impl Command for ListRefhosts { verbose, matched_total, )); - if let Some(notice) = notice { + if let Some(notice) = trunc_notice { session.display.println(¬ice); } + if let Some(note) = probe_note { + session.display.println(¬e); + } Ok(()) } } @@ -1200,7 +1220,7 @@ default: } #[tokio::test] - async fn row_budget_json_stays_parseable_with_trailing_notice() { + async fn row_budget_json_emits_envelope() { use crate::commands::testkit::matches; let mut yaml = String::from("default:\n"); for i in 0..150 { @@ -1212,31 +1232,20 @@ default: let args = matches(&ListRefhosts, &["--json"]); ListRefhosts.call(&mut session, &args).await.unwrap(); let out = buf.contents(); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let obj = parsed.as_object().unwrap(); + assert!(obj.contains_key("rows"), "{out}"); + assert!(obj.contains_key("truncation"), "{out}"); + let rows = obj["rows"].as_array().unwrap(); assert!( - out.lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")), - "{out}" - ); - assert!( - serde_json::from_str::(out.trim()).is_err(), - "naive parse of full stdout must fail loudly: {out}" - ); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); - let arr = parsed.as_array().unwrap(); - assert!( - arr.len() <= super::super::row_budget::ROW_CAP, + rows.len() <= super::super::row_budget::ROW_CAP, "{}", - arr.len() + rows.len() ); - assert!(arr.iter().any(|r| r["name"] == "host-000"), "{out}"); - assert!(arr.iter().any(|r| r["name"] == "host-149"), "{out}"); - assert!(!arr.iter().any(|r| r["name"] == "host-060"), "{out}"); + assert!(rows.iter().any(|r| r["name"] == "host-000"), "{out}"); + assert!(rows.iter().any(|r| r["name"] == "host-149"), "{out}"); + assert!(!rows.iter().any(|r| r["name"] == "host-060"), "{out}"); + assert!(!out.contains("…[truncated"), "no trailing plaintext: {out}"); } #[tokio::test] @@ -1274,8 +1283,9 @@ default: let base = clap::Command::new("list_refhosts").no_binary_name(true); let mut cmd = ListRefhosts.configure(base); let help = cmd.render_help().to_string(); - assert!(help.contains("…[truncated"), "{help}"); - assert!(help.contains("strip lines starting with"), "{help}"); + assert!(help.contains("truncation"), "{help}"); + assert!(help.contains("envelope"), "{help}"); + assert!(help.contains("valid JSON"), "{help}"); } #[tokio::test] @@ -1306,8 +1316,9 @@ default: async fn free_window_reports_pre_window_totals() { use crate::commands::testkit::matches; // Tiny inventory so the --free probes fail fast (unresolvable names); - // windowing must happen before probing and the notice/footer must - // still name the pre-window matched total. + // windowing must happen before probing and the footer must still name + // the pre-window matched total. A pure window emits only the probe + // note, never a truncation notice with fabricated anomaly counts. let mut yaml = String::from("default:\n"); for i in 0..5 { yaml.push_str(&format!( @@ -1330,10 +1341,45 @@ default: assert!( out.lines() .last() - .is_some_and(|l| l.starts_with("…[truncated")), - "probed window still notices the unprobed remainder: {out}" + .is_some_and(|l| l.starts_with("…[probed")), + "probed window notices the unprobed remainder: {out}" ); - assert!(out.contains("3 of 5 rows"), "{out}"); - assert!(out.contains("2/2 anomalies kept"), "{out}"); + assert!(out.contains("probed 2 of 5 hosts"), "{out}"); + assert!( + !out.contains("…[truncated"), + "user windowing alone never truncates: {out}" + ); + assert!( + !out.contains("anomalies kept"), + "probe note carries no fabricated anomaly counts: {out}" + ); + } + + #[tokio::test] + async fn free_limit_json_emits_probe_envelope_without_truncation() { + use crate::commands::testkit::matches; + // `--free --limit 10 --json` over 150 hosts: windowed 10 fits the row + // budget, so no truncation envelope — only a probe object, and stdout + // stays one valid JSON document. + let mut yaml = String::from("default:\n"); + for i in 0..150 { + yaml.push_str(&format!( + " - name: host-{i:03}\n arch: x86_64\n product:\n name: sles\n version:\n major: 15\n minor: 6\n" + )); + } + let (mut session, buf, _dir) = session_with_refhosts_file(&yaml); + let args = matches(&ListRefhosts, &["--free", "--limit", "10", "--json"]); + ListRefhosts.call(&mut session, &args).await.unwrap(); + let out = buf.contents(); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let obj = parsed.as_object().unwrap(); + assert!(obj.contains_key("rows"), "{out}"); + assert!(!obj.contains_key("truncation"), "{out}"); + assert!(obj.contains_key("probe"), "{out}"); + assert_eq!(obj["probe"]["probed"], 10, "{out}"); + assert_eq!(obj["probe"]["total"], 150, "{out}"); + assert_eq!(obj["rows"].as_array().unwrap().len(), 10, "{out}"); + assert!(!out.contains("…[truncated"), "{out}"); + assert!(!out.contains("…[probed"), "{out}"); } } diff --git a/crates/mtui-core/src/commands/row_budget.rs b/crates/mtui-core/src/commands/row_budget.rs index 7b3db498..0be87e45 100644 --- a/crates/mtui-core/src/commands/row_budget.rs +++ b/crates/mtui-core/src/commands/row_budget.rs @@ -2,10 +2,29 @@ //! //! Row budget: keep first-40 + last-10 + all anomaly rows, exact-dedup identical rows, hard cap 100. //! Row-cap is not byte-cap: MCP `max_output_bytes` can still cut mid-array on huge rows. +//! +//! Shared truncation-notice convention (converged for #610/#611/#620): +//! - Emit a truncation notice only on a real row-budget cut +//! (`crushed.truncated > 0`); user `--limit`/`--offset` windowing alone +//! never notices. +//! - Count anomalies over the full deduped dataset (`0..total`), not just the +//! middle slice, so a `0/0` notice never shows while anomalies are visible. +//! - Human output appends one trailing plaintext notice via [`row_notice`]; +//! it omits the `(K/L anomalies kept)` parenthetical when `L == 0`. +//! - `--json` output stays parseable: over-cap stdout is a JSON envelope +//! (`{"rows": [...], "truncation": {...}}`, plus `"probe"` where a survey +//! was windowed) instead of array plus trailing plaintext; small outputs +//! stay a plain array. +//! - Partial-survey probe notes (e.g. `--free` windowing leaves hosts +//! unprobed) are decoupled from truncation: [`probe_notice`] (human) and +//! the envelope `"probe"` object (JSON) never fabricate anomaly counts over +//! unprobed rows. use std::collections::HashSet; use std::hash::Hash; +use serde_json::{Value, json}; + /// Head rows always kept. pub(crate) const ROW_HEAD: usize = 40; /// Tail rows always kept. @@ -22,9 +41,9 @@ pub(crate) struct CrushOutcome { pub total: usize, /// `total - kept.len()`; zero means nothing was dropped. pub truncated: usize, - /// Middle anomalies kept (severity-selected, capped). + /// Anomalies kept, counted over the full dataset (head + selected middle + tail). pub anomaly_kept: usize, - /// Middle anomalies found pre-cap; head/tail always kept, uncounted. + /// Anomalies found over the full dataset (`0..total`), head/tail included. pub anomaly_total: usize, } @@ -56,15 +75,25 @@ pub(crate) fn crush( }; } let tail_start = total - ROW_TAIL; + let head_anomalies = items[..ROW_HEAD] + .iter() + .filter(|it| severity_of(it) > 0) + .count(); + let tail_anomalies = items[tail_start..] + .iter() + .filter(|it| severity_of(it) > 0) + .count(); let mut scored: Vec<(u8, usize)> = (ROW_HEAD..tail_start) .map(|i| (severity_of(&items[i]), i)) .filter(|(s, _)| *s > 0) .collect(); - let anomaly_total = scored.len(); + let middle_total = scored.len(); + let anomaly_total = head_anomalies + middle_total + tail_anomalies; // Stable severity-desc: severe survives, ties keep positional order. scored.sort_by_key(|(s, _)| std::cmp::Reverse(*s)); scored.truncate(ROW_CAP - ROW_HEAD - ROW_TAIL); - let anomaly_kept = scored.len(); + let middle_kept = scored.len(); + let anomaly_kept = head_anomalies + middle_kept + tail_anomalies; let anomaly_idx: Vec = scored.into_iter().map(|(_, i)| i).collect(); keep_head_anomaly_tail( items, @@ -105,14 +134,24 @@ pub(crate) fn crush_slice<'a, T, K: Eq + Hash>( }; } let tail_start = total - ROW_TAIL; + let head_anomalies = uniq[..ROW_HEAD] + .iter() + .filter(|it| severity_of(it) > 0) + .count(); + let tail_anomalies = uniq[tail_start..] + .iter() + .filter(|it| severity_of(it) > 0) + .count(); let mut scored: Vec<(u8, usize)> = (ROW_HEAD..tail_start) .map(|i| (severity_of(uniq[i]), i)) .filter(|(s, _)| *s > 0) .collect(); - let anomaly_total = scored.len(); + let middle_total = scored.len(); + let anomaly_total = head_anomalies + middle_total + tail_anomalies; scored.sort_by_key(|(s, _)| std::cmp::Reverse(*s)); scored.truncate(ROW_CAP - ROW_HEAD - ROW_TAIL); - let anomaly_kept = scored.len(); + let middle_kept = scored.len(); + let anomaly_kept = head_anomalies + middle_kept + tail_anomalies; let anomaly_idx: Vec = scored.into_iter().map(|(_, i)| i).collect(); keep_head_anomaly_tail( uniq, @@ -162,9 +201,70 @@ pub(crate) fn row_notice( anomaly_total: usize, hint: &str, ) -> String { - format!( - "…[truncated {truncated} of {total} rows ({anomaly_kept}/{anomaly_total} anomalies kept); narrow with {hint}]" - ) + if anomaly_total == 0 { + format!("…[truncated {truncated} of {total} rows; narrow with {hint}]") + } else { + format!( + "…[truncated {truncated} of {total} rows ({anomaly_kept}/{anomaly_total} anomalies kept); narrow with {hint}]" + ) + } +} + +/// Human probe note for a windowed survey (e.g. `--free` with `--limit`). +/// Decoupled from [`row_notice`]: it carries no anomaly counts, since unprobed +/// rows were never surveyed. +#[must_use] +pub(crate) fn probe_notice(probed: usize, total: usize) -> String { + format!("…[probed {probed} of {total} hosts; unprobed hosts not surveyed for locks]") +} + +/// Structured truncation metadata for `--json` envelopes. Omits the anomaly +/// keys when `anomaly_total == 0`, mirroring [`row_notice`]. +#[must_use] +pub(crate) fn truncation_meta( + truncated: usize, + total: usize, + anomaly_kept: usize, + anomaly_total: usize, + hint: &str, +) -> Value { + if anomaly_total == 0 { + json!({"truncated": truncated, "total": total, "hint": hint}) + } else { + json!({ + "truncated": truncated, + "total": total, + "anomaly_kept": anomaly_kept, + "anomaly_total": anomaly_total, + "hint": hint, + }) + } +} + +/// Structured probe metadata for `--json` envelopes. +#[must_use] +pub(crate) fn probe_meta(probed: usize, total: usize) -> Value { + json!({"probed": probed, "total": total}) +} + +/// Render a `--json` envelope: kept `rows` plus truncation/probe metadata. +/// stdout stays one valid JSON document; callers emit a plain array when both +/// are `None`. +#[must_use] +pub(crate) fn json_envelope( + rows: Vec, + truncation: Option, + probe: Option, +) -> String { + let mut obj = serde_json::Map::new(); + obj.insert("rows".to_owned(), Value::Array(rows)); + if let Some(t) = truncation { + obj.insert("truncation".to_owned(), t); + } + if let Some(p) = probe { + obj.insert("probe".to_owned(), p); + } + serde_json::to_string_pretty(&Value::Object(obj)).expect("envelope serializes") } #[cfg(test)] @@ -218,7 +318,7 @@ mod tests { }, ); assert_eq!(out.kept.len(), ROW_CAP); - assert_eq!((out.anomaly_kept, out.anomaly_total), (50, 250)); + assert_eq!((out.anomaly_kept, out.anomaly_total), (60, 260)); assert!( out.kept.contains(&250), "severe middle row survives: {:?}", @@ -235,7 +335,7 @@ mod tests { out.anomaly_total, "--limit", ); - assert!(n.contains("50/250 anomalies kept"), "{n}"); + assert!(n.contains("60/260 anomalies kept"), "{n}"); // Display stays in original order despite severity selection. let mut sorted = out.kept.clone(); sorted.sort_unstable(); @@ -283,6 +383,60 @@ mod tests { assert!(n.contains("--limit/--field/-G"), "{n}"); } + #[test] + fn notice_omits_anomaly_parenthetical_when_none() { + let n = row_notice(51, 101, 0, 0, "--limit"); + assert_eq!(n, "…[truncated 51 of 101 rows; narrow with --limit]"); + assert!(!n.contains("anomalies kept"), "{n}"); + } + + #[test] + fn anomaly_counts_cover_head_and_tail() { + // Anomalies in head (5) and tail (145) must count even though they are + // always kept; middle-only counting would report 0/0 while anomalies + // are visible. + let items: Vec = (0..150).collect(); + let out = crush(items, |v| *v, |v| u8::from(*v == 5 || *v == 145)); + assert_eq!((out.anomaly_kept, out.anomaly_total), (2, 2)); + assert!(out.kept.contains(&5) && out.kept.contains(&145)); + let n = row_notice( + out.truncated, + out.total, + out.anomaly_kept, + out.anomaly_total, + "--limit", + ); + assert!(n.contains("2/2 anomalies kept"), "{n}"); + } + + #[test] + fn snapshot_notice_and_probe_shapes() { + let with = row_notice(90, 150, 5, 60, "--limit/--field/-G"); + let without = row_notice(51, 101, 0, 0, "--limit"); + let probe = probe_notice(2, 5); + insta::assert_snapshot!(format!("{with}\n{without}\n{probe}")); + } + + #[test] + fn snapshot_json_envelope_shape() { + let rows = vec![json!({"id": "row-000"}), json!({"id": "row-149"})]; + let trunc = truncation_meta(98, 150, 1, 1, "--limit"); + let probe = probe_meta(2, 5); + insta::assert_snapshot!(json_envelope(rows, Some(trunc), Some(probe))); + } + + #[test] + fn truncation_meta_omits_anomaly_keys_when_none() { + let v = truncation_meta(51, 101, 0, 0, "--limit"); + assert_eq!(v["truncated"], 51); + assert_eq!(v["total"], 101); + assert!(v.get("anomaly_kept").is_none(), "{v}"); + assert!(v.get("anomaly_total").is_none(), "{v}"); + let v = truncation_meta(90, 150, 5, 60, "--limit"); + assert_eq!(v["anomaly_kept"], 5); + assert_eq!(v["anomaly_total"], 60); + } + #[test] fn slice_matches_owned_keep_without_cloning() { // Borrowed keys prove the no-clone path compiles and keeps identically. diff --git a/crates/mtui-core/src/commands/snapshots/mtui_core__commands__row_budget__tests__snapshot_json_envelope_shape.snap b/crates/mtui-core/src/commands/snapshots/mtui_core__commands__row_budget__tests__snapshot_json_envelope_shape.snap new file mode 100644 index 00000000..f50c517b --- /dev/null +++ b/crates/mtui-core/src/commands/snapshots/mtui_core__commands__row_budget__tests__snapshot_json_envelope_shape.snap @@ -0,0 +1,25 @@ +--- +source: crates/mtui-core/src/commands/row_budget.rs +expression: "json_envelope(rows, Some(trunc), Some(probe))" +--- +{ + "probe": { + "probed": 2, + "total": 5 + }, + "rows": [ + { + "id": "row-000" + }, + { + "id": "row-149" + } + ], + "truncation": { + "anomaly_kept": 1, + "anomaly_total": 1, + "hint": "--limit", + "total": 150, + "truncated": 98 + } +} diff --git a/crates/mtui-core/src/commands/snapshots/mtui_core__commands__row_budget__tests__snapshot_notice_and_probe_shapes.snap b/crates/mtui-core/src/commands/snapshots/mtui_core__commands__row_budget__tests__snapshot_notice_and_probe_shapes.snap new file mode 100644 index 00000000..6ff627f5 --- /dev/null +++ b/crates/mtui-core/src/commands/snapshots/mtui_core__commands__row_budget__tests__snapshot_notice_and_probe_shapes.snap @@ -0,0 +1,7 @@ +--- +source: crates/mtui-core/src/commands/row_budget.rs +expression: "format!(\"{with}\\n{without}\\n{probe}\")" +--- +…[truncated 90 of 150 rows (5/60 anomalies kept); narrow with --limit/--field/-G] +…[truncated 51 of 101 rows; narrow with --limit] +…[probed 2 of 5 hosts; unprobed hosts not surveyed for locks] diff --git a/crates/mtui-core/src/commands/updates.rs b/crates/mtui-core/src/commands/updates.rs index c9c58ec3..df64d994 100644 --- a/crates/mtui-core/src/commands/updates.rs +++ b/crates/mtui-core/src/commands/updates.rs @@ -13,7 +13,7 @@ use crate::commands::apicall::teregen_client; use crate::error::{CommandError, CommandResult}; use crate::session::Session; -use super::row_budget::{crush, row_notice}; +use super::row_budget::{crush, json_envelope, row_notice, truncation_meta}; /// Narrowing flags named in the row-budget notice. const UPDATES_HINT: &str = "--limit/--offset/--field/-G"; @@ -38,7 +38,10 @@ fn update_key( Option, ) { let field = |k: &str| v.get(k).map(ToString::to_string); - let id = field("id"); + let id = v + .get("id") + .filter(|v| !v.is_null()) + .map(ToString::to_string); // Id-less rows have no stable identity: fall back to the full row there. let rest = id.is_none().then(|| v.to_string()); (id, field("status"), field("priority"), rest) @@ -113,9 +116,8 @@ impl Command for Updates { "print the raw TeReGen rows as a JSON array (each row \ emitted whole, unlike -F; honours --limit/--offset; not combinable \ with -F); an empty queue prints []; over-cap stdout is a JSON \ - array of kept rows plus a trailing `…[truncated …` notice line — \ - naive parse of full stdout fails, strip lines starting with that \ - prefix before parsing", + envelope {\"rows\": [...], \"truncation\": {...}} so stdout stays \ + valid JSON", ), ) .arg( @@ -377,8 +379,8 @@ impl Command for Updates { // Row-cap is not byte-cap: MCP max_output_bytes can still cut mid-array on huge rows. let crushed = crush(windowed, update_key, anomaly_severity); let shown = &crushed.kept; - // Over-cap --json stays in-band like the byte-cap convention: JSON array - // plus a trailing notice line, so MCP captures the signal too. + // Over-cap --json stays in-band like the byte-cap convention, but as a + // structured envelope so stdout stays valid JSON for jq/MCP parsers. let notice = (crushed.truncated > 0).then(|| { row_notice( crushed.truncated, @@ -390,13 +392,23 @@ impl Command for Updates { }); if as_json { - let doc = Value::Array(shown.to_vec()); - session.display.println( - &serde_json::to_string_pretty(&doc) - .expect("serialising a serde_json::Value is infallible"), - ); - if let Some(notice) = notice { - session.display.println(¬ice); + if crushed.truncated > 0 { + let meta = truncation_meta( + crushed.truncated, + crushed.total, + crushed.anomaly_kept, + crushed.anomaly_total, + UPDATES_HINT, + ); + session + .display + .println(&json_envelope(shown.to_vec(), Some(meta), None)); + } else { + let doc = Value::Array(shown.to_vec()); + session.display.println( + &serde_json::to_string_pretty(&doc) + .expect("serialising a serde_json::Value is infallible"), + ); } return Ok(()); } @@ -1728,7 +1740,7 @@ mod tests { } #[tokio::test] - async fn row_budget_json_stays_parseable_with_trailing_notice() { + async fn row_budget_json_emits_envelope() { let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/updates")) @@ -1742,24 +1754,22 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); + // Envelope keeps stdout valid JSON: naive parse must succeed. + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let obj = parsed.as_object().unwrap(); + assert!(obj.contains_key("rows"), "{out}"); + assert!(obj.contains_key("truncation"), "{out}"); + let trunc = &obj["truncation"]; + assert_eq!(trunc["total"], 150, "{out}"); + assert!(trunc["truncated"].as_u64().unwrap() > 0, "{out}"); assert!( - out.lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")), + obj["truncation"]["hint"] + .as_str() + .unwrap() + .contains("--limit"), "{out}" ); - assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); - assert!( - serde_json::from_str::(out.trim()).is_err(), - "naive parse of full stdout must fail loudly: {out}" - ); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); - let rows = parsed.as_array().unwrap(); + let rows = obj["rows"].as_array().unwrap(); assert!( rows.len() <= super::super::row_budget::ROW_CAP, "{}", @@ -1767,6 +1777,7 @@ mod tests { ); assert!(rows.iter().any(|r| r["id"] == "row-anomaly"), "{out}"); assert!(!rows.iter().any(|r| r["id"] == "row-060"), "{out}"); + assert!(!out.contains("…[truncated"), "no trailing plaintext: {out}"); } #[test] @@ -1774,8 +1785,9 @@ mod tests { let base = clap::Command::new("updates").no_binary_name(true); let mut cmd = Updates.configure(base); let help = cmd.render_help().to_string(); - assert!(help.contains("…[truncated"), "{help}"); - assert!(help.contains("strip lines starting with"), "{help}"); + assert!(help.contains("truncation"), "{help}"); + assert!(help.contains("envelope"), "{help}"); + assert!(help.contains("valid JSON"), "{help}"); } #[test] @@ -1794,6 +1806,18 @@ mod tests { assert_eq!(update_key(&u1), update_key(&u1.clone())); } + #[test] + fn null_id_falls_back_to_full_row_key() { + // Explicit `"id": null` must not collapse distinct rows into one + // `Some("null")` bucket. + let n1 = + serde_json::json!({"id": null, "status": "testing", "priority": 1, "title": "one"}); + let n2 = + serde_json::json!({"id": null, "status": "testing", "priority": 1, "title": "two"}); + assert_ne!(update_key(&n1), update_key(&n2)); + assert_eq!(update_key(&n1), update_key(&n1.clone())); + } + #[test] fn unknown_status_is_anomaly_keep() { for row in [ @@ -1854,7 +1878,7 @@ mod tests { .collect(); assert!(ids.contains(&"row-severe"), "severe survives: {ids:?}"); assert!(!ids.contains(&"row-mild"), "mild drops: {ids:?}"); - assert_eq!((out.anomaly_kept, out.anomaly_total), (50, 249)); + assert_eq!((out.anomaly_kept, out.anomaly_total), (52, 251)); } #[tokio::test] @@ -1881,21 +1905,9 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - assert!( - out.lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")), - "{out}" - ); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); - let kept: Vec<_> = parsed - .as_array() - .unwrap() + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let rows = parsed["rows"].as_array().unwrap(); + let kept: Vec<_> = rows .iter() .map(|r| r["id"].as_str().unwrap_or("?")) .collect(); @@ -1932,8 +1944,8 @@ mod tests { } #[tokio::test] - async fn boundary_100_no_notice_101_truncated() { - for (n, expect_notice, expect_kept) in [(100, false, 100), (101, true, 50)] { + async fn boundary_100_plain_array_101_envelope() { + for (n, expect_envelope, expect_kept) in [(100, false, 100), (101, true, 50)] { let rows: Vec = (0..n) .map(|i| { serde_json::json!({ @@ -1954,36 +1966,32 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - let has_notice = out - .lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")); - assert_eq!(has_notice, expect_notice, "n={n}: {out}"); - if expect_notice { - assert!( - serde_json::from_str::(out.trim()).is_err(), - "naive parse must fail when notice present: n={n}: {out}" + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + if expect_envelope { + let obj = parsed.as_object().unwrap(); + assert!(obj.contains_key("rows"), "n={n}: {out}"); + assert!(obj.contains_key("truncation"), "n={n}: {out}"); + assert_eq!( + obj["rows"].as_array().unwrap().len(), + expect_kept, + "n={n}: {out}" + ); + assert!(!out.contains("…[truncated"), "n={n}: {out}"); + } else { + assert!(parsed.is_array(), "n={n}: {out}"); + assert_eq!( + parsed.as_array().unwrap().len(), + expect_kept, + "n={n}: {out}" ); } - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); - assert!(parsed.is_array(), "n={n}: {out}"); - assert_eq!( - parsed.as_array().unwrap().len(), - expect_kept, - "n={n}: {out}" - ); } } #[tokio::test] - async fn json_filter_keeps_data_line_containing_truncated() { - // A data line containing "[truncated" must not be mistaken for the notice: - // only lines starting with the `…[truncated` prefix are stripped. + async fn json_bracket_id_survives_in_plain_array() { + // A data id containing "[truncated" must survive verbatim; stdout stays + // one valid JSON document with no stripping. let rows = vec![serde_json::json!({ "priority": 1, "status": "testing", "kind": "Maintenance", "id": "row-[truncated]-fake", @@ -2000,12 +2008,7 @@ mod tests { let args = matches(&Updates, &["--status", "all", "--json"]); Updates.call(&mut session, &args).await.unwrap(); let out = buf.contents(); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); assert_eq!(parsed[0]["id"], "row-[truncated]-fake", "{out}"); } } diff --git a/crates/mtui-mcp/tests/json_crush.rs b/crates/mtui-mcp/tests/json_crush.rs index 4e26a3e8..0c3d4ef4 100644 --- a/crates/mtui-mcp/tests/json_crush.rs +++ b/crates/mtui-mcp/tests/json_crush.rs @@ -2,9 +2,8 @@ //! //! Drives the real `updates` / `list_refhosts` commands through //! [`McpSession::run_command`] with unbounded mocked backends: over-cap `--json` -//! tool output carries the truncation notice in-band after the JSON array (like -//! the byte-cap convention), so naive parse fails loudly and clients strip -//! `…[truncated` lines; under-cap stays pure JSON. Also pins the additive +//! tool output is a structured envelope (`{"rows": [...], "truncation": {...}}`) +//! so stdout stays valid JSON; under-cap stays a pure array. Also pins the additive //! paging flags and that row-cap is not byte-cap. #![cfg(feature = "mcp")] @@ -30,7 +29,7 @@ fn queue_fixture() -> serde_json::Value { serde_json::json!({"updates": rows}) } -/// `updates --json` over an unbounded queue: notice in-band, anomaly kept. +/// `updates --json` over an unbounded queue: envelope in-band, anomaly kept. #[tokio::test] async fn updates_json_crush_carries_notice_in_band() { use wiremock::matchers::{method, path}; @@ -56,24 +55,12 @@ async fn updates_json_crush_carries_notice_in_band() { .run_command(®istry, "updates", &argv) .await .expect("updates succeeds"); - assert!( - out.lines() - .last() - .is_some_and(|l| l.starts_with("…[truncated")), - "{out}" - ); - assert!(out.contains("--limit/--offset/--field/-G"), "{out}"); - assert!( - serde_json::from_str::(out.trim()).is_err(), - "naive parse of full stdout must fail loudly: {out}" - ); - let json_part: String = out - .lines() - .filter(|l| !l.starts_with("…[truncated")) - .collect::>() - .join("\n"); - let parsed: serde_json::Value = serde_json::from_str(&json_part).unwrap(); - let rows = parsed.as_array().unwrap(); + let parsed: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let obj = parsed.as_object().unwrap(); + assert!(obj.contains_key("rows"), "{out}"); + assert!(obj.contains_key("truncation"), "{out}"); + assert!(!out.contains("…[truncated"), "no trailing plaintext: {out}"); + let rows = obj["rows"].as_array().unwrap(); assert!(rows.len() <= 100, "row budget holds: {}", rows.len()); assert!(rows.iter().any(|r| r["id"] == "row-anomaly"), "{out}"); assert!(!rows.iter().any(|r| r["id"] == "row-060"), "{out}"); diff --git a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap index 5a4c9fb7..3495644c 100644 --- a/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap +++ b/crates/mtui-mcp/tests/snapshots/it__slim_profile__slimmed_command_tool_schemas_snapshot.snap @@ -1,6 +1,5 @@ --- source: crates/mtui-mcp/tests/slim_profile.rs -assertion_line: 40 expression: pretty --- [ @@ -607,7 +606,7 @@ expression: pretty }, "json": { "default": false, - "description": "emit a JSON array of kept rows; over-cap stdout adds a trailing `…[truncated …` notice line — naive parse of full stdout fails, strip lines starting with that prefix before parsing", + "description": "emit a JSON array of kept rows; over-cap stdout is a JSON envelope {\"rows\": [...], \"truncation\": {...}} so stdout stays valid JSON (--free windowing adds a \"probe\" object)", "type": "boolean" }, "limit": { @@ -1670,7 +1669,7 @@ expression: pretty }, "json": { "default": false, - "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is a JSON array of kept rows plus a trailing `…[truncated …` notice line — naive parse of full stdout fails, strip lines starting with that prefix before parsing", + "description": "print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is a JSON envelope {\"rows\": [...], \"truncation\": {...}} so stdout stays valid JSON", "type": "boolean" }, "limit": { diff --git a/docs/src/cli.md b/docs/src/cli.md index da8c310f..015125d6 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -710,7 +710,7 @@ Options: select output fields by osc-qam name (e.g. -F Rating -F 'Assigned Roles'); repeatable, rendered as one block per update; names match case-insensitively, ignoring spaces/hyphens/underscores; narrow large queues with --limit --json - print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is a JSON array of kept rows plus a trailing `…[truncated …` notice line — naive parse of full stdout fails, strip lines starting with that prefix before parsing + print the raw TeReGen rows as a JSON array (each row emitted whole, unlike -F; honours --limit/--offset; not combinable with -F); an empty queue prints []; over-cap stdout is a JSON envelope {"rows": [...], "truncation": {...}} so stdout stays valid JSON --status filter by status (default: testing); use 'all' for every status @@ -1074,7 +1074,7 @@ Options: group by test-target slot (product+version+arch+addons) --json - emit a JSON array of kept rows; over-cap stdout adds a trailing `…[truncated …` notice line — naive parse of full stdout fails, strip lines starting with that prefix before parsing + emit a JSON array of kept rows; over-cap stdout is a JSON envelope {"rows": [...], "truncation": {...}} so stdout stays valid JSON (--free windowing adds a "probe" object) --limit cap the number of rows after --offset (0 = all) From bc1cdb2dd59e3ce85ebfe944aa113a01347ad5f7 Mon Sep 17 00:00:00 2001 From: Martin Pluskal Date: Sun, 13 Sep 2026 09:40:54 +0200 Subject: [PATCH 8/8] test(mcp): pin crush_slice head/tail anomaly counts --- crates/mtui-core/src/commands/row_budget.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/mtui-core/src/commands/row_budget.rs b/crates/mtui-core/src/commands/row_budget.rs index 0be87e45..e5d5ce0a 100644 --- a/crates/mtui-core/src/commands/row_budget.rs +++ b/crates/mtui-core/src/commands/row_budget.rs @@ -409,6 +409,17 @@ mod tests { assert!(n.contains("2/2 anomalies kept"), "{n}"); } + #[test] + fn slice_anomaly_counts_cover_head_and_tail() { + // Mirror of `anomaly_counts_cover_head_and_tail` for the borrowed + // path: head (5) and tail (145) anomalies must count even though + // they are always kept; middle-only counting would report 0/0. + let items: Vec = (0..150).collect(); + let out = crush_slice(&items, |v| *v, |v| u8::from(*v == 5 || *v == 145)); + assert_eq!((out.anomaly_kept, out.anomaly_total), (2, 2)); + assert!(out.kept.contains(&&5) && out.kept.contains(&&145)); + } + #[test] fn snapshot_notice_and_probe_shapes() { let with = row_notice(90, 150, 5, 60, "--limit/--field/-G");