Review fixes for #777: report sections per document, fmt, docs and ROADMAP

The report kept a placed document's LINKED FILES and FONTS inside its parent's list. Each document
now gets its own sections, the packaged one first and unchanged (no Document: line), then each
placed document's, headed by its file. Drops a debug_assert and an unneeded mut, runs rustfmt, and
documents the nested packaging in docs/mcp.md and the ROADMAP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/doc/src/links.rs

@@ -276,11 +276,8 @@ impl Document {
                 walk(child, f);
             }
         }
-        let roots = self
-            .layers
-            .iter_mut()
-            .chain(self.symbols.iter_mut().map(|s| &mut s.art))
-            .chain(self.patterns.iter_mut().flat_map(|p| &mut p.art));
+        let roots =
+            self.layers.iter_mut().chain(self.symbols.iter_mut().map(|s| &mut s.art)).chain(self.patterns.iter_mut().flat_map(|p| &mut p.art));
         for root in roots {
             walk(root, &mut f);
         }

crates/engine/src/cmd/package.rs

@@ -8,8 +8,8 @@ use std::collections::{BTreeMap, HashSet};
 use std::io::Write;
 use std::path::{Path, PathBuf};
 
-use vectorcraft_doc::{Document, LinkInfo};
 use vectorcraft_doc::links::hash_bytes;
+use vectorcraft_doc::{Document, LinkInfo};
 
 use serde_json::{Value, json};
 
@@ -95,10 +95,8 @@ fn relative_in_package(document: &str, asset: &str) -> String {
 }
 
 fn safe_asset_name(name: &str) -> String {
-    let name: String = name
-        .chars()
-        .map(|c| if c.is_control() || matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c })
-        .collect();
+    let name: String =
+        name.chars().map(|c| if c.is_control() || matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c }).collect();
     let name = name.trim().trim_matches('.');
     if name.is_empty() { "asset".into() } else { name.into() }
 }
@@ -119,7 +117,9 @@ struct Collector<'a> {
     missing: Vec<String>,
     skipped: Vec<Value>,
     warnings: Vec<String>,
-    lines: Vec<String>,
+    /// The report's lines, one section per document: the packaged one first, then each placed
+    /// document in the order it is reached.
+    sections: Vec<Vec<String>>,
 }
 
 impl<'a> Collector<'a> {
@@ -139,37 +139,47 @@ impl<'a> Collector<'a> {
             missing: Vec::new(),
             skipped: Vec::new(),
             warnings: Vec::new(),
-            lines: Vec::new(),
+            sections: Vec::new(),
         }
     }
 
-    fn warn(&mut self, message: String) {
-        self.lines.push(format!("WARNING: {message}"));
+    fn warn(&mut self, lines: &mut Vec<String>, message: String) {
+        lines.push(format!("WARNING: {message}"));
         self.warnings.push(message);
     }
 
     fn collect(&mut self, doc: &mut Document, source: &str, destination: &str, depth: usize) -> Result<()> {
         // The caller checks cycles and depth before descending; active documents have
         // a destination reserved so back-links can point at their single packaged copy.
-        let previous = self.visiting.insert(source.to_string(), destination.to_string());
-        debug_assert!(previous.is_none(), "collector visits each document once");
+        self.visiting.insert(source.to_string(), destination.to_string());
+        // This document's section comes before those of the documents it places.
+        let section = self.sections.len();
+        self.sections.push(vec![]);
+        let mut lines = vec![];
+        // The packaged document's sections read as before; a placed document's name its file.
+        let heading = |lines: &mut Vec<String>, title: &str| {
+            lines.push(title.into());
+            if depth > 0 {
+                lines.push(format!("Document: {source}"));
+            }
+        };
         // A file's saved path is only meaningful inside its own document.
         // Different linked documents can reuse the same stale absolute path yet
         // resolve it to distinct local assets. Keep a mapping per document.
         let mut local = BTreeMap::<String, Copied>::new();
         if self.opts.copy_links {
             let mut placed = HashSet::new();
-            doc.visit_placed(|_, p| { placed.insert(p.link.path.clone()); });
-            // Keep this exact header for programs parsing Package reports.
-            self.lines.push("LINKED FILES".into());
-            self.lines.push(format!("Document: {source}"));
+            doc.visit_placed(|_, p| {
+                placed.insert(p.link.path.clone());
+            });
+            heading(&mut lines, "LINKED FILES");
             for file in super::links::linked_files(doc, Some(source)) {
                 let found_path = file.found_path.as_deref().unwrap_or(&file.path);
                 if let Some(ancestor) = self.visiting.get(found_path).cloned() {
                     // The ancestor is already included (possibly the root document). A
                     // cyclic reference cannot have a final content hash until both files
                     // have been written, so leave its size/hash unspecified, not stale.
-                    self.warn(format!("{}: circular placed-document link to {ancestor}; stopped following the cycle", file.path));
+                    self.warn(&mut lines, format!("{}: circular placed-document link to {ancestor}; stopped following the cycle", file.path));
                     local.insert(file.path.clone(), Copied { name: ancestor, size: None, hash: None });
                     continue;
                 }
@@ -178,7 +188,7 @@ impl<'a> Collector<'a> {
                     continue;
                 }
                 let Some(mut bytes) = file.bytes else {
-                    self.lines.push(format!("{}: not found ({}), not copied", file.name, file.path));
+                    lines.push(format!("{}: not found ({}), not copied", file.name, file.path));
                     self.missing.push(if depth == 0 { file.name.clone() } else { format!("{} ({source})", file.name) });
                     continue;
                 };
@@ -191,18 +201,15 @@ impl<'a> Collector<'a> {
                 let destination_file = format!("{dir}{unique}");
                 if placed.contains(&file.path) {
                     if depth >= vectorcraft_doc::placed_document::MAX_DEPTH {
-                        self.warn(format!("{}: nesting limit reached; copied unchanged without collecting its own links", file.path));
+                        self.warn(&mut lines, format!("{}: nesting limit reached; copied unchanged without collecting its own links", file.path));
                     } else {
                         match vectorcraft_format::load_file(&bytes) {
                             Ok(saved) => {
                                 let preview = vectorcraft_format::preview(&bytes);
                                 let pdf = vectorcraft_format::pdf_content(&bytes);
                                 let compressed = bytes.starts_with(&[0x1f, 0x8b]);
-                                let include_linked = saved
-                                    .doc
-                                    .linked_only_images()
-                                    .iter()
-                                    .any(|key| saved.doc.images.get(key).is_some_and(|blob| !blob.is_proxy()));
+                                let include_linked =
+                                    saved.doc.linked_only_images().iter().any(|key| saved.doc.images.get(key).is_some_and(|blob| !blob.is_proxy()));
                                 let mut nested = saved.doc;
                                 self.collect(&mut nested, found_path, &destination_file, depth + 1)?;
                                 if self.opts.relink {
@@ -215,26 +222,34 @@ impl<'a> Collector<'a> {
                                     options.include_linked = include_linked;
                                     match vectorcraft_format::save_with(&nested, &options) {
                                         Ok(rewritten) => bytes = rewritten,
-                                        Err(e) => self.warn(format!("{}: could not rewrite linked document ({e}); copied unchanged, its own links were not relinked", file.path)),
+                                        Err(e) => self.warn(
+                                            &mut lines,
+                                            format!(
+                                                "{}: could not rewrite linked document ({e}); copied unchanged, its own links were not relinked",
+                                                file.path
+                                            ),
+                                        ),
                                     }
                                 }
                             }
-                            Err(e) => self.warn(format!("{}: unreadable linked document ({e}); copied unchanged, its own links were not collected", file.path)),
+                            Err(e) => self.warn(
+                                &mut lines,
+                                format!("{}: unreadable linked document ({e}); copied unchanged, its own links were not collected", file.path),
+                            ),
                         }
                     }
                 }
                 let copy = Copied { name: destination_file.clone(), size: Some(bytes.len() as u64), hash: Some(hash_bytes(&bytes)) };
                 self.copies.insert(found_path.to_string(), copy.clone());
                 local.insert(file.path.clone(), copy);
-                self.lines.push(format!("{} → {destination_file}", file.path));
+                lines.push(format!("{} → {destination_file}", file.path));
                 self.entries.push((destination_file, bytes));
             }
-            self.lines.push(String::new());
+            lines.push(String::new());
         }
         if self.opts.copy_fonts {
             let db = vectorcraft_text::FontDb::global();
-            self.lines.push("FONTS".into());
-            self.lines.push(format!("Document: {source}"));
+            heading(&mut lines, "FONTS");
             for used in super::fonts::used_fonts(doc) {
                 let font = super::fonts::font_label(&used);
                 let (family, style, version) = used;
@@ -257,16 +272,16 @@ impl<'a> Collector<'a> {
                             self.entries.push((format!("Fonts/{file}"), data.to_vec()));
                             self.fonts += 1;
                         }
-                        self.lines.push(format!("{font}{shown} → Fonts/{file}"));
+                        lines.push(format!("{font}{shown} → Fonts/{file}"));
                         continue;
                     }
                     Some(_) => "its licence doesn't allow embedding",
                     None => "not available on this computer",
                 };
-                self.lines.push(format!("{font}{shown}: {reason}, not copied"));
+                lines.push(format!("{font}{shown}: {reason}, not copied"));
                 self.skipped.push(json!({ "font": font, "reason": reason }));
             }
-            self.lines.push(String::new());
+            lines.push(String::new());
         }
         if self.opts.relink {
             // The child files have already been rewritten; the hash and size of each
@@ -282,7 +297,10 @@ impl<'a> Collector<'a> {
                 }
             });
         }
-        let _ = self.visiting.remove(source);
+        if let Some(s) = self.sections.get_mut(section) {
+            *s = lines;
+        }
+        self.visiting.remove(source);
         Ok(())
     }
 }
@@ -319,7 +337,7 @@ fn package(s: &mut Session, p: &Value) -> Result<Value> {
     let mut entries = std::mem::take(&mut collector.entries);
     entries.insert(0, (doc_name.clone(), vectorcraft_format::save_file(&doc)));
     let mut taken = std::mem::take(&mut collector.taken);
-    let mut lines = std::mem::take(&mut collector.lines);
+    let lines = collector.sections.concat();
     let missing = std::mem::take(&mut collector.missing);
     let skipped = std::mem::take(&mut collector.skipped);
     let warnings = std::mem::take(&mut collector.warnings);

crates/engine/src/tests_package.rs

@@ -317,13 +317,8 @@ fn package_keeps_distinct_linked_documents_with_colliding_file_names() {
     assert_eq!(result["links"], 4, "{result}");
     assert_eq!(result["missingLinks"], json!([]));
     let folder = dir.0.join("delivery/poster Folder");
-    let packaged: Vec<_> = result["files"]
-        .as_array()
-        .unwrap()
-        .iter()
-        .filter_map(Value::as_str)
-        .filter(|name| name.ends_with(".vectorcraft"))
-        .collect();
+    let packaged: Vec<_> =
+        result["files"].as_array().unwrap().iter().filter_map(Value::as_str).filter(|name| name.ends_with(".vectorcraft")).collect();
     assert_eq!(packaged.len(), 3, "both nested documents must be retained: {packaged:?}");
     std::fs::remove_dir_all(dir.0.join("art1")).unwrap();
     std::fs::remove_dir_all(dir.0.join("art2")).unwrap();
@@ -391,7 +386,13 @@ fn identical_stale_paths_in_different_nested_documents_keep_separate_assets() {
     }
     assert_eq!(actual.len(), 2, "stale source paths must not cause asset deduplication");
     assert_ne!(actual[0], actual[1], "different original illustrations must keep different pixels");
-    for filename in result["files"].as_array().unwrap().iter().filter_map(Value::as_str).filter(|name| name.starts_with("Links/") && name.ends_with(".vectorcraft")) {
+    for filename in result["files"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .filter_map(Value::as_str)
+        .filter(|name| name.starts_with("Links/") && name.ends_with(".vectorcraft"))
+    {
         let mut nested = session();
         let opened = open(&mut nested, &folder.join(filename).to_string_lossy());
         assert_eq!(opened["missingLinks"], json!([]), "{filename}: {opened}");
@@ -426,6 +427,22 @@ fn packaging_nested_files_preserves_embedded_preview_pdf_and_compression() {
     assert_eq!(images[0].as_ref().unwrap().relative.as_deref(), Some("photo.png"));
 }
 
+/// The report keeps the packaged document's sections as they were, then gives each placed
+/// document's own, headed by its file, instead of nesting them inside the parent's list.
+#[test]
+fn the_report_lists_a_placed_documents_links_in_a_section_of_its_own() {
+    let dir = Folder::new("package-report-sections");
+    let mut parent = poster_with_placed_document(&dir);
+    run(&mut parent, "file.package", json!({ "folder": dir.file("delivery"), "copyFonts": false }));
+    let report = std::fs::read_to_string(dir.0.join("delivery/poster Folder/poster Report.txt")).unwrap();
+    let lines: Vec<&str> = report.lines().collect();
+    let sections: Vec<usize> = lines.iter().enumerate().filter(|(_, l)| **l == "LINKED FILES").map(|(i, _)| i).collect();
+    let [poster, part] = sections[..] else { panic!("two link sections: {report}") };
+    assert!(lines[poster + 1].ends_with("→ Links/part.vectorcraft"), "the packaged document's section as before: {report}");
+    assert!(lines[part + 1].starts_with("Document: ") && lines[part + 1].ends_with("part.vectorcraft"), "{report}");
+    assert!(lines[part + 2].ends_with("→ Links/photo.png"), "{report}");
+}
+
 /// A damaged or newer-format linked native file is still an asset worth delivering.
 /// Package must preserve the exact bytes instead of failing the entire handoff.
 #[test]

docs/mcp.md

@@ -1844,7 +1844,9 @@ bounds; `clip` puts it in a clip group of the old bounds when it is larger). The
 unsaved one is an error) into `folder/name` (default name `<document> Folder`): `<document>.vectorcraft`, its linked
 files in `Links/` (relinked: the packaged document points at the copies; the open one doesn't change), the fonts its
 type uses in `Fonts/` (fonts whose licence doesn't allow embedding are listed in `skippedFonts` instead) and
-`<document> Report.txt`. Every option defaults to true. Without `folder` (the web, or an agent that wants the bytes) the
+`<document> Report.txt`. A placed `.vectorcraft` document is packaged with its own linked files and fonts, relinked to
+the copies (each file copied once); one that can't be read is copied as it is, and a cycle of placed documents or one
+past the nesting limit stops there, each with a line in `warnings` and the report. Every option defaults to true. Without `folder` (the web, or an agent that wants the bytes) the
 result carries the same files as a zip (`{name: "<name>.zip", dataBase64}`, entries under `<name>/`).
 
 `document.info {selectionOnly?, category?, format?: "text"}` adds `sections` (`[{id, title, rows: [[label, value]]}]`: