fix(agent): stop the image repairs from reading an oversized plate as intent
Both of today's image repairs trusted an authored fixed size as the design's intent. A weak model routinely drops a 400x300 plate into a 390px phone card and CROPS it - which looks right - so the rail repair widened every card to 400 (one card ate the screen, the rest clipped away) and the band repair grew every photo band to 300 (a wall of empty space under each deal). Both now demand that the number be plausible for the slot it is in: a card is starved only when it is genuinely unusable and its demand still leaves the rail scrollable, and a band only takes a photo's height when the result is not taller than the band is wide. Oversized plates go back to the fixer that shrinks them.
This commit is contained in:
parent
d992449a1c
commit
46a082df2f
|
|
@ -188,7 +188,7 @@ pub fn geometry_validate_and_fix(sink: &mut dyn DocSink, root_id: &str) -> usize
|
|||
collect_frame_overflow_fixes(&v, &rects, &mut cmds);
|
||||
collect_oversized_image_fixes(&v, &mut cmds);
|
||||
collect_absolute_fill_image_fixes(&v, &rects, &mut cmds);
|
||||
collect_image_slot_fixes(&v, &mut cmds);
|
||||
collect_image_slot_fixes(&v, &rects, &mut cmds);
|
||||
collect_grow_to_fit_fixes(&v, &rects, &mut cmds);
|
||||
collect_starved_rail_card_fixes(&v, &rects, &mut cmds);
|
||||
collect_row_gap_fixes(&v, &rects, &mut cmds);
|
||||
|
|
@ -1126,6 +1126,17 @@ fn collect_grow_to_fit_fixes(
|
|||
/// cards straight back to fill_container.
|
||||
const RAIL_STARVE_EPS: f64 = 12.0;
|
||||
const RAIL_MIN_CARDS: usize = 3;
|
||||
/// A card is only STARVED when it is genuinely unusable — the measured case
|
||||
/// was 58px cards around 160px photos. A card that merely CROPS an oversized
|
||||
/// photo (a 400x300 plate clipped into a 170px card) looks right and is not
|
||||
/// starved; widening it to the plate would blow one card across the whole
|
||||
/// screen. That happened (user report 2026-07-12: "自检又违背设计意图了").
|
||||
const RAIL_CARD_STARVED_W: f64 = 120.0;
|
||||
/// …and the demand is only credible as an INTENT if the resulting cards still
|
||||
/// read as a scroll rail (the next card peeks). A "demand" wider than this
|
||||
/// share of the rail is an oversized image, not a width intent — that class
|
||||
/// belongs to `collect_oversized_image_fixes`, which shrinks the image.
|
||||
const RAIL_CARD_MAX_FRACTION: f64 = 0.72;
|
||||
|
||||
/// The widest fixed-width DESCENDANT plus the card's own side padding — the
|
||||
/// width this card provably needs to show its authored content. Descendants,
|
||||
|
|
@ -1169,14 +1180,23 @@ fn collect_starved_rail_card_fixes(
|
|||
&& c.get("width").and_then(Value::as_str) == Some("fill_container")
|
||||
})
|
||||
.collect();
|
||||
if cards.len() >= RAIL_MIN_CARDS {
|
||||
let rail_w = v
|
||||
.get("id")
|
||||
.and_then(Value::as_str)
|
||||
.and_then(|id| rects.get(id))
|
||||
.map(|r| r.w)
|
||||
.unwrap_or(0.0);
|
||||
if cards.len() >= RAIL_MIN_CARDS && rail_w > 0.0 {
|
||||
let all_starved = cards.iter().all(|c| {
|
||||
let demand = fixed_content_demand(c);
|
||||
demand > 0.0
|
||||
&& demand <= rail_w * RAIL_CARD_MAX_FRACTION
|
||||
&& c.get("id")
|
||||
.and_then(Value::as_str)
|
||||
.and_then(|id| rects.get(id))
|
||||
.is_some_and(|r| demand > r.w + RAIL_STARVE_EPS)
|
||||
.is_some_and(|r| {
|
||||
r.w < RAIL_CARD_STARVED_W && demand > r.w + RAIL_STARVE_EPS
|
||||
})
|
||||
});
|
||||
if all_starved {
|
||||
for c in &cards {
|
||||
|
|
@ -1290,6 +1310,9 @@ fn collect_absolute_fill_image_fixes(
|
|||
/// height. (A wrapper that already fits, or crops only mildly, is left
|
||||
/// alone - intentional letterboxing stays intentional.)
|
||||
const IMAGE_BAND_MIN_VISIBLE_FRACTION: f64 = 0.5;
|
||||
/// Tallest a grown photo band may be relative to its own width — a card photo
|
||||
/// is at most mildly portrait; beyond this the "photo" is an oversized plate.
|
||||
const IMAGE_BAND_MAX_ASPECT: f64 = 1.2;
|
||||
|
||||
fn is_image_filled_frame(v: &Value) -> bool {
|
||||
matches!(
|
||||
|
|
@ -1315,7 +1338,11 @@ fn fixed_height(v: &Value) -> Option<f64> {
|
|||
}
|
||||
}
|
||||
|
||||
fn collect_image_slot_fixes(v: &Value, cmds: &mut Vec<EditorCommand>) {
|
||||
fn collect_image_slot_fixes(
|
||||
v: &Value,
|
||||
rects: &HashMap<String, Rect>,
|
||||
cmds: &mut Vec<EditorCommand>,
|
||||
) {
|
||||
let kids = children(v);
|
||||
let image_nodes: Vec<&Value> = kids
|
||||
.iter()
|
||||
|
|
@ -1353,7 +1380,19 @@ fn collect_image_slot_fixes(v: &Value, cmds: &mut Vec<EditorCommand>) {
|
|||
.filter_map(|f| fixed_height(f))
|
||||
.fold(0.0, f64::max)
|
||||
};
|
||||
if photo_h > 0.0 && band < photo_h * IMAGE_BAND_MIN_VISIBLE_FRACTION {
|
||||
// The photo's declared height is only a credible BAND height if the
|
||||
// band would still read as a card photo. A plate taller than its own
|
||||
// band is wide is an oversized image (a 400x300 plate in a phone
|
||||
// card), not art direction — growing the band to it left Deals cards
|
||||
// with a wall of empty space (user report 2026-07-12).
|
||||
let band_w = v
|
||||
.get("id")
|
||||
.and_then(Value::as_str)
|
||||
.and_then(|id| rects.get(id))
|
||||
.map(|r| r.w)
|
||||
.unwrap_or(0.0);
|
||||
let plausible = band_w > 0.0 && photo_h <= band_w * IMAGE_BAND_MAX_ASPECT;
|
||||
if photo_h > 0.0 && plausible && band < photo_h * IMAGE_BAND_MIN_VISIBLE_FRACTION {
|
||||
if let Some(id) = v.get("id").and_then(Value::as_str) {
|
||||
cmds.push(EditorCommand::UpdateNode {
|
||||
node_id: NodeId::new(id.to_string()),
|
||||
|
|
@ -1369,7 +1408,7 @@ fn collect_image_slot_fixes(v: &Value, cmds: &mut Vec<EditorCommand>) {
|
|||
}
|
||||
}
|
||||
for c in kids {
|
||||
collect_image_slot_fixes(c, cmds);
|
||||
collect_image_slot_fixes(c, rects, cmds);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2242,7 +2242,8 @@ fn double_image_slot() -> serde_json::Value {
|
|||
#[test]
|
||||
fn duplicate_image_plate_is_dropped_and_the_band_keeps_the_photo_height() {
|
||||
let mut cmds = Vec::new();
|
||||
collect_image_slot_fixes(&double_image_slot(), &mut cmds);
|
||||
let slot = double_image_slot();
|
||||
collect_image_slot_fixes(&slot, &slot_rects(&slot), &mut cmds);
|
||||
|
||||
assert!(
|
||||
cmds.iter().any(|c| matches!(c,
|
||||
|
|
@ -2278,7 +2279,7 @@ fn a_lone_image_filled_plate_is_never_deleted() {
|
|||
]
|
||||
});
|
||||
let mut cmds = Vec::new();
|
||||
collect_image_slot_fixes(&slot, &mut cmds);
|
||||
collect_image_slot_fixes(&slot, &slot_rects(&slot), &mut cmds);
|
||||
assert!(
|
||||
cmds.is_empty(),
|
||||
"the only image in the slot stays: {cmds:?}"
|
||||
|
|
@ -2300,7 +2301,7 @@ fn an_image_filled_hero_with_content_on_it_is_not_a_duplicate_plate() {
|
|||
]
|
||||
});
|
||||
let mut cmds = Vec::new();
|
||||
collect_image_slot_fixes(&slot, &mut cmds);
|
||||
collect_image_slot_fixes(&slot, &slot_rects(&slot), &mut cmds);
|
||||
assert!(
|
||||
!cmds
|
||||
.iter()
|
||||
|
|
@ -2322,6 +2323,130 @@ fn a_mild_crop_stays_intentional() {
|
|||
]
|
||||
});
|
||||
let mut cmds = Vec::new();
|
||||
collect_image_slot_fixes(&slot, &mut cmds);
|
||||
collect_image_slot_fixes(&slot, &slot_rects(&slot), &mut cmds);
|
||||
assert!(cmds.is_empty(), "a mild crop is left alone: {cmds:?}");
|
||||
}
|
||||
|
||||
/// Resolved rects for a slot fixture: the band spans a 200px card, children
|
||||
/// take their authored size (enough for the aspect guard to reason about).
|
||||
fn slot_rects(slot: &serde_json::Value) -> std::collections::HashMap<String, Rect> {
|
||||
fn walk(v: &serde_json::Value, out: &mut std::collections::HashMap<String, Rect>, w: f64) {
|
||||
if let Some(id) = v.get("id").and_then(|x| x.as_str()) {
|
||||
let cw = v.get("width").and_then(|x| x.as_f64()).unwrap_or(w);
|
||||
let ch = v.get("height").and_then(|x| x.as_f64()).unwrap_or(120.0);
|
||||
out.insert(
|
||||
id.to_string(),
|
||||
Rect {
|
||||
x: 0.0,
|
||||
y: 0.0,
|
||||
w: cw,
|
||||
h: ch,
|
||||
},
|
||||
);
|
||||
}
|
||||
for c in v
|
||||
.get("children")
|
||||
.and_then(|c| c.as_array())
|
||||
.map(Vec::as_slice)
|
||||
.unwrap_or(&[])
|
||||
{
|
||||
walk(c, out, w);
|
||||
}
|
||||
}
|
||||
let mut out = std::collections::HashMap::new();
|
||||
walk(slot, &mut out, 200.0);
|
||||
out
|
||||
}
|
||||
|
||||
/// The regression that wrecked a GOOD design: a 400x300 plate cropped inside a
|
||||
/// phone card is an oversized image, NOT the card's width intent. Widening the
|
||||
/// cards to it blew one card across the whole screen and clipped the rest.
|
||||
#[test]
|
||||
fn an_oversized_plate_is_not_a_width_intent_for_the_rail() {
|
||||
let cards: Vec<serde_json::Value> = (0..4)
|
||||
.map(|i| {
|
||||
json!({
|
||||
"type": "frame", "id": format!("card{i}"), "width": "fill_container",
|
||||
"height": "fill_container", "layout": "vertical", "clipContent": true,
|
||||
"children": [
|
||||
{ "type": "frame", "id": format!("img{i}"), "width": "fill_container",
|
||||
"height": 200, "clipContent": true, "children": [
|
||||
{ "type": "image", "id": format!("plate{i}"), "width": 400, "height": 300,
|
||||
"src": "data:image/jpeg;base64,AAAA" }
|
||||
]}
|
||||
]
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
let rail = json!({
|
||||
"type": "frame", "id": "rail", "width": "fill_container",
|
||||
"layout": "horizontal", "gap": 12, "children": cards
|
||||
});
|
||||
let mut rects = std::collections::HashMap::new();
|
||||
rects.insert(
|
||||
"rail".to_string(),
|
||||
Rect {
|
||||
x: 0.0,
|
||||
y: 0.0,
|
||||
w: 390.0,
|
||||
h: 260.0,
|
||||
},
|
||||
);
|
||||
for i in 0..4 {
|
||||
// Healthy cards: two visible per screen, the rest scroll.
|
||||
rects.insert(
|
||||
format!("card{i}"),
|
||||
Rect {
|
||||
x: i as f64 * 182.0,
|
||||
y: 0.0,
|
||||
w: 170.0,
|
||||
h: 260.0,
|
||||
},
|
||||
);
|
||||
}
|
||||
let mut cmds = Vec::new();
|
||||
collect_starved_rail_card_fixes(&rail, &rects, &mut cmds);
|
||||
assert!(
|
||||
cmds.is_empty(),
|
||||
"a 170px card cropping a 400px plate is fine — leave it alone: {cmds:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Same guard on the band: growing a 200px band to a 300px plate's height
|
||||
/// left a wall of empty space under the photo in every deal card.
|
||||
#[test]
|
||||
fn an_oversized_plate_does_not_stretch_the_photo_band() {
|
||||
let slot = json!({
|
||||
"type": "frame", "id": "band", "width": "fill_container", "height": 120,
|
||||
"clipContent": true,
|
||||
"children": [
|
||||
{ "type": "image", "id": "plate", "width": 400, "height": 300,
|
||||
"src": "data:image/jpeg;base64,AAAA" }
|
||||
]
|
||||
});
|
||||
let mut rects = std::collections::HashMap::new();
|
||||
rects.insert(
|
||||
"band".to_string(),
|
||||
Rect {
|
||||
x: 0.0,
|
||||
y: 0.0,
|
||||
w: 170.0,
|
||||
h: 120.0,
|
||||
},
|
||||
);
|
||||
rects.insert(
|
||||
"plate".to_string(),
|
||||
Rect {
|
||||
x: 0.0,
|
||||
y: 0.0,
|
||||
w: 400.0,
|
||||
h: 300.0,
|
||||
},
|
||||
);
|
||||
let mut cmds = Vec::new();
|
||||
collect_image_slot_fixes(&slot, &rects, &mut cmds);
|
||||
assert!(
|
||||
cmds.is_empty(),
|
||||
"a 300px plate in a 170px-wide band is oversized, not a band height: {cmds:?}"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue