Repository navigation
Enhance validation and reminder logic for pending user response automation - #23020
nil-is-all wants to merge 19 commits into
Conversation
…response automation Refactor reminder logic to improve user response handling and comment retrieval. Earlier version just printed logs, not closing or removing labels. It also did not fetch comments and detect latest automated reminder well.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23020
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 59e5cda with merge base 0a4a05e ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@claude check if above comments are resolved by latest commit |
|
Claude finished @nil-is-all's task in 1m 17s —— View job Most of the earlier comments are resolved in Detailed status and remaining issuesPR Review: #23020 (at
|
| # | Item | Status |
|---|---|---|
| 1 | Naive vs. aware datetime | ✅ Resolved |
| 2 | Issues never closed | ❌ Regressed, see R1 below |
| 3 | @0 in reminder |
✅ Resolved |
| 4 | Issues with no comments never reminded | ✅ Resolved: labeled_at falls back to issue.created_at (line 52) |
| 5 | Reopened issues closed again | ✅ Resolved: the label is removed on close (line 85) |
| 6 | Only the author's reply counts | |
| A | Re-labeled issues lose the label or get closed at once | ✅ Resolved: the logic is now limited to comments after the latest labeled event (lines 44–55) |
| B | Reminders after the author replied / reminder and close in the same run | ✅ Half resolved: the reply check now runs first (line 58). The "up to 6 bot comments" half is replaced by R1 |
| — | DRY_RUN not set in .yml |
✅ Resolved: false for schedule, true for workflow_dispatch (yml line 26) |
| — | Reminder text uses DAYS_BEFORE_REMINDER for the close deadline |
❌ Not fixed (line 18) |
| — | f"\nHi @{{0}}..." written as an f-string only to escape braces |
❌ Not fixed (line 17). Line 81 print(f"Close issue/PR due to inactivity") is a new f-string with no placeholders (F541) |
| — | Trailing whitespace | ❌ New occurrences on lines 46, 48, 59 |
| — | Pin PyGithub>=2,<3 |
Not done (optional) |
Code Quality
R1. The close branch can't be reached, so issues are reminded weekly forever (.github/workflows/pending_user_response.py:74-85)
The elif order is: reply check, then first reminder, then follow-up if the last reminder is ≥ 7 days old, then close. The cron runs every 7 days, so at each run reminders[-1] is about 7 days old and the follow-up branch matches. The close branch only runs when the last reminder is < 7 days old and the first one is ≥ 30 days old. That only happens through scheduling jitter (for example, a run that starts a few seconds earlier than last week's gives .days == 6). In practice the issue gets "Please respond… this will be closed in 30 days" every Monday and stays open. This is the same symptom as the original item #2.
Fix: drop the follow-up branch and REMINDER_COOLDOWN_DAYS, as suggested last time. If weekly follow-ups are wanted, move the close check above the follow-up check:
elif (now - reminders[0].created_at).days >= DAYS_BEFORE_CLOSE:
# close
elif (now - reminders[-1].created_at).days >= REMINDER_COOLDOWN_DAYS:
# follow-upInfrastructure
R2. The workflow may not have permission to comment, label or close (.github/workflows/pending_user_response.yml:9)
Every other workflow here that writes to issues or PRs declares permissions: explicitly (stale.yml, docathon-sync-label.yml, pin-bump-ci-handler.yml, claude-code.yml). That suggests the default GITHUB_TOKEN in this repo is read-only. If so, the first scheduled run with DRY_RUN=false will fail with 403 on create_comment, remove_from_labels and edit. Confidence is low because I can't see the repo's token settings. Adding this to the job removes the doubt:
permissions:
issues: write
pull-requests: writeRecommendation
Request Changes. R1 must be fixed before this runs for real: DRY_RUN is now false on the schedule, so next Monday's run will start posting weekly reminders that never lead to a close. R2 should be confirmed or fixed in the same change.
I reviewed by reading the code only. I didn't run linters or change any code, and the GitHub App can't edit files under .github/workflows/ anyway.
Put close block before reminder block to ensure issue is not in a limbo
Added permissions for issues and pull requests.
|
@claude review |
|
Claude finished @nil-is-all's task in 1m 4s —— View job The latest commits fix R1 (close ordering) and R2 (permissions), but they add a blocking crash: Detailed review (at
|
added checks for review comments and commits ensured to check for issues and PRs
Refactor reminder logic to improve user response handling and comment retrieval. Earlier version just printed logs, not closing or removing labels. It also did not fetch comments and detect latest automated reminder well.