fix(github): use committer date for commit range to handle rebases

[IsmaelMartinez]

Fixes #2844.

get_commit_range compared author dates against the previous review timestamp. Git preserves the author date across a rebase, so after a force-push the commit range came back empty and the reviewer reported no changes when the code had changed.

Now prefers the committer date, which advances on rebase, falling back to the author date when a committer is absent. This is what _GitLabIncrementalCommit already does on the GitLab side.

Four regression tests: rebased commit detected as new, pre-review commit still excluded, author-date fallback, and mixed-commit scoping. The first fails on main and passes with the fix.

[qodo-code-review]

PR Summary by Qodo

Fix GitHub incremental reviews after rebases

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Anchor GitHub incremental ranges on committer dates so rebased commits remain reviewable.
• Fall back to author dates when GitHub omits committer metadata.
• Cover rebases, stale commits, fallback behavior, and mixed ranges with regression tests.

Diagram

graph TD
  A["Previous review"] --> D{"Commit newer?"}
  B["PR commits"] --> C["Timeline date"] --> D
  D -->|Yes| E["Commit range"] --> F["Changed files"]
  D -->|No| G["Reviewed boundary"]

Loading

High-Level Assessment

The focused timestamp preference is the best fit: it matches Git's rebase semantics and the existing GitLab behavior while preserving compatibility through author-date fallback. Persisting and comparing reviewed SHAs would require additional review metadata and handling for force-pushed-away commits, adding disproportionate complexity.

Files changed (2) +64 / -1

Bug fix (1) +8 / -1

github_provider.pyUse committer timestamps for incremental commit ranges +8/-1

Use committer timestamps for incremental commit ranges

• Adds a timeline-date helper that prefers each commit's committer timestamp and falls back to its author timestamp. Incremental range selection now detects commits rewritten by rebases after the previous review.

pr_agent/git_providers/github_provider.py

Tests (1) +56 / -0

test_github_provider_incremental.pyAdd regression coverage for incremental range timestamps +56/-0

Add regression coverage for incremental range timestamps

• Adds focused unit tests for rebased commits, commits predating review, missing committer metadata, and mixed reviewed/new commit ranges.

tests/unittest/test_github_provider_incremental.py

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (4) 📜 Skill insights (0)

1. Rebase loses diff baseline ✓ Resolved 🐞 Bug ≡ Correctness

Description

When a rebase gives every PR commit a committer date after the previous review, get_commit_range()
returns all commits but never sets last_seen_commit. The incremental diff path then requests
original file content with a None baseline SHA, so it can compare against the wrong ref or
silently use empty content instead of falling back to a full review.

Code

pr_agent/git_providers/github_provider.py[200]

+            if self._commit_timeline_date(self.pr_commits[index]) > last_review_time:

Relevance

●●● Strong

Accepted incremental-range precedents require preserving baselines and disabling incremental mode
when reliable range computation fails.

PR-#2381
PR-#2389

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed comparison can place every rebased commit after the review; the loop sets
last_seen_commit only in its else branch, while IncrementalPR.last_seen_commit_sha is None
when that branch is never reached. GitHub nevertheless populates the incremental file map and passes
that missing SHA to get_contents, whose exception path silently returns an empty string; analogous
GitLab and Azure flows explicitly fall back to full review when no baseline exists.

pr_agent/git_providers/github_provider.py[193-206]
pr_agent/git_providers/github_provider.py[175-185]
pr_agent/git_provider.py[621-634]
pr_agent/git_providers/github_provider.py[328-340]
pr_agent/git_providers/github_provider.py[1271-1299]
pr_agent/git_providers/gitlab_provider.py[547-562]
pr_agent/git_providers/azuredevops_provider.py[448-458]
PR-#2381

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A fully rebased PR can make every committer timestamp newer than the review, leaving `last_seen_commit` unset even though an incremental commit range is returned. Downstream GitHub diff generation requires `last_seen_commit_sha`, so this state must fall back to a full review rather than loading original content with a `None` ref.

## Issue Context
Preserve the legitimate empty-range case, but disable incremental mode when new commits exist and no historical baseline commit can be established. Add a regression test that exercises incremental diff setup after all commits are rebased, not only `get_commit_range()` in isolation.

## Fix Focus Areas
- pr_agent/git_providers/github_provider.py[171-206]
- pr_agent/git_providers/github_provider.py[328-340]
- tests/unittest/test_github_provider_incremental.py[26-32]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Fallback comment is narrative 📘 Rule violation ⚙ Maintainability ⭐ New

Description

The new fallback block comment narrates commit state with Every commit post-dates and `there is no
baseline` instead of using imperative phrasing as required.

Code

pr_agent/git_providers/github_provider.py[R179-181]

+                # Every commit post-dates the review (e.g. the branch was fully rebased), so there
+                # is no baseline commit to diff against. Fall back to a full review rather than
+                # diffing against a None ref, which silently yields empty original content.

Relevance

●●● Strong

Recent repository precedents explicitly accept rewriting narrative behavior comments into imperative
phrasing.

PR-#2797
PR-#2791

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited rule requires behavior comments to use imperative mood, while the new block begins with
the narrative statements Every commit post-dates the review and there is no baseline commit.

Rule 2694688: Docstrings and comments must use imperative phrasing
pr_agent/git_providers/github_provider.py[179-181]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Rewrite the fallback block comment so it uses imperative phrasing rather than narrative statements.

## Issue Context
PR Compliance ID 2694688 requires newly added comments describing behavior to be phrased as commands or instructions.

## Fix Focus Areas
- pr_agent/git_providers/github_provider.py[179-181]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Rebase test comment narrative 📘 Rule violation ⚙ Maintainability ⭐ New

Description

The new test comment describes the commit state with Every commit post-dates rather than
expressing the behavior in imperative phrasing.

Code

tests/unittest/test_github_provider_incremental.py[51]

+    # Every commit post-dates the review, so there is no baseline commit to diff against.

Relevance

●● Moderate

Comment-style outcomes are mixed: recent imperative rewrites accepted, but closely similar
test-comment rewrites rejected.

PR-#2774
PR-#2785
PR-#2703

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited rule requires imperative phrasing for behavior comments, but this newly added test comment
is a declarative narrative sentence.

Rule 2694688: Docstrings and comments must use imperative phrasing
tests/unittest/test_github_provider_incremental.py[51-51]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Rewrite the test comment as an imperative instruction describing the scenario being established.

## Issue Context
PR Compliance ID 2694688 applies to newly added comments, including comments in tests.

## Fix Focus Areas
- tests/unittest/test_github_provider_incremental.py[51-51]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. getattr keys use single quotes 📘 Rule violation ⚙ Maintainability

Description

The added getattr calls use single-quoted Python string literals even though double quotes require
no extra escaping. This violates the repository string-literal convention.

Code

pr_agent/git_providers/github_provider.py[193]

+        committer_date = getattr(getattr(commit.commit, 'committer', None), 'date', None)

Relevance

●●● Strong

Recent GitHub-provider precedent accepted converting newly added single-quoted literals to double
quotes.

PR-#2796
PR-#2776

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2694657 requires double quotes for Python string literals, while the added line uses
'committer' and 'date' without an escaping-related justification.

Rule 2694657: Use double quotes for all Python string literals
pr_agent/git_providers/github_provider.py[193-193]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Replace the single-quoted `getattr` attribute names with double-quoted string literals.

## Issue Context
Compliance rule 2694657 requires double quotes for Python string literals unless double quotes would require additional escaping.

## Fix Focus Areas
- pr_agent/git_providers/github_provider.py[193-193]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View medium (1)

5. Force-push comment is narrative 📘 Rule violation ⚙ Maintainability

Description

The added comment narrates force-push behavior instead of expressing it in imperative phrasing. This
violates the required comment style.

Code

tests/unittest/test_github_provider_incremental.py[27]

+    # A force-push rewrites the commit after the review, but git preserves the author date.

Relevance

●● Moderate

Imperative-comment findings are often accepted, but a recent same-context comment-style finding was
rejected.

PR-#2797
PR-#2774

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2694688 requires behavioral comments to use command or instruction phrasing, but the added
comment states narratively that a force-push rewrites the commit and Git preserves the author date.

Rule 2694688: Docstrings and comments must use imperative phrasing
tests/unittest/test_github_provider_incremental.py[27-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Rewrite the behavioral comment as an imperative instruction, or remove it if the test name and setup already make the scenario clear.

## Issue Context
Compliance rule 2694688 requires newly added behavioral comments to use imperative rather than narrative phrasing.

## Fix Focus Areas
- tests/unittest/test_github_provider_incremental.py[27-27]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

• Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Context sources

✅ Compliance rules (platform): 34 rules

Review mode: ⚖️ Balanced: This is a localized but behavior-changing fix in incremental review state management; an incorrect fallback could suppress or broaden reviews, so it warrants a careful single-pass review, though it is not dense enough for extended.

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 56f8eca ⚖️ Balanced

Results up to commit e000704 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX issues (0) 📜 Skill insights (0)

1. Rebase loses diff baseline ✓ Resolved 🐞 Bug ≡ Correctness

Description

When a rebase gives every PR commit a committer date after the previous review, get_commit_range()
returns all commits but never sets last_seen_commit. The incremental diff path then requests
original file content with a None baseline SHA, so it can compare against the wrong ref or
silently use empty content instead of falling back to a full review.

Code

pr_agent/git_providers/github_provider.py[200]

+            if self._commit_timeline_date(self.pr_commits[index]) > last_review_time:

Relevance

●●● Strong

Accepted incremental-range precedents require preserving baselines and disabling incremental mode
when reliable range computation fails.

PR-#2381
PR-#2389

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed comparison can place every rebased commit after the review; the loop sets
last_seen_commit only in its else branch, while IncrementalPR.last_seen_commit_sha is None
when that branch is never reached. GitHub nevertheless populates the incremental file map and passes
that missing SHA to get_contents, whose exception path silently returns an empty string; analogous
GitLab and Azure flows explicitly fall back to full review when no baseline exists.

pr_agent/git_providers/github_provider.py[193-206]
pr_agent/git_providers/github_provider.py[175-185]
pr_agent/git_provider.py[621-634]
pr_agent/git_providers/github_provider.py[328-340]
pr_agent/git_providers/github_provider.py[1271-1299]
pr_agent/git_providers/gitlab_provider.py[547-562]
pr_agent/git_providers/azuredevops_provider.py[448-458]
PR-#2381

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A fully rebased PR can make every committer timestamp newer than the review, leaving `last_seen_commit` unset even though an incremental commit range is returned. Downstream GitHub diff generation requires `last_seen_commit_sha`, so this state must fall back to a full review rather than loading original content with a `None` ref.

## Issue Context
Preserve the legitimate empty-range case, but disable incremental mode when new commits exist and no historical baseline commit can be established. Add a regression test that exercises incremental diff setup after all commits are rebased, not only `get_commit_range()` in isolation.

## Fix Focus Areas
- pr_agent/git_providers/github_provider.py[171-206]
- pr_agent/git_providers/github_provider.py[328-340]
- tests/unittest/test_github_provider_incremental.py[26-32]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. getattr keys use single quotes 📘 Rule violation ⚙ Maintainability

Description

The added getattr calls use single-quoted Python string literals even though double quotes require
no extra escaping. This violates the repository string-literal convention.

Code

pr_agent/git_providers/github_provider.py[193]

+        committer_date = getattr(getattr(commit.commit, 'committer', None), 'date', None)

Relevance

●●● Strong

Recent GitHub-provider precedent accepted converting newly added single-quoted literals to double
quotes.

PR-#2796
PR-#2776

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2694657 requires double quotes for Python string literals, while the added line uses
'committer' and 'date' without an escaping-related justification.

Rule 2694657: Use double quotes for all Python string literals
pr_agent/git_providers/github_provider.py[193-193]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Replace the single-quoted `getattr` attribute names with double-quoted string literals.

## Issue Context
Compliance rule 2694657 requires double quotes for Python string literals unless double quotes would require additional escaping.

## Fix Focus Areas
- pr_agent/git_providers/github_provider.py[193-193]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Force-push comment is narrative 📘 Rule violation ⚙ Maintainability

Description

The added comment narrates force-push behavior instead of expressing it in imperative phrasing. This
violates the required comment style.

Code

tests/unittest/test_github_provider_incremental.py[27]

+    # A force-push rewrites the commit after the review, but git preserves the author date.

Relevance

●● Moderate

Imperative-comment findings are often accepted, but a recent same-context comment-style finding was
rejected.

PR-#2797
PR-#2774

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2694688 requires behavioral comments to use command or instruction phrasing, but the added
comment states narratively that a force-push rewrites the commit and Git preserves the author date.

Rule 2694688: Docstrings and comments must use imperative phrasing
tests/unittest/test_github_provider_incremental.py[27-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Rewrite the behavioral comment as an imperative instruction, or remove it if the test name and setup already make the scenario clear.

## Issue Context
Compliance rule 2694688 requires newly added behavioral comments to use imperative rather than narrative phrasing.

## Fix Focus Areas
- tests/unittest/test_github_provider_incremental.py[27-27]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 56f8eca