mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-18 09:06:53 +03:00
Preserve reference-bound typography across region remeasurement
Report exact invalid component bounds and duplicate IDs. The reviewed oracle change adds only the decoded-reference fingerprint; measurement and fidelity outputs remain unchanged. Includes failing/passing regressions. AI-assisted implementation and validation with Codex.
This commit is contained in:
@@ -12,6 +12,7 @@ use impeccable_comp::raster::{self as r, Image};
|
||||
use once_cell::sync::Lazy;
|
||||
use regex::Regex;
|
||||
use serde_json::{json, Map, Value};
|
||||
use sha2::{Digest, Sha256};
|
||||
|
||||
use crate::util::{self, arg, arg_or, flag, num, r4, r4f, round};
|
||||
|
||||
@@ -1040,7 +1041,7 @@ pub fn run(argv: &[String], io: &mut Io) -> i32 {
|
||||
io.err("comp-spec: pass --grid to get the coordinate grid, then --regions <json> (or --auto for band regions)\n");
|
||||
return 1;
|
||||
};
|
||||
let spec = match measure_regions(&comp, ®ions_input, comp_path) {
|
||||
let mut spec = match measure_regions(&comp, ®ions_input, comp_path) {
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
io.err(&format!("comp-spec: {e}\n"));
|
||||
@@ -1048,6 +1049,17 @@ pub fn run(argv: &[String], io: &mut Io) -> i32 {
|
||||
}
|
||||
};
|
||||
let spec_out = resolve(io, &spec_path);
|
||||
// Bind cached font evidence to the decoded reference, including dimensions.
|
||||
// Legacy specs without this identity are deliberately remeasured once.
|
||||
let mut hasher = Sha256::new();
|
||||
hasher.update(comp.width.to_le_bytes());
|
||||
hasher.update(comp.height.to_le_bytes());
|
||||
hasher.update(&comp.data);
|
||||
spec["compSha256"] = json!(format!("{:x}", hasher.finalize()));
|
||||
if let Some(previous) = std::fs::read(&spec_out).ok()
|
||||
.and_then(|bytes| serde_json::from_slice::<Value>(&bytes).ok()) {
|
||||
preserve_typography(&mut spec, &previous);
|
||||
}
|
||||
if let Some(parent) = spec_out.parent() {
|
||||
let _ = std::fs::create_dir_all(parent);
|
||||
}
|
||||
@@ -1058,6 +1070,25 @@ pub fn run(argv: &[String], io: &mut Io) -> i32 {
|
||||
0
|
||||
}
|
||||
|
||||
/// Remeasuring an unrelated region must not erase measured font work. Reuse
|
||||
/// only the existing spec's evidence, never a `type` claim in the input file.
|
||||
fn preserve_typography(spec: &mut Value, previous: &Value) {
|
||||
if !spec["compSha256"].is_string() || spec["compSha256"] != previous["compSha256"] {
|
||||
return;
|
||||
}
|
||||
let Some(old_regions) = previous["regions"].as_array() else { return; };
|
||||
let Some(regions) = spec["regions"].as_array_mut() else { return; };
|
||||
for region in regions {
|
||||
if !matches!(region["kind"].as_str(), Some("text" | "control")) { continue; }
|
||||
let Some(old) = old_regions.iter().find(|old| old["id"] == region["id"]) else { continue; };
|
||||
if ["kind", "medium", "box", "px", "text"].iter().all(|key| old[*key] == region[*key]) {
|
||||
if let Some(ty) = old.get("type").filter(|ty| ty.is_object()) {
|
||||
region["type"] = ty.clone();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod reference_tests {
|
||||
use super::*;
|
||||
|
||||
@@ -120,3 +120,44 @@ fn measure_regions_refuses_oversized_code_region() {
|
||||
let err = comp_spec::measure_regions(&comp, &input, "comp.png").unwrap_err();
|
||||
assert!(err.contains("covers 100% of the comp") || err.contains("% of the comp"), "got: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remeasure_keeps_only_unchanged_reference_typography() {
|
||||
let dir = std::env::temp_dir().join(format!("impeccable-remeasure-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
std::fs::copy(fixtures().join("comp.png"), dir.join("comp.png")).unwrap();
|
||||
let mut input = json!({"allowUncovered":true,"regions":[
|
||||
{"id":"heading","kind":"text","note":"Main heading text","text":"Welcome","box":{"x":0.1,"y":0.1,"w":0.5,"h":0.1},"snap":false},
|
||||
{"id":"other","kind":"text","note":"Other small text","text":"Details","box":{"x":0.1,"y":0.4,"w":0.5,"h":0.1},"snap":false}
|
||||
]});
|
||||
let run = |input: &Value| {
|
||||
std::fs::write(dir.join("regions.json"), input.to_string()).unwrap();
|
||||
let (mut io, _) = impeccable_common::Io::captured("", dir.clone(), Default::default());
|
||||
let args = ["--comp","comp.png","--regions","regions.json","--spec","spec.json"].map(String::from);
|
||||
assert_eq!(comp_spec::run(&args, &mut io), 0);
|
||||
serde_json::from_slice::<Value>(&std::fs::read(dir.join("spec.json")).unwrap()).unwrap()
|
||||
};
|
||||
let mut first = run(&input);
|
||||
let measured = json!({"comp":{"capHeightPx":12},"chosen":{"family":"Example","stamp":"existing-stamp"}});
|
||||
first["regions"][0]["type"] = measured.clone();
|
||||
first["regions"][1]["type"] = measured.clone();
|
||||
std::fs::write(dir.join("spec.json"), first.to_string()).unwrap();
|
||||
input["regions"][1]["box"]["y"] = json!(0.5);
|
||||
let next = run(&input);
|
||||
assert_eq!(next["regions"][0]["type"], measured, "unrelated region edit erased typography");
|
||||
assert!(next["regions"][1]["type"].is_null(), "moved region reused stale measurement");
|
||||
input["regions"][0]["text"] = json!("Different");
|
||||
assert!(run(&input)["regions"][0]["type"].is_null());
|
||||
let mut prior = run(&input);
|
||||
prior["regions"][0]["type"] = measured.clone();
|
||||
std::fs::write(dir.join("spec.json"), prior.to_string()).unwrap();
|
||||
std::fs::copy(fixtures().join("build_flat.png"), dir.join("comp.png")).unwrap();
|
||||
assert!(run(&input)["regions"][0]["type"].is_null(), "replaced comp reused stale measurement");
|
||||
// Unbound legacy records must be remeasured once, never guessed current.
|
||||
let mut legacy = run(&input);
|
||||
legacy.as_object_mut().unwrap().remove("compSha256");
|
||||
legacy["regions"][0]["type"] = measured;
|
||||
std::fs::write(dir.join("spec.json"), legacy.to_string()).unwrap();
|
||||
assert!(run(&input)["regions"][0]["type"].is_null());
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
@@ -182,8 +182,11 @@ pub fn freeze(project: &Path, input: &Value) -> Result<(Value, BTreeMap<String,
|
||||
}
|
||||
}
|
||||
let id = string(c, "id")?.to_string();
|
||||
if !ids.insert(id) || !valid_box(&c["box"]) {
|
||||
return Err("duplicate component or invalid box".into());
|
||||
if !ids.insert(id.clone()) {
|
||||
return Err(format!("duplicate component id {id:?}; each component needs a unique id"));
|
||||
}
|
||||
if !valid_box(&c["box"]) {
|
||||
return Err(format!("component {id:?} has invalid box {}. Expected normalized {{x,y,w,h}}: x/y >= 0, w/h > 0, x+w and y+h <= 1 (tolerance 0.00001).", c["box"]));
|
||||
}
|
||||
for k in ["name", "medium", "note"] {
|
||||
if !c[k].is_string() {
|
||||
|
||||
@@ -677,3 +677,17 @@ fn review_groups_preserve_instances_and_require_a_shared_code_document() {
|
||||
input["components"][2]["preview"]["path"]=json!("different.html");
|
||||
assert!(manifest::freeze(&f.project,&input).unwrap_err().contains("share one code document"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_component_geometry_names_the_component_and_bounds() {
|
||||
let f = Fixture::new();
|
||||
let mut input = f.manifest();
|
||||
input["components"][0]["box"]["w"] = json!(2);
|
||||
let error = manifest::freeze(&f.project, &input).unwrap_err();
|
||||
assert!(error.contains("art") && error.contains("box") && error.contains("2") && error.contains("normalized"), "{error}");
|
||||
let mut input = f.manifest();
|
||||
let duplicate = input["components"][0].clone();
|
||||
input["components"].as_array_mut().unwrap().push(duplicate);
|
||||
let error = manifest::freeze(&f.project, &input).unwrap_err();
|
||||
assert!(error.contains("duplicate component id") && error.contains("art"), "{error}");
|
||||
}
|
||||
|
||||
@@ -176,3 +176,7 @@ stderr are unchanged. The golden was updated to enforce these exact results;
|
||||
this is not an open-ended accepted delta. Frozen function call vectors remain
|
||||
unchanged. The Rust narrow-region regression independently checks that changing
|
||||
only neighbouring pixels leaves the measured crop identical.
|
||||
|
||||
## Recorded 2026-09-17: reference-bound typography reuse
|
||||
|
||||
- `comp-spec-regions`: the written spec adds `compSha256`, a SHA-256 of decoded dimensions and pixels. This binds retained typography to the exact reference when regions are remeasured. Structured comparison verified that only this field changed; stdout, stderr, exit status, regions, palettes and bounds are identical. The failing/passing regression separately verifies preservation and invalidation.
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user