fix(create-chapter): rebrand file names + xlsx inline strings; render via Slides API not LibreOffice

lib/aaif_events/slides_export.py

@@ -0,0 +1,96 @@
+"""Render a single slide of a Drive-hosted .pptx to PNG via the Slides API,
+instead of a local LibreOffice conversion. LibreOffice substitutes local
+system fonts for the deck's actual brand fonts whenever the render machine
+doesn't have those fonts installed (true for effectively every machine but
+the original designer's), silently producing wrong-looking exports; Google's
+own renderer (used here) does not.
+
+Requires the `gws` CLI (Drive + Slides scopes) on PATH and authenticated.
+"""
+import json
+import os
+import subprocess
+import sys
+import time
+import urllib.request
+
+_SLIDES_MIME = "application/vnd.google-apps.presentation"
+
+# Mirrors skills/aaif-create-chapter/scripts/create_chapter.py's _gws/gws_json —
+# same retry list and empty/non-JSON-output guards, so a transient Drive/Slides
+# hiccup here self-heals the same way it does for the rest of the toolkit.
+_TRANSIENT = ("timed out", "internalError", "HTTP request failed",
+              "Connection", "temporarily", "rateLimit", "userRateLimit",
+              "backendError", "503", "500", "502")
+
+
+def _gws(cmd, retries=5):
+    for i in range(retries):
+        r = subprocess.run(cmd, capture_output=True, text=True)
+        if r.returncode == 0:
+            return r.stdout
+        msg = (r.stderr or "") + (r.stdout or "")
+        if i < retries - 1 and any(k in msg for k in _TRANSIENT):
+            time.sleep(2 * (i + 1))
+            continue
+        raise RuntimeError("gws failed (%s) for %s: %s"
+                          % (r.returncode, " ".join(cmd)[:200], msg.strip()[:400]))
+
+
+def _gws_json(*args, params=None, body=None):
+    cmd = ["gws", *args]
+    if params is not None:
+        cmd += ["--params", json.dumps(params)]
+    if body is not None:
+        cmd += ["--json", json.dumps(body)]
+    out = _gws(cmd)
+    s = "\n".join(l for l in out.split("\n") if "keyring backend" not in l).strip()
+    if not s:
+        raise RuntimeError("gws produced no JSON output for: %s" % " ".join(args))
+    try:
+        return json.loads(s)
+    except json.JSONDecodeError:
+        raise RuntimeError("gws returned non-JSON output for %s: %s" % (" ".join(args), s[:200]))
+
+
+def render_slide_png(file_id, out_path, slide_index=0, thumbnail_size="WIDTH2000_PX"):
+    """Export slide `slide_index` (0-based) of the Drive pptx `file_id` to a PNG
+    at `out_path`. Makes a throwaway Google Slides copy to render from (Slides
+    thumbnails only work on native Slides files, not stored .pptx blobs) and
+    trashes it afterward — the source file is never modified."""
+    presentation_id = None
+    try:
+        copy = _gws_json("drive", "files", "copy",
+                          params={"fileId": file_id, "supportsAllDrives": True, "fields": "id"},
+                          body={"name": "TEMP - render_slide_png", "mimeType": _SLIDES_MIME})
+        presentation_id = copy["id"]
+
+        presentation = _gws_json("slides", "presentations", "get",
+                                  params={"presentationId": presentation_id, "fields": "slides.objectId"})
+        page_object_id = presentation["slides"][slide_index]["objectId"]
+
+        thumb = _gws_json("slides", "presentations", "pages", "getThumbnail",
+                           params={"presentationId": presentation_id, "pageObjectId": page_object_id,
+                                   "thumbnailProperties.thumbnailSize": thumbnail_size})
+        os.makedirs(os.path.dirname(out_path) or ".", exist_ok=True)
+        urllib.request.urlretrieve(thumb["contentUrl"], out_path)
+        if os.path.getsize(out_path) < 1000:
+            raise RuntimeError("rendered thumbnail suspiciously small (%s)" % out_path)
+        return out_path
+    finally:
+        # The throwaway copy has no `parents` set, so it lands in the SAME
+        # folder as the source file - not an out-of-sight scratch location.
+        # If trashing repeatedly fails and this is ever run against
+        # TemplateCity (aaif-create-chapter's own recalibration step does
+        # exactly that), a stray "TEMP - render_slide_png" file would get
+        # picked up and cloned into every subsequent chapter. Surface
+        # cleanup failures loudly instead of swallowing them silently so a
+        # stray copy gets noticed and trashed by hand.
+        if presentation_id is not None:
+            try:
+                _gws_json("drive", "files", "update",
+                          params={"fileId": presentation_id, "supportsAllDrives": True},
+                          body={"trashed": True})
+            except Exception as e:
+                print("WARNING: could not trash temp Slides copy %s: %s" % (presentation_id, e),
+                      file=sys.stderr)

lib/aaif_events/tests/test_slides_export.py

@@ -0,0 +1,144 @@
+import unittest
+from unittest import mock
+
+from aaif_events import slides_export as se
+
+
+class TestGws(unittest.TestCase):
+    """_gws's retry contract, mirroring TestCall in test_luma.py: transient
+    errors retry, everything else fails fast. time.sleep is patched so the
+    backoff doesn't actually happen."""
+
+    def setUp(self):
+        p = mock.patch("time.sleep")
+        p.start()
+        self.addCleanup(p.stop)
+
+    def _result(self, returncode, stdout="", stderr=""):
+        return mock.Mock(returncode=returncode, stdout=stdout, stderr=stderr)
+
+    def test_retries_transient_then_succeeds(self):
+        with mock.patch("subprocess.run",
+                        side_effect=[self._result(1, stderr="503 backendError"),
+                                     self._result(0, stdout="ok")]) as run:
+            self.assertEqual(se._gws(["gws", "x"]), "ok")
+            self.assertEqual(run.call_count, 2)
+
+    def test_non_transient_fails_immediately(self):
+        with mock.patch("subprocess.run",
+                        return_value=self._result(1, stderr="permission denied")) as run:
+            with self.assertRaises(RuntimeError) as cm:
+                se._gws(["gws", "drive", "files", "get"])
+            self.assertEqual(run.call_count, 1)
+            self.assertIn("gws drive files get", str(cm.exception))
+
+    def test_retries_exhausted_raises_with_cmd_context(self):
+        with mock.patch("subprocess.run",
+                        return_value=self._result(1, stderr="500 internalError")) as run:
+            with self.assertRaises(RuntimeError) as cm:
+                se._gws(["gws", "slides", "presentations", "get"], retries=2)
+            self.assertEqual(run.call_count, 2)
+            self.assertIn("slides presentations get", str(cm.exception))
+
+
+class TestGwsJson(unittest.TestCase):
+    def test_empty_output_raises(self):
+        with mock.patch.object(se, "_gws", return_value="   \n  "):
+            with self.assertRaises(RuntimeError) as cm:
+                se._gws_json("drive", "files", "copy")
+            self.assertIn("no JSON output", str(cm.exception))
+
+    def test_non_json_output_raises(self):
+        with mock.patch.object(se, "_gws", return_value="<html>oops</html>"):
+            with self.assertRaises(RuntimeError) as cm:
+                se._gws_json("drive", "files", "copy")
+            self.assertIn("non-JSON output", str(cm.exception))
+
+    def test_strips_keyring_backend_noise_line(self):
+        with mock.patch.object(se, "_gws", return_value='Using keyring backend: keyring\n{"id": "abc"}'):
+            self.assertEqual(se._gws_json("drive", "files", "copy"), {"id": "abc"})
+
+    def test_params_and_body_become_cli_flags(self):
+        with mock.patch("subprocess.run",
+                        return_value=mock.Mock(returncode=0, stdout="{}", stderr="")) as run:
+            se._gws_json("drive", "files", "copy", params={"fileId": "f1"}, body={"name": "n"})
+            cmd = run.call_args[0][0]
+            self.assertIn("--params", cmd)
+            self.assertIn("--json", cmd)
+
+
+class TestRenderSlidePng(unittest.TestCase):
+    """render_slide_png's orchestration, with _gws_json faked per gws subcommand
+    so no real network/CLI call happens."""
+
+    def _fake_gws_json(self, responses):
+        def fn(*args, **kwargs):
+            key = args
+            if key not in responses:
+                raise AssertionError("unexpected gws call: %r" % (args,))
+            result = responses[key]
+            if isinstance(result, Exception):
+                raise result
+            return result
+        return fn
+
+    def test_cleanup_runs_even_when_body_raises(self):
+        responses = {
+            ("drive", "files", "copy"): {"id": "pres1"},
+            ("slides", "presentations", "get"): {"slides": [{"objectId": "p1"}]},
+            ("slides", "presentations", "pages", "getThumbnail"): {"contentUrl": "https://example.invalid/x.png"},
+            ("drive", "files", "update"): {"id": "pres1"},
+        }
+        trash_calls = []
+        def fake(*args, **kwargs):
+            if args == ("drive", "files", "update"):
+                trash_calls.append(kwargs.get("params", {}).get("fileId"))
+            return self._fake_gws_json(responses)(*args, **kwargs)
+
+        with mock.patch.object(se, "_gws_json", side_effect=fake), \
+                mock.patch("urllib.request.urlretrieve"), \
+                mock.patch("os.path.getsize", return_value=10):  # < 1000 -> triggers the raise
+            with self.assertRaises(RuntimeError) as cm:
+                se.render_slide_png("file1", "/tmp/out.png")
+            self.assertIn("suspiciously small", str(cm.exception))
+
+        # cleanup (trash) must still have run against the copy it made, even
+        # though the function body raised.
+        self.assertEqual(trash_calls, ["pres1"])
+
+    def test_no_cleanup_attempted_when_copy_itself_fails(self):
+        with mock.patch.object(se, "_gws_json", side_effect=RuntimeError("copy failed")) as gj:
+            with self.assertRaises(RuntimeError):
+                se.render_slide_png("file1", "/tmp/out.png")
+            # only the failed copy call - no trash call, since there is no
+            # presentation_id to trash.
+            self.assertEqual(gj.call_count, 1)
+
+    def test_cleanup_failure_does_not_mask_original_exception(self):
+        responses = {
+            ("drive", "files", "copy"): {"id": "pres1"},
+            ("slides", "presentations", "get"): RuntimeError("get failed"),
+            ("drive", "files", "update"): RuntimeError("trash also failed"),
+        }
+        with mock.patch.object(se, "_gws_json", side_effect=self._fake_gws_json(responses)):
+            with self.assertRaises(RuntimeError) as cm:
+                se.render_slide_png("file1", "/tmp/out.png")
+            # the ORIGINAL failure propagates, not the cleanup failure
+            self.assertIn("get failed", str(cm.exception))
+
+    def test_successful_render_returns_out_path_and_trashes_copy(self):
+        responses = {
+            ("drive", "files", "copy"): {"id": "pres1"},
+            ("slides", "presentations", "get"): {"slides": [{"objectId": "p1"}, {"objectId": "p2"}]},
+            ("slides", "presentations", "pages", "getThumbnail"): {"contentUrl": "https://example.invalid/x.png"},
+            ("drive", "files", "update"): {"id": "pres1"},
+        }
+        with mock.patch.object(se, "_gws_json", side_effect=self._fake_gws_json(responses)), \
+                mock.patch("urllib.request.urlretrieve"), \
+                mock.patch("os.path.getsize", return_value=5000):
+            out = se.render_slide_png("file1", "/tmp/out.png", slide_index=1)
+            self.assertEqual(out, "/tmp/out.png")
+
+
+if __name__ == "__main__":
+    unittest.main()

skills/aaif-create-chapter/SKILL.md

@@ -100,10 +100,15 @@ override (see Verify) and re-run.
    green dot sits on the correct city (the `Slides.pptx` line shows `+map dot` when
    it was moved). If the city is in **East Asia / Oceania** and the dot looks off,
    add a `PIXEL_OVERRIDES` entry in `create_chapter.py` (pixel coords on the
-   1123×794 `image18.png`) and re-run. To recalibrate against a render:
+   1123×794 `image18.png`) and re-run. To recalibrate against a render, export
+   slide 5 to PNG via the Slides API (**not LibreOffice** — `soffice` silently
+   substitutes local system fonts for the deck's brand fonts, so a LibreOffice
+   render can look wrong in ways the live file isn't):
    ```bash
-   # macOS: /Applications/LibreOffice.app/Contents/MacOS/soffice
-   soffice --headless --convert-to pdf --outdir . Slides.pptx   # check page 5
+   PYTHONPATH=lib python3 -c "
+   from aaif_events.slides_export import render_slide_png
+   render_slide_png('<Slides.pptx file id>', 'slide5.png', slide_index=4)
+   "
    ```
 
 ## How it works / maintenance

skills/aaif-create-chapter/scripts/create_chapter.py

@@ -111,6 +111,10 @@ def rebrand_part(part_name, data, name, upper, newslug):
         xml = _process_paragraphs(xml, "w:p", "w:t", tx)
     elif part_name == "xl/sharedStrings.xml":
         xml = _process_paragraphs(xml, "si", "t", tx)
+    elif re.match(r"xl/worksheets/sheet\d+\.xml$", part_name):
+        # Cells can hold inline strings (<is><t>...</t></is>) instead of a
+        # sharedStrings.xml reference — e.g. the CRM's "Guide" sheet title.
+        xml = _process_paragraphs(xml, "is", "t", tx)
     elif part_name in ("docProps/core.xml", "docProps/app.xml"):
         # metadata: chapter labelled "AAIF SF" -> "AAIF <UPPER>"
         xml = xml.replace("AAIF SF", "AAIF " + upper)
@@ -442,10 +446,13 @@ def clone_and_rebrand(folder_id, parent, name, ctx, indent=""):
     print("%s+ %s/" % (indent, name))
     for child in list_children(folder_id):
         cname, cid, mime = child["name"], child["id"], child["mimeType"]
+        # Names (not just file content) can carry the source city, e.g. "San
+        # Francisco CRM.xlsx" -> "New York CRM.xlsx" — same transform as content.
+        new_cname = transform_text(cname, ctx["name"], ctx["upper"], ctx["slug"])
         if mime == FOLDER:
-            clone_and_rebrand(cid, new_id, cname, ctx, indent + "  ")
+            clone_and_rebrand(cid, new_id, new_cname, ctx, indent + "  ")
         else:
-            copy_id = copy_file(cid, cname, new_id)
+            copy_id = copy_file(cid, new_cname, new_id)
             ext = os.path.splitext(cname)[1].lower()
             if ext in MIME_BY_EXT:
                 tmp = os.path.join(ctx["tmp"], "f" + copy_id + ext)
@@ -463,15 +470,15 @@ def clone_and_rebrand(folder_id, parent, name, ctx, indent=""):
                         gws_upload(copy_id, tmp, MIME_BY_EXT[ext])
                     left = residual_tokens(tmp)
                     if left:
-                        ctx["residuals"].append((cname, left))
+                        ctx["residuals"].append((new_cname, left))
                     flag = "  !! residual %s" % left if left else ""
                     dot = " +map dot" if moved else ""
-                    print("%s  - %s (%d parts%s)%s" % (indent, cname, n, dot, flag))
+                    print("%s  - %s (%d parts%s)%s" % (indent, new_cname, n, dot, flag))
                 finally:
                     if os.path.exists(tmp):
                         os.remove(tmp)
             else:
-                print("%s  - %s (copied)" % (indent, cname))
+                print("%s  - %s (copied)" % (indent, new_cname))
     return new_id
 
 def main():

skills/aaif-create-chapter/scripts/test_create_chapter.py

@@ -84,6 +84,37 @@ def offsets_by_shape(path):
     return out
 
 
+class TestTransformText(unittest.TestCase):
+    """transform_text() on filename-shaped strings — short, no surrounding
+    sentence, unlike the prose paragraphs it's normally exercised against via
+    _process_paragraphs. The bare-"SF" case heuristic looks ±30 chars around
+    the match for a capitalized neighbor word, which behaves differently on a
+    short filename than on a full paragraph."""
+
+    def test_full_city_name_in_filename(self):
+        self.assertEqual(
+            cc.transform_text("San Francisco CRM.xlsx", "New York", "NEW YORK", "newyork"),
+            "New York CRM.xlsx")
+
+    def test_bare_sf_abbreviation_in_filename_title_case(self):
+        # "SF" is the only capitalized neighbor in a short filename; the
+        # heuristic must still resolve to title case (matching how "AAIF SF
+        # Kickoff" reads in prose), not upper-case "NEW YORK".
+        self.assertEqual(
+            cc.transform_text("SF Kickoff Deck.pptx", "New York", "NEW YORK", "newyork"),
+            "New York Kickoff Deck.pptx")
+
+    def test_luma_slug_in_filename(self):
+        self.assertEqual(
+            cc.transform_text("aaif-sanfrancisco-banner.png", "New York", "NEW YORK", "newyork"),
+            "aaif-newyork-banner.png")
+
+    def test_filename_with_no_source_tokens_is_unchanged(self):
+        self.assertEqual(
+            cc.transform_text("About.docx", "New York", "NEW YORK", "newyork"),
+            "About.docx")
+
+
 class TestProjection(unittest.TestCase):
     def test_sf_selfcheck_matches_real_deck(self):
         # Ground truth: the real San Francisco chapter deck's dot sits at exactly
@@ -323,5 +354,44 @@ def test_missing_lat_lon_fields_is_none(self):
         self.assertIsNone(cc.geocode_city("Paris"))
 
 
+class TestRebrandWorksheetInlineStrings(unittest.TestCase):
+    """Regression test for the xl/worksheets/sheetN.xml branch of rebrand_part:
+    cells can hold an inline string (<is><t>...</t></is>) instead of a
+    sharedStrings.xml reference, e.g. the CRM's "Guide" sheet title — those
+    were previously left untouched by the rebrand engine."""
+
+    SHEET_XML = (
+        '<worksheet><sheetData><row r="2">'
+        '<c r="B2" t="inlineStr"><is><t>AAIF SF — Attendee CRM</t></is></c>'
+        '</row></sheetData></worksheet>'
+    )
+
+    def test_inline_string_cell_is_rebranded(self):
+        out = cc.rebrand_part("xl/worksheets/sheet2.xml", self.SHEET_XML.encode("utf-8"),
+                              "New York", "NEW YORK", "newyork")
+        text = out.decode("utf-8")
+        self.assertIn("AAIF New York — Attendee CRM", text)
+        self.assertNotIn(">AAIF SF", text)
+
+    def test_rels_lookalike_is_handled_by_the_rels_branch_not_worksheets(self):
+        # "sheet2.xml.rels" does NOT match the xl/worksheets/sheet\d+.xml$ regex
+        # (it ends in ".rels"), so it's routed to the existing `.rels` elif
+        # instead - which does its own (unrelated) Luma-slug substitution. This
+        # fixture has no slug for that branch to touch, so the bytes are
+        # unchanged, but via the .rels path, not because worksheets/*.rels is
+        # inert - rebrand_part has no truly-inert branch for anything under
+        # xl/worksheets/, so this only proves the two regexes don't collide.
+        out = cc.rebrand_part("xl/worksheets/_rels/sheet2.xml.rels",
+                              self.SHEET_XML.encode("utf-8"), "New York", "NEW YORK", "newyork")
+        self.assertEqual(out, self.SHEET_XML.encode("utf-8"))
+
+    def test_unrelated_part_type_is_left_untouched(self):
+        # A genuinely unhandled OOXML part (falls through every elif to the
+        # final `else: return data`) must come back byte-for-byte identical.
+        out = cc.rebrand_part("xl/drawings/drawing1.xml",
+                              self.SHEET_XML.encode("utf-8"), "New York", "NEW YORK", "newyork")
+        self.assertEqual(out, self.SHEET_XML.encode("utf-8"))
+
+
 if __name__ == "__main__":
     unittest.main()