Merge PR #3765: Refuse an OCR batch whose inputs would write one output file

Cargo.lock: took main's.
crates/automation/src/a11y.rs: kept both sides (PR's helpers, then main's request_json doc comment).

crates/automation/src/a11y.rs

@@ -606,11 +606,22 @@ impl Automation {
         for p in &paths {
             self.resolve_for(p, crate::file_access::Access::Read, true)?;
         }
+        // Two inputs that would write the same output file overwrite each other while both report
+        // success, so the whole batch is refused here: before the folder is created, anything is
+        // read, or the OCR engines load.
+        let targets: Vec<std::path::PathBuf> = paths.iter().map(|p| child(&folder, &input_file_name(p))).collect();
+        let shared = shared_file_names(&targets);
+        if !shared.is_empty() {
+            return Err(ToolError::InvalidArgs(format!(
+                "more than one input would write to the same output file, so each would replace the one before it. Colliding file names: {}. Give each input a different file name, or run them in separate batches with different folders.",
+                shared.join(", ")
+            )));
+        }
         std::fs::create_dir_all(&folder).map_err(|e| failed(e.to_string()))?;
         let ocr = pdfcraft_engine::ocr::engine().map_err(failed)?;
         let mut out = Vec::new();
         for p in paths {
-            let name = std::path::Path::new(p).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "document.pdf".into());
+            let name = input_file_name(p);
             let result =
                 self.resolve(p, false).map_err(|e| e.to_string()).and_then(|src| crate::read_input_bytes(&src).map_err(|e| e.to_string())).and_then(
                     |bytes| pdfcraft_engine::ocr::recognize_file(&name, std::sync::Arc::new(bytes), None, settings.clone(), &ocr, |_, _| true),
@@ -659,6 +670,36 @@ impl Automation {
     }
 }
 
+/// The file name an input is written under: its own file name, or `document.pdf` when it has none.
+fn input_file_name(path: &str) -> String {
+    std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "document.pdf".into())
+}
+
+/// The output file names that two or more of `targets` share, each spelling listed once, in input
+/// order. Names are compared the way the default filesystem compares them: without regard to case
+/// on Windows and macOS and exactly elsewhere; on Windows a trailing dot or space is not part of
+/// the name (`scan.pdf.` is `scan.pdf`). The folder is the same for every target, so only the
+/// names are compared.
+fn shared_file_names(targets: &[std::path::PathBuf]) -> Vec<String> {
+    let mut groups: std::collections::BTreeMap<String, (usize, Vec<String>)> = std::collections::BTreeMap::new();
+    for target in targets {
+        let name = target.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
+        let key = if cfg!(windows) {
+            name.trim_end_matches(['.', ' ']).to_lowercase()
+        } else if cfg!(target_os = "macos") {
+            name.to_lowercase()
+        } else {
+            name.clone()
+        };
+        let (count, spellings) = groups.entry(key).or_insert_with(|| (0, Vec::new()));
+        *count += 1;
+        if !spellings.contains(&name) {
+            spellings.push(name);
+        }
+    }
+    groups.into_values().filter(|(count, _)| *count > 1).flat_map(|(_, spellings)| spellings).collect()
+}
+
 /// A script's request to the viewer as the JSON that running a script reports; a side panel
 /// goes by its fixed id.
 fn request_json(r: &pdfcraft_engine::js::Request) -> Value {
@@ -1056,3 +1097,21 @@ mod review_html_tests {
         assert!(!html.contains("<script>"));
     }
 }
+
+#[cfg(test)]
+mod shared_output_name_tests {
+    use super::*;
+
+    #[test]
+    fn shared_output_names_follow_the_default_filesystem() {
+        let targets = |names: &[&str]| -> Vec<std::path::PathBuf> { names.iter().map(|n| std::path::Path::new("out").join(n)).collect() };
+        // A spelling that repeats is one shared name, listed once.
+        assert_eq!(shared_file_names(&targets(&["a.pdf", "b.pdf", "a.pdf"])), vec!["a.pdf".to_string()]);
+        assert!(shared_file_names(&targets(&["a.pdf", "b.pdf"])).is_empty());
+        // Case is ignored on Windows and macOS only; a trailing dot is ignored on Windows only.
+        let case = shared_file_names(&targets(&["scan.pdf", "Scan.PDF"]));
+        let dot = shared_file_names(&targets(&["scan.pdf", "scan.pdf."]));
+        assert_eq!(case.len(), if cfg!(any(windows, target_os = "macos")) { 2 } else { 0 }, "{case:?}");
+        assert_eq!(dot.len(), if cfg!(windows) { 2 } else { 0 }, "{dot:?}");
+    }
+}

crates/automation/src/tools.rs

@@ -1216,7 +1216,7 @@ pub fn tools() -> Vec<ToolDef> {
                 }),
                 &["doc"],
             )),
-        t("ocr_recognize_files", "Recognize text in multiple files", "Scan & OCR ▸ Recognize text ▸ In multiple files: read every page of each PDF in paths and write the searchable result into folder under the same name (pages that already have text are left alone unless skip_text_pages is false, as in ocr_recognize). Returns, per file, the output path, word count and skipped pages, or the error.")
+        t("ocr_recognize_files", "Recognize text in multiple files", "Scan & OCR ▸ Recognize text ▸ In multiple files: read every page of each PDF in paths and write the searchable result into folder under the same name (pages that already have text are left alone unless skip_text_pages is false, as in ocr_recognize). Two inputs that would write the same output file are refused before anything is read or written. Returns, per file, the output path, word count and skipped pages, or the error.")
             .cmd("ocr.recognize_batch")
             .with(schema(
                 json!({

crates/automation/tests/it/automation.rs

@@ -6214,6 +6214,60 @@ fn ocr_recognize_files_writes_searchable_copies() {
     assert!(found.to_string().contains("\"page\""), "{found}");
 }
 
+/// Two inputs that share a file name (from different folders) would both write `out/scan.pdf`:
+/// the second output replaces the first while both results report success. The batch is refused
+/// up front, naming the file, before the folder is created, anything is read or the OCR engines
+/// load, so the outcome is the same with or without the models installed.
+#[test]
+fn ocr_recognize_files_refuses_inputs_that_share_a_file_name() {
+    let dir = workdir("ocr-files-same-name");
+    let mut a = auto(&dir);
+    let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": "one/scan.pdf" }));
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": "two/scan.pdf" }));
+    let r = a.call("ocr_recognize_files", &json!({ "paths": ["one/scan.pdf", "two/scan.pdf"], "folder": "out" }));
+    assert!(matches!(&r, Err(ToolError::InvalidArgs(m)) if m.contains("scan.pdf")), "{r:?}");
+    assert!(!dir.join("out").exists(), "nothing was started: the output folder was not created");
+}
+
+/// File names are compared the way the default filesystem compares them: without regard to case
+/// on Windows and macOS (`scan.pdf` and `Scan.PDF` are one file there), and exactly elsewhere,
+/// where they are two files and the batch is not refused.
+#[test]
+fn ocr_recognize_files_compares_names_the_way_the_filesystem_does() {
+    let dir = workdir("ocr-files-case");
+    let mut a = auto(&dir);
+    let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": "one/scan.pdf" }));
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": "two/Scan.PDF" }));
+    let r = a.call("ocr_recognize_files", &json!({ "paths": ["one/scan.pdf", "two/Scan.PDF"], "folder": "out" }));
+    if cfg!(any(windows, target_os = "macos")) {
+        assert!(matches!(&r, Err(ToolError::InvalidArgs(m)) if m.contains("Scan.PDF")), "{r:?}");
+        assert!(!dir.join("out").exists(), "nothing was started: the output folder was not created");
+    } else {
+        assert!(!matches!(&r, Err(ToolError::InvalidArgs(_))), "{r:?}");
+    }
+}
+
+/// Inputs with distinct names are not refused, and the batch writes one searchable copy per input
+/// under its own name. Without the models it stops at the models error, as it did before.
+#[test]
+fn ocr_recognize_files_with_distinct_names_runs_as_before() {
+    let dir = workdir("ocr-files-distinct");
+    let mut a = auto(&dir);
+    let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": "one/scan.pdf" }));
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": "two/other.pdf" }));
+    if ok(&mut a, "ocr_status", json!({}))["available"] == true {
+        let r = ok(&mut a, "ocr_recognize_files", json!({ "paths": ["one/scan.pdf", "two/other.pdf"], "folder": "out" }));
+        assert!(r["files"][0]["error"].is_null() && r["files"][1]["error"].is_null(), "{r}");
+        assert!(dir.join("out/scan.pdf").is_file() && dir.join("out/other.pdf").is_file(), "{r}");
+    } else {
+        let r = a.call("ocr_recognize_files", &json!({ "paths": ["one/scan.pdf", "two/other.pdf"], "folder": "out" }));
+        assert!(matches!(&r, Err(ToolError::Failed(m)) if m.contains("not installed")), "{r:?}");
+    }
+}
+
 #[test]
 fn javascript_through_tools() {
     let dir = workdir("js");