From 833bf59135d654206d992119b089ba0f7de1cd1c Mon Sep 17 00:00:00 2001 From: Fini Date: Fri, 31 Jul 2026 21:06:11 +0800 Subject: [PATCH] refactor(desktop): retry exhausted stock-image searches from a dedicated arm A node whose stock search came back empty was parked in a terminal failure state with no way back, so a design shipped with placeholder slots even when a second attempt would have found art. Split the retry policy out of `image_enrich_cli` into its own `retry` module and give the session an explicit `retry_search_failures` entry point that re-admits those nodes for a bounded, caller-managed retry. Only Search/Auto nodes are re-admitted: an explicit Generate target that failed is never silently converted into a stock search, since that would substitute different art than the design asked for. `image_request_mode` makes that distinction a property of the node rather than something each call site re-derives. Claude-Session: https://claude.ai/code/session_01FqKQqNj8exYwopGDpYUU7x --- .../op-host-desktop/src/image_enrich_cli.rs | 153 +----- .../src/image_enrich_cli/retry.rs | 493 ++++++++++++++++++ .../src/image_search_session.rs | 12 +- .../src/image_search_session/fetch.rs | 41 +- .../src/image_search_session/targets.rs | 36 +- .../image_search_session_tests/providers.rs | 58 +++ 6 files changed, 628 insertions(+), 165 deletions(-) create mode 100644 crates/op-host-desktop/src/image_enrich_cli/retry.rs diff --git a/crates/op-host-desktop/src/image_enrich_cli.rs b/crates/op-host-desktop/src/image_enrich_cli.rs index 9fb180a63..232b8a278 100644 --- a/crates/op-host-desktop/src/image_enrich_cli.rs +++ b/crates/op-host-desktop/src/image_enrich_cli.rs @@ -8,20 +8,19 @@ //! an explicit Generate target fails instead of silently changing acquisition //! mode. -use std::collections::HashSet; use std::ffi::OsString; use std::fmt; -use std::path::PathBuf; -use std::time::{Duration, Instant}; +use std::path::{Path, PathBuf}; +use std::time::Duration; -use jian_ops_schema::node::PenNode; -use jian_ops_schema::style::PenFill; -use op_editor_core::{walkers, EditorState, NodeId, PenNodeExt}; +use op_editor_core::EditorState; use serde::Serialize; -use crate::image_search_session::{ - collect_targets, ImageSearchSession, SEARCH_FAILED_PLACEHOLDER_SRC, -}; +use crate::image_search_session::ImageSearchSession; +#[cfg(test)] +use crate::image_search_session::SEARCH_FAILED_PLACEHOLDER_SRC; + +mod retry; const DEFAULT_TIMEOUT_SECONDS: u64 = 120; const POLL_INTERVAL: Duration = Duration::from_millis(10); @@ -229,136 +228,27 @@ fn enrich_document(request: &EnrichRequest) -> Result Result { if summary.failed != 0 || summary.unresolved != 0 { return Err(EnrichError::Failed(summary)); } - op_host_services::doc_io::save_to_path(&state, &request.output).map_err(|error| { - EnrichError::Save { - path: request.output.clone(), - message: error.to_string(), - } + op_host_services::doc_io::save_to_path(state, output).map_err(|error| EnrichError::Save { + path: output.to_path_buf(), + message: error.to_string(), })?; Ok(summary) } fn enrich_state(state: &mut EditorState, timeout: Duration) -> Result { let mut session = ImageSearchSession::new(); - enrich_state_with_session(state, timeout, &mut session) -} - -fn enrich_state_with_session( - state: &mut EditorState, - timeout: Duration, - session: &mut ImageSearchSession, -) -> Result { - // One deadline covers the complete enrichment phase across every page. - // Document load and the final atomic save intentionally sit outside it. - let started = Instant::now(); - let timeout_seconds = timeout.as_secs(); - let page_count = state.page_count(); - let mut targets = 0usize; - let mut failed = 0usize; - let mut unresolved = 0usize; - - for page in 0..page_count { - if !state.set_active_page(page) { - return Err(EnrichError::InvalidPage { page, page_count }); - } - let mut target_ids: HashSet = collect_targets(state, &HashSet::new()) - .into_iter() - .map(|target| target.node_id) - .collect(); - collect_failure_sentinel_ids(state.active_children(), &mut target_ids); - targets += target_ids.len(); - - loop { - let scene = op_pen_loader::editor_state_to_active_page_layout_scene(state); - let enqueued = session.enqueue_missing_with_scene(state, &scene); - let was_pending = session.is_pending(); - let changed = session.poll_into_with_scene(state, &scene); - if !session.is_pending() && !enqueued && !was_pending && !changed { - break; - } - // A synchronous or just-completed job gets an immediate - // quiescence pass even at the deadline. Only pending work can - // time out; this prevents a completed final poll from being - // misreported as a timeout. - if !session.is_pending() { - continue; - } - if started.elapsed() >= timeout { - // Poll once more at the boundary so a completion that raced - // the first poll wins over the deadline. - let scene = op_pen_loader::editor_state_to_active_page_layout_scene(state); - let _ = session.poll_into_with_scene(state, &scene); - if !session.is_pending() { - continue; - } - return Err(EnrichError::Timeout { - page, - seconds: timeout_seconds, - }); - } - std::thread::sleep(POLL_INTERVAL); - } - - let remaining = collect_targets(state, &HashSet::new()); - unresolved += remaining.len(); - failed += target_ids - .iter() - .filter(|node_id| node_has_failure_sentinel(state, node_id)) - .count(); - } - - Ok(EnrichSummary { - pages: page_count, - targets, - resolved: targets.saturating_sub(failed.saturating_add(unresolved)), - failed, - unresolved, - }) -} - -fn collect_failure_sentinel_ids(children: &[PenNode], out: &mut HashSet) { - for node in children { - if node_contains_failure_sentinel(node) { - if let Some(id) = NodeId::new_opt(node.id_str()) { - out.insert(id); - } - } - if let Some(children) = node.children() { - collect_failure_sentinel_ids(children, out); - } - } -} - -fn node_contains_failure_sentinel(node: &PenNode) -> bool { - match node { - PenNode::Image(image) => image.src == SEARCH_FAILED_PLACEHOLDER_SRC, - PenNode::Frame(frame) => fills_have_failure_sentinel(frame.container.fill.as_deref()), - PenNode::Rectangle(rectangle) => { - fills_have_failure_sentinel(rectangle.container.fill.as_deref()) - } - _ => false, - } -} - -fn node_has_failure_sentinel(state: &EditorState, node_id: &NodeId) -> bool { - let Some(node) = walkers::find_node(state.active_children(), node_id) else { - return false; - }; - node_contains_failure_sentinel(node) -} - -fn fills_have_failure_sentinel(fills: Option<&[PenFill]>) -> bool { - fills.is_some_and(|fills| { - fills.iter().any(|fill| { - matches!( - fill, - PenFill::Image(image) if image.url == SEARCH_FAILED_PLACEHOLDER_SRC - ) - }) - }) + retry::enrich_state_with_session(state, timeout, &mut session) } #[cfg(test)] @@ -499,7 +389,7 @@ mod tests { } #[test] - fn preexisting_failure_sentinel_is_counted_as_a_failed_target() { + fn preexisting_generate_failure_sentinel_is_terminal() { let source = format!( r#"{{ "version": "1.0", @@ -509,6 +399,7 @@ mod tests { "id": "failed", "name": "Failed search", "src": "{SEARCH_FAILED_PLACEHOLDER_SRC}", + "imagePrompt": "paint a moonlit forest", "width": 160, "height": 90 }} diff --git a/crates/op-host-desktop/src/image_enrich_cli/retry.rs b/crates/op-host-desktop/src/image_enrich_cli/retry.rs new file mode 100644 index 000000000..ad81c1ced --- /dev/null +++ b/crates/op-host-desktop/src/image_enrich_cli/retry.rs @@ -0,0 +1,493 @@ +use std::collections::{HashMap, HashSet}; +use std::time::{Duration, Instant}; + +use jian_ops_schema::node::PenNode; +use jian_ops_schema::style::{PenFill, SolidFillBody}; +use op_editor_core::{walkers, EditorState, NodeId, PenNodeExt as _}; + +use crate::image_search_session::{ + collect_targets, image_request_mode, ImageRequestMode, ImageSearchSession, + SEARCH_FAILED_PLACEHOLDER_SRC, +}; + +use super::{EnrichError, EnrichSummary, POLL_INTERVAL}; + +// Stock providers occasionally return a transient empty result. Three total +// attempts bound latency while recovering isolated failures in large batches. +const MAX_STOCK_SEARCH_ATTEMPTS: usize = 3; + +pub(super) trait EnrichSession { + fn enqueue( + &mut self, + state: &EditorState, + scene: &op_editor_ui::layout_scene::LayoutScene, + ) -> bool; + fn poll( + &mut self, + state: &mut EditorState, + scene: &op_editor_ui::layout_scene::LayoutScene, + ) -> bool; + fn is_pending(&self) -> bool; + fn prepare_search_retry(&mut self, node_ids: &HashSet); +} + +impl EnrichSession for ImageSearchSession { + fn enqueue( + &mut self, + state: &EditorState, + scene: &op_editor_ui::layout_scene::LayoutScene, + ) -> bool { + self.enqueue_missing_with_scene(state, scene) + } + + fn poll( + &mut self, + state: &mut EditorState, + scene: &op_editor_ui::layout_scene::LayoutScene, + ) -> bool { + self.poll_into_with_scene(state, scene) + } + + fn is_pending(&self) -> bool { + self.is_pending() + } + + fn prepare_search_retry(&mut self, node_ids: &HashSet) { + self.retry_search_failures(node_ids); + } +} + +#[derive(Clone)] +struct TargetRecord { + mode: ImageRequestMode, + query: String, + reset_node: PenNode, +} + +pub(super) fn enrich_state_with_session( + state: &mut EditorState, + timeout: Duration, + session: &mut S, +) -> Result { + // One deadline covers every retry on every page. Document load and the + // final atomic save intentionally sit outside it. + let started = Instant::now(); + let timeout_seconds = timeout.as_secs(); + let page_count = state.page_count(); + let mut targets = 0usize; + let mut failed = 0usize; + let mut unresolved = 0usize; + + for page in 0..page_count { + if !state.set_active_page(page) { + return Err(EnrichError::InvalidPage { page, page_count }); + } + let records = collect_target_records(state); + targets += records.len(); + + // A previous invocation may have persisted a failure sentinel. Only + // Search/Auto nodes are cleared; Generate remains a terminal failure. + let preexisting_retry_ids = retryable_failure_ids(state, &records); + restore_retry_nodes(state, &records, &preexisting_retry_ids); + session.prepare_search_retry(&preexisting_retry_ids); + + for attempt in 0..MAX_STOCK_SEARCH_ATTEMPTS { + drive_until_quiescent(state, session, page, started, timeout, timeout_seconds)?; + let retry_ids = retryable_failure_ids(state, &records); + if retry_ids.is_empty() || attempt + 1 == MAX_STOCK_SEARCH_ATTEMPTS { + break; + } + restore_retry_nodes(state, &records, &retry_ids); + session.prepare_search_retry(&retry_ids); + } + report_failed_targets(state, &records); + + let remaining_ids: HashSet = collect_targets(state, &HashSet::new()) + .into_iter() + .map(|target| target.node_id) + .collect(); + unresolved += records + .keys() + .filter(|node_id| remaining_ids.contains(*node_id)) + .count(); + failed += records + .keys() + .filter(|node_id| node_has_failure_sentinel(state, node_id)) + .count(); + } + + Ok(EnrichSummary { + pages: page_count, + targets, + resolved: targets.saturating_sub(failed.saturating_add(unresolved)), + failed, + unresolved, + }) +} + +fn drive_until_quiescent( + state: &mut EditorState, + session: &mut S, + page: usize, + started: Instant, + timeout: Duration, + timeout_seconds: u64, +) -> Result<(), EnrichError> { + loop { + let scene = op_pen_loader::editor_state_to_active_page_layout_scene(state); + let enqueued = session.enqueue(state, &scene); + let was_pending = session.is_pending(); + let changed = session.poll(state, &scene); + if !session.is_pending() && !enqueued && !was_pending && !changed { + return Ok(()); + } + // A synchronous or just-completed job gets an immediate quiescence + // pass at the deadline. Only pending work can time out. + if !session.is_pending() { + continue; + } + if started.elapsed() >= timeout { + let scene = op_pen_loader::editor_state_to_active_page_layout_scene(state); + let _ = session.poll(state, &scene); + if !session.is_pending() { + continue; + } + return Err(EnrichError::Timeout { + page, + seconds: timeout_seconds, + }); + } + std::thread::sleep(POLL_INTERVAL); + } +} + +fn collect_target_records(state: &EditorState) -> HashMap { + let mut records = HashMap::new(); + for target in collect_targets(state, &HashSet::new()) { + if let Some(node) = walkers::find_node(state.active_children(), &target.node_id) { + records.insert( + target.node_id, + TargetRecord { + mode: target.mode, + query: target.query, + reset_node: node.clone(), + }, + ); + } + } + collect_sentinel_records(state.active_children(), &mut records); + records +} + +fn collect_sentinel_records(children: &[PenNode], records: &mut HashMap) { + for node in children { + if node_contains_failure_sentinel(node) { + if let Some(node_id) = NodeId::new_opt(node.id_str()) { + let mode = image_request_mode(node); + records.entry(node_id).or_insert_with(|| TargetRecord { + mode, + query: String::new(), + reset_node: retry_reset_node(node, mode), + }); + } + } + if let Some(children) = node.children() { + collect_sentinel_records(children, records); + } + } +} + +fn report_failed_targets(state: &EditorState, records: &HashMap) { + for (node_id, record) in records { + if !node_has_failure_sentinel(state, node_id) { + continue; + } + let name = record.reset_node.base().name.as_deref().unwrap_or(""); + eprintln!( + "[ENRICH] failed node={} mode={:?} name={:?} query={:?}", + node_id.as_str(), + record.mode, + name, + record.query + ); + } +} + +fn retry_reset_node(node: &PenNode, mode: ImageRequestMode) -> PenNode { + let mut reset = node.clone(); + if mode == ImageRequestMode::Generate { + return reset; + } + match &mut reset { + PenNode::Image(image) => image.src = "".into(), + PenNode::Frame(frame) => reset_failure_fills(frame.container.fill.as_mut()), + PenNode::Rectangle(rectangle) => { + reset_failure_fills(rectangle.container.fill.as_mut()); + } + _ => {} + } + reset +} + +fn reset_failure_fills(fills: Option<&mut Vec>) { + let Some(fills) = fills else { + return; + }; + for fill in fills { + if matches!( + fill, + PenFill::Image(image) if image.url == SEARCH_FAILED_PLACEHOLDER_SRC + ) { + *fill = PenFill::Solid(SolidFillBody { + color: "#D1D5DB".to_string(), + explain: None, + opacity: None, + blend_mode: None, + }); + } + } +} + +fn retryable_failure_ids( + state: &EditorState, + records: &HashMap, +) -> HashSet { + records + .iter() + .filter(|(node_id, record)| { + record.mode != ImageRequestMode::Generate && node_has_failure_sentinel(state, node_id) + }) + .map(|(node_id, _)| node_id.clone()) + .collect() +} + +fn restore_retry_nodes( + state: &mut EditorState, + records: &HashMap, + node_ids: &HashSet, +) { + let mut changed = false; + for node_id in node_ids { + let Some(record) = records.get(node_id) else { + continue; + }; + let Some(node) = walkers::find_node_mut(state.active_children_mut(), node_id) else { + continue; + }; + *node = record.reset_node.clone(); + changed = true; + } + if changed { + state.mark_document_changed(); + } +} + +fn node_has_failure_sentinel(state: &EditorState, node_id: &NodeId) -> bool { + walkers::find_node(state.active_children(), node_id).is_some_and(node_contains_failure_sentinel) +} + +fn node_contains_failure_sentinel(node: &PenNode) -> bool { + match node { + PenNode::Image(image) => image.src == SEARCH_FAILED_PLACEHOLDER_SRC, + PenNode::Frame(frame) => fills_have_failure_sentinel(frame.container.fill.as_deref()), + PenNode::Rectangle(rectangle) => { + fills_have_failure_sentinel(rectangle.container.fill.as_deref()) + } + _ => false, + } +} + +fn fills_have_failure_sentinel(fills: Option<&[PenFill]>) -> bool { + fills.is_some_and(|fills| { + fills.iter().any(|fill| { + matches!( + fill, + PenFill::Image(image) if image.url == SEARCH_FAILED_PLACEHOLDER_SRC + ) + }) + }) +} + +#[cfg(test)] +mod tests { + use std::collections::{HashMap, HashSet, VecDeque}; + + use super::*; + use crate::image_search_session::apply_result; + + #[derive(Default)] + struct ScriptedSession { + outcomes: HashMap>>, + attempts: HashMap, + completed: HashSet, + pending: Vec<(NodeId, ImageRequestMode)>, + } + + impl ScriptedSession { + fn with_outcomes(node_id: &str, outcomes: Vec>) -> Self { + Self { + outcomes: HashMap::from([( + node_id.to_string(), + outcomes + .into_iter() + .map(|value| value.map(str::to_string)) + .collect(), + )]), + ..Self::default() + } + } + + fn and_outcomes(mut self, node_id: &str, outcomes: Vec>) -> Self { + self.outcomes.insert( + node_id.to_string(), + outcomes + .into_iter() + .map(|value| value.map(str::to_string)) + .collect(), + ); + self + } + } + + impl EnrichSession for ScriptedSession { + fn enqueue( + &mut self, + state: &EditorState, + _scene: &op_editor_ui::layout_scene::LayoutScene, + ) -> bool { + let mut known = self.completed.clone(); + known.extend( + self.pending + .iter() + .map(|(node_id, _)| node_id.as_str().to_string()), + ); + let targets = collect_targets(state, &known); + for target in targets { + *self + .attempts + .entry(target.node_id.as_str().to_string()) + .or_default() += 1; + self.pending.push((target.node_id, target.mode)); + } + !self.pending.is_empty() + } + + fn poll( + &mut self, + state: &mut EditorState, + _scene: &op_editor_ui::layout_scene::LayoutScene, + ) -> bool { + let pending = std::mem::take(&mut self.pending); + let mut changed = false; + for (node_id, _) in pending { + let id = node_id.as_str().to_string(); + let outcome = self + .outcomes + .get_mut(&id) + .and_then(VecDeque::pop_front) + .flatten(); + let url = outcome.as_deref().unwrap_or(SEARCH_FAILED_PLACEHOLDER_SRC); + changed |= apply_result(state, &node_id, url); + self.completed.insert(id); + } + changed + } + + fn is_pending(&self) -> bool { + !self.pending.is_empty() + } + + fn prepare_search_retry(&mut self, node_ids: &HashSet) { + for node_id in node_ids { + self.completed.remove(node_id.as_str()); + } + } + } + + fn load(source: &str) -> EditorState { + op_host_services::doc_io::load_editor_state_from_source( + source, + op_editor_core::Locale::EnUs, + ) + .expect("load enrichment fixture") + } + + #[test] + fn retryable_failure_succeeds_without_replacing_first_round_success() { + let mut state = load( + r#"{"version":"1.0","children":[{"type":"image","id":"stable","src":"","imageSearchQuery":"forest trail","width":160,"height":90},{"type":"image","id":"retry","src":"","imageSearchQuery":"mountain lake","width":160,"height":90}]}"#, + ); + let mut session = + ScriptedSession::with_outcomes("stable", vec![Some("data:image/png;base64,AA==")]) + .and_outcomes("retry", vec![None, Some("data:image/png;base64,AQ==")]); + + let summary = + enrich_state_with_session(&mut state, Duration::ZERO, &mut session).expect("enrich"); + + assert_eq!(session.attempts.get("stable"), Some(&1)); + assert_eq!(session.attempts.get("retry"), Some(&2)); + assert_eq!( + summary, + EnrichSummary { + pages: 1, + targets: 2, + resolved: 2, + failed: 0, + unresolved: 0, + } + ); + let sources: Vec<_> = state + .active_children() + .iter() + .map(|node| match node { + PenNode::Image(image) => image.src.to_string(), + _ => panic!("expected image"), + }) + .collect(); + assert_eq!( + sources, + ["data:image/png;base64,AA==", "data:image/png;base64,AQ=="] + ); + } + + #[test] + fn explicit_generate_failure_is_not_retried() { + let mut state = load( + r#"{"version":"1.0","children":[{"type":"image","id":"art","src":"","imagePrompt":"paint a moonlit forest","width":160,"height":90}]}"#, + ); + let mut session = ScriptedSession::with_outcomes("art", vec![None]); + + let summary = + enrich_state_with_session(&mut state, Duration::ZERO, &mut session).expect("enrich"); + + assert_eq!(session.attempts.get("art"), Some(&1)); + assert_eq!(summary.failed, 1); + assert_eq!(summary.unresolved, 0); + } + + #[test] + fn exhausted_search_retries_preserve_existing_output() { + let mut state = load( + r#"{"version":"1.0","children":[{"type":"image","id":"photo","src":"","imageSearchQuery":"mountain lake","width":160,"height":90}]}"#, + ); + let mut session = ScriptedSession::with_outcomes("photo", vec![None, None, None]); + let summary = + enrich_state_with_session(&mut state, Duration::ZERO, &mut session).expect("enrich"); + assert_eq!(session.attempts.get("photo"), Some(&3)); + + let directory = + std::env::temp_dir().join(format!("openpencil-enrich-retry-{}", std::process::id())); + std::fs::create_dir_all(&directory).expect("create fixture directory"); + let output = directory.join("output.op"); + std::fs::write(&output, b"keep-existing-output").expect("seed output"); + + let result = super::super::save_enriched_state(&state, &output, summary); + + assert!(matches!(result, Err(EnrichError::Failed(_)))); + assert_eq!( + std::fs::read(&output).expect("read preserved output"), + b"keep-existing-output" + ); + std::fs::remove_dir_all(directory).expect("remove fixture directory"); + } +} diff --git a/crates/op-host-desktop/src/image_search_session.rs b/crates/op-host-desktop/src/image_search_session.rs index fdb945b6c..bb3456cca 100644 --- a/crates/op-host-desktop/src/image_search_session.rs +++ b/crates/op-host-desktop/src/image_search_session.rs @@ -242,6 +242,16 @@ impl ImageSearchSession { !self.jobs.is_empty() } + /// Re-admit terminal stock-search failures for a bounded caller-managed + /// retry. Callers must restore only Search/Auto nodes before invoking this; + /// explicit Generate targets are intentionally never re-admitted here. + pub(crate) fn retry_search_failures(&mut self, node_ids: &HashSet) { + for node_id in node_ids { + self.completed.remove(node_id.as_str()); + } + self.invalidate_scan_gate(); + } + /// Force the next `enqueue_missing` to re-walk the tree even when the /// `(revision, active page)` key looks unchanged (e.g. because a fresh /// `EditorState` restarts its revision at 0 and its active page index at @@ -588,7 +598,7 @@ mod targets; use fetch::fetch_first_image_url_blocking; pub(crate) use fetch::fetch_image_data_url; -pub(crate) use targets::collect_targets; +pub(crate) use targets::{collect_targets, image_request_mode}; use targets::{ collect_targets_with_scene, current_intent_fingerprints, intent_fingerprint, is_frame_placeholder_still_unfilled, is_image_area_rectangle_by_heuristic, diff --git a/crates/op-host-desktop/src/image_search_session/fetch.rs b/crates/op-host-desktop/src/image_search_session/fetch.rs index 671686299..9e3af4f03 100644 --- a/crates/op-host-desktop/src/image_search_session/fetch.rs +++ b/crates/op-host-desktop/src/image_search_session/fetch.rs @@ -122,10 +122,19 @@ fn result_title(result: &serde_json::Value) -> String { .to_lowercase() } +fn meaningful_tokens(value: &str) -> HashSet { + value + .to_lowercase() + .split(|character: char| !character.is_alphanumeric()) + .filter(|token| token.chars().count() > 2) + .map(str::to_string) + .collect() +} + /// Pick the best of the returned results instead of blindly trusting rank 1: -/// drop junk-titled entries, then prefer the first whose title shares a word -/// with the query (Openverse relevance degrades fast on niche queries), then -/// the first non-junk entry. +/// drop junk/used entries, then rank by complete query-token overlap. Equal +/// scores preserve provider order, and an all-zero set falls back to the first +/// usable result. pub(crate) fn select_openverse_result<'results>( results: &'results [serde_json::Value], query: &str, @@ -152,20 +161,18 @@ pub(crate) fn select_openverse_result<'results>( .any(|marker| title.contains(marker)) }) .collect(); - let query_words: Vec = query - .to_lowercase() - .split_whitespace() - .filter(|w| w.len() > 2) - .map(str::to_string) - .collect(); - non_junk - .iter() - .find(|result| { - let title = result_title(result); - query_words.iter().any(|word| title.contains(word.as_str())) - }) - .copied() - .or_else(|| non_junk.first().copied()) + let query_tokens = meaningful_tokens(query); + let mut best = None; + let mut best_overlap = 0usize; + for result in non_junk { + let title_tokens = meaningful_tokens(&result_title(result)); + let overlap = query_tokens.intersection(&title_tokens).count(); + if best.is_none() || overlap > best_overlap { + best = Some(result); + best_overlap = overlap; + } + } + best } fn openverse_result_identity(result: &serde_json::Value) -> Option { diff --git a/crates/op-host-desktop/src/image_search_session/targets.rs b/crates/op-host-desktop/src/image_search_session/targets.rs index cbe391b26..000ad4c86 100644 --- a/crates/op-host-desktop/src/image_search_session/targets.rs +++ b/crates/op-host-desktop/src/image_search_session/targets.rs @@ -240,7 +240,25 @@ fn image_search_target_for( PenNode::Image(image) => image.image_prompt.clone(), _ => None, }; - let mode = match node { + let mode = image_request_mode(node); + 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(width, height), + prompt, + mode, + width, + height, + }) +} + +pub(crate) fn image_request_mode(node: &PenNode) -> ImageRequestMode { + 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 @@ -282,21 +300,7 @@ fn image_search_target_for( 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(width, height), - prompt, - mode, - width, - height, - }) + } } fn is_placeholder_src(src: &str) -> bool { diff --git a/crates/op-host-desktop/src/image_search_session_tests/providers.rs b/crates/op-host-desktop/src/image_search_session_tests/providers.rs index d92460da3..7dad0115e 100644 --- a/crates/op-host-desktop/src/image_search_session_tests/providers.rs +++ b/crates/op-host-desktop/src/image_search_session_tests/providers.rs @@ -133,6 +133,64 @@ fn openverse_selection_skips_junk_and_prefers_query_overlap() { assert_ne!(second["url"], "https://x/3.jpg", "used URL is skipped"); } +#[test] +fn openverse_selection_ranks_by_complete_token_overlap_stably() { + use serde_json::json; + let empty = std::collections::HashSet::new(); + + let ranked = vec![ + json!({"title": "Limestone architecture", "url": "https://x/one.jpg"}), + json!({"title": "Limestone lounge chair in a quiet studio", "url": "https://x/many.jpg"}), + ]; + let picked = select_openverse_result( + &ranked, + "limestone lounge chair editorial furniture", + &empty, + ) + .expect("ranked result"); + assert_eq!( + picked["url"], "https://x/many.jpg", + "more complete query tokens beat an earlier one-token match" + ); + + let substring = vec![ + json!({"title": "Cathedral facade", "url": "https://x/substring.jpg"}), + json!({"title": "Minimal lounge interior", "url": "https://x/exact.jpg"}), + ]; + let picked = select_openverse_result(&substring, "cat lounge", &empty).expect("exact result"); + assert_eq!( + picked["url"], "https://x/exact.jpg", + "a query token inside a longer title word is not an overlap" + ); + + let tied = vec![ + json!({"title": "Oak lounge", "url": "https://x/first.jpg"}), + json!({"title": "Lounge lighting", "url": "https://x/second.jpg"}), + ]; + let picked = + select_openverse_result(&tied, "lounge chair", &empty).expect("stable tied result"); + assert_eq!( + picked["url"], "https://x/first.jpg", + "provider order breaks equal-overlap ties" + ); +} + +#[test] +fn openverse_selection_still_excludes_used_and_junk_before_ranking() { + use serde_json::json; + let results = vec![ + json!({"id": "junk", "title": "Limestone lounge chair placeholder", "url": "https://x/junk.jpg"}), + json!({"id": "used", "title": "Limestone lounge chair studio", "url": "https://x/used.jpg"}), + json!({"id": "available", "title": "Limestone chair", "url": "https://x/available.jpg"}), + ]; + let used = std::collections::HashSet::from(["openverse:used".to_string()]); + + let picked = select_openverse_result(&results, "limestone lounge chair", &used) + .expect("unused non-junk result"); + + assert_eq!(picked["url"], "https://x/available.jpg"); +} + #[test] fn simplify_strips_design_artifact_words_but_never_to_empty() { // "synthwave album cover neon" → the corpus has no album covers, but it