Merge branch 'main' into fix/ai-author-diff-line

Author: shenxianpeng

main.py

@@ -502,6 +502,22 @@ def keep_ai_rules(scope: ScopeResult) -> ScopeResult:
     return skip_when_no_ai_rule_ran(scope)
 
 
+#: Rules about a commit rather than about a line of text: it has a body
+#: (CC011), a sign-off (CC012), an author that is no AI tool (CC017). A pull
+#: request title has neither body nor trailers nor an author of its own, so
+#: judged as a commit message it could never pass the first two, and the
+#: third would judge whatever git identity the runner has.
+COMMIT_ONLY_RULE_IDS = frozenset({"CC011", "CC012", "CC017"})
+
+
+def drop_commit_only_rules(scope: ScopeResult) -> ScopeResult:
+    """Narrow the PR title scope to the rules a one-line title can meet."""
+    scope.checks = [
+        c for c in scope.checks if c.get("rule_id") not in COMMIT_ONLY_RULE_IDS
+    ]
+    return scope
+
+
 def get_pr_title() -> str | None:
     """Read PR title from GitHub event payload."""
     if not is_pr_event():
@@ -972,7 +988,8 @@ def run_commit_check() -> tuple[int, list[ScopeResult]]:
     if PR_TITLE_ENABLED and is_pr_event():
         pr_title = get_pr_title()
         if pr_title:
-            results.append(check_scope("PR title", ["--message"], input_text=pr_title))
+            scope = check_scope("PR title", ["--message"], input_text=pr_title)
+            results.append(drop_commit_only_rules(scope))
 
     # ---- 2. PR description ------------------------------------------------
     if (PR_BODY_AI_ATTRIBUTION_ENABLED or ai_label_name()) and is_pr_event():
@@ -2041,7 +2058,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
@@ -2050,6 +2085,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.
@@ -2077,14 +2116,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)
 
 
@@ -2093,9 +2140,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

@@ -1004,6 +1004,33 @@ def test_pr_title_check_runs_when_enabled(self):
             "PR title", ["--message"], input_text="feat: a feature"
         )
 
+    def test_pr_title_is_not_held_to_commit_only_rules(self):
+        # require_body and require_signed_off_by judge a commit; a title has
+        # no body or trailers, so it could never pass them.
+        title = main.ScopeResult(
+            label="PR title",
+            checks=[
+                make_check("message", "pass", "CC001", value="feat: add login"),
+                make_check("require_body", "fail", "CC011", value="feat: add login"),
+                make_check("require_signed_off_by", "fail", "CC012"),
+                make_check("ai_author", "fail", "CC017", value="Claude <x@y.z>"),
+            ],
+        )
+        with (
+            patch("main.PR_TITLE_ENABLED", True),
+            patch("main.MESSAGE_ENABLED", False),
+            patch("main.BRANCH_ENABLED", False),
+            patch("main.AUTHOR_NAME_ENABLED", False),
+            patch("main.AUTHOR_EMAIL_ENABLED", False),
+            patch("main.is_pr_event", return_value=True),
+            patch("main.get_pr_title", return_value="feat: add login"),
+            patch("main.check_scope", return_value=title),
+            patch("main.run_other_checks", return_value=[]),
+        ):
+            rc, results = main.run_commit_check()
+        self.assertEqual(rc, 0)
+        self.assertEqual([c["rule_id"] for c in results[0].checks], ["CC001"])
+
     def test_pr_title_failure_propagates(self):
         with (
             patch("main.PR_TITLE_ENABLED", True),
@@ -2744,6 +2771,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."""
 
@@ -4261,6 +4377,18 @@ def test_failing_message(self):
             main.render_report([scope]),
         )
 
+    def test_a_title_meets_only_the_rules_a_title_can(self):
+        with open(self.config, "w", encoding="utf-8") as f:
+            f.write("[commit]\nrequire_body = true\nrequire_signed_off_by = true\n")
+        rc, data = self._run("feat: add login page")
+        failed = {c["rule_id"] for c in data["checks"] if c["status"] == "fail"}
+        self.assertEqual(rc, 1)
+        self.assertEqual(failed, {"CC011", "CC012"})
+        scope = main.drop_commit_only_rules(
+            main.ScopeResult(label="PR title", checks=data["checks"])
+        )
+        self.assertEqual(scope.status, "pass")
+
 
 class TestStderrIsNotPartOfTheJson(unittest.TestCase):
     """stdout is the JSON; stderr is what the CLI has to say to a person.