From 416426e6ca1c2c9a5d836b6293e2daace303c2ba Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Wed, 16 Sep 2026 12:54:35 -0700 Subject: [PATCH] Fix: honor reviewed text styling at native finish Preserve original comp scores and integrity gates; bind the additional text comparison to the private approved assembly and unchanged inputs. AI assistance: implemented and validated with OpenAI Codex. --- crates/cli/src/capture_service.rs | 17 +- crates/cli/src/lib.rs | 2 + crates/cli/src/main.rs | 2 +- crates/cli/src/reviewed_entry.rs | 328 +++++++++++++++++++++++++ crates/comp-verbs/src/build_phase.rs | 12 + crates/comp-verbs/src/entry_capture.rs | 2 + docs/CLI-CONTRACT.md | 6 + 7 files changed, 365 insertions(+), 4 deletions(-) create mode 100644 crates/cli/src/reviewed_entry.rs diff --git a/crates/cli/src/capture_service.rs b/crates/cli/src/capture_service.rs index d9ac5ac48..176cb9d2a 100644 --- a/crates/cli/src/capture_service.rs +++ b/crates/cli/src/capture_service.rs @@ -1,6 +1,6 @@ //! Project-scoped capture transport. Approval still belongs to the shared gate. //! Host adapters must independently audit retained evidence before accepting a run. -use crate::entry_capture::CdpEntryRenderer; +use crate::reviewed_entry::ReviewedEntryRenderer; use base64::Engine; use impeccable_comp_verbs::entry_capture::{ CapturedEntry, EntryRenderer, EntryRequest, EntryStage, @@ -98,6 +98,7 @@ pub fn serve(args: &[String]) -> Result<(), Box> { if key.len() < 32 { return Err("capability too short".into()); } + let renderer=ReviewedEntryRenderer{session:std::env::var_os("IMPECCABLE_CAPTURE_REVIEW_SESSION").map(PathBuf::from)}; let listener = TcpListener::bind("127.0.0.1:0")?; listener.set_nonblocking(true)?; std::fs::write( @@ -141,7 +142,7 @@ pub fn serve(args: &[String]) -> Result<(), Box> { "responsive" => EntryStage::Responsive, _ => return Err("invalid stage".into()), }; - let captured = CdpEntryRenderer.capture_entry(&EntryRequest { + let captured = renderer.capture_entry(&EntryRequest { root: PathBuf::from(&root), artifact: text("entry")?.into(), spec: text("spec")?.into(), @@ -155,7 +156,7 @@ pub fn serve(args: &[String]) -> Result<(), Box> { let evidence = captured.evidence(); let frames:Vec<_>=evidence.frames.iter().map(|f|json!({"name":f.name,"png":base64::engine::general_purpose::STANDARD.encode(&f.png),"regions":f.regions.iter().map(|r|r.receipt.clone()).collect::>()})).collect(); let response = - json!({"ok":true,"handle":handle,"report":evidence.report,"frames":frames}); + json!({"ok":true,"handle":handle,"report":evidence.report,"frames":frames,"approvedReference":captured.approved_reference().map(|a|json!({"png":base64::engine::general_purpose::STANDARD.encode(&a.png),"proof":a.proof}))}); if serde_json::to_vec(&response) .map_err(|e| e.to_string())? .len() @@ -246,6 +247,7 @@ impl ServiceEntry { } } impl CapturedEntry for ServiceEntry { + fn approved_reference(&self)->Option<&impeccable_comp_verbs::entry_capture::ApprovedReference>{self.source.approved_reference()} fn evidence(&self) -> &impeccable_comp_verbs::entry_capture::EntryEvidence { &self.evidence } @@ -301,6 +303,7 @@ fn audit_saved( if report != evidence.report { return Err("saved capture report differs from host evidence".into()); } + if let Some(approved)=capture.approved_reference(){if read("human-approved.png")?!=approved.png{return Err("saved human reference differs from reviewed capture".into())}} for frame in &evidence.frames { if read(&format!("{}.png", frame.name))? != frame.png { return Err("saved frame differs from host capture".into()); @@ -385,10 +388,12 @@ impl RemoteEntryRenderer { } struct RemoteEntry { renderer: RemoteEntryRenderer, + approved: Option, id: String, evidence: impeccable_comp_verbs::entry_capture::EntryEvidence, } impl CapturedEntry for RemoteEntry { + fn approved_reference(&self)->Option<&impeccable_comp_verbs::entry_capture::ApprovedReference>{self.approved.as_ref()} fn evidence(&self) -> &impeccable_comp_verbs::entry_capture::EntryEvidence { &self.evidence } @@ -467,6 +472,12 @@ impl EntryRenderer for RemoteEntryRenderer { }) .collect::, _>>()?; Ok(Box::new(RemoteEntry { + approved: if result["approvedReference"].is_null(){None}else{ + let a=&result["approvedReference"]; + let png=base64::engine::general_purpose::STANDARD.decode(a["png"].as_str().ok_or("missing reviewed PNG")?).map_err(|e|e.to_string())?; + if a["proof"]["schema"]!="human-assembled-reference-v1" || a["proof"]["sha256"]!=impeccable_comp_verbs::asset_capture::capture_sha256(&png) || a["proof"]!=result["report"]["humanTextReview"] {return Err("invalid human reference proof".into())} + Some(impeccable_comp_verbs::entry_capture::ApprovedReference{png,proof:a["proof"].clone()}) + }, renderer: self.clone(), id: id.clone(), evidence: EntryEvidence { diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 9c77a2bcd..357f4502d 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -6,3 +6,5 @@ pub mod capture_snapshot; pub mod entry_capture; pub mod capture_service; + +pub mod reviewed_entry; diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 5a3b3f11c..ad61cba66 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -89,7 +89,7 @@ fn run(args: &[String], io: &mut Io) -> i32 { }; match impeccable::capture_service::RemoteEntryRenderer::from_env(&io.env) { Ok(Some(renderer)) => impeccable_comp_verbs::build_phase::run_with_renderer(rest, io, &organic, Some(&renderer)), - Ok(None) => impeccable_comp_verbs::build_phase::run_with_renderer(rest, io, &organic, Some(&impeccable::entry_capture::CdpEntryRenderer)), + Ok(None) => { let renderer=impeccable::reviewed_entry::ReviewedEntryRenderer::local(&io.cwd,io.home().as_deref()); impeccable_comp_verbs::build_phase::run_with_renderer(rest, io, &organic, Some(&renderer)) }, Err(e) => { io.err(&format!("Native capture service: {e}\n")); 1 } } } diff --git a/crates/cli/src/reviewed_entry.rs b/crates/cli/src/reviewed_entry.rs new file mode 100644 index 000000000..da9ffe747 --- /dev/null +++ b/crates/cli/src/reviewed_entry.rs @@ -0,0 +1,328 @@ +//! A human-approved assembled capture is evidence for text styling, never a +//! replacement for native integrity, missing-region, or overall fidelity gates. +use crate::entry_capture::CdpEntryRenderer; +use impeccable_comp_verbs::asset_capture::capture_sha256; +use impeccable_comp_verbs::entry_capture::{ + ApprovedReference, CapturedEntry, EntryEvidence, EntryRenderer, EntryRequest, +}; +use serde_json::{json, Value}; +use std::{ + fs, + path::{Path, PathBuf}, +}; + +pub struct ReviewedEntryRenderer { + pub session: Option, +} +struct ReviewedEntry { + source: Box, + evidence: EntryEvidence, + approved: Option, + session: Option, + request: EntryRequest, +} +fn read(path: &Path) -> Result { + serde_json::from_slice(&fs::read(path).map_err(|e| e.to_string())?).map_err(|e| e.to_string()) +} +fn file(root: &Path, path: &str) -> Result, String> { + let mut full = root.to_path_buf(); + for c in Path::new(path).components() { + let std::path::Component::Normal(c) = c else { + return Err("invalid reviewed path".into()); + }; + full.push(c); + if fs::symlink_metadata(&full) + .map_err(|e| e.to_string())? + .file_type() + .is_symlink() + { + return Err("symlink in reviewed source".into()); + } + } + fs::read(full).map_err(|e| e.to_string()) +} +/// Read only private native session state. The host selects the session; neither +/// a model-written receipt nor a saved build-phase report is an authority. +fn reference( + session: &Path, + r: &EntryRequest, + evidence: &EntryEvidence, +) -> Result { + let root = r.root.canonicalize().map_err(|e| e.to_string())?; + if session + .canonicalize() + .map_err(|e| e.to_string())? + .starts_with(&root) + { + return Err("review session must be outside the project".into()); + } + let state = read(&session.join("current.json"))?; + let packet = &state["packet"]; + let receipt = &state["receipt"]; + if packet["stage"] != "hero" + || receipt["visualDecision"] != "approved" + || receipt["captureVerified"] != true + || receipt["capture"] != state["capture"] + || state["capture"]["schema"] != "native-component-previews-v1" + || receipt["submission"]["packetRevision"] != packet["revision"] + || receipt["submission"]["requestId"] != packet["id"] + { + return Err("assembled review is not approved".into()); + } + let components = packet["components"] + .as_array() + .filter(|c| c.len() == 1) + .ok_or("expected one assembled page")?; + let component = &components[0]; + if component["box"] != json!({"x":0,"y":0,"w":1,"h":1}) + || component["preview"]["sourceKind"] != "page" + { + return Err("review did not cover the assembled viewport".into()); + } + let capture = &state["capture"]["components"][0]["views"]["preview"]; + if capture["kind"] != "assembled-page" + || capture["entry"] != r.artifact + || capture["viewport"]["dpr"] != 1 + || capture["viewport"]["width"] != packet["comp"]["width"] + || capture["viewport"]["height"] != packet["comp"]["height"] + { + return Err("reviewed viewport or entry differs".into()); + } + let sources = state["sources"] + .as_object() + .ok_or("missing review sources")?; + for (path, hash) in sources { + if hash.as_str() != Some(&capture_sha256(&file(&root, path)?)) { + return Err(format!("reviewed source changed: {path}")); + } + } + if !sources.contains_key(&r.artifact) || !sources.contains_key(&r.reference) { + return Err("review must bind entry and original comp".into()); + } + let comp_url = packet["comp"]["url"] + .as_str() + .ok_or("missing reviewed comp")?; + if comp_url + != format!( + "/files/{}/{}", + packet["revision"].as_str().ok_or("missing revision")?, + r.reference + ) + { + return Err("reviewed reference differs".into()); + } + // A newly added served file must not borrow an earlier approval. + for input in evidence.report["manifest"]["files"] + .as_array() + .ok_or("missing current native inputs")? + { + let path = input["path"].as_str().ok_or("invalid input")?; + let raster = matches!( + Path::new(path).extension().and_then(|s| s.to_str()), + Some("png" | "jpg" | "jpeg" | "webp" | "avif" | "gif") + ); + if input["served"] == true + && !raster + && sources.get(input["path"].as_str().ok_or("invalid input")?) != Some(&input["sha256"]) + { + return Err("native inputs differ from reviewed inputs".into()); + } + } + // Available but unused raster variants are not dependencies. Every raster + // actually observed at the captured breakpoint must belong to the approval. + for frame in &evidence.frames { + for region in &frame.regions { + for binding in region.receipt["resourceBindings"] + .as_array() + .ok_or("missing observed raster bindings")? + { + if !sources + .values() + .any(|h| h == &binding["responseSha256"] && h.is_string()) + { + return Err("unreviewed raster contributes to current page".into()); + } + } + } + } + let hash = capture["screenshotSha256"] + .as_str() + .filter(|h| h.len() == 64 && h.bytes().all(|c| c.is_ascii_hexdigit())) + .ok_or("missing reviewed screenshot digest")?; + let preview = component["preview"]["url"] + .as_str() + .ok_or("missing preview")?; + let relative = preview + .strip_prefix(&format!("/files/{}/", packet["revision"].as_str().unwrap())) + .ok_or("preview revision differs")?; + if state["files"][relative] != hash { + return Err("reviewed preview digest differs".into()); + } + let png = fs::read(session.join("blobs").join(hash)).map_err(|e| e.to_string())?; + if capture_sha256(&png) != hash { + return Err("reviewed screenshot changed".into()); + } + Ok(ApprovedReference { + png, + proof: json!({"schema":"human-assembled-reference-v1","requestId":packet["id"],"packetRevision":packet["revision"],"sha256":hash,"scope":"Text styling accepted in a source-bound assembled-page review; all other gates retained"}), + }) +} +impl EntryRenderer for ReviewedEntryRenderer { + fn capture_entry(&self, r: &EntryRequest) -> Result, String> { + let source = CdpEntryRenderer.capture_entry(r)?; + let candidate = self + .session + .as_ref() + .map(|s| reference(s, r, source.evidence())); + let mut report = source.evidence().report.clone(); + if let Some(Err(reason)) = &candidate { + report["humanTextReview"] = json!({"status":"not-current","reason":reason}); + } + let approved = candidate.and_then(Result::ok); + if let Some(a) = &approved { + report["humanTextReview"] = a.proof.clone(); + } + // Frames are produced once by the native adapter; copy bytes for the transport. + let frames = source + .evidence() + .frames + .iter() + .map(|f| impeccable_comp_verbs::entry_capture::FrameEvidence { + name: f.name.clone(), + png: f.png.clone(), + regions: f + .regions + .iter() + .map(|r| impeccable_comp_verbs::asset_capture::AssetCapture { + receipt: r.receipt.clone(), + images: vec![], + }) + .collect(), + }) + .collect(); + Ok(Box::new(ReviewedEntry { + source, + evidence: EntryEvidence { report, frames }, + approved, + session: self.session.clone(), + request: EntryRequest { + root: r.root.clone(), + artifact: r.artifact.clone(), + spec: r.spec.clone(), + reference: r.reference.clone(), + stage: r.stage, + }, + })) + } +} +impl CapturedEntry for ReviewedEntry { + fn evidence(&self) -> &EntryEvidence { + &self.evidence + } + fn approved_reference(&self) -> Option<&ApprovedReference> { + self.approved.as_ref() + } + fn verify_current(&self) -> Result<(), String> { + self.source.verify_current()?; + if let Some(a) = &self.approved { + let current = reference( + self.session.as_ref().unwrap(), + &self.request, + self.source.evidence(), + )?; + if current.png != a.png || current.proof != a.proof { + return Err("human review changed during capture".into()); + } + } + Ok(()) + } +} +impl ReviewedEntryRenderer { + pub fn local(root: &Path, home: Option<&Path>) -> Self { + let session = (|| { + let manifest = read(&root.join(".impeccable/review/hero.json")).ok()?; + let project = root.canonicalize().ok()?; + let id = manifest["id"].as_str()?; + // Same project/id key as the shared component-review store. + let key = capture_sha256(format!("{}\0{}", project.display(), id).as_bytes()); + Some(home?.join(".impeccable/component-reviews").join(key)) + })(); + Self { session } + } +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn approval_is_bound_to_reviewed_sources_and_the_native_capture() { + let dir = std::env::temp_dir().join(format!("review-reference-{}", std::process::id())); + let root = dir.join("project"); + let session = dir.join("session"); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&root).unwrap(); + fs::create_dir_all(session.join("blobs")).unwrap(); + fs::write(root.join("index.html"), b"page").unwrap(); + fs::write(root.join("comp.png"), b"comp").unwrap(); + let page = capture_sha256(b"page"); + let comp = capture_sha256(b"comp"); + let png = capture_sha256(b"png"); + fs::write(session.join("blobs").join(&png), b"png").unwrap(); + let capture = json!({"schema":"native-component-previews-v1","components":[{"views":{"preview":{"kind":"assembled-page","entry":"index.html","viewport":{"width":1536,"height":1024,"dpr":1},"screenshotSha256":png}}}]}); + let state = json!({"sources":{"index.html":page,"comp.png":comp},"files":{"preview.png":png},"capture":capture,"packet":{"stage":"hero","id":"hero","revision":"rev","comp":{"width":1536,"height":1024,"url":"/files/rev/comp.png"},"components":[{"box":{"x":0,"y":0,"w":1,"h":1},"preview":{"sourceKind":"page","url":"/files/rev/preview.png"}}]},"receipt":{"visualDecision":"approved","captureVerified":true,"capture":capture,"submission":{"requestId":"hero","packetRevision":"rev"}}}); + let save = |s: &Value| { + fs::write(session.join("current.json"), serde_json::to_vec(s).unwrap()).unwrap() + }; + save(&state); + let request = EntryRequest { + root: root.clone(), + artifact: "index.html".into(), + spec: "spec.json".into(), + reference: "comp.png".into(), + stage: impeccable_comp_verbs::entry_capture::EntryStage::Responsive, + }; + let evidence = EntryEvidence { + report: json!({"manifest":{"files":[{"path":"index.html","sha256":page,"served":true}]}}), + frames: vec![], + }; + assert_eq!( + reference(&session, &request, &evidence).unwrap().png, + b"png" + ); + for (pointer, value) in [ + ("/receipt/visualDecision", json!("changes-requested")), + ("/receipt/submission/packetRevision", json!("old")), + ("/packet/stage", json!("components")), + ("/packet/components/0/box/w", json!(0.5)), + ("/packet/comp/url", json!("/files/rev/other.png")), + ( + "/capture/components/0/views/preview/viewport/width", + json!(1440), + ), + ] { + let mut broken = state.clone(); + *broken.pointer_mut(pointer).unwrap() = value; + save(&broken); + assert!( + reference(&session, &request, &evidence).is_err(), + "{pointer}" + ); + } + save(&state); + fs::write(root.join("index.html"), b"changed").unwrap(); + assert!(reference(&session, &request, &evidence).is_err()); + fs::write(root.join("index.html"), b"page").unwrap(); + let mut added = EntryEvidence { + report: evidence.report.clone(), + frames: vec![], + }; + added.report["manifest"]["files"] + .as_array_mut() + .unwrap() + .push(json!({"path":"late.css","sha256":"new","served":true})); + assert!(reference(&session, &request, &added).is_err()); + fs::write(session.join("blobs").join(&png), b"replaced").unwrap(); + assert!(reference(&session, &request, &evidence).is_err()); + let _ = fs::remove_dir_all(dir); + } +} diff --git a/crates/comp-verbs/src/build_phase.rs b/crates/comp-verbs/src/build_phase.rs index 01bb12a82..a3632d1d8 100644 --- a/crates/comp-verbs/src/build_phase.rs +++ b/crates/comp-verbs/src/build_phase.rs @@ -1163,6 +1163,7 @@ fn prepare_native_capture( ); let save = (|| -> Result<(), String> { std::fs::create_dir_all(abs(io, &directory)).map_err(|e| e.to_string())?; + if let Some(approved)=capture.approved_reference(){std::fs::write(abs(io,&format!("{directory}/human-approved.png")),&approved.png).map_err(|e|e.to_string())?;} for frame in &capture.evidence().frames { if !matches!(frame.name.as_str(), "hero" | "desktop" | "mobile") { return Err("unknown native capture frame".into()); @@ -2139,6 +2140,12 @@ fn gate_responsive_inner(io: &Io, state: &mut Value, min: f64, out_dir: &str, na true }) .cloned().collect(); + // Keep the original scores and all missing/integrity/overall blockers. + // A human-approved assembly can qualify a text-style contradiction only + // when that region still matches the approved rendering at desktop width. + let accepted_text = native.and_then(|n| n.capture.approved_reference().map(|a|(n,a))) + .and_then(|(n,a)| hero_diff_labeled(io,&n.path("human-approved"),desktop,spec.as_ref(),&format!("{out_dir}/human-reviewed"),"human-reviewed").ok().map(|(r,_)|(r,a.proof.clone()))); + if let Some((comparison,proof))=&accepted_text {report["humanTextReview"]=json!({"proof":proof,"comparison":comparison});} let contradicted_direction: Vec = regions.iter().filter(|r| r.get("verdict").and_then(Value::as_str) == Some("contradicted") && matches!(r.get("kind").and_then(Value::as_str), Some("text" | "control"))).cloned().collect(); for region in &mut regions { if region.get("verdict").and_then(Value::as_str) == Some("missing") @@ -2165,6 +2172,11 @@ fn gate_responsive_inner(io: &Io, state: &mut Value, min: f64, out_dir: &str, na push_region_blocker(&mut reasons, &mut region_reasons, id, format!("at desktop width, region {id} is missing")); } for r in &contradicted_direction { + let accepted = r["kind"]=="text" && accepted_text.as_ref().is_some_and(|(comparison,_)| comparison["regions"].as_array().is_some_and(|reviewed| reviewed.iter().any(|region|region["id"]==r["id"] && matches!(region["verdict"].as_str(),Some("match"|"drift")) && rscore(region,"structure")>=0.75))); + if accepted { + report["humanTextReview"]["acceptedTextRegions"].as_array_mut().map(|v|v.push(r["id"].clone())).unwrap_or_else(||{report["humanTextReview"]["acceptedTextRegions"]=json!([r["id"].clone()]);}); + continue; + } push_region_blocker(&mut reasons, &mut region_reasons, r.get("id").and_then(Value::as_str).unwrap_or(""), format!( "at desktop width, region {} ({}) is contradicted (structure {}%)", r.get("id").and_then(Value::as_str).unwrap_or(""), diff --git a/crates/comp-verbs/src/entry_capture.rs b/crates/comp-verbs/src/entry_capture.rs index 0bd1f82c7..832777fb0 100644 --- a/crates/comp-verbs/src/entry_capture.rs +++ b/crates/comp-verbs/src/entry_capture.rs @@ -24,7 +24,9 @@ pub struct EntryEvidence { pub report: Value, pub frames: Vec, } +pub struct ApprovedReference { pub png: Vec, pub proof: Value } pub trait CapturedEntry { + fn approved_reference(&self) -> Option<&ApprovedReference> { None } fn evidence(&self) -> &EntryEvidence; /// Recheck original bytes while this in-process capture still owns its snapshot. fn verify_current(&self) -> Result<(), String>; diff --git a/docs/CLI-CONTRACT.md b/docs/CLI-CONTRACT.md index c94206904..a87b4ba9f 100644 --- a/docs/CLI-CONTRACT.md +++ b/docs/CLI-CONTRACT.md @@ -1862,3 +1862,9 @@ The packet and evidence contract is documented in ### Component review verification `impeccable component-review verify --manifest ` reads the current native capture and user receipt. It succeeds only for an approved, natively captured round whose manifest and dependency bytes are unchanged. Pending reviews, requested repairs, changed files, and an unrelated manifest sharing the same id fail. It neither creates nor submits approvals. + +### Human-reviewed text at the final comp gate + +For native comp-led runs, a verified assembled-page review can resolve a text-style contradiction against the original comp. The renderer reads the private native review session, binds the approved screenshot and source bytes, and compares the disputed text region against that human-approved rendering at the current desktop breakpoint. The original comp scores remain in the report; accepted region IDs and the review revision are recorded separately under `humanTextReview`. + +This does not waive missing regions, overall comp fidelity, controls, raster production/placement, source integrity, or native capture checks. Changed reviewed sources, an unreviewed current dependency, a stale receipt, a partial component capture, or a changed approved image cannot provide this evidence. A host capture service may select its private native review session with `IMPECCABLE_CAPTURE_REVIEW_SESSION` at service startup; this is not a caller-supplied capture parameter. Standalone native builds use the matching local component-review session.