diff --git a/crates/automation/README.md b/crates/automation/README.md index 9da911c932..e25ff9b8b2 100644 --- a/crates/automation/README.md +++ b/crates/automation/README.md @@ -61,7 +61,8 @@ do not keep an old PDF byte buffer alive. This bounds retained text-cache data, not total process memory: active extraction batches, renderers, document buffers and the requested output have separate lifetimes. Search still reads -growing batches (8 to 128 pages), with one lookahead match to determine continuation. +growing batches (8 to 128 pages), with one lookahead match to determine continuation, unless +`stop_at_limit` is set (below), which starts at one page and looks no further than its limit. `text_find` keeps matches from readable pages when another reached page fails extraction. It returns `warnings` with 1-based `page`, `error` and `error_truncated`, plus `warning_count` and @@ -75,6 +76,16 @@ that reaches the end with warnings therefore has `complete: false` **without** ` should follow `next` when present and separately inspect warnings. A capped search can return both. The existing `query`, `count`, `matches` and rectangle semantics are unchanged. +That lookahead means a search with exactly `limit` matches reads on to the end of the document to +learn that no further match exists. `stop_at_limit: true` skips it: the search ends at the +`limit`th match. It reads one page first, then batches of 2, 4, … up to 128 pages, so nothing past +the batch holding that match is read or can add warnings (a match on the first page reads only that +page). The answer carries +`stopped_at_limit`. When that is `true`, `complete` is `false` and `next` is only the position just +past the last match, not a promise of another one; following it may return no matches and +`complete: true`. When it is `false`, the search ran out of pages first and `complete` means what +it always does. + `text_extract`, comment placement and redaction keep strict all-requested-pages-must-succeed semantics; they never silently act on partial text. diff --git a/crates/automation/src/pages.rs b/crates/automation/src/pages.rs index 6db58ab475..9212b20ce6 100644 --- a/crates/automation/src/pages.rs +++ b/crates/automation/src/pages.rs @@ -270,6 +270,8 @@ impl Automation { /// `limit` is reached, so a bounded search doesn't extract (and cache) the whole document /// (#971). One match past the limit supplies `next`. Reached extraction failures are reported /// as bounded warnings; `complete` requires both no continuation and no failed pages. + /// `stop_at_limit` drops that lookahead: the search ends at the `limit`th match, so a lone match + /// never sends it to the end of the document, and `next` is only where to resume (#5328). pub(crate) fn text_find(&mut self, a: &Args) -> Result { self.text_find_with(a, &mut extract_parallel) } @@ -289,11 +291,15 @@ impl Automation { Some(p) => return Err(ToolError::InvalidArgs(format!("start_page {p} is out of range: the document has {n} pages"))), }; let skip = usize::try_from(a.opt_int("skip")?.unwrap_or(0).max(0)).unwrap_or(usize::MAX); + let stop_at_limit = a.opt_bool("stop_at_limit")?.unwrap_or(false); let mut matches = Vec::new(); let mut next = None; + let mut stopped = false; let mut warnings = Vec::new(); let mut warning_count = 0usize; - let (mut first, mut batch) = (start, FIND_FIRST_BATCH); + // A search that stops at its limit starts with one page, so a match on it reads nothing + // more; the batches still double for a match further in. + let (mut first, mut batch) = (start, if stop_at_limit { 1 } else { FIND_FIRST_BATCH }); 'search: while first < n { let end = first.saturating_add(batch).min(n); let pages: Vec = (first..end).collect(); @@ -315,13 +321,19 @@ impl Automation { let skip_here = if p == start { skip } else { 0 }; // One extra match proves that the caller needs a continuation. Include skipped // matches on the starting page when bounding the temporary per-page result. - let needed = limit.saturating_sub(matches.len()).saturating_add(skip_here).saturating_add(1); + let needed = limit.saturating_sub(matches.len()).saturating_add(skip_here).saturating_add(usize::from(!stop_at_limit)); for (k, r) in text.find_opts_limited(query, false, false, needed).into_iter().enumerate().skip(skip_here) { if matches.len() == limit { next = Some(json!({ "start_page": p + 1, "skip": k })); break 'search; } matches.push(json!({ "page": p + 1, "text": text.text_of(r.clone()), "rects": text.line_rects(r) })); + if stop_at_limit && matches.len() == limit { + // Resume just past this match; whether another exists isn't looked at. + next = Some(json!({ "start_page": p + 1, "skip": k.saturating_add(1) })); + stopped = true; + break 'search; + } } } first = end; @@ -329,8 +341,13 @@ impl Automation { } let mut out = json!({ "query": query, "count": matches.len(), "complete": next.is_none() && warning_count == 0, "matches": matches, "warnings_truncated": warning_count > warnings.len(), "warning_count": warning_count, "warnings": warnings }); - if let (Some(next), Some(o)) = (next, out.as_object_mut()) { - o.insert("next".into(), next); + if let Some(o) = out.as_object_mut() { + if let Some(next) = next { + o.insert("next".into(), next); + } + if stop_at_limit { + o.insert("stopped_at_limit".into(), stopped.into()); + } } Ok(out) } diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index acfa204b4b..0e97ffd540 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -146,6 +146,84 @@ fn search_bounds_warning_volume_and_does_not_reach_distant_failures() { assert!(warnings.iter().all(|w| w["error"].as_str().unwrap().len() <= FIND_MAX_WARNING_BYTES && w["error_truncated"] == true)); } +#[test] +fn stop_at_limit_ends_at_a_lone_match_without_reading_the_distant_tail() { + let mut a = Automation::new(); + let id = search_document(&mut a, 200); + // #5328: "Page 1" is only on page 1 (every other readable page reads "Page 2"), and the last + // page fails, so only a search that runs on to the end hears about it. + let requested = std::cell::RefCell::new(Vec::new()); + let mut extract = |bytes: &Arc>, password: Option>, pages: &[usize]| { + requested.borrow_mut().extend_from_slice(pages); + let filler = extract_parallel(bytes, password.clone(), &[1]).remove(0).1; + let first = extract_parallel(bytes, password, &[0]).remove(0).1; + pages + .iter() + .map(|&p| { + ( + p, + match p { + 0 => first.clone(), + 199 => Err("synthetic page 200 failure".into()), + _ => filler.clone(), + }, + ) + }) + .collect::>() + }; + let looked_ahead = a.text_find_with(&Args(&json!({ "doc": id.0, "query": "Page 1", "limit": 1 })), &mut extract).unwrap(); + assert_eq!((looked_ahead["count"].clone(), looked_ahead["warning_count"].clone()), (json!(1), json!(1)), "{looked_ahead}"); + assert!(looked_ahead.get("next").is_none() && looked_ahead.get("stopped_at_limit").is_none()); + + a.texts.remove(&id); + requested.borrow_mut().clear(); + let args = json!({ "doc": id.0, "query": "Page 1", "limit": 1, "stop_at_limit": true }); + let stopped = a.text_find_with(&Args(&args), &mut extract).unwrap(); + assert_eq!(stopped["matches"][0]["page"], 1); + assert_eq!((stopped["count"].clone(), stopped["warning_count"].clone()), (json!(1), json!(0)), "{stopped}"); + assert_eq!((stopped["complete"].clone(), stopped["stopped_at_limit"].clone()), (json!(false), json!(true))); + assert_eq!(stopped["next"], json!({ "start_page": 1, "skip": 1 })); + assert_eq!(*requested.borrow(), [0], "a match on the first page reads only that page"); + + // Resuming finds nothing more and reaches the failing page, which it now reports. + let mut resume = args.clone(); + resume["start_page"] = stopped["next"]["start_page"].clone(); + resume["skip"] = stopped["next"]["skip"].clone(); + let rest = a.text_find_with(&Args(&resume), &mut extract).unwrap(); + assert_eq!((rest["count"].clone(), rest["stopped_at_limit"].clone(), rest["complete"].clone()), (json!(0), json!(false), json!(false))); + assert_eq!(rest["warnings"][0]["page"], 200); + assert!(rest.get("next").is_none()); +} + +#[test] +fn stop_at_limit_continuations_return_every_match_once_and_end_complete() { + let mut a = Automation::new(); + let id = search_document(&mut a, 120); + let all = json(&mut a, "text_find", json!({ "doc": id.0, "query": "1", "limit": 10_000 })); + assert_eq!(all["complete"], true); + // "1" appears twice on pages like 11 and 111, so resuming has to skip within a page. + for limit in [1, 2, 3] { + let mut args = json!({ "doc": id.0, "query": "1", "limit": limit, "stop_at_limit": true }); + let mut pieced = Vec::new(); + for _ in 0..=all["count"].as_u64().unwrap() { + let part = json(&mut a, "text_find", args.clone()); + assert_eq!(part["warning_count"], 0); + pieced.extend(part["matches"].as_array().unwrap().iter().cloned()); + if part["stopped_at_limit"] == false { + assert_eq!(part["complete"], true); + assert!(part.get("next").is_none()); + break; + } + assert_eq!((part["count"].clone(), part["complete"].clone()), (json!(limit), json!(false))); + args["start_page"] = part["next"]["start_page"].clone(); + args["skip"] = part["next"]["skip"].clone(); + } + assert_eq!(pieced, *all["matches"].as_array().unwrap(), "limit {limit}: no match lost or repeated"); + } + let none = json(&mut a, "text_find", json!({ "doc": id.0, "query": "absent", "limit": 1, "stop_at_limit": true })); + assert_eq!((none["count"].clone(), none["complete"].clone(), none["stopped_at_limit"].clone()), (json!(0), json!(true), json!(false))); +} + #[test] fn a_bounded_text_find_reads_only_the_pages_it_needs_and_says_it_stopped() { let dir = std::env::temp_dir().join(format!("pdfcraft-automation-find-{}", std::process::id())); diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 5d20fe1dae..21ec09afee 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -246,12 +246,13 @@ pub fn tools() -> Vec { }), &["doc"], )), - t("text_find", "Find text", "Find a phrase (case-insensitive, whitespace-normalised) and return matches in page order with page and line rectangles in points (origin top-left). Reads growing batches, looking one match past the limit for a continuation. Failed pages do not discard readable matches: warnings contains up to 100 {page, error, error_truncated} entries (errors capped at 1024 UTF-8 bytes), warning_count counts failed pages, and warnings_truncated reports a capped list. complete is false after a page failure or when more matches exist; next is present only for a match-limit continuation.") + t("text_find", "Find text", "Find a phrase (case-insensitive, whitespace-normalised) and return matches in page order with page and line rectangles in points (origin top-left). Reads growing batches, looking one match past the limit for a continuation. Failed pages do not discard readable matches: warnings contains up to 100 {page, error, error_truncated} entries (errors capped at 1024 UTF-8 bytes), warning_count counts failed pages, and warnings_truncated reports a capped list. complete is false after a page failure or when more matches exist; next is present only for a match-limit continuation. With stop_at_limit the search ends at the limit-th match instead of looking for one more, so a lone early match doesn't read the rest of the document.") .ro() .cmd("edit.find") .with(schema(json!({ "doc": doc(), "query": { "type": "string", "minLength": 1 }, "limit": { "type": "integer", "minimum": 1, "description": "Maximum matches (default 500). When another match exists, next holds start_page and skip to continue. complete is false for either additional matches or failed pages; an exhausted search with warnings has no next." }, "start_page": { "type": "integer", "minimum": 1, "description": "Page to start searching from (default 1)." }, - "skip": { "type": "integer", "minimum": 0, "description": "Matches to pass over on start_page before returning any (default 0)." } }), &["doc", "query"])), + "skip": { "type": "integer", "minimum": 0, "description": "Matches to pass over on start_page before returning any (default 0)." }, + "stop_at_limit": { "type": "boolean", "description": "Stop at the limit-th match without looking for another (default false). The answer then has stopped_at_limit; when true, complete is false and next is only where to resume, so following it may find nothing more." } }), &["doc", "query"])), t("page_rotate", "Rotate pages", "Rotate pages by a multiple of 90 degrees (positive is clockwise): the listed pages (default all), filtered like Acrobat's Rotate Pages by subset (all, even, odd page numbers) and orientation (all, landscape, portrait). Undoable.") .cmd("page.rotate") .with(schema(