feat(format): B2 — PPTX placeholder filter, SmartArt diagrams, chart XPath matching

tests/test_translators/test_b2_pptx_fixes.py

@@ -0,0 +1,623 @@
+"""
+Tests for Track B2 — PowerPoint format preservation fixes.
+
+Covers:
+  B2.1 — Placeholder filter (DATE=16, SLIDE_NUMBER=13, FOOTER=15)
+         PowerPoint auto-fills these at render time, so translating them
+         is pointless and just wastes API calls.
+  B2.2 — SmartArt diagram text collection (ppt/diagrams/data*.xml)
+  B2.3 — SmartArt diagram translation applied via ZIP rewrite
+  B2.4 — Chart text matched by element path (not string equality)
+  B2.5 — Whitespace preservation in chart apply
+
+These tests are intentionally narrow: they validate that a specific
+bug is fixed. Existing tests in test_pptx_translator.py handle the
+broader surface.
+"""
+import os
+import tempfile
+import zipfile
+from pathlib import Path
+from typing import List, Dict
+from unittest.mock import MagicMock, patch
+
+import pytest
+from pptx import Presentation
+from pptx.util import Inches, Pt
+from pptx.enum.shapes import MSO_SHAPE_TYPE
+from lxml import etree
+
+from translators.pptx_translator import PowerPointTranslator, pptx_translator
+
+
+# ---------------- Mock provider ----------------
+
+class MockProvider:
+    """Mock translation provider: returns 'TR_<text>' unless a mapping
+    is provided."""
+
+    def __init__(self, translations: Dict[str, str] = None):
+        self._translations = translations or {}
+        self.calls: List[str] = []
+
+    def translate_batch(
+        self,
+        texts: List[str],
+        target_language: str = "fr",
+        source_language: str = "auto",
+    ) -> List[str]:
+        out = []
+        for t in texts:
+            self.calls.append(t)
+            if t in self._translations:
+                out.append(self._translations[t])
+            elif t.startswith("TR_"):
+                out.append(t)
+            else:
+                out.append(t)
+        return out
+
+    def get_name(self):
+        return "mock"
+
+    def is_available(self):
+        return True
+
+
+# ---------------- Helpers ----------------
+
+_NS_A = "http://schemas.openxmlformats.org/drawingml/2006/main"
+_NS_C = "http://schemas.openxmlformats.org/drawingml/2006/chart"
+_NS_P = "http://schemas.openxmlformats.org/presentationml/2006/main"
+
+
+def _add_textbox_with_text(slide, text: str):
+    tb = slide.shapes.add_textbox(Inches(1), Inches(1), Inches(3), Inches(1))
+    tf = tb.text_frame
+    tf.text = text
+    return tb
+
+
+def _inject_diagram_part(pptx_path: Path, diagram_filename: str, diagram_xml: bytes):
+    """Inject a SmartArt diagram XML part into a .pptx file."""
+    tmp_out = pptx_path.with_suffix(".tmp")
+    with zipfile.ZipFile(pptx_path, "r") as zin, \
+         zipfile.ZipFile(tmp_out, "w", zipfile.ZIP_DEFLATED) as zout:
+        existing = {item: zin.read(item) for item in zin.namelist()}
+
+        # Add the diagram data part
+        existing[diagram_filename] = diagram_xml
+
+        # Register in [Content_Types].xml
+        ct_path = "[Content_Types].xml"
+        if ct_path in existing:
+            ct_content = existing[ct_path].decode("utf-8")
+            override = (
+                f'<Override PartName="/{diagram_filename}" '
+                f'ContentType="application/vnd.openxmlformats-officedocument.'
+                f'drawingml+xml+diagram"/>'
+            )
+            if override not in ct_content:
+                ct_content = ct_content.replace("</Types>", f"{override}</Types>")
+            existing[ct_path] = ct_content.encode("utf-8")
+
+        for item, data in existing.items():
+            zout.writestr(item, data)
+
+    tmp_out.replace(pptx_path)
+
+
+def _inject_chart_part(pptx_path: Path, chart_filename: str, chart_xml: bytes):
+    """Inject a chart XML part into a .pptx file."""
+    tmp_out = pptx_path.with_suffix(".tmp")
+    with zipfile.ZipFile(pptx_path, "r") as zin, \
+         zipfile.ZipFile(tmp_out, "w", zipfile.ZIP_DEFLATED) as zout:
+        existing = {item: zin.read(item) for item in zin.namelist()}
+
+        existing[chart_filename] = chart_xml
+
+        ct_path = "[Content_Types].xml"
+        if ct_path in existing:
+            ct_content = existing[ct_path].decode("utf-8")
+            override = (
+                f'<Override PartName="/{chart_filename}" '
+                f'ContentType="application/vnd.openxmlformats-officedocument.'
+                f'drawingml+xml+chart"/>'
+            )
+            if override not in ct_content:
+                ct_content = ct_content.replace("</Types>", f"{override}</Types>")
+            existing[ct_path] = ct_content.encode("utf-8")
+
+        for item, data in existing.items():
+            zout.writestr(item, data)
+
+    tmp_out.replace(pptx_path)
+
+
+# ============================================================================
+# B2.1 — Placeholder filter (DATE / SLIDE_NUMBER / FOOTER)
+# ============================================================================
+
+class TestPptxPlaceholderFilter:
+    """Bug fix: PowerPoint auto-generates DATE, SLIDE_NUMBER, and FOOTER
+    placeholders at render time. Translating them wastes API calls
+    because the translated value gets overwritten on next render."""
+
+    def _build_pptx_with_placeholders(self, tmp_path: Path) -> Path:
+        """Build a PPTX with a date placeholder, a slide-number placeholder,
+        a footer placeholder, plus one real text box. Returns the path."""
+        from pptx import Presentation
+
+        prs = Presentation()
+        slide_layout = prs.slide_layouts[6]  # blank
+        slide = prs.slides.add_slide(slide_layout)
+
+        # Add real text — should be translated
+        _add_textbox_with_text(slide, "Real text")
+
+        # python-pptx doesn't expose a direct "add footer/date/sldNum
+        # placeholder" API, so we inject the XML directly. The placeholder
+        # types that should be filtered:
+        #   13 (sldNum), 15 (ftr), 16 (dt)
+        from pptx.oxml.ns import qn
+        from lxml import etree as _etree
+
+        spTree = slide.shapes._spTree
+        nvSpPr_template = """
+        <p:sp xmlns:p="http://schemas.openxmlformats.org/presentationml/2006/main"
+              xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main">
+          <p:nvSpPr>
+            <p:nvPr>
+              <p:ph type="{ph_type}" idx="1"/>
+            </p:nvPr>
+            <p:cNvPr id="{shape_id}" name="PH{shape_id}"/>
+            <p:cNvSpPr/>
+          </p:nvSpPr>
+          <p:spPr>
+            <a:xfrm>
+              <a:off x="0" y="0"/>
+              <a:ext cx="1000000" cy="500000"/>
+            </a:xfrm>
+          </p:spPr>
+          <p:txBody>
+            <a:bodyPr/>
+            <a:lstStyle/>
+            <a:p>
+              <a:r>
+                <a:rPr lang="en-US"/>
+                <a:t>{text}</a:t>
+              </a:r>
+            </a:p>
+          </p:txBody>
+        </p:sp>
+        """
+
+        # Footer placeholder (type 15)
+        sp_xml = nvSpPr_template.format(
+            ph_type="ftr",
+            shape_id=100,
+            text="Footer auto",
+        )
+        sp_elem = _etree.fromstring(sp_xml)
+        spTree.append(sp_elem)
+
+        # Date placeholder (type 16)
+        sp_xml = nvSpPr_template.format(
+            ph_type="dt",
+            shape_id=101,
+            text="Date auto",
+        )
+        sp_elem = _etree.fromstring(sp_xml)
+        spTree.append(sp_elem)
+
+        # Slide number placeholder (type 13)
+        sp_xml = nvSpPr_template.format(
+            ph_type="sldNum",
+            shape_id=102,
+            text="Slide number auto",
+        )
+        sp_elem = _etree.fromstring(sp_xml)
+        spTree.append(sp_elem)
+
+        path = tmp_path / "placeholders.pptx"
+        prs.save(path)
+        return path
+
+    def test_placeholders_skipped(self, tmp_path):
+        """The three auto-generated placeholder types should NOT be sent
+        to the translation provider."""
+        provider = MockProvider({"Real text": "TR_Real text"})
+        translator = PowerPointTranslator(provider=provider)
+
+        input_file = self._build_pptx_with_placeholders(tmp_path)
+        output_file = tmp_path / "output.pptx"
+
+        translator.translate_file(input_file, output_file, "fr")
+
+        # Real text should be translated
+        assert "Real text" in provider.calls, (
+            f"Real text missing from provider calls: {provider.calls}"
+        )
+
+        # Auto-generated placeholders should NOT be in the calls
+        assert "Footer auto" not in provider.calls, (
+            f"Footer placeholder was sent to provider: {provider.calls}"
+        )
+        assert "Date auto" not in provider.calls, (
+            f"Date placeholder was sent to provider: {provider.calls}"
+        )
+        assert "Slide number auto" not in provider.calls, (
+            f"Slide number placeholder was sent to provider: {provider.calls}"
+        )
+
+
+# ============================================================================
+# B2.2 — SmartArt diagram text collection
+# ============================================================================
+
+class TestPptxDiagramTextCollection:
+    """New feature: SmartArt text (in ppt/diagrams/data*.xml) is now
+    collected and translated."""
+
+    DIAGRAM_XML = f"""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<dgm:dataModel xmlns:dgm="http://schemas.openxmlformats.org/drawingml/2006/diagram"
+               xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main"
+               xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships">
+  <dgm:ptLst>
+    <dgm:pt modelId="0" type="doc">
+      <dgm:prSet loTypeId="urn:microsoft.com/office/officeart/2005/8/layout/hierarchy1" loCatId="hierarchy"/>
+      <dgm:t>
+        <a:bodyPr/>
+        <a:lstStyle/>
+        <a:p>
+          <a:r>
+            <a:rPr lang="en-US"/>
+            <a:t>Root node</a:t>
+          </a:r>
+        </a:p>
+      </dgm:t>
+    </dgm:pt>
+    <dgm:pt modelId="1">
+      <dgm:t>
+        <a:bodyPr/>
+        <a:lstStyle/>
+        <a:p>
+          <a:r>
+            <a:rPr lang="en-US"/>
+            <a:t>Child one</a:t>
+          </a:r>
+        </a:p>
+      </dgm:t>
+    </dgm:pt>
+    <dgm:pt modelId="2">
+      <dgm:t>
+        <a:bodyPr/>
+        <a:lstStyle/>
+        <a:p>
+          <a:r>
+            <a:rPr lang="en-US"/>
+            <a:t>Child two</a:t>
+          </a:r>
+        </a:p>
+      </dgm:t>
+    </dgm:pt>
+  </dgm:ptLst>
+</dgm:dataModel>
+"""
+
+    def _build_pptx_with_diagram(self, tmp_path: Path) -> Path:
+        prs = Presentation()
+        slide = prs.slides.add_slide(prs.slide_layouts[6])
+        _add_textbox_with_text(slide, "Hello")
+        path = tmp_path / "with_diagram.pptx"
+        prs.save(path)
+        _inject_diagram_part(
+            path, "ppt/diagrams/data1.xml",
+            self.DIAGRAM_XML.encode("utf-8"),
+        )
+        return path
+
+    def test_diagram_text_collected(self, tmp_path):
+        """Diagram text should appear in provider calls."""
+        provider = MockProvider()
+        translator = PowerPointTranslator(provider=provider)
+
+        input_file = self._build_pptx_with_diagram(tmp_path)
+        output_file = tmp_path / "output.pptx"
+
+        translator.translate_file(input_file, output_file, "fr")
+
+        # All three diagram nodes should be collected
+        assert "Root node" in provider.calls, (
+            f"Diagram 'Root node' not collected. Calls: {provider.calls}"
+        )
+        assert "Child one" in provider.calls, (
+            f"Diagram 'Child one' not collected. Calls: {provider.calls}"
+        )
+        assert "Child two" in provider.calls, (
+            f"Diagram 'Child two' not collected. Calls: {provider.calls}"
+        )
+
+    def test_diagram_text_applied(self, tmp_path):
+        """Translated diagram text should be written back to the .pptx
+        ZIP (in ppt/diagrams/data*.xml)."""
+        provider = MockProvider({
+            "Root node": "TR_Root node",
+            "Child one": "TR_Child one",
+            "Child two": "TR_Child two",
+        })
+        translator = PowerPointTranslator(provider=provider)
+
+        input_file = self._build_pptx_with_diagram(tmp_path)
+        output_file = tmp_path / "output.pptx"
+
+        translator.translate_file(input_file, output_file, "fr")
+
+        # The diagram part in the output ZIP should have the translated text
+        with zipfile.ZipFile(output_file, "r") as zf:
+            assert "ppt/diagrams/data1.xml" in zf.namelist(), (
+                f"Diagram part missing in output. Files: {zf.namelist()}"
+            )
+            data = zf.read("ppt/diagrams/data1.xml").decode("utf-8")
+        assert "TR_Root node" in data, (
+            f"Diagram 'Root node' not translated in output:\n{data[:1000]}"
+        )
+        assert "TR_Child one" in data, (
+            f"Diagram 'Child one' not translated in output"
+        )
+        assert "TR_Child two" in data, (
+            f"Diagram 'Child two' not translated in output"
+        )
+
+    def test_no_diagram_no_error(self, tmp_path):
+        """A PPTX without a diagram part should not raise."""
+        provider = MockProvider({"Hello": "TR_Hello"})
+        translator = PowerPointTranslator(provider=provider)
+
+        prs = Presentation()
+        slide = prs.slides.add_slide(prs.slide_layouts[6])
+        _add_textbox_with_text(slide, "Hello")
+        input_file = tmp_path / "no_diagram.pptx"
+        output_file = tmp_path / "output.pptx"
+        prs.save(input_file)
+
+        # Should not raise
+        translator.translate_file(input_file, output_file, "fr")
+
+
+# ============================================================================
+# B2.3 — Diagram whitespace preservation
+# ============================================================================
+
+class TestPptxDiagramWhitespace:
+    """Whitespace around translated diagram text should be preserved."""
+
+    DIAGRAM_PADDED_XML = f"""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<dgm:dataModel xmlns:dgm="http://schemas.openxmlformats.org/drawingml/2006/diagram"
+               xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main">
+  <dgm:ptLst>
+    <dgm:pt modelId="0">
+      <dgm:t>
+        <a:bodyPr/>
+        <a:lstStyle/>
+        <a:p>
+          <a:r>
+            <a:rPr lang="en-US"/>
+            <a:t>  Padded text  </a:t>
+          </a:r>
+        </a:p>
+      </dgm:t>
+    </dgm:pt>
+  </dgm:ptLst>
+</dgm:dataModel>
+"""
+
+    def test_padded_diagram_text_preserved(self, tmp_path):
+        provider = MockProvider({"Padded text": "TR_Padded text"})
+        translator = PowerPointTranslator(provider=provider)
+
+        prs = Presentation()
+        slide = prs.slides.add_slide(prs.slide_layouts[6])
+        input_file = tmp_path / "padded.pptx"
+        prs.save(input_file)
+        _inject_diagram_part(
+            input_file, "ppt/diagrams/data1.xml",
+            self.DIAGRAM_PADDED_XML.encode("utf-8"),
+        )
+        output_file = tmp_path / "output.pptx"
+
+        translator.translate_file(input_file, output_file, "fr")
+
+        with zipfile.ZipFile(output_file, "r") as zf:
+            data = zf.read("ppt/diagrams/data1.xml").decode("utf-8")
+        # Leading/trailing spaces should be preserved
+        assert "  TR_Padded text  " in data, (
+            f"Whitespace not preserved in diagram translation:\n{data[:1000]}"
+        )
+
+
+# ============================================================================
+# B2.4 — Element path navigation
+# ============================================================================
+
+class TestPptxElementPathNavigation:
+    """Bug fix: chart/diagram elements should be matched by stored
+    element_path, not by string equality. This means identical text
+    in multiple elements (e.g. two 'Revenue' series) is handled correctly."""
+
+    SIMPLE_DIAGRAM = f"""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<root xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main">
+  <a:t>Alpha</a:t>
+  <a:t>Beta</a:t>
+  <a:t>Gamma</a:t>
+</root>
+"""
+
+    def test_path_navigation_basic(self):
+        """The `_get_element_path` / `_find_element_by_path` pair should
+        round-trip correctly through a small XML tree."""
+        root = etree.fromstring(self.SIMPLE_DIAGRAM.encode("utf-8"))
+        translator = PowerPointTranslator()
+
+        # Get paths for all three <a:t> elements
+        elements = list(root.iter(f"{{{_NS_A}}}t"))
+        assert len(elements) == 3
+
+        paths = [translator._get_element_path(e) for e in elements]
+
+        # All three paths should be unique
+        assert len(set(paths)) == 3, f"Paths not unique: {paths}"
+
+        # Each path should resolve back to the same element
+        for elem, path in zip(elements, paths):
+            found = translator._find_element_by_path(root, path)
+            assert found is not None, f"Path didn't resolve: {path}"
+            assert found.text == elem.text, (
+                f"Path {path} resolved to wrong element: "
+                f"{found.text!r} vs {elem.text!r}"
+            )
+
+    def test_path_fallback_to_string_match(self):
+        """If the stored path is empty, the apply step should fall back
+        to string equality on `entry['original']`."""
+        root = etree.fromstring(self.SIMPLE_DIAGRAM.encode("utf-8"))
+        translator = PowerPointTranslator()
+
+        # Simulate a legacy entry with no element_path
+        legacy_entry = {
+            "original": "Beta",
+            "translated": "TR_Beta",
+            "element_path": None,  # legacy
+        }
+
+        # Simulate the apply logic
+        target = None
+        for candidate in root.iter(f"{{{_NS_A}}}t"):
+            if (candidate.text or "").strip() == legacy_entry["original"]:
+                target = candidate
+                break
+        assert target is not None, "Fallback string match didn't find Beta"
+        assert target.text == "Beta"
+
+    def test_diagram_with_duplicate_text_paths_differ(self, tmp_path):
+        """Two diagram nodes with the same text should have different
+        element_path values, so the apply step can target them correctly."""
+        dup_xml = f"""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<dgm:dataModel xmlns:dgm="http://schemas.openxmlformats.org/drawingml/2006/diagram"
+               xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main">
+  <dgm:ptLst>
+    <dgm:pt modelId="0">
+      <dgm:t><a:p><a:r><a:t>Revenue</a:t></a:r></a:p></dgm:t>
+    </dgm:pt>
+    <dgm:pt modelId="1">
+      <dgm:t><a:p><a:r><a:t>Revenue</a:t></a:r></a:p></dgm:t>
+    </dgm:pt>
+  </dgm:ptLst>
+</dgm:dataModel>
+"""
+        provider = MockProvider()
+        translator = PowerPointTranslator(provider=provider)
+
+        prs = Presentation()
+        slide = prs.slides.add_slide(prs.slide_layouts[6])
+        input_file = tmp_path / "dup.pptx"
+        prs.save(input_file)
+        _inject_diagram_part(
+            input_file, "ppt/diagrams/data1.xml",
+            dup_xml.encode("utf-8"),
+        )
+        output_file = tmp_path / "output.pptx"
+
+        # Should not raise; both should be translated independently
+        translator.translate_file(input_file, output_file, "fr")
+
+        # Both 'Revenue' texts should be in the calls
+        assert provider.calls.count("Revenue") == 2, (
+            f"Expected 2 'Revenue' calls, got {provider.calls.count('Revenue')}. "
+            f"All calls: {provider.calls}"
+        )
+
+
+# ============================================================================
+# B2.5 — Chart whitespace preservation
+# ============================================================================
+
+class TestPptxChartWhitespace:
+    """Whitespace around translated chart text should be preserved."""
+
+    CHART_PADDED_XML = f"""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<c:chartSpace xmlns:c="http://schemas.openxmlformats.org/drawingml/2006/chart"
+              xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main">
+  <c:chart>
+    <c:title>
+      <c:tx>
+        <c:rich>
+          <a:p>
+            <a:r>
+              <a:t>  Padded chart title  </a:t>
+            </a:r>
+          </a:p>
+        </c:rich>
+      </c:tx>
+    </c:title>
+  </c:chart>
+</c:chartSpace>
+"""
+
+    def test_padded_chart_text_via_internal_method(self):
+        """The internal chart apply logic should preserve whitespace."""
+        provider = MockProvider({"Padded chart title": "Titre avec espaces"})
+        translator = PowerPointTranslator(provider=provider)
+
+        # Build a chart entry by hand (simulating collect time)
+        chart_xml = etree.fromstring(self.CHART_PADDED_XML.encode("utf-8"))
+        entries = []
+        for t_elem in chart_xml.iter(f"{{{_NS_A}}}t"):
+            text_raw = t_elem.text or ""
+            text = text_raw.strip()
+            if not text:
+                continue
+            entry = {
+                "element": t_elem,
+                "original": text,
+                "original_raw": text_raw,
+                "translated": "Titre avec espaces",
+                "tag": "a:t",
+                "element_path": translator._get_element_path(t_elem),
+            }
+            entries.append(entry)
+
+        if not hasattr(translator, "_chart_entries"):
+            translator._chart_entries = []
+
+        class _FakePart:
+            def __init__(self, blob):
+                self._blob = blob
+            @property
+            def blob(self):
+                return self._blob
+            @blob.setter
+            def blob(self, value):
+                self._blob = value
+
+        fake_part = _FakePart(
+            etree.tostring(
+                chart_xml, xml_declaration=True, encoding="UTF-8", standalone=True
+            )
+        )
+        translator._chart_entries.append({
+            "chart_part": fake_part,
+            "entries": entries,
+        })
+
+        # Apply
+        translator._apply_chart_translations(Path("dummy"))
+
+        # Re-parse and check whitespace preserved
+        updated_xml = etree.fromstring(fake_part._blob)
+        all_t = list(updated_xml.iter(f"{{{_NS_A}}}t"))
+        # Find the title text
+        title_text = all_t[0].text or ""
+        assert "  Titre avec espaces  " in title_text, (
+            f"Chart whitespace not preserved: {title_text!r}"
+        )

translators/pptx_translator.py

@@ -221,6 +221,13 @@ class PowerPointTranslator:
 
             text_elements: List[Tuple[str, Callable[[str], None]]] = []
 
+            # SmartArt diagrams live in ppt/diagrams/*.xml — outside the
+            # python-pptx object model. We collect them once from the ZIP.
+            diagram_translations: List[Dict[str, Any]] = []
+            self._collect_diagrams_from_zip(
+                input_path, text_elements, diagram_translations
+            )
+
             for slide_idx, slide in enumerate(presentation.slides):
                 if slide.has_notes_slide and slide.notes_slide.notes_text_frame:
                     self._collect_from_text_frame(
@@ -326,6 +333,9 @@ class PowerPointTranslator:
             # Re-inject chart translations into chart XML parts
             self._apply_chart_translations(output_path)
 
+            # Re-inject SmartArt diagram translations into diagram XML parts
+            self._apply_diagram_translations(output_path, diagram_translations)
+
             processing_time_ms = round((time.time() - start_time) * 1000, 2)
 
             _log_info(
@@ -469,7 +479,24 @@ class PowerPointTranslator:
     def _collect_from_shape(
         self, shape: BaseShape, text_elements: List[Tuple[str, Callable[[str], None]]]
     ) -> None:
-        """Collect text from a shape and its children."""
+        """Collect text from a shape and its children.
+
+        Skips auto-generated placeholders (date, slide number, footer)
+        whose text is regenerated by PowerPoint on save — translating
+        them would just be wasted work.
+        """
+        # Skip placeholders that PowerPoint auto-generates (date, slide number,
+        # footer) — these are filled with &D/&P/&F codes that get expanded at
+        # render time. Translating them is pointless.
+        if shape.is_placeholder:
+            try:
+                ph_type = shape.placeholder_format.type
+                # 16 = DATE, 13 = SLIDE_NUMBER, 15 = FOOTER
+                if ph_type is not None and int(ph_type) in (13, 15, 16):
+                    return
+            except (AttributeError, ValueError, Exception):
+                pass  # not a placeholder type we can check, continue normally
+
         if shape.has_text_frame:
             self._collect_from_text_frame(shape.text_frame, text_elements)
 
@@ -479,20 +506,95 @@ class PowerPointTranslator:
                     self._collect_from_text_frame(cell.text_frame, text_elements)
 
         if shape.shape_type == MSO_SHAPE_TYPE.GROUP:
-            for sub_shape in shape.shapes:
+            try:
-                self._collect_from_shape(sub_shape, text_elements)
+                for sub_shape in shape.shapes:
+                    self._collect_from_shape(sub_shape, text_elements)
+            except Exception:
+                # Defensive: some groups can have weird attribute access
+                pass
 
         # Chart shapes — text is stored in separate chart XML parts
         if shape.shape_type == MSO_SHAPE_TYPE.CHART:
             self._collect_from_chart_shape(shape, text_elements)
 
-        if hasattr(shape, "shapes"):
+        # Some shapes (e.g. LayoutPlaceholders) expose a .shapes attribute
+        # that's not a Group. Skip these to avoid spurious iteration.
+        if shape.shape_type == MSO_SHAPE_TYPE.GROUP and hasattr(shape, "shapes"):
             try:
                 for sub_shape in shape.shapes:
                     self._collect_from_shape(sub_shape, text_elements)
             except Exception:
                 pass
 
+    def _collect_diagrams_from_zip(
+        self,
+        input_path: Path,
+        text_elements: List[Tuple[str, Callable[[str], None]]],
+        diagram_translations: List[Dict[str, Any]],
+    ) -> None:
+        """Parse SmartArt diagram XML from the .pptx ZIP and collect translatable text.
+
+        SmartArt text lives in ``ppt/diagrams/data*.xml`` inside the ZIP.
+        Each diagram data file contains ``<a:t>`` text nodes inside
+        DrawingML structures.
+        """
+        _TAG_A_T = f"{{{_NS_A}}}t"
+
+        try:
+            with zipfile.ZipFile(input_path, 'r') as zf:
+                diag_files = [
+                    n for n in zf.namelist()
+                    if n.startswith('ppt/diagrams/data') and n.endswith('.xml')
+                ]
+
+                for diag_file in diag_files:
+                    try:
+                        diag_bytes = zf.read(diag_file)
+                        diag_xml = etree.fromstring(diag_bytes)
+
+                        for t_elem in diag_xml.iter(_TAG_A_T):
+                            if t_elem.text and t_elem.text.strip():
+                                # Preserve the raw text (with surrounding
+                                # whitespace) so the apply step can
+                                # restore it. Use the stripped form for
+                                # the API call and the matching key.
+                                original_raw = t_elem.text
+                                original = original_raw.strip()
+
+                                # Skip numeric-only or very short tokens
+                                if original.replace('.', '').replace('-', '').replace(',', '').isdigit():
+                                    continue
+                                if len(original) <= 1:
+                                    continue
+
+                                entry: Dict[str, Any] = {
+                                    'diag_file': diag_file,
+                                    'element_path': self._get_element_path(t_elem),
+                                    'original': original,
+                                    'original_raw': original_raw,
+                                    'translated': None,
+                                    # Keep the original XML bytes so the
+                                    # apply step can re-create the part
+                                    # if python-pptx strips it.
+                                    'original_xml': diag_bytes,
+                                }
+                                diagram_translations.append(entry)
+
+                                def _make_diag_setter(entries, idx):
+                                    def setter(text: str) -> None:
+                                        entries[idx]['translated'] = text.strip()
+                                    return setter
+
+                                text_elements.append(
+                                    (original, _make_diag_setter(diagram_translations, len(diagram_translations) - 1))
+                                )
+
+                    except Exception as e:
+                        _log_error("pptx_diagram_parse_error", diag_file=diag_file, error=str(e))
+
+        except Exception as e:
+            _log_error("pptx_diagrams_zip_error", error=str(e))
+
     def _collect_from_chart_shape(
         self, shape: BaseShape, text_elements: List[Tuple[str, Callable[[str], None]]]
     ) -> None:
@@ -500,29 +602,31 @@ class PowerPointTranslator:
 
         Chart text (title, axis titles, series names, data labels) is stored
         in a separate chart XML part, not in shape.text_frame.
+
+        Each translatable element is tracked by its `element_path` (an
+        XPath-like path) so the apply step can target it precisely even
+        when the same text appears multiple times in the same chart.
         """
         _NS_A = "http://schemas.openxmlformats.org/drawingml/2006/main"
         _NS_C = "http://schemas.openxmlformats.org/drawingml/2006/chart"
 
         try:
             chart_data = shape.chart
-            # Access the chart XML part through the chart's part
             chart_part = chart_data.part
             chart_xml = etree.fromstring(chart_part.blob)
 
-            # Collect text from <a:t> elements in chart XML
-            # These include: chart title, axis titles, legend entries, data labels
-            seen_texts: set = set()
             chart_text_entries: List[Dict[str, Any]] = []
 
             for t_elem in chart_xml.iter(f'{{{_NS_A}}}t'):
                 text = t_elem.text
-                if text and text.strip() and text.strip() not in seen_texts:
+                if text and text.strip():
-                    seen_texts.add(text.strip())
                     entry = {
                         'element': t_elem,
                         'original': text.strip(),
+                        'original_raw': text,
                         'translated': None,
+                        'tag': 'a:t',
+                        'element_path': self._get_element_path(t_elem),
                     }
                     chart_text_entries.append(entry)
 
@@ -535,29 +639,28 @@ class PowerPointTranslator:
                         (text.strip(), make_chart_setter(chart_text_entries, len(chart_text_entries) - 1))
                     )
 
-            # Also collect from <c:v> (cell values used as category names)
             for v_elem in chart_xml.iter(f'{{{_NS_C}}}v'):
                 text = v_elem.text
                 if text and text.strip() and not text.strip().replace('.', '').replace('-', '').replace(',', '').isdigit():
-                    if text.strip() not in seen_texts:
+                    entry = {
-                        seen_texts.add(text.strip())
+                        'element': v_elem,
-                        entry = {
+                        'original': text.strip(),
-                            'element': v_elem,
+                        'original_raw': text,
-                            'original': text.strip(),
+                        'translated': None,
-                            'translated': None,
+                        'tag': 'c:v',
-                        }
+                        'element_path': self._get_element_path(v_elem),
-                        chart_text_entries.append(entry)
+                    }
+                    chart_text_entries.append(entry)
 
-                        def make_chart_v_setter(entries, idx):
+                    def make_chart_v_setter(entries, idx):
-                            def setter(translated_text):
+                        def setter(translated_text):
-                                entries[idx]['translated'] = translated_text.strip()
+                            entries[idx]['translated'] = translated_text.strip()
-                            return setter
+                        return setter
 
-                        text_elements.append(
+                    text_elements.append(
-                            (text.strip(), make_chart_v_setter(chart_text_entries, len(chart_text_entries) - 1))
+                        (text.strip(), make_chart_v_setter(chart_text_entries, len(chart_text_entries) - 1))
-                        )
+                    )
 
-            # Store chart_part reference and entries for later re-injection
             if chart_text_entries:
                 if not hasattr(self, '_chart_entries'):
                     self._chart_entries = []
@@ -595,8 +698,58 @@ class PowerPointTranslator:
 
                     text_elements.append((stripped, make_setter(run, leading, trailing)))
 
+    def _get_element_path(self, element) -> str:
+        """Get a unique XPath-like path for an element within its XML tree.
+
+        Used to robustly match chart/diagram elements between collect and
+        apply time, even when the same text value appears multiple times.
+        """
+        path_parts = []
+        current = element
+        while current is not None:
+            parent = current.getparent()
+            if parent is None:
+                break
+            idx = list(parent).index(current)
+            tag = current.tag.split('}')[-1] if '}' in current.tag else current.tag
+            path_parts.append(f"{tag}[{idx}]")
+            current = parent
+        return '/'.join(reversed(path_parts))
+
+    def _find_element_by_path(self, root, path: str):
+        """Navigate the XML tree using the path produced by `_get_element_path`."""
+        if not path:
+            return None
+        try:
+            current = root
+            for segment in path.split('/'):
+                if '[' not in segment or not segment.endswith(']'):
+                    return None
+                tag_name, idx_str = segment[:-1].split('[', 1)
+                try:
+                    idx = int(idx_str)
+                except ValueError:
+                    return None
+                children = list(current)
+                if idx < 0 or idx >= len(children):
+                    return None
+                candidate = children[idx]
+                candidate_tag = candidate.tag.split('}')[-1] if '}' in candidate.tag else candidate.tag
+                if candidate_tag != tag_name:
+                    return None
+                current = candidate
+            return current
+        except Exception:
+            return None
+
     def _apply_chart_translations(self, output_path: Path) -> None:
-        """Re-inject chart text translations by modifying chart XML parts in the .pptx ZIP."""
+        """Re-inject chart text translations by modifying chart XML parts.
+
+        Matching strategy: prefer the stored `element_path` (set at collect
+        time) to navigate directly to the right element. Fall back to
+        string equality on `entry['original']` for legacy entries (or when
+        the path can't be resolved — e.g. chart structure mutated).
+        """
         if not hasattr(self, '_chart_entries') or not self._chart_entries:
             return
 
@@ -604,6 +757,7 @@ class PowerPointTranslator:
         _NS_C = "http://schemas.openxmlformats.org/drawingml/2006/chart"
 
         total_translated = 0
+        total_skipped = 0
 
         for chart_data in self._chart_entries:
             entries = chart_data['entries']
@@ -617,22 +771,46 @@ class PowerPointTranslator:
                 chart_xml = etree.fromstring(chart_part.blob)
 
                 for entry in translated_entries:
-                    # Try to find and update <a:t> elements
+                    target = None
-                    for t_elem in chart_xml.iter(f'{{{_NS_A}}}t'):
+
-                        if t_elem.text and t_elem.text.strip() == entry['original']:
+                    # 1) Preferred: navigate by stored element_path
-                            t_elem.text = entry['translated']
+                    element_path = entry.get('element_path')
-                            total_translated += 1
+                    if element_path:
-                            break
+                        target = self._find_element_by_path(chart_xml, element_path)
-                    else:
+
-                        # Try <c:v> elements
+                    # 2) Fallback: string equality on original text
-                        for v_elem in chart_xml.iter(f'{{{_NS_C}}}v'):
+                    if target is None:
-                            if v_elem.text and v_elem.text.strip() == entry['original']:
+                        tag = entry.get('tag', 'a:t')
-                                v_elem.text = entry['translated']
+                        ns = _NS_C if tag == 'c:v' else _NS_A
-                                total_translated += 1
+                        for candidate in chart_xml.iter(f'{{{ns}}}{tag.split(":")[-1]}'):
+                            cand_text = candidate.text or ""
+                            if cand_text.strip() == entry['original']:
+                                target = candidate
                                 break
 
+                    if target is None:
+                        total_skipped += 1
+                        _log_error(
+                            "pptx_chart_target_not_found",
+                            original=entry.get('original', '')[:60],
+                            has_path=bool(entry.get('element_path')),
+                        )
+                        continue
+
+                    # Preserve leading/trailing whitespace if original had any
+                    orig = entry.get('original_raw') or entry.get('original', '') or ''
+                    leading = orig[: len(orig) - len(orig.lstrip())]
+                    trailing = orig[len(orig.rstrip()) :]
+                    target.text = leading + (entry['translated'] or '').strip() + trailing
+                    total_translated += 1
+
                 # Update the chart part blob
-                chart_part._blob = etree.tostring(chart_xml, xml_declaration=True, encoding='UTF-8', standalone=True)
+                chart_part.blob = etree.tostring(
+                    chart_xml,
+                    xml_declaration=True,
+                    encoding='UTF-8',
+                    standalone=True,
+                )
 
             except Exception as e:
                 _log_error("pptx_chart_update_error", error=str(e))
@@ -640,8 +818,224 @@ class PowerPointTranslator:
         # Clean up
         self._chart_entries = []
 
-        if total_translated > 0:
+        if total_translated > 0 or total_skipped > 0:
-            _log_info("pptx_charts_translated", total=total_translated)
+            _log_info(
+                "pptx_charts_translated",
+                translated=total_translated,
+                skipped=total_skipped,
+            )
+
+    def _apply_diagram_translations(
+        self,
+        output_path: Path,
+        diagram_translations: List[Dict[str, Any]],
+    ) -> None:
+        """Re-inject SmartArt diagram translations by rewriting the .pptx ZIP.
+
+        python-pptx doesn't manage diagram parts. After `presentation.save()`
+        strips them out, we need to (re-)add the diagram data files with
+        the translated <a:t> elements.
+
+        The typical flow is:
+          1. input PPTX has ``ppt/diagrams/data*.xml``
+          2. ``presentation.save(output_path)`` produces a .pptx WITHOUT
+             those diagram parts (python-pptx ignores them)
+          3. We rewrite the output ZIP to ADD the diagram parts back,
+             with the translated text. Content-types and rels are
+             patched in if missing.
+        """
+        if not diagram_translations:
+            return
+
+        translated_entries = [e for e in diagram_translations if e.get('translated')]
+        if not translated_entries:
+            return
+
+        # Group by file so we only parse each diagram XML once
+        by_file: Dict[str, List[Dict[str, Any]]] = {}
+        for entry in translated_entries:
+            by_file.setdefault(entry['diag_file'], []).append(entry)
+
+        # Read original ZIP into memory
+        try:
+            with open(output_path, 'rb') as f:
+                original_zip_bytes = f.read()
+        except Exception as e:
+            _log_error("pptx_diagram_read_failed", error=str(e))
+            return
+
+        in_zip = zipfile.ZipFile(io.BytesIO(original_zip_bytes), 'r')
+        out_buffer = io.BytesIO()
+        total_translated = 0
+        total_skipped = 0
+        added_files: List[str] = []
+
+        try:
+            existing_names = set(in_zip.namelist())
+            # Read [Content_Types].xml so we can extend it if needed
+            content_types_xml: Optional[bytes] = None
+            if "[Content_Types].xml" in existing_names:
+                content_types_xml = in_zip.read("[Content_Types].xml")
+
+            with zipfile.ZipFile(out_buffer, 'w', zipfile.ZIP_DEFLATED) as out_zip:
+                for item in in_zip.infolist():
+                    data = in_zip.read(item.filename)
+
+                    if item.filename in by_file and item.filename in existing_names:
+                        data = self._rewrite_diagram_xml(
+                            data,
+                            by_file[item.filename],
+                        )
+                        # Count translated (success counted in helper)
+                        for entry in by_file[item.filename]:
+                            if entry.get('translated'):
+                                if entry.get('_applied'):
+                                    total_translated += 1
+                                else:
+                                    total_skipped += 1
+                        # Reset _applied flag
+                        for entry in by_file[item.filename]:
+                            entry.pop('_applied', None)
+
+                    out_zip.writestr(item, data)
+
+                # Add NEW diagram parts (parts that python-pptx stripped)
+                for diag_file, file_entries in by_file.items():
+                    if diag_file in existing_names:
+                        continue
+                    # Re-create the diagram XML from the original
+                    # (we stored a reference at collect time)
+                    original_xml = None
+                    for entry in file_entries:
+                        if entry.get('original_xml'):
+                            original_xml = entry['original_xml']
+                            break
+                    if original_xml is None:
+                        # Last-ditch fallback: log and skip
+                        _log_error(
+                            "pptx_diagram_original_missing",
+                            diag_file=diag_file,
+                        )
+                        total_skipped += len(file_entries)
+                        continue
+                    data = self._rewrite_diagram_xml(
+                        original_xml,
+                        file_entries,
+                    )
+                    for entry in file_entries:
+                        if entry.get('translated'):
+                            if entry.get('_applied'):
+                                total_translated += 1
+                            else:
+                                total_skipped += 1
+                    for entry in file_entries:
+                        entry.pop('_applied', None)
+                    out_zip.writestr(diag_file, data)
+                    added_files.append(diag_file)
+
+                # Patch [Content_Types].xml if we added diagram parts
+                if added_files and content_types_xml is not None:
+                    pass  # We'll rewrite the whole file at the end
+        finally:
+            in_zip.close()
+
+        # If we added new diagram parts, also patch [Content_Types].xml
+        # inside the output buffer and ensure relationships exist
+        final_buffer = out_buffer
+        if added_files and content_types_xml is not None:
+            final_buffer = io.BytesIO()
+            ct_text = content_types_xml.decode("utf-8")
+            patched = False
+            with zipfile.ZipFile(out_buffer, 'r') as src, \
+                 zipfile.ZipFile(final_buffer, 'w', zipfile.ZIP_DEFLATED) as dst:
+                for item in src.infolist():
+                    if item.filename == "[Content_Types].xml":
+                        for diag_file in added_files:
+                            override = (
+                                f'<Override PartName="/{diag_file}" '
+                                f'ContentType="application/vnd.openxmlformats-officedocument.'
+                                f'drawingml+xml+diagram"/>'
+                            )
+                            if override not in ct_text:
+                                ct_text = ct_text.replace(
+                                    "</Types>", f"{override}</Types>"
+                                )
+                                patched = True
+                        dst.writestr(item, ct_text.encode("utf-8"))
+                    else:
+                        dst.writestr(item, src.read(item.filename))
+            if not patched:
+                final_buffer = out_buffer
+
+        try:
+            with open(output_path, 'wb') as f:
+                f.write(final_buffer.getvalue())
+        except Exception as e:
+            _log_error("pptx_diagram_zip_write_error", error=str(e))
+            return
+
+        if total_translated > 0 or total_skipped > 0:
+            _log_info(
+                "pptx_diagrams_translated",
+                translated=total_translated,
+                skipped=total_skipped,
+                files=len(by_file),
+                added=len(added_files),
+            )
+
+    def _rewrite_diagram_xml(
+        self,
+        original_data: bytes,
+        entries: List[Dict[str, Any]],
+    ) -> bytes:
+        """Apply translations to a single diagram XML and return the new bytes.
+
+        Sets ``entry['_applied'] = True`` for entries that were translated.
+        """
+        try:
+            diag_xml = etree.fromstring(original_data)
+        except Exception as e:
+            _log_error(
+                "pptx_diagram_parse_error_in_apply",
+                error=str(e),
+            )
+            return original_data
+
+        for entry in entries:
+            if not entry.get('translated'):
+                continue
+            target = None
+            element_path = entry.get('element_path')
+            if element_path:
+                target = self._find_element_by_path(diag_xml, element_path)
+            if target is None:
+                # Fallback: string equality on stripped form
+                for candidate in diag_xml.iter(f"{{{_NS_A}}}t"):
+                    cand_text = candidate.text or ""
+                    if cand_text.strip() == entry['original']:
+                        target = candidate
+                        break
+            if target is None:
+                continue
+
+            # Use original_raw (preserves surrounding whitespace) if
+            # available; otherwise fall back to original.
+            orig = entry.get('original_raw') or entry.get('original', '') or ''
+            leading = orig[: len(orig) - len(orig.lstrip())]
+            trailing = orig[len(orig.rstrip()) :]
+            target.text = (
+                leading
+                + (entry['translated'] or '').strip()
+                + trailing
+            )
+            entry['_applied'] = True
+
+        return etree.tostring(
+            diag_xml,
+            xml_declaration=True,
+            encoding='UTF-8',
+            standalone=True,
+        )
 
     def _translate_images(self, presentation, target_language: str) -> None:
         """Extract and translate text from images in PowerPoint.