Fix: retain visual approvals for unchanged component captures

Keep source identity and integrity validation separate from submitted visual decisions. Reuse approval only for verified identical views and scope, preserve review provenance, and support refreshing an existing unsubmitted draft.

AI assistance: implemented and tested with Codex at the maintainer’s request.
This commit is contained in:
Paul Bakaus
2026-09-16 13:56:24 -07:00
parent 416426e6ca
commit 8797054096
6 changed files with 296 additions and 13 deletions
@@ -72,7 +72,10 @@ pub fn between(previous: &Value, current: &Value) -> Value {
.filter(|p| before.get(*p) != after.get(*p))
.cloned()
.collect();
let unchanged = prior["revision"] == component["revision"];
let source_changed = prior["revision"] != component["revision"];
let unchanged = !source_changed || (current["visualApprovalCarry"][id]["basis"] == "identical-native-captures-v1"
&& current["visualApprovalCarry"][id]["fromPacketRevision"] == previous["packet"]["revision"]);
let mut reasons = Vec::new();
if !files.is_empty() {
reasons.push("files");
@@ -83,7 +86,7 @@ pub fn between(previous: &Value, current: &Value) -> Value {
if !unchanged && reasons.is_empty() {
reasons.push("definition");
}
json!({"kind":if unchanged{"unchanged"}else{"changed"},"files":files,"reasons":reasons})
json!({"kind":if unchanged{"unchanged"}else{"changed"},"files":files,"reasons":reasons,"sourceChanged":source_changed})
} else {
json!({"kind":"added","files":[],"reasons":[]})
};
+9 -4
View File
@@ -4,9 +4,10 @@ mod history;
mod manifest;
mod server;
mod store;
pub mod verify;
#[cfg(test)]
mod tests;
pub mod verify;
mod visual_approval;
use impeccable_common::Io;
use serde_json::json;
use std::path::PathBuf;
@@ -78,13 +79,17 @@ pub fn run_with_capturer(
io.out(&format!("{}\n", receipt));
Ok(())
}
Some("serve") | Some("status") => {
Some("serve") | Some("status") | Some("refresh-approvals") => {
let id = arg(args, "--session").ok_or("needs --session <id from prepare>")?;
if id.len() != 64 || !id.bytes().all(|b| b.is_ascii_hexdigit()) {
return Err("invalid session id".into());
}
let dir = store.join(id);
if args[0] == "serve" {
if args[0] == "refresh-approvals" {
let count=store::refresh_approvals(&dir)?;
io.out(&format!("{}\n",json!({"carried":count})));
Ok(())
} else if args[0] == "serve" {
let port = arg(args, "--port")
.unwrap_or_else(|| "0".into())
.parse::<u16>()
@@ -102,7 +107,7 @@ pub fn run_with_capturer(
Ok(())
}
}
_ => Err("usage: impeccable component-review prepare|capture|verify --manifest <file> | serve --session <id> [--port 0] | status --session <id> [--store <outside-project-dir>]".into())
_ => Err("usage: impeccable component-review prepare|capture|verify --manifest <file> | serve --session <id> [--port 0] | status|refresh-approvals --session <id> [--store <outside-project-dir>]".into())
}
})();
match result {
+35 -7
View File
@@ -1,5 +1,5 @@
use super::manifest::{digest, freeze, relative, string, valid_box};
use serde_json::{Value, json};
use serde_json::{json, Value};
use std::{
fs,
io::Write,
@@ -216,12 +216,10 @@ fn prepare_bound(
packet["round"] = json!(round);
// URLs include the frozen revision, so a new round cannot silently replace old preview pixels.
let prefix = format!("/files/{rev}/");
packet["comp"]["url"] = json!(
packet["comp"]["url"]
.as_str()
.unwrap()
.replacen("/files/", &prefix, 1)
);
packet["comp"]["url"] = json!(packet["comp"]["url"]
.as_str()
.unwrap()
.replacen("/files/", &prefix, 1));
for c in packet["components"].as_array_mut().unwrap() {
for key in ["preview", "context", "thumbnail"] {
if let Some(url) = c[key]["url"].as_str() {
@@ -257,6 +255,9 @@ fn prepare_bound(
}
}
let mut state = json!({"schemaVersion":1,"contentRevision":content_revision,"project":project,"packet":packet,"files":hashes,"sources":sources,"capture":capture,"draft":draft,"receipt":null});
if let Some(previous) = &old {
super::visual_approval::carry(previous, &mut state, &blobs);
}
state["history"] = old
.as_ref()
.map(|previous| super::history::between(previous, &state))
@@ -375,3 +376,30 @@ pub fn submit(dir: &Path, body: &Value) -> Result<Value, String> {
// current.json is the authoritative atomic commit; a receipt export is not approval authority.
Ok(receipt)
}
/// Update only an unsubmitted draft, retaining packet/source identity and old receipts.
pub fn refresh_approvals(dir: &Path) -> Result<usize, String> {
let _guard = lock(dir)?;
let mut state = read(&dir.join("current.json"))?;
if !state["receipt"].is_null() {
return Ok(0);
}
sources_current(&state)?;
let Some(rev) = state["history"]["packet"]["revision"].as_str() else {
return Ok(0);
};
if rev.len() != 64 || !rev.bytes().all(|b| b.is_ascii_hexdigit()) {
return Err("invalid previous revision".into());
}
let previous = read(&dir.join(format!("revisions/{rev}.json")))?;
if previous["packet"]["revision"] != rev {
return Err("previous revision mismatch".into());
}
let count = super::visual_approval::carry(&previous, &mut state, &dir.join("blobs"));
let history = super::history::between(&previous, &state);
if count > 0 || state["history"] != history {
state["history"] = history;
write(&dir.join("current.json"), &state)?;
}
Ok(count)
}
@@ -525,3 +525,119 @@ fn shared_target_changes_invalidate_other_isolated_components() {
input["stage"] = Value::Null;
assert!(manifest::freeze(&f.project,&input).is_err());
}
#[test]
fn visual_approvals_survive_shared_source_edits_but_not_changed_scope_or_pixels() {
struct Renderer(&'static [u8]);
impl super::capture::ComponentCapturer for Renderer {
fn capture(
&mut self,
packet: &mut Value,
_: &std::collections::BTreeMap<String, Vec<u8>>,
) -> Result<super::capture::CapturedPreviews, String> {
packet["components"][1]["preview"] = json!({"kind":"image","sourceKind":"page","url":"/files/_review_captures/control.png"});
Ok(super::capture::CapturedPreviews {
files: std::collections::BTreeMap::from([(
"_review_captures/control.png".into(),
self.0.to_vec(),
)]),
evidence: json!({"schema":"native-component-previews-v1","components":[{"id":"art"},{"id":"control","views":{"preview":{"kind":"static-code","entry":"control.html","screenshotSha256":manifest::digest(self.0),"viewport":{"width":100,"height":100,"dpr":1}}}}]}),
})
}
}
let f = Fixture::new();
let dir = store::prepare_captured(
&f.store,
&f.project,
&f.manifest(),
Some(&mut Renderer(b"same pixels")),
)
.unwrap();
let before = store::read(&dir.join("current.json")).unwrap();
store::submit(&dir, &approve(&before)).unwrap();
fs::write(
f.project.join("shared.css"),
b"button{color:red} .unrelated{color:blue}",
)
.unwrap();
assert!(store::sources_current(&before).is_err());
store::prepare_captured(
&f.store,
&f.project,
&f.manifest(),
Some(&mut Renderer(b"same pixels")),
)
.unwrap();
let mut after = store::read(&dir.join("current.json")).unwrap();
assert_ne!(
before["packet"]["components"][1]["revision"],
after["packet"]["components"][1]["revision"]
);
assert_eq!(after["draft"]["decisions"]["control"]["action"], "approve");
assert_eq!(
after["visualApprovalCarry"]["control"]["basis"],
"identical-native-captures-v1"
);
assert!(after["receipt"].is_null());
assert_eq!(after["history"]["changes"]["control"]["kind"], "unchanged");
assert_eq!(after["history"]["changes"]["control"]["sourceChanged"], true);
assert_eq!(after["history"]["changes"]["control"]["carried"], true);
// Existing pending packets can gain carry-forward without changing their revision.
after["draft"]["decisions"]
.as_object_mut()
.unwrap()
.remove("control");
store::write(&dir.join("current.json"), &after).unwrap();
assert_eq!(store::refresh_approvals(&dir).unwrap(), 1);
assert_eq!(store::refresh_approvals(&dir).unwrap(), 0);
let carried = store::read(&dir.join("current.json")).unwrap();
assert_eq!(carried["packet"], after["packet"]);
assert_eq!(carried["sources"], after["sources"]);
// Changed scope, comp, preview bytes, absent proof and corrupt blobs fail closed.
let previous = store::read(&dir.join(format!(
"revisions/{}.json",
before["packet"]["revision"].as_str().unwrap()
)))
.unwrap();
for field in ["box", "medium", "note", "context"] {
let mut changed = after.clone();
changed["packet"]["components"][1][field] = json!("changed");
assert_eq!(
super::visual_approval::carry(&previous, &mut changed, &dir.join("blobs")),
0,
"{field}"
);
}
let mut changed = after.clone();
changed["capture"] = Value::Null;
assert_eq!(
super::visual_approval::carry(&previous, &mut changed, &dir.join("blobs")),
0
);
let mut changed = after.clone();
changed["draft"]["decisions"]["control"] = json!({"action":"revise"});
assert_eq!(
super::visual_approval::carry(&previous, &mut changed, &dir.join("blobs")),
0
);
let mut changed = after.clone();
changed["capture"]["components"][1]["views"]["preview"]["viewport"]["width"] = json!(200);
assert_eq!(super::visual_approval::carry(&previous, &mut changed, &dir.join("blobs")), 0);
let mut unsubmitted = previous.clone();
unsubmitted["receipt"] = Value::Null;
assert_eq!(super::visual_approval::carry(&unsubmitted, &mut after.clone(), &dir.join("blobs")), 0);
let pixel_path = dir.join("blobs").join(manifest::digest(b"same pixels"));
fs::write(&pixel_path, b"corrupted capture").unwrap();
assert_eq!(super::visual_approval::carry(&previous, &mut after.clone(), &dir.join("blobs")), 0);
fs::write(&pixel_path, b"same pixels").unwrap();
store::submit(&dir, &approve(&carried)).unwrap();
store::prepare_captured(
&f.store,
&f.project,
&f.manifest(),
Some(&mut Renderer(b"different pixels")),
)
.unwrap();
let changed = store::read(&dir.join("current.json")).unwrap();
assert!(changed["draft"]["decisions"]["control"].is_null());
}
@@ -0,0 +1,116 @@
//! Visual decisions bind reviewed pixels and scope; source integrity remains separate.
use super::manifest::digest;
use serde_json::{json, Value};
use std::path::Path;
fn identity(state: &Value, component: &Value, blobs: &Path) -> Option<Value> {
if state["capture"]["schema"] != "native-component-previews-v1" {
return None;
}
let evidence = state["capture"]["components"]
.as_array()?
.iter()
.find(|c| c["id"] == component["id"])?;
let prefix = format!("/files/{}/", state["packet"]["revision"].as_str()?);
let mut definition = component.clone();
definition.as_object_mut()?.remove("revision");
// Dependency paths remain part of scope; their bytes are checked by sources_current.
let mut views = serde_json::Map::new();
for key in ["preview", "context", "thumbnail", "comp"] {
let view = if key == "comp" {
&state["packet"]["comp"]
} else {
&component[key]
};
if view.is_null() {
continue;
}
let path = view["url"].as_str()?.strip_prefix(&prefix)?;
let hash = state["files"][path].as_str()?;
if hash.len() != 64 || !hash.bytes().all(|b| b.is_ascii_hexdigit()) {
return None;
}
if digest(&std::fs::read(blobs.join(hash)).ok()?) != hash {
return None;
}
let mut scoped = view.clone();
scoped.as_object_mut()?.remove("url");
scoped["sha256"] = json!(hash);
if key != "comp" {
definition[key] = scoped.clone();
}
let proof = &evidence["views"][key];
if view["sourceKind"] == "page" {
if proof["kind"] != "static-code"
|| proof["screenshotSha256"] != hash
|| !proof["viewport"].is_object()
{
return None;
}
// Keep capture geometry and representation. Whole-document hashes and
// resource hashes are deliberately not visual approval identities.
for field in [
"kind",
"entry",
"viewport",
"box",
"cropMethod",
"reducedMotion",
"svgElements",
"rasterElements",
"semanticControls",
] {
scoped[field] = proof[field].clone();
}
}
views.insert(key.into(), scoped);
}
Some(
json!({"component":definition,"views":views,"stage":state["packet"]["stage"],"request":state["packet"]["id"]}),
)
}
/// Rebind a submitted visual approval to an unchanged captured presentation.
/// Never manufacture a receipt or suppress a source change.
pub fn carry(previous: &Value, current: &mut Value, blobs: &Path) -> usize {
if previous["receipt"]["captureVerified"] != true
|| previous["receipt"]["submission"]["packetRevision"] != previous["packet"]["revision"]
{
return 0;
}
let Some(components) = current["packet"]["components"].as_array().cloned() else {
return 0;
};
let mut count = 0;
for component in components {
let Some(id) = component["id"].as_str() else {
continue;
};
// Preserve all new human work, including needs-work decisions.
if !current["draft"]["decisions"][id].is_null() {
continue;
}
let Some(prior) = previous["packet"]["components"]
.as_array()
.and_then(|cs| cs.iter().find(|c| c["id"] == id))
else {
continue;
};
let decision = &previous["receipt"]["submission"]["decisions"][id];
if decision["action"] != "approve" || decision["revision"] != prior["revision"] {
continue;
}
let Some(before) = identity(previous, prior, blobs) else {
continue;
};
if identity(current, &component, blobs).as_ref() != Some(&before) {
continue;
}
let mut carried = decision.clone();
carried["revision"] = component["revision"].clone();
current["draft"]["decisions"][id] = carried;
current["visualApprovalCarry"][id] = json!({"fromPacketRevision":previous["packet"]["revision"],"fromComponentRevision":prior["revision"],"basis":"identical-native-captures-v1"});
count += 1;
}
count
}
+15
View File
@@ -1868,3 +1868,18 @@ The packet and evidence contract is documented in
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.
### Visual approval carry-forward
Native component review carries a submitted approval when all displayed image
bytes and review scope are identical, even if shared source files changed.
The comparison includes the approved comp, region, representation, declared
component definition, and native capture geometry. Changed or unverified views
remain unapproved. Source revisions and integrity validation remain independent;
carrying a visual decision never creates a submitted receipt or accepts stale files.
`component-review refresh-approvals --session <id> --store <directory>` applies
this rule to an existing unsubmitted draft. It verifies stored image blobs and
current source integrity, preserves any new reviewer decisions, records the
prior approval revision, and leaves packet revisions and old receipts unchanged.