fix: keep the PR comment under GitHub's size limit (#338)

Author: shenxianpeng

main.py

@@ -2044,7 +2044,25 @@ def _report_footer() -> str:
     )
 
 
-def render_report(results: list[ScopeResult]) -> str:
+def _run_url() -> str:
+    """Link to this workflow run, whose page holds the job summary."""
+    repository = os.getenv("GITHUB_REPOSITORY", "")
+    run_id = os.getenv("GITHUB_RUN_ID", "")
+    if not (repository and run_id):
+        return ""
+    server = os.getenv("GITHUB_SERVER_URL") or "https://github.com"
+    return f"{server.rstrip('/')}/{repository}/actions/runs/{run_id}"
+
+
+def _elsewhere(what: str) -> str:
+    """Point to where the parts a shortened comment left out can be read."""
+    where = "job summary" if _summary_written() else "step log"
+    url = _run_url()
+    place = f"[{where}]({url})" if url else where
+    return f"{what} are in the {place} of this run."
+
+
+def render_report(results: list[ScopeResult], detail: str = "full") -> str:
     """Render the Markdown report shared by the job summary and PR comment.
 
     Opens with the hidden marker and a heading that is the verdict — the
@@ -2053,6 +2071,10 @@ def render_report(results: list[ScopeResult]) -> str:
     listed any rule under ``warn``, the folded "How to fix", and the folded
     listing of every scope.
 
+    ``detail`` is for a PR comment that would not fit (``render_pr_comment``):
+    ``"short"`` leaves out the listing, ``"headline"`` everything below the
+    group lines, and either says where to read what it left out.
+
     A warned scope counts toward "passed", never toward "failed": it ran,
     found something, and is reported in full, but the config asked for it to
     be surfaced rather than enforced.
@@ -2080,14 +2102,22 @@ def render_report(results: list[ScopeResult]) -> str:
         lines.extend([_group_lines(results), ""])
     if skipped and _skip_reasons(results):
         lines.extend([_skip_reasons(results), ""])
+    if detail == "headline":
+        what = "The findings and how to fix them" if failed or warned else "All checks"
+        lines.extend([_elsewhere(what), "", _report_footer()])
+        return "\n".join(lines)
     if failed:
         lines.extend([*_diff_block(results), ""])
     if warned:
         lines.extend(["**Warnings**", "", *_diff_block(results, "warn"), ""])
     if failed:
         how = _how_to_fix(results)
         lines.extend([*how, ""] if how else [])
-    lines.extend([*_all_checks(results), "", _report_footer()])
+    if detail == "short":
+        lines.extend([_elsewhere(f"All {_plural(total, 'check')}"), ""])
+    else:
+        lines.extend([*_all_checks(results), ""])
+    lines.append(_report_footer())
     return "\n".join(lines)
 
 
@@ -2096,9 +2126,31 @@ def render_job_summary(results: list[ScopeResult]) -> str:
     return render_report(results)
 
 
+#: The longest comment GitHub accepts, in UTF-16 code units.
+COMMENT_LIMIT = 65536
+
+
+def _comment_length(body: str) -> int:
+    """Length of a comment as GitHub counts it: in UTF-16 code units."""
+    return len(body.encode("utf-16-le")) // 2
+
+
 def render_pr_comment(results: list[ScopeResult]) -> str:
-    """Create the Markdown body for the PR comment (same report as summary)."""
-    return render_report(results)
+    """Create the Markdown body for the PR comment (same report as summary).
+
+    The report grows with the pull request: every failing commit's message
+    is quoted in the diff, the prompt and the listing. A few dozen failing
+    commits ran past GitHub's limit, the API refused the comment, and an
+    earlier one, perhaps saying "passed", stayed on the pull request. So a
+    report that does not fit drops the listing, then everything under the
+    group lines, and points to the run for the rest. The job summary, with
+    its far higher limit, keeps the full report.
+    """
+    for detail in ("full", "short"):
+        body = render_report(results, detail)
+        if _comment_length(body) <= COMMENT_LIMIT:
+            return body
+    return render_report(results, "headline")
 
 
 # ---------------------------------------------------------------------------

main_test.py

@@ -2732,6 +2732,95 @@ def test_failure_matches_job_summary(self):
         self.assertIn("```diff\n@@ Commit 1/1 @@\n", comment)
 
 
+class TestPrCommentSizeLimit(unittest.TestCase):
+    """A report too long for a comment gives up its longest parts first."""
+
+    RUN_URL = "https://github.com/o/r/actions/runs/42"
+
+    def setUp(self):
+        self.RUN = {
+            "GITHUB_REPOSITORY": "o/r",
+            "GITHUB_RUN_ID": "42",
+            "GITHUB_SERVER_URL": "https://github.com",
+        }
+
+    def _failing_commits(self, count: int) -> list[main.ScopeResult]:
+        body = "\n".join(
+            f"Line {i} of a body that explains the change." for i in range(8)
+        )
+        return [
+            main.ScopeResult(
+                label=f"Commit {i}/{count}",
+                sha=f"{i:040x}",
+                checks=[
+                    make_check(
+                        "require_signed_off_by",
+                        "fail",
+                        "CC012",
+                        value=f"Update module {i}\n\n{body}",
+                        error="Signed-off-by trailer not found in the commit message",
+                        fix=f"Update module {i}\n\n{body}\n\nSigned-off-by: A <a@b.c>",
+                    )
+                ],
+            )
+            for i in range(1, count + 1)
+        ]
+
+    def test_a_report_that_fits_is_the_job_summary(self):
+        results = self._failing_commits(3)
+        self.assertEqual(
+            main.render_pr_comment(results), main.render_job_summary(results)
+        )
+
+    def test_dozens_of_failing_commits_still_fit(self):
+        results = self._failing_commits(60)
+        self.assertGreater(len(main.render_job_summary(results)), main.COMMENT_LIMIT)
+        with (
+            patch.dict(os.environ, self.RUN),
+            patch("main.JOB_SUMMARY_ENABLED", True),
+            patch("main.GITHUB_STEP_SUMMARY", "/tmp/summary.md"),
+        ):
+            comment = main.render_pr_comment(results)
+        self.assertLessEqual(main._comment_length(comment), main.COMMENT_LIMIT)
+        self.assertTrue(comment.startswith(main.COMMENT_MARKER))
+        self.assertIn("Commit Check failed", comment)
+        self.assertNotIn("<summary>All 60 checks</summary>", comment)
+        self.assertIn(
+            f"All 60 checks are in the [job summary]({self.RUN_URL}) of this run.",
+            comment,
+        )
+
+    def test_the_diff_goes_too_when_the_listing_was_not_enough(self):
+        results = self._failing_commits(2)
+        with (
+            patch.dict(os.environ, self.RUN),
+            patch("main.JOB_SUMMARY_ENABLED", False),
+        ):
+            short = main.render_report(results, "short")
+            with patch("main.COMMENT_LIMIT", len(short) - 1):
+                comment = main.render_pr_comment(results)
+        self.assertNotIn("```diff", comment)
+        self.assertNotIn("How to fix</b>", comment)
+        self.assertIn(
+            "The findings and how to fix them are in the "
+            f"[step log]({self.RUN_URL}) of this run.",
+            comment,
+        )
+        self.assertIn("Commits: 2 of 2 failed", comment)
+
+    def test_a_passing_headline_names_no_findings(self):
+        env = {k: v for k, v in os.environ.items() if k != "GITHUB_RUN_ID"}
+        with (
+            patch.dict(os.environ, env, clear=True),
+            patch("main.JOB_SUMMARY_ENABLED", False),
+        ):
+            body = main.render_report([pass_scope("Branch")], "headline")
+        self.assertIn("All checks are in the step log of this run.", body)
+
+    def test_length_is_counted_as_github_counts_it(self):
+        self.assertEqual(main._comment_length("a🤖"), 3)
+
+
 class TestUnparsableScopeInTheReport(unittest.TestCase):
     """A CLI response that is not JSON is a failure with no rule to link."""