Make PR screenshots and videos fit laptop screens

.agents/skills/visual-acceptance/SKILL.md

@@ -8,7 +8,8 @@ description: Capture, inspect, and attach feature-specific iOS simulator screens
 Make the requested behavior quick for Danis to assess in the PR description.
 Use the existing macOS CI/XCUI evidence pipeline; read
 [docs/remote-ios-verification.md](../../../docs/remote-ios-verification.md)
-for retrieval, inspection, publication commands, and simulator limits.
+for retrieval, inspection, publication commands, and simulator limits, including
+the [compact PR media rules](../../../docs/remote-ios-verification.md#compact-pr-media).
 
 1. Translate the request into observable acceptance criteria. Extend the relevant
    `GrowingUpUI` journey with assertions and named screenshot checkpoints that
@@ -25,7 +26,7 @@ for retrieval, inspection, publication commands, and simulator limits.
    animation. Describe what you inspected, its outcome, and any device-only gaps
    in a temporary Markdown summary. Publish through `scripts/pr-visual-evidence.py`;
    it embeds native GitHub images/video in the PR body and preserves other sections.
-4. Read back the PR description and confirm the media renders. After a new push or
+4. Inspect the rendered PR attachments under the compact media rules. After a new push or
    CI rerun, replace evidence with reviewed media for the latest head/attempt before
    handoff. Media upload failures leave the description untouched; inspect a failed
    PR-update request before retrying. Remove

.github/pull_request_template.md

@@ -11,6 +11,8 @@
 
 <!-- For user-visible features/fixes, use .agents/skills/visual-acceptance/SKILL.md.
 Replace this section with inspected screenshots or a short demo from current-head CI.
+Every screenshot in any PR description must follow docs/remote-ios-verification.md#compact-pr-media.
+Publish reviewed CI evidence through scripts/pr-visual-evidence.py.
 For work without a visible app effect, explain why visual evidence does not apply. -->
 
 Pending visual acceptance. TestFlight deployment requires Danis's explicit approval.

.github/workflows/ci.yml

@@ -43,6 +43,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v7
+      - name: Install media tools
+        run: sudo apt-get update && sudo apt-get install -y ffmpeg
       - name: Check evidence scripts
         run: |
           bash -n scripts/run-ui-journeys.sh

AGENTS.md

@@ -5,7 +5,8 @@
 - Never place DerivedData, `.xcresult` bundles, downloaded CI artifacts, or other reproducible validation output inside the repository. Use Xcode's default DerivedData location or a `mktemp` directory, and remove temporary validation artifacts when the run is complete.
 - Reuse an existing compatible simulator for local builds and tests. Never create a local simulator unless Danis explicitly asks for one; if temporary simulator creation is explicitly approved, delete it as soon as the run completes.
 - On Linux, validate iOS changes through the PR's macOS CI and `scripts/pr-evidence.sh PR_NUMBER`. Follow `docs/remote-ios-verification.md`: inspect relevant exported screenshots and recording frames for the latest head, fix concrete failures on the same PR, and remove downloaded evidence after review. A green build alone does not verify UI behavior.
-- For user-visible features and UI fixes, use `.agents/skills/visual-acceptance/SKILL.md` before merging a PR or deploying to TestFlight. Attach inspected screenshots or a short feature demo to the PR description for the current head; choose the medium that proves the requested behavior. Deploy to TestFlight only after Danis explicitly approves deployment.
+- Every screenshot in any PR description must follow the [compact media rules](docs/remote-ios-verification.md#compact-pr-media) for sizing, layout and rendered inspection. Publish reviewed CI screenshots through `scripts/pr-visual-evidence.py`.
+- For user-visible features and UI fixes, use `.agents/skills/visual-acceptance/SKILL.md` before merging a PR or deploying to TestFlight. Attach inspected screenshots or a short feature demo to the PR description for the current head. Deploy to TestFlight only after Danis explicitly approves deployment.
 
 ## Code Style
 

docs/remote-ios-verification.md

@@ -106,12 +106,11 @@ on Linux (replace the example start and duration with the inspected segment):
 
 ```bash
 ffmpeg -ss 00:00:30 -i /tmp/EVIDENCE/ui-DEVICE-RUNTIME/journeys.mp4 -t 15 \
-  -vf 'scale=720:-2' -c:v libx264 -crf 28 -preset fast -an -movflags +faststart \
+  -c:v libx264 -crf 28 -preset fast -pix_fmt yuv420p -an -movflags +faststart \
   /tmp/EVIDENCE/ui-DEVICE-RUNTIME/feature-demo.mp4
 ```
 
-Inspect the resulting clip too. Keep each selected file below 10 MiB so it works
-with GitHub's free-plan upload limit. Use screenshots from this run's exported
+Inspect the resulting clip too. Use screenshots from this run's exported
 attachments, not checked-in comparison baselines. Write a temporary review summary
 that names the acceptance criteria demonstrated, the checkpoints/journey inspected,
 and any limits. Then publish, using real exported filenames:
@@ -124,18 +123,18 @@ python3 scripts/pr-visual-evidence.py PR_NUMBER /tmp/EVIDENCE/ui-DEVICE-RUNTIME
 ```
 
 Either `--image` or `--video` suffices; repeat the flags for additional relevant
-media. The helper checks the PR head, latest successful CI run and attempt, and
-artifact outcomes before uploading. It replaces only the marked visual acceptance
-section, preserving the rest of the PR description, and records the full commit,
-run link, simulator, and agent's review. Media is uploaded first through `gh api`
-using the same native attachment endpoint as `gh --attach`; the helper then reads
+media. Follow the [compact media rules](#compact-pr-media) for upload copies,
+layout, and rendered inspection. The helper checks the PR head, latest successful
+CI run and attempt, and artifact outcomes before uploading. It replaces only the
+marked visual acceptance section, preserving the rest of the PR description, and
+records the full commit, run link, simulator, and agent's review. Media is uploaded
+first through `gh api` using the same native attachment endpoint as `gh --attach`; the helper then reads
 the latest description before replacing its evidence section. Upload failures
 leave the description untouched. GitHub does not support conditional PR writes,
 so an edit arriving between the final read and update can still race; coordinate
-description edits during that final update. Read back the PR and check that
-screenshots display and videos play. Inspect a failed PR-update request before
-retrying. A concurrent push aborts publication or invalidates the helper's exact
-section after publication, preserving fresh surrounding text. Retrieve and review
+description edits during that final update. Inspect a failed PR-update request
+before retrying. A concurrent push aborts publication or invalidates the helper's
+exact section after publication, preserving fresh surrounding text. Retrieve and review
 evidence again before handoff. Upload errors are blockers,
 not a reason to substitute artifact download links for visible feature evidence.
 
@@ -152,14 +151,42 @@ References: [GitHub CLI attachments](https://docs.github.com/en/github-cli/githu
 The upload request follows the [GitHub CLI implementation](https://github.com/cli/cli/blob/v2.102.0/internal/attachments/client.go);
 conditional-write limits are documented in [GitHub REST best practices](https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api#use-conditional-requests).
 
+## Compact PR media
+
+PR attachments fit within **320×640 pixels**, preserve aspect ratio, and never
+upscale. The publisher requires local `ffmpeg` and its `ffprobe` tool for video timing;
+install them with `brew install ffmpeg`
+on macOS or `sudo apt install ffmpeg` on Ubuntu. It creates temporary upload copies
+and leaves the full-resolution originals intact for visual comparison. Sources may
+exceed 10 MiB; every copy must be nonempty, have a valid PNG or MP4 signature, and
+fit within 10 MiB. All copies are converted and checked before the first upload.
+Temporary copies are removed on success or failure.
+
+Videos retain the selected clip's complete duration and timing. Upload copies use
+H.264/yuv420p MP4 with fast start and even dimensions within the same bounds.
+Trimming selects the demonstrated interaction; preserve real timing for animation.
+
+Place two screenshot embeds in one Markdown paragraph, separated by a space, with
+a blank line between pairs. Choose descriptive checkpoint filenames; the publisher
+turns them into meaningful alt labels. Keep each native video URL on its own line
+so GitHub renders its player.
+
+Inspect the original checkpoints and selected recording before publication. Then
+inspect every rendered compact attachment in the PR at a **1280×800 viewport**:
+confirm screenshot sharpness, compact bounds, and two-image grouping, and play
+each native video to its end. Fix unreadable or poorly grouped evidence before
+handoff; a successful upload alone does not complete visual acceptance.
+
 ## Maintain the evidence tools
 
+Install `ffmpeg` (including `ffprobe`) before running the helper tests.
+
 ```bash
 bash -n scripts/run-ui-journeys.sh
 bash -n scripts/pr-evidence.sh
 PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s scripts/tests
 ```
 
-The focused tests exercise recorder finalization, failed-test status preservation,
-build failures and portable report generation. Real simulator behavior is verified
-by both macOS UI jobs.
+The focused tests exercise real media conversion and publication, recorder
+finalization, failed-test status preservation, build failures and portable report
+generation. Real simulator behavior is verified by both macOS UI jobs.

scripts/pr-visual-evidence.py

@@ -3,15 +3,20 @@
 
 import argparse
 import json
+import re
+import shutil
 import subprocess
 import tempfile
+from fractions import Fraction
 from pathlib import Path
 from urllib.parse import urlencode
 
 
 START = "<!-- visual-evidence:start -->"
 END = "<!-- visual-evidence:end -->"
 MAX_BYTES = 10 * 1024 * 1024
+MAX_WIDTH = 320
+MAX_HEIGHT = 640
 
 
 def gh(*arguments):
@@ -34,11 +39,91 @@ def media_file(directory, relative, suffix):
     path = (directory / relative).resolve()
     if not path.is_relative_to(directory) or path.suffix.lower() != suffix:
         raise ValueError(f"Expected a {suffix} file inside the evidence directory: {relative}")
-    if not path.is_file() or not 0 < path.stat().st_size <= MAX_BYTES:
-        raise ValueError(f"Media must be nonempty and at most 10 MiB: {relative}")
+    if not path.is_file() or path.stat().st_size == 0:
+        raise ValueError(f"Media must be a nonempty file: {relative}")
+    with path.open("rb") as stream:
+        header = stream.read(12)
+    if suffix == ".png" and not header.startswith(b"\x89PNG\r\n\x1a\n"):
+        raise ValueError("Screenshot is not a PNG file")
+    if suffix == ".mp4" and header[4:8] != b"ftyp":
+        raise ValueError("Recording is not an MP4 file")
     return path
 
 
+def compact_media(directory, images, videos, temporary):
+    sources = ([media_file(directory, name, ".png") for name in images]
+               + [media_file(directory, name, ".mp4") for name in videos])
+    ffmpeg = shutil.which("ffmpeg")
+    if not ffmpeg:
+        raise ValueError("ffmpeg is required to create compact PR evidence; install it with `brew install ffmpeg` or `sudo apt install ffmpeg`.")
+    ffprobe = shutil.which("ffprobe") if videos else None
+    if videos and not ffprobe:
+        raise ValueError("ffprobe is required for video timing; install the ffmpeg package with `brew install ffmpeg` or `sudo apt install ffmpeg`.")
+
+    compacted = []
+    for index, source in enumerate(sources):
+        output_directory = temporary / f"{index:02d}"
+        output_directory.mkdir()
+        output = output_directory / source.name
+        if source.suffix.lower() == ".png":
+            command = [ffmpeg, "-nostdin", "-hide_banner", "-loglevel", "error", "-y",
+                       "-i", str(source), "-frames:v", "1", "-vf",
+                       f"scale=w='min({MAX_WIDTH},iw)':h='min({MAX_HEIGHT},ih)':force_original_aspect_ratio=decrease",
+                       "-compression_level", "9", str(output)]
+        else:
+            # Passthrough preserves PTS; the final source packet fixes the encoder's trailing frame duration.
+            timing = json.loads(subprocess.check_output([
+                ffprobe, "-v", "error", "-select_streams", "v:0", "-show_entries",
+                "stream=time_base:packet=pts,duration:format=start_time",
+                "-show_packets", "-show_streams", "-show_format", "-of", "json", str(source),
+            ], text=True))
+            packets = timing.get("packets", [])
+            if not packets or not timing.get("streams"):
+                raise ValueError(f"Recording has no video timing: {source.name}")
+            # Packet order follows decoding, so select the last presented frame even with B-frames.
+            final_packet = max(packets, key=lambda packet: int(packet["pts"]))
+            time_base = Fraction(timing["streams"][0]["time_base"])
+            duration = time_base * int(final_packet.get("duration", 0))
+            if duration <= 0:
+                raise ValueError(f"Recording has no final frame duration: {source.name}")
+            final_pts = int(final_packet["pts"]) * time_base - Fraction(timing["format"]["start_time"])
+            packet_timing = (
+                "setts=pts=PTS:dts=DTS:duration="
+                f"'if(eq(PTS,round(({final_pts})/TB)),({duration})/TB,DURATION)'"
+            )
+            encoded_directory = output_directory / "encoded"
+            encoded_directory.mkdir()
+            encoded = encoded_directory / source.name
+            command = [ffmpeg, "-nostdin", "-hide_banner", "-loglevel", "error", "-y",
+                       "-i", str(source), "-map", "0:v:0", "-map", "0:a?", "-vf",
+                       f"scale=w='min({MAX_WIDTH},iw)':h='min({MAX_HEIGHT},ih)':force_original_aspect_ratio=decrease:force_divisible_by=2",
+                       "-c:v", "libx264", "-preset", "veryfast", "-crf", "28", "-pix_fmt", "yuv420p", "-bf", "0",
+                       "-c:a", "aac", "-b:a", "96k",
+                       "-fps_mode", "passthrough", "-enc_time_base", str(time_base), "-bsf:v", packet_timing,
+                       str(encoded)]
+        try:
+            subprocess.run(command, check=True, capture_output=True, text=True)
+            if source.suffix.lower() == ".mp4":
+                # Finalize movie metadata from encoded packet timing, preserving both streams without re-encoding.
+                subprocess.run([
+                    ffmpeg, "-nostdin", "-hide_banner", "-loglevel", "error", "-y", "-i", str(encoded),
+                    "-map", "0", "-c", "copy", "-bsf:v", packet_timing, "-movflags", "+faststart", str(output),
+                ], check=True, capture_output=True, text=True)
+        except subprocess.CalledProcessError as error:
+            detail = error.stderr.strip() or "unknown conversion error"
+            raise ValueError(f"Could not create compact evidence for {source.name}: {detail}") from error
+        media_file(output_directory, output.name, source.suffix.lower())
+        if output.stat().st_size > MAX_BYTES:
+            raise ValueError(f"Compact media must be at most 10 MiB: {source.name}")
+        compacted.append(output)
+    return compacted
+
+
+def image_label(path):
+    label = " ".join(word.capitalize() for word in re.findall(r"[A-Za-z0-9]+", path.stem))
+    return label or "Screenshot"
+
+
 def checked_run(repo, sha, metadata):
     workflow = metadata.get("workflow", "ci.yml")
     if workflow not in ("ci.yml", "app-store-screenshots.yml"):
@@ -89,9 +174,9 @@ def publish(number, directory, summary, images, videos):
         raise ValueError("Describe the behavior inspected and any limits in the summary file.")
     if START in summary or END in summary:
         raise ValueError("Summary must not contain visual evidence markers.")
-    media = [media_file(directory, name, ".png") for name in images]
-    media += [media_file(directory, name, ".mp4") for name in videos]
-    if not 1 <= len(media) <= 50 or len(set(media)) != len(media):
+    sources = [media_file(directory, name, ".png") for name in images]
+    sources += [media_file(directory, name, ".mp4") for name in videos]
+    if not 1 <= len(sources) <= 50 or len(set(sources)) != len(sources):
         raise ValueError("Select 1–50 distinct screenshots or short videos.")
     repository = json.loads(gh("api", "repos/{owner}/{repo}"))
     repo = repository["full_name"]
@@ -103,19 +188,23 @@ def publish(number, directory, summary, images, videos):
     checked_run(repo, sha, metadata)
     section(pr["body"] or "", "")  # Reject malformed existing markers before uploading.
 
-    # Same native attachment endpoint used by gh --attach, without editing the PR during uploads.
-    attachments = []
-    for path in media:
-        content_type = "video/mp4" if path.suffix.lower() == ".mp4" else "image/png"
-        query = urlencode({"repository_id": repository["id"], "name": path.name, "content_type": content_type})
-        asset = json.loads(gh(
-            "api", f"https://uploads.github.com/user-attachments/assets?{query}",
-            "--method", "POST", "--header", "Content-Type: application/octet-stream", "--input", str(path),
-        ))
-        url = asset.get("url", "")
-        if not url.startswith("https://github.com/user-attachments/assets/"):
-            raise ValueError("GitHub did not return a native attachment URL.")
-        attachments.append((path, url))
+    # Derive and validate every attachment before uploading; remove temporary copies on every exit.
+    with tempfile.TemporaryDirectory(prefix="growingup-compact-evidence-") as temporary_directory:
+        media = compact_media(directory, images, videos, Path(temporary_directory))
+
+        # Same native attachment endpoint used by gh --attach, without editing the PR during uploads.
+        attachments = []
+        for path in media:
+            content_type = "video/mp4" if path.suffix.lower() == ".mp4" else "image/png"
+            query = urlencode({"repository_id": repository["id"], "name": path.name, "content_type": content_type})
+            asset = json.loads(gh(
+                "api", f"https://uploads.github.com/user-attachments/assets?{query}",
+                "--method", "POST", "--header", "Content-Type: application/octet-stream", "--input", str(path),
+            ))
+            url = asset.get("url", "")
+            if not url.startswith("https://github.com/user-attachments/assets/"):
+                raise ValueError("GitHub did not return a native attachment URL.")
+            attachments.append((path, url))
 
     run = checked_run(repo, sha, metadata)
 
@@ -125,8 +214,13 @@ def publish(number, directory, summary, images, videos):
         f"{metadata['device']} / iOS {metadata['runtime']} / Xcode {metadata['xcode']}\n\n"
         f"{summary.strip()}\n\n"
     )
+    image_attachments = [(path, url) for path, url in attachments if path.suffix.lower() == ".png"]
+    for index in range(0, len(image_attachments), 2):
+        row = image_attachments[index:index + 2]
+        evidence += " ".join(f"![{image_label(path)}]({url})" for path, url in row) + "\n\n"
     for path, url in attachments:
-        evidence += url + "\n\n" if path.suffix.lower() == ".mp4" else f"![Simulator checkpoint]({url})\n\n"
+        if path.suffix.lower() == ".mp4":
+            evidence += url + "\n\n"
     evidence += "TestFlight deployment awaits Danis's explicit approval.\n" + END
     # GitHub does not support conditional PR writes. Re-read after the slow uploads, immediately before PATCH.
     current = json.loads(gh("api", endpoint))