fix(mcp): harden batch design and image semantics

This commit is contained in:
Fini 2026-07-14 00:27:55 +08:00
parent 44a4fe4e24
commit eea8901042
17 changed files with 2283 additions and 456 deletions

View file

@ -1,6 +1,7 @@
//! Background image-search enrichment for generated image nodes.
use std::collections::{HashMap, HashSet};
use std::hash::{Hash, Hasher};
use std::sync::mpsc::{self, Receiver, TryRecvError};
use std::sync::{Arc, Mutex};
use std::time::Duration;
@ -145,12 +146,21 @@ pub(crate) struct ImageSearchTarget {
/// AI-generation prompt bound to the node (`image_prompt`), if any. Used when
/// an image-gen model is configured; falls back to `query`.
pub prompt: Option<String>,
/// Explicit `G()` acquisition mode, or Auto for legacy/heuristic slots.
pub mode: ImageRequestMode,
/// Resolved numeric dimensions (for the gen provider's aspect mapping).
pub width: Option<f64>,
pub height: Option<f64>,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum ImageRequestMode {
Auto,
Search,
Generate,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(crate) enum ImageAspectRatio {
Wide,
Tall,
@ -191,9 +201,26 @@ impl OpenverseCredentials {
struct ImageSearchJob {
node_id: NodeId,
/// Exact node intent at enqueue time. Production jobs always set this;
/// hand-built unit jobs may omit it when testing unrelated bookkeeping.
intent: Option<String>,
rx: Receiver<Option<String>>,
}
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
struct SearchIntentKey {
query: String,
aspect_ratio: Option<ImageAspectRatio>,
}
enum SearchMemoEntry {
Pending {
request_id: u64,
waiters: Vec<mpsc::Sender<Option<String>>>,
},
Ready(String),
}
#[derive(Default)]
pub(crate) struct ImageSearchSession {
in_flight: HashSet<String>,
@ -224,26 +251,64 @@ pub(crate) struct ImageSearchSession {
/// target is already known via `in_flight`/`completed`).
#[cfg(test)]
scan_count: u32,
/// Result URLs already used this session — similar queries ("playlist
/// cover daily mix" / "... chill vibes" / "... discover weekly") share
/// their Openverse top hit, which filled three different cards with the
/// SAME photo (measured: test0711-22). Selection skips these best-effort.
/// Canonical provider identities plus compact content digests already used
/// this session. Similar queries otherwise share their Openverse top hit
/// and fill several cards with the same artwork (measured: test0711-22).
/// Never stores full embedded data URLs.
used_urls: Arc<Mutex<HashSet<String>>>,
/// Query → the photo that query already resolved to, this session.
/// Full stock-search intent → pending waiters or the resolved photo.
///
/// The dedup above must NOT fire when the SAME subject comes back: the
/// model rebuilds a section mid-run (fresh node ids), the same query
/// ("Bali Indonesia") searches again, and its own good photo is now in
/// `used_urls` — so the second search skipped it and took a junk result
/// the used-image set — so the second search skipped it and took a junk result
/// instead (measured 2026-07-12: a real Bali temple photo turned into a
/// plain blue sky halfway through a run). One subject, one photo: a repeat
/// query resolves from this memo with no network call at all.
resolved: Arc<Mutex<HashMap<String, String>>>,
search_memo: Arc<Mutex<HashMap<SearchIntentKey, SearchMemoEntry>>>,
/// Monotonic identity for a memo fetch. It is deliberately not reset:
/// an old network thread may finish after `reset()` and after the new
/// document has enqueued the same intent. Matching the request id avoids
/// that old completion consuming the new waiters (the classic ABA race).
next_search_request_id: u64,
}
/// Memo key — the subject, not its spelling.
fn query_key(query: &str) -> String {
query.trim().to_ascii_lowercase()
/// Memo key for the authored stock-search intent.
///
/// This deliberately does NOT use `simplify_search_query`: that function is a
/// lossy provider adapter (it drops words such as `album` / `cover` and caps
/// the request at four keywords). Those transformations are useful for a
/// photo corpus, but they must not make two distinct authored subjects share a
/// cached image or make the stale-result guard treat a changed intent as the
/// same intent. Case, punctuation, and repeated whitespace are canonicalized;
/// every authored word remains part of identity. Aspect remains part of intent
/// so a square cover never reuses a wide hero.
fn search_intent_key(query: &str, aspect_ratio: Option<ImageAspectRatio>) -> SearchIntentKey {
SearchIntentKey {
query: canonical_search_intent_query(query),
aspect_ratio,
}
}
fn canonical_search_intent_query(query: &str) -> String {
let mut canonical = String::with_capacity(query.len());
let mut pending_separator = false;
for character in query.trim().to_lowercase().chars() {
if character.is_alphanumeric() {
if pending_separator && !canonical.is_empty() {
canonical.push(' ');
}
canonical.push(character);
pending_separator = false;
} else {
pending_separator = true;
}
}
if canonical.is_empty() {
query.trim().to_lowercase()
} else {
canonical
}
}
impl ImageSearchSession {
@ -268,8 +333,11 @@ impl ImageSearchSession {
self.completed.clear();
self.jobs.clear();
self.invalidate_scan_gate();
self.used_urls.lock().unwrap().clear();
self.resolved.lock().unwrap().clear();
// Replace the generations, do not merely clear them. Detached network
// threads still own the old Arcs and may finish after reset; writing to
// those abandoned maps must not contaminate the replacement document.
self.used_urls = Arc::new(Mutex::new(HashSet::new()));
self.search_memo = Arc::new(Mutex::new(HashMap::new()));
}
pub(crate) fn is_pending(&self) -> bool {
@ -317,23 +385,30 @@ impl ImageSearchSession {
if targets.is_empty() {
return false;
}
// Strategy: a configured image-GEN model wins (generate from the bound
// `image_prompt`); otherwise fall back to stock SEARCH (Openverse). The
// prompt/query stay on the node either way, so the UI can re-gen/re-search
// and a later config change re-resolves on the next enqueue.
// An explicit G(...,"search"|"generate",...) mode wins. Legacy and
// heuristic slots remain Auto: configured generation first, otherwise
// stock search. A generate request without a configured provider fails
// visibly; it is never silently changed into a stock-photo request.
let gen_profile = crate::image_panel_host::active_image_gen_profile(state).cloned();
let credentials = OpenverseCredentials::from_state(state);
for target in targets {
let id = target.node_id.as_str().to_string();
self.in_flight.insert(id);
let job = match &gen_profile {
Some(profile) => spawn_gen_job(target, profile.clone()),
None => spawn_job(
target,
credentials.clone(),
Arc::clone(&self.used_urls),
Arc::clone(&self.resolved),
),
let job = match (target.mode, &gen_profile) {
(ImageRequestMode::Generate, Some(profile))
| (ImageRequestMode::Auto, Some(profile)) => spawn_gen_job(target, profile.clone()),
(ImageRequestMode::Generate, None) => spawn_unavailable_gen_job(target),
(ImageRequestMode::Search, _) | (ImageRequestMode::Auto, None) => {
let request_id = self.next_search_request_id;
self.next_search_request_id = self.next_search_request_id.wrapping_add(1);
spawn_job(
target,
credentials.clone(),
Arc::clone(&self.used_urls),
Arc::clone(&self.search_memo),
request_id,
)
}
};
self.jobs.push(job);
}
@ -357,6 +432,15 @@ impl ImageSearchSession {
// revision, but the gate invalidation still covers the
// failure path.)
self.last_scanned = None;
if job.intent.as_ref().is_some_and(|expected| {
current_intent_fingerprint(state, &job.node_id).as_ref() != Some(expected)
}) {
tracing::info!(
node_id = %job.node_id,
"discarding stale image result because the node's image intent changed"
);
continue;
}
// Ours: a failed search lands the theme-adaptive dashed
// placeholder (sentinel src) instead of a bare grey box.
let url = url.unwrap_or_else(|| SEARCH_FAILED_PLACEHOLDER_SRC.to_string());
@ -391,18 +475,46 @@ fn spawn_job(
target: ImageSearchTarget,
credentials: Option<OpenverseCredentials>,
used_urls: Arc<Mutex<HashSet<String>>>,
resolved: Arc<Mutex<HashMap<String, String>>>,
search_memo: Arc<Mutex<HashMap<SearchIntentKey, SearchMemoEntry>>>,
request_id: u64,
) -> ImageSearchJob {
let (tx, rx) = mpsc::channel();
let node_id = target.node_id.clone();
let aspect_ratio = target.aspect_ratio;
let key = query_key(&target.query);
// One subject, one photo: a query this session already answered resolves
// from the memo — no network, and (crucially) no dedup-forced downgrade
// when a rebuilt section asks for the same picture again.
if let Some(url) = resolved.lock().unwrap().get(&key).cloned() {
let _ = tx.send(Some(url));
return ImageSearchJob { node_id, rx };
let key = search_intent_key(&target.query, aspect_ratio);
let intent = Some(intent_fingerprint(&target, None));
// One full search intent, one in-flight request and one session result.
// Rebuilt nodes subscribe to the same pending request instead of racing
// duplicate searches; completed intents return from the memo.
{
let mut memo = search_memo.lock().unwrap();
match memo.get_mut(&key) {
Some(SearchMemoEntry::Ready(url)) => {
let _ = tx.send(Some(url.clone()));
return ImageSearchJob {
node_id,
intent,
rx,
};
}
Some(SearchMemoEntry::Pending { waiters, .. }) => {
waiters.push(tx);
return ImageSearchJob {
node_id,
intent,
rx,
};
}
None => {
memo.insert(
key.clone(),
SearchMemoEntry::Pending {
request_id,
waiters: vec![tx],
},
);
}
}
}
std::thread::spawn(move || {
let url = fetch_first_image_url_blocking(
@ -411,12 +523,48 @@ fn spawn_job(
credentials.as_ref(),
&used_urls,
);
if let Some(found) = url.as_ref() {
resolved.lock().unwrap().insert(key, found.clone());
}
let _ = tx.send(url);
publish_search_result(&search_memo, key, request_id, url);
});
ImageSearchJob { node_id, rx }
ImageSearchJob {
node_id,
intent,
rx,
}
}
/// Publish only into the exact Pending entry that launched this request. A
/// reset may remove it and a new document may insert the same key before the
/// old thread returns; the request id keeps that old result isolated.
fn publish_search_result(
search_memo: &Arc<Mutex<HashMap<SearchIntentKey, SearchMemoEntry>>>,
key: SearchIntentKey,
request_id: u64,
url: Option<String>,
) -> bool {
let waiters = {
let mut memo = search_memo.lock().unwrap();
let matches_request = matches!(
memo.get(&key),
Some(SearchMemoEntry::Pending {
request_id: pending_id,
..
}) if *pending_id == request_id
);
if !matches_request {
return false;
}
let Some(SearchMemoEntry::Pending { waiters, .. }) = memo.remove(&key) else {
unreachable!("request identity was checked under the same lock")
};
if let Some(found) = url.as_ref() {
memo.insert(key, SearchMemoEntry::Ready(found.clone()));
}
waiters
};
for waiter in waiters {
let _ = waiter.send(url.clone());
}
true
}
/// Enrich via the configured image-GEN model instead of stock search. Prefers the
@ -426,6 +574,7 @@ fn spawn_job(
fn spawn_gen_job(target: ImageSearchTarget, profile: ImageGenProfile) -> ImageSearchJob {
let (tx, rx) = mpsc::channel();
let node_id = target.node_id.clone();
let intent = Some(intent_fingerprint(&target, Some(&profile)));
std::thread::spawn(move || {
let prompt = target
.prompt
@ -441,44 +590,93 @@ fn spawn_gen_job(target: ImageSearchTarget, profile: ImageGenProfile) -> ImageSe
.ok();
let _ = tx.send(url);
});
ImageSearchJob { node_id, rx }
ImageSearchJob {
node_id,
intent,
rx,
}
}
fn spawn_unavailable_gen_job(target: ImageSearchTarget) -> ImageSearchJob {
let (tx, rx) = mpsc::channel();
let node_id = target.node_id.clone();
let intent = Some(intent_fingerprint(&target, None));
let _ = tx.send(None);
ImageSearchJob {
node_id,
intent,
rx,
}
}
fn intent_fingerprint(target: &ImageSearchTarget, profile: Option<&ImageGenProfile>) -> String {
let generate = target.mode == ImageRequestMode::Generate
|| (target.mode == ImageRequestMode::Auto && profile.is_some());
if generate {
let (profile_id, model) = profile
.map(|profile| (profile.id.as_str(), profile.model.as_str()))
.unwrap_or(("unconfigured", "unconfigured"));
format!(
"generate|{profile_id}|{model}|{}|{:?}|{:?}",
target
.prompt
.as_deref()
.filter(|prompt| !prompt.trim().is_empty())
.unwrap_or(target.query.as_str())
.trim(),
target.width.map(f64::to_bits),
target.height.map(f64::to_bits)
)
} else {
let key = search_intent_key(&target.query, target.aspect_ratio);
format!("search|{}|{:?}", key.query, key.aspect_ratio)
}
}
fn current_intent_fingerprint(state: &EditorState, node_id: &NodeId) -> Option<String> {
let target = collect_targets(state, &HashSet::new())
.into_iter()
.find(|target| &target.node_id == node_id)?;
let profile = crate::image_panel_host::active_image_gen_profile(state);
Some(intent_fingerprint(&target, profile))
}
pub(crate) fn collect_targets(
state: &EditorState,
known_node_ids: &HashSet<String>,
) -> Vec<ImageSearchTarget> {
let rects = resolved_sizes(state);
let resolved_sizes = resolved_node_sizes(state);
let mut targets = Vec::new();
collect_from_children(
state.active_children(),
known_node_ids,
&rects,
&resolved_sizes,
&mut targets,
&[],
&[],
);
targets
}
/// Every node's RESOLVED size, from the real layout pass.
///
/// Name-and-authored-size heuristics are blind to the shape weak models
/// actually ship: DeepSeek built every card's photo area as an UNNAMED
/// rectangle sized `fill_container` x `fill_container`, so it carried neither
/// a keyword nor a number and the whole page came out as grey boxes (measured
/// test0711-1-ds, 2026-07-12). What a slot IS is a question about geometry —
/// so ask the layout.
fn resolved_sizes(state: &EditorState) -> HashMap<String, (f32, f32)> {
/// Resolved node dimensions from the real layout pass. This map is used only
/// after a node has independently qualified as media; geometry never turns an
/// anonymous surface into an image target. It lets a G()-created fill/fill
/// child inherit the actual slot aspect for search, generation, and stale-job
/// detection instead of losing that intent as `None`.
fn resolved_node_sizes(state: &EditorState) -> HashMap<String, (f64, f64)> {
let scene = op_pen_loader::editor_state_to_layout_scene(state);
let mut out = HashMap::new();
fn walk(
nodes: &[op_editor_ui::layout_scene::SceneNode],
out: &mut HashMap<String, (f32, f32)>,
out: &mut HashMap<String, (f64, f64)>,
) {
for node in nodes {
let bounds = node.aggregate_bounds();
out.insert(node.id.clone(), (bounds.size.x, bounds.size.y));
if bounds.size.x > 0.0 && bounds.size.y > 0.0 {
out.insert(
node.id.clone(),
(f64::from(bounds.size.x), f64::from(bounds.size.y)),
);
}
walk(&node.children, out);
}
}
@ -488,78 +686,37 @@ fn resolved_sizes(state: &EditorState) -> HashMap<String, (f32, f32)> {
out
}
/// The smallest box that reads as a picture rather than a swatch or a rule.
const RESOLVED_SLOT_MIN_W: f32 = 80.0;
const RESOLVED_SLOT_MIN_H: f32 = 60.0;
/// An EMPTY, painted box that resolved to picture size, carries no name of its
/// own (or only a generic one), and sits in a card that has words in it — the
/// card's words say what the picture is. Geometry decides; the name is only
/// allowed to VETO (a box called "Divider" is not a photo).
fn is_resolved_media_slot(
node: &PenNode,
rects: &HashMap<String, (f32, f32)>,
sibling_text: &[String],
) -> bool {
let (base, container) = match node {
PenNode::Frame(f) => (&f.base, &f.container),
PenNode::Rectangle(r) => (&r.base, &r.container),
_ => return false,
};
if sibling_text.is_empty() {
return false;
}
if node.children().is_some_and(|kids| !kids.is_empty()) {
return false;
}
if !matches!(container.fill.as_deref(), Some([PenFill::Solid(_)])) {
return false;
}
if let Some(name) = base.name.as_deref().map(str::trim) {
if !name.is_empty() && !is_generic_placeholder_name(name) && !has_image_area_keyword(name) {
return false;
}
}
rects
.get(base.id.as_str())
.is_some_and(|(w, h)| *w >= RESOLVED_SLOT_MIN_W && *h >= RESOLVED_SLOT_MIN_H)
}
fn collect_from_children(
children: &[PenNode],
known_node_ids: &HashSet<String>,
rects: &HashMap<String, (f32, f32)>,
resolved_sizes: &HashMap<String, (f64, f64)>,
targets: &mut Vec<ImageSearchTarget>,
parent_names: &[String],
inherited_text: &[String],
) {
// Sibling text of a bare anonymous slot IS its subject ("Blinding
// Lights" next to a nameless 120px square = that track's cover).
// Direct sibling text of a bare anonymous slot may name its subject
// ("Blinding Lights" next to a nameless 120px square = that track's
// cover). Do not search sibling container subtrees or inherit text across
// levels: at a rail/list boundary those words belong to cousin cards, not
// to this slot.
for (index, node) in children.iter().enumerate() {
// The words that name a picture are rarely the slot's literal siblings:
// a card is [photo band, info frame] and the title lives INSIDE the
// info frame (measured test0711-1-ds — every photo area came out
// contextless and the page shipped as grey boxes). Take the words from
// the OTHER siblings' subtrees; a slot alone in its band inherits its
// card's words. Cousin cards are never consulted — that would name the
// Bali card's photo "Santorini".
let mut context: Vec<String> = Vec::new();
for (other_index, other) in children.iter().enumerate() {
if other_index == index {
continue;
}
collect_text_from_subtree(other, 3, &mut context);
if context.len() >= 2 {
break;
}
}
context.truncate(2);
if context.is_empty() {
context = inherited_text.to_vec();
}
let context: Vec<String> = children
.iter()
.enumerate()
.filter(|(other_index, _)| *other_index != index)
.filter_map(|(_, other)| match other {
PenNode::Text(text) => match &text.content {
TextContent::Plain(value) if !value.trim().is_empty() => {
Some(value.trim().to_string())
}
_ => None,
},
_ => None,
})
.take(2)
.collect();
if let Some(target) =
image_search_target_for(node, known_node_ids, rects, parent_names, &context)
image_search_target_for(node, known_node_ids, resolved_sizes, parent_names, &context)
{
targets.push(target);
}
@ -577,40 +734,18 @@ fn collect_from_children(
collect_from_children(
grand,
known_node_ids,
rects,
resolved_sizes,
targets,
&child_parent_names,
&context,
);
}
}
}
fn collect_text_from_subtree(node: &PenNode, depth: usize, out: &mut Vec<String>) {
if out.len() >= 2 {
return;
}
if let PenNode::Text(t) = node {
if let TextContent::Plain(text) = &t.content {
let trimmed = text.trim();
if !trimmed.is_empty() {
out.push(trimmed.to_string());
}
}
return;
}
if depth == 0 {
return;
}
for child in node.children().into_iter().flatten() {
collect_text_from_subtree(child, depth - 1, out);
}
}
fn image_search_target_for(
node: &PenNode,
known_node_ids: &HashSet<String>,
rects: &HashMap<String, (f32, f32)>,
resolved_sizes: &HashMap<String, (f64, f64)>,
parent_names: &[String],
sibling_text: &[String],
) -> Option<ImageSearchTarget> {
@ -624,7 +759,8 @@ fn image_search_target_for(
// way with no names and no G() bindings (measured test0711-2-ds); the
// sibling text is the only, and a good, subject source.
let bare_slot_with_context = !sibling_text.is_empty()
&& (is_bare_anonymous_slot(node) || is_resolved_media_slot(node, rects, sibling_text));
&& !has_non_media_context(parent_names)
&& is_bare_anonymous_slot(node);
let needs_image = match node {
PenNode::Image(image) => is_placeholder_src(&image.src),
PenNode::Frame(_) => is_frame_placeholder_still_unfilled(node) || bare_slot_with_context,
@ -677,13 +813,62 @@ fn image_search_target_for(
PenNode::Image(image) => image.image_prompt.clone(),
_ => None,
};
let mode = match node {
// Legacy / script-generated Image nodes intentionally carry both
// fields: generation uses the richer prompt when a profile exists,
// otherwise stock search falls back to the query. `G("search")` and
// `G("generate")` remain unambiguous because they emit only one field.
PenNode::Image(image)
if image
.image_prompt
.as_deref()
.is_some_and(|prompt| !prompt.trim().is_empty())
&& image
.image_search_query
.as_deref()
.is_some_and(|query| !query.trim().is_empty()) =>
{
ImageRequestMode::Auto
}
PenNode::Image(image)
if image
.image_prompt
.as_deref()
.is_some_and(|prompt| !prompt.trim().is_empty()) =>
{
ImageRequestMode::Generate
}
PenNode::Image(image)
if image
.image_search_query
.as_deref()
.is_some_and(|query| !query.trim().is_empty()) =>
{
ImageRequestMode::Search
}
PenNode::Frame(frame)
if frame
.image_search_query
.as_deref()
.is_some_and(|query| !query.trim().is_empty()) =>
{
ImageRequestMode::Search
}
_ => ImageRequestMode::Auto,
};
let (width, height) = resolved_sizes
.get(id)
.copied()
.map(|(width, height)| (Some(width), Some(height)))
.unwrap_or_else(|| (node.width_px(), node.height_px()));
Some(ImageSearchTarget {
node_id: NodeId::new(id),
query,
aspect_ratio: infer_aspect_ratio(node),
aspect_ratio: infer_aspect_ratio(width, height),
prompt,
width: node.width_px(),
height: node.height_px(),
mode,
width,
height,
})
}
@ -794,6 +979,9 @@ fn is_unnamed_media_slot_in_context(node: &PenNode, parent_names: &[String]) ->
let PenNode::Rectangle(rect) = node else {
return false;
};
if has_non_media_context(parent_names) {
return false;
}
if rect
.base
.name
@ -826,6 +1014,40 @@ fn is_unnamed_media_slot_in_context(node: &PenNode, parent_names: &[String]) ->
})
}
/// Explicit structural/control vocabulary vetoes the anonymous-slot fallback.
/// These surfaces often have the same small rounded solid geometry as cover
/// art, but filling a KPI, swatch, badge, or button with a photo is always a
/// worse failure than leaving an ambiguous box untouched. Explicit image
/// nodes, placeholder roles, and media-named slots do not depend on this
/// fallback and remain eligible.
fn has_non_media_context(parent_names: &[String]) -> bool {
const NON_MEDIA_WORDS: [&str; 18] = [
"kpi",
"metric",
"stat",
"stats",
"analytics",
"chart",
"graph",
"swatch",
"palette",
"button",
"control",
"badge",
"indicator",
"progress",
"separator",
"divider",
"toggle",
"status",
];
parent_names.iter().any(|name| {
name.to_ascii_lowercase()
.split(|character: char| !character.is_ascii_alphanumeric())
.any(|token| NON_MEDIA_WORDS.contains(&token))
})
}
fn is_image_area_rectangle_by_heuristic(node: &PenNode) -> bool {
let PenNode::Rectangle(rect) = node else {
return false;
@ -872,14 +1094,8 @@ fn image_area_dimension_ok(size: &Option<SizingBehavior>, min_px: f64) -> (bool,
}
}
fn infer_aspect_ratio(node: &PenNode) -> Option<ImageAspectRatio> {
let (width, height) = match node {
PenNode::Image(image) => (&image.width, &image.height),
PenNode::Frame(frame) => (&frame.container.width, &frame.container.height),
PenNode::Rectangle(rect) => (&rect.container.width, &rect.container.height),
_ => return None,
};
let (Some(width), Some(height)) = (dimension_number(width), dimension_number(height)) else {
fn infer_aspect_ratio(width: Option<f64>, height: Option<f64>) -> Option<ImageAspectRatio> {
let (Some(width), Some(height)) = (width, height) else {
return None;
};
if width <= 0.0 || height <= 0.0 {
@ -903,6 +1119,21 @@ fn dimension_number(size: &Option<SizingBehavior>) -> Option<f64> {
}
fn has_image_area_keyword(name: &str) -> bool {
let compact: String = name
.chars()
.filter(|character| character.is_ascii_alphanumeric())
.flat_map(char::to_lowercase)
.collect();
if compact.ends_with("img")
|| compact.ends_with("image")
|| compact.ends_with("photo")
|| compact.ends_with("cover")
|| compact.ends_with("thumbnail")
|| compact.ends_with("artwork")
|| compact.ends_with("media")
{
return true;
}
name.split(|c: char| !c.is_ascii_alphanumeric())
.map(str::to_ascii_lowercase)
.any(|word| {
@ -1162,16 +1393,12 @@ fn fetch_first_image_url_blocking(
.enable_all()
.build()
.ok()?;
let picked = runtime.block_on(fetch_first_image_url(
runtime.block_on(fetch_first_image_url(
query,
aspect_ratio,
credentials,
used_urls,
));
if let Some(url) = picked.as_ref() {
used_urls.lock().unwrap().insert(url.clone());
}
picked
))
}
async fn fetch_first_image_url(
@ -1198,11 +1425,11 @@ async fn fetch_first_image_url(
{
return Some(url);
}
if let Some(url) = fetch_wikimedia(&client, &truncated).await {
if let Some(url) = fetch_wikimedia(&client, &truncated, used_urls).await {
return Some(url);
}
}
fetch_wikimedia(&client, &query).await
fetch_wikimedia(&client, &query, used_urls).await
}
pub(crate) fn simplify_search_query(prompt: &str) -> String {
@ -1261,8 +1488,7 @@ async fn fetch_openverse(
}
let json: serde_json::Value = resp.json().await.ok()?;
let results = json.get("results")?.as_array()?;
let used = used_urls.lock().unwrap().clone();
let result = select_openverse_result(results, query, &used)?;
let (result, identity) = claim_openverse_result(results, query, used_urls)?;
let mut candidates = Vec::new();
push_candidate_url(
&mut candidates,
@ -1272,7 +1498,8 @@ async fn fetch_openverse(
&mut candidates,
result.get("url").and_then(serde_json::Value::as_str),
);
first_renderable_image_src(client, candidates).await
let outcome = first_unused_renderable_image_src(client, candidates, used_urls).await;
settle_provider_identity(used_urls, &identity, outcome)
}
/// Titles that mark a result as noise no matter how well it ranks — the
@ -1309,12 +1536,15 @@ pub(crate) fn select_openverse_result<'results>(
used_urls: &HashSet<String>,
) -> Option<&'results serde_json::Value> {
let is_used = |result: &serde_json::Value| {
["url", "thumbnail"].iter().any(|key| {
result
.get(*key)
.and_then(serde_json::Value::as_str)
.is_some_and(|url| used_urls.contains(url))
})
openverse_result_identity(result).is_some_and(|identity| used_urls.contains(&identity))
// Backward-compatible URL keys keep the pure selector useful to
// callers/tests, but production claims canonical result identity.
|| ["url", "thumbnail"].iter().any(|key| {
result
.get(*key)
.and_then(serde_json::Value::as_str)
.is_some_and(|url| used_urls.contains(url))
})
};
let non_junk: Vec<&serde_json::Value> = results
.iter()
@ -1342,6 +1572,36 @@ pub(crate) fn select_openverse_result<'results>(
.or_else(|| non_junk.first().copied())
}
fn openverse_result_identity(result: &serde_json::Value) -> Option<String> {
if let Some(id) = result.get("id") {
if let Some(id) = id.as_str().filter(|id| !id.trim().is_empty()) {
return Some(format!("openverse:{id}"));
}
if id.is_number() {
return Some(format!("openverse:{id}"));
}
}
["url", "thumbnail"].iter().find_map(|field| {
result
.get(*field)
.and_then(serde_json::Value::as_str)
.map(str::trim)
.filter(|url| !url.is_empty())
.map(|url| format!("openverse-url:{url}"))
})
}
fn claim_openverse_result<'results>(
results: &'results [serde_json::Value],
query: &str,
used_images: &Mutex<HashSet<String>>,
) -> Option<(&'results serde_json::Value, String)> {
let mut used = used_images.lock().unwrap();
let result = select_openverse_result(results, query, &used)?;
let identity = openverse_result_identity(result)?;
used.insert(identity.clone()).then_some((result, identity))
}
fn openverse_search_url(
query: &str,
aspect_ratio: Option<ImageAspectRatio>,
@ -1384,7 +1644,11 @@ pub(crate) async fn fetch_openverse_token(
.map(str::to_string)
}
async fn fetch_wikimedia(client: &reqwest::Client, query: &str) -> Option<String> {
async fn fetch_wikimedia(
client: &reqwest::Client,
query: &str,
used_urls: &Mutex<HashSet<String>>,
) -> Option<String> {
let url = reqwest::Url::parse_with_params(
"https://commons.wikimedia.org/w/api.php",
&[
@ -1408,28 +1672,55 @@ async fn fetch_wikimedia(client: &reqwest::Client, query: &str) -> Option<String
let json: serde_json::Value = resp.json().await.ok()?;
let pages = json.get("query")?.get("pages")?.as_object()?;
for page in pages.values() {
if let Some(info) = page
.get("imageinfo")
.and_then(serde_json::Value::as_array)
.and_then(|items| items.first())
{
let mut candidates = Vec::new();
push_candidate_url(
&mut candidates,
info.get("thumburl").and_then(serde_json::Value::as_str),
);
push_candidate_url(
&mut candidates,
info.get("url").and_then(serde_json::Value::as_str),
);
if let Some(src) = first_renderable_image_src(client, candidates).await {
return Some(src);
}
let Some(identity) = wikimedia_page_identity(page) else {
continue;
};
if !used_urls.lock().unwrap().insert(identity.clone()) {
continue;
}
// An empty candidate list deliberately settles as Unavailable below,
// releasing the reservation for pages with missing/empty imageinfo.
let candidates = wikimedia_image_candidates(page);
let outcome = first_unused_renderable_image_src(client, candidates, used_urls).await;
if let Some(src) = settle_provider_identity(used_urls, &identity, outcome) {
return Some(src);
}
}
None
}
fn wikimedia_page_identity(page: &serde_json::Value) -> Option<String> {
if let Some(page_id) = page.get("pageid") {
if page_id.is_number() || page_id.is_string() {
return Some(format!("wikimedia:{page_id}"));
}
}
page.get("title")
.and_then(serde_json::Value::as_str)
.map(str::trim)
.filter(|title| !title.is_empty())
.map(|title| format!("wikimedia-title:{title}"))
}
fn wikimedia_image_candidates(page: &serde_json::Value) -> Vec<String> {
let mut candidates = Vec::new();
if let Some(info) = page
.get("imageinfo")
.and_then(serde_json::Value::as_array)
.and_then(|items| items.first())
{
push_candidate_url(
&mut candidates,
info.get("thumburl").and_then(serde_json::Value::as_str),
);
push_candidate_url(
&mut candidates,
info.get("url").and_then(serde_json::Value::as_str),
);
}
candidates
}
fn push_candidate_url(candidates: &mut Vec<String>, url: Option<&str>) {
let Some(url) = url.map(str::trim).filter(|url| !url.is_empty()) else {
return;
@ -1439,16 +1730,66 @@ fn push_candidate_url(candidates: &mut Vec<String>, url: Option<&str>) {
}
}
async fn first_renderable_image_src(
client: &reqwest::Client,
candidates: Vec<String>,
#[derive(Debug, PartialEq, Eq)]
enum ImageCandidateClaim {
/// A renderable image won the session-wide content claim.
Claimed(String),
/// A renderable download matched content another provider result already
/// owns. The provider identity stays used because its artwork is known to
/// be a duplicate even though this request cannot return it.
Duplicate,
/// No candidate URL produced a renderable image. The provider reservation
/// must be released so a later request can retry a transient failure.
Unavailable,
}
fn settle_provider_identity(
used_urls: &Mutex<HashSet<String>>,
identity: &str,
outcome: ImageCandidateClaim,
) -> Option<String> {
for candidate in candidates {
if let Some(src) = fetch_image_data_url(client, &candidate).await {
return Some(src);
match outcome {
ImageCandidateClaim::Claimed(src) => Some(src),
ImageCandidateClaim::Duplicate => None,
ImageCandidateClaim::Unavailable => {
used_urls.lock().unwrap().remove(identity);
None
}
}
None
}
async fn first_unused_renderable_image_src(
client: &reqwest::Client,
candidates: Vec<String>,
used_urls: &Mutex<HashSet<String>>,
) -> ImageCandidateClaim {
let mut found_duplicate = false;
for candidate in candidates {
if let Some(src) = fetch_image_data_url(client, &candidate).await {
// Claim the embedded result under one lock. Different queries run
// concurrently and may resolve to the same underlying image; a
// snapshot-then-insert check lets both win. The atomic claim lets
// the loser continue to another candidate/fallback instead.
if claim_unused_image_src(used_urls, &src) {
return ImageCandidateClaim::Claimed(src);
}
found_duplicate = true;
}
}
if found_duplicate {
ImageCandidateClaim::Duplicate
} else {
ImageCandidateClaim::Unavailable
}
}
fn claim_unused_image_src(used_urls: &Mutex<HashSet<String>>, src: &str) -> bool {
let mut hasher = std::collections::hash_map::DefaultHasher::new();
src.hash(&mut hasher);
used_urls
.lock()
.unwrap()
.insert(format!("content:{:016x}", hasher.finish()))
}
pub(crate) async fn fetch_image_data_url(client: &reqwest::Client, url: &str) -> Option<String> {

View file

@ -434,6 +434,7 @@ fn poll_into_applies_finished_job_to_placeholder_frame() {
completed: HashSet::new(),
jobs: vec![ImageSearchJob {
node_id: NodeId::new("photo"),
intent: None,
rx,
}],
..Default::default()
@ -472,6 +473,7 @@ fn successful_apply_does_not_suppress_later_unfilled_retry() {
completed: HashSet::new(),
jobs: vec![ImageSearchJob {
node_id: NodeId::new("photo"),
intent: None,
rx,
}],
..Default::default()
@ -529,6 +531,7 @@ fn poll_into_completion_invalidates_scan_gate() {
completed: HashSet::new(),
jobs: vec![ImageSearchJob {
node_id: NodeId::new("img1"),
intent: None,
rx,
}],
..Default::default()
@ -553,6 +556,96 @@ fn poll_into_completion_invalidates_scan_gate() {
);
}
#[test]
fn poll_discards_a_result_when_the_nodes_image_intent_changed() {
let mut state = EditorState::default();
state.active_children_mut().clear();
state
.active_children_mut()
.push(image_node("img1", "", Some("burger fries")));
let original = collect_targets(&state, &HashSet::new())
.into_iter()
.next()
.expect("original target");
let expected = intent_fingerprint(&original, None);
let PenNode::Image(image) = &mut state.active_children_mut()[0] else {
panic!("image")
};
image.image_search_query = Some("latte cup".into());
state.mark_document_changed();
let (tx, rx) = std::sync::mpsc::channel();
tx.send(Some("https://stale.example.com/burger.jpg".to_string()))
.unwrap();
let mut session = ImageSearchSession {
in_flight: HashSet::from(["img1".to_string()]),
jobs: vec![ImageSearchJob {
node_id: NodeId::new("img1"),
intent: Some(expected),
rx,
}],
..Default::default()
};
assert!(!session.poll_into(&mut state));
let PenNode::Image(image) = &state.active_children()[0] else {
panic!("image")
};
assert!(image.src.is_empty(), "stale URL must not land");
assert!(!session.completed.contains("img1"));
}
#[test]
fn poll_discards_a_result_when_only_provider_truncated_words_changed() {
let before = "santorini greece white buildings blue dome";
let after = "santorini greece white buildings sunset beach";
assert_eq!(
simplify_search_query(before),
simplify_search_query(after),
"the provider intentionally sees the same four-keyword request"
);
let mut state = EditorState::default();
state.active_children_mut().clear();
state
.active_children_mut()
.push(image_node("img1", "", Some(before)));
let original = collect_targets(&state, &HashSet::new())
.into_iter()
.next()
.expect("original target");
let expected = intent_fingerprint(&original, None);
let PenNode::Image(image) = &mut state.active_children_mut()[0] else {
panic!("image")
};
image.image_search_query = Some(after.into());
state.mark_document_changed();
let (tx, rx) = std::sync::mpsc::channel();
tx.send(Some("https://stale.example.com/blue-dome.jpg".to_string()))
.unwrap();
let mut session = ImageSearchSession {
in_flight: HashSet::from(["img1".to_string()]),
jobs: vec![ImageSearchJob {
node_id: NodeId::new("img1"),
intent: Some(expected),
rx,
}],
..Default::default()
};
assert!(!session.poll_into(&mut state));
let PenNode::Image(image) = &state.active_children()[0] else {
panic!("image")
};
assert!(
image.src.is_empty(),
"the provider-level collision must not weaken authored intent identity"
);
}
#[test]
fn reset_invalidates_scan_gate() {
let mut state = EditorState::default();
@ -737,6 +830,7 @@ fn document_replacement_reset_drops_stale_pending_job_so_it_cannot_apply_to_new_
completed: HashSet::new(),
jobs: vec![ImageSearchJob {
node_id: NodeId::new("photo"),
intent: None,
rx,
}],
..Default::default()
@ -903,6 +997,7 @@ fn failed_search_writes_the_adaptive_placeholder_sentinel() {
in_flight: HashSet::from(["img1".to_string()]),
jobs: vec![ImageSearchJob {
node_id: NodeId::new("img1"),
intent: None,
rx,
}],
..Default::default()
@ -1016,6 +1111,81 @@ fn anonymous_cover_slot_uses_sibling_text_as_query() {
);
}
#[test]
fn anonymous_slot_does_not_borrow_text_from_a_cousin_card() {
let mut slot = frame_node("slot", "", None, Some(vec![solid_fill()]), vec![]);
if let PenNode::Frame(frame) = &mut slot {
frame.base.name = None;
frame.container.width = Some(SizingBehavior::Number(120.0));
frame.container.height = Some(SizingBehavior::Number(120.0));
frame.container.clip_content = Some(true);
}
let mut empty_card = frame_node("empty-card", "", None, None, vec![slot]);
if let PenNode::Frame(frame) = &mut empty_card {
frame.base.name = None;
}
let other_card = frame_node(
"other-card",
"Santorini Card",
None,
None,
vec![frame_node(
"other-info",
"Info",
None,
None,
vec![text_label("other-title", None, "Santorini, Greece")],
)],
);
let rail = frame_node(
"rail",
"Destination Rail",
None,
None,
vec![empty_card, other_card],
);
let mut state = EditorState::default();
state.active_children_mut().clear();
state.active_children_mut().push(rail);
let targets = collect_targets(&state, &HashSet::new());
assert!(
targets
.iter()
.all(|target| target.node_id.as_str() != "slot"),
"an unlabelled card must stay empty, not borrow its cousin's title: {targets:?}"
);
}
#[test]
fn rounded_kpi_tile_is_not_an_anonymous_image_slot() {
let mut tile = frame_node("tile", "", None, Some(vec![solid_fill()]), vec![]);
if let PenNode::Frame(frame) = &mut tile {
frame.base.name = None;
frame.container.width = Some(SizingBehavior::Number(64.0));
frame.container.height = Some(SizingBehavior::Number(64.0));
frame.container.corner_radius = Some(jian_ops_schema::node::CornerRadius::Uniform(16.0));
}
let card = frame_node(
"kpi",
"Revenue KPI Card",
None,
None,
vec![tile, text_label("label", None, "Monthly revenue")],
);
let mut state = EditorState::default();
state.active_children_mut().clear();
state.active_children_mut().push(card);
let targets = collect_targets(&state, &HashSet::new());
assert!(
targets
.iter()
.all(|target| target.node_id.as_str() != "tile"),
"rounded KPI geometry plus a label is not media intent: {targets:?}"
);
}
/// The measured churn: the model rebuilds a section mid-run, the same subject
/// searches again, and the session-wide dedup skips the very photo it picked
/// the first time — so a real Bali temple photo became a plain blue sky. One
@ -1023,30 +1193,40 @@ fn anonymous_cover_slot_uses_sibling_text_as_query() {
/// only ever guards DIFFERENT subjects from sharing a picture.
#[test]
fn a_repeat_query_gets_the_same_photo_back_not_a_dedup_downgrade() {
use super::{query_key, spawn_job, ImageSearchTarget};
use super::{
search_intent_key, spawn_job, ImageSearchTarget, SearchIntentKey, SearchMemoEntry,
};
use std::collections::{HashMap, HashSet};
use std::sync::{Arc, Mutex};
let used_urls: Arc<Mutex<HashSet<String>>> = Arc::new(Mutex::new(HashSet::new()));
let resolved: Arc<Mutex<HashMap<String, String>>> = Arc::new(Mutex::new(HashMap::new()));
let resolved: Arc<Mutex<HashMap<SearchIntentKey, SearchMemoEntry>>> =
Arc::new(Mutex::new(HashMap::new()));
// The first search already answered "Bali Indonesia" and marked its photo used.
let good = "https://example.org/bali-temple.jpg".to_string();
resolved
.lock()
.unwrap()
.insert(query_key("Bali Indonesia"), good.clone());
resolved.lock().unwrap().insert(
search_intent_key("Bali, Indonesia", None),
SearchMemoEntry::Ready(good.clone()),
);
used_urls.lock().unwrap().insert(good.clone());
// The rebuilt card asks again — differently spelled, same subject.
let target = ImageSearchTarget {
node_id: op_editor_core::NodeId::new("n99".to_string()),
query: " bali indonesia ".to_string(),
query: " bali indonesia ".to_string(),
prompt: None,
mode: ImageRequestMode::Search,
aspect_ratio: None,
width: None,
height: None,
};
let job = spawn_job(target, None, Arc::clone(&used_urls), Arc::clone(&resolved));
let job = spawn_job(
target,
None,
Arc::clone(&used_urls),
Arc::clone(&resolved),
1,
);
let answer = job
.rx
.recv_timeout(std::time::Duration::from_secs(1))
@ -1058,6 +1238,203 @@ fn a_repeat_query_gets_the_same_photo_back_not_a_dedup_downgrade() {
);
}
#[test]
fn a_pending_search_intent_is_singleflight() {
use super::{
search_intent_key, spawn_job, ImageSearchTarget, SearchIntentKey, SearchMemoEntry,
};
use std::collections::{HashMap, HashSet};
use std::sync::{Arc, Mutex};
let key = search_intent_key("Bali Indonesia", None);
let memo: Arc<Mutex<HashMap<SearchIntentKey, SearchMemoEntry>>> =
Arc::new(Mutex::new(HashMap::from([(
key.clone(),
SearchMemoEntry::Pending {
request_id: 7,
waiters: Vec::new(),
},
)])));
let target = ImageSearchTarget {
node_id: NodeId::new("n100"),
query: "Bali, Indonesia".into(),
prompt: None,
mode: ImageRequestMode::Search,
aspect_ratio: None,
width: None,
height: None,
};
let job = spawn_job(
target,
None,
Arc::new(Mutex::new(HashSet::new())),
Arc::clone(&memo),
8,
);
let waiters = match memo.lock().unwrap().remove(&key) {
Some(SearchMemoEntry::Pending { waiters, .. }) => waiters,
_ => panic!("the second caller joins the pending intent"),
};
assert_eq!(waiters.len(), 1, "no second fetch thread was created");
waiters[0]
.send(Some("https://example.org/bali.jpg".into()))
.unwrap();
assert_eq!(
job.rx.recv_timeout(Duration::from_secs(1)).unwrap(),
Some("https://example.org/bali.jpg".into())
);
}
#[test]
fn stale_pre_reset_request_cannot_publish_into_same_key_in_new_session() {
use super::{publish_search_result, search_intent_key, SearchIntentKey, SearchMemoEntry};
use std::collections::HashMap;
use std::sync::{mpsc, Arc, Mutex};
let key = search_intent_key("Bali Indonesia", None);
let memo: Arc<Mutex<HashMap<SearchIntentKey, SearchMemoEntry>>> =
Arc::new(Mutex::new(HashMap::new()));
let (old_tx, _old_rx) = mpsc::channel();
memo.lock().unwrap().insert(
key.clone(),
SearchMemoEntry::Pending {
request_id: 10,
waiters: vec![old_tx],
},
);
// reset(), followed by a new document asking for the same intent.
memo.lock().unwrap().clear();
let (new_tx, new_rx) = mpsc::channel();
memo.lock().unwrap().insert(
key.clone(),
SearchMemoEntry::Pending {
request_id: 11,
waiters: vec![new_tx],
},
);
assert!(!publish_search_result(
&memo,
key.clone(),
10,
Some("https://old.example/bali.jpg".into())
));
assert!(matches!(new_rx.try_recv(), Err(mpsc::TryRecvError::Empty)));
assert!(publish_search_result(
&memo,
key,
11,
Some("https://new.example/bali.jpg".into())
));
assert_eq!(
new_rx.recv_timeout(Duration::from_secs(1)).unwrap(),
Some("https://new.example/bali.jpg".into())
);
}
#[test]
fn reset_detaches_dedup_and_memo_generations_from_old_threads() {
let mut session = ImageSearchSession::default();
let old_used = std::sync::Arc::clone(&session.used_urls);
let old_memo = std::sync::Arc::clone(&session.search_memo);
old_used.lock().unwrap().insert("openverse:old".into());
session.reset();
assert!(!std::sync::Arc::ptr_eq(&old_used, &session.used_urls));
assert!(!std::sync::Arc::ptr_eq(&old_memo, &session.search_memo));
old_used
.lock()
.unwrap()
.insert("openverse:late-old-thread".into());
assert!(
session.used_urls.lock().unwrap().is_empty(),
"late old-document claims stay in the detached generation"
);
}
#[test]
fn search_memo_separates_aspect_ratio_intents() {
use super::{search_intent_key, ImageAspectRatio};
assert_eq!(
search_intent_key("Bali, Indonesia", Some(ImageAspectRatio::Square)),
search_intent_key("bali indonesia", Some(ImageAspectRatio::Square)),
"fetch-equivalent spelling shares one intent"
);
assert_ne!(
search_intent_key("Bali Indonesia", Some(ImageAspectRatio::Square)),
search_intent_key("Bali Indonesia", Some(ImageAspectRatio::Wide)),
"a cover and hero must not share a cached crop"
);
}
#[test]
fn memo_identity_keeps_lossy_provider_query_collisions_separate() {
let album = "album cover neon lights night";
let playlist = "playlist artwork neon lights night";
assert_eq!(
simplify_search_query(album),
simplify_search_query(playlist),
"both authored intents intentionally adapt to the same photo-corpus query"
);
assert_ne!(
search_intent_key(album, Some(ImageAspectRatio::Square)),
search_intent_key(playlist, Some(ImageAspectRatio::Square)),
"provider adaptation must not merge authored image identity"
);
let dome = "santorini greece white buildings blue dome";
let beach = "santorini greece white buildings sunset beach";
assert_eq!(simplify_search_query(dome), simplify_search_query(beach));
assert_ne!(
search_intent_key(dome, Some(ImageAspectRatio::Wide)),
search_intent_key(beach, Some(ImageAspectRatio::Wide)),
"words beyond the provider's four-keyword cap remain part of identity"
);
}
#[test]
fn g_style_fill_image_uses_resolved_parent_slot_aspect() {
let state_for_slot = |width: f64, height: f64| {
let doc: jian_ops_schema::PenDocument = serde_json::from_value(serde_json::json!({
"version":"1.0", "children":[{
"type":"frame", "id":"root", "width":600, "height":400, "layout":"vertical",
"children":[{
"type":"frame", "id":"slot", "name":"Bali Hero", "width":width,
"height":height, "layout":"vertical", "clipContent":true, "children":[{
"type":"image", "id":"photo", "name":"Bali Indonesia", "src":"",
"imagePrompt":"Bali Indonesia", "width":"fill_container",
"height":"fill_container", "objectFit":"crop"
}]
}]
}]
}))
.expect("G-shaped document");
EditorState::from_document(doc)
};
let wide = collect_targets(&state_for_slot(320.0, 180.0), &HashSet::new())
.into_iter()
.find(|target| target.node_id.as_str() == "photo")
.expect("wide image target");
let square = collect_targets(&state_for_slot(180.0, 180.0), &HashSet::new())
.into_iter()
.find(|target| target.node_id.as_str() == "photo")
.expect("square image target");
assert_eq!(wide.aspect_ratio, Some(ImageAspectRatio::Wide));
assert_eq!(square.aspect_ratio, Some(ImageAspectRatio::Square));
assert_eq!((wide.width, wide.height), (Some(320.0), Some(180.0)));
assert_ne!(
intent_fingerprint(&wide, None),
intent_fingerprint(&square, None),
"a parent-slot aspect change invalidates the in-flight search"
);
}
/// MiniMax-M3 builds every card around a RECTANGLE named "img" (or a "ph"
/// rectangle inside a frame named "img") — neither word was in the keyword
/// table, so a whole page of destination cards shipped as grey boxes with no
@ -1113,13 +1490,11 @@ fn m3_style_img_and_ph_rectangles_are_image_slots() {
assert_eq!(by_id.get("img"), Some(&"Bali, Indonesia"));
}
/// DeepSeek builds a card's photo area as an UNNAMED rectangle sized
/// `fill_container` x `fill_container` — no keyword, no number — so the
/// name-and-authored-size heuristics saw nothing and the page shipped as grey
/// boxes (measured test0711-1-ds, 2026-07-12). What a slot IS is a question
/// about geometry: the resolved layout answers it.
/// Geometry alone cannot distinguish a photo slot from a chart, swatch, or
/// decorative surface. The in-loop diagnostic can ask the model about it, but
/// background enrichment requires explicit media semantics.
#[test]
fn an_unnamed_fill_container_rectangle_in_a_card_is_an_image_slot() {
fn an_unnamed_fill_container_rectangle_is_not_auto_filled_from_geometry() {
let doc: jian_ops_schema::PenDocument = serde_json::from_str(
r##"{ "version": "1.0", "children": [{
"type": "frame", "id": "root", "width": 390, "height": 844, "layout": "vertical",
@ -1142,14 +1517,11 @@ fn an_unnamed_fill_container_rectangle_in_a_card_is_an_image_slot() {
.expect("parse");
let state = op_editor_core::EditorState::from_document(doc);
let targets = super::collect_targets(&state, &std::collections::HashSet::new());
let slot = targets
.iter()
.find(|t| t.node_id.as_str() == "slot")
.expect("the photo area is a slot: {targets:?}");
assert!(
slot.query.contains("Santorini") || slot.query.contains("Card"),
"the card's own words say what the picture is: {}",
slot.query
targets
.iter()
.all(|target| target.node_id.as_str() != "slot"),
"an unnamed solid box needs an explicit role/name/query: {targets:?}"
);
}
@ -1178,6 +1550,159 @@ fn a_thin_divider_rectangle_is_not_an_image_slot() {
);
}
#[test]
fn image_fields_preserve_search_generate_and_legacy_auto_modes() {
let doc: jian_ops_schema::PenDocument = serde_json::from_str(
r##"{ "version":"1.0", "children":[{
"type":"frame", "id":"root", "width":390, "height":844, "layout":"vertical",
"children":[
{"type":"image", "id":"search", "name":"Search Photo", "src":"",
"imageSearchQuery":"Kyoto temple", "width":160, "height":90},
{"type":"image", "id":"generate", "name":"Generated Art", "src":"",
"imagePrompt":"surreal Kyoto at dusk", "width":160, "height":90},
{"type":"image", "id":"legacy-auto", "name":"Compatible Art", "src":"",
"imageSearchQuery":"Kyoto dusk", "imagePrompt":"surreal Kyoto at dusk",
"width":160, "height":90}
]
}] }"##,
)
.expect("parse");
let state = op_editor_core::EditorState::from_document(doc);
let targets = collect_targets(&state, &std::collections::HashSet::new());
assert_eq!(
targets
.iter()
.find(|target| target.node_id.as_str() == "search")
.expect("search target")
.mode,
ImageRequestMode::Search
);
assert_eq!(
targets
.iter()
.find(|target| target.node_id.as_str() == "generate")
.expect("generate target")
.mode,
ImageRequestMode::Generate
);
assert_eq!(
targets
.iter()
.find(|target| target.node_id.as_str() == "legacy-auto")
.expect("legacy auto target")
.mode,
ImageRequestMode::Auto
);
}
#[test]
fn image_result_claim_is_atomic_across_queries() {
let used = std::sync::Arc::new(std::sync::Mutex::new(std::collections::HashSet::new()));
let winners = (0..8)
.map(|_| {
let used = std::sync::Arc::clone(&used);
std::thread::spawn(move || claim_unused_image_src(&used, "data:image/png;base64,SAME"))
})
.map(|thread| thread.join().expect("claim thread"))
.filter(|claimed| *claimed)
.count();
assert_eq!(winners, 1, "only one concurrent query may claim an image");
assert!(
used.lock()
.unwrap()
.iter()
.all(|key| key.starts_with("content:") && !key.contains("base64")),
"dedup stores compact digests, not multi-megabyte data URIs"
);
}
#[test]
fn provider_identity_reservation_releases_only_unavailable_downloads() {
let used = std::sync::Mutex::new(std::collections::HashSet::from([
"openverse:unavailable".to_string(),
"openverse:claimed".to_string(),
"openverse:duplicate".to_string(),
]));
assert_eq!(
settle_provider_identity(
&used,
"openverse:unavailable",
ImageCandidateClaim::Unavailable,
),
None
);
assert_eq!(
settle_provider_identity(
&used,
"openverse:claimed",
ImageCandidateClaim::Claimed("data:image/png;base64,OK".into()),
),
Some("data:image/png;base64,OK".into())
);
assert_eq!(
settle_provider_identity(&used, "openverse:duplicate", ImageCandidateClaim::Duplicate,),
None
);
let used = used.lock().unwrap();
assert!(!used.contains("openverse:unavailable"));
assert!(used.contains("openverse:claimed"));
assert!(
used.contains("openverse:duplicate"),
"a successfully downloaded duplicate remains excluded"
);
}
#[test]
fn wikimedia_missing_or_empty_imageinfo_releases_the_page_reservation() {
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect("runtime");
let client = reqwest::Client::new();
for page in [
serde_json::json!({"pageid": 41, "title": "Missing imageinfo"}),
serde_json::json!({"pageid": 42, "title": "Empty imageinfo", "imageinfo": []}),
] {
let identity = wikimedia_page_identity(&page).expect("page identity");
let used = std::sync::Mutex::new(std::collections::HashSet::from([identity.clone()]));
let candidates = wikimedia_image_candidates(&page);
assert!(candidates.is_empty());
let outcome = runtime.block_on(first_unused_renderable_image_src(
&client, candidates, &used,
));
assert_eq!(outcome, ImageCandidateClaim::Unavailable);
assert_eq!(settle_provider_identity(&used, &identity, outcome), None);
assert!(
!used.lock().unwrap().contains(&identity),
"an unusable page must remain retryable"
);
}
}
#[test]
fn openverse_claim_uses_artwork_identity_not_thumbnail_variant() {
let used = std::sync::Mutex::new(std::collections::HashSet::new());
let first = vec![serde_json::json!({
"id":"art-42", "title":"Kyoto temple", "thumbnail":"https://x/thumb-400.jpg",
"url":"https://x/full.jpg"
})];
let resized = vec![serde_json::json!({
"id":"art-42", "title":"Kyoto temple", "thumbnail":"https://x/thumb-800.jpg",
"url":"https://x/full.jpg"
})];
assert!(claim_openverse_result(&first, "Kyoto temple", &used).is_some());
assert!(
claim_openverse_result(&resized, "Kyoto temple", &used).is_none(),
"one artwork id owns all thumbnail/full-size URL variants"
);
}
/// Forensic: `OP_SLOT_PROBE=<path.op>` prints every slot the enrichment pass
/// would enqueue for a saved document, with the query it would search.
#[test]

View file

@ -91,7 +91,7 @@ pub const TOOL_SCHEMAS: &[&str] = &[
r#"{"name":"paste_clipboard","description":"Cmd+V parity. Pastes the document clipboard as top-level siblings on the active page, offset by offset_px doc-px (defaults to 10). Mints fresh ids past max_node_id(). Replaces selection with the new ids. Apply-time false when the clipboard is empty or id-space is exhausted.","inputSchema":{"type":"object","properties":{"offset_px":{"type":"string","description":"i32 doc-px offset; defaults to 10 when omitted"}},"required":[]}}"#,
// --- write tools ---
r##"{"name":"set_variable_color","description":"Set a Color-kind variable's value.","inputSchema":{"type":"object","properties":{"name":{"type":"string"},"hex":{"type":"string","description":"#rgb / #rrggbb / #rrggbbaa"}},"required":["name","hex"]}}"##,
r#"{"name":"batch_design","description":"Insert or refine design content. Accepts nodes_json, I(parent,nodeJson) insert operations, multi-line I/U/C/R/G/M/D programs, or a sandboxed JavaScript program via script. Transactional: if any operation fails, NONE of the batch is applied (the document is unchanged) and errors[] lists every failing line — fix them and resend the whole batch. Keep each call to <=25 operations; split large screens into logical section batches.","inputSchema":{"type":"object","properties":{"filePath":{"type":"string","description":"Optional target .op file path; omit to use the server document"},"nodes_json":{"type":"string","description":"JSON array of simple leaf descriptors"},"operations":{"type":"string","description":"TS batch_design DSL, e.g. root=I(null,{...}), U(\"n1\",{\"x\":10}), D(\"n1\"), M(\"n1\",null), copy=C(root,null,{...}), img=G(\"root\",\"search\",\"prompt\")"},"script":{"type":"string","description":"JavaScript program run in a sandboxed QuickJS: build nodes by calling I(parent, obj) (loops and data arrays allowed; C/U/D/M/R/G and console are no-op stubs). Expands to the operations DSL. Mutually exclusive with nodes_json/operations. Limits: 256 KiB source, 4096 inserts, 2s, 64 MiB, 8 MiB recorded output."},"postProcess":{"type":"boolean"},"canvasWidth":{"type":"number"},"pageId":{"type":"string"}}}}"#,
r#"{"name":"batch_design","description":"Insert or refine design content. Accepts nodes_json, I(parent,nodeJson) insert operations, multi-line I/U/C/R/G/M/D programs, or a sandboxed JavaScript program via script. Transactional: if any operation fails, NONE of the batch is applied (the document is unchanged) and errors[] lists every failing line — fix them and resend the whole batch. Keep each call to <=25 operations; split large screens into logical section batches.","inputSchema":{"type":"object","properties":{"filePath":{"type":"string","description":"Optional target .op file path; omit to use the server document"},"nodes_json":{"type":"string","description":"JSON array of simple leaf descriptors"},"operations":{"type":"string","description":"TS batch_design DSL, e.g. root=I(null,{...}), U(\"n1\",{\"x\":10}), D(\"n1\"), M(\"n1\",null), copy=C(root,null,{...}), img=G(\"image-slot-id\",\"search\",\"prompt\") requires an existing EMPTY target, while img=G(\"rail-id\",\"search\",\"prompt\",\"append\") is accepted only when the parent declares horizontal/vertical layout and should be followed by U(img,{\"width\":120,\"height\":90}); null/populated slot targets and layout-none/omitted append targets are rejected"},"script":{"type":"string","description":"JavaScript program run in a sandboxed QuickJS: build nodes by calling I(parent, obj) or K(kitId,parent,overrides), with loops and data arrays allowed. C/U/D/M/R/G are rejected here with an instruction to use operations; console is a no-op. Expands to the operations DSL. Mutually exclusive with nodes_json/operations. Limits: 256 KiB source, 4096 inserts, 2s, 64 MiB, 8 MiB recorded output."},"postProcess":{"type":"boolean"},"canvasWidth":{"type":"number"},"pageId":{"type":"string"}}}}"#,
r#"{"name":"get_design_prompt","description":"Get OpenPencil design-generation prompt knowledge. Pass section for a focused subset; omit it for all sections. style and design-md are derived from the live document's design.md when present.","inputSchema":{"type":"object","properties":{"section":{"type":"string","description":"Prompt section name, e.g. all, layout, style, design-md, elements, codegen-react"},"filePath":{"type":"string","description":"Optional target .op file path; omit to use the server document"}},"required":[]}}"#,
r#"{"name":"design_skeleton","description":"Layered design workflow phase 1: create a root frame plus section frames. Accepts TS-style rootFrame/sections plus optional canvasWidth/pageId; legacy nodes_json/operations payloads remain accepted for compatibility.","inputSchema":{"type":"object","properties":{"filePath":{"type":"string","description":"Optional target .op file path; omit to use the server document"},"rootFrame":{"type":"object","description":"Root frame definition: name, width, height, layout, gap, fill, padding"},"sections":{"type":"array","description":"Section frame definitions; each item needs name and may include height, layout, padding, gap, fill, role, justifyContent, alignItems"},"styleGuide":{"type":"object","description":"Optional style guide metadata"},"canvasWidth":{"type":"number","description":"Canvas width for section contentWidth estimates"},"pageId":{"type":"string","description":"Target page ID or index"},"nodes_json":{"type":"string","description":"Legacy JSON array of simple leaf descriptors"},"operations":{"type":"string","description":"Legacy TS batch_design DSL"}},"required":["rootFrame","sections"]}}"#,
r#"{"name":"design_content","description":"Layered design workflow phase 2: fill child nodes into a section frame created by design_skeleton. Accepts TS-style sectionId/children plus optional postProcess/canvasWidth/pageId; legacy nodes_json/operations payloads remain accepted for compatibility.","inputSchema":{"type":"object","properties":{"filePath":{"type":"string","description":"Optional target .op file path; omit to use the server document"},"sectionId":{"type":"string","description":"ID of the section frame from design_skeleton"},"children":{"type":"array","description":"Child node definitions to insert under the section"},"postProcess":{"type":"boolean","description":"Apply post-processing after insertion; default true in the TS MCP"},"canvasWidth":{"type":"number","description":"Canvas width for post-processing; default 1200"},"pageId":{"type":"string","description":"Target page ID or index"},"nodes_json":{"type":"string","description":"Legacy JSON array of simple leaf descriptors"},"operations":{"type":"string","description":"Legacy TS batch_design DSL with I(parent,nodeJson) inserts or single U/D/M refine op"}},"required":["sectionId","children"]}}"#,

View file

@ -7,7 +7,7 @@ use jian_ops_schema::node::PenNode;
use jian_ops_schema::promote::{promote_frame, widget_kind_for, PromoteNote};
use op_editor_core::{NodeId, PenNodeExt};
use super::batch_direct_ops::parse_single_direct_operation;
use super::batch_direct_ops::{is_direct_image_operation, parse_single_direct_operation};
use super::batch_layered::{dispatch_design_content, dispatch_design_skeleton};
use super::batch_page::{command_with_outer_page_id, optional_page_id};
use super::write_tools::{validate_hex, ALLOWED_KINDS};
@ -61,9 +61,61 @@ pub(crate) fn carries_input(args: &BTreeMap<String, String>, key: &str) -> bool
})
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum BatchInputKind {
Script,
Operations,
NodesJson,
}
pub(crate) type BatchInputError = (ToolErrorCode, String);
/// Select the one non-empty write payload. Empty placeholders do not compete,
/// but two real payloads are always an error regardless of argument order.
pub(crate) fn select_batch_input(
args: &BTreeMap<String, String>,
) -> Result<BatchInputKind, BatchInputError> {
let active: Vec<BatchInputKind> = [
("script", BatchInputKind::Script),
("operations", BatchInputKind::Operations),
("nodes_json", BatchInputKind::NodesJson),
]
.into_iter()
.filter_map(|(key, kind)| carries_input(args, key).then_some(kind))
.collect();
match active.as_slice() {
[kind] => Ok(*kind),
[] => {
// Preserve a lone slot's own parser error (`nodes_json:{}` is
// malformed, `script:""` is empty) while treating placeholders
// as absent whenever another slot carries the real input.
let present: Vec<BatchInputKind> = [
("script", BatchInputKind::Script),
("operations", BatchInputKind::Operations),
("nodes_json", BatchInputKind::NodesJson),
]
.into_iter()
.filter_map(|(key, kind)| args.contains_key(key).then_some(kind))
.collect();
match present.as_slice() {
[kind] => Ok(*kind),
_ => Err((
ToolErrorCode::MissingArgument,
"one non-empty input is required: script, operations, or nodes_json".into(),
)),
}
}
_ => Err((
ToolErrorCode::InvalidArgument,
"provide only one of script, operations, or nodes_json".into(),
)),
}
}
/// Expand a `script` arg into the `operations` DSL program the rest of
/// `batch_design` already understands. Returns:
/// - `None` — `args` carries no `script` key; caller proceeds unchanged.
/// - `None` — the one active input is `operations` or `nodes_json`; an empty
/// `script` placeholder does not steal the route.
/// - `Some(Ok(rewritten))` — `script` removed, `operations` set to the
/// program the sandboxed runner recorded. Caller re-dispatches with the
/// rewritten args so BOTH the flat `dispatch_batch_design` path (used by
@ -72,22 +124,17 @@ pub(crate) fn carries_input(args: &BTreeMap<String, String>, key: &str) -> bool
/// (`BatchDesign::call`, which intercepts `operations` before ever
/// calling `dispatch_batch_design` — see `batch_design_result.rs`) see
/// the exact same expansion and report through their own native shape.
/// - `Some(Err(outcome))` — `script` combined with `operations`/
/// `nodes_json`, or (feature off) `script` used at all.
/// - `Some(Err(outcome))` — zero/multiple real inputs, or (feature off) a real
/// `script` input.
pub(crate) fn expand_script_arg(
args: &BTreeMap<String, String>,
) -> Option<Result<BTreeMap<String, String>, ToolOutcome>> {
let script = args.get("script")?;
// Only a COMPETING input is an error. Models routinely send the unused
// slots along as empty placeholders (`"operations": []`, `"nodes_json":
// ""`, a literal `null`) — rejecting those killed a whole batch over an
// empty field the caller never meant to fill (measured 2026-07-12).
if carries_input(args, "operations") || carries_input(args, "nodes_json") {
return Some(Err(ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
"provide only one of script, operations, or nodes_json".into(),
)));
match select_batch_input(args) {
Ok(BatchInputKind::Script) => {}
Ok(BatchInputKind::Operations | BatchInputKind::NodesJson) => return None,
Err((code, message)) => return Some(Err(ToolOutcome::Err(code, message))),
}
let script = args.get("script").expect("selected script exists");
#[cfg(feature = "script")]
{
let program = match crate::script_runner::run_script_to_program(script) {
@ -119,16 +166,21 @@ pub(crate) fn dispatch_batch_design(
Err(outcome) => outcome,
};
}
let input = match select_batch_input(args) {
Ok(input) => input,
Err((code, message)) => return ToolOutcome::Err(code, message),
};
let page_id = optional_page_id(args);
// An empty placeholder (`[]`, `""`, `null`) is not a choice of input: when
// the OTHER slot carries the real program, fall through to it. A lone empty
// slot still reports its own error (an empty descriptor list is a mistake,
// not a missing argument).
let nodes_json_is_real = carries_input(args, "nodes_json");
if let Some(operations) = args
.get("operations")
.filter(|_| carries_input(args, "operations") || !nodes_json_is_real)
{
if input == BatchInputKind::Operations {
let operations = args.get("operations").expect("selected operations exists");
if let Some(phase) = phase.filter(|_| is_direct_image_operation(operations)) {
return ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
format!(
"design_{phase} legacy operations cannot execute G() safely: this compatibility path has no document snapshot, so it cannot enforce G() placement or target geometry. Use batch_design with the same operations payload."
),
);
}
return match parse_operations(operations) {
Ok(ParsedOperations::Insert {
parent_id,
@ -171,12 +223,9 @@ pub(crate) fn dispatch_batch_design(
Err(e) => ToolOutcome::Err(ToolErrorCode::InvalidArgument, e),
};
}
let Some(raw) = args.get("nodes_json") else {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"nodes_json or operations is required".into(),
);
};
let raw = args
.get("nodes_json")
.expect("selected nodes_json exists after script expansion");
match parse_batch_items(raw) {
Ok(items) if items.is_empty() => ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
@ -645,7 +694,6 @@ pub(crate) fn normalize_node_shape(value: &mut serde_json::Value) {
// correctly, then every `U(n1,{layout:{type:horizontal…}})` was rejected and
// the tree thrashed to empty).
normalize_layout_object(obj);
default_container_layout(obj);
if let Some(fill) = obj.get_mut("fill") {
normalize_fill(fill);
}
@ -922,42 +970,6 @@ fn normalize_text_growth(obj: &mut serde_json::Map<String, serde_json::Value>) {
}
}
/// A container with flow children and NO `layout` renders as a ROW — the
/// engine's flex default. That is never what an omission means: a section
/// whose children are [title row, card rail] wants them STACKED, and models
/// that want a row always say so explicitly (measured test0711-1-ds: every
/// section frame omitted `layout`, so each title landed to the LEFT of its
/// rail and the cards ran off the screen). An omitted layout stacks.
///
/// Absolutely-positioned children are out of flow, so a frame whose children
/// all carry `x`/`y` is left alone — its direction is irrelevant, and forcing
/// one could contradict a `layout: none` the caller means to set later.
fn default_container_layout(obj: &mut serde_json::Map<String, serde_json::Value>) {
const CONTAINERS: [&str; 3] = ["frame", "group", "rectangle"];
let is_container = obj
.get("type")
.and_then(serde_json::Value::as_str)
.is_some_and(|t| CONTAINERS.contains(&t));
if !is_container || obj.contains_key("layout") {
return;
}
let flow_children = obj
.get("children")
.and_then(serde_json::Value::as_array)
.map(|kids| {
kids.iter()
.filter(|c| c.get("x").is_none_or(serde_json::Value::is_null))
.count()
})
.unwrap_or(0);
if flow_children >= 2 {
obj.insert(
"layout".to_string(),
serde_json::Value::String("vertical".to_string()),
);
}
}
fn normalize_layout_keyword(obj: &mut serde_json::Map<String, serde_json::Value>, key: &str) {
let Some(serde_json::Value::String(value)) = obj.get_mut(key) else {
return;

View file

@ -20,8 +20,10 @@ use op_editor_core::{EditorState, NodeId, PenNodeExt};
use serde_json::{json, Value};
use super::batch_design::{
carries_input, dispatch_batch_design, expand_script_arg, parse_operations, ParsedOperations,
dispatch_batch_design, expand_script_arg, parse_operations, select_batch_input, BatchInputKind,
ParsedOperations,
};
use super::batch_direct_ops::is_direct_image_operation;
use super::batch_page::optional_page_id;
use super::read_nodes::{page_nodes_snapshots, PageNodes};
use super::{EditorCommand, McpTool, ToolOutcome};
@ -59,16 +61,14 @@ impl McpTool for BatchDesign {
Err(outcome) => outcome,
};
}
// An EMPTY `operations` placeholder beside a real `nodes_json` is not a
// program — fall through to the flat dispatch, which picks the slot the
// caller actually filled (a lone empty program still errors there).
let Some(operations) = args
.get("operations")
.filter(|_| carries_input(args, "operations") || !carries_input(args, "nodes_json"))
else {
// nodes_json (a Rust convenience) / missing → flat dispatch.
return dispatch_batch_design(args, None);
let input = match select_batch_input(args) {
Ok(input) => input,
Err((code, message)) => return ToolOutcome::Err(code, message),
};
if input == BatchInputKind::NodesJson {
return dispatch_batch_design(args, None);
}
let operations = args.get("operations").expect("selected operations exists");
match parse_operations(operations) {
Ok(ParsedOperations::Insert {
parent_id,
@ -77,8 +77,13 @@ impl McpTool for BatchDesign {
bindings,
promoted,
}) => self.insert_with_result(args, parent_id, nodes, bindings, promoted),
// A single direct op keeps the flat dispatch (which re-parses
// and returns the existing command-per-op shape).
// G() needs the document snapshot to size the generated image to
// its target slot. The flat direct parser has no parent geometry,
// so route even a single G through the program simulator.
Ok(ParsedOperations::Direct(_)) if is_direct_image_operation(operations) => {
super::batch_program::run_batch_design_program(&self.state, operations, args)
}
// Other single direct ops keep the flat command-per-op shape.
Ok(ParsedOperations::Direct(_)) => dispatch_batch_design(args, None),
// Everything else — multi-line MIXED programs, per-line parse
// failures — runs the DSL program executor. Transactional by

View file

@ -5,7 +5,7 @@
//! `EditorState::apply` checks; the apply-path correctness is covered
//! by `op-editor-core`'s `command_tests.rs`.
use super::test_fixtures::sample;
use super::test_fixtures::{frame, sample, state_with};
use super::{BatchInsertItem, EditorCommand, McpTool, ToolErrorCode, ToolOutcome};
use crate::batch_design_snapshot;
use op_editor_core::PenNodeExt;
@ -880,62 +880,98 @@ fn batch_design_accepts_bound_single_replace_operation() {
#[test]
fn batch_design_accepts_single_image_operation_without_fetcher() {
let tool = batch_design_snapshot(&sample());
let state = state_with(vec![frame("slot", "Slot", 0.0, 0.0, 160.0, 90.0, vec![])]);
let tool = batch_design_snapshot(&state);
let mut args = BTreeMap::new();
args.insert(
"operations".into(),
r##"G("n10", "search", "hero product photo")"##.into(),
r##"G("slot", "search", "hero product photo")"##.into(),
);
match tool.call(&args) {
ToolOutcome::OkWithCommand(
ToolOutcome::OkJsonWithCommand(
result,
EditorCommand::InsertSubtree {
EditorCommand::InsertAuthoredSubtree {
nodes,
parent_id,
page_id,
},
) => {
assert_eq!(result.get("count"), Some(&"1".to_string()));
assert_eq!(parent_id.as_str(), "n10");
let result: serde_json::Value = serde_json::from_str(&result).unwrap();
assert_eq!(result["results"].as_array().map(Vec::len), Some(1));
assert_eq!(parent_id.as_str(), "slot");
assert!(page_id.is_none());
assert_eq!(nodes.len(), 1);
assert!(matches!(nodes[0], jian_ops_schema::node::PenNode::Image(_)));
let jian_ops_schema::node::PenNode::Image(image) = &nodes[0] else {
panic!("expected image")
};
assert!(matches!(
image.width,
Some(jian_ops_schema::sizing::SizingBehavior::Keyword(
jian_ops_schema::sizing::SizingKeyword::FillContainer
))
));
assert!(matches!(
image.height,
Some(jian_ops_schema::sizing::SizingBehavior::Keyword(
jian_ops_schema::sizing::SizingKeyword::FillContainer
))
));
assert!(matches!(
image.object_fit,
Some(jian_ops_schema::node::ImageFitMode::Crop)
));
assert_eq!(nodes[0].base().name.as_deref(), Some("hero product photo"));
}
other => panic!("expected image InsertSubtree command, got {other:?}"),
other => panic!("expected image InsertAuthoredSubtree command, got {other:?}"),
}
}
#[test]
fn batch_design_accepts_bound_single_image_operation_without_fetcher() {
let tool = batch_design_snapshot(&sample());
let state = state_with(vec![frame("slot", "Slot", 0.0, 0.0, 160.0, 90.0, vec![])]);
let tool = batch_design_snapshot(&state);
let mut args = BTreeMap::new();
args.insert(
"operations".into(),
r##"hero=G(null, "generate", "dashboard background")"##.into(),
r##"hero=G("slot", "generate", "dashboard background")"##.into(),
);
match tool.call(&args) {
ToolOutcome::OkWithCommand(
ToolOutcome::OkJsonWithCommand(
result,
EditorCommand::InsertSubtree {
EditorCommand::InsertAuthoredSubtree {
nodes,
parent_id,
page_id,
},
) => {
assert_eq!(result.get("count"), Some(&"1".to_string()));
assert!(!parent_id.is_real());
let result: serde_json::Value = serde_json::from_str(&result).unwrap();
assert_eq!(result["results"].as_array().map(Vec::len), Some(1));
assert_eq!(parent_id.as_str(), "slot");
assert!(page_id.is_none());
assert_eq!(nodes.len(), 1);
assert!(matches!(nodes[0], jian_ops_schema::node::PenNode::Image(_)));
let jian_ops_schema::node::PenNode::Image(image) = &nodes[0] else {
panic!("expected image")
};
assert!(matches!(
image.width,
Some(jian_ops_schema::sizing::SizingBehavior::Keyword(
jian_ops_schema::sizing::SizingKeyword::FillContainer
))
));
assert!(matches!(
image.height,
Some(jian_ops_schema::sizing::SizingBehavior::Keyword(
jian_ops_schema::sizing::SizingKeyword::FillContainer
))
));
assert_eq!(
nodes[0].base().name.as_deref(),
Some("dashboard background")
);
}
other => panic!("expected bound image InsertSubtree command, got {other:?}"),
other => panic!("expected bound image InsertAuthoredSubtree command, got {other:?}"),
}
}
@ -1466,11 +1502,47 @@ fn a_lone_empty_operations_list_still_reports_its_own_error() {
}
}
/// Measured test0711-1-ds: every section frame omitted `layout`, so the engine
/// laid [title, card rail] out as a ROW — each section title sat to the LEFT of
/// its cards and the rail ran off the screen. An omitted layout stacks.
#[test]
fn a_container_without_a_layout_stacks_its_children() {
fn two_real_batch_inputs_are_rejected_symmetrically() {
let tool = batch_design_snapshot(&sample());
let mut args = BTreeMap::new();
args.insert(
"operations".into(),
r#"root=I(null,{"type":"frame","width":100,"height":100})"#.into(),
);
args.insert(
"nodes_json".into(),
r#"[{"kind":"rect","name":"A","x":0,"y":0,"width":10,"height":10}]"#.into(),
);
match tool.call(&args) {
ToolOutcome::Err(ToolErrorCode::InvalidArgument, message) => {
assert!(message.contains("only one of"));
}
other => panic!("two real inputs must not silently prefer one: {other:?}"),
}
}
#[test]
fn an_empty_script_does_not_block_real_operations() {
let tool = batch_design_snapshot(&sample());
let mut args = BTreeMap::new();
args.insert("script".into(), String::new());
args.insert(
"operations".into(),
r#"root=I(null,{"type":"frame","width":100,"height":100})"#.into(),
);
assert!(matches!(
tool.call(&args),
ToolOutcome::OkJsonWithCommand(_, EditorCommand::InsertAuthoredSubtree { .. })
));
}
/// Layout omission is ambiguous: a title+rail section probably stacks, while
/// a toolbar or comparison group probably forms a row. Shape normalization
/// must preserve the omission; the agent loop reports it and asks the model to
/// choose explicitly instead of silently inventing design intent.
#[test]
fn a_container_without_a_layout_remains_unspecified() {
let mut section = serde_json::json!({
"type": "frame", "id": "n6", "name": "Popular Destinations",
"width": "fill_container", "height": 240,
@ -1480,10 +1552,9 @@ fn a_container_without_a_layout_stacks_its_children() {
]
});
crate::batch_design::normalize_node_shape(&mut section);
assert_eq!(
section.get("layout").and_then(|v| v.as_str()),
Some("vertical"),
"the section stacks its title above its rail"
assert!(
section.get("layout").is_none(),
"normalization must not guess whether the children form a row or column"
);
}

View file

@ -33,6 +33,18 @@ pub(crate) fn parse_single_direct_operation(line: &str) -> Result<Option<EditorC
Ok(None)
}
/// Whether a legacy single-line operations payload is a `G()` call, with or
/// without a result binding. Phase tools use this before the flat direct
/// parser because they do not hold the document snapshot needed to enforce
/// image placement and target geometry safely.
pub(crate) fn is_direct_image_operation(line: &str) -> bool {
let line = line.trim().trim_end_matches(';').trim();
let operation = find_top_level_char(line, '=')
.map(|eq| line[eq + 1..].trim())
.unwrap_or(line);
operation.starts_with("G(")
}
fn strip_bound_direct_operation(line: &str) -> &str {
let Some(eq) = find_top_level_char(line, '=') else {
return line;
@ -200,8 +212,8 @@ fn parse_replace_operation(body: &str) -> Result<EditorCommand, String> {
fn parse_image_operation(body: &str) -> Result<EditorCommand, String> {
let parts = split_top_level_args(body);
if parts.len() != 3 {
return Err("G() requires parent id, mode, and prompt".into());
if !matches!(parts.len(), 3 | 4) {
return Err("G() requires parent id, mode, prompt, and optional placement".into());
}
let parent_id = parse_parent_node_id(parts[0])?;
let mode = parse_ref_token(parts[1])?;
@ -211,17 +223,39 @@ fn parse_image_operation(body: &str) -> Result<EditorCommand, String> {
));
}
let prompt = parse_ref_token(parts[2])?;
if let Some(raw) = parts.get(3) {
let placement = parse_ref_token(raw)?;
if !matches!(placement.as_str(), "slot" | "append") {
return Err(format!(
"G() placement must be \"slot\" or \"append\", got {placement:?}"
));
}
}
let name = prompt.chars().take(40).collect::<String>();
let node: PenNode = serde_json::from_value(serde_json::json!({
let (width, height) = if parent_id.is_real() {
(
serde_json::json!("fill_container"),
serde_json::json!("fill_container"),
)
} else {
(serde_json::json!(400), serde_json::json!(300))
};
let mut value = serde_json::json!({
"type": "image",
"id": "__op_tmp_image_1",
"name": name,
"imagePrompt": prompt,
"src": "",
"width": 400,
"height": 300
}))
.map_err(|e| format!("invalid G() image node: {e}"))?;
"objectFit": "crop",
"width": width,
"height": height
});
if mode == "generate" {
value["imagePrompt"] = serde_json::json!(prompt);
} else {
value["imageSearchQuery"] = serde_json::json!(prompt);
}
let node: PenNode =
serde_json::from_value(value).map_err(|e| format!("invalid G() image node: {e}"))?;
Ok(EditorCommand::InsertSubtree {
nodes: vec![node],
parent_id,

View file

@ -353,6 +353,43 @@ fn design_refine_rejects_script_combined_with_structured_payload() {
}
}
#[test]
fn legacy_phase_tools_reject_direct_images_with_batch_design_guidance() {
fn assert_rejected(outcome: ToolOutcome, phase_tool: &str) {
match outcome {
ToolOutcome::Err(code, message) => {
assert_eq!(code, ToolErrorCode::InvalidArgument);
assert!(message.contains(phase_tool), "{message}");
assert!(message.contains("G()"), "{message}");
assert!(message.contains("placement"), "{message}");
assert!(message.contains("target geometry"), "{message}");
assert!(message.contains("batch_design"), "{message}");
}
other => panic!("expected legacy phase G() rejection, got {other:?}"),
}
}
let mut args = BTreeMap::new();
args.insert(
"operations".into(),
r#"G("slot-1", "search", "Prague coffee")"#.into(),
);
assert_rejected(design_skeleton_snapshot().call(&args), "design_skeleton");
args.insert(
"operations".into(),
r#"photo=G("row-1", "generate", "Coffee shop", "append")"#.into(),
);
assert_rejected(design_content_snapshot().call(&args), "design_content");
args.insert(
"operations".into(),
r#"G("slot-2", "search", "Coffee cup", "slot")"#.into(),
);
let state = EditorState::new();
assert_rejected(design_refine_snapshot(&state).call(&args), "design_refine");
}
#[test]
fn design_refine_missing_root_errors_like_ts() {
// TS design-refine throws "Root node not found: <id>" when the root is

View file

@ -53,9 +53,10 @@
//! first, so override keys (source ids) never match — a no-op there,
//! an explicit skip here.
use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};
use jian_ops_schema::node::PenNode;
use jian_ops_schema::node::{ContainerProps, LayoutMode, PenNode};
use jian_scene::layout_scene::SceneNode;
use op_editor_core::command_node::remap_subtree_ids_mapping;
use op_editor_core::{EditorState, NodeId, PenNodeExt};
use regex::Regex;
@ -84,6 +85,7 @@ pub(crate) fn run_batch_design_program(
.or_else(|| args.get("post_process"))
.map(|raw| matches!(raw.trim(), "true" | "1"))
.unwrap_or(false);
let lines = split_operations(operations);
let mut ctx = ProgramCtx {
sim: snapshot.clone(),
page_id: page_id.clone(),
@ -93,6 +95,8 @@ pub(crate) fn run_batch_design_program(
commands: Vec::new(),
post_process,
auto_seq: 0,
current_line: 0,
explicitly_sized_append_lines: explicitly_sized_append_lines(&lines),
};
// Pin the sim's active page to the requested page so sim READS
// (path lookups, node counts) see the same children every emitted
@ -113,7 +117,8 @@ pub(crate) fn run_batch_design_program(
let transactional = args.get("_line_policy").map(String::as_str) != Some("best_effort");
let mut errors: Vec<Value> = Vec::new();
for line in split_operations(operations) {
for (line_index, line) in lines.into_iter().enumerate() {
ctx.current_line = line_index;
if let Err(error) = execute_line(&line, &mut ctx) {
errors.push(json!({ "line": line_preview(&line), "error": error }));
}
@ -174,6 +179,11 @@ struct ProgramCtx {
post_process: bool,
/// Monotonic counter for `_auto_*` bindless-line bindings.
auto_seq: usize,
/// Index of the operation currently being executed.
current_line: usize,
/// Append G() lines whose result binding receives explicit positive
/// numeric width and height later in this same program.
explicitly_sized_append_lines: BTreeSet<usize>,
}
impl ProgramCtx {
@ -265,6 +275,99 @@ fn execute_line(line: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
Err(format!("Cannot parse operation: {line}"))
}
/// Return append-G line indexes that are robustly sized by later U() calls in
/// the same program. Parsing uses the DSL's top-level delimiter rules, so an
/// '=' inside a quoted image prompt is never mistaken for a result binding.
fn explicitly_sized_append_lines(lines: &[String]) -> BTreeSet<usize> {
let mut sized = BTreeSet::new();
for (index, line) in lines.iter().enumerate() {
let Some((Some(binding), 'G', args)) = parsed_operation(line) else {
continue;
};
let parts = split_top_level_args(args);
let is_append = parts.len() == 4
&& matches!(
serde_json::from_str::<String>(parts[3].trim()),
Ok(placement) if placement == "append"
);
if !is_append {
continue;
}
let mut width_is_positive_number = false;
let mut height_is_positive_number = false;
for later in &lines[index + 1..] {
let Some((later_binding, op, later_args)) = parsed_operation(later) else {
continue;
};
// Rebinding closes this append's sizing window. A U() beyond it
// would target the newer node, not this image.
if later_binding == Some(binding) {
break;
}
if op != 'U' {
continue;
}
let Some(comma) = find_top_level_char(later_args, ',') else {
continue;
};
let target = strip_outer_quotes(later_args[..comma].trim());
if target != binding {
continue;
}
let Ok(value) = parse_json_arg(&later_args[comma + 1..]) else {
continue;
};
let Some(patch) = value.as_object() else {
continue;
};
if let Some(width) = patch.get("width") {
width_is_positive_number = positive_json_number(width);
}
if let Some(height) = patch.get("height") {
height_is_positive_number = positive_json_number(height);
}
}
if width_is_positive_number && height_is_positive_number {
sized.insert(index);
}
}
sized
}
/// Parse one complete DSL operation without splitting on delimiters nested in
/// calls or quoted strings. Returns `(binding, opcode, argument body)`.
fn parsed_operation(line: &str) -> Option<(Option<&str>, char, &str)> {
let line = line.trim().trim_end_matches(';').trim();
let (binding, call) = match find_top_level_char(line, '=') {
Some(eq) => {
let binding = line[..eq].trim();
if binding.is_empty()
|| !binding
.chars()
.all(|ch| ch.is_ascii_alphanumeric() || ch == '_')
{
return None;
}
(Some(binding), line[eq + 1..].trim())
}
None => (None, line),
};
let mut chars = call.chars();
let op = chars.next()?;
let rest = chars.as_str();
if !rest.starts_with('(') || !call.ends_with(')') {
return None;
}
Some((binding, op, &rest[1..rest.len() - 1]))
}
fn positive_json_number(value: &Value) -> bool {
value
.as_f64()
.is_some_and(|number| number.is_finite() && number > 0.0)
}
fn execute_assign(op: &str, binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
match op {
"I" => execute_insert(binding, args, ctx),
@ -537,28 +640,138 @@ fn execute_replace(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()
Ok(())
}
/// `binding=G(parent, mode, prompt)` — emit an image node. TS requires
/// every argument quoted; `mode` must be `search` or `generate`. No
/// fetcher at this layer — `src` stays empty (browser-caller parity);
/// `binding=G(parent, mode, prompt[, placement])` — emit an image node. The
/// parent accepts a binding or quoted existing id (`null` is rejected because
/// both placements require a concrete target); `mode` must be `search` or
/// `generate`. Placement defaults to `slot`; the explicit `append` escape
/// hatch allows a new sibling only under a horizontal/vertical flow parent.
/// No fetcher at this layer — `src` stays empty (browser-caller parity);
/// the host's own image pipeline enriches later.
fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
let g = regex(r#"^"([^"]+)"\s*,\s*"(search|generate)"\s*,\s*"([^"]+)"$"#);
let Some(c) = g.captures(args.trim()) else {
let parts = split_top_level_args(args);
if !matches!(parts.len(), 3 | 4) {
return Err(format!("Invalid G() syntax: {args}"));
}
let parent_raw = parts[0].trim();
let parent = if matches!(parent_raw, "null" | "undefined" | "0" | "\"\"" | "\"0\"") {
String::new()
} else {
resolve_path_expr(parent_raw, &ctx.bindings)
};
let parent = resolve_ref(c.get(1).map_or("", |m| m.as_str()), &ctx.bindings);
let prompt = c.get(3).map_or("", |m| m.as_str());
let mode = serde_json::from_str::<String>(parts[1].trim())
.map_err(|_| format!("Invalid G() syntax: {args}"))?;
if !matches!(mode.as_str(), "search" | "generate") {
return Err(format!("G() mode must be search or generate: {mode}"));
}
let prompt = serde_json::from_str::<String>(parts[2].trim())
.map_err(|_| format!("Invalid G() syntax: {args}"))?;
let placement = match parts.get(3) {
None => "slot".to_string(),
Some(raw) => serde_json::from_str::<String>(raw.trim())
.map_err(|_| format!("Invalid G() syntax: {args}"))?,
};
if !matches!(placement.as_str(), "slot" | "append") {
return Err(format!(
"G() placement must be \"slot\" or \"append\", got {placement:?}"
));
}
let name: String = prompt.chars().take(40).collect();
let node: PenNode = serde_json::from_value(json!({
let mut value = json!({
"type": "image",
"id": "__op_tmp_image_1",
"name": name,
"imagePrompt": prompt,
"src": "",
"objectFit": "crop",
"width": 400,
"height": 300
}))
.map_err(|e| format!("invalid G() image node: {e}"))?;
});
if mode == "generate" {
value["imagePrompt"] = json!(prompt);
} else {
value["imageSearchQuery"] = json!(prompt);
}
if parent.trim().is_empty() || parent.trim() == "0" {
return Err(format!(
"G() placement {placement:?} requires an explicit frame/rectangle target id; create the target first instead of using null"
));
}
let target = find_node_by_path(ctx.sim.active_children(), &parent, &ctx.alias)
.ok_or_else(|| format!("G() parent not found or not a container: {parent}"))?;
// `parent` may be a slash path or an authored id that `find_node_by_path`
// translated through `ctx.alias`. The emitted insert must target the live
// resolved node id, never the caller's path/alias spelling.
let target_id = target.id_str().to_string();
let container = node_container(target)
.ok_or_else(|| format!("G() parent not found or not a container: {parent}"))?;
// Placement is an explicit structural contract. Slot-fill accepts only an
// EMPTY target; append accepts only an explicitly-authored flow parent.
// Never recover intent from names, dimensions, child kinds, or position.
match placement.as_str() {
"slot" => {
let child_ids = target
.children()
.into_iter()
.flatten()
.map(PenNode::id_str)
.collect::<Vec<_>>();
if !child_ids.is_empty() {
return Err(format!(
"G() slot target {} must be empty, but it has children [{}]. Pass the exact empty frame/rectangle slot id; use \"append\" only for an intentional child of an explicit horizontal/vertical flow parent",
target.id_str(),
child_ids.join(", ")
));
}
}
"append" => {
if explicit_flow_layout(container).is_none() {
return Err(format!(
"G() append target {} must declare layout \"horizontal\" or \"vertical\"; got {}. Append means a new flow sibling and is never an absolute overlay",
target.id_str(),
layout_label(container)
));
}
if !ctx
.explicitly_sized_append_lines
.contains(&ctx.current_line)
{
return Err(
"G() append requires a result binding followed in the same batch by U(binding, {\"width\": <positive number>, \"height\": <positive number>}); refusing an unsized flow child with default fill_container width and height"
.into(),
);
}
}
_ => unreachable!("placement validated above"),
}
if matches!(container.layout, Some(LayoutMode::None)) {
let has_declared_size = container.width.is_some() && container.height.is_some();
let resolved = has_declared_size
.then(|| resolved_node_size(&ctx.sim, &target_id))
.flatten();
let width = target.width_px().or_else(|| resolved.map(|size| size.0));
let height = target.height_px().or_else(|| resolved.map(|size| size.1));
let (Some(width), Some(height)) = (width, height) else {
return Err(format!(
"G() target {target_id} uses layout none, so it needs declared width and height that resolve above zero before an image can fill it"
));
};
if width <= 0.0 || height <= 0.0 {
return Err(format!(
"G() target {target_id} uses layout none, so it needs declared width and height that resolve above zero before an image can fill it"
));
}
value["x"] = json!(0);
value["y"] = json!(0);
value["width"] = json!(width);
value["height"] = json!(height);
} else {
// A G() image serves its target slot; its intrinsic/search size is
// not evidence for card geometry. Flex sizing keeps the image
// inside the slot and lets crop/object-fit do the visual work.
value["width"] = json!("fill_container");
value["height"] = json!("fill_container");
}
let node: PenNode =
serde_json::from_value(value).map_err(|e| format!("invalid G() image node: {e}"))?;
let mut nodes = vec![node];
let map = ctx.remap(&mut nodes)?;
let image_id = map
@ -568,7 +781,7 @@ fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
ctx.emit(
EditorCommand::InsertAuthoredSubtree {
nodes,
parent_id: parent_node_id(Some(&parent)),
parent_id: NodeId::new(&target_id),
page_id: ctx.page_id.clone(),
},
&format!("G() parent not found or not a container: {parent}"),
@ -577,6 +790,53 @@ fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
Ok(())
}
fn explicit_flow_layout(container: &ContainerProps) -> Option<&'static str> {
match container.layout {
Some(LayoutMode::Horizontal) => Some("horizontal"),
Some(LayoutMode::Vertical) => Some("vertical"),
_ => None,
}
}
fn layout_label(container: &ContainerProps) -> &'static str {
match container.layout {
Some(LayoutMode::Horizontal) => "horizontal",
Some(LayoutMode::Vertical) => "vertical",
Some(LayoutMode::None) => "none",
None => "omitted",
}
}
fn node_container(node: &PenNode) -> Option<&ContainerProps> {
match node {
PenNode::Frame(node) => Some(&node.container),
PenNode::Group(node) => Some(&node.container),
PenNode::Rectangle(node) => Some(&node.container),
_ => None,
}
}
fn resolved_node_size(state: &EditorState, node_id: &str) -> Option<(f64, f64)> {
fn find(nodes: &[SceneNode], node_id: &str) -> Option<(f64, f64)> {
for node in nodes {
if node.id == node_id {
let bounds = node.aggregate_bounds();
return Some((f64::from(bounds.size.x), f64::from(bounds.size.y)));
}
if let Some(size) = find(&node.children, node_id) {
return Some(size);
}
}
None
}
let scene = op_pen_loader::editor_state_to_layout_scene(state);
scene
.pages
.iter()
.find_map(|page| find(&page.children, node_id))
}
/// `U(path, data)` — shallow-patch the node at `path`. No result entry
/// (TS call-form ops don't push results).
fn execute_update(args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {

View file

@ -0,0 +1,379 @@
//! Image/G() placement tests for the multi-op batch-design executor.
use super::*;
#[test]
fn image_op_accepts_binding_or_quoted_parent_syntax() {
// Best-effort policy: the deliberately-bad third line must be
// dropped (not roll back the batch) so the G() parse + binding
// resolution of the good lines stays observable.
let mut state = sample();
let program = r##"wrap=I(null, {"type":"frame","name":"Wrap","x":500,"y":0,"width":400,"height":300})
img=G(wrap, "search", "sunset photo")
G(wrap, "search")"##;
let (envelope, cmd) = call_operations_best_effort(&state, program);
let errors = envelope["errors"].as_array().expect("errors");
assert_eq!(errors.len(), 1, "{envelope}");
assert!(
errors[0]["error"]
.as_str()
.unwrap()
.starts_with("Invalid G() syntax:"),
"{envelope}"
);
let wrap_id = binding_id(&envelope, "wrap");
let img_id = binding_id(&envelope, "img");
assert!(state.apply(cmd.expect("command")));
let wrap = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new(&wrap_id))
.expect("wrap");
let img = wrap
.children()
.expect("children")
.iter()
.find(|c| c.id_str() == img_id)
.expect("image under wrap");
let jian_ops_schema::node::PenNode::Image(image) = img else {
panic!("expected image")
};
assert_eq!(img.base().name.as_deref(), Some("sunset photo"));
assert_eq!(image.image_search_query.as_deref(), Some("sunset photo"));
assert_eq!(image.image_prompt, None);
}
#[test]
fn image_op_preserves_generate_mode_on_the_node() {
let mut state = sample();
let program = r##"slot=I(null, {"type":"frame","name":"Hero","x":500,"y":0,"width":160,"height":90,"layout":"none"})
img=G(slot, "generate", "cinematic sunset coast")"##;
let (envelope, cmd) = call_operations(&state, program);
let image_id = binding_id(&envelope, "img");
assert!(state.apply(cmd.expect("command")));
let image = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new(image_id))
.expect("image");
let jian_ops_schema::node::PenNode::Image(image) = image else {
panic!("expected image")
};
assert_eq!(
image.image_prompt.as_deref(),
Some("cinematic sunset coast")
);
assert_eq!(image.image_search_query, None);
}
#[test]
fn image_op_rejects_a_populated_flow_row_and_accepts_its_explicit_slot() {
// Exact live-QA failure shape: the row already owns a cover slot, text
// column, and play control. G(row, ...) used to append the image as a
// fourth sibling, leaving the cover slot empty. The contract gate uses
// only explicit layout/parent structure; it does not infer intent from
// these fixture names or their 56px dimensions.
let row: jian_ops_schema::node::PenNode = serde_json::from_value(serde_json::json!({
"type": "frame", "id": "n42", "name": "Daily Mix 1",
"layout": "horizontal", "width": "fill_container", "height": "fit_content",
"children": [
{"type": "frame", "id": "n43", "name": "Daily Mix 1 Cover", "layout": "none", "width": 56, "height": 56},
{"type": "frame", "id": "n44", "name": "Daily Mix 1 Text", "layout": "vertical", "width": "fill_container", "height": "fit_content"},
{"type": "icon_font", "id": "n47", "iconFontFamily": "lucide", "iconFontName": "play", "width": 20, "height": 20}
]
}))
.unwrap();
let mut state = state_with(vec![row]);
let (rejected, command) = call_operations(
&state,
r##"img=G("n42", "search", "green forest ambient")"##,
);
assert!(
command.is_none(),
"the bad parent must not mutate: {rejected}"
);
assert_eq!(rejected["applied"], Value::Bool(false));
let message = rejected["errors"][0]["error"]
.as_str()
.expect("G target error");
assert!(
message.contains("slot target n42 must be empty"),
"{message}"
);
assert!(message.contains("[n43, n44, n47]"), "{message}");
assert!(message.contains("exact empty"), "{message}");
let (accepted, command) = call_operations(
&state,
r##"img=G("n43", "search", "green forest ambient")"##,
);
let image_id = binding_id(&accepted, "img");
assert!(state.apply(command.expect("slot-targeted G command")));
let slot = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new("n43"))
.expect("explicit slot");
assert!(
slot.children()
.is_some_and(|children| children.iter().any(|child| child.id_str() == image_id)),
"the accepted image must land inside the explicit slot"
);
let row = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new("n42"))
.expect("row");
assert_eq!(
row.children().map(Vec::len),
Some(3),
"no fourth row sibling"
);
let (duplicate, command) =
call_operations(&state, r##"duplicate=G("n43", "search", "second cover")"##);
assert!(command.is_none(), "populated slot must reject: {duplicate}");
let message = duplicate["errors"][0]["error"]
.as_str()
.expect("populated slot error");
assert!(
message.contains("slot target n43 must be empty"),
"{message}"
);
assert!(message.contains(&image_id), "{message}");
let (overlay, command) = call_operations(
&state,
r##"overlay=G("n43", "search", "overlay", "append")"##,
);
assert!(
command.is_none(),
"layout-none append must reject: {overlay}"
);
let message = overlay["errors"][0]["error"]
.as_str()
.expect("append layout error");
assert!(message.contains("must declare layout"), "{message}");
assert!(message.contains("got none"), "{message}");
assert!(message.contains("never an absolute overlay"), "{message}");
let (unsized_result, command) = call_operations(
&state,
r##"img=G("n42", "search", "unsized gallery artwork", "append")"##,
);
assert!(
command.is_none(),
"unsized append must reject: {unsized_result}"
);
assert_eq!(
unsized_result["applied"],
Value::Bool(false),
"{unsized_result}"
);
let message = unsized_result["errors"][0]["error"]
.as_str()
.expect("append sizing error");
assert!(
message.contains("followed in the same batch by U"),
"{message}"
);
assert!(message.contains("positive number"), "{message}");
assert!(message.contains("fill_container"), "{message}");
let (appended, command) = call_operations(
&state,
r##"img=G("n42", "search", "new gallery artwork", "append")
U(img, {"width":72,"height":72})"##,
);
let appended_id = binding_id(&appended, "img");
assert!(state.apply(command.expect("explicit append command")));
let row = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new("n42"))
.expect("row");
let appended = row
.children()
.expect("row children")
.iter()
.find(|child| child.id_str() == appended_id)
.expect("explicit image sibling");
assert_eq!(appended.width_px(), Some(72.0));
assert_eq!(appended.height_px(), Some(72.0));
assert_eq!(row.children().map(Vec::len), Some(4));
}
#[test]
fn bindless_image_with_equals_in_prompt_cannot_bypass_populated_flow_slot_gate() {
let row: jian_ops_schema::node::PenNode = serde_json::from_value(serde_json::json!({
"type": "frame", "id": "n42", "name": "Event Rail",
"layout": "horizontal", "width": "fill_container", "height": "fit_content",
"children": [
{"type": "frame", "id": "n43", "name": "Cover Slot", "layout": "none", "width": 168, "height": 112},
{"type": "frame", "id": "n44", "name": "Event Details", "layout": "vertical", "width": "fill_container", "height": "fit_content"}
]
}))
.unwrap();
let state = state_with(vec![row]);
// The '=' is data inside the quoted prompt, not a result-binding
// delimiter. This bindless call must still route through the snapshot
// program validator instead of the geometry-blind direct parser.
let (rejected, command) = call_operations(
&state,
r##"G("n42", "search", "festival crowd ratio=16:9")"##,
);
assert!(command.is_none(), "populated row must reject: {rejected}");
assert_eq!(rejected["applied"], Value::Bool(false), "{rejected}");
let message = rejected["errors"][0]["error"]
.as_str()
.expect("strict G slot error");
assert!(
message.contains("slot target n42 must be empty"),
"{message}"
);
assert!(message.contains("[n43, n44]"), "{message}");
}
#[test]
fn image_op_inserts_under_live_target_resolved_from_slash_path_and_authored_alias() {
let mut state = op_editor_core::EditorState::new();
let program = r##"root=I(null, {"type":"frame","id":"auth-root","name":"Root","width":390,"height":844,"layout":"vertical","children":[{"type":"frame","id":"auth-slot","name":"Hero Slot","width":160,"height":90,"layout":"none"}]})
img=G(root+"/auth-slot", "search", "sunset coast ratio=16:9")"##;
let (envelope, command) = call_operations(&state, program);
assert!(envelope.get("errors").is_none(), "{envelope}");
let root_id = binding_id(&envelope, "root");
let image_id = binding_id(&envelope, "img");
assert!(state.apply(command.expect("slash-path G command")));
let root = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new(&root_id))
.expect("root");
let slot = root
.children()
.expect("root children")
.iter()
.find(|child| child.base().name.as_deref() == Some("Hero Slot"))
.expect("authored slot remapped to a live child");
assert_ne!(slot.id_str(), "auth-slot", "authored id must be remapped");
assert!(
slot.children()
.is_some_and(|children| children.iter().any(|child| child.id_str() == image_id)),
"image must be inserted under the resolved live slot id"
);
}
#[test]
fn image_op_rejects_populated_layout_omitted_slot_and_omitted_append_parent() {
let populated = sample();
let (slot_result, command) =
call_operations(&populated, r##"img=G("n10", "search", "product photo")"##);
assert!(
command.is_none(),
"populated slot must reject: {slot_result}"
);
let message = slot_result["errors"][0]["error"]
.as_str()
.expect("slot error");
assert!(
message.contains("slot target n10 must be empty"),
"{message}"
);
assert!(message.contains("[n11, n12]"), "{message}");
let empty_omitted = state_with(vec![frame("slot", "Slot", 0.0, 0.0, 100.0, 80.0, vec![])]);
let (append_result, command) = call_operations(
&empty_omitted,
r##"img=G("slot", "search", "product photo", "append")"##,
);
assert!(
command.is_none(),
"omitted-layout append must reject: {append_result}"
);
let message = append_result["errors"][0]["error"]
.as_str()
.expect("append error");
assert!(message.contains("must declare layout"), "{message}");
assert!(message.contains("got omitted"), "{message}");
}
#[test]
fn image_op_rejects_a_missing_target_for_slot_and_append() {
let state = sample();
for operation in [
r##"img=G(null, "search", "photo")"##,
r##"img=G(null, "search", "photo", "append")"##,
] {
let (envelope, command) = call_operations(&state, operation);
assert!(command.is_none(), "null target must reject: {envelope}");
assert!(envelope["errors"][0]["error"]
.as_str()
.is_some_and(|message| message.contains("explicit frame/rectangle target id")));
}
}
#[test]
fn image_op_rejects_an_unknown_placement() {
let state = sample();
let (envelope, command) = call_operations(
&state,
r##"img=G("n10", "search", "product photo", "guess")"##,
);
assert!(command.is_none(), "transaction rolls back: {envelope}");
assert!(envelope["errors"][0]["error"]
.as_str()
.is_some_and(|message| message.contains("placement must be")));
}
#[test]
fn image_op_fills_an_absolute_slot_without_importing_400x300_geometry() {
let mut state = sample();
let program = r##"slot=I(null, {"type":"frame","name":"Hero Image","x":500,"y":0,"width":160,"height":90,"layout":"none","clipContent":true})
img=G("slot", "search", "sunset coast")"##;
let (envelope, cmd) = call_operations(&state, program);
let slot_id = binding_id(&envelope, "slot");
let img_id = binding_id(&envelope, "img");
assert!(state.apply(cmd.expect("command")));
let slot = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new(slot_id))
.expect("slot");
let image = slot
.children()
.expect("children")
.iter()
.find(|node| node.id_str() == img_id)
.expect("image");
assert_eq!(image.width_px(), Some(160.0));
assert_eq!(image.height_px(), Some(90.0));
assert_eq!(image.base().x, Some(0.0));
assert_eq!(image.base().y, Some(0.0));
}
#[test]
fn image_op_uses_resolved_width_for_fill_sized_absolute_slot() {
let mut state = sample();
let program = r##"root=I(null, {"type":"frame","name":"Travel","x":500,"y":0,"width":390,"height":844,"layout":"vertical"})
slot=I(root, {"type":"frame","name":"Hero Image","width":"fill_container","height":140,"layout":"none","clipContent":true})
img=G(slot, "search", "sunset coast")"##;
let (envelope, cmd) = call_operations(&state, program);
let slot_id = binding_id(&envelope, "slot");
let img_id = binding_id(&envelope, "img");
assert!(state.apply(cmd.expect("command")));
let slot = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new(slot_id))
.expect("slot");
let image = slot
.children()
.expect("children")
.iter()
.find(|node| node.id_str() == img_id)
.expect("image");
assert_eq!(image.width_px(), Some(390.0));
assert_eq!(image.height_px(), Some(140.0));
assert_eq!(image.base().x, Some(0.0));
assert_eq!(image.base().y, Some(0.0));
}
#[test]
fn image_op_rejects_an_unsized_absolute_slot() {
let state = sample();
let program = r##"slot=I(null, {"type":"frame","name":"Hero Image","layout":"none"})
img=G("slot", "search", "sunset coast")"##;
let (envelope, cmd) = call_operations(&state, program);
assert!(cmd.is_none(), "transaction rolls back: {envelope}");
assert!(envelope["errors"].as_array().is_some_and(|errors| errors
.iter()
.any(|error| error["error"]
.as_str()
.is_some_and(|message| message.contains("declared width and height")))));
}

View file

@ -223,6 +223,115 @@ b=I("n10", {"type":"rectangle","name":"B","width":10,"height":10})"##;
);
}
#[test]
fn failed_rail_reconstruction_keeps_all_four_populated_cards_byte_identical() {
let cards: Vec<jian_ops_schema::node::PenNode> = (1..=4)
.map(|index| {
serde_json::from_value(serde_json::json!({
"type": "frame",
"id": format!("card-{index}"),
"name": format!("Event Card {index}"),
"layout": "vertical",
"width": 168,
"height": 220,
"children": [
{
"type": "image",
"id": format!("card-{index}-image"),
"name": format!("Event {index} Photo"),
"src": format!("https://example.invalid/event-{index}.jpg"),
"objectFit": "crop",
"width": "fill_container",
"height": 112
},
{
"type": "frame",
"id": format!("card-{index}-details"),
"name": format!("Event {index} Details"),
"layout": "vertical",
"width": "fill_container",
"height": "fit_content",
"children": [
{
"type": "text",
"id": format!("card-{index}-title"),
"name": "Title",
"content": format!("Popular Event {index}"),
"width": "fill_container",
"height": 24
},
{
"type": "text",
"id": format!("card-{index}-venue"),
"name": "Venue",
"content": format!("Venue {index}"),
"width": "fill_container",
"height": 18
}
]
}
]
}))
.expect("populated event card")
})
.collect();
let rail: jian_ops_schema::node::PenNode = serde_json::from_value(serde_json::json!({
"type": "frame",
"id": "event-rail",
"name": "Popular Near You",
"layout": "horizontal",
"width": 390,
"height": 220,
"children": cards
}))
.expect("populated rail");
let state = state_with(vec![rail]);
let before = serde_json::to_vec(state.active_children()).expect("pre-transaction bytes");
// This mirrors the destructive-redraft failure mode: four valid cards are
// deleted in the simulated batch, then reconstruction targets a bad id.
// Transactional execution must ship no partial delete commands.
let program = r##"D("card-1")
D("card-2")
D("card-3")
D("card-4")
replacement=I("missing-rail-parent", {"type":"frame","name":"Rebuilt Card","width":168,"height":220,"children":[{"type":"text","content":"replacement","width":120,"height":24}]})"##;
let (envelope, command) = call_operations(&state, program);
assert!(
command.is_none(),
"failed reconstruction must not emit deletes"
);
assert_eq!(envelope["applied"], Value::Bool(false), "{envelope}");
assert_eq!(envelope["results"], serde_json::json!([]), "{envelope}");
assert!(envelope["errors"][0]["error"]
.as_str()
.is_some_and(|message| message.contains("missing-rail-parent")));
assert_eq!(
serde_json::to_vec(state.active_children()).expect("post-transaction bytes"),
before,
"the original populated rail must remain byte-identical"
);
let rail =
op_editor_core::walkers::find_node(state.active_children(), &NodeId::new("event-rail"))
.expect("original rail");
let cards = rail.children().expect("original cards");
assert_eq!(cards.len(), 4, "all four cards must remain");
for (index, card) in cards.iter().enumerate() {
let children = card.children().expect("populated card subtree");
assert_eq!(children.len(), 2, "card {} lost content", index + 1);
assert!(matches!(
children.first(),
Some(jian_ops_schema::node::PenNode::Image(image)) if !image.src.as_str().is_empty()
));
assert!(children
.get(1)
.and_then(jian_ops_schema::node::PenNode::children)
.is_some_and(|details| details.len() == 2));
}
}
#[test]
fn best_effort_policy_keeps_ts_survivor_semantics_for_internal_callers() {
// The orchestrator's script-gen path (`program_gen.rs`) opts back
@ -479,40 +588,8 @@ D("ghost")"##
);
}
#[test]
fn image_op_requires_ts_quoted_syntax_and_resolves_binding_parents() {
// Best-effort policy: the deliberately-bad third line must be
// dropped (not roll back the batch) so the G() parse + binding
// resolution of the good lines stays observable.
let mut state = sample();
let program = r##"wrap=I(null, {"type":"frame","name":"Wrap","x":500,"y":0,"width":400,"height":300,"children":[{"type":"text","name":"caption","content":"x","width":10,"height":10}]})
img=G("wrap", "search", "sunset photo")
G(wrap, "search", "bad syntax")"##;
let (envelope, cmd) = call_operations_best_effort(&state, program);
let errors = envelope["errors"].as_array().expect("errors");
assert_eq!(errors.len(), 1, "{envelope}");
assert!(
errors[0]["error"]
.as_str()
.unwrap()
.starts_with("Invalid G() syntax:"),
"{envelope}"
);
let wrap_id = binding_id(&envelope, "wrap");
let img_id = binding_id(&envelope, "img");
assert!(state.apply(cmd.expect("command")));
let wrap = op_editor_core::walkers::find_node(state.active_children(), &NodeId::new(&wrap_id))
.expect("wrap");
let img = wrap
.children()
.expect("children")
.iter()
.find(|c| c.id_str() == img_id)
.expect("image under wrap");
assert!(matches!(img, jian_ops_schema::node::PenNode::Image(_)));
assert_eq!(img.base().name.as_deref(), Some("sunset photo"));
}
#[path = "batch_program_image_tests.rs"]
mod image_tests;
#[test]
fn post_process_flag_marks_the_envelope() {

View file

@ -72,7 +72,7 @@ const AESTHETIC_QUALITY_GUIDE: &str = r#"AESTHETIC QUALITY BAR:
const RUST_ELEMENT_TOOL_GUIDE: &str = r##"RUST MCP ELEMENT TOOL COMPATIBILITY:
- This Rust MCP server does not expose the TS `add_*_v1` element-tool family unless those exact tools appear in tools/list. Do not call `add_*` tools just because older prompt text or examples mention them.
- For custom UI trees, use `batch_design` with the TS operations DSL in the `operations` argument. Supported writes: `binding=I(parent, nodeJson)` for inserts, plus one-at-a-time `U(nodeId, patchJson)`, `D(nodeId)`, `M(nodeId, parent, index?)`, `binding=C(sourceId, parent, overrides?)`, `binding=R(nodeId, nodeJson)`, and `binding=G(parent, "search"|"generate", prompt)` refine operations.
- For custom UI trees, use `batch_design` with the TS operations DSL in the `operations` argument. Supported writes: `binding=I(parent, nodeJson)` for inserts, plus one-at-a-time `U(nodeId, patchJson)`, `D(nodeId)`, `M(nodeId, parent, index?)`, `binding=C(sourceId, parent, overrides?)`, `binding=R(nodeId, nodeJson)`, and `binding=G(slotIdOrBinding, "search"|"generate", prompt[, "append"])` refine operations. The default 3-argument G is strict slot-fill: its target must exist and have zero children. The explicit fourth argument `"append"` is accepted only on a parent that declares layout `"horizontal"` or `"vertical"`; it never overlays a layout-none/omitted parent. Size the appended binding with `U()` in the same batch.
- Parent can be `null` for the active page root, a previous binding name, or one real existing parent id. The node JSON is canonical PenNode JSON; omit `id` if you do not care, because Rust remaps inserted ids.
- `U` currently patches geometry/name/fill fields; use dedicated `set_node_*` tools for text, rotation, stroke, font, effects, and other specialized fields.

View file

@ -72,7 +72,11 @@ fn get_design_prompt_elements_section_points_to_batch_design_operations() {
assert!(prompt.contains("U(nodeId, patchJson)"));
assert!(prompt.contains("C(sourceId, parent"));
assert!(prompt.contains("R(nodeId, nodeJson)"));
assert!(prompt.contains("G(parent"));
assert!(prompt.contains("G(slotIdOrBinding"));
assert!(prompt.contains("target must exist and have zero children"));
assert!(prompt.contains("accepted only on a parent that declares layout"));
assert!(prompt.contains("never overlays"));
assert!(prompt.contains("Size the appended binding"));
assert!(!prompt.contains("add_card_row_v1"));
}
other => panic!("expected prompt ok, got {other:?}"),

View file

@ -8,7 +8,6 @@ use std::collections::BTreeMap;
use jian_ops_schema::node::PenNode;
use jian_scene::layout_scene::{NodeKind, SceneNode};
use op_editor_core::geometry::aggregate_bounds;
use op_editor_core::pen_node_ext::PenNodeExt;
use op_editor_core::walkers::find_node;
use op_editor_core::{EditorState, NodeId};
@ -335,16 +334,32 @@ fn page_layout_snapshots(state: &EditorState) -> (Vec<PageLayout>, String) {
}
fn page_space_snapshots(state: &EditorState) -> (Vec<PageSpace>, String) {
let scene = op_pen_loader::editor_state_to_layout_scene(state);
match state.doc.pages.as_ref() {
Some(pages) if !pages.is_empty() => {
let active = state.ui.active_page_index.min(pages.len() - 1);
let out = pages
.iter()
.map(|page| page_space(&page.id, &page.children))
.enumerate()
.map(|(idx, page)| {
let roots = scene
.pages
.get(idx)
.map(|scene_page| scene_page.children.as_slice())
.unwrap_or_default();
page_space(&page.id, roots)
})
.collect();
(out, pages[active].id.clone())
}
_ => (vec![page_space("0", &state.doc.children)], "0".into()),
_ => {
let roots = scene
.pages
.first()
.map(|page| page.children.as_slice())
.unwrap_or_default();
(vec![page_space("0", roots)], "0".into())
}
}
}
@ -506,13 +521,11 @@ fn layout_records_in(roots: &[PenNode], all_nodes: &[PenNode]) -> Vec<LayoutReco
out
}
fn page_space(id: &str, roots: &[PenNode]) -> PageSpace {
fn walk(nodes: &[PenNode], out: &mut Vec<BoundsRecord>) {
fn page_space(id: &str, roots: &[SceneNode]) -> PageSpace {
fn walk(nodes: &[SceneNode], out: &mut Vec<BoundsRecord>) {
for n in nodes {
out.push(bounds_record(n));
if let Some(children) = n.children() {
walk(children, out);
}
walk(&n.children, out);
}
}
let root_bounds = roots.iter().map(bounds_record).collect();
@ -525,14 +538,14 @@ fn page_space(id: &str, roots: &[PenNode]) -> PageSpace {
}
}
fn bounds_record(node: &PenNode) -> BoundsRecord {
let b = aggregate_bounds(node);
fn bounds_record(node: &SceneNode) -> BoundsRecord {
let b = node.aggregate_bounds();
BoundsRecord {
id: node.id_str().to_string(),
x: b.x as i32,
y: b.y as i32,
w: b.w as i32,
h: b.h as i32,
id: node.id.clone(),
x: b.origin.x as i32,
y: b.origin.y as i32,
w: b.size.x as i32,
h: b.size.y as i32,
}
}
@ -600,3 +613,60 @@ fn optional_i32_arg(
None => Ok(default),
}
}
#[cfg(test)]
mod tests {
use super::*;
fn bottom_args(page_id: Option<&str>) -> BTreeMap<String, String> {
let mut args = BTreeMap::from([
("width".into(), "50".into()),
("height".into(), "50".into()),
("padding".into(), "10".into()),
("direction".into(), "bottom".into()),
]);
if let Some(id) = page_id {
args.insert("pageId".into(), id.into());
}
args
}
#[test]
fn empty_space_uses_resolved_omitted_height_and_requested_page() {
let src = r#"{
"version":"1.0.0","pages":[
{"id":"hug-page","name":"Hug","children":[
{"type":"frame","id":"hug","x":10,"y":20,"width":390,
"layout":"vertical","gap":5,"padding":[10,20],"children":[
{"type":"rectangle","id":"a","width":"fill_container","height":40},
{"type":"rectangle","id":"b","width":"fill_container","height":50}
]},
{"type":"frame","id":"fixed","x":500,"y":30,"width":200,"height":20}
]},
{"id":"other-page","name":"Other","children":[
{"type":"frame","id":"other","x":900,"y":100,"width":20,"height":30}
]}
],"children":[]
}"#;
let parsed = jian_ops_schema::load_str(src).expect("parse fixture");
let mut state = EditorState::from_document(parsed.value);
state.ui.active_page_index = 1;
let tool = find_empty_space_snapshot(&state);
let ToolOutcome::Ok(active) = tool.call(&bottom_args(None)) else {
panic!("active-page lookup should succeed");
};
assert_eq!(active.get("x").map(String::as_str), Some("900"));
assert_eq!(active.get("y").map(String::as_str), Some("140"));
let ToolOutcome::Ok(hug) = tool.call(&bottom_args(Some("hug-page"))) else {
panic!("requested-page lookup should succeed");
};
assert_eq!(hug.get("x").map(String::as_str), Some("10"));
assert_eq!(
hug.get("y").map(String::as_str),
Some("145"),
"bottom placement must include the resolved 115px Hug root"
);
}
}

View file

@ -8,7 +8,6 @@
use std::collections::BTreeMap;
use jian_ops_schema::node::PenNode;
use op_editor_core::geometry::aggregate_bounds;
use op_editor_core::pen_node_ext::PenNodeExt;
use op_editor_core::EditorState;
@ -48,34 +47,16 @@ impl McpTool for GetCanvasBounds {
}
pub fn get_canvas_bounds_snapshot(state: &EditorState) -> GetCanvasBounds {
let children = active_children(state);
if children.is_empty() {
return GetCanvasBounds { bounds: None };
}
let mut min_x = f64::INFINITY;
let mut min_y = f64::INFINITY;
let mut max_x = f64::NEG_INFINITY;
let mut max_y = f64::NEG_INFINITY;
for n in children {
let b = aggregate_bounds(n);
if b.is_empty() {
continue;
}
min_x = min_x.min(b.x);
min_y = min_y.min(b.y);
max_x = max_x.max(b.x + b.w);
max_y = max_y.max(b.y + b.h);
}
if !min_x.is_finite() {
return GetCanvasBounds { bounds: None };
}
let scene = op_pen_loader::editor_state_to_layout_scene(state);
GetCanvasBounds {
bounds: Some((
min_x as i32,
min_y as i32,
(max_x - min_x) as i32,
(max_y - min_y) as i32,
)),
bounds: scene.content_bounds().map(|bounds| {
(
bounds.origin.x as i32,
bounds.origin.y as i32,
bounds.size.x as i32,
bounds.size.y as i32,
)
}),
}
}
@ -346,3 +327,42 @@ pub fn get_selection_set_snapshot(state: &EditorState) -> GetSelectionSet {
.collect(),
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn canvas_bounds_use_resolved_hug_root_and_active_page() {
let src = r#"{
"version":"1.0.0","pages":[
{"id":"hug-page","name":"Hug","children":[
{"type":"frame","id":"hug","x":10,"y":20,"width":390,
"height":"fit_content","layout":"vertical","gap":5,"padding":[10,20],
"children":[
{"type":"rectangle","id":"a","width":"fill_container","height":40},
{"type":"rectangle","id":"b","width":"fill_container","height":50}
]},
{"type":"frame","id":"fixed","x":500,"y":30,"width":200,"height":20}
]},
{"id":"other-page","name":"Other","children":[
{"type":"frame","id":"other","x":900,"y":100,"width":20,"height":30}
]}
],"children":[]
}"#;
let parsed = jian_ops_schema::load_str(src).expect("parse fixture");
let mut state = EditorState::from_document(parsed.value);
assert_eq!(
get_canvas_bounds_snapshot(&state).bounds,
Some((10, 20, 690, 115)),
"multi-root union must include the layout-resolved Hug height"
);
state.ui.active_page_index = 1;
assert_eq!(
get_canvas_bounds_snapshot(&state).bounds,
Some((900, 100, 20, 30)),
"fixed-size bounds and active-page selection stay unchanged"
);
}
}

View file

@ -32,14 +32,17 @@ globalThis.I = function (parent, obj) {
globalThis.K = function (kitComponentId, parent, overrides) {
return __recordK(JSON.stringify(String(kitComponentId)), parent == null ? "null" : String(parent), JSON.stringify(overrides == null ? {} : overrides));
};
var __stubSeq = 0;
function __opStub() { __stubSeq += 1; return "stub" + __stubSeq; }
globalThis.C = __opStub;
globalThis.U = __opStub;
globalThis.D = __opStub;
globalThis.M = __opStub;
globalThis.R = __opStub;
globalThis.G = __opStub;
function __unsupported(op) {
return function () {
throw new Error("OP_SCRIPT_MODE_UNSUPPORTED: " + op + "() has no effect in script mode; use batch_design operations mode");
};
}
globalThis.C = __unsupported("C");
globalThis.U = __unsupported("U");
globalThis.D = __unsupported("D");
globalThis.M = __unsupported("M");
globalThis.R = __unsupported("R");
globalThis.G = __unsupported("G");
var __noop = function () {};
globalThis.console = { log: __noop, warn: __noop, error: __noop, info: __noop, debug: __noop };
"#;
@ -191,6 +194,7 @@ fn eval_to_program(script: &str) -> Result<String, String> {
}
match outcome {
Ok(()) => Ok(program),
Err(e) if e.contains("OP_SCRIPT_MODE_UNSUPPORTED") => Err(e),
Err(e) if program.trim().is_empty() => Err(e),
Err(e) => {
tracing::warn!(

View file

@ -221,38 +221,26 @@ fn unrepairable_garbage_still_errors() {
}
#[test]
fn pencil_ops_and_console_are_noop_stubs() {
// The PRELUDE advertises C/U/D/M/R/G plus console.* as no-op stubs so a
// script generated against the batch_design DSL vocabulary (and any
// stray console logging) never aborts the sandbox — only I(...) may
// cause an effect. Calling every stub plus a couple of real inserts must
// still return Ok with exactly the I() lines recorded.
fn unsupported_mutations_are_rejected_instead_of_silently_dropped() {
let error = run_script_to_program(
r#"const root = I(null, {type: "frame", name: "Root"});
U(root, {x: 10});"#,
)
.expect_err("U() must not look successful while doing nothing");
assert!(error.contains("OP_SCRIPT_MODE_UNSUPPORTED"), "{error}");
assert!(error.contains("operations mode"), "{error}");
}
#[test]
fn console_remains_a_noop_while_insert_records() {
let program = run_script_to_program(
r#"console.log("building card");
console.warn("heads up");
console.error("nope");
console.info("fyi");
console.debug("trace");
G("root", "search", "prompt");
C(null, {type: "frame", name: "Copy"});
U("n1", {x: 10});
D("n2");
M("n3", null);
R("n4", "n5");
I(null, {type: "frame", name: "Root"});
I(null, {type: "text", content: "Hi"});"#,
I(null, {type: "frame", name: "Root"});"#,
)
.expect("pencil-op + console stubs must not abort the script");
let lines: Vec<&str> = program.lines().collect();
assert_eq!(
lines.len(),
2,
"only the two I() calls record a line; stub calls are no-ops"
);
assert!(lines[0].starts_with("b0=I(null, "));
assert!(lines[0].contains(r#""name":"Root""#));
assert!(lines[1].starts_with("b1=I(null, "));
assert!(lines[1].contains(r#""content":"Hi""#));
.expect("console does not affect the design program");
assert_eq!(program.lines().count(), 1);
assert!(program.contains(r#""name":"Root""#));
}
#[test]