Skip to content

Track comment timestamps per PR instead of globally - #453

Merged
stbenjam merged 2 commits into
mainfrom
fix-per-pr-since-tracking
Jul 26, 2026
Merged

Track comment timestamps per PR instead of globally#453
stbenjam merged 2 commits into
mainfrom
fix-per-pr-since-tracking

Conversation

@not-stbenjam

@not-stbenjam not-stbenjam commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The pr-followup workflow used a single global since timestamp to fetch new comments, causing it to miss bot comments (devin, codex, codecov) on PR Add first-class OpenAI Codex plugin and marketplace linting #451 — they were posted before the global timestamp but had never been seen by any run
  • Switches to per-PR timestamps cached in .pr-followup-since/pr-<number>. First time a PR is seen: all comments fetched. Subsequent runs: only comments since that PR was last processed
  • Also fetches comments directly from per-PR endpoints instead of repo-wide, avoiding the 100-item pagination limit across all PRs

Test plan

  • Verify workflow syntax is valid (CI)
  • Label a PR with agent and confirm first run fetches all comments ("first time — fetching all comments")
  • Confirm second run uses the per-PR timestamp ("fetching comments since ...")
  • Confirm bot comments (devin, codex, codecov, coderabbitai) appear in the agent prompt

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved pull request follow-up detection by tracking updates independently for each open pull request.
    • Prevented missed or duplicated review and discussion comments across workflow runs.
    • Filtered follow-up content to trusted contributors and removed hidden HTML comment content before processing.

The discover step used a single global timestamp to fetch new
comments. If a PR gained the agent label between runs, all
comments posted before the last run were permanently missed.

Switch to per-PR timestamps cached in .pr-followup-since/. When
a PR is seen for the first time, all its comments are fetched
(no since cutoff). On subsequent runs, only new comments since
the last time that specific PR was processed are fetched.

Also fetches comments per-PR directly from the PR's own
endpoints instead of repo-wide, which avoids the 100-item
pagination limit across all PRs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@not-stbenjam
not-stbenjam requested a review from stbenjam as a code owner July 26, 2026 01:30
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@not-stbenjam, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 50 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f58d1d7c-26ae-434c-81f8-d886f6a7bcde

📥 Commits

Reviewing files that changed from the base of the PR and between 6606f0a and 0f98495.

📒 Files selected for processing (1)
  • .github/workflows/skillsaw-pr-followup.yml
📝 Walkthrough

Walkthrough

The PR follow-up workflow now tracks discovery timestamps per pull request, fetches comments individually using those timestamps, filters and sanitizes results, persists updated state, and passes the resulting comment blocks to the follow-up prompt.

Changes

PR follow-up discovery

Layer / File(s) Summary
Per-PR timestamp cache
.github/workflows/skillsaw-pr-followup.yml
The discover job restores and saves .pr-followup-since/ data instead of a single global timestamp.
Per-PR comment fetching and prompt assembly
.github/workflows/skillsaw-pr-followup.yml
Review and issue comments are fetched per PR, filtered, sanitized, merged into per-PR JSON files, and supplied to the follow-up prompt without the previous “since” header.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant discover job
  participant GitHub API
  participant follow-up job
  discover job->>GitHub API: Fetch review and issue comments per PR
  GitHub API-->>discover job: Return filtered comment data
  discover job->>follow-up job: Provide per-PR review_comments and pr_comments
Loading

Possibly related PRs

Suggested reviewers: stbenjam

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: moving comment timestamp tracking from a single global value to per-PR timestamps.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-per-pr-since-tracking

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Track per-PR comment “since” timestamps in pr-followup workflow

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Cache and track “since” timestamps per PR to avoid missing previously unseen comments.
• Fetch PR comments via per-PR endpoints with pagination instead of repo-wide 100-item lists.
• Update workflow prompt inputs to remove the global since timestamp dependency.
Diagram

graph TD
  A["Discover job"] --> B[("Per-PR since cache (.pr-followup-since/")]) --> C["Per-PR fetch loop"] --> D{{"GitHub API"}} --> E["/tmp/pr-<n>.json"] --> F["Follow-up job"]
  C -- "write pr-<n> since" --> B

  subgraph Legend
    direction LR
    _p["Process/step"] ~~~ _c[("Cache/state") ] ~~~ _e{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Persist max seen comment timestamp (not “now”) per PR
  • ➕ Avoids skipping comments created between fetch time and cache-write time
  • ➕ More deterministic replay behavior if workflow runtime varies
  • ➖ Requires computing max(created_at) across both comment sources (and handling empty results)
  • ➖ Slightly more jq/shell complexity
2. Persist last seen comment IDs per PR (review + issue comments)
  • ➕ Eliminates timestamp boundary/race conditions entirely
  • ➕ Can dedupe by ID across runs
  • ➖ Requires tracking two streams and merging/deduping logic
  • ➖ More state files/format complexity than timestamps
3. Use GraphQL timeline with cursors per PR
  • ➕ Unified PR timeline (comments, reviews, bots) with cursor-based pagination
  • ➕ First-class incremental fetch pattern
  • ➖ Bigger implementation change; higher maintenance and auth/query complexity
  • ➖ Harder to debug than REST + gh api

Recommendation: The per-PR since-cache approach is the right direction and fixes the core correctness gap (PRs newly labeled ‘agent’ won’t miss older comments). Consider a small follow-up improvement: store the maximum fetched comment created_at (or the run start time) rather than writing the current time at the end, to avoid a race where comments posted during the run can fall between the fetch and the cached timestamp and be skipped next run.

Files changed (1) +35 / -44

Bug fix (1) +35 / -44
skillsaw-pr-followup.ymlReplace global since tracking with cached per-PR comment fetching +35/-44

Replace global since tracking with cached per-PR comment fetching

• Reworks the discover job to restore/save a cache directory of per-PR since timestamps and uses those to decide whether to fetch all comments (first seen) or only recent comments (subsequent runs). Switches from repo-wide comment endpoints to per-PR endpoints with pagination and updates logging and downstream prompt construction to remove the global since output.

.github/workflows/skillsaw-pr-followup.yml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/skillsaw-pr-followup.yml:
- Around line 65-94: Update the comment-fetching flow around PR_REVIEW_COMMENTS
and PR_COMMENTS to capture a NEXT_SINCE UTC timestamp immediately before the
first gh api request. After both requests complete successfully, persist
NEXT_SINCE to SINCE_FILE instead of generating a new timestamp at the end,
ensuring the cursor covers comments posted during fetching.
- Around line 76-83: Update the PR_REVIEW_COMMENTS and PR_COMMENTS jq pipelines
to flatten the multiple JSON arrays emitted by gh api --paginate into a single
comment stream before applying the --argjson collabs filter. Preserve the
existing collaborator selection, body cleanup, and projected fields while
ensuring discovery succeeds across more than one page of results.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d0161051-498c-4dba-a956-509fb1744e1b

📥 Commits

Reviewing files that changed from the base of the PR and between 13073ff and 6606f0a.

📒 Files selected for processing (1)
  • .github/workflows/skillsaw-pr-followup.yml

Comment thread .github/workflows/skillsaw-pr-followup.yml Outdated
Comment thread .github/workflows/skillsaw-pr-followup.yml Outdated
@qodo-code-review

qodo-code-review Bot commented Jul 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 136 rules
✅ Skills: 5 invoked
  skillsaw-pr-review
  skillsaw-issue-solver
  skillsaw-pr-followup
  skillsaw-create-plugin
  skillsaw-review-panel

Grey Divider


Action required

1. Since checkpoint skips comments ✓ Resolved 🐞 Bug ☼ Reliability
Description
In discover, the per-PR since file is set to the current wall-clock time after fetching comments,
so a comment created after the API call returns but before the timestamp write will be older than
the stored cutoff and never fetched in a later run. This can silently drop collaborator/bot comments
from the follow-up agent input.
Code

.github/workflows/skillsaw-pr-followup.yml[R75-94]

+            # Fetch review comments for this PR
+            PR_REVIEW_COMMENTS=$(gh api "repos/${{ github.repository }}/pulls/${NUM}/comments?per_page=100${SINCE_PARAM}" --paginate \
+              | jq --argjson collabs "$COLLABS" \
+              '[.[] | select(.user.login as $u | $collabs | index($u)) | {id, user: .user.login, body: (.body // "" | gsub("(?s)<!--.*?-->"; "") | gsub("(?s)<!--.*"; "")), path, line, created_at}]')
+
+            # Fetch PR-level comments for this PR
+            PR_COMMENTS=$(gh api "repos/${{ github.repository }}/issues/${NUM}/comments?per_page=100${SINCE_PARAM}" --paginate \
+              | jq --argjson collabs "$COLLABS" \
+              '[.[] | select(.user.login as $u | $collabs | index($u)) | {id, user: .user.login, body: (.body // "" | gsub("(?s)<!--.*?-->"; "") | gsub("(?s)<!--.*"; "")), created_at}]')

            echo "$pr" | jq \
              --argjson rc "$PR_REVIEW_COMMENTS" \
              --argjson pc "$PR_COMMENTS" \
              '. + {review_comments: $rc, pr_comments: $pc}' \
              > "/tmp/pr-${NUM}.json"
+
+            echo "  Review comments: $(echo "$PR_REVIEW_COMMENTS" | jq length), PR comments: $(echo "$PR_COMMENTS" | jq length)"
+
+            # Update per-PR timestamp for next run
+            date -u '+%Y-%m-%dT%H:%M:%SZ' > "$SINCE_FILE"
Relevance

⭐⭐⭐ High

Similar workflow reliability edge cases (comment handling/guards) have been accepted repeatedly;
likely they’ll fix this checkpoint race.

PR-#447
PR-#450
PR-#370

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow fetches both comment streams and only then writes the per-PR cutoff timestamp using the
current time, creating a window where a comment can be posted but then excluded by the next run’s
since filter.

.github/workflows/skillsaw-pr-followup.yml[63-95]

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

### Issue description
The workflow updates `.pr-followup-since/pr-<NUM>` using `date -u ...` after the comment fetches. If a new comment is created after the fetch snapshot but before the file write, the next run’s `since=` cutoff will exclude that comment forever.

### Issue Context
There are two independent fetches per PR (inline review comments and PR-level issue comments). The checkpoint needs to be advanced based on what was actually observed (max `created_at`) or a safe pre-fetch timestamp, not post-fetch wall clock.

### Fix Focus Areas
- .github/workflows/skillsaw-pr-followup.yml[63-95]

### Suggested fix approach
- Capture a per-PR `FETCH_STARTED` timestamp **before** either `gh api` call.
- After both fetches, compute `NEW_SINCE` as:
 - the max `created_at` across both returned arrays (review + PR comments) when at least one exists, else
 - `FETCH_STARTED`.
- Write `NEW_SINCE` to `$SINCE_FILE`.

This ensures comments created during the run but not included in the snapshot are still eligible to be fetched next run, without needlessly moving the checkpoint past unseen events.

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



Remediation recommended

2. skillsaw-pr-followup lacks regression test 📘 Rule violation ▣ Testability
Description
This PR fixes missed PR comments by switching the skillsaw-pr-followup workflow to per-PR since
timestamps, but it adds no automated regression test to prevent a future reintroduction of the
global timestamp behavior. This violates the requirement that bug fixes include regression tests.
Code

.github/workflows/skillsaw-pr-followup.yml[R59-95]

+          # For each PR, fetch comments using that PR's own since timestamp
          echo "$PRS" | jq -c '.[]' | while read -r pr; do
            NUM=$(echo "$pr" | jq -r '.number')

-            # Filter review comments to this PR
-            PR_REVIEW_COMMENTS=$(cat /tmp/new-review-comments.json \
-              | jq --arg num "$NUM" '[.[] | select(.pull_request_url | endswith("/" + $num))]')
-
-            # Filter PR-level comments to this PR
-            PR_COMMENTS=$(cat /tmp/new-pr-comments.json \
-              | jq --arg num "$NUM" '[.[] | select(.issue_url | endswith("/" + $num))]')
+            # Per-PR since: use last-processed timestamp if available,
+            # otherwise fetch all comments (no since cutoff)
+            SINCE_FILE=".pr-followup-since/pr-${NUM}"
+            SINCE_PARAM=""
+            if [ -f "$SINCE_FILE" ]; then
+              PR_SINCE=$(cat "$SINCE_FILE")
+              SINCE_PARAM="&since=${PR_SINCE}"
+              echo "PR #${NUM}: fetching comments since ${PR_SINCE}"
+            else
+              echo "PR #${NUM}: first time — fetching all comments"
+            fi
+
+            # Fetch review comments for this PR
+            PR_REVIEW_COMMENTS=$(gh api "repos/${{ github.repository }}/pulls/${NUM}/comments?per_page=100${SINCE_PARAM}" --paginate \
+              | jq --argjson collabs "$COLLABS" \
+              '[.[] | select(.user.login as $u | $collabs | index($u)) | {id, user: .user.login, body: (.body // "" | gsub("(?s)<!--.*?-->"; "") | gsub("(?s)<!--.*"; "")), path, line, created_at}]')
+
+            # Fetch PR-level comments for this PR
+            PR_COMMENTS=$(gh api "repos/${{ github.repository }}/issues/${NUM}/comments?per_page=100${SINCE_PARAM}" --paginate \
+              | jq --argjson collabs "$COLLABS" \
+              '[.[] | select(.user.login as $u | $collabs | index($u)) | {id, user: .user.login, body: (.body // "" | gsub("(?s)<!--.*?-->"; "") | gsub("(?s)<!--.*"; "")), created_at}]')

            echo "$pr" | jq \
              --argjson rc "$PR_REVIEW_COMMENTS" \
              --argjson pc "$PR_COMMENTS" \
              '. + {review_comments: $rc, pr_comments: $pc}' \
              > "/tmp/pr-${NUM}.json"
+
+            echo "  Review comments: $(echo "$PR_REVIEW_COMMENTS" | jq length), PR comments: $(echo "$PR_COMMENTS" | jq length)"
+
+            # Update per-PR timestamp for next run
+            date -u '+%Y-%m-%dT%H:%M:%SZ' > "$SINCE_FILE"
          done
Relevance

⭐⭐ Medium

Team often accepts adding regression tests for bug fixes, but workflow-level regression testing
precedent unclear.

PR-#429
PR-#186

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2314726 requires a regression test for bug fixes. The workflow logic was changed to
implement per-PR since tracking and per-PR comment fetching, but no tests were added alongside
this behavioral change.

Rule 2314726: Add regression tests for every bug fix
.github/workflows/skillsaw-pr-followup.yml[59-95]

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

## Issue description
The PR changes comment-fetch behavior in `.github/workflows/skillsaw-pr-followup.yml` to fix a missed-comments bug, but there is no regression test to lock in the new per-PR `since` tracking behavior.

## Issue Context
The workflow now persists per-PR timestamps under `.pr-followup-since/pr-<number>` and fetches comments from per-PR endpoints with an optional `since` parameter. A regression test should fail if someone accidentally reverts to a single global `since` cutoff or repo-wide comment endpoints.

## Fix Focus Areas
- .github/workflows/skillsaw-pr-followup.yml[59-95]
- tests/test_pr_followup_workflow.py[1-200]

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


Grey Divider

Qodo Logo

Comment thread .github/workflows/skillsaw-pr-followup.yml
Comment thread .github/workflows/skillsaw-pr-followup.yml Outdated
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.54%. Comparing base (13073ff) to head (0f98495).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #453   +/-   ##
=======================================
  Coverage   93.54%   93.54%           
=======================================
  Files         145      145           
  Lines       11418    11418           
=======================================
  Hits        10681    10681           
  Misses        737      737           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6606f0a37e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .github/workflows/skillsaw-pr-followup.yml Outdated
Comment thread .github/workflows/skillsaw-pr-followup.yml
Capture the since-timestamp before API calls (not after) so comments
posted during fetching aren't missed on the next run. Add --slurp to
gh api --paginate calls and use .[][] in jq to correctly flatten
multi-page responses for PRs with >100 comments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@stbenjam
stbenjam enabled auto-merge (squash) July 26, 2026 01:44
@stbenjam
stbenjam merged commit 81a95ef into main Jul 26, 2026
16 checks passed
@stbenjam
stbenjam deleted the fix-per-pr-since-tracking branch July 27, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants