Format: retain imported chart parts through native saves (#851)

* M2.1: preserve chart parts through native saves

Serialize retained chart relationships and payloads in native files.
Add native and headless file-command round-trip regressions.

Contribution on behalf of @MMexicann.

* Format: bound opaque chart parts as native ZIP entries

Address maintainer feedback on the existing PR. Preserve upstream work and document tested scope and validation limits.

* Model: track transient native chart part entries

Address maintainer feedback on the existing PR. Preserve upstream work and document tested scope and validation limits.

* PPTX: initialize native chart entry metadata

Address maintainer feedback on the existing PR. Preserve upstream work and document tested scope and validation limits.

* Engine: adapt chart native roundtrip fixtures

Address maintainer feedback on the existing PR. Preserve upstream work and document tested scope and validation limits.

* Docs: describe bounded chart sidecars and validation

Address maintainer feedback on the existing PR. Preserve upstream work and document tested scope and validation limits.

* Roadmap: record bounded chart persistence evidence

Address maintainer feedback on the existing PR. Preserve upstream work and document tested scope and validation limits.

---------

Co-authored-by: skylar taylor-barrick <hi@skylar.cc>

crates/format/src/chart_parts.rs

@@ -0,0 +1,112 @@
+//! Keep opaque chart payloads out of the JSON manifest and within the media load budget.
+use std::collections::HashSet;
+use std::io::Cursor;
+use std::sync::Arc;
+
+use deckcraft_model::chart::ChartPart;
+use deckcraft_model::{Master, Presentation, Shape, ShapeKind};
+
+use crate::{FormatError, MAX_ENTRY, Result, read_entry_max};
+
+const PREFIX: &str = "chart-parts/";
+type Payload = (String, String, Vec<u8>);
+
+pub(super) fn separate(p: &mut Presentation) -> Result<Vec<Payload>> {
+    let mut payloads = Vec::new();
+    visit_parts(p, |part| {
+        // The native namespace is independent of the original OPC names, which may repeat
+        // when charts from several decks are combined. No user-supplied path is extracted.
+        let entry = format!("{PREFIX}{}", payloads.len());
+        part.native_entry = Some(entry.clone());
+        payloads.push((entry, part.content_type.clone(), std::mem::take(&mut part.bytes)));
+        Ok(())
+    })?;
+    Ok(payloads)
+}
+
+pub(super) fn restore(
+    p: &mut Presentation,
+    z: &mut zip::ZipArchive<Cursor<&[u8]>>,
+    left: &mut u64,
+    mut items: usize,
+    max_items: usize,
+    damaged: &mut Vec<String>,
+) -> Result<()> {
+    let mut seen = HashSet::new();
+    visit_parts(p, |part| {
+        if items >= max_items {
+            return Err(FormatError::Damaged(format!("media and chart parts exceed {max_items} items")));
+        }
+        items += 1;
+        if let Some(entry) = part.native_entry.take() {
+            // Accept only names produced by separate(), and inflate each member once.
+            let canonical = entry.strip_prefix(PREFIX).and_then(|s| s.parse::<usize>().ok()).is_some_and(|n| entry == format!("{PREFIX}{n}"));
+            if !canonical || !seen.insert(entry.clone()) || !part.bytes.is_empty() {
+                return Err(FormatError::Damaged("invalid or ambiguous chart part reference".into()));
+            }
+            if z.index_for_name(&entry).is_none() {
+                // Missing members consume no inflation work and can be reported for recovery.
+                damaged.push(format!("{entry}: missing chart part"));
+                return Ok(());
+            }
+            if z.by_name(&entry).is_ok_and(|f| f.size() > *left) {
+                return Err(FormatError::Damaged("embedded media and chart parts are larger than DeckCraft can open".into()));
+            }
+            // A CRC/decode error may already have inflated bytes. Stop on that first error
+            // rather than letting later corrupt members reuse the unchanged byte budget.
+            let bytes = read_entry_max(z, &entry, (*left).min(MAX_ENTRY))?;
+            *left -= bytes.len() as u64;
+            part.bytes = bytes;
+        } else {
+            // Older manifests used JSON byte arrays. Charge those too, after the existing
+            // bounded manifest parser has read them, so resaving migrates them losslessly.
+            let size = part.bytes.len() as u64;
+            if size > (*left).min(MAX_ENTRY) {
+                return Err(FormatError::Damaged("inline chart part is larger than DeckCraft can open".into()));
+            }
+            *left -= size;
+        }
+        Ok(())
+    })
+}
+
+fn visit_parts(p: &mut Presentation, mut visit: impl FnMut(&mut ChartPart) -> Result<()>) -> Result<()> {
+    // Use an explicit stack for groups. Charts can also live on masters and their layouts,
+    // including the notes and handout masters; none of these owners may leak inline bytes.
+    for slide in &mut p.slides {
+        shapes(&mut Arc::make_mut(slide).shapes, &mut visit)?;
+    }
+    for master in &mut p.masters {
+        master_parts(Arc::make_mut(master), &mut visit)?;
+    }
+    for master in [&mut p.notes_master, &mut p.handout_master].into_iter().flatten() {
+        master_parts(Arc::make_mut(master), &mut visit)?;
+    }
+    Ok(())
+}
+
+fn master_parts(master: &mut Master, visit: &mut impl FnMut(&mut ChartPart) -> Result<()>) -> Result<()> {
+    shapes(&mut master.shapes, visit)?;
+    for layout in &mut master.layouts {
+        shapes(&mut layout.shapes, visit)?;
+    }
+    Ok(())
+}
+
+fn shapes(root: &mut [Shape], visit: &mut impl FnMut(&mut ChartPart) -> Result<()>) -> Result<()> {
+    let mut pending = vec![root];
+    while let Some(shapes) = pending.pop() {
+        for shape in shapes {
+            match &mut shape.kind {
+                ShapeKind::Group { children, .. } => pending.push(children),
+                ShapeKind::Chart(chart) => {
+                    for part in &mut chart.kept.parts {
+                        visit(part)?;
+                    }
+                }
+                _ => {}
+            }
+        }
+    }
+    Ok(())
+}

crates/format/src/chart_parts_tests.rs

@@ -0,0 +1,234 @@
+use super::*;
+use deckcraft_model::chart::{Chart, ChartPart, ChartRel, ChartType};
+use deckcraft_model::{Master, Shape, ShapeId};
+use serde_json::{Value, json};
+
+const PART: &str = "ppt/embeddings/owned.xlsx";
+
+fn fixture(payload: Vec<u8>) -> Presentation {
+    // Construct through the old manifest representation: this also exercises compatibility.
+    let mut part: ChartPart = serde_json::from_value(json!({
+        "name": PART, "contentType": "application/zip", "bytes": []
+    }))
+    .unwrap();
+    part.bytes = payload;
+    let mut chart = Chart::sample(ChartType::Pie);
+    chart.kept.rels.push(ChartRel {
+        id: "rWorkbook".into(),
+        rel_type: "http://schemas.openxmlformats.org/officeDocument/2006/relationships/package".into(),
+        target: "../embeddings/owned.xlsx".into(),
+        part: PART.into(),
+        external: false,
+    });
+    chart.kept.parts.push(part);
+    let mut p = Presentation::default();
+    let child = Shape { id: ShapeId(p.alloc_id().unwrap()), kind: ShapeKind::Chart(Box::new(chart)), ..Default::default() };
+    let group = Shape {
+        id: ShapeId(p.alloc_id().unwrap()),
+        kind: ShapeKind::Group { children: vec![child], child: Default::default() },
+        ..Default::default()
+    };
+    Arc::make_mut(&mut p.slides[0]).shapes = vec![group];
+    p
+}
+
+fn part_json(value: &mut Value) -> Option<&mut Value> {
+    if value.get("name").and_then(Value::as_str) == Some(PART) && value.get("contentType").is_some() {
+        return Some(value);
+    }
+    match value {
+        Value::Array(values) => values.iter_mut().find_map(part_json),
+        Value::Object(values) => values.values_mut().find_map(part_json),
+        _ => None,
+    }
+}
+
+fn rewrite_manifest(bytes: &[u8], edit: impl FnOnce(&mut Value)) -> Vec<u8> {
+    let mut z = zip::ZipArchive::new(Cursor::new(bytes)).unwrap();
+    let mut json: Value = serde_json::from_slice(&read_entry(&mut z, "presentation.json").unwrap()).unwrap();
+    edit(&mut json);
+    let mut out = zip::ZipWriter::new(Cursor::new(Vec::new()));
+    for i in 0..z.len() {
+        let f = z.by_index(i).unwrap();
+        if f.name() == "presentation.json" {
+            out.start_file("presentation.json", zip::write::SimpleFileOptions::default()).unwrap();
+            out.write_all(&serde_json::to_vec(&json).unwrap()).unwrap();
+        } else {
+            out.raw_copy_file(f).unwrap();
+        }
+    }
+    out.finish().unwrap().into_inner()
+}
+
+#[test]
+fn large_chart_payload_is_a_referenced_zip_entry() {
+    let payload = vec![200; 3 * 1024 * 1024];
+    let p = fixture(payload.clone());
+    let bytes = save(&p).unwrap();
+    let mut z = zip::ZipArchive::new(Cursor::new(bytes.as_slice())).unwrap();
+    assert!(z.by_name("presentation.json").unwrap().size() < 128 * 1024, "binary chart bytes must not become JSON numbers");
+    let mut manifest: Value = serde_json::from_slice(&read_entry(&mut z, "presentation.json").unwrap()).unwrap();
+    let part = part_json(&mut manifest).unwrap();
+    assert!(part.get("bytes").is_none(), "new manifests keep only metadata and a reference");
+    let entry = part["nativeEntry"].as_str().unwrap();
+    assert!(entry.starts_with("chart-parts/"));
+    assert_eq!(read_entry(&mut z, entry).unwrap(), payload);
+    assert!(read_entry_max(&mut z, entry, payload.len() as u64 - 1).is_err());
+    assert_eq!(load(&bytes).unwrap(), p);
+}
+
+#[test]
+fn chart_parts_share_media_byte_and_item_budgets() {
+    let mut p = fixture(vec![7; 600]);
+    p.add_media("a.bin", "application/octet-stream", vec![8; 600]).unwrap();
+    let bytes = save(&p).unwrap();
+    assert!(load_within(&bytes, Budget { total: 1200, items: 2 }).is_ok());
+    assert!(load_within(&bytes, Budget { total: 1199, items: 2 }).is_err(), "chart payloads share the cumulative media budget");
+    assert!(load_within(&bytes, Budget { total: 1200, items: 1 }).is_err(), "chart parts share the media item budget");
+    let duplicated = rewrite_manifest(&bytes, |manifest| {
+        let media = manifest["presentation"]["media"].as_array_mut().unwrap();
+        media.push(media[0].clone());
+    });
+    assert!(load_within(&duplicated, Budget { total: 1200, items: 2 }).is_err(), "duplicate media IDs still count as listed items");
+}
+
+#[test]
+fn legacy_inline_chart_bytes_are_budgeted_and_preserved() {
+    let mut p = fixture(vec![9; 600]);
+    p.add_media("a.bin", "application/octet-stream", vec![8; 600]).unwrap();
+    let legacy = rewrite_manifest(&save(&p).unwrap(), |manifest| {
+        let part = part_json(manifest).unwrap().as_object_mut().unwrap();
+        part.remove("nativeEntry");
+        part.insert("bytes".into(), json!(vec![9_u8; 600]));
+    });
+    assert_eq!(load_within(&legacy, Budget { total: 1200, items: 2 }).unwrap().0, p);
+    assert!(load_within(&legacy, Budget { total: 1199, items: 2 }).is_err());
+    assert!(load_within(&legacy, Budget { total: 1200, items: 1 }).is_err());
+}
+
+#[test]
+fn chart_payloads_in_every_shape_owner_get_distinct_entries() {
+    let mut p = fixture(vec![42; 4096]);
+    let shapes = p.slides[0].shapes.clone();
+    let master = Arc::make_mut(&mut p.masters[0]);
+    master.shapes = shapes.clone();
+    master.layouts[0].shapes = shapes.clone();
+    p.notes_master = Some(Arc::new(Master { shapes: shapes.clone(), ..Default::default() }));
+    p.handout_master = Some(Arc::new(Master { shapes, ..Default::default() }));
+    let bytes = save(&p).unwrap();
+    let z = zip::ZipArchive::new(Cursor::new(bytes.as_slice())).unwrap();
+    assert_eq!(z.file_names().filter(|name| name.starts_with("chart-parts/")).count(), 5);
+    assert_eq!(load(&bytes).unwrap(), p);
+}
+
+#[test]
+fn missing_or_corrupt_chart_payload_is_reported() {
+    let p = fixture(b"SYNTHETIC-CHART-PAYLOAD-0123456789".to_vec());
+    let bytes = save(&p).unwrap();
+    let mut z = zip::ZipArchive::new(Cursor::new(bytes.as_slice())).unwrap();
+    let mut json: Value = serde_json::from_slice(&read_entry(&mut z, "presentation.json").unwrap()).unwrap();
+    let name = part_json(&mut json).unwrap()["nativeEntry"].as_str().unwrap().to_owned();
+    let start = z.by_name(&name).unwrap().data_start() as usize;
+    let mut out = zip::ZipWriter::new(Cursor::new(Vec::new()));
+    for i in 0..z.len() {
+        let f = z.by_index(i).unwrap();
+        if f.name() != name {
+            out.raw_copy_file(f).unwrap();
+        }
+    }
+    let missing = out.finish().unwrap().into_inner();
+    let (q, damaged) = load_checked(&missing).unwrap();
+    assert_eq!(damaged.len(), 1, "{damaged:?}");
+    let ShapeKind::Group { children, .. } = &q.slides[0].shapes[0].kind else { panic!("group") };
+    let ShapeKind::Chart(chart) = &children[0].kind else { panic!("chart") };
+    assert!(chart.kept.parts[0].bytes.is_empty());
+    let mut corrupt = bytes.clone();
+    corrupt[start] ^= 0xff;
+    assert!(load_checked(&corrupt).is_err());
+}
+
+#[test]
+fn chart_references_cannot_alias_other_members_or_mix_inline_bytes() {
+    let p = fixture(vec![3; 10]);
+    let bytes = save(&p).unwrap();
+    for entry in ["media/1", "chart-parts/../0", "chart-parts/00", "chart-parts/-1"] {
+        let changed = rewrite_manifest(&bytes, |manifest| {
+            part_json(manifest).unwrap()["nativeEntry"] = json!(entry);
+        });
+        assert!(load(&changed).is_err(), "accepted {entry}");
+    }
+    let ambiguous = rewrite_manifest(&bytes, |manifest| {
+        part_json(manifest).unwrap()["bytes"] = json!([3]);
+    });
+    assert!(load(&ambiguous).is_err());
+
+    let mut p = p;
+    let ShapeKind::Group { children, .. } = &mut Arc::make_mut(&mut p.slides[0]).shapes[0].kind else { panic!("group") };
+    let ShapeKind::Chart(chart) = &mut children[0].kind else { panic!("chart") };
+    chart.kept.parts.push(chart.kept.parts[0].clone());
+    let aliased = rewrite_manifest(&save(&p).unwrap(), |manifest| {
+        fn alias(value: &mut Value) {
+            if let Some(entry) = value.get_mut("nativeEntry") {
+                *entry = json!("chart-parts/0");
+            }
+            match value {
+                Value::Array(values) => values.iter_mut().for_each(alias),
+                Value::Object(values) => values.values_mut().for_each(alias),
+                _ => {}
+            }
+        }
+        alias(manifest);
+    });
+    assert!(load(&aliased).is_err(), "one ZIP member must not be inflated repeatedly");
+}
+
+fn with_two_parts() -> Presentation {
+    let mut p = fixture(vec![3; 10]);
+    let ShapeKind::Group { children, .. } = &mut Arc::make_mut(&mut p.slides[0]).shapes[0].kind else { panic!("group") };
+    let ShapeKind::Chart(chart) = &mut children[0].kind else { panic!("chart") };
+    let mut second = chart.kept.parts[0].clone();
+    second.bytes = vec![4; 10];
+    chart.kept.parts.push(second);
+    p
+}
+
+#[test]
+fn corrupt_chart_members_stop_before_reusing_the_inflation_budget() {
+    let mut bytes = save(&with_two_parts()).unwrap();
+    let offsets: Vec<_> = {
+        let mut z = zip::ZipArchive::new(Cursor::new(bytes.as_slice())).unwrap();
+        (0..2).map(|i| z.by_name(&format!("chart-parts/{i}")).unwrap().data_start() as usize).collect()
+    };
+    for offset in offsets {
+        bytes[offset] ^= 0xff;
+    }
+    // A failed CRC may already have inflated bytes. Continuing with the same remaining
+    // budget would let each corrupt member consume that work budget again.
+    let result = load_within(&bytes, Budget { total: 10, items: 2 });
+    assert!(result.is_err(), "two corrupt 10-byte members must not reuse a 10-byte budget");
+    assert!(result.unwrap_err().to_string().contains("chart-parts/0"));
+}
+
+#[test]
+fn duplicate_physical_zip_names_are_indexed_and_inflated_once() {
+    let mut bytes = save(&with_two_parts()).unwrap();
+    // ZipWriter refuses duplicate names. Forge equal-length local/central-directory names
+    // instead, to exercise ZipArchive's duplicate-name indexing on an untrusted archive.
+    let original = b"chart-parts/1";
+    let mut replaced = 0;
+    for offset in 0..=bytes.len() - original.len() {
+        if &bytes[offset..offset + original.len()] == original {
+            bytes[offset + original.len() - 1] = b'0';
+            replaced += 1;
+        }
+    }
+    assert_eq!(replaced, 2);
+    let z = zip::ZipArchive::new(Cursor::new(bytes.as_slice())).unwrap();
+    assert_eq!(z.file_names().filter(|n| n.starts_with("chart-parts/")).count(), 1);
+    let (q, damaged) = load_within(&bytes, Budget { total: 10, items: 2 }).unwrap();
+    assert_eq!(damaged.len(), 1, "the second reference is missing, not inflated again");
+    let ShapeKind::Group { children, .. } = &q.slides[0].shapes[0].kind else { panic!("group") };
+    let ShapeKind::Chart(chart) = &children[0].kind else { panic!("chart") };
+    assert_eq!(chart.kept.parts[0].bytes, vec![4; 10]);
+    assert!(chart.kept.parts[1].bytes.is_empty());
+}

crates/format/src/lib.rs

@@ -1,11 +1,15 @@
 //! The native `.deckcraft` format: a zip holding `presentation.json` (the model, minus media
-//! bytes), `media/<id>` files with the embedded media, and `mimetype`. Readers ignore unknown
+//! bytes), `media/<id>` and `chart-parts/<index>` binary entries, and `mimetype`. Readers ignore unknown
 //! fields, so newer files open in older builds with what those builds understand.
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 #![forbid(unsafe_code)]
 
+mod chart_parts;
 mod finite;
 
+#[cfg(test)]
+mod chart_parts_tests;
+
 use std::io::{Cursor, Read, Write};
 use std::sync::Arc;
 
@@ -20,9 +24,9 @@ pub const EXTENSION: &str = "deckcraft";
 pub const VERSION: u32 = 1;
 /// Largest entry we will inflate (guards against zip bombs).
 const MAX_ENTRY: u64 = 2 << 30;
-/// All embedded media together, inflated (#393).
+/// All embedded media and chart sidecars together, inflated (#393).
 const MAX_TOTAL_MEDIA: u64 = 4 << 30;
-/// Media items one file may list.
+/// Media items and chart sidecar parts one file may list.
 const MAX_MEDIA_ITEMS: usize = 100_000;
 
 /// What one load may inflate.
@@ -62,12 +66,10 @@ pub fn save(p: &Presentation) -> Result<Vec<u8>> {
         let w = |e: zip::result::ZipError| FormatError::Write(e.to_string());
         z.start_file("mimetype", stored).map_err(w)?;
         z.write_all(MIMETYPE.as_bytes()).map_err(|e| FormatError::Write(e.to_string()))?;
-        let m = Manifest {
-            format: "deckcraft".into(),
-            version: VERSION,
-            generator: format!("DeckCraft {}", env!("CARGO_PKG_VERSION")),
-            presentation: p.clone(),
-        };
+        let mut presentation = p.clone();
+        let parts = chart_parts::separate(&mut presentation)?;
+        let m =
+            Manifest { format: "deckcraft".into(), version: VERSION, generator: format!("DeckCraft {}", env!("CARGO_PKG_VERSION")), presentation };
         let json = serde_json::to_vec(&finite::Finite(&m)).map_err(|e| FormatError::Write(e.to_string()))?;
         z.start_file("presentation.json", deflate).map_err(w)?;
         z.write_all(&json).map_err(|e| FormatError::Write(e.to_string()))?;
@@ -80,6 +82,11 @@ pub fn save(p: &Presentation) -> Result<Vec<u8>> {
             z.start_file(format!("media/{}", item.id.0), opts).map_err(w)?;
             z.write_all(&item.data).map_err(|e| FormatError::Write(e.to_string()))?;
         }
+        for (name, content_type, bytes) in parts {
+            let opts = if is_compressed(&content_type) { stored } else { deflate };
+            z.start_file(name, opts).map_err(w)?;
+            z.write_all(&bytes).map_err(|e| FormatError::Write(e.to_string()))?;
+        }
         z.finish().map_err(w)?;
     }
     Ok(buf.into_inner())
@@ -119,14 +126,15 @@ fn read_entry_max(z: &mut zip::ZipArchive<Cursor<&[u8]>>, name: &str, max: u64)
     Ok(out)
 }
 
-/// Read a presentation. Embedded media that is missing or damaged is left empty; use
-/// [`load_checked`] to learn which.
+/// Read a presentation. Missing media/chart parts are left empty; use [`load_checked`] to
+/// learn which. A damaged chart member refuses the file before further chart inflation.
 pub fn load(bytes: &[u8]) -> Result<Presentation> {
     load_checked(bytes).map(|(p, _)| p)
 }
 
-/// Read a presentation and list the embedded media that couldn't be read (missing, too large
-/// or failing its CRC), one message each. Linked media saved without its bytes isn't damage.
+/// Read a presentation and list media that couldn't be read, plus missing chart members.
+/// A chart member that is present but fails decoding, CRC or its entry limit refuses the file.
+/// Linked media saved without its bytes isn't damage.
 pub fn load_checked(bytes: &[u8]) -> Result<(Presentation, Vec<String>)> {
     load_within(bytes, Budget { total: MAX_TOTAL_MEDIA, items: MAX_MEDIA_ITEMS })
 }
@@ -140,7 +148,8 @@ fn load_within(bytes: &[u8], budget: Budget) -> Result<(Presentation, Vec<String
     }
     let mut p = m.presentation;
     let mut damaged = vec![];
-    if p.media.len() > budget.items {
+    let items = p.media.len();
+    if items > budget.items {
         return Err(FormatError::Damaged(format!("{} media items (at most {})", p.media.len(), budget.items)));
     }
     // A repeated id names the same member: keep the first item, so the member is inflated once
@@ -163,6 +172,7 @@ fn load_within(bytes: &[u8], budget: Budget) -> Result<(Presentation, Vec<String
             Err(e) => damaged.push(e.to_string()),
         }
     }
+    chart_parts::restore(&mut p, &mut z, &mut left, items, budget.items, &mut damaged)?;
     repair(&mut p);
     Ok((p, damaged))
 }
@@ -663,6 +673,54 @@ pub fn slides_to_outline(p: &Presentation) -> String {
 mod tests {
     use super::*;
 
+    #[test]
+    fn chart_sidecars_are_optional_for_older_native_files() {
+        use deckcraft_model::chart::{Chart, ChartType};
+        let chart = Chart::sample(ChartType::Pie);
+        let mut json = serde_json::to_value(&chart).expect("chart JSON");
+        assert!(json.get("kept").is_none(), "empty sidecars should not enlarge ordinary files");
+        assert_eq!(serde_json::from_value::<Chart>(json.clone()).expect("old chart without kept"), chart);
+        json["kept"] = serde_json::json!({});
+        assert_eq!(serde_json::from_value::<Chart>(json).expect("empty sidecar"), chart);
+    }
+
+    #[test]
+    fn native_save_keeps_chart_relationships_and_payloads() {
+        use deckcraft_model::chart::{Chart, ChartKept, ChartPart, ChartRel, ChartType};
+        use deckcraft_model::{Shape, ShapeId, ShapeKind};
+        let kept = ChartKept {
+            rels: vec![ChartRel {
+                id: "rWorkbook".into(),
+                rel_type: "http://schemas.openxmlformats.org/officeDocument/2006/relationships/package".into(),
+                target: "../embeddings/data.xlsx".into(),
+                part: "ppt/embeddings/data.xlsx".into(),
+                external: false,
+            }],
+            parts: vec![ChartPart {
+                name: "ppt/embeddings/data.xlsx".into(),
+                content_type: "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet".into(),
+                bytes: vec![0, 255, 128, 42],
+                native_entry: None,
+            }],
+        };
+        let mut p = defaults::new_presentation(None);
+        let chart = Chart { kept: kept.clone(), ..Chart::sample(ChartType::Pie) };
+        let s = Shape { id: ShapeId(p.alloc_id().expect("id")), kind: ShapeKind::Chart(Box::new(chart)), ..Default::default() };
+        // Serialization must reach charts inside groups as well as top-level charts.
+        Arc::make_mut(&mut p.slides[0]).shapes = vec![Shape {
+            id: ShapeId(p.alloc_id().expect("group id")),
+            kind: ShapeKind::Group { children: vec![s], child: Default::default() },
+            ..Default::default()
+        }];
+        let mut q = load(&save(&p).expect("save")).expect("load");
+        for _ in 0..2 {
+            let ShapeKind::Group { children, .. } = &q.slides[0].shapes[0].kind else { panic!("group") };
+            let ShapeKind::Chart(chart) = &children[0].kind else { panic!("chart") };
+            assert_eq!(chart.kept, kept);
+            q = load(&save(&q).expect("save again")).expect("load again");
+        }
+    }
+
     /// The same package without the entries `drop` names.
     fn without(bytes: &[u8], drop: &str) -> Vec<u8> {
         let mut z = zip::ZipArchive::new(Cursor::new(bytes)).unwrap();