add a size report to Pull Requests

punkle · punkle · commit bbbdaf09d52f · 2026-03-07T22:14:53.000Z
diff --git a/.github/workflows/main_matrix.yml b/.github/workflows/main_matrix.yml
@@ -238,47 +238,150 @@ jobs:
           description: "Download firmware-${{matrix.arch}}-${{ needs.version.outputs.long }}.zip. This artifact will be available for 90 days from creation"
           github-token: ${{ secrets.GITHUB_TOKEN }}
 
-  shame:
+  firmware-size-report:
     if: github.repository == 'meshtastic/firmware'
     continue-on-error: true
+    permissions:
+      contents: read
+      pull-requests: write
+      actions: read
     runs-on: ubuntu-latest
     needs: [build]
     steps:
       - uses: actions/checkout@v6
-        if: github.event_name == 'pull_request_target'
         with:
-          filter: blob:none # means we download all the git history but none of the commit (except ones with checkout like the head)
-          fetch-depth: 0
-      - name: Download the current manifests
+          ref: ${{github.event.pull_request.head.ref}}
+          repository: ${{github.event.pull_request.head.repo.full_name}}
+
+      - name: Download current manifests
         uses: actions/download-artifact@v8
         with:
-          path: ./manifests-new/
+          path: ./manifests/
           pattern: manifest-*
           merge-multiple: true
-      - name: Upload combined manifests for later commit and global stats crunching.
+
+      - name: Collect current firmware sizes
+        run: python3 bin/collect_sizes.py ./manifests/ ./current-sizes.json
+
+      - name: Upload size report artifact
         uses: actions/upload-artifact@v7
-        id: upload-manifest
         with:
-          name: manifests-${{ github.sha }}
+          name: firmware-sizes-${{ github.sha }}
           overwrite: true
-          path: manifests-new/*.mt.json
-      - name: Find the merge base
+          path: ./current-sizes.json
+          retention-days: 90
+
+      - name: Download baseline sizes from develop
         if: github.event_name == 'pull_request_target'
-        run: echo "MERGE_BASE=$(git merge-base "origin/$base" "$head")" >> $GITHUB_ENV
+        continue-on-error: true
+        id: baseline-develop
         env:
-          base: ${{ github.base_ref }}
-          head: ${{ github.sha }}
-      # Currently broken (for-loop through EVERY artifact -- rate limiting)
-      # - name: Download the old manifests
-      #   if: github.event_name == 'pull_request_target'
-      #   run: gh run download -R "$repo" --name "manifests-$merge_base" --dir manifest-old/
-      #   env:
-      #     GH_TOKEN: ${{ github.token }}
-      #     merge_base: ${{ env.MERGE_BASE }}
-      #     repo: ${{ github.repository }}
-      # - name: Do scan and post comment
-      #   if: github.event_name == 'pull_request_target'
-      #   run: python3 bin/shame.py ${{ github.event.pull_request.number }} manifests-old/ manifests-new/
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          # Find the latest successful CI run on develop and get its firmware-sizes artifact
+          RUN_ID=$(gh run list -R "${{ github.repository }}" \
+            --workflow CI --branch develop --status success \
+            --limit 1 --json databaseId --jq '.[0].databaseId')
+          if [ -n "$RUN_ID" ]; then
+            ARTIFACT_NAME=$(gh api "repos/${{ github.repository }}/actions/runs/${RUN_ID}/artifacts" \
+              --jq '.artifacts[] | select(.name | startswith("firmware-sizes-")) | .name' | head -1)
+            if [ -n "$ARTIFACT_NAME" ]; then
+              gh run download "$RUN_ID" -R "${{ github.repository }}" \
+                --name "$ARTIFACT_NAME" --dir ./baseline-develop/
+              cp ./baseline-develop/current-sizes.json ./develop-sizes.json
+              echo "found=true" >> "$GITHUB_OUTPUT"
+            else
+              echo "found=false" >> "$GITHUB_OUTPUT"
+            fi
+          else
+            echo "found=false" >> "$GITHUB_OUTPUT"
+          fi
+
+      - name: Download baseline sizes from master
+        if: github.event_name == 'pull_request_target'
+        continue-on-error: true
+        id: baseline-master
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          RUN_ID=$(gh run list -R "${{ github.repository }}" \
+            --workflow CI --branch master --status success \
+            --limit 1 --json databaseId --jq '.[0].databaseId')
+          if [ -n "$RUN_ID" ]; then
+            ARTIFACT_NAME=$(gh api "repos/${{ github.repository }}/actions/runs/${RUN_ID}/artifacts" \
+              --jq '.artifacts[] | select(.name | startswith("firmware-sizes-")) | .name' | head -1)
+            if [ -n "$ARTIFACT_NAME" ]; then
+              gh run download "$RUN_ID" -R "${{ github.repository }}" \
+                --name "$ARTIFACT_NAME" --dir ./baseline-master/
+              cp ./baseline-master/current-sizes.json ./master-sizes.json
+              echo "found=true" >> "$GITHUB_OUTPUT"
+            else
+              echo "found=false" >> "$GITHUB_OUTPUT"
+            fi
+          else
+            echo "found=false" >> "$GITHUB_OUTPUT"
+          fi
+
+      - name: Generate size comparison report
+        if: github.event_name == 'pull_request_target'
+        id: report
+        run: |
+          ARGS="./current-sizes.json"
+          if [ -f ./develop-sizes.json ]; then
+            ARGS="$ARGS --baseline develop:./develop-sizes.json"
+          fi
+          if [ -f ./master-sizes.json ]; then
+            ARGS="$ARGS --baseline master:./master-sizes.json"
+          fi
+          REPORT=$(python3 bin/size_report.py $ARGS)
+          if [ -z "$REPORT" ]; then
+            echo "has_report=false" >> "$GITHUB_OUTPUT"
+          else
+            echo "has_report=true" >> "$GITHUB_OUTPUT"
+            {
+              echo '<!-- firmware-size-report -->'
+              echo '# Firmware Size Report'
+              echo ''
+              echo "$REPORT"
+              echo ''
+              echo '---'
+              echo "*Updated for ${{ github.sha }}*"
+            } > ./size-report.md
+            cat ./size-report.md >> "$GITHUB_STEP_SUMMARY"
+          fi
+
+      - name: Post or update PR comment
+        if: github.event_name == 'pull_request_target' && steps.report.outputs.has_report == 'true'
+        uses: actions/github-script@v8
+        with:
+          script: |
+            const fs = require('fs');
+            const marker = '<!-- firmware-size-report -->';
+            const body = fs.readFileSync('./size-report.md', 'utf8');
+            const prNumber = context.payload.pull_request.number;
+
+            const { data: comments } = await github.rest.issues.listComments({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              issue_number: prNumber,
+            });
+
+            const existing = comments.find(c => c.body.includes(marker));
+            if (existing) {
+              await github.rest.issues.updateComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                comment_id: existing.id,
+                body,
+              });
+            } else {
+              await github.rest.issues.createComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: prNumber,
+                body,
+              });
+            }
 
   release-artifacts:
     runs-on: ubuntu-latest
diff --git a/bin/collect_sizes.py b/bin/collect_sizes.py
@@ -0,0 +1,53 @@
+#!/usr/bin/env python3
+
+"""Collect firmware binary sizes from manifest (.mt.json) files into a single report."""
+
+import json
+import os
+import sys
+
+
+def collect_sizes(manifest_dir):
+    """Scan manifest_dir for .mt.json files and return {board: size_bytes} dict."""
+    sizes = {}
+    for fname in sorted(os.listdir(manifest_dir)):
+        if not fname.endswith(".mt.json"):
+            continue
+        path = os.path.join(manifest_dir, fname)
+        with open(path) as f:
+            data = json.load(f)
+        board = data.get("platformioTarget", fname.replace(".mt.json", ""))
+        # Find the main firmware .bin size (largest .bin, excluding OTA/littlefs/bleota)
+        bin_size = None
+        for entry in data.get("files", []):
+            name = entry.get("name", "")
+            if name.startswith("firmware-") and name.endswith(".bin"):
+                bin_size = entry["bytes"]
+                break
+        # Fallback: any .bin that isn't ota/littlefs/bleota
+        if bin_size is None:
+            for entry in data.get("files", []):
+                name = entry.get("name", "")
+                if name.endswith(".bin") and not any(
+                    x in name for x in ["littlefs", "bleota", "ota"]
+                ):
+                    bin_size = entry["bytes"]
+                    break
+        if bin_size is not None:
+            sizes[board] = bin_size
+    return sizes
+
+
+if __name__ == "__main__":
+    if len(sys.argv) != 3:
+        print(f"Usage: {sys.argv[0]} <manifest_dir> <output.json>", file=sys.stderr)
+        sys.exit(1)
+
+    manifest_dir = sys.argv[1]
+    output_path = sys.argv[2]
+
+    sizes = collect_sizes(manifest_dir)
+    with open(output_path, "w") as f:
+        json.dump(sizes, f, indent=2, sort_keys=True)
+
+    print(f"Collected sizes for {len(sizes)} targets -> {output_path}")
diff --git a/bin/size_report.py b/bin/size_report.py
@@ -0,0 +1,167 @@
+#!/usr/bin/env python3
+
+"""Compare firmware size reports and generate a markdown summary.
+
+Usage:
+    size_report.py <new_sizes.json> [--baseline <label>:<old_sizes.json>]...
+
+Examples:
+    # Compare PR against develop and master baselines
+    size_report.py pr.json --baseline develop:develop.json --baseline master:master.json
+
+    # Single baseline comparison
+    size_report.py pr.json --baseline develop:develop.json
+
+    # No baselines — shows sizes with blank delta columns
+    size_report.py pr.json
+"""
+
+import argparse
+import json
+import sys
+
+
+def load_sizes(path):
+    with open(path) as f:
+        return json.load(f)
+
+
+def format_delta(n):
+    """Format byte delta with sign and human-friendly suffix."""
+    sign = "+" if n > 0 else ""
+    if abs(n) >= 1024:
+        return f"{sign}{n:,} ({sign}{n / 1024:.1f} KB)"
+    return f"{sign}{n:,}"
+
+
+def generate_markdown(new_sizes, baselines, top_n=5):
+    """Generate a single table with current size and delta columns per baseline.
+
+    baselines: list of (label, old_sizes_dict), may be empty
+    """
+    labels = [label for label, _ in baselines]
+
+    # Build rows: (board, current_size, [(delta, abs_delta) per baseline])
+    rows = []
+    for board in sorted(new_sizes):
+        current = new_sizes[board]
+        deltas = []
+        for _, old_sizes in baselines:
+            old = old_sizes.get(board)
+            if old is not None:
+                d = current - old
+                deltas.append((d, abs(d)))
+            else:
+                deltas.append((None, 0))
+        # Sort key: max abs delta across baselines (biggest changes first)
+        max_abs = max((ad for _, ad in deltas), default=0)
+        rows.append((board, current, deltas, max_abs))
+
+    rows.sort(key=lambda r: r[3], reverse=True)
+
+    # Summary line
+    sections = []
+    summary_parts = [f"{len(new_sizes)} targets"]
+    for i, (label, old_sizes) in enumerate(baselines):
+        increases = sum(
+            1 for _, _, deltas, _ in rows if deltas[i][0] is not None and deltas[i][0] > 0
+        )
+        decreases = sum(
+            1 for _, _, deltas, _ in rows if deltas[i][0] is not None and deltas[i][0] < 0
+        )
+        net = sum(
+            deltas[i][0] for _, _, deltas, _ in rows if deltas[i][0] is not None
+        )
+        parts = []
+        if increases:
+            parts.append(f"{increases} increased")
+        if decreases:
+            parts.append(f"{decreases} decreased")
+        if parts:
+            parts.append(f"net {format_delta(net)}")
+            summary_parts.append(f"vs `{label}`: {', '.join(parts)}")
+        else:
+            summary_parts.append(f"vs `{label}`: no changes")
+
+    if not baselines:
+        summary_parts.append("no baseline available yet")
+
+    sections.append(f"**{' | '.join(summary_parts)}**\n")
+
+    # Table header
+    header = "| Target | Size |"
+    separator = "|--------|-----:|"
+    for label in labels:
+        header += f" vs `{label}` |"
+        separator += "----------:|"
+    sections.append(header)
+    sections.append(separator)
+
+    def format_row(board, current, deltas):
+        row = f"| `{board}` | {current:,} |"
+        for d, _ in deltas:
+            if d is None:
+                row += " |"
+            elif d == 0:
+                row += " 0 |"
+            else:
+                icon = "📈" if d > 0 else "📉"
+                row += f" {icon} {format_delta(d)} |"
+        return row
+
+    # Top N rows always visible
+    top = rows[:top_n]
+    for board, current, deltas, _ in top:
+        sections.append(format_row(board, current, deltas))
+
+    # Remaining rows in expandable section
+    rest = rows[top_n:]
+    if rest:
+        sections.append("")
+        sections.append(
+            f"<details><summary>Show {len(rest)} more target(s)</summary>\n"
+        )
+        sections.append(header)
+        sections.append(separator)
+        for board, current, deltas, _ in rest:
+            sections.append(format_row(board, current, deltas))
+        sections.append("\n</details>")
+
+    sections.append("")
+    return "\n".join(sections)
+
+
+def main():
+    parser = argparse.ArgumentParser(description="Compare firmware size reports")
+    parser.add_argument("new_sizes", help="Path to new sizes JSON")
+    parser.add_argument(
+        "--baseline",
+        action="append",
+        default=[],
+        metavar="LABEL:PATH",
+        help="Baseline to compare against (e.g. develop:develop.json)",
+    )
+    parser.add_argument(
+        "--top",
+        type=int,
+        default=5,
+        help="Number of top changes to show before collapsing (default: 5)",
+    )
+    args = parser.parse_args()
+
+    new_sizes = load_sizes(args.new_sizes)
+
+    baselines = []
+    for b in args.baseline:
+        if ":" not in b:
+            print(f"Error: baseline must be LABEL:PATH, got '{b}'", file=sys.stderr)
+            sys.exit(1)
+        label, path = b.split(":", 1)
+        baselines.append((label, load_sizes(path)))
+
+    md = generate_markdown(new_sizes, baselines, top_n=args.top)
+    print(md)
+
+
+if __name__ == "__main__":
+    main()
