From d50af1e1361a5905ed9139c21b2889081355cb28 Mon Sep 17 00:00:00 2001 From: Andy Gonzales Date: Fri, 8 May 2026 09:07:55 -0400 Subject: [PATCH 1/7] fix(azure-devops): implement incremental review (-i) support Closes #2379. `pr-agent review -i` against an Azure DevOps PR crashed with `object of type 'NoneType' has no len()` because AzureDevopsProvider inherited the no-op `GitProvider.get_incremental_commits` stub, leaving `incremental.commits_range` as None. The hasattr guard in `_can_run_incremental_review` was satisfied by the inherited stub. Bug: - Add `commits_range is None` guard in `_can_run_incremental_review` so any provider without real incremental support degrades gracefully. - Defensive `previous_review.html_url` read in the no-new-files branch. Feature (AzureDevopsProvider): - Implement `get_incremental_commits`, `_get_incremental_commits`, `_get_commit_range`, `get_previous_review` mirroring GitHubProvider. - `_AzureCommitAdapter` exposes the GitHub-shape (.sha, .commit.author.date, .commit.message, .parents) so shared code in pr_reviewer/IncrementalPR needs no changes. - Reverse Azure's newest-first commit order to oldest-first to match GitHub iteration. - Normalize tz-aware UTC datetimes (Azure SDK) to naive UTC, matching PyGithub semantics and `pr_reviewer`'s naive `datetime.now()` compare. - Bridge Azure Comment to GitHub-shape attributes (`html_url` via existing `get_comment_url`, `created_at` from `published_date`). - Filter `get_diff_files` to `unreviewed_files_set` and rebuild patches against `last_seen_commit_sha` when incremental, so token savings actually materialize. - `get_files` override returns the unreviewed set when incremental. - Skip merge commits (multiple parents) when collecting changes. - Early-return in `get_diff_files` when `pr.last_merge_commit is None` instead of crashing on `head_sha.commit_id`. Tests: tests/unittest/test_azure_devops_incremental.py (9 cases) covers tz normalization, adapter shape, no-previous-review fallback, full incremental path, merge-commit skip, and the commits_range None guard. --- .../git_providers/azuredevops_provider.py | 164 ++++++++++++++++- pr_agent/tools/pr_reviewer.py | 10 +- .../unittest/test_azure_devops_incremental.py | 169 ++++++++++++++++++ 3 files changed, 339 insertions(+), 4 deletions(-) create mode 100644 tests/unittest/test_azure_devops_incremental.py diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index b9d2f3990c..4f12d3e016 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -1,5 +1,6 @@ from __future__ import annotations +import datetime as _dt import os from typing import Optional, Tuple from urllib.parse import urlparse @@ -8,12 +9,12 @@ from ..algo.file_filter import filter_ignored from ..algo.language_handler import is_valid_file -from ..algo.utils import (PRDescriptionHeader, clip_tokens, +from ..algo.utils import (PRDescriptionHeader, PRReviewHeader, clip_tokens, find_line_number_of_relevant_line_in_file, load_large_diff) from ..config_loader import get_settings from ..log import get_logger -from .git_provider import GitProvider +from .git_provider import GitProvider, IncrementalPR AZURE_DEVOPS_AVAILABLE = True ADO_APP_CLIENT_DEFAULT_ID = "499b84ac-1321-427f-aa17-267ca6975798/.default" @@ -32,6 +33,33 @@ AZURE_DEVOPS_AVAILABLE = False +def _to_naive_utc(dt): + if dt is None: + return None + if getattr(dt, "tzinfo", None) is not None: + return dt.astimezone(_dt.timezone.utc).replace(tzinfo=None) + return dt + + +class _AzureCommitInner: + def __init__(self, raw): + self.message = getattr(raw, "comment", "") or "" + author = getattr(raw, "author", None) + author_date = _to_naive_utc(getattr(author, "date", None)) if author else None + self.author = type("_AzureAuthor", (), {"date": author_date})() + + +class _AzureCommitAdapter: + """Mimics PyGithub `Commit` shape (.sha, .commit.author.date, .commit.message, .parents).""" + + def __init__(self, raw): + self._raw = raw + self.sha = raw.commit_id + self.commit_id = raw.commit_id + self.commit = _AzureCommitInner(raw) + self.parents = list(getattr(raw, "parents", None) or []) + + class AzureDevopsProvider(GitProvider): def __init__( @@ -51,6 +79,9 @@ def __init__( self.pr = None self.temp_comments = [] self.incremental = incremental + self.unreviewed_files_set = {} + self.pr_commits = None + self.previous_review = None if pr_url: self.set_pr(pr_url) @@ -158,6 +189,89 @@ def set_pr(self, pr_url: str): self.workspace_slug, self.repo_slug, self.pr_num = self._parse_pr_url(pr_url) self.pr = self._get_pr() + def get_incremental_commits(self, incremental=None): + if incremental is None: + incremental = IncrementalPR(False) + self.incremental = incremental + if self.incremental.is_incremental: + self.unreviewed_files_set = {} + self._get_incremental_commits() + + def _get_incremental_commits(self): + if not self.pr_commits: + raw = list(self.azure_devops_client.get_pull_request_commits( + project=self.workspace_slug, + repository_id=self.repo_slug, + pull_request_id=self.pr_num, + )) + # Azure returns newest-first; oldest-first matches GitHub iteration order. + raw.reverse() + self.pr_commits = [_AzureCommitAdapter(c) for c in raw] + + self.previous_review = self.get_previous_review(full=True, incremental=True) + if not self.previous_review: + get_logger().info("No previous review found, will review the entire PR") + self.incremental.is_incremental = False + return + + self.incremental.commits_range = self._get_commit_range() + for commit in self.incremental.commits_range: + if len(commit.parents) > 1: + get_logger().info(f"Skipping merge commit {commit.sha}") + continue + try: + changes_obj = self.azure_devops_client.get_changes( + project=self.workspace_slug, + repository_id=self.repo_slug, + commit_id=commit.commit_id, + ) + except Exception as e: + get_logger().warning(f"Failed to fetch changes for {commit.commit_id}: {e}") + continue + for change in (getattr(changes_obj, "changes", None) or []): + try: + item = change["item"] + except (KeyError, TypeError): + item = getattr(change, "additional_properties", {}).get("item", {}) or {} + if not isinstance(item, dict) or item.get("gitObjectType") == "tree": + continue + path = item.get("path") + if path: + self.unreviewed_files_set[path] = path + + def _get_commit_range(self): + last_review_time = _to_naive_utc(getattr(self.previous_review, "created_at", None)) + if last_review_time is None or not self.pr_commits: + return [] + first_new_commit_index = None + for index in range(len(self.pr_commits) - 1, -1, -1): + cdate = self.pr_commits[index].commit.author.date + if cdate is None: + continue + if cdate > last_review_time: + self.incremental.first_new_commit = self.pr_commits[index] + first_new_commit_index = index + else: + self.incremental.last_seen_commit = self.pr_commits[index] + break + return self.pr_commits[first_new_commit_index:] if first_new_commit_index is not None else [] + + def get_previous_review(self, *, full: bool, incremental: bool): + if not (full or incremental): + raise ValueError("At least one of full or incremental must be True") + prefixes = [] + if full: + prefixes.append(PRReviewHeader.REGULAR.value) + if incremental: + prefixes.append(PRReviewHeader.INCREMENTAL.value) + for comment in self.get_issue_comments(): + body = getattr(comment, "body", None) + if body and any(body.startswith(p) for p in prefixes): + comment.html_url = self.get_comment_url(comment) + comment.created_at = _to_naive_utc(getattr(comment, "published_date", None)) + return comment + return None + def get_repo_settings(self): try: contents = self.azure_devops_client.get_item_content( @@ -175,6 +289,13 @@ def get_repo_settings(self): return "" def get_files(self): + if (isinstance(getattr(self, "incremental", None), IncrementalPR) + and self.incremental.is_incremental + and self.unreviewed_files_set): + return list(self.unreviewed_files_set.keys()) + return self._get_files_full() + + def _get_files_full(self): files = [] for i in self.azure_devops_client.get_pull_request_commits( project=self.workspace_slug, @@ -197,6 +318,14 @@ def get_diff_files(self) -> list[FilePatchInfo]: if self.diff_files: return self.diff_files + if self.pr.last_merge_commit is None or self.pr.last_merge_target_commit is None: + get_logger().info( + f"PR {self.pr_num} has no last_merge_commit/last_merge_target_commit; " + f"cannot compute diff files." + ) + self.diff_files = [] + return [] + base_sha = self.pr.last_merge_target_commit head_sha = self.pr.last_merge_commit @@ -263,6 +392,15 @@ def get_diff_files(self) -> list[FilePatchInfo]: except Exception: pass + incremental_active = ( + isinstance(getattr(self, "incremental", None), IncrementalPR) + and self.incremental.is_incremental + and bool(self.unreviewed_files_set) + and self.incremental.last_seen_commit_sha + ) + if incremental_active: + diffs = [f for f in diffs if f in self.unreviewed_files_set] + invalid_files_names = [] for file in diffs: if not is_valid_file(file): @@ -321,9 +459,31 @@ def get_diff_files(self) -> list[FilePatchInfo]: get_logger().error(f"Failed to retrieve original file content of {file} at version {version}", error=error) original_file_content_str = "" + if incremental_active: + inc_version = GitVersionDescriptor( + version=self.incremental.last_seen_commit_sha, version_type="commit" + ) + try: + inc_original = self.azure_devops_client.get_item( + repository_id=self.repo_slug, + path=file, + project=self.workspace_slug, + version_descriptor=inc_version, + download=False, + include_content=True, + ) + original_file_content_str = inc_original.content or "" + except Exception as error: + get_logger().warning( + f"Failed to retrieve original of {file} at {self.incremental.last_seen_commit_sha}: {error}" + ) + original_file_content_str = "" + patch = load_large_diff( file, new_file_content_str, original_file_content_str, show_warning=False ).rstrip() + if incremental_active: + self.unreviewed_files_set[file] = patch # count number of lines added and removed patch_lines = patch.splitlines(keepends=True) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index c4917f3597..425b5f5323 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -142,8 +142,8 @@ async def run(self) -> None: if self.incremental.is_incremental and hasattr(self.git_provider, "unreviewed_files_set") and not self.git_provider.unreviewed_files_set: get_logger().info(f"Incremental review is enabled for {self.pr_url} but there are no new files") previous_review_url = "" - if hasattr(self.git_provider, "previous_review"): - previous_review_url = self.git_provider.previous_review.html_url + if hasattr(self.git_provider, "previous_review") and self.git_provider.previous_review is not None: + previous_review_url = getattr(self.git_provider.previous_review, "html_url", "") or "" if get_settings().config.publish_output: self.git_provider.publish_comment(f"Incremental Review Skipped\n" f"No files were changed since the [previous PR Review]({previous_review_url})") @@ -337,6 +337,12 @@ def _can_run_incremental_review(self) -> bool: if not hasattr(self.git_provider, "get_incremental_commits"): get_logger().info(f"Incremental review is not supported for {get_settings().config.git_provider}") return False + if self.incremental.commits_range is None: + get_logger().info( + f"Incremental review not initialized for {get_settings().config.git_provider}; " + f"falling back to full review." + ) + return False # checking if there are enough commits to start the review num_new_commits = len(self.incremental.commits_range) num_commits_threshold = get_settings().pr_reviewer.minimal_commits_for_incremental_review diff --git a/tests/unittest/test_azure_devops_incremental.py b/tests/unittest/test_azure_devops_incremental.py new file mode 100644 index 0000000000..e693fcb57a --- /dev/null +++ b/tests/unittest/test_azure_devops_incremental.py @@ -0,0 +1,169 @@ +import datetime as _dt +import unittest +from unittest.mock import MagicMock, patch + +from pr_agent.git_providers import AzureDevopsProvider +from pr_agent.git_providers.azuredevops_provider import ( + _AzureCommitAdapter, + _to_naive_utc, +) +from pr_agent.git_providers.git_provider import IncrementalPR + + +def _raw_commit(commit_id, comment, author_date, parents=None): + raw = MagicMock() + raw.commit_id = commit_id + raw.comment = comment + raw.author = MagicMock() + raw.author.date = author_date + raw.parents = parents or [] + return raw + + +def _comment(body, published_date): + c = MagicMock() + c.body = body + c.content = body + c.published_date = published_date + c.thread_id = 7 + return c + + +class TestNaiveUtc(unittest.TestCase): + def test_strips_tz_from_aware(self): + aware = _dt.datetime(2024, 1, 1, 12, 0, tzinfo=_dt.timezone.utc) + naive = _to_naive_utc(aware) + self.assertIsNone(naive.tzinfo) + self.assertEqual(naive, _dt.datetime(2024, 1, 1, 12, 0)) + + def test_passes_naive_through(self): + naive = _dt.datetime(2024, 1, 1, 12, 0) + self.assertEqual(_to_naive_utc(naive), naive) + + def test_none_returns_none(self): + self.assertIsNone(_to_naive_utc(None)) + + +class TestAzureCommitAdapter(unittest.TestCase): + def test_exposes_github_shape(self): + date = _dt.datetime(2024, 1, 1, 12, 0, tzinfo=_dt.timezone.utc) + raw = _raw_commit("abc123", "fix bug", date, parents=["p1"]) + adapter = _AzureCommitAdapter(raw) + self.assertEqual(adapter.sha, "abc123") + self.assertEqual(adapter.commit_id, "abc123") + self.assertEqual(adapter.commit.message, "fix bug") + self.assertIsNone(adapter.commit.author.date.tzinfo) + self.assertEqual(adapter.parents, ["p1"]) + + def test_handles_missing_author(self): + raw = MagicMock() + raw.commit_id = "x" + raw.comment = "" + raw.author = None + raw.parents = None + adapter = _AzureCommitAdapter(raw) + self.assertIsNone(adapter.commit.author.date) + self.assertEqual(adapter.parents, []) + + +class TestGetIncrementalCommits(unittest.TestCase): + def _make_provider(self): + with patch.object( + AzureDevopsProvider, "_get_azure_devops_client", + return_value=(MagicMock(), MagicMock()), + ): + provider = AzureDevopsProvider() + provider.workspace_slug = "ws" + provider.repo_slug = "repo" + provider.pr_num = 1 + provider.pr_url = "https://dev.azure.com/o/ws/_git/repo/pullrequest/1" + return provider + + def test_no_previous_review_disables_incremental(self): + provider = self._make_provider() + provider.azure_devops_client.get_pull_request_commits = MagicMock(return_value=[]) + provider.get_issue_comments = MagicMock(return_value=[]) + + inc = IncrementalPR(True) + provider.get_incremental_commits(inc) + + self.assertFalse(provider.incremental.is_incremental) + self.assertIsNone(provider.previous_review) + + def test_populates_commits_range_and_files(self): + provider = self._make_provider() + + review_time = _dt.datetime(2024, 6, 1, 10, 0, tzinfo=_dt.timezone.utc) + old = _raw_commit( + "old1", "first", _dt.datetime(2024, 5, 1, tzinfo=_dt.timezone.utc), parents=["p0"], + ) + new1 = _raw_commit( + "new1", "second", _dt.datetime(2024, 6, 2, tzinfo=_dt.timezone.utc), parents=["old1"], + ) + new2 = _raw_commit( + "new2", "third", _dt.datetime(2024, 6, 3, tzinfo=_dt.timezone.utc), parents=["new1"], + ) + # Azure returns newest-first. + provider.azure_devops_client.get_pull_request_commits = MagicMock( + return_value=[new2, new1, old] + ) + + prev = _comment("## PR Reviewer Guide\nbody", review_time) + provider.get_issue_comments = MagicMock(return_value=[prev]) + + changes_obj = MagicMock() + changes_obj.changes = [ + {"item": {"path": "/foo.py", "gitObjectType": "blob"}}, + {"item": {"path": "/bar.py", "gitObjectType": "blob"}}, + {"item": {"path": "/somedir", "gitObjectType": "tree"}}, + ] + provider.azure_devops_client.get_changes = MagicMock(return_value=changes_obj) + + inc = IncrementalPR(True) + provider.get_incremental_commits(inc) + + self.assertTrue(provider.incremental.is_incremental) + self.assertEqual(len(provider.incremental.commits_range), 2) + self.assertEqual(provider.incremental.first_new_commit.sha, "new1") + self.assertEqual(provider.incremental.last_seen_commit.sha, "old1") + self.assertEqual(provider.incremental.last_seen_commit_sha, "old1") + self.assertIn("/foo.py", provider.unreviewed_files_set) + self.assertIn("/bar.py", provider.unreviewed_files_set) + self.assertNotIn("/somedir", provider.unreviewed_files_set) + self.assertEqual(prev.html_url, provider.get_comment_url(prev)) + + def test_skips_merge_commits(self): + provider = self._make_provider() + review_time = _dt.datetime(2024, 6, 1, tzinfo=_dt.timezone.utc) + merge = _raw_commit( + "m1", "merge", _dt.datetime(2024, 6, 2, tzinfo=_dt.timezone.utc), + parents=["a", "b"], + ) + old = _raw_commit("o", "old", _dt.datetime(2024, 5, 1, tzinfo=_dt.timezone.utc)) + provider.azure_devops_client.get_pull_request_commits = MagicMock( + return_value=[merge, old] + ) + provider.get_issue_comments = MagicMock( + return_value=[_comment("## PR Reviewer Guide", review_time)] + ) + provider.azure_devops_client.get_changes = MagicMock() + + provider.get_incremental_commits(IncrementalPR(True)) + provider.azure_devops_client.get_changes.assert_not_called() + + +class TestPrReviewerGuard(unittest.TestCase): + def test_can_run_returns_false_when_commits_range_none(self): + from pr_agent.tools.pr_reviewer import PRReviewer + reviewer = PRReviewer.__new__(PRReviewer) + reviewer.is_auto = False + reviewer.pr_url = "u" + reviewer.incremental = IncrementalPR(True) + # incremental.commits_range stays None — provider has the method but didn't populate it. + reviewer.git_provider = MagicMock(spec=["get_incremental_commits"]) + # Should not raise NoneType len() error. + self.assertFalse(reviewer._can_run_incremental_review()) + + +if __name__ == "__main__": + unittest.main() From d2b3b96648c4f48098f9240642eb42357aa92e53 Mon Sep 17 00:00:00 2001 From: Andy Gonzales Date: Fri, 8 May 2026 11:19:48 -0400 Subject: [PATCH 2/7] fix(azure-devops): address Qodo review on incremental review - Guard against None additional_properties when parsing commit changes. - Apply filter_ignored/is_valid_file when building unreviewed_files_set so it matches get_diff_files() filtering. - When commits_range is None, disable incremental and proceed with a full review instead of returning early. --- pr_agent/git_providers/azuredevops_provider.py | 10 +++++++++- pr_agent/tools/pr_reviewer.py | 3 ++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index 4f12d3e016..17e9620e5d 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -215,6 +215,7 @@ def _get_incremental_commits(self): return self.incremental.commits_range = self._get_commit_range() + candidate_paths = [] for commit in self.incremental.commits_range: if len(commit.parents) > 1: get_logger().info(f"Skipping merge commit {commit.sha}") @@ -232,11 +233,18 @@ def _get_incremental_commits(self): try: item = change["item"] except (KeyError, TypeError): - item = getattr(change, "additional_properties", {}).get("item", {}) or {} + additional = getattr(change, "additional_properties", None) or {} + item = additional.get("item") or {} if not isinstance(item, dict) or item.get("gitObjectType") == "tree": continue path = item.get("path") if path: + candidate_paths.append(path) + + if candidate_paths: + filtered = filter_ignored(list(set(candidate_paths)), 'azure') + for path in filtered: + if is_valid_file(path): self.unreviewed_files_set[path] = path def _get_commit_range(self): diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 425b5f5323..a7bf90c234 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -342,7 +342,8 @@ def _can_run_incremental_review(self) -> bool: f"Incremental review not initialized for {get_settings().config.git_provider}; " f"falling back to full review." ) - return False + self.incremental.is_incremental = False + return True # checking if there are enough commits to start the review num_new_commits = len(self.incremental.commits_range) num_commits_threshold = get_settings().pr_reviewer.minimal_commits_for_incremental_review From 63b58a6bcddff8fbaa1aa7d4adffc72f935b4843 Mon Sep 17 00:00:00 2001 From: Andy Gonzales Date: Fri, 8 May 2026 14:11:22 -0400 Subject: [PATCH 3/7] fix(azure-devops): pick newest previous review, preserve dedup order - get_previous_review() now selects the most recent matching PR-Agent review by published_date instead of returning the first match. The prior behavior could feed a stale timestamp into the commit-range computation when multiple reviews existed. - Replace set(candidate_paths) with dict.fromkeys(...) so incremental file-list dedup is order-preserving and deterministic across runs. Co-Authored-By: Claude Opus 4.7 --- pr_agent/git_providers/azuredevops_provider.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index 17e9620e5d..db7b527392 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -242,7 +242,8 @@ def _get_incremental_commits(self): candidate_paths.append(path) if candidate_paths: - filtered = filter_ignored(list(set(candidate_paths)), 'azure') + deduped = list(dict.fromkeys(candidate_paths)) + filtered = filter_ignored(deduped, 'azure') for path in filtered: if is_valid_file(path): self.unreviewed_files_set[path] = path @@ -272,13 +273,17 @@ def get_previous_review(self, *, full: bool, incremental: bool): prefixes.append(PRReviewHeader.REGULAR.value) if incremental: prefixes.append(PRReviewHeader.INCREMENTAL.value) + matches = [] for comment in self.get_issue_comments(): body = getattr(comment, "body", None) if body and any(body.startswith(p) for p in prefixes): - comment.html_url = self.get_comment_url(comment) - comment.created_at = _to_naive_utc(getattr(comment, "published_date", None)) - return comment - return None + matches.append(comment) + if not matches: + return None + latest = max(matches, key=lambda c: _to_naive_utc(getattr(c, "published_date", None)) or _dt.datetime.min) + latest.html_url = self.get_comment_url(latest) + latest.created_at = _to_naive_utc(getattr(latest, "published_date", None)) + return latest def get_repo_settings(self): try: From 4041c38914c3630d3bd4c5ed39f7645a946d7e6d Mon Sep 17 00:00:00 2001 From: Andy Gonzales Date: Fri, 8 May 2026 14:47:55 -0400 Subject: [PATCH 4/7] fix(azure-devops): gate fallback returns False; treat fetch errors as fallback - _can_run_incremental_review() now returns False (not True) when incremental.commits_range is None, satisfying compliance ID 6 and the test_can_run_returns_false_when_commits_range_none unit test. - PRReviewer.run() distinguishes "gate disabled incremental" (fall through to full review) from "gate said skip" (return early), so the previous fallback log line now produces an actual full review. - _get_incremental_commits() tracks had_errors so that transient Azure get_changes failures disable incremental and fall back to a full review instead of being silently skipped as "no new files". Co-Authored-By: Claude Opus 4.7 --- pr_agent/git_providers/azuredevops_provider.py | 7 +++++++ pr_agent/tools/pr_reviewer.py | 9 ++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index db7b527392..db9fff78d7 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -216,6 +216,7 @@ def _get_incremental_commits(self): self.incremental.commits_range = self._get_commit_range() candidate_paths = [] + had_errors = False for commit in self.incremental.commits_range: if len(commit.parents) > 1: get_logger().info(f"Skipping merge commit {commit.sha}") @@ -227,6 +228,7 @@ def _get_incremental_commits(self): commit_id=commit.commit_id, ) except Exception as e: + had_errors = True get_logger().warning(f"Failed to fetch changes for {commit.commit_id}: {e}") continue for change in (getattr(changes_obj, "changes", None) or []): @@ -247,6 +249,11 @@ def _get_incremental_commits(self): for path in filtered: if is_valid_file(path): self.unreviewed_files_set[path] = path + elif had_errors and self.incremental.commits_range: + get_logger().warning( + "Failed to fetch changes for incremental commits; falling back to full review." + ) + self.incremental.is_incremental = False def _get_commit_range(self): last_review_time = _to_naive_utc(getattr(self.previous_review, "created_at", None)) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index a7bf90c234..a3d7eeee1e 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -123,8 +123,11 @@ async def run(self) -> None: get_logger().info(f"PR has no files: {self.pr_url}, skipping review") return None - if self.incremental.is_incremental and not self._can_run_incremental_review(): - return None + if self.incremental.is_incremental: + can_run = self._can_run_incremental_review() + # If the gate disabled incremental (e.g., commits_range is None), fall through to full review. + if not can_run and self.incremental.is_incremental: + return None # if isinstance(self.args, list) and self.args and self.args[0] == 'auto_approve': # get_logger().info(f'Auto approve flow PR: {self.pr_url} ...') @@ -343,7 +346,7 @@ def _can_run_incremental_review(self) -> bool: f"falling back to full review." ) self.incremental.is_incremental = False - return True + return False # checking if there are enough commits to start the review num_new_commits = len(self.incremental.commits_range) num_commits_threshold = get_settings().pr_reviewer.minimal_commits_for_incremental_review From 55cd982576f8a06f2d201b583065257a3c2b6aa1 Mon Sep 17 00:00:00 2001 From: Andy Gonzales Date: Fri, 8 May 2026 15:05:43 -0400 Subject: [PATCH 5/7] fix(azure-devops): rm redundant base fetch, cache empty diff, rename map, polish - get_diff_files: incremental path now substitutes for the base-SHA original fetch instead of fetching base then overwriting, saving one Azure get_item call per modified file in incremental runs. - get_diff_files: cache guard switched to `is not None` so the early-return on missing last_merge_commit/last_merge_target_commit is cached (`[]` is falsy and was being recomputed each call). - Rename `unreviewed_files_set` -> `unreviewed_files_map` across Azure, GitHub, Gitea providers, the reviewer, and the unit test. The attribute has always been a dict (path -> path/patch); the old name was misleading. - Wrap get_previous_review max() lambda for readability. - Normalize filter_ignored(..., "azure") to double-quoted literal. Co-Authored-By: Claude Opus 4.7 --- .../git_providers/azuredevops_provider.py | 63 ++++++++++--------- pr_agent/git_providers/gitea_provider.py | 6 +- pr_agent/git_providers/github_provider.py | 12 ++-- pr_agent/tools/pr_reviewer.py | 2 +- .../unittest/test_azure_devops_incremental.py | 6 +- 5 files changed, 47 insertions(+), 42 deletions(-) diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index db9fff78d7..447233b436 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -79,7 +79,7 @@ def __init__( self.pr = None self.temp_comments = [] self.incremental = incremental - self.unreviewed_files_set = {} + self.unreviewed_files_map = {} self.pr_commits = None self.previous_review = None if pr_url: @@ -194,7 +194,7 @@ def get_incremental_commits(self, incremental=None): incremental = IncrementalPR(False) self.incremental = incremental if self.incremental.is_incremental: - self.unreviewed_files_set = {} + self.unreviewed_files_map = {} self._get_incremental_commits() def _get_incremental_commits(self): @@ -245,10 +245,10 @@ def _get_incremental_commits(self): if candidate_paths: deduped = list(dict.fromkeys(candidate_paths)) - filtered = filter_ignored(deduped, 'azure') + filtered = filter_ignored(deduped, "azure") for path in filtered: if is_valid_file(path): - self.unreviewed_files_set[path] = path + self.unreviewed_files_map[path] = path elif had_errors and self.incremental.commits_range: get_logger().warning( "Failed to fetch changes for incremental commits; falling back to full review." @@ -287,7 +287,10 @@ def get_previous_review(self, *, full: bool, incremental: bool): matches.append(comment) if not matches: return None - latest = max(matches, key=lambda c: _to_naive_utc(getattr(c, "published_date", None)) or _dt.datetime.min) + latest = max( + matches, + key=lambda c: _to_naive_utc(getattr(c, "published_date", None)) or _dt.datetime.min, + ) latest.html_url = self.get_comment_url(latest) latest.created_at = _to_naive_utc(getattr(latest, "published_date", None)) return latest @@ -311,8 +314,8 @@ def get_repo_settings(self): def get_files(self): if (isinstance(getattr(self, "incremental", None), IncrementalPR) and self.incremental.is_incremental - and self.unreviewed_files_set): - return list(self.unreviewed_files_set.keys()) + and self.unreviewed_files_map): + return list(self.unreviewed_files_map.keys()) return self._get_files_full() def _get_files_full(self): @@ -335,7 +338,7 @@ def _get_files_full(self): def get_diff_files(self) -> list[FilePatchInfo]: try: - if self.diff_files: + if self.diff_files is not None: return self.diff_files if self.pr.last_merge_commit is None or self.pr.last_merge_target_commit is None: @@ -403,7 +406,7 @@ def get_diff_files(self) -> list[FilePatchInfo]: # diffs = list(set(diffs)) diffs_original = diffs - diffs = filter_ignored(diffs_original, 'azure') + diffs = filter_ignored(diffs_original, "azure") if diffs_original != diffs: try: get_logger().info(f"Filtered out [ignore] files for pull request:", extra= @@ -415,11 +418,11 @@ def get_diff_files(self) -> list[FilePatchInfo]: incremental_active = ( isinstance(getattr(self, "incremental", None), IncrementalPR) and self.incremental.is_incremental - and bool(self.unreviewed_files_set) + and bool(self.unreviewed_files_map) and self.incremental.last_seen_commit_sha ) if incremental_active: - diffs = [f for f in diffs if f in self.unreviewed_files_set] + diffs = [f for f in diffs if f in self.unreviewed_files_map] invalid_files_names = [] for file in diffs: @@ -459,43 +462,45 @@ def get_diff_files(self) -> list[FilePatchInfo]: elif "rename" in diff_types[file]: # diff_type can be `rename` | `edit, rename` edit_type = EDIT_TYPE.RENAMED - version = GitVersionDescriptor( - version=base_sha.commit_id, version_type="commit" - ) if edit_type == EDIT_TYPE.ADDED or edit_type == EDIT_TYPE.RENAMED: original_file_content_str = "" - else: + elif incremental_active: + inc_version = GitVersionDescriptor( + version=self.incremental.last_seen_commit_sha, version_type="commit" + ) try: - original_file_content_str = self.azure_devops_client.get_item( + inc_original = self.azure_devops_client.get_item( repository_id=self.repo_slug, path=file, project=self.workspace_slug, - version_descriptor=version, + version_descriptor=inc_version, download=False, include_content=True, ) - original_file_content_str = original_file_content_str.content + original_file_content_str = inc_original.content or "" except Exception as error: - get_logger().error(f"Failed to retrieve original file content of {file} at version {version}", error=error) + get_logger().warning( + f"Failed to retrieve original of {file} at {self.incremental.last_seen_commit_sha}: {error}" + ) original_file_content_str = "" - - if incremental_active: - inc_version = GitVersionDescriptor( - version=self.incremental.last_seen_commit_sha, version_type="commit" + else: + base_version = GitVersionDescriptor( + version=base_sha.commit_id, version_type="commit" ) try: - inc_original = self.azure_devops_client.get_item( + base_original = self.azure_devops_client.get_item( repository_id=self.repo_slug, path=file, project=self.workspace_slug, - version_descriptor=inc_version, + version_descriptor=base_version, download=False, include_content=True, ) - original_file_content_str = inc_original.content or "" + original_file_content_str = base_original.content except Exception as error: - get_logger().warning( - f"Failed to retrieve original of {file} at {self.incremental.last_seen_commit_sha}: {error}" + get_logger().error( + f"Failed to retrieve original file content of {file} at version {base_version}", + error=error, ) original_file_content_str = "" @@ -503,7 +508,7 @@ def get_diff_files(self) -> list[FilePatchInfo]: file, new_file_content_str, original_file_content_str, show_warning=False ).rstrip() if incremental_active: - self.unreviewed_files_set[file] = patch + self.unreviewed_files_map[file] = patch # count number of lines added and removed patch_lines = patch.splitlines(keepends=True) diff --git a/pr_agent/git_providers/gitea_provider.py b/pr_agent/git_providers/gitea_provider.py index 89a6248e9b..d3d7f20624 100644 --- a/pr_agent/git_providers/gitea_provider.py +++ b/pr_agent/git_providers/gitea_provider.py @@ -64,7 +64,7 @@ def __init__(self, url: Optional[str] = None): self.diff_files = [] self.incremental = IncrementalPR(False) self.comments_list = [] - self.unreviewed_files_set = dict() + self.unreviewed_files_map = dict() if "pulls" in url: self.pr_url = url @@ -475,9 +475,9 @@ def get_diff_files(self) -> List[FilePatchInfo]: # Get file content from this pr head_file = self.file_contents.get(filename,"") - if self.incremental.is_incremental and self.unreviewed_files_set: + if self.incremental.is_incremental and self.unreviewed_files_map: base_file = self._get_file_content_from_latest_commit(filename) - self.unreviewed_files_set[filename] = patch + self.unreviewed_files_map[filename] = patch else: if avoid_load: base_file = "" diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index fa52b7dc05..901f66b5d1 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -79,7 +79,7 @@ def _get_issue_handle(self, issue_url) -> Optional[Issue]: def get_incremental_commits(self, incremental=IncrementalPR(False)): self.incremental = incremental if self.incremental.is_incremental: - self.unreviewed_files_set = dict() + self.unreviewed_files_map = dict() self._get_incremental_commits() def is_supported(self, capability: str) -> bool: @@ -162,7 +162,7 @@ def _get_incremental_commits(self): if commit.commit.message.startswith(f"Merge branch '{self._get_repo().default_branch}'"): get_logger().info(f"Skipping merge commit {commit.commit.message}") continue - self.unreviewed_files_set.update({file.filename: file for file in commit.files}) + self.unreviewed_files_map.update({file.filename: file for file in commit.files}) else: get_logger().info("No previous review found, will review the entire PR") self.incremental.is_incremental = False @@ -194,8 +194,8 @@ def get_previous_review(self, *, full: bool, incremental: bool): return self.comments[index] def get_files(self): - if self.incremental.is_incremental and self.unreviewed_files_set: - return self.unreviewed_files_set.values() + if self.incremental.is_incremental and self.unreviewed_files_map: + return self.unreviewed_files_map.values() try: git_files = context.get("git_files", None) if git_files: @@ -296,10 +296,10 @@ def get_diff_files(self) -> list[FilePatchInfo]: else: new_file_content_str = self._get_pr_file_content(file, self.pr.head.sha) # communication with GitHub - if self.incremental.is_incremental and self.unreviewed_files_set: + if self.incremental.is_incremental and self.unreviewed_files_map: original_file_content_str = self._get_pr_file_content(file, self.incremental.last_seen_commit_sha) patch = load_large_diff(file.filename, new_file_content_str, original_file_content_str) - self.unreviewed_files_set[file.filename] = patch + self.unreviewed_files_map[file.filename] = patch else: if avoid_load: original_file_content_str = "" diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index a3d7eeee1e..67882a27d5 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -142,7 +142,7 @@ async def run(self) -> None: # ticket extraction if exists await extract_and_cache_pr_tickets(self.git_provider, self.vars) - if self.incremental.is_incremental and hasattr(self.git_provider, "unreviewed_files_set") and not self.git_provider.unreviewed_files_set: + if self.incremental.is_incremental and hasattr(self.git_provider, "unreviewed_files_map") and not self.git_provider.unreviewed_files_map: get_logger().info(f"Incremental review is enabled for {self.pr_url} but there are no new files") previous_review_url = "" if hasattr(self.git_provider, "previous_review") and self.git_provider.previous_review is not None: diff --git a/tests/unittest/test_azure_devops_incremental.py b/tests/unittest/test_azure_devops_incremental.py index e693fcb57a..79c220a2b8 100644 --- a/tests/unittest/test_azure_devops_incremental.py +++ b/tests/unittest/test_azure_devops_incremental.py @@ -127,9 +127,9 @@ def test_populates_commits_range_and_files(self): self.assertEqual(provider.incremental.first_new_commit.sha, "new1") self.assertEqual(provider.incremental.last_seen_commit.sha, "old1") self.assertEqual(provider.incremental.last_seen_commit_sha, "old1") - self.assertIn("/foo.py", provider.unreviewed_files_set) - self.assertIn("/bar.py", provider.unreviewed_files_set) - self.assertNotIn("/somedir", provider.unreviewed_files_set) + self.assertIn("/foo.py", provider.unreviewed_files_map) + self.assertIn("/bar.py", provider.unreviewed_files_map) + self.assertNotIn("/somedir", provider.unreviewed_files_map) self.assertEqual(prev.html_url, provider.get_comment_url(prev)) def test_skips_merge_commits(self): From 2690d2774d02550529f49a0e13e32ee64fe7f88b Mon Sep 17 00:00:00 2001 From: Andy Gonzales Date: Fri, 8 May 2026 15:20:36 -0400 Subject: [PATCH 6/7] fix(azure-devops): fall back to full review when commit dates are unreliable - _get_commit_range now returns None (not []) when the previous review has no timestamp, no PR commits exist, or every commit author date is None, and disables incremental so PRReviewer's existing `commits_range is None` fallback path runs a full review instead of silently exiting via the threshold check. - _get_incremental_commits short-circuits when _get_commit_range returns None, avoiding TypeError on the iteration that follows. - Wraps the over-120-char incremental-skip conditional in PRReviewer.run for ruff compliance. Co-Authored-By: Claude Opus 4.7 --- pr_agent/git_providers/azuredevops_provider.py | 18 +++++++++++++++++- pr_agent/tools/pr_reviewer.py | 6 +++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index 447233b436..228727137c 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -215,6 +215,8 @@ def _get_incremental_commits(self): return self.incremental.commits_range = self._get_commit_range() + if self.incremental.commits_range is None: + return candidate_paths = [] had_errors = False for commit in self.incremental.commits_range: @@ -258,18 +260,32 @@ def _get_incremental_commits(self): def _get_commit_range(self): last_review_time = _to_naive_utc(getattr(self.previous_review, "created_at", None)) if last_review_time is None or not self.pr_commits: - return [] + get_logger().info( + "Cannot compute incremental commit range " + "(missing previous review timestamp or PR commits); falling back to full review." + ) + self.incremental.is_incremental = False + return None first_new_commit_index = None + saw_reliable_date = False for index in range(len(self.pr_commits) - 1, -1, -1): cdate = self.pr_commits[index].commit.author.date if cdate is None: continue + saw_reliable_date = True if cdate > last_review_time: self.incremental.first_new_commit = self.pr_commits[index] first_new_commit_index = index else: self.incremental.last_seen_commit = self.pr_commits[index] break + if not saw_reliable_date: + get_logger().info( + "All PR commit author dates are missing; cannot compute incremental range. " + "Falling back to full review." + ) + self.incremental.is_incremental = False + return None return self.pr_commits[first_new_commit_index:] if first_new_commit_index is not None else [] def get_previous_review(self, *, full: bool, incremental: bool): diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 67882a27d5..2b1f358011 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -142,7 +142,11 @@ async def run(self) -> None: # ticket extraction if exists await extract_and_cache_pr_tickets(self.git_provider, self.vars) - if self.incremental.is_incremental and hasattr(self.git_provider, "unreviewed_files_map") and not self.git_provider.unreviewed_files_map: + if ( + self.incremental.is_incremental + and hasattr(self.git_provider, "unreviewed_files_map") + and not self.git_provider.unreviewed_files_map + ): get_logger().info(f"Incremental review is enabled for {self.pr_url} but there are no new files") previous_review_url = "" if hasattr(self.git_provider, "previous_review") and self.git_provider.previous_review is not None: From 5cddd81c0004bb14bc5f80f6b77b0113f93e6701 Mon Sep 17 00:00:00 2001 From: Andy Gonzales Date: Fri, 8 May 2026 15:50:26 -0400 Subject: [PATCH 7/7] pr comments --- .../git_providers/azuredevops_provider.py | 8 +- .../unittest/test_azure_devops_incremental.py | 83 ++++++++++++------- 2 files changed, 58 insertions(+), 33 deletions(-) diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index 228727137c..1d05ac5a3e 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -53,7 +53,6 @@ class _AzureCommitAdapter: """Mimics PyGithub `Commit` shape (.sha, .commit.author.date, .commit.message, .parents).""" def __init__(self, raw): - self._raw = raw self.sha = raw.commit_id self.commit_id = raw.commit_id self.commit = _AzureCommitInner(raw) @@ -219,10 +218,12 @@ def _get_incremental_commits(self): return candidate_paths = [] had_errors = False + non_merge_seen = False for commit in self.incremental.commits_range: if len(commit.parents) > 1: get_logger().info(f"Skipping merge commit {commit.sha}") continue + non_merge_seen = True try: changes_obj = self.azure_devops_client.get_changes( project=self.workspace_slug, @@ -256,6 +257,11 @@ def _get_incremental_commits(self): "Failed to fetch changes for incremental commits; falling back to full review." ) self.incremental.is_incremental = False + elif self.incremental.commits_range and not non_merge_seen: + get_logger().info( + "Incremental range only contains merge commits; falling back to full review." + ) + self.incremental.is_incremental = False def _get_commit_range(self): last_review_time = _to_naive_utc(getattr(self.previous_review, "created_at", None)) diff --git a/tests/unittest/test_azure_devops_incremental.py b/tests/unittest/test_azure_devops_incremental.py index 79c220a2b8..a58f2603a2 100644 --- a/tests/unittest/test_azure_devops_incremental.py +++ b/tests/unittest/test_azure_devops_incremental.py @@ -1,5 +1,4 @@ import datetime as _dt -import unittest from unittest.mock import MagicMock, patch from pr_agent.git_providers import AzureDevopsProvider @@ -29,31 +28,31 @@ def _comment(body, published_date): return c -class TestNaiveUtc(unittest.TestCase): +class TestNaiveUtc: def test_strips_tz_from_aware(self): aware = _dt.datetime(2024, 1, 1, 12, 0, tzinfo=_dt.timezone.utc) naive = _to_naive_utc(aware) - self.assertIsNone(naive.tzinfo) - self.assertEqual(naive, _dt.datetime(2024, 1, 1, 12, 0)) + assert naive.tzinfo is None + assert naive == _dt.datetime(2024, 1, 1, 12, 0) def test_passes_naive_through(self): naive = _dt.datetime(2024, 1, 1, 12, 0) - self.assertEqual(_to_naive_utc(naive), naive) + assert _to_naive_utc(naive) == naive def test_none_returns_none(self): - self.assertIsNone(_to_naive_utc(None)) + assert _to_naive_utc(None) is None -class TestAzureCommitAdapter(unittest.TestCase): +class TestAzureCommitAdapter: def test_exposes_github_shape(self): date = _dt.datetime(2024, 1, 1, 12, 0, tzinfo=_dt.timezone.utc) raw = _raw_commit("abc123", "fix bug", date, parents=["p1"]) adapter = _AzureCommitAdapter(raw) - self.assertEqual(adapter.sha, "abc123") - self.assertEqual(adapter.commit_id, "abc123") - self.assertEqual(adapter.commit.message, "fix bug") - self.assertIsNone(adapter.commit.author.date.tzinfo) - self.assertEqual(adapter.parents, ["p1"]) + assert adapter.sha == "abc123" + assert adapter.commit_id == "abc123" + assert adapter.commit.message == "fix bug" + assert adapter.commit.author.date.tzinfo is None + assert adapter.parents == ["p1"] def test_handles_missing_author(self): raw = MagicMock() @@ -62,11 +61,11 @@ def test_handles_missing_author(self): raw.author = None raw.parents = None adapter = _AzureCommitAdapter(raw) - self.assertIsNone(adapter.commit.author.date) - self.assertEqual(adapter.parents, []) + assert adapter.commit.author.date is None + assert adapter.parents == [] -class TestGetIncrementalCommits(unittest.TestCase): +class TestGetIncrementalCommits: def _make_provider(self): with patch.object( AzureDevopsProvider, "_get_azure_devops_client", @@ -87,8 +86,8 @@ def test_no_previous_review_disables_incremental(self): inc = IncrementalPR(True) provider.get_incremental_commits(inc) - self.assertFalse(provider.incremental.is_incremental) - self.assertIsNone(provider.previous_review) + assert provider.incremental.is_incremental is False + assert provider.previous_review is None def test_populates_commits_range_and_files(self): provider = self._make_provider() @@ -122,15 +121,15 @@ def test_populates_commits_range_and_files(self): inc = IncrementalPR(True) provider.get_incremental_commits(inc) - self.assertTrue(provider.incremental.is_incremental) - self.assertEqual(len(provider.incremental.commits_range), 2) - self.assertEqual(provider.incremental.first_new_commit.sha, "new1") - self.assertEqual(provider.incremental.last_seen_commit.sha, "old1") - self.assertEqual(provider.incremental.last_seen_commit_sha, "old1") - self.assertIn("/foo.py", provider.unreviewed_files_map) - self.assertIn("/bar.py", provider.unreviewed_files_map) - self.assertNotIn("/somedir", provider.unreviewed_files_map) - self.assertEqual(prev.html_url, provider.get_comment_url(prev)) + assert provider.incremental.is_incremental + assert len(provider.incremental.commits_range) == 2 + assert provider.incremental.first_new_commit.sha == "new1" + assert provider.incremental.last_seen_commit.sha == "old1" + assert provider.incremental.last_seen_commit_sha == "old1" + assert "/foo.py" in provider.unreviewed_files_map + assert "/bar.py" in provider.unreviewed_files_map + assert "/somedir" not in provider.unreviewed_files_map + assert prev.html_url == provider.get_comment_url(prev) def test_skips_merge_commits(self): provider = self._make_provider() @@ -151,8 +150,32 @@ def test_skips_merge_commits(self): provider.get_incremental_commits(IncrementalPR(True)) provider.azure_devops_client.get_changes.assert_not_called() + def test_all_merge_commits_falls_back_to_full(self): + provider = self._make_provider() + review_time = _dt.datetime(2024, 6, 1, tzinfo=_dt.timezone.utc) + merge1 = _raw_commit( + "m1", "merge1", _dt.datetime(2024, 6, 2, tzinfo=_dt.timezone.utc), + parents=["a", "b"], + ) + merge2 = _raw_commit( + "m2", "merge2", _dt.datetime(2024, 6, 3, tzinfo=_dt.timezone.utc), + parents=["c", "d"], + ) + provider.azure_devops_client.get_pull_request_commits = MagicMock( + return_value=[merge2, merge1] + ) + provider.get_issue_comments = MagicMock( + return_value=[_comment("## PR Reviewer Guide", review_time)] + ) + provider.azure_devops_client.get_changes = MagicMock() -class TestPrReviewerGuard(unittest.TestCase): + provider.get_incremental_commits(IncrementalPR(True)) + + assert provider.incremental.is_incremental is False + provider.azure_devops_client.get_changes.assert_not_called() + + +class TestPrReviewerGuard: def test_can_run_returns_false_when_commits_range_none(self): from pr_agent.tools.pr_reviewer import PRReviewer reviewer = PRReviewer.__new__(PRReviewer) @@ -162,8 +185,4 @@ def test_can_run_returns_false_when_commits_range_none(self): # incremental.commits_range stays None — provider has the method but didn't populate it. reviewer.git_provider = MagicMock(spec=["get_incremental_commits"]) # Should not raise NoneType len() error. - self.assertFalse(reviewer._can_run_incremental_review()) - - -if __name__ == "__main__": - unittest.main() + assert reviewer._can_run_incremental_review() is False