diff --git a/crates/comp-verbs/src/build_phase.rs b/crates/comp-verbs/src/build_phase.rs index ee00dc28d..731cd4fbc 100644 --- a/crates/comp-verbs/src/build_phase.rs +++ b/crates/comp-verbs/src/build_phase.rs @@ -480,7 +480,11 @@ fn gate_plates(io: &Io) -> Gate { g.plates = Some(vec![]); return g; } - let comp = spec.get("comp").and_then(Value::as_str).and_then(|c| load_raster(io, c).ok()); + let Some(comp) = spec.get("comp").and_then(Value::as_str).and_then(|c| load_raster(io, c).ok()) else { + let mut gate = Gate::fail(vec!["cannot validate plates: the spec's comp is missing or unreadable".into()]); + gate.plates = Some(vec![]); + return gate; + }; let mut reasons: Vec = Vec::new(); let mut plates: Vec = Vec::new(); for rr in &raster_regions { @@ -512,8 +516,9 @@ fn gate_plates(io: &Io) -> Gate { img.image.width, px_w as i64, round(min_w) as i64 )); } - let mut score_val: Option = None; - if let Some(comp) = &comp { + let score_val; + { + let comp = ∁ let refimg = plate_reference(comp, &spec, rr); // composite transparent plates over the region's sampled ground let mut build = img.image.clone(); @@ -600,6 +605,19 @@ fn plate_receipt_current(io: &Io, state: &Value, spec: &Value, region: &Value) - && receipt.get("regionHash").and_then(Value::as_str) == Some(sha256_bytes(util::json_pretty(region).as_bytes()).as_str()) } +fn revalidate_plates(io: &Io, state: &mut Value, spec: Option<&Value>) -> Option { + let spec = spec?; + let stale = spec_regions(spec).iter() + .filter(|r| r.get("medium").and_then(Value::as_str) == Some("raster")) + .any(|r| !plate_receipt_current(io, state, spec, r)); + if stale { + let gate = gate_plates(io); + save_plate_receipts(state, &gate); + if !gate.ok { return Some(gate); } + } + None +} + fn hex_rgba(hex: &str) -> Option<[u8; 4]> { let re = regex_hex(); let caps = re.captures(hex)?; @@ -957,6 +975,7 @@ struct HeroReadings { chrome: Vec, plates: Vec, invented: Value, + region_ids: std::collections::HashMap>, } fn hero_readings(io: &Io, state: &Value, spec: Option<&Value>, build_path: &str) -> Option { @@ -974,7 +993,9 @@ fn hero_readings(io: &Io, state: &Value, spec: Option<&Value>, build_path: &str) let mut text: Vec = Vec::new(); let mut chrome: Vec = Vec::new(); let mut plates: Vec = Vec::new(); + let mut region_ids = std::collections::HashMap::>::new(); for rr in spec_regions(spec) { + let starts = (text.len(), chrome.len(), plates.len()); let px = rr.get("px"); if px.is_none() { continue; @@ -1017,9 +1038,12 @@ fn hero_readings(io: &Io, state: &Value, spec: Option<&Value>, build_path: &str) )); } } + for message in text[starts.0..].iter().chain(chrome[starts.1..].iter()).chain(plates[starts.2..].iter()) { + region_ids.entry(message.clone()).or_default().push(region.id.clone()); + } } let invented = invented_ink(&comp, &aligned); - Some(HeroReadings { text, chrome, plates, invented }) + Some(HeroReadings { text, chrome, plates, invented, region_ids }) } // ---- hero gate ------------------------------------------------------------- @@ -1043,7 +1067,7 @@ fn hero_diff(io: &Io, comp_path: &str, build_path: &str, spec: Option<&Value>, o let comp = load_raster(io, comp_path)?; let build = load_raster(io, build_path)?; let res = compare(&comp, &build, spec, "top", "hero", None); - let files = write_artifacts(&res, &comp, &abs(io, out_dir)); + let files = write_artifacts(&res, &comp, &abs(io, out_dir)).map_err(|e| format!("cannot persist comparison artifacts: {e}"))?; let meta = json!({ "label": "hero", "comp": comp_path, @@ -1053,22 +1077,19 @@ fn hero_diff(io: &Io, comp_path: &str, build_path: &str, spec: Option<&Value>, o "buildSize": format!("{}x{}", build.width, build.height), }); let report = build_report(&res, Some(&files), &meta); - let _ = std::fs::write(abs(io, &format!("{out_dir}/report.json")), util::json_pretty(&report)); + Ok((report, res)) } #[allow(clippy::too_many_arguments)] fn gate_hero(io: &Io, state: &mut Value, build_path: &str, min: f64, out_dir: &str, artifact: Option<&str>, organic_scan: OrganicScan) -> Gate { + let pending = Gate::fail(vec!["hero comparison has not completed".into()]); + if let Err(e) = unavailable_report(io, out_dir, &pending) { + return Gate::fail(vec![format!("cannot persist hero gate evidence: {e}")]); + } let mut gate = gate_hero_inner(io, state, build_path, min, out_dir, artifact, organic_scan); - if gate.score.is_none() { - // An earlier capture must not masquerade as evidence for a failed preflight. - let report = json!({ "interpretation": "hero-gate", "measurementsAvailable": false, - "regions": [], "gate": { "ok": gate.ok, "reasons": gate.reasons, "advisories": gate.advisories, - "unscopedReasons": gate.reasons } }); - let result = std::fs::create_dir_all(abs(io, out_dir)).and_then(|_| { - std::fs::write(abs(io, &format!("{out_dir}/report.json")), util::json_pretty(&report)) - }); - if let Err(e) = result { + if gate.report.is_none() { + if let Err(e) = unavailable_report(io, out_dir, &gate) { gate.ok = false; gate.reasons.push(format!("cannot persist hero gate evidence: {e}")); } @@ -1076,6 +1097,25 @@ fn gate_hero(io: &Io, state: &mut Value, build_path: &str, min: f64, out_dir: &s gate } +fn atomic_report(path: &Path, report: &Value) -> Result<(), String> { + static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + let temp = path.with_extension(format!("tmp-{}-{}", std::process::id(), NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed))); + let result = (|| { + if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } + std::fs::write(&temp, util::json_pretty(report))?; + std::fs::rename(&temp, path) + })(); + if result.is_err() { let _ = std::fs::remove_file(temp); } + result.map_err(|e: std::io::Error| e.to_string()) +} + +fn unavailable_report(io: &Io, out_dir: &str, gate: &Gate) -> Result<(), String> { + let report = json!({ "interpretation": "hero-gate", "measurementsAvailable": false, + "regions": [], "gate": { "ok": false, "reasons": gate.reasons, + "advisories": gate.advisories, "unscopedReasons": gate.reasons } }); + atomic_report(&abs(io, &format!("{out_dir}/report.json")), &report) +} + #[allow(clippy::too_many_arguments)] fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_dir: &str, artifact: Option<&str>, organic_scan: OrganicScan) -> Gate { let s = self_cmd(io); @@ -1086,16 +1126,7 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d let spec_gate = gate_spec(io, state); if !spec_gate.ok { return spec_gate; } let spec_for_refs = load_spec(&abs(io, SPEC_PATH)); - if let Some(spec) = &spec_for_refs { - let stale = spec_regions(spec).iter() - .filter(|r| r.get("medium").and_then(Value::as_str) == Some("raster")) - .any(|r| !plate_receipt_current(io, state, spec, r)); - if stale { - let plates = gate_plates(io); - save_plate_receipts(state, &plates); - if !plates.ok { return plates; } - } - } + if let Some(failure) = revalidate_plates(io, state, spec_for_refs.as_ref()) { return failure; } // resolve the page let mut page_file: Option = artifact.map(String::from).or_else(|| state.get("artifact").and_then(Value::as_str).map(String::from)); if page_file.as_ref().map(|p| !abs(io, p).exists()).unwrap_or(true) { @@ -1227,7 +1258,7 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d && state.get("plates").and_then(|p| p.get(id)).and_then(|p| p.get("score")).and_then(Value::as_f64).is_some_and(|s| s >= PLATE_MIN)) .unwrap_or(false) }; - let mut placement_notes: Vec = Vec::new(); + let mut placement_notes: Vec<(String, String)> = Vec::new(); for r in regions.iter_mut() { let kind = r.get("kind").and_then(Value::as_str).unwrap_or("").to_string(); let id = r.get("id").and_then(Value::as_str).unwrap_or("").to_string(); @@ -1257,10 +1288,10 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d let (bw_, bh_, bx, by) = (cf(&ib, "w"), cf(&ib, "h"), cf(&ib, "x"), cf(&ib, "y")); let off = (bw_ - cw_).abs() > cw_ * 0.2 || (bh_ - ch_).abs() > ch_ * 0.2 || (bx - cx).abs() > cw_ * 0.15 || (by - cy).abs() > ch_ * 0.15; if off { - placement_notes.push(format!( + placement_notes.push((id.to_string(), format!( "plate {id} is placed but not at the comp's box: its ink spans {}x{}px at ({},{}) in the comp region and {}x{}px at ({},{}) in the build; size and position the to the spec box (object-fit: cover), not to the surrounding layout", cw_ as i64, ch_ as i64, cx as i64, cy as i64, bw_ as i64, bh_ as i64, bx as i64, by as i64 - )); + ))); } } } @@ -1274,11 +1305,11 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d } } } - for n in placement_notes { + for (id, n) in placement_notes { if above_bar { advisories.push(format!("(advisory, above the {}% bar) {n}", pct0(min))); } else { - reasons.push(n); + push_region_blocker(&mut reasons, &mut region_reasons, &id, n); } } // contradicted @@ -1355,12 +1386,16 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d let other_contradicted: Vec<&Value> = contradicted.iter().filter(|r| !direction_contradicted.iter().any(|d| d.get("id") == r.get("id"))).collect(); let allow = 1usize.max(regions.len() / 3); if other_contradicted.len() > allow { - reasons.push(format!( + let message = format!( "{} of {} regions contradicted: {}", other_contradicted.len(), regions.len(), other_contradicted.iter().filter_map(|r| r.get("id").and_then(Value::as_str)).collect::>().join(", ") - )); + ); + for r in &other_contradicted { + if let Some(id) = r.get("id").and_then(Value::as_str) { record_region_reason(&mut region_reasons, id, &message); } + } + reasons.push(message); } // organic clip + svg illustrations let artifact_file = page_file.clone(); @@ -1369,7 +1404,7 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d for o in organic_clip_regions(io, af, spec, organic_scan) { let id = o.get("id").and_then(Value::as_str).unwrap_or(""); let snip = o.get("snippet").and_then(Value::as_str).unwrap_or(""); - reasons.push(format!("artifact draws an organic clip-path ({snip}) inside raster region {id}'s box; that region ships as its plate, never as a polygon")); + push_region_blocker(&mut reasons, &mut region_reasons, id, format!("artifact draws an organic clip-path ({snip}) inside raster region {id}'s box; that region ships as its plate, never as a polygon")); } let svgs = std::fs::read_to_string(abs(io, af)).map(|h| svg_illustrations(&h)).unwrap_or_default(); for v in svgs.iter().take(6) { @@ -1392,7 +1427,6 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d if let Some(readings) = readings { use once_cell::sync::Lazy; static FOLD: Lazy = Lazy::new(|| Regex::new(r"(?i)^text ([a-z0-9]+(?:-[a-z0-9]+)*?)(?:-(?:\d+|[a-z]))?: (cap height|\d+ lines? in the build|the face renders|ink is|its first line|it starts|line pitch)").unwrap()); - static IDM: Lazy = Lazy::new(|| Regex::new(r"^text ([^:]+):").unwrap()); static CAP: Lazy = Lazy::new(|| Regex::new(r"cap height").unwrap()); static LINES: Lazy = Lazy::new(|| Regex::new(r"lines? in the build").unwrap()); static HEAV: Lazy = Lazy::new(|| Regex::new(r"heavier|lighter").unwrap()); @@ -1401,7 +1435,8 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d let order = |f: &str| -> u8 { if CAP.is_match(f) { 0 } else if LINES.is_match(f) { 1 } else if HEAV.is_match(f) { 2 } else if INKIS.is_match(f) { 3 } else { 4 } }; - // fold sibling text findings + // Keep region provenance when sibling text findings are folded. + let mut reading_ids = readings.region_ids.clone(); let mut folded: Vec<(String, String, Vec)> = Vec::new(); // (key, first, ids) for f in &readings.text { let key = if let Some(m) = FOLD.captures(f) { @@ -1409,24 +1444,21 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d } else { f.clone() }; - let idm = IDM.captures(f).map(|m| m[1].to_string()); + let ids = readings.region_ids.get(f).cloned().unwrap_or_default(); if let Some(entry) = folded.iter_mut().find(|(k, _, _)| *k == key) { - if let Some(id) = idm { - entry.2.push(id); - } + entry.2.extend(ids); } else { - let ids = idm.map(|id| vec![id]).unwrap_or_default(); folded.push((key, f.clone(), ids)); } } let mut text: Vec = folded .into_iter() .map(|(_, first, ids)| { - if ids.len() > 1 { + let message = if ids.len() > 1 { format!("{first} (also {})", ids[1..].join(", ")) - } else { - first - } + } else { first }; + reading_ids.insert(message.clone(), ids); + message }) .collect(); text.sort_by_key(|a| order(a)); @@ -1466,19 +1498,15 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d advisories.push(format!(" {f}")); } } else { - if !kept.is_empty() { - let of = if fresh.len() > kept.len() { format!("{} of {}, the rest after these", kept.len(), fresh.len()) } else { format!("{}", kept.len()) }; - reasons.push(format!("READINGS, each one CSS edit ({of}):")); - } for f in &kept { - reasons.push(f.clone()); + push_reading_blocker(&mut reasons, &mut region_reasons, &reading_ids, f); } } for f in &advisory_stale { advisories.push(format!("(advisory, unchanged for 3+ attempts) {f}")); } for f in &readings.plates { - reasons.push(f.clone()); + push_reading_blocker(&mut reasons, &mut region_reasons, &reading_ids, f); } let cells = readings.invented.get("cells").and_then(Value::as_array).cloned().unwrap_or_default(); let fraction = readings.invented.get("fraction").and_then(Value::as_f64).unwrap_or(0.0); @@ -1494,8 +1522,7 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d } } // worst regions - let mut worst_sorted = regions.clone(); - worst_sorted.sort_by(|a, b| rscore(a, "overall").partial_cmp(&rscore(b, "overall")).unwrap()); + let worst_sorted = repair_regions(®ions, ®ion_reasons); let worst_top: Vec<&Value> = worst_sorted.iter().take(3).collect(); let region_dir = format!("{out_dir}/regions"); let mut g = Gate::blank(); @@ -1528,23 +1555,45 @@ fn gate_hero_inner(io: &Io, state: &mut Value, build_path: &str, min: f64, out_d let raw = report.clone(); apply_gate_evidence(&mut report, &mut measured, ®ions, &g); report["rawReport"] = json!(raw_path); - let evidence_write = std::fs::write(abs(io, &raw_path), util::json_pretty(&raw)) - .and_then(|_| std::fs::write(abs(io, &format!("{out_dir}/report.json")), util::json_pretty(&report))); + let evidence_write = (|| { + atomic_report(&abs(io, &raw_path), &raw)?; + write_region_artifacts(&measured, &abs(io, out_dir), report.get("regions").and_then(Value::as_array).map(Vec::as_slice))?; + atomic_report(&abs(io, &format!("{out_dir}/report.json")), &report) + })(); if let Err(e) = evidence_write { g.ok = false; g.reasons.push(format!("cannot persist hero gate evidence: {e}")); - } else { - write_region_artifacts(&measured, &abs(io, out_dir), report.get("regions").and_then(Value::as_array).map(Vec::as_slice)); + g.report = None; + g.side_by_side = None; + g.worst_crops.clear(); } g } fn push_region_blocker(reasons: &mut Vec, regions: &mut Map, id: &str, message: String) { - regions.entry(id.to_string()).or_insert_with(|| json!([])).as_array_mut().unwrap().push(json!(message)); + record_region_reason(regions, id, &message); reasons.push(message); } +fn record_region_reason(regions: &mut Map, id: &str, message: &str) { + regions.entry(id.to_string()).or_insert_with(|| json!([])).as_array_mut().unwrap().push(json!(message)); +} + +fn push_reading_blocker(reasons: &mut Vec, regions: &mut Map, ids: &std::collections::HashMap>, message: &str) { + for id in ids.get(message).into_iter().flatten() { record_region_reason(regions, id, message); } + reasons.push(message.to_string()); +} + +fn repair_regions(regions: &[Value], blockers: &Map) -> Vec { + let mut ordered: Vec = regions.iter().filter(|region| { + region.get("id").and_then(Value::as_str).is_some_and(|id| + blockers.get(id).and_then(Value::as_array).is_some_and(|reasons| !reasons.is_empty())) + }).cloned().collect(); + ordered.sort_by(|a, b| rscore(a, "overall").total_cmp(&rscore(b, "overall"))); + ordered +} + /// Raw scores never change during interpretation. Only gate verdicts and their /// basis are published alongside them; the original report is retained separately. fn apply_gate_evidence(report: &mut Value, measured: &mut CompareResult, regions: &[Value], gate: &Gate) { @@ -1608,7 +1657,7 @@ fn hash_file(io: &Io, file: &str) -> Option { Some(d.iter().map(|b| format!("{b:02x}")).collect::()[..12].to_string()) } -fn gate_responsive(io: &Io, state: &Value, min: f64, out_dir: &str) -> Gate { +fn gate_responsive(io: &Io, state: &mut Value, min: f64, out_dir: &str) -> Gate { let desktop = ".impeccable/review/desktop.png"; let mobile = ".impeccable/review/mobile.png"; let mut reasons = Vec::new(); @@ -1621,7 +1670,10 @@ fn gate_responsive(io: &Io, state: &Value, min: f64, out_dir: &str) -> Gate { if !reasons.is_empty() { return Gate::fail(reasons); } + let spec_gate = gate_spec(io, state); + if !spec_gate.ok { return spec_gate; } let spec = load_spec(&abs(io, SPEC_PATH)); + if let Some(failure) = revalidate_plates(io, state, spec.as_ref()) { return failure; } let comp_path = state.get("comp").and_then(Value::as_str).unwrap_or(""); let report = match hero_diff_labeled(io, comp_path, desktop, spec.as_ref(), out_dir, "desktop") { Ok(r) => r, @@ -1635,7 +1687,8 @@ fn gate_responsive(io: &Io, state: &Value, min: f64, out_dir: &str) -> Gate { return false; } let id = r.get("id").and_then(Value::as_str).unwrap_or(""); - let passed = state.pointer(&format!("/plates/{id}/status")).and_then(Value::as_str) == Some("ok"); + let passed = spec.as_ref().is_some_and(|spec| spec_regions(spec).iter().any(|region| + region.get("id").and_then(Value::as_str) == Some(id) && plate_receipt_current(io, state, spec, region))); let kind = r.get("kind").and_then(Value::as_str).unwrap_or(""); if (kind == "plate" || kind == "image") && passed { let present = rscore_opt(r, "detailRaw").map(|v| v >= 0.3).unwrap_or(rscore(r, "detail") >= 0.3); @@ -1680,7 +1733,7 @@ fn hero_diff_labeled(io: &Io, comp_path: &str, build_path: &str, spec: Option<&V let comp = load_raster(io, comp_path)?; let build = load_raster(io, build_path)?; let res = compare(&comp, &build, spec, "top", label, None); - let files = write_artifacts(&res, &comp, &abs(io, out_dir)); + let files = write_artifacts(&res, &comp, &abs(io, out_dir)).map_err(|e| format!("cannot persist comparison artifacts: {e}"))?; let meta = json!({ "label": label, "comp": comp_path, "build": build_path, "spec": if spec.is_some() { Value::String(SPEC_PATH.into()) } else { Value::Null }, @@ -1688,7 +1741,7 @@ fn hero_diff_labeled(io: &Io, comp_path: &str, build_path: &str, spec: Option<&V "buildSize": format!("{}x{}", build.width, build.height), }); let report = build_report(&res, Some(&files), &meta); - let _ = std::fs::write(abs(io, &format!("{out_dir}/report.json")), util::json_pretty(&report)); + atomic_report(&abs(io, &format!("{out_dir}/report.json")), &report)?; Ok(report) } @@ -1722,11 +1775,11 @@ fn force_allowed(reason: Option<&str>) -> bool { if reason.trim().chars().count() < 20 { return false; } - static NAMES_USER: Lazy = Lazy::new(|| Regex::new(r"(?i)\buser\b|\bthey (said|asked|told|chose|picked)\b|\bpaul\b").unwrap()); - static QUOTE: Lazy = Lazy::new(|| Regex::new(r#""([^"]+)"|“([^”]+)”|'([^']+)'|‘([^’]+)’"#).unwrap()); + // Match a direct attribution together with its quotation. A user mention + // elsewhere in the reason cannot authorize a different speaker's words. + static QUOTE: Lazy = Lazy::new(|| Regex::new(r#"(?i)(?:^|[.!?]\s+)(?:the\s+)?(?:user|paul)(?:\s+(?:said|says|wrote|replied|answered|confirmed|asked)\s*[:,]?|\s*:)\s*(?:"([^"]+)"|“([^”]+)”|'([^']+)'|‘([^’]+)’)"#).unwrap()); static DOWNGRADE: Lazy = Lazy::new(|| Regex::new(r"(?i)^\s*(please\s+)?(ignore|waive|relax|skip|drop|disregard) (the )?(approved )?(comp|mockup|fidelity|plate|region)\b|^\s*(the )?(comp|mockup|fidelity|plate|region)\b[^.!?;\n]{0,40}\b(is optional|is not required|does not need to match|doesn't need to match|need not match|can differ|can be skipped)\b").unwrap()); - if !NAMES_USER.is_match(reason) { return false; } - QUOTE.captures_iter(reason).any(|capture| { + QUOTE.captures_iter(reason.trim()).any(|capture| { (1..=4).filter_map(|i| capture.get(i)).any(|q| DOWNGRADE.is_match(q.as_str())) }) @@ -1906,7 +1959,8 @@ mod transparency_guidance_tests { fn missing_plate_guidance_uses_the_configured_launcher() { let dir = std::env::temp_dir().join(format!("impeccable-plate-launcher-{}", std::process::id())); std::fs::create_dir_all(dir.join(BUILD_DIR)).unwrap(); - std::fs::write(dir.join(SPEC_PATH), json!({"regions": [{"id": "art", "medium": "raster", "plate": "missing.png"}]}).to_string()).unwrap(); + std::fs::write(dir.join(SPEC_PATH), json!({"comp":"comp.png","regions": [{"id": "art", "medium": "raster", "plate": "missing.png"}]}).to_string()).unwrap(); + std::fs::write(dir.join("comp.png"), png_io::encode_png(&r::create_image(8,8,[255,255,255,255]), &[]).unwrap()).unwrap(); let env = [("IMPECCABLE_SELF".into(), "/custom/impeccable".into())].into(); let (io, _) = Io::captured("", dir.clone(), env); let reasons = gate_plates(&io).reasons.join("\n"); diff --git a/crates/comp-verbs/src/build_phase/integrity_tests.rs b/crates/comp-verbs/src/build_phase/integrity_tests.rs index 323107d86..3e7743ca5 100644 --- a/crates/comp-verbs/src/build_phase/integrity_tests.rs +++ b/crates/comp-verbs/src/build_phase/integrity_tests.rs @@ -267,3 +267,145 @@ fn preflight_failure_replaces_stale_success_report() { assert_eq!(report["regions"], json!([])); assert_eq!(report["gate"]["reasons"], json!(gate.reasons)); } + +#[test] +fn unrelated_user_mention_cannot_authorize_another_speakers_quote() { + for reason in [ + "The user requested dark mode. The designer said \"ignore the comp fidelity requirement.\"", + "The user asked to proceed. I will \"ignore the comp fidelity requirement\"", + "The designer said the user said \"ignore the comp fidelity requirement\"", + "The user said \"Keep the comp.\" The designer said \"Ignore the comp.\"", + ] { + assert!(!force_allowed(Some(reason)), "{reason}"); + } + for reason in [ + "The user said \"Ignore the comp fidelity requirement.\"", + "User: ‘Please waive the comp requirement.’", + "Paul wrote: “The comp is optional.”", + ] { + assert!(force_allowed(Some(reason)), "{reason}"); + } +} + +fn simple_hero_workspace() -> (Workspace, Value) { + let ws = Workspace::new(); + let comp = r::create_image(100, 100, [150, 70, 30, 255]); + ws.write("comp.png", &png_io::encode_png(&comp, &[]).unwrap()); + ws.write("index.html", b"
"); + ws.write( + SPEC_PATH, + util::json_pretty(&json!({"comp":"comp.png","regions":[{ + "id":"button","kind":"control","medium":"code","box":{"x":0,"y":0,"w":1,"h":1}, + "px":{"x":0,"y":0,"w":100,"h":100}}]})) + .as_bytes(), + ); + (ws, json!({"comp":"comp.png","phases":{"hero":{}}})) +} + +#[test] +fn failed_evidence_writes_cannot_publish_success() { + for blocked_file in ["regions/button.png", "raw-report.json"] { + let (ws, mut state) = simple_hero_workspace(); + let g = gate_hero( + &ws.io(), + &mut state, + "comp.png", + HERO_MIN, + "diff", + Some("index.html"), + &no_organic_scan, + ); + assert!(g.ok, "fixture: {:?}", g.reasons); + let blocked = ws.path.join("diff").join(blocked_file); + std::fs::remove_file(&blocked).unwrap(); + std::fs::create_dir(&blocked).unwrap(); + let g = gate_hero( + &ws.io(), + &mut state, + "comp.png", + HERO_MIN, + "diff", + Some("index.html"), + &no_organic_scan, + ); + assert!(!g.ok, "write failure must block: {blocked_file}"); + let report: Value = + serde_json::from_slice(&std::fs::read(ws.path.join("diff/report.json")).unwrap()) + .unwrap(); + assert_eq!(report["gate"]["ok"], false); + assert_eq!(report["measurementsAvailable"], false); + assert!(report["gate"]["reasons"] + .as_array() + .unwrap() + .iter() + .any(|r| r.as_str().unwrap().contains("persist"))); + } +} + +#[test] +fn missing_comp_cannot_approve_plates() { + let ws = Workspace::new(); + let art = r::create_image(100, 100, [140, 60, 20, 255]); + ws.write("art.png", &png_io::encode_png(&art, &[]).unwrap()); + ws.write(SPEC_PATH, util::json_pretty(&json!({"comp":"missing.png","regions":[{ + "id":"art","kind":"plate","medium":"raster","plate":"art.png","px":{"x":0,"y":0,"w":10,"h":10}}]})).as_bytes()); + let g = gate_plates(&ws.io()); + assert!(!g.ok); + assert!(g.reasons.iter().any(|r| r.contains("comp"))); +} + +#[test] +fn responsive_revalidates_legacy_or_changed_plate_receipts() { + let (ws, mut state) = simple_hero_workspace(); + let bytes = std::fs::read(ws.path.join("comp.png")).unwrap(); + ws.write(".impeccable/review/desktop.png", &bytes); + ws.write(".impeccable/review/mobile.png", &bytes); + let region = json!({"id":"art","kind":"plate","medium":"raster","plate":"removed.png", + "box":{"x":0,"y":0,"w":1,"h":1},"px":{"x":0,"y":0,"w":100,"h":100}}); + ws.write( + SPEC_PATH, + util::json_pretty(&json!({"comp":"comp.png","regions":[region]})).as_bytes(), + ); + state["plates"] = json!({"art":{"status":"ok","score":0.9}}); + let g = gate_responsive(&ws.io(), &mut state, RESPONSIVE_MIN, "diff"); + assert!(!g.ok, "a missing asset cannot inherit legacy approval"); + assert!( + g.reasons.iter().any(|r| r.contains("plate missing")), + "{:?}", + g.reasons + ); +} + +#[test] +fn repair_crops_follow_blockers_not_the_lowest_raw_score() { + let regions = vec![ + json!({"id":"advisory-art","score":{"overall":0.3}}), + json!({"id":"blocking-control","score":{"overall":0.6}}), + ]; + let mut blockers = Map::new(); + record_region_reason(&mut blockers, "blocking-control", "control still differs"); + let repairs = repair_regions(®ions, &blockers); + assert_eq!(repairs.len(), 1); + assert_eq!(repairs[0]["id"], "blocking-control"); + assert!( + repair_regions(®ions, &Map::new()).is_empty(), + "global blockers do not justify guessing which asset to regenerate" + ); +} + +#[test] +fn folded_readings_keep_all_region_ids_without_becoming_unscoped() { + let mut reasons = vec![]; + let mut bindings = Map::new(); + let message = "text title-1: cap height differs (also title-2)"; + let ids = [( + message.to_string(), + vec!["title-1".into(), "title-2".into()], + )] + .into(); + push_reading_blocker(&mut reasons, &mut bindings, &ids, message); + assert_eq!(reasons, vec![message]); + for id in ["title-1", "title-2"] { + assert_eq!(bindings[id], json!([message])); + } +} diff --git a/crates/comp-verbs/src/comp_diff.rs b/crates/comp-verbs/src/comp_diff.rs index 93b704e6a..8cccba3d2 100644 --- a/crates/comp-verbs/src/comp_diff.rs +++ b/crates/comp-verbs/src/comp_diff.rs @@ -285,8 +285,12 @@ pub fn resolve_regions(comp: &Image, spec: Option<&Value>) -> Vec { /// JS: regionCrop(img, r). fn region_crop(img: &Image, rr: &RegionBox) -> Image { // Sampling support must not add neighbouring elements to a declared region. - r::crop(img, rr.x * img.width as f64, rr.y * img.height as f64, - rr.w * img.width as f64, rr.h * img.height as f64) + let bounds = r::clamp_rect(img, rr.x * img.width as f64, rr.y * img.height as f64, + rr.w * img.width as f64, rr.h * img.height as f64); + // A subpixel box still samples an actual pixel, including at the far edge. + r::crop(img, bounds.x.min(img.width.saturating_sub(1)) as f64, + bounds.y.min(img.height.saturating_sub(1)) as f64, + bounds.w.max(1) as f64, bounds.h.max(1) as f64) } fn ink_box_json(b: &Option) -> Value { @@ -538,31 +542,32 @@ fn write_png(path: &Path, img: &Image) -> Result<(), String> { } /// JS: writeArtifacts(result, comp, outDir). -pub fn write_artifacts(result: &CompareResult, comp: &Image, out_dir: &Path) -> Value { - let _ = std::fs::create_dir_all(out_dir.join("regions")); +pub fn write_artifacts(result: &CompareResult, comp: &Image, out_dir: &Path) -> Result { + std::fs::create_dir_all(out_dir.join("regions")).map_err(|e| e.to_string())?; let side = render_side_by_side(comp, &result.aligned, &result.label, &result.whole); let side_path = out_dir.join("side-by-side.png"); - let _ = write_png(&side_path, &side); + write_png(&side_path, &side)?; let heat_path = out_dir.join("heatmap.png"); - let _ = write_png(&heat_path, &render_heatmap(comp, &result.aligned)); - let region_files = write_region_artifacts(result, out_dir, None); - json!({ + write_png(&heat_path, &render_heatmap(comp, &result.aligned))?; + let region_files = write_region_artifacts(result, out_dir, None)?; + Ok(json!({ "sideBySide": path_str(&side_path), "heatmap": path_str(&heat_path), "regionFiles": region_files, - }) + })) } /// Refresh labels after gate interpretation, without rerunning measurements. -pub fn write_region_artifacts(result: &CompareResult, out_dir: &Path, gate_regions: Option<&[Value]>) -> Vec { +pub fn write_region_artifacts(result: &CompareResult, out_dir: &Path, gate_regions: Option<&[Value]>) -> Result, String> { let mut region_files: Vec = Vec::new(); for rg in &result.regions { let file = out_dir.join("regions").join(format!("{}.png", rg.id)); let gate_region = gate_regions.and_then(|regions| regions.iter().find(|r| r.get("id").and_then(Value::as_str) == Some(rg.id.as_str()))); - let _ = write_png(&file, &render_region_pair(&rg.a, &rg.b, &rg.id, &rg.score, &rg.verdict, gate_region)); + write_png(&file, &render_region_pair(&rg.a, &rg.b, &rg.id, &rg.score, &rg.verdict, gate_region)) + .map_err(|e| format!("{}: {e}", file.display()))?; region_files.push(json!(path_str(&file))); } - region_files + Ok(region_files) } fn path_str(p: &Path) -> String { @@ -740,7 +745,10 @@ pub fn run(argv: &[String], io: &mut Io) -> i32 { let files = if flag(argv, "no-files") { None } else { - Some(write_artifacts(&result, &comp, &resolve(io, &out_dir))) + match write_artifacts(&result, &comp, &resolve(io, &out_dir)) { + Ok(files) => Some(files), + Err(e) => { io.err(&format!("comp-diff: cannot persist comparison artifacts: {e}\n")); return 1; } + } }; let meta = json!({ "label": label, @@ -753,7 +761,9 @@ pub fn run(argv: &[String], io: &mut Io) -> i32 { let report = build_report(&result, files.as_ref(), &meta); if files.is_some() { let rp = resolve(io, &out_dir).join("report.json"); - let _ = std::fs::write(&rp, util::json_pretty(&report)); + if let Err(e) = std::fs::write(&rp, util::json_pretty(&report)) { + io.err(&format!("comp-diff: cannot persist report: {e}\n")); return 1; + } } if flag(argv, "json") { io.out(&format!("{}\n", util::json_pretty(&report))); @@ -779,6 +789,19 @@ pub fn run(argv: &[String], io: &mut Io) -> i32 { mod region_isolation_regression { use super::*; #[test] + fn subpixel_regions_sample_real_pixels() { + let a = r::create_image(10, 10, [220, 10, 20, 255]); + let b = r::create_image(10, 10, [10, 30, 210, 255]); + for (x, y) in [(0.2, 0.2), (0.99, 0.99)] { + let rr = RegionBox { id: "tiny".into(), x, y, w:0.001, h:0.001, kind:None }; + let ac = region_crop(&a, &rr); + let bc = region_crop(&b, &rr); + assert_eq!(&ac.data[..4], &[220,10,20,255]); + assert_eq!(&bc.data[..4], &[10,30,210,255]); + assert_ne!(ac.data, bc.data); + } + } + #[test] fn small_region_does_not_sample_neighbours() { let a = r::create_image(256, 128, [240, 220, 190, 255]); let mut b = a.clone(); diff --git a/docs/COMP-GATE-INTEGRITY.md b/docs/COMP-GATE-INTEGRITY.md index b537bc7e9..cf54bea06 100644 --- a/docs/COMP-GATE-INTEGRITY.md +++ b/docs/COMP-GATE-INTEGRITY.md @@ -7,7 +7,8 @@ existing plate file does not satisfy a missing-region check. ## Measurements and decisions `comp-diff` measures the declared region bounds. It does not enlarge a narrow -region to include neighbouring elements. Coverage and region-kind checks remain +region to include neighbouring elements. Subpixel regions sample at least one +real source pixel, including at image edges. Coverage and region-kind checks remain the responsibility of `comp-spec`; this change does not authorize omitting regions or shrinking their bounds to exclude required work. @@ -31,6 +32,12 @@ When capture, spec, or plate validation fails before measurement, the current report has `measurementsAvailable: false`, no region results, and the failed gate reasons. It does not present the preceding capture as current evidence. +Repair crops include only regions with current blockers; shared readings retain +all affected region IDs. Frame-wide blockers keep their whole-frame evidence. +The current report is marked incomplete before image writes and committed +atomically only after the evidence files succeed. Write failures block the gate +and, when the report location is writable, publish an unavailable-evidence report. + Stall feedback follows repeated blocking reasons. It never chooses an asset to regenerate solely because that asset has the lowest raw score. The feedback is additional context; it does not clear a finding or advance the phase. @@ -38,7 +45,7 @@ additional context; it does not clear a finding or advance the phase. ## Plate validation Successful plate receipts include SHA-256 fingerprints of the asset bytes, -measured region, and comp. A changed or deleted file, a changed region, or a +measured region, and comp. Hero and responsive gates revalidate receipts. A changed or deleted file, a changed region, or a changed comp invalidates the receipt. Legacy score-only receipts are revalidated. An invalid plate cannot receive an `ok` receipt merely because its PNG decoded. Rendered presence is still checked after asset validation, so a file hidden in @@ -47,7 +54,8 @@ the page does not count as placed. ## Overrides A `--force --reason` must contain a direct quoted downgrade of comp authority -attributed to the user. Generic delegation, the builder's surrounding claim that +immediately attributed to the user. An unrelated mention of the user does not +authorize a quote from another speaker. Generic delegation, the builder's surrounding claim that it may proceed, or a gate exception does not establish that authorization. The quote parser is deliberately conservative. It cannot authenticate a quote: the calling harness must retain the actual user answer and assess provenance.