M3: text_find can stop at its limit without reading on to the end (#5328) (#5335)

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.
 

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<Value> {
         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<usize> = (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)
     }

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<Vec<u8>>, password: Option<Arc<str>>, 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::<Vec<_>>()
+    };
+    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()));

crates/automation/src/tools.rs

@@ -246,12 +246,13 @@ pub fn tools() -> Vec<ToolDef> {
                 }),
                 &["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(