From 068ca4c1cffe1d3c8c8b619013688effb1cd0419 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Fri, 18 Sep 2026 12:24:02 -0700 Subject: [PATCH] Expose partial reference masks during region mapping Surface partial masks in the inspection queue and CLI summary, with links between affected artwork and excluding code bounds. Explain granular foreground bounds in the shared mapping contract. Keep matcher pixels and approval semantics unchanged. Validation: 578 Rust tests, default Bun/Node suite against the rebuilt engine, provider build, browser inspection, and offline hotel reference parity. AI assistance: implemented and verified with Codex. --- crates/comp-verbs/src/map_inspection.html | 77 +++++++++++++++----- crates/comp-verbs/src/map_inspection.rs | 41 ++++++++++- crates/comp-verbs/src/region-map.schema.json | 2 +- docs/CLI-CONTRACT.md | 2 +- skill/reference/region-map.md | 2 +- 5 files changed, 102 insertions(+), 22 deletions(-) diff --git a/crates/comp-verbs/src/map_inspection.html b/crates/comp-verbs/src/map_inspection.html index 188eaee64..667b3f9f5 100644 --- a/crates/comp-verbs/src/map_inspection.html +++ b/crates/comp-verbs/src/map_inspection.html @@ -14,11 +14,13 @@ --line: #d2d9d4; --accent: #00685f; --danger: #a6361d; + --warning: #86550b; font: 14px/1.45 'Avenir Next', system-ui, sans-serif; color: var(--ink); background: var(--paper); } * { box-sizing: border-box; } + [hidden] { display: none !important; } body { margin: 0; } button, select, a { font: inherit; } button, select { @@ -68,6 +70,13 @@ .region[aria-current=true] { background: #e4eeea; border-color: #8dbbb0; } .region .dot { flex: 0 0 7px; height: 7px; background: var(--line); border-radius: 50%; } .region.error .dot { background: var(--danger); } + .region.masked .dot { background: var(--warning); } + .region .name { flex: 1; } + .region .status { font-size: 11px; white-space: nowrap; color: var(--warning); font-variant-numeric: tabular-nums; } + .region.error .status { color: var(--danger); } + #related { display: flex; flex-wrap: wrap; align-items: center; gap: 6px; margin-top: 10px; font-size: 12px; color: var(--muted); } + #related:empty { display: none; } + #related button { padding: 4px 8px; font-size: 12px; } main { min-width: 0; overflow: auto; padding: 24px; } .heading { display: flex; justify-content: space-between; align-items: start; gap: 20px; margin-bottom: 20px; } h2 { font-size: 24px; line-height: 1.2; margin: 0 0 8px; font-weight: 600; } @@ -138,15 +147,15 @@
-

Check the region map

- +

Check the region map

+
@@ -178,7 +187,14 @@ const $ = id => document.getElementById(id); const ns = 'http://www.w3.org/2000/svg'; const errors = id => data.issues.filter(i => i.regionId === id && i.severity === 'error'); - const problemRegions = data.regions.filter(r => errors(r.id).length); + // A partial exclusion needs visual inspection even when all boxes are valid. + // Do not claim it is a failed asset or let zero hard errors hide it. + const partialMasks = data.regions.filter(r => r.reference?.excludedPixels > 0 && !r.reference.fullyExcluded); + const problemRegions = data.regions.filter(r => errors(r.id).length || r.reference?.excludedPixels > 0) + .sort((a, b) => Number(!!errors(b.id).length) - Number(!!errors(a.id).length) + || (b.reference?.excludedFraction || 0) - (a.reference?.excludedFraction || 0)); + const affectedBy = id => data.regions.filter(r => r.reference?.regions.some(m => m.id === id)); + const percent = ref => (100 * ref.excludedFraction).toFixed(1).replace(/\.0$/, ''); let selected = null; let filter = problemRegions.length ? 'problems' : 'all'; let wholeComp = false; @@ -199,23 +215,31 @@ } function renderList() { $('list').replaceChildren(); - $('problems').textContent = `Problems (${problemRegions.length})`; + $('problems').textContent = `Inspect (${problemRegions.length})`; $('all').textContent = `All (${data.regions.length})`; for (const key of ['problems', 'all']) $(key).setAttribute('aria-pressed', String(filter === key)); for (const r of filter === 'problems' ? problemRegions : data.regions) { - const button = element('button', '', `region ${errors(r.id).length ? 'error' : ''}`); + const button = element('button', '', `region ${errors(r.id).length ? 'error' : r.reference?.excludedPixels ? 'masked' : ''}`); + button.setAttribute('aria-label', name(r)); button.setAttribute('aria-current', String(selected?.id === r.id)); - button.append(element('span', '', 'dot'), element('span', name(r))); + button.append(element('span', '', 'dot'), element('span', name(r), 'name')); + if (errors(r.id).length || r.reference?.excludedPixels) { + const label = errors(r.id).length ? 'Error' : `${percent(r.reference)}%`; + const badge = element('span', label, 'status'); + badge.title = errors(r.id).length ? 'Map error' : `${label} excluded — inspect foreground bounds`; + button.setAttribute('aria-description', badge.title); + button.append(badge); + } button.onclick = () => select(r); $('list').append(button); } - if (!$('list').children.length) $('list').append(element('p', 'No drawable regions have errors. Other map findings lists rejected entries.', 'empty')); + if (!$('list').children.length) $('list').append(element('p', 'No drawable regions have errors or masked pixels. Crop accuracy still needs visual inspection; rejected entries are in Other map findings.', 'empty')); } function renderContext() { if (!selected) return; const svg = $('context'); svg.replaceChildren(); - const covering = (selected.reference?.regions || []).map(m => data.regions.find(r => r.id === m.id)).filter(Boolean); + const covering = selected.reference ? selected.reference.regions.map(m => data.regions.find(r => r.id === m.id)).filter(Boolean) : affectedBy(selected.id); const boxes = [selected, ...covering]; const left = Math.min(...boxes.map(r => r.px.x)); const top = Math.min(...boxes.map(r => r.px.y)); @@ -232,6 +256,7 @@ } svg.append(svgElement('rect', { x: selected.px.x, y: selected.px.y, width: selected.px.w, height: selected.px.h, fill: 'none', stroke: '#00685f', 'stroke-width': 3, 'vector-effect': 'non-scaling-stroke' })); $('covering-key').hidden = covering.length === 0; + $('covering-key').lastChild.textContent = selected.reference ? ' Overlapping foreground' : ' Affected artwork'; $('context-toggle').textContent = wholeComp ? 'Show nearby context' : 'Show whole comp'; $('context-toggle').setAttribute('aria-pressed', String(wholeComp)); } @@ -262,20 +287,35 @@ selected = r; $('title').textContent = name(r); const ref = r.reference; - const percent = ref ? (100 * ref.excludedFraction).toFixed(1).replace(/\.0$/, '') : '0'; - const covering = ref?.regions.map(m => name(data.regions.find(r => r.id === m.id))).join(', '); + const percentage = ref ? percent(ref) : '0'; + const sources = (ref?.regions || []).map(m => data.regions.find(r => r.id === m.id)).filter(Boolean); + const affected = affectedBy(r.id); + const covering = sources.map(name).join(', '); $('explanation').className = errors(r.id).length ? 'error' : ''; if (ref?.fullyExcluded) { $('explanation').textContent = `The “${covering}” box hides this entire asset from the checker.`; $('impact').textContent = 'Every hatched pixel is ignored. Nothing remains to compare, even if the generated asset is perfect. The covering box needs correcting.'; } else if (ref?.excludedPixels) { - $('explanation').textContent = `${percent}% of this asset is excluded by overlapping text or controls.`; - $('impact').textContent = 'Hatched pixels are ignored; the rest is compared. This is expected only where text or controls actually sit in front of the artwork.'; + $('explanation').textContent = `${percentage}% excluded — check the foreground bounds.`; + $('impact').textContent = 'Hatching should cover foreground text or controls only. If it covers artwork in the gaps, split the foreground into tighter boxes. Keep every text element.'; + } else if (affected.length) { + $('explanation').textContent = `This box excludes pixels from ${affected.length} ${affected.length === 1 ? 'asset' : 'assets'}.`; + $('impact').textContent = 'The entire overlap is masked, including empty space. Bound separate text elements separately; use a container only to describe their layout, retaining every child.'; } else { $('explanation').textContent = ref ? 'No overlapping text or controls hide this asset from the checker.' : 'This region is built in code. Check that its box names one element.'; $('impact').textContent = ref ? 'All pixels in this crop remain available for comparison. Still check that the crop contains the right artwork.' : 'Code regions that overlap artwork can exclude those pixels from asset comparison. Containers are treated separately.'; } - $('ignored-label').textContent = ref ? `Ignored pixels · ${percent}%` : 'Same region'; + $('related').replaceChildren(); + const related = ref ? sources : affected; + if (related.length) { + $('related').append(element('span', ref ? 'Excluding boxes:' : 'Inspect affected assets:')); + for (const other of related) { + const button = element('button', name(other)); + button.onclick = () => select(other); + $('related').append(button); + } + } + $('ignored-label').textContent = ref ? `Ignored pixels · ${percentage}%` : 'Same region'; $('asset-key').textContent = ref ? 'Asset boundary' : 'Region boundary'; $('details').replaceChildren(); for (const text of [r.id, r.note, `${r.kind} · ${r.px.w} × ${r.px.h} original pixels · x ${r.px.x}, y ${r.px.y}`, r.parentId && `Parent: ${r.parentId}`, r.reviewGroup && `Review group: ${r.reviewGroup} (every member retained)`].filter(Boolean)) $('details').append(element('p', text)); @@ -287,14 +327,16 @@ } $('details').append(element('p', 'Reference only. These images cannot ship as page assets. Inspection does not change build state or approvals.')); $('next').hidden = problemRegions.length < 2; - $('next').textContent = `Next problem (${Math.max(0, problemRegions.findIndex(p => p.id === r.id)) + 1}/${problemRegions.length}) →`; + const queueIndex = problemRegions.findIndex(p => p.id === r.id); + $('next').textContent = queueIndex >= 0 ? `Next check (${queueIndex + 1}/${problemRegions.length}) →` : 'Next check →'; + history.replaceState(null, '', `#region=${encodeURIComponent(r.id)}`); renderList(); renderContext(); renderCrop($('original'), false); renderCrop($('ignored'), true); } for (const key of ['all', 'problems']) $(key).onclick = () => { filter = key; renderList(); }; $('next').onclick = () => select(problemRegions[(problemRegions.findIndex(r => r.id === selected.id) + 1) % problemRegions.length]); $('context-toggle').onclick = () => { wholeComp = !wholeComp; renderContext(); }; $('zoom').onchange = () => { if (selected) { renderCrop($('original'), false); renderCrop($('ignored'), true); } }; - $('summary').textContent = `${data.inputRegionCount} regions · ${data.issues.filter(i => i.severity === 'error').length} errors`; + $('summary').textContent = `${data.inputRegionCount} regions · ${data.issues.filter(i => i.severity === 'error').length} errors · ${partialMasks.length} partial masks to inspect`; const otherIssues = data.issues.filter(i => i.code !== 'fully-masked' && i.code !== 'foreground-mask'); $('diagnostics-label').textContent = `Other map findings (${otherIssues.length})`; for (const i of otherIssues) { @@ -302,7 +344,8 @@ item.append(element('b', i.regionId || i.code.replaceAll('-', ' ')), element('span', i.message)); $('issues').append(item); } - if (data.regions.length) select(problemRegions[0] || data.regions[0]); + const requestedRegion = new URLSearchParams(location.hash.slice(1)).get('region'); + if (data.regions.length) select(data.regions.find(r => r.id === requestedRegion) || problemRegions[0] || data.regions[0]); else { $('inspection').hidden = true; $('next').hidden = true; $('diagnostics').open = true; renderList(); } diff --git a/crates/comp-verbs/src/map_inspection.rs b/crates/comp-verbs/src/map_inspection.rs index 0bfb7bf9b..febd0ce84 100644 --- a/crates/comp-verbs/src/map_inspection.rs +++ b/crates/comp-verbs/src/map_inspection.rs @@ -217,7 +217,8 @@ fn inspect(comp: &Image, input: &Value, comp_path: &str) -> Value { if let Some(message) = reference.issue(&id) { issue(&mut issues, "error", "fully-masked", Some(&id), message); } else if reference.excluded_pixels > 0 { - issue(&mut issues,"info","foreground-mask",Some(&id),format!("Foreground regions exclude {:.1}% of this reference. Inspect the crop and mask together.",100.*reference.excluded_pixels as f64/reference.total_pixels as f64)); + let sources = reference.excluded_regions.iter().filter_map(|r| r["id"].as_str()).collect::>().join(", "); + issue(&mut issues,"warning","foreground-mask",Some(&id),format!("{sources} exclude {:.1}% of this reference. Check that the excluded pixels contain foreground, not artwork between separate text elements. Split overly broad foreground bounds; review grouping must not change geometry.",100.*reference.excluded_pixels as f64/reference.total_pixels as f64)); } region["reference"] = reference.audit(); } @@ -395,7 +396,8 @@ pub fn run(argv: &[String], io: &mut Io, comp: &Image, comp_path: &str) -> i32 { if flag(argv, "json") { io.out(&format!("{report}\n")); } else { - io.out(&format!("MAP {}/index.html\nOVERLAY {}/overlay.png\n{} regions, {errors} errors. Reference only; no build state or approvals changed.\n",report["outputDir"].as_str().unwrap(),report["outputDir"].as_str().unwrap(),report["inputRegionCount"])); + let partial_masks = report["issues"].as_array().unwrap().iter().filter(|i| i["code"] == "foreground-mask").count(); + io.out(&format!("MAP {}/index.html\nOVERLAY {}/overlay.png\n{} regions, {errors} errors, {partial_masks} partial masks to inspect. Zero errors does not certify crop accuracy. Reference only; no build state or approvals changed.\n",report["outputDir"].as_str().unwrap(),report["outputDir"].as_str().unwrap(),report["inputRegionCount"])); for i in report["issues"].as_array().unwrap() { io.out(&format!( "{} {}: {}\n", @@ -419,6 +421,41 @@ mod tests { use super::*; use impeccable_comp::raster::create_image; + #[test] + fn partial_masks_need_attention_even_without_geometry_errors() { + let image = create_image(100, 100, [240, 240, 240, 255]); + let input = json!({"regions":[ + {"id":"art", "kind":"image", "note":"Decorative artwork", "pixelBox":{"x":50,"y":50,"w":20,"h":20}}, + {"id":"details", "kind":"text", "note":"Separate text elements", "pixelBox":{"x":30,"y":30,"w":30,"h":30}} + ]}); + let report = inspect(&image, &input, "comp.png"); + let findings = report["issues"].as_array().unwrap(); + assert!(!findings.iter().any(|i| i["severity"] == "error")); + let mask = findings.iter().find(|i| i["code"] == "foreground-mask").unwrap(); + assert_eq!(mask["severity"], "warning"); + assert!(mask["message"].as_str().unwrap().contains("details")); + assert_eq!(report["regions"][0]["reference"]["excludedPixels"], 100); + } + + #[test] + fn granular_text_bounds_preserve_art_in_gaps_without_dropping_foreground() { + let image = create_image(100, 100, [240, 240, 240, 255]); + let input = json!({"regions":[ + {"id":"art", "kind":"image", "note":"Decorative artwork", "pixelBox":{"x":50,"y":50,"w":20,"h":20}}, + {"id":"details", "kind":"chrome", "container":true, "note":"Text layout extent", "pixelBox":{"x":30,"y":30,"w":30,"h":30}}, + {"id":"title", "kind":"text", "parentId":"details", "reviewGroup":"titles", "note":"First title text", "pixelBox":{"x":30,"y":30,"w":30,"h":8}}, + {"id":"title-2", "kind":"text", "reviewGroup":"titles", "note":"Second title text", "pixelBox":{"x":5,"y":30,"w":30,"h":8}}, + {"id":"caption", "kind":"text", "parentId":"details", "note":"Caption overlapping art", "pixelBox":{"x":30,"y":50,"w":22,"h":5}} + ]}); + let report = inspect(&image, &input, "comp.png"); + let reference = &report["regions"][0]["reference"]; + assert_eq!(reference["excludedPixels"], 10, "only the actual caption overlap is masked"); + assert_eq!(reference["regions"][0]["id"], "caption"); + assert_eq!(reference["ignoredContainers"], json!(["details"])); + assert_eq!(report["reviewGroups"]["titles"], json!(["title", "title-2"])); + assert_eq!(report["regions"].as_array().unwrap().len(), 5); + } + #[test] fn mixed_review_groups_are_reported_without_discarding_geometry() { let comp = Image { width: 100, height: 100, data: vec![255; 100*100*4] }; diff --git a/crates/comp-verbs/src/region-map.schema.json b/crates/comp-verbs/src/region-map.schema.json index 71a96adb7..15088f6a5 100644 --- a/crates/comp-verbs/src/region-map.schema.json +++ b/crates/comp-verbs/src/region-map.schema.json @@ -64,7 +64,7 @@ "exclusiveMinimum": 0 } }, - "description": "Whole pixels in the original comp. Must fit within its dimensions; inspect the crop." + "description": "Whole pixels in the original comp. Must fit within its dimensions; inspect the crop. Non-container code bounds mask their full overlap with artwork. Bound separate text elements separately, not their combined layout extent." }, "box": { "type": "object", diff --git a/docs/CLI-CONTRACT.md b/docs/CLI-CONTRACT.md index 862b83016..3e79f9102 100644 --- a/docs/CLI-CONTRACT.md +++ b/docs/CLI-CONTRACT.md @@ -449,7 +449,7 @@ retain their local-development trust behavior. See [bundle signing](BUNDLE-SIGNI Ported from the former `skill/scripts/{comp-spec,comp-diff,font-match,build-phase}.mjs` (+ `lib/{png,raster,image-metrics,font-fingerprint,font-index,hero-checks}.mjs`) into the engine; invoked as `{{scripts_path}}/impeccable `. All four resolve paths against the process cwd. Printed commands spell the launcher via `IMPECCABLE_SELF` (default `impeccable`), so they name `{{scripts_path}}/impeccable `, never `node …mjs`. ISO `createdAt`/`startedAt` timestamps in stdout and written JSON are the only run-dependent output. -- **`comp-spec`** — turns an approved comp into a measured build spec. `--schema` prints the region-map JSON schema without reading or writing a project. `--comp --regions --inspect-map` writes reference-only crops, overlay and report in a new directory; exit 2 means diagnosed hard errors, exit 1 means command/input failure. Mapping metadata (`parentId`, `reviewGroup`) survives measurement. Review groups must contain peer code regions of one kind and container status; mixed groups and grouped raster assets are refused both at measurement and at component-review preparation. `--comp --grid` writes `.impeccable/build/comp-grid.png` (10x10 labeled grid) and prints PALETTE/BANDS/NEXT; `--comp --regions ` measures regions into `.impeccable/build/spec.json` (region box, sampled palette, medium, aspect, detail energy, plate path for raster kinds) and prints the spec; `--comp --auto [--out ]` writes approximate bands to a new draft file (default `.impeccable/build/regions.draft.json`), without modifying the measured spec or build state; `--print` prints the compact spec; `--crop [--out f] [--scale n] [--raw]` writes a reference crop; `--plate-prompt ` prints the regeneration prompt. `--spec ` overrides the spec path (default `.impeccable/build/spec.json`). Validation refusals (stderr, exit 1) are the JS strings verbatim: a region with no id / duplicate id / no note, a code-kind region whose note names painted material, a code region over 25% of the comp, a grid span that is not `:`, uncovered ink cells without `allowUncovered`. spec.json is byte-identical to the JS output. +- **`comp-spec`** — turns an approved comp into a measured build spec. `--schema` prints the region-map JSON schema without reading or writing a project. `--comp --regions --inspect-map` writes reference-only crops, overlay and report in a new directory; exit 2 means diagnosed hard errors, exit 1 means command/input failure. Partial foreground masks are warnings naming the excluding regions and remain in the inspector’s main inspection queue even with zero hard errors. The CLI summary counts them separately; they do not imply an asset failure or visual approval. Code rectangles exclude their full overlap, so separate text elements need separate bounds. Mapping metadata (`parentId`, `reviewGroup`) survives measurement. Review groups must contain peer code regions of one kind and container status; mixed groups and grouped raster assets are refused both at measurement and at component-review preparation. `--comp --grid` writes `.impeccable/build/comp-grid.png` (10x10 labeled grid) and prints PALETTE/BANDS/NEXT; `--comp --regions ` measures regions into `.impeccable/build/spec.json` (region box, sampled palette, medium, aspect, detail energy, plate path for raster kinds) and prints the spec; `--comp --auto [--out ]` writes approximate bands to a new draft file (default `.impeccable/build/regions.draft.json`), without modifying the measured spec or build state; `--print` prints the compact spec; `--crop [--out f] [--scale n] [--raw]` writes a reference crop; `--plate-prompt ` prints the regeneration prompt. `--spec ` overrides the spec path (default `.impeccable/build/spec.json`). Validation refusals (stderr, exit 1) are the JS strings verbatim: a region with no id / duplicate id / no note, a code-kind region whose note names painted material, a code region over 25% of the comp, a grid span that is not `:`, uncovered ink cells without `allowUncovered`. spec.json is byte-identical to the JS output. Automatic drafts require decomposition into actual visible elements before measurement; a draft flag blocks accidental submission, and existing draft files are never overwritten. Successful `--regions` measurements record the source path and SHA-256. Spec and plate gates reject changed or missing source files, so a failed region edit cannot silently reuse the previous measurements. Legacy specs without source metadata remain readable. Region inputs support three coordinate representations: inclusive `grid` cells, normalized `box: {x,y,w,h}`, or `pixelBox: {x,y,w,h}` in whole original-comp pixels. Pixel boxes cannot be combined with the other formats and must be positive-sized and contained in the comp; they avoid snapping an asset boundary to a neighbouring grid cell. Foreground UI excluded from a plate reference is excluded at the same aligned coordinates from the candidate during scoring; unmasked asset bytes still undergo provenance checks. - **`comp-diff`** — `--comp --build [--spec spec.json] [--out-dir dir] [--align top|stretch|cover] [--label name] [--threshold t] [--json] [--no-files]`. Scores structure / color / detail / bands and per-region verdicts (`match`/`drift`/`missing`/`contradicted`); writes `side-by-side.png`, `heatmap.png`, `regions/.png`, and `report.json` under `--out-dir` (unless `--no-files`); prints the text summary or, with `--json`, the report. Exit 0 measured, 1 usage/unreadable input, 3 below `--threshold`. The JSON report and text summary are byte-identical to the JS. diff --git a/skill/reference/region-map.md b/skill/reference/region-map.md index c5abf29a4..d33f14623 100644 --- a/skill/reference/region-map.md +++ b/skill/reference/region-map.md @@ -5,7 +5,7 @@ A region map names what is actually visible in the approved comp before asset pr 1. Run `{{scripts_path}}/impeccable comp-spec --comp --grid` and open the original and gridded images. 2. Run `{{scripts_path}}/impeccable comp-spec --schema` for the JSON fields. Write `regions.json` with a `regions` array. Each region needs a stable `id`, `kind`, `note`, and exactly one of `pixelBox`, normalized `box`, or `grid`. Use the original comp’s dimensions. 3. Run `{{scripts_path}}/impeccable comp-spec --comp --regions regions.json --inspect-map`. The output points to a report, overlay and exact crops. The default prints findings; `--json` prints the entire report. -4. Open the crops and compare their bounds with the original. Inspect excluded foreground pixels as well as geometry errors. Correct the map and inspect again; use a new output directory each time. Coverage warnings are hints, not proof of completeness. +4. Open the crops and compare their bounds with the original. Inspect excluded foreground pixels as well as geometry errors. Every overlapping non-container code box is masked in full, including empty space inside it. Bound separate text elements separately so artwork in the gaps stays visible; a container describes their layout extent and never replaces its children. Correct the map and inspect again; use a new output directory each time. Zero errors does not certify crop accuracy. Coverage warnings are hints, not proof of completeness. If the request ends at mapping, stop with the map, inspection report and unresolved findings. To continue a build, measure the inspected map with `comp-spec --comp --regions regions.json` and follow [new-work.md](new-work.md).