From 6aca7c1798a9b7bd2c724f532c2ae46a6b698c3a Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 10 Oct 2026 23:39:57 +0200 Subject: [PATCH] #781: cargo xtask parity measures menu coverage Problem: the menu parity figure in docs/target-app-parity.md came from an ad hoc label-matching script over string literals; nothing repeatable measured the real menu bar (docs/gaps.md G7). Fix: `cargo xtask parity` dumps the shipped menu bar (new `menus::default_menu_items`, deterministic: English, default prefs, built-in workspaces; printed by `examples/menu_tree.rs`) and compares it with a reference list of menu paths, by default the local, never-committed `plan/premiere/menus.json` (any JSON of paths via --reference; parsing is depth- and size-capped). Presence and placement are reported separately (same path, elsewhere, missing); app/Help/recent/account/macOS items are out of scope. Writes a markdown checklist (target/parity/ or --out) and --min fails below a presence threshold. Without a reference it prints our inventory only. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/examples/menu_tree.rs | 26 ++ crates/ui-egui/src/menus.rs | 32 ++ crates/ui-egui/src/panels/workspaces.rs | 18 +- docs/gaps.md | 17 +- docs/target-app-parity.md | 4 +- xtask/src/main.rs | 10 +- xtask/src/parity.rs | 507 ++++++++++++++++++++++++ 7 files changed, 602 insertions(+), 12 deletions(-) create mode 100644 crates/ui-egui/examples/menu_tree.rs create mode 100644 xtask/src/parity.rs diff --git a/crates/ui-egui/examples/menu_tree.rs b/crates/ui-egui/examples/menu_tree.rs new file mode 100644 index 000000000..62610ad1f --- /dev/null +++ b/crates/ui-egui/examples/menu_tree.rs @@ -0,0 +1,26 @@ +//! Print FilmCraft's menu bar as it ships (English, default preferences) as JSON: one object per +//! item with its `id`, `label` and menu `path`. `cargo xtask parity` reads this to measure menu +//! coverage against a reference list. +//! +//! ```sh +//! cargo run -p filmcraft-ui-egui --example menu_tree +//! ``` + +use std::process::ExitCode; + +use serde_json::json; + +fn main() -> ExitCode { + let items: Vec = + filmcraft_ui_egui::menus::default_menu_items().into_iter().map(|it| json!({"id": it.id, "label": it.label, "path": it.path})).collect(); + match serde_json::to_string_pretty(&items) { + Ok(text) => { + println!("{text}"); + ExitCode::SUCCESS + } + Err(e) => { + eprintln!("menu_tree: {e}"); + ExitCode::FAILURE + } + } +} diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 358a6530e..c0095c6e6 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -636,6 +636,16 @@ pub fn menu_items(app: &FilmcraftApp) -> Vec { v } +/// The menu bar as it ships, without a running app: English labels, default preferences, the +/// built-in workspaces, nothing checked. Deterministic, so `cargo xtask parity` can measure it +/// (`examples/menu_tree.rs` prints it as JSON). +pub fn default_menu_items() -> Vec { + let session = filmcraft_engine::Session::default(); + let mut v = menu_items_for(&session); + crate::panels::workspaces::default_menu(&mut v, &session); + v +} + /// Menu entries with their live shortcuts and enablement. pub fn menu_items_for(session: &filmcraft_engine::Session) -> Vec { let mut out = Vec::new(); @@ -869,3 +879,25 @@ mod parse_shortcut_tests { assert_eq!(parse_shortcut("+").map(|(_, k)| k), Some(egui::Key::Plus)); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// What `cargo xtask parity` measures: every item sits under a top-level menu, and Window ▸ + /// Workspaces lists the built-in workspaces before the layout commands, none checked. + #[test] + fn default_menu_items_are_the_shipped_menu_bar() { + let items = default_menu_items(); + assert!(items.len() > 300, "{} items", items.len()); + assert!(items.iter().all(|it| it.path.first().is_some_and(|top| MENUS.contains(&top.as_str())))); + let ws: Vec<&MenuItem> = items.iter().filter(|it| it.path == ["Window", "Workspaces"]).collect(); + for name in crate::dock::WORKSPACES { + assert!(ws.iter().any(|it| it.label == name && it.checked.is_none()), "no workspace {name}"); + } + let editing = ws.iter().position(|it| it.label == "Editing"); + let reset = ws.iter().position(|it| it.id == "window.workspace.reset"); + assert!(matches!((editing, reset), (Some(e), Some(r)) if e < r), "workspaces come before the layout commands"); + assert!(items.iter().any(|it| it.id == "file.save" && it.path == ["File"])); + } +} diff --git a/crates/ui-egui/src/panels/workspaces.rs b/crates/ui-egui/src/panels/workspaces.rs index 69d529441..502e30911 100644 --- a/crates/ui-egui/src/panels/workspaces.rs +++ b/crates/ui-egui/src/panels/workspaces.rs @@ -129,21 +129,31 @@ fn delete(app: &mut FilmcraftApp, name: &str) -> Result { /// Rebuild the Window ▸ Workspaces submenu: every workspace (the current one checked), then the /// layout commands in table order. pub fn menu(app: &FilmcraftApp, items: &mut Vec) { + splice(items, &app.workspaces, &app.session, Some(&app.ui.workspace), |n| app.ui.language.tr(n).to_string()); +} + +/// The same submenu without a running app: the built-in workspaces in English, none checked +/// (`menus::default_menu_items`). +pub fn default_menu(items: &mut Vec, session: &filmcraft_engine::Session) { + splice(items, &dock::WorkspacePrefs::default(), session, None, str::to_string); +} + +fn splice(items: &mut Vec, prefs: &dock::WorkspacePrefs, session: &filmcraft_engine::Session, current: Option<&str>, tr: impl Fn(&str) -> String) { let is_ws = |it: &MenuItem| it.path.len() == 2 && it.path[0] == "Window" && it.path[1] == "Workspaces"; let Some(at) = items.iter().position(is_ws) else { return }; let actions: Vec = items.iter().filter(|it| is_ws(it) && it.id.strip_prefix("window.workspace.").is_some_and(|k| ACTIONS.contains(&k))).cloned().collect(); items.retain(|it| !is_ws(it)); - let mut sub: Vec = dock::names(&app.workspaces) + let mut sub: Vec = dock::names(prefs) .into_iter() .map(|n| { let id = format!("window.workspace.{}", slug(&n)); MenuItem { - label: app.ui.language.tr(&n).to_string(), + label: tr(&n), path: vec!["Window".into(), "Workspaces".into()], - shortcut: app.session.shortcuts.primary(&id), + shortcut: session.shortcuts.primary(&id), enabled: true, - checked: Some(n == app.ui.workspace), + checked: current.map(|c| n == c), id, } }) diff --git a/docs/gaps.md b/docs/gaps.md index 3532ae7b5..c57e02b65 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -122,12 +122,17 @@ platforms** G4, G9, G17; **performance** G16; **ecosystem** G11; **measurement** ## G7. No measured parity tool in CI -- **Missing:** `cargo xtask parity` (menus, commands, effects, transitions, panels, preferences, - formats against `plan/premiere/` snapshots), presence and fidelity reported separately (exact, - approximate, stub), generating `docs/parity-checklist.md`. -- **Evidence:** the 92% menu figure in this pass came from an ad hoc label-matching script. -- **Estimate:** 10–20 h (the menu snapshot is local-only, so the tool needs a committed, Adobe-free - list of menu paths or runs only where `plan/` exists). +- **Done:** `cargo xtask parity` measures menus (#781): our shipped menu bar (the tree the app + builds, `examples/menu_tree.rs`) against a reference list of menu paths, by default the local + `plan/premiere/menus.json`; presence and placement reported separately (same path, elsewhere, + missing), app / Help / recent / account / macOS items out of scope, a markdown checklist, and + `--min` to fail below a presence threshold. Without a reference it prints our inventory only. +- **Missing:** the other areas (commands, effects, transitions, panels, preferences, formats), + fidelity tiers (exact, approximate, stub), a CI job, and a committed `docs/parity-checklist.md`. +- **Evidence:** the 92% menu figure in this pass came from an ad hoc label-matching script; it has + not yet been re-measured with the tool. +- **Estimate:** 8–17 h left (the menu snapshot is local-only, so CI needs a committed, Adobe-free + list of menu paths for `--reference`, or the check runs only where `plan/` exists). ## G8. UI fidelity: monitor handles, Effect Controls, docking diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 29b2f7067..2e3f7fb80 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -45,7 +45,9 @@ issues now document. That is new evidence, not drift: see [Calibration](#calibra Audio Clip / Track Effect Editor, Sequence Index, AI Models and Collaboration settings, UXP / Exchange plug-in entries. A label match is presence only, and can over-count (a label that exists in a different place); it is a lower bound on misses, not proof of behaviour. The script - is ad hoc; turning it into `cargo xtask parity` is gap [G7](gaps.md#g7-no-measured-parity-tool-in-ci). + was ad hoc; `cargo xtask parity` now measures the shipped menu bar against the snapshot, + reporting presence and placement separately (not yet re-run for this figure); the other areas + are gap [G7](gaps.md#g7-no-measured-parity-tool-in-ci). - **Effects (measured by tests).** `premiere_26_video_effects_catalogue`, `premiere_audio_set_is_complete_and_foldered` and `premiere_26_transition_tree` check the catalogue against Premiere's Effects panel: video effects 93/93 (+Legacy, Obsolete), audio diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 648f217b0..1ea276afe 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -11,10 +11,14 @@ //! - `fixtures [crate…]`: pre-generate the ffmpeg fixture matrix of the oracle tests (runs each //! crate's ignored `generate_fixtures` test, i.e. the same generators the tests use) and print //! what was made, reused or skipped. +//! - `parity [--reference FILE] [--out FILE] [--min PERCENT]`: measured menu parity of our shipped +//! menu bar against a reference list of menu paths (default: the local `plan/premiere/menus.json`), +//! presence and placement reported separately (`parity.rs`, docs/gaps.md G7). //! - `ico …`: pack PNGs into a Windows `.ico` (used by `packaging/icons.sh`). //! - `ci`: fmt check, clippy -D warnings, tests, layers, assets, wasm. mod ico; +mod parity; mod version; use std::process::{Command, ExitCode}; @@ -487,12 +491,16 @@ fn main() -> ExitCode { let rest: Vec = std::env::args().skip(2).collect(); version::run(&root(), &rest.iter().map(String::as_str).collect::>()) } + "parity" => { + let rest: Vec = std::env::args().skip(2).collect(); + parity::run(&root(), &rest.iter().map(String::as_str).collect::>()) + } "ico" => { let rest: Vec = std::env::args().skip(2).collect(); ico::run(&rest.iter().map(String::as_str).collect::>()) } _ => Err( - "usage: cargo xtask " + "usage: cargo xtask " .into(), ), }; diff --git a/xtask/src/parity.rs b/xtask/src/parity.rs new file mode 100644 index 000000000..e11a570fd --- /dev/null +++ b/xtask/src/parity.rs @@ -0,0 +1,507 @@ +//! `cargo xtask parity [--reference FILE] [--ours FILE] [--out FILE] [--min PERCENT]`: measured +//! menu parity (docs/gaps.md G7). +//! +//! Compares FilmCraft's shipped menu bar (`crates/ui-egui/examples/menu_tree.rs`, or a saved dump +//! of it given with `--ours`) with a reference list of menu paths, and reports presence and +//! placement separately: a reference item is *same place* when we have its label at the same menu +//! path, *elsewhere* when we have the label under another path, and *missing* otherwise. A label +//! match is presence only, not behaviour. +//! +//! The reference defaults to the maintainers' local menu snapshot `plan/premiere/menus.json` +//! (gitignored, never committed). Without one the tool prints our own menu inventory and measures +//! nothing. The reference may be any JSON of menu paths: an array of paths (`["File", "New", +//! "Sequence…"]`) or of `"File > New > Sequence…"` strings, a tree of nodes (`{"name": …, +//! "children" | "items" | "submenu": […]}`), or nested objects keyed by label. App, Apple and Help +//! menus, recent-file lists, account / cloud items and macOS system items are counted out of scope. +//! +//! The report goes to `target/parity/parity-checklist.md` (or `--out`); `--min` fails when presence +//! is below the given percentage. + +use std::collections::{BTreeMap, HashMap, HashSet}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; + +use serde_json::Value; + +/// Deepest menu nesting accepted from a reference file. +const MAX_DEPTH: usize = 16; +/// Most menu items accepted from a reference file. +const MAX_ITEMS: usize = 20_000; +/// Keys holding a node's label, and its children. +const LABEL_KEYS: [&str; 3] = ["name", "title", "label"]; +const CHILD_KEYS: [&str; 6] = ["children", "items", "submenu", "menu", "menuItems", "menu_items"]; +/// Keys of a wrapper object around the whole menu bar. +const WRAPPER_KEYS: [&str; 5] = ["menus", "menuBar", "menu_bar", "menubar", "items"]; + +struct Args { + reference: Option, + ours: Option, + out: Option, + min: Option, +} + +fn parse_args(args: &[&str]) -> Result { + let mut a = Args { reference: None, ours: None, out: None, min: None }; + let mut it = args.iter(); + while let Some(flag) = it.next() { + let mut value = || it.next().map(|v| v.to_string()).ok_or_else(|| format!("{flag} needs a value")); + match *flag { + "--reference" => a.reference = Some(value()?.into()), + "--ours" => a.ours = Some(value()?.into()), + "--out" => a.out = Some(value()?.into()), + "--min" => { + let v = value()?; + let pct: f64 = v.parse().map_err(|_| format!("--min {v}: not a number"))?; + if !(0.0..=100.0).contains(&pct) { + return Err(format!("--min {v}: must be 0–100")); + } + a.min = Some(pct); + } + other => { + return Err(format!( + "parity: unknown argument {other} (usage: cargo xtask parity [--reference FILE] [--ours FILE] [--out FILE] [--min PERCENT])" + )); + } + } + } + Ok(a) +} + +pub fn run(root: &Path, args: &[&str]) -> Result<(), String> { + let args = parse_args(args)?; + let ours_json = match &args.ours { + Some(p) => std::fs::read_to_string(p).map_err(|e| format!("{}: {e}", p.display()))?, + None => dump_ours()?, + }; + let ours = our_items(&serde_json::from_str(&ours_json).map_err(|e| format!("our menu dump: {e}"))?)?; + let default_ref = root.join("plan/premiere/menus.json"); + let reference = match &args.reference { + Some(p) => Some(p.clone()), + None => default_ref.exists().then_some(default_ref), + }; + let Some(reference) = reference else { + print!("{}", inventory(&ours)); + println!("\nno reference menu list (plan/premiere/menus.json, or --reference FILE): menu parity not measured"); + return match args.min { + Some(_) => Err("--min needs a reference menu list".into()), + None => Ok(()), + }; + }; + let text = std::fs::read_to_string(&reference).map_err(|e| format!("{}: {e}", reference.display()))?; + let value: Value = serde_json::from_str(&text).map_err(|e| format!("{}: {e}", reference.display()))?; + let paths = reference_paths(&value)?; + let name = reference.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); + let report = compare(&paths, &ours); + let out = args.out.unwrap_or_else(|| crate::target_dir().join("parity/parity-checklist.md")); + if let Some(dir) = out.parent() { + std::fs::create_dir_all(dir).map_err(|e| format!("{}: {e}", dir.display()))?; + } + std::fs::write(&out, report.markdown(&name, ours.len())).map_err(|e| format!("{}: {e}", out.display()))?; + println!("{}", report.summary()); + println!("report: {}", out.display()); + match args.min { + Some(min) if report.presence() < min => Err(format!("menu presence {:.1}% is below --min {min}%", report.presence())), + _ => Ok(()), + } +} + +/// Our menu bar as JSON, from the `menu_tree` example. +fn dump_ours() -> Result { + let mut cmd = Command::new(env!("CARGO")); + cmd.args(["run", "-q", "-p", "filmcraft-ui-egui", "--example", "menu_tree"]).stderr(Stdio::inherit()); + let out = cmd.output().map_err(|e| format!("menu_tree: {e}"))?; + if !out.status.success() { + return Err(format!("menu_tree failed: {}", out.status)); + } + String::from_utf8(out.stdout).map_err(|e| format!("menu_tree: {e}")) +} + +/// Full paths (menu path + label) of our menu items, from the `menu_tree` dump. +fn our_items(v: &Value) -> Result>, String> { + let items = v.as_array().ok_or("our menu dump is not an array")?; + Ok(items + .iter() + .filter_map(|it| { + let label = it.get("label")?.as_str()?; + let mut path: Vec = it.get("path")?.as_array()?.iter().filter_map(|p| p.as_str().map(str::to_string)).collect(); + path.push(label.to_string()); + (path.len() >= 2).then_some(path) + }) + .collect()) +} + +/// Item counts per top-level menu, for runs without a reference. +fn inventory(ours: &[Vec]) -> String { + let mut per: BTreeMap<&str, usize> = BTreeMap::new(); + for p in ours { + if let Some(top) = p.first() { + *per.entry(top.as_str()).or_default() += 1; + } + } + let mut s = format!("FilmCraft menu bar: {} items\n", ours.len()); + for (top, n) in per { + s.push_str(&format!(" {top:<22} {n}\n")); + } + s +} + +/// Leaf menu paths (top-level menu first, item label last) of a reference file. +fn reference_paths(v: &Value) -> Result>, String> { + let mut root = v; + if let Value::Object(o) = v + && label_of(o).is_none() + && let Some(inner) = WRAPPER_KEYS.iter().find_map(|k| o.get(*k).filter(|c| c.is_array() || c.is_object())) + { + root = inner; + } + let mut out = Vec::new(); + walk(root, &[], 0, &mut out)?; + Ok(out) +} + +fn label_of(o: &serde_json::Map) -> Option<&str> { + LABEL_KEYS.iter().find_map(|k| o.get(*k)?.as_str()) +} + +/// A separator or placeholder rather than a menu item. +fn is_separator(label: &str) -> bool { + let l = label.trim(); + l.is_empty() || l.chars().all(|c| matches!(c, '-' | '—' | '–' | '_')) || l.eq_ignore_ascii_case("missing value") || l.eq_ignore_ascii_case("separator") +} + +fn push_leaf(prefix: &[String], label: &str, out: &mut Vec>) -> Result<(), String> { + if is_separator(label) || prefix.iter().any(|p| is_separator(p)) { + return Ok(()); + } + if out.len() >= MAX_ITEMS { + return Err(format!("reference has more than {MAX_ITEMS} menu items")); + } + let mut path = prefix.to_vec(); + path.push(label.trim().to_string()); + if path.len() >= 2 { + out.push(path); + } + Ok(()) +} + +fn split_path(s: &str) -> Option> { + let parts: Vec = s.replace('\u{a0}', " ").split(['>', '▸']).map(|p| p.trim().to_string()).collect(); + (parts.len() >= 2 && parts.iter().all(|p| !p.is_empty())).then_some(parts) +} + +fn walk(v: &Value, prefix: &[String], depth: usize, out: &mut Vec>) -> Result<(), String> { + if depth > MAX_DEPTH { + return Err(format!("reference menus are nested deeper than {MAX_DEPTH} levels")); + } + let with = |label: &str| { + let mut p = prefix.to_vec(); + p.push(label.trim().to_string()); + p + }; + match v { + Value::String(s) => match split_path(s).filter(|_| prefix.is_empty()) { + Some(parts) => match parts.split_last() { + Some((label, menus)) => push_leaf(menus, label, out), + None => Ok(()), + }, + None => push_leaf(prefix, s, out), + }, + Value::Array(a) => { + for el in a { + match el { + Value::Array(path) if !path.is_empty() && path.iter().all(Value::is_string) => { + let parts: Vec = prefix.iter().cloned().chain(path.iter().filter_map(|p| p.as_str().map(str::to_string))).collect(); + if let Some((label, menus)) = parts.split_last() { + push_leaf(menus, label, out)?; + } + } + _ => walk(el, prefix, depth + 1, out)?, + } + } + Ok(()) + } + Value::Object(o) => match label_of(o) { + Some(label) => { + let children = CHILD_KEYS.iter().find_map(|k| o.get(*k).filter(|c| has_entries(c))); + match children { + Some(c) if !is_separator(label) => walk(c, &with(label), depth + 1, out), + Some(_) => Ok(()), + None => push_leaf(prefix, label, out), + } + } + None => { + for (k, val) in o { + if has_entries(val) { + walk(val, &with(k), depth + 1, out)?; + } else { + push_leaf(prefix, k, out)?; + } + } + Ok(()) + } + }, + _ => Ok(()), + } +} + +fn has_entries(v: &Value) -> bool { + match v { + Value::Array(a) => !a.is_empty(), + Value::Object(o) => !o.is_empty(), + _ => false, + } +} + +/// A label compared case-insensitively, without ellipsis, with `&` as "and" and one space. +fn norm(label: &str) -> String { + let l = label.to_lowercase().replace('…', "").replace("...", "").replace(['’', '‘'], "'").replace('&', " and "); + l.split_whitespace().collect::>().join(" ") +} + +/// Why a reference item is not counted, if it isn't. +fn out_of_scope(path: &[String]) -> Option<&'static str> { + let (top, label) = (norm(path.first()?), norm(path.last()?)); + if top == "apple" || top == "help" || top.contains("premiere") { + return Some("app, Apple and Help menus"); + } + let parents = path.get(..path.len().saturating_sub(1)).unwrap_or_default(); + if parents.iter().skip(1).any(|p| norm(p).starts_with("open recent") || norm(p).starts_with("recent")) { + return Some("recent-file lists"); + } + if ["adobe", "creative cloud", "sign in", "sign out", "log in", "log out"].iter().any(|w| label.contains(w)) { + return Some("account and cloud items"); + } + if ["autofill", "start dictation", "emoji and symbols", "writing tools", "services"].iter().any(|w| label.starts_with(w)) { + return Some("macOS system items"); + } + None +} + +enum Placement { + Same, + Elsewhere(Vec), + Missing, +} + +struct Report { + /// (reference path, where it is in FilmCraft). + items: Vec<(Vec, Placement)>, + out_of_scope: BTreeMap<&'static str, usize>, + ours_only: Vec>, +} + +fn compare(reference: &[Vec], ours: &[Vec]) -> Report { + let key = |p: &[String]| p.iter().map(|s| norm(s)).collect::>(); + let our_paths: HashSet> = ours.iter().map(|p| key(p)).collect(); + let mut by_label: HashMap> = HashMap::new(); + for p in ours { + if let Some(l) = p.last() { + by_label.entry(norm(l)).or_insert(p); + } + } + let mut report = Report { items: Vec::new(), out_of_scope: BTreeMap::new(), ours_only: Vec::new() }; + let mut seen = HashSet::new(); + for p in reference { + if !seen.insert(key(p)) { + continue; + } + if let Some(why) = out_of_scope(p) { + *report.out_of_scope.entry(why).or_default() += 1; + continue; + } + let label = p.last().map(|l| norm(l)).unwrap_or_default(); + let placement = if our_paths.contains(&key(p)) { + Placement::Same + } else if let Some(at) = by_label.get(&label) { + Placement::Elsewhere((*at).clone()) + } else { + Placement::Missing + }; + report.items.push((p.clone(), placement)); + } + let ref_labels: HashSet = reference.iter().filter_map(|p| p.last().map(|l| norm(l))).collect(); + report.ours_only = ours.iter().filter(|p| p.last().is_some_and(|l| !ref_labels.contains(&norm(l)))).cloned().collect(); + report +} + +fn pct(n: usize, of: usize) -> f64 { + if of == 0 { 0.0 } else { n as f64 * 100.0 / of as f64 } +} + +fn show(p: &[String]) -> String { + p.join(" ▸ ") +} + +impl Report { + fn counts(&self) -> (usize, usize, usize) { + let same = self.items.iter().filter(|(_, pl)| matches!(pl, Placement::Same)).count(); + let elsewhere = self.items.iter().filter(|(_, pl)| matches!(pl, Placement::Elsewhere(_))).count(); + (same, elsewhere, self.items.len().saturating_sub(same + elsewhere)) + } + + /// Reference items whose label we have anywhere, in percent of the items in scope. + fn presence(&self) -> f64 { + let (same, elsewhere, _) = self.counts(); + pct(same + elsewhere, self.items.len()) + } + + fn summary(&self) -> String { + let (same, elsewhere, missing) = self.counts(); + let n = self.items.len(); + format!( + "menus: presence {}/{n} ({:.1}%), same place {same}/{n} ({:.1}%), elsewhere {elsewhere}, missing {missing}, out of scope {}", + same + elsewhere, + self.presence(), + pct(same, n), + self.out_of_scope.values().sum::() + ) + } + + fn markdown(&self, reference: &str, ours: usize) -> String { + let (same, elsewhere, missing) = self.counts(); + let n = self.items.len(); + let mut s = String::from("# Menu parity checklist\n\n"); + s.push_str(&format!( + "Generated by `cargo xtask parity` from `{reference}` ({n} items in scope, {} out of scope) against FilmCraft's \ + shipped menu bar ({ours} items). *Same place*: we have the label at the same menu path; *elsewhere*: under \ + another path; *missing*: nowhere. A label match is presence only, not behaviour.\n\n", + self.out_of_scope.values().sum::() + )); + s.push_str("| Menu | Items | Same place | Elsewhere | Missing | Presence |\n|---|---|---|---|---|---|\n"); + let mut per: Vec<(&str, [usize; 3])> = Vec::new(); + for (p, pl) in &self.items { + let top = p.first().map(String::as_str).unwrap_or_default(); + let i = match per.iter().position(|(t, _)| *t == top) { + Some(i) => i, + None => { + per.push((top, [0; 3])); + per.len() - 1 + } + }; + let col = match pl { + Placement::Same => 0, + Placement::Elsewhere(_) => 1, + Placement::Missing => 2, + }; + if let Some(c) = per.get_mut(i).and_then(|(_, c)| c.get_mut(col)) { + *c += 1; + } + } + for (top, [a, b, c]) in &per { + let total = a + b + c; + s.push_str(&format!("| {top} | {total} | {a} | {b} | {c} | {:.1}% |\n", pct(a + b, total))); + } + s.push_str(&format!("| **Total** | **{n}** | **{same}** | **{elsewhere}** | **{missing}** | **{:.1}%** |\n", self.presence())); + s.push_str(&format!("\n## Missing ({missing})\n\n")); + for (p, _) in self.items.iter().filter(|(_, pl)| matches!(pl, Placement::Missing)) { + s.push_str(&format!("- [ ] {}\n", show(p))); + } + s.push_str(&format!("\n## Elsewhere in FilmCraft ({elsewhere})\n\n")); + for (p, pl) in &self.items { + if let Placement::Elsewhere(at) = pl { + s.push_str(&format!("- {} → {}\n", show(p), show(at))); + } + } + s.push_str("\n## Out of scope\n\n"); + for (why, k) in &self.out_of_scope { + s.push_str(&format!("- {why}: {k}\n")); + } + s.push_str(&format!("\n## FilmCraft only ({})\n\n", self.ours_only.len())); + for p in &self.ours_only { + s.push_str(&format!("- {}\n", show(p))); + } + s + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn paths(v: &[&[&str]]) -> Vec> { + v.iter().map(|p| p.iter().map(|s| s.to_string()).collect()).collect() + } + + #[test] + fn reference_formats_give_the_same_paths() { + let want = paths(&[&["File", "New", "Sequence…"], &["File", "Save"], &["Edit", "Undo"]]); + let forms = [ + json!([["File", "New", "Sequence…"], ["File", "Save"], ["Edit", "Undo"]]), + json!(["File > New > Sequence…", "File ▸ Save", "Edit > Undo"]), + json!({"menus": [ + {"name": "File", "children": [ + {"name": "New", "items": [{"name": "Sequence…", "shortcut": "Cmd+N"}]}, + {"name": "missing value"}, + {"title": "Save"} + ]}, + {"name": "Edit", "submenu": [{"label": "Undo"}]} + ]}), + json!({"Edit": {"Undo": "Cmd+Z"}, "File": {"New": ["Sequence…"], "-": null, "Save": null}}), + ]; + for f in forms { + let mut got = reference_paths(&f).unwrap(); + got.sort(); + let mut w = want.clone(); + w.sort(); + assert_eq!(got, w, "{f}"); + } + } + + #[test] + fn hostile_reference_is_refused_not_overflowed() { + let mut v = json!("leaf"); + for _ in 0..200 { + v = json!({ "name": "m", "children": [v] }); + } + assert!(reference_paths(&v).unwrap_err().contains("nested deeper")); + let many: Vec = (0..MAX_ITEMS + 1).map(|i| format!("File > Item {i}")).collect(); + assert!(reference_paths(&json!(many)).unwrap_err().contains("more than")); + // odd values are skipped, not trusted + assert_eq!(reference_paths(&json!([1, null, true, {"name": 5}, ["File"]])).unwrap(), Vec::>::new()); + } + + #[test] + fn presence_and_placement_are_reported_separately() { + let reference = paths(&[ + &["File", "New", "Sequence..."], + &["File", "Export Frame"], + &["Edit", "Find & Replace"], + &["Edit", "Spelling"], + &["Help", "Premiere Pro Help…"], + &["File", "Open Recent", "clip.prproj"], + &["Edit", "AutoFill"], + &["Edit", "Spelling"], + ]); + let ours = paths(&[&["File", "New", "Sequence…"], &["File", "Export", "Export Frame"], &["Edit", "Find and Replace…"], &["Clip", "Nest…"]]); + let r = compare(&reference, &ours); + assert_eq!(r.counts(), (2, 1, 1), "duplicates count once; out-of-scope items are not counted"); + assert!((r.presence() - 75.0).abs() < 1e-9); + assert_eq!(r.out_of_scope.values().sum::(), 3); + assert_eq!(r.ours_only, paths(&[&["Clip", "Nest…"]])); + let md = r.markdown("menus.json", ours.len()); + assert!(md.contains("| **Total** | **4** | **2** | **1** | **1** | **75.0%** |"), "{md}"); + assert!(md.contains("- [ ] Edit ▸ Spelling")); + assert!(md.contains("- File ▸ Export Frame → File ▸ Export ▸ Export Frame")); + assert!(r.summary().contains("presence 3/4 (75.0%), same place 2/4 (50.0%)")); + } + + #[test] + fn empty_reference_measures_zero_without_dividing_by_zero() { + let r = compare(&[], &paths(&[&["File", "Save"]])); + assert_eq!(r.presence(), 0.0); + assert!(r.summary().contains("presence 0/0 (0.0%)")); + } + + #[test] + fn our_dump_and_arguments_are_checked() { + let ours = our_items(&json!([{"id": "file.save", "label": "Save", "path": ["File"]}, {"id": "x", "label": "Orphan", "path": []}, {"bad": 1}])).unwrap(); + assert_eq!(ours, paths(&[&["File", "Save"]])); + assert!(our_items(&json!({})).is_err()); + assert!(parse_args(&["--min", "101"]).is_err()); + assert!(parse_args(&["--min", "x"]).is_err()); + assert!(parse_args(&["--out"]).is_err()); + assert!(parse_args(&["--bogus"]).is_err()); + assert!(parse_args(&["--min", "90", "--reference", "r.json"]).is_ok_and(|a| a.min == Some(90.0) && a.reference.is_some())); + } +}