speed up PR review workflow

.github/workflows/pr-review.yml

@@ -34,4 +34,4 @@ jobs:
                   GITHUB_TOKEN: ${{ github.token }}
                   GITHUB_REPOSITORY: ${{ github.repository }}
                   PR_NUMBER: ${{ github.event.pull_request.number }}
-              run: uv run python scripts/review_pr.py
+              run: uv run python scripts/review_pr.py --skip-pull-request-duplicates

scripts/review_pr.py

@@ -71,6 +71,11 @@ def parse_args() -> argparse.Namespace:
         description="Review one awesome-quant pull request."
     )
     parser.add_argument("--pr-number", type=int, default=None)
+    parser.add_argument(
+        "--skip-pull-request-duplicates",
+        action="store_true",
+        help="skip duplicate checks against open and recently closed PRs",
+    )
     return parser.parse_args()
 
 
@@ -252,7 +257,7 @@ class PinnedHTTPSConnection(http.client.HTTPSConnection):
         super().__init__(
             hostname,
             port=port,
-            timeout=10,
+            timeout=5,
             context=context,
         )
         self.ip_address = ip_address
@@ -382,9 +387,21 @@ def pull_request_has_duplicate(
     pull_request: Any,
     name: str,
     urls: list[str],
+    *,
+    readme_cache: dict[str, str] | None = None,
 ) -> bool:
-    base_readme = read_readme(repository, pull_request.base.sha)
+    cache = readme_cache if readme_cache is not None else {}
-    head_readme = read_readme(repository, pull_request.head.sha)
+
+    def cached_readme(ref: str) -> str:
+        if ref not in cache:
+            cache[ref] = read_readme(repository, ref)
+        return cache[ref]
+
+    head_readme = cached_readme(pull_request.head.sha)
+    if not readme_has_duplicate(head_readme, name, urls):
+        return False
+
+    base_readme = cached_readme(pull_request.base.sha)
     added_lines, _removed_lines = readme_changed_lines(base_readme, head_readme)
     return any(
         entry_line_has_duplicate(line, name, urls)
@@ -400,7 +417,9 @@ def repository_has_pull_request_duplicate(
     urls: list[str],
     *,
     now: datetime,
+    readme_cache: dict[str, str] | None = None,
 ) -> bool:
+    cache = readme_cache if readme_cache is not None else {}
     cutoff = now - timedelta(days=RECENT_CLOSED_PULL_DAYS)
     for state in ("open", "closed"):
         pulls = repository.get_pulls(
@@ -417,7 +436,13 @@ def repository_has_pull_request_duplicate(
                 closed_at = pull_request.closed_at
                 if closed_at is None or closed_at < cutoff:
                     continue
-            if pull_request_has_duplicate(repository, pull_request, name, urls):
+            if pull_request_has_duplicate(
+                repository,
+                pull_request,
+                name,
+                urls,
+                readme_cache=cache,
+            ):
                 return True
     return False
 
@@ -428,6 +453,7 @@ def review_pr(
     client: Github,
     *,
     now: datetime | None = None,
+    check_pull_request_duplicates: bool = True,
 ) -> tuple[list[Finding], str]:
     current_time = now or datetime.now(timezone.utc)
 
@@ -558,19 +584,23 @@ def review_pr(
                     )
                 )
 
-    if not url_reachable(url):
+    if not primary_github and not url_reachable(url):
         findings.append(Finding("reachability", f"primary URL is not reachable: {url}"))
 
     if readme_has_duplicate(base_readme, name, [url, *github_urls]):
         findings.append(
             Finding("duplicates", "project name or URL already exists in README.md")
         )
-    elif repository_has_pull_request_duplicate(
+    elif check_pull_request_duplicates and repository_has_pull_request_duplicate(
         repository,
         pr_number,
         name,
         [url, *github_urls],
         now=current_time,
+        readme_cache={
+            pull_request.base.sha: base_readme,
+            pull_request.head.sha: head_readme,
+        },
     ):
         findings.append(
             Finding(
@@ -606,15 +636,25 @@ def main() -> int:
         if not repository_name:
             return fail("GITHUB_REPOSITORY is required")
         client = Github(auth=Auth.Token(token))
-        findings, title = review_pr(repository_name, pr_number, client)
+        findings, title = review_pr(
+            repository_name,
+            pr_number,
+            client,
+            check_pull_request_duplicates=not args.skip_pull_request_duplicates,
+        )
     except Exception as exc:
         return fail(str(exc))
 
     print(f"PR #{pr_number}: {title}")
     print("Entries reviewed: 1")
+    if args.skip_pull_request_duplicates:
+        print("Cross-PR duplicate check: skipped")
     if not findings:
         for check in CHECK_ORDER:
-            print(f"- {check}: pass")
+            if check == "duplicates" and args.skip_pull_request_duplicates:
+                print("- duplicates: pass (existing README only)")
+            else:
+                print(f"- {check}: pass")
         print("Verdict: APPROVE")
         print("Recommended action: merge")
         return 0

tests/test_pr_review_workflow.py

@@ -40,7 +40,7 @@ jobs:
                   GITHUB_TOKEN: ${{ github.token }}
                   GITHUB_REPOSITORY: ${{ github.repository }}
                   PR_NUMBER: ${{ github.event.pull_request.number }}
-              run: uv run python scripts/review_pr.py
+              run: uv run python scripts/review_pr.py --skip-pull-request-duplicates
 """
 
 

tests/test_review_pr.py

@@ -10,6 +10,7 @@ from github import GithubException
 
 from scripts.review_pr import (
     Finding,
+    PinnedHTTPSConnection,
     main,
     readme_has_duplicate,
     review_pr,
@@ -373,6 +374,36 @@ class PullRequestDuplicateTests(unittest.TestCase):
 
         self.assertIn("duplicates", {finding.check for finding in findings})
 
+    def test_unrelated_candidates_require_only_one_readme_request_each(self):
+        unrelated_patch = """@@ -1,1 +1,2 @@
+ ## Trading & Backtesting
++- [Other](https://github.com/example/other) - `Python` - Other project.
+"""
+        candidates = [
+            FakePull(
+                number,
+                files=[SimpleNamespace(filename="README.md", patch=unrelated_patch)],
+            )
+            for number in range(100, 200)
+        ]
+
+        findings, _title, client = self.review(other_pulls=candidates)
+
+        self.assertEqual(findings, [])
+        self.assertEqual(len(client.repository.content_refs), 102)
+        candidate_base_refs = {f"base-{number}" for number in range(100, 200)}
+        self.assertTrue(
+            candidate_base_refs.isdisjoint(client.repository.content_refs)
+        )
+
+    def test_reuses_the_current_base_readme_when_confirming_a_duplicate(self):
+        duplicate = FakePull(9, base_sha="base-sha")
+
+        findings, _title, client = self.review(other_pulls=[duplicate])
+
+        self.assertIn("duplicates", {finding.check for finding in findings})
+        self.assertEqual(client.repository.content_refs.count("base-sha"), 1)
+
     def test_ignores_old_closed_pull_request(self):
         old_duplicate = FakePull(
             7,
@@ -449,6 +480,45 @@ class PullRequestDuplicateTests(unittest.TestCase):
 
         self.assertEqual(findings, [])
 
+    def test_can_skip_cross_pull_request_duplicate_scanning(self):
+        candidates = [
+            FakePull(number, content_error=True)
+            for number in range(100, 200)
+        ]
+        repository = FakeBaseRepository(FakePull(10), other_pulls=candidates)
+        client = FakeClient(repository)
+
+        with patch("scripts.review_pr.url_reachable", return_value=True):
+            findings, _title = review_pr(
+                "owner/list",
+                10,
+                client,
+                now=NOW,
+                check_pull_request_duplicates=False,
+            )
+
+        self.assertEqual(findings, [])
+        self.assertEqual(repository.content_refs, ["base-sha", "head-sha"])
+
+    def test_skip_still_rejects_a_duplicate_in_the_base_readme(self):
+        base_readme = (
+            "## Trading & Backtesting\n"
+            "- [Fresh](https://github.com/example/old) - `Python` - Existing project.\n"
+        )
+        repository = FakeBaseRepository(FakePull(10), base_readme=base_readme)
+        client = FakeClient(repository)
+
+        with patch("scripts.review_pr.url_reachable", return_value=True):
+            findings, _title = review_pr(
+                "owner/list",
+                10,
+                client,
+                now=NOW,
+                check_pull_request_duplicates=False,
+            )
+
+        self.assertIn("duplicates", {finding.check for finding in findings})
+
     def test_pull_request_search_errors_fail_closed(self):
         repository = FakeBaseRepository(FakePull(10))
         repository.pull_query_error = RuntimeError("pull search failed")
@@ -530,6 +600,41 @@ class ValidationPipelineTests(unittest.TestCase):
         self.assertIn("head-sha", repository.content_refs)
         self.assertNotIn("main", repository.content_refs)
 
+    def test_does_not_probe_a_github_primary_url_after_api_validation(self):
+        repository = FakeBaseRepository(FakePull(10))
+        client = FakeClient(repository)
+
+        with patch("scripts.review_pr.url_reachable") as url_checker:
+            findings, _title = review_pr("owner/list", 10, client, now=NOW)
+
+        self.assertEqual(findings, [])
+        url_checker.assert_not_called()
+
+    def test_still_probes_an_external_primary_url(self):
+        patch_text = """@@ -1,1 +1,2 @@
+ ## Trading & Backtesting
++- [Fresh](https://example.com/fresh) - `Python` - Fresh project. [GitHub](https://github.com/example/fresh)
+"""
+        base_readme = "# awesome-quant\n\n## Trading & Backtesting\n"
+        head_readme = base_readme + patch_text.splitlines()[-1][1:] + "\n"
+        repository = FakeBaseRepository(
+            FakePull(10, files=[SimpleNamespace(filename="README.md", patch=patch_text)]),
+            base_readme=base_readme,
+            head_readme=head_readme,
+        )
+        client = FakeClient(repository)
+
+        with patch("scripts.review_pr.url_reachable", return_value=True) as url_checker:
+            findings, _title = review_pr("owner/list", 10, client, now=NOW)
+
+        self.assertEqual(findings, [])
+        url_checker.assert_called_once_with("https://example.com/fresh")
+
+    def test_url_connections_use_a_short_timeout(self):
+        connection = PinnedHTTPSConnection("example.com", "93.184.216.34", 443)
+
+        self.assertEqual(connection.timeout, 5)
+
     def test_accepts_entry_when_heading_is_outside_patch_context(self):
         patch_text = """@@ -20,0 +21,1 @@
 +- [Fresh](https://github.com/example/fresh) - `Python` - Fresh project.
@@ -748,7 +853,15 @@ class ValidationPipelineTests(unittest.TestCase):
             self.review(configure_project=fail_readme_lookup)
 
     def test_rejects_unreachable_primary_url(self):
-        self.assertIn("reachability", self.review(reachable=False))
+        patch_text = """@@ -1,1 +1,2 @@
+ ## Trading & Backtesting
++- [Fresh](https://example.com/fresh) - `Python` - Fresh project. [GitHub](https://github.com/example/fresh)
+"""
+
+        self.assertIn(
+            "reachability",
+            self.review(patch_text=patch_text, reachable=False),
+        )
 
     def test_rejects_duplicate_in_base_readme(self):
         base_readme = (
@@ -789,6 +902,40 @@ class MainTests(unittest.TestCase):
         self.assertIn("description: pass", stdout)
         self.assertIn("duplicates: pass", stdout)
 
+    def test_skip_flag_is_reported_and_forwarded(self):
+        stdout = io.StringIO()
+        stderr = io.StringIO()
+        environment = {
+            "GITHUB_TOKEN": "token",
+            "GITHUB_REPOSITORY": "owner/list",
+            "PR_NUMBER": "10",
+        }
+        with (
+            patch.dict("os.environ", environment, clear=True),
+            patch(
+                "sys.argv",
+                ["review_pr.py", "--skip-pull-request-duplicates"],
+            ),
+            patch("scripts.review_pr.Github"),
+            patch(
+                "scripts.review_pr.review_pr",
+                return_value=([], "Add Fresh"),
+            ) as reviewer,
+            redirect_stdout(stdout),
+            redirect_stderr(stderr),
+        ):
+            result = main()
+
+        self.assertEqual(result, 0)
+        self.assertIn("Cross-PR duplicate check: skipped", stdout.getvalue())
+        self.assertIn(
+            "duplicates: pass (existing README only)",
+            stdout.getvalue(),
+        )
+        self.assertFalse(
+            reviewer.call_args.kwargs["check_pull_request_duplicates"]
+        )
+
     def test_authentication_does_not_emit_a_deprecation_warning(self):
         stdout = io.StringIO()
         stderr = io.StringIO()