feat: preserve presentation slide identity in the document model

src/formats/odf/mod.rs

@@ -70,16 +70,27 @@ fn is_encrypted(pkg: &RefCell<Package>) -> Result<bool, ConvertError> {
 
 fn parse_presentation(pres: &Element, ctx: &Ctx) -> Result<Vec<Block>, ConvertError> {
     let mut blocks = Vec::new();
-    for page in pres.find_all(ns::DRAW, "page") {
+    let mut emitted_content_slide = false;
+    for (slide_index, page) in pres.find_all(ns::DRAW, "page").enumerate() {
+        let mut page_blocks =
+            vec![Block::Paragraph(vec![Inline::Anchor(format!("slide-{}", slide_index + 1))])];
         let mut title = Vec::new();
         let mut body = Vec::new();
         let mut notes = Vec::new();
         walk_shapes(page, ctx, &mut title, &mut body, &mut notes)?;
-        blocks.append(&mut title);
-        blocks.append(&mut body);
+        page_blocks.append(&mut title);
+        page_blocks.append(&mut body);
         // Speaker notes are included (fixed policy), set off as a quote.
         if !notes.is_empty() {
-            blocks.push(Block::BlockQuote(notes));
+            page_blocks.push(Block::BlockQuote(notes));
+        }
+        let has_content = page_blocks.len() > 1;
+        if has_content && emitted_content_slide {
+            blocks.push(Block::Rule);
+        }
+        blocks.append(&mut page_blocks);
+        if has_content {
+            emitted_content_slide = true;
         }
     }
     Ok(blocks)

src/formats/ppt/mod.rs

@@ -106,12 +106,20 @@ struct PendingShape {
     styles: Option<StyleRuns>,
 }
 
+struct Segment {
+    blocks: Vec<Block>,
+    id: Option<u32>,
+    is_notes: bool,
+    slide_number: Option<usize>,
+}
+
 #[derive(Default)]
 struct Extractor {
-    /// Finished segments: (blocks, pairing id, is_notes).
-    segments: Vec<(Vec<Block>, Option<u32>, bool)>,
+    /// Finished slide and notes segments in source order.
+    segments: Vec<Segment>,
     current: Vec<Block>,
     current_is_notes: bool,
+    current_slide_number: Option<usize>,
     list_run: Vec<ListEntry>,
     pending: Option<PendingShape>,
     /// Master style tables in master-list order: (masterId, styles).
@@ -265,6 +273,7 @@ impl Extractor {
     ) -> Result<(), ConvertError> {
         // (persistIdRef, slideId) of the page whose container is pending.
         let mut pending: Option<(u32, u32)> = None;
+        let mut slide_number = 0usize;
         for (ver_inst, rec_type, body) in children(list) {
             match rec_type {
                 // SlidePersistAtom: the next slide/notes page begins.
@@ -273,6 +282,12 @@ impl Extractor {
                         self.finish_slide(pending.take(), persist, data, container_type, is_notes)?;
                     self.end_segment(id);
                     self.current_is_notes = is_notes;
+                    if is_notes {
+                        self.current_slide_number = None;
+                    } else {
+                        slide_number += 1;
+                        self.current_slide_number = Some(slide_number);
+                    }
                     pending = get_u32(body, 0).map(|p| (p, get_u32(body, 12).unwrap_or(0)));
                     if !is_notes {
                         self.select_master(pending.map(|(p, _)| p), persist, data);
@@ -320,29 +335,45 @@ impl Extractor {
     fn end_segment(&mut self, id: Option<u32>) {
         self.flush_shape();
         flush_list(&mut self.current, &mut self.list_run);
-        if !self.current.is_empty() {
-            let blocks = std::mem::take(&mut self.current);
-            self.segments.push((blocks, id, self.current_is_notes));
+        let slide_number = self.current_slide_number.take();
+        if !self.current.is_empty() || slide_number.is_some() {
+            self.segments.push(Segment {
+                blocks: std::mem::take(&mut self.current),
+                id,
+                is_notes: self.current_is_notes,
+                slide_number,
+            });
         }
     }
 
     fn into_blocks(mut self) -> Vec<Block> {
         self.end_segment(None);
-        let mut slides: Vec<(Option<u32>, Vec<Block>)> = Vec::new();
+        let mut slides: Vec<(Option<usize>, Option<u32>, Vec<Block>)> = Vec::new();
         let mut notes: Vec<(Option<u32>, Vec<Block>)> = Vec::new();
-        for (blocks, id, is_notes) in self.segments {
+        for Segment { blocks, id, is_notes, slide_number } in self.segments {
             if is_notes {
                 notes.push((id, blocks));
             } else {
-                slides.push((id, blocks));
+                slides.push((slide_number, id, blocks));
             }
         }
         // Notes pages pair to slides by their stored slide id, not by list
         // position: the notes list may be sparse (notes on only some
         // slides), which order-based zipping would misattribute.
         let mut used = vec![false; notes.len()];
         let mut out = Vec::new();
-        for (sid, blocks) in slides {
+        let mut emitted_content_slide = false;
+        for (slide_number, sid, blocks) in slides {
+            if let Some(slide_number) = slide_number {
+                let has_content = !blocks.is_empty();
+                if has_content && emitted_content_slide {
+                    out.push(Block::Rule);
+                }
+                out.push(Block::Paragraph(vec![Inline::Anchor(format!("slide-{slide_number}"))]));
+                if has_content {
+                    emitted_content_slide = true;
+                }
+            }
             out.extend(blocks);
             for (i, (nid, nblocks)) in notes.iter_mut().enumerate() {
                 if !used[i] && sid.is_some() && *nid == sid {

src/formats/pptx/mod.rs

@@ -31,8 +31,6 @@ const MASTER_REL: &str =
     "http://schemas.openxmlformats.org/officeDocument/2006/relationships/slideMaster";
 const NOTES_REL: &str =
     "http://schemas.openxmlformats.org/officeDocument/2006/relationships/notesSlide";
-const SLIDE_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/slide";
-
 /// Namespaces whose markup this frontend understands; `mc:Choice` branches
 /// requiring anything else fall back to `mc:Fallback`.
 const SUPPORTED_NS: &[&str] = &[ns::P, ns::A, ns::R, ns::MC];
@@ -93,35 +91,34 @@ pub fn parse(bytes: &[u8]) -> Result<Document, ConvertError> {
     let mut blocks: Vec<Block> = Vec::new();
     let mut failed = 0usize;
     let instance_counter = StdCell::new(0u64);
-    // Every slide has a start anchor id so internal slide-to-slide links
-    // resolve after concatenation; the anchor node is emitted only on
-    // slides some link actually targets.
-    let slide_anchors: HashMap<String, String> = slide_paths
-        .iter()
-        .enumerate()
-        .map(|(i, p)| (p.clone(), format!("slide-{}", i + 1)))
-        .collect();
+    // Slide identity is positional in `sldIdLst`, so this path-keyed map is
+    // only for resolving internal slide-to-slide links. If a malformed deck
+    // lists one part more than once, links keep the first occurrence's
+    // identity instead of being retargeted by a later map insertion.
+    let mut slide_anchors: HashMap<String, String> = HashMap::new();
+    for (slide_index, slide_path) in slide_paths.iter().enumerate() {
+        slide_anchors
+            .entry(slide_path.clone())
+            .or_insert_with(|| format!("slide-{}", slide_index + 1));
+    }
     let mut all_rels: Vec<Relationships> = Vec::with_capacity(slide_paths.len());
     for p in &slide_paths {
         all_rels.push(read_rels(&mut pkg.borrow_mut(), &rels_part_for(p))?);
     }
-    let targeted: std::collections::HashSet<String> = slide_paths
-        .iter()
-        .zip(&all_rels)
-        .flat_map(|(p, rels)| {
-            rels.iter()
-                .filter(|(_, r)| r.rel_type == SLIDE_REL && r.mode == TargetMode::Internal)
-                .filter_map(move |(_, r)| path::resolve(p, &r.target).ok().map(|t| t.path))
-        })
-        .filter(|t| slide_anchors.contains_key(t))
-        .collect();
+    let mut emitted_content_slide = false;
 
     for (slide_index, slide_path) in slide_paths.iter().enumerate() {
+        // Unlike link resolution above, emission must preserve every
+        // positional occurrence, including duplicate paths.
+        let mut slide_blocks =
+            vec![Block::Paragraph(vec![Inline::Anchor(format!("slide-{}", slide_index + 1))])];
+
         let tree = match pkg.borrow_mut().optional_xml_part(slide_path)? {
             Some(t) => t,
             None => {
                 log::warn!("skipping unusable slide {slide_path}");
                 failed += 1;
+                blocks.extend(slide_blocks);
                 continue;
             }
         };
@@ -132,6 +129,7 @@ pub fn parse(bytes: &[u8]) -> Result<Document, ConvertError> {
         else {
             log::warn!("skipping slide {slide_path}: no shape tree");
             failed += 1;
+            blocks.extend(slide_blocks);
             continue;
         };
         let slide_rels = &all_rels[slide_index];
@@ -163,12 +161,7 @@ pub fn parse(bytes: &[u8]) -> Result<Document, ConvertError> {
             instance_counter: &instance_counter,
             slide_anchors: &slide_anchors,
         };
-        if targeted.contains(slide_path)
-            && let Some(anchor) = slide_anchors.get(slide_path)
-        {
-            blocks.push(Block::Paragraph(vec![Inline::Anchor(anchor.clone())]));
-        }
-        parse_shapes(sp_tree, &ctx, &mut blocks)?;
+        parse_shapes(sp_tree, &ctx, &mut slide_blocks)?;
 
         // Speaker notes, set off as a quote (fixed policy: included). The
         // tree is loaded before the `if let` so the package borrow is not
@@ -203,9 +196,17 @@ pub fn parse(bytes: &[u8]) -> Result<Document, ConvertError> {
                 }
             }
             if !notes_blocks.is_empty() {
-                blocks.push(Block::BlockQuote(notes_blocks));
+                slide_blocks.push(Block::BlockQuote(notes_blocks));
             }
         }
+        let has_content = slide_blocks.len() > 1;
+        if has_content && emitted_content_slide {
+            blocks.push(Block::Rule);
+        }
+        blocks.extend(slide_blocks);
+        if has_content {
+            emitted_content_slide = true;
+        }
     }
     if failed == slide_paths.len() {
         return Err(ConvertError::malformed("no slide in the presentation could be read"));

src/package/relationships.rs

@@ -48,10 +48,6 @@ impl Relationships {
         self.0.get(id).filter(|r| r.mode == TargetMode::Internal).map(|r| r.target.as_str())
     }
 
-    pub fn iter(&self) -> impl Iterator<Item = (&str, &Relationship)> {
-        self.0.iter().map(|(k, v)| (k.as_str(), v))
-    }
-
     /// The internal-mode relationship of a given type, lowest id first so
     /// the pick is deterministic when a producer emits duplicates.
     pub fn first_of_type(&self, rel_type: &str) -> Option<&Relationship> {