M6: a document script's error is told apart by where it came from, not its wording (#3940)

Review follow-up on #3985: JsOutput deduplicated document-script errors by
their text (document script N: ...), so a field script that threw the
same words was dropped by append, and a document script's error was
dropped by absorb when a field script had already reported that text.
JsOutput now records which errors came from document scripts
(doc_errors) and deduplicates only those; pdfcraft_js::is_doc_error,
which guessed from the wording, is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC

crates/engine/src/js.rs

@@ -19,6 +19,10 @@ pub struct JsOutput {
     pub requests: Vec<Request>,
     /// Script errors (as Acrobat's console shows them).
     pub errors: Vec<String>,
+    /// Which of `errors` are document scripts' (#3940), told apart by where they came from and
+    /// not by their text, which a field script's error can share: each is reported once, however
+    /// often the document scripts run again.
+    pub doc_errors: Vec<String>,
 }
 
 impl JsOutput {
@@ -34,7 +38,8 @@ impl JsOutput {
         // The document scripts run before every field script, so a failing one would repeat
         // once per calculation; it is reported once.
         for e in &o.doc_errors {
-            if !self.errors.contains(e) {
+            if !self.doc_errors.contains(e) {
+                self.doc_errors.push(e.clone());
                 self.errors.push(e.clone());
             }
         }
@@ -45,11 +50,17 @@ impl JsOutput {
         self.console.extend(other.console);
         self.requests.extend(other.requests);
         // As in `absorb`: a document script's error, already reported, is not repeated; a field
-        // script's errors are each their own.
+        // script's errors are each their own, even one worded like a document script's.
+        let mut docs = other.doc_errors;
         for e in other.errors {
-            if !(pdfcraft_js::is_doc_error(&e) && self.errors.contains(&e)) {
-                self.errors.push(e);
+            if let Some(k) = docs.iter().position(|d| *d == e) {
+                docs.swap_remove(k);
+                if self.doc_errors.contains(&e) {
+                    continue;
+                }
+                self.doc_errors.push(e.clone());
             }
+            self.errors.push(e);
         }
     }
 }
@@ -462,22 +473,29 @@ mod review_tests {
     }
 
     /// Review on #3985: merging two outputs reports a failing document script once, as one run
-    /// does, while each field script error stays.
+    /// does, while each field script error stays: told apart by where an error came from, so a
+    /// field script that throws a document script's very words is still reported.
     #[test]
     fn merged_output_reports_a_document_script_error_once() {
-        let run = |errors: &[&str]| JsOutput { errors: errors.iter().map(|e| e.to_string()).collect(), ..JsOutput::default() };
-        let mut out = run(&["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined"]);
-        out.append(run(&["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined"]));
-        assert_eq!(
-            out.errors,
-            ["document script 2: TypeError: not a callable function", "ReferenceError: x is not defined", "ReferenceError: x is not defined"]
-        );
-        // A field script's message that only looks like one (no script number) is still its own.
-        for look_alike in ["document script x: thrown by a field", "document script : thrown by a field"] {
-            out.append(run(&[look_alike]));
-            out.append(run(&[look_alike]));
-            assert_eq!(out.errors.iter().filter(|e| *e == look_alike).count(), 2, "{look_alike}");
-        }
+        let doc = "document script 2: TypeError: not a callable function";
+        let field = "ReferenceError: x is not defined";
+        let run = |error: &str| {
+            let mut out = JsOutput::default();
+            out.absorb(&Outcome { error: Some(error.into()), doc_errors: vec![doc.into()], ..Outcome::default() });
+            out
+        };
+        let mut out = run(field);
+        out.append(run(field));
+        assert_eq!(out.errors, [field, doc, field]);
+        // CodeRabbit on #3985: a field script that throws the document script's text.
+        out.append(run(doc));
+        assert_eq!(out.errors, [field, doc, field, doc]);
+        let alone = run(doc);
+        assert_eq!(alone.errors, [doc, doc], "the field's error and the document script's");
+        let count = |o: &JsOutput| o.errors.iter().filter(|e| *e == doc).count();
+        let mut merged = run(field);
+        merged.append(alone);
+        assert_eq!(count(&merged), 2, "the field's copy stays; the document script's is listed once");
     }
 
     #[test]

crates/js/src/lib.rs

@@ -1274,21 +1274,9 @@ impl HostHooks for CheckedCodeFromStrings {
     }
 }
 
-/// The prefix of a document script's error ([`doc_error`]).
-const DOC_ERROR_PREFIX: &str = "document script ";
-
 /// How [`Outcome::doc_errors`] names a failing document script: by its 1-based place.
 fn doc_error(index: usize, error: &dyn std::fmt::Display) -> String {
-    format!("{DOC_ERROR_PREFIX}{}: {error}", index.saturating_add(1))
-}
-
-/// Whether `error` is one of [`Outcome::doc_errors`]: a document script's failure, which repeats
-/// each time the document scripts run again, unlike a field script's own error.
-pub fn is_doc_error(error: &str) -> bool {
-    error
-        .strip_prefix(DOC_ERROR_PREFIX)
-        .and_then(|rest| rest.split_once(": "))
-        .is_some_and(|(n, _)| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit()))
+    format!("document script {}: {error}", index.saturating_add(1))
 }
 
 /// `doc_scripts` are the document scripts to run with their places among all of them;

crates/ui-egui/src/js_ui.rs

@@ -88,7 +88,13 @@ impl PdfCraftApp {
                 {
                     self.views[i].document_changed(&info);
                 }
-                let out = JsOutput { alerts: o.alerts, console: o.console, requests: o.requests, errors: o.error.into_iter().collect() };
+                let out = JsOutput {
+                    alerts: o.alerts,
+                    console: o.console,
+                    requests: o.requests,
+                    errors: o.error.into_iter().collect(),
+                    doc_errors: Vec::new(),
+                };
                 self.handle_js(id, out);
             }
             Err(e) => self.notify_fmt("{field}: {e}", &[("field", field), ("e", &e.to_string())]),
@@ -128,7 +134,7 @@ impl PdfCraftApp {
                     self.views[i].document_changed(&info);
                 }
                 let (error, result) = (o.error.clone(), o.result.clone());
-                let out = JsOutput { alerts: o.alerts, console: o.console, requests: o.requests, errors: Vec::new() };
+                let out = JsOutput { alerts: o.alerts, console: o.console, requests: o.requests, errors: Vec::new(), doc_errors: Vec::new() };
                 self.handle_js(id, out);
                 match error {
                     Some(e) => self.js_console.log.push(format!("Error: {e}")),