Fix flake 22820 :UnboundLocalError: local variable 'updated_pr_dict' referenced before assignment (Send Pending Reviews Notification) by Yashbhu · Pull Request #13 · oppia/stale-review-request-notifier

Yashbhu · 2026-04-25T20:09:21Z

issue : The script was failing with an UnboundLocalError. This occurred because a key variable, updated_pr_dict, was only being defined if the script successfully found an "assigned" event in the Pull Request's activity history (timeline).

If a Pull Request had assignees but the specific "assigned" event was missing from the history which happens if the assignment is very old or if the history entries are limited the variable remained unassigned, causing the script to crash when it tried to process the PR.

Fixes #22820 by ensuring updated_pr_dict is initialized before the timeline processing loop. Added a fallback to the PR's creation time for assignees missing specific assignment events in the history to prevent crashes.

Changes:

->Initialized updated_pr_dict early with copy.deepcopy.
->Used setdefault to handle missing assignment timestamps.
->Added a regression test for missing timeline events.
proof:

…ly and providing fallback for missing assignment events

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes a flake caused by updated_pr_dict being conditionally initialized when no "assigned" timeline events exist, and adds a regression test to prevent crashes for older/limited PR timelines.

Changes:

Initialize updated_pr_dict before timeline processing and pass it through timestamp enrichment.
Add a fallback assignment timestamp (PR creation time) when assignment events are missing.
Add a regression test covering PRs with assignees but no "assigned" events in the timeline.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/github_services.py`	Ensures `updated_pr_dict` is always initialized and assignees get a fallback timestamp when assignment events are absent.
`tests/github_services_test.py`	Adds a regression test to ensure PR parsing does not crash when timelines omit assignment events.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T20:15:38Z

+
+        self.assertIsInstance(response, github_domain.PullRequest)
+        self.assertEqual(response.url, mocked_response['html_url'])
+


The new regression test validates the crash is avoided, but it doesn’t assert the newly introduced fallback behavior (that assignees missing an assigned event get a timestamp derived from the PR creation time). To prevent future regressions where the code stops setting the fallback but still returns a PullRequest, add assertions that the parsed result includes both assignees and that their associated timestamp equals the PR created_at when no assignment events are present (using whatever field/mapping PullRequest.from_github_response exposes for reviewer assignment times).

Suggested change

expected_assignees = {

assignee['login'] for assignee in mocked_response['assignees']

}

expected_created_at = datetime.datetime(

2023, 7, 31, 22, 24, 38, tzinfo=tzutc())

assignment_time_mapping = None

for attr_name in (

'reviewer_assignment_timestamps_by_username',

'reviewer_assignment_timestamps',

'assignee_assignment_timestamps_by_username',

'assignee_assignment_timestamps'):

if hasattr(response, attr_name):

assignment_time_mapping = getattr(response, attr_name)

break

self.assertIsNotNone(

assignment_time_mapping,

'PullRequest should expose reviewer/assignee assignment times.')

self.assertEqual(set(assignment_time_mapping.keys()), expected_assignees)

for assignee in expected_assignees:

self.assertEqual(

assignment_time_mapping[assignee], expected_created_at)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

U8NWXD

If a Pull Request had assignees but the specific "assigned" event was missing from the history which happens if the assignment is very old or if the history entries are limited the variable remained unassigned, ...

Could you elaborate on why some PR histories lack an assigned event for assigned reviewers? Why are these events missing for old assignments? What circumstances cause the history entries to be limited?

One idea: could this be happening because the PR timeline is paginated and we don't retrieve all the pages of events?

Added a fallback to the PR's creation time for assignees missing specific assignment events in the history to prevent crashes.

I don't think PR creation time is a good proxy for reviewer assignment. I'd prefer we get the actual assignment time, which I think is info GitHub should have

Yashbhu · 2026-05-04T11:58:52Z

If a Pull Request had assignees but the specific "assigned" event was missing from the history which happens if the assignment is very old or if the history entries are limited the variable remained unassigned, ...

Could you elaborate on why some PR histories lack an assigned event for assigned reviewers? Why are these events missing for old assignments? What circumstances cause the history entries to be limited?

One idea: could this be happening because the PR timeline is paginated and we don't retrieve all the pages of events?

Added a fallback to the PR's creation time for assignees missing specific assignment events in the history to prevent crashes.

I don't think PR creation time is a good proxy for reviewer assignment. I'd prefer we get the actual assignment time, which I think is info GitHub should have

No, it's not because of pagination or limits.
The events are missing for two specific reasons:

They are review_requested events, not assigned events. In Oppia, reviewers are usually requested using the "Reviewers" feature on GitHub. This creates a review_requested event in the timeline. Since the code was only looking for the word "assigned", it skipped all of these completely.
They were assigned exactly when the PR was created. If someone adds an assignee at the exact moment they create the Pull Request, GitHub doesn't generate a separate assignment event in the timeline at all. The assignment is just bundled into the PR creation.

To get the actual assignment time (and avoid relying on PR creation time unnecessarily), I went ahead and updated github_services.py to parse review_requested events too.

Now the code will grab the exact timestamps for anyone requested as a reviewer. For the rare cases where someone was assigned at the exact moment the PR was created (and neither event exists), falling back to the PR creation time is correct i think , because that was their assignment time

Yashbhu · 2026-05-04T11:59:50Z

@U8NWXD PTAL

U8NWXD · 2026-05-08T13:41:59Z

In Oppia, reviewers are usually requested using the "Reviewers" feature on GitHub.

No, we use the "Assignments" feature to assign reviewers. When a PR is opened, GitHub automatically adds reviewers using the "Reviewers" feature based on code ownership. We then have a bot that actually assigns these reviewers. Reviewers aren't expected to look at a PR until they've been assigned so that if one reviewer needs to take a first pass, we can de-assign the other reviewers while still tracking that they need to eventually approve the PR with the "Reviewers" feature.

Just to check, were you aware of this distinction between how we use reviewers and assignees at Oppia? Given your contribution history, it's something I'd have expected you to be familiar with. If this is new info to you, we might need to update our contribution guidance to make this clearer for new contributors.

A review request is only stale if the reviewer has been assigned using the "Assignments" feature. Having a requested review using the "Reviewers" feature doesn't count.

They were assigned exactly when the PR was created. If someone adds an assignee at the exact moment they create the Pull Request, GitHub doesn't generate a separate assignment event in the timeline at all. The assignment is just bundled into the PR creation.

In this case, can we parse the PR creation event to find the assignment? That would be safer than falling back to PR creation time, which might hide errors later on.

Yashbhu · 2026-05-22T13:36:21Z

In Oppia, reviewers are usually requested using the "Reviewers" feature on GitHub.

No, we use the "Assignments" feature to assign reviewers. When a PR is opened, GitHub automatically adds reviewers using the "Reviewers" feature based on code ownership. We then have a bot that actually assigns these reviewers. Reviewers aren't expected to look at a PR until they've been assigned so that if one reviewer needs to take a first pass, we can de-assign the other reviewers while still tracking that they need to eventually approve the PR with the "Reviewers" feature.

Just to check, were you aware of this distinction between how we use reviewers and assignees at Oppia? Given your contribution history, it's something I'd have expected you to be familiar with. If this is new info to you, we might need to update our contribution guidance to make this clearer for new contributors.

A review request is only stale if the reviewer has been assigned using the "Assignments" feature. Having a requested review using the "Reviewers" feature doesn't count.

They were assigned exactly when the PR was created. If someone adds an assignee at the exact moment they create the Pull Request, GitHub doesn't generate a separate assignment event in the timeline at all. The assignment is just bundled into the PR creation.

In this case, can we parse the PR creation event to find the assignment? That would be safer than falling back to PR creation time, which might hide errors later on.

Thanks for clarifying the distinction between the "Assignments" and "Reviewers" features at Oppia yes i knew about this very well over contribution journey but i think i got a bit confused about it
thanks for correcting me

In this case, can we parse the PR creation event to find the assignment? That would be safer than falling back to PR creation time...

Unfortunately, no. While GitHub does emit a PullRequestEvent (with action: opened) for Webhooks and the repo-wide /events firehose, that event does not exist in the Issue Timeline API (/issues/{issue_number}/timeline) which our script relies on. The Timeline API only tracks historical actions that happen after the initial creation payload is saved.

Because our script queries the Timeline API to fetch PR history reliably (avoiding the 300-event limit of the /events firehose), we physically cannot parse a creation event.

so , i think fallback is neseccary and safe since Oppiabot handles most assignments after a PR is opened, those will always generate a proper assigned event in the timeline that the script will successfully catch.
However, for the rare cases where someone is manually assigned exactly at creation time (and thus no assigned event exists), querying the main Pull Request object and falling back to its created_at timestamp is the only way to get their assignment time via the REST API.

I hope this clarifies why the fallback is structurally necessary for those edge cases! Let me know if you'd like any other adjustments.

Yashbhu · 2026-05-22T13:37:50Z

@U8NWXD

Yashbhu · 2026-05-26T10:38:50Z

@U8NWXD ptal

U8NWXD · 2026-06-03T02:36:32Z

avoiding the 300-event limit of the /events firehose

Where are you seeing this 300-event limit? I don't see it in the REST API docs. From those docs, it looks like we should be able to retrieve as many events as we like, so long as we use the pagination.

They were assigned exactly when the PR was created. If someone adds an assignee at the exact moment they create the Pull Request, GitHub doesn't generate a separate assignment event in the timeline at all. The assignment is just bundled into the PR creation.

Why do you think this is the case? I'm not so sure. I created a test PR (oppia/oppia#26292) where I assigned kevintab95 when initially drafting the PR description. This should get the assignment as close to concurrent with PR creation as possible, but the assignment event still showed up separately in the PR timeline (see https://api.github.com/repos/oppia/oppia/issues/26292/timeline):

[
...
  {
    "id": 26265635276,
    "node_id": "AE_lADOAmzXy88AAAABEMZTds8AAAAGHY3JzA",
    "url": "https://api.github.com/repos/oppia/oppia/issues/events/26265635276",
    "actor": {
      "login": "U8NWXD",
      "id": 19878639,
      "node_id": "MDQ6VXNlcjE5ODc4NjM5",
      "avatar_url": "https://avatars.githubusercontent.com/u/19878639?v=4",
      "gravatar_id": "",
      "url": "https://api.github.com/users/U8NWXD",
      "html_url": "https://github.com/U8NWXD",
      "followers_url": "https://api.github.com/users/U8NWXD/followers",
      "following_url": "https://api.github.com/users/U8NWXD/following{/other_user}",
      "gists_url": "https://api.github.com/users/U8NWXD/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/U8NWXD/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/U8NWXD/subscriptions",
      "organizations_url": "https://api.github.com/users/U8NWXD/orgs",
      "repos_url": "https://api.github.com/users/U8NWXD/repos",
      "events_url": "https://api.github.com/users/U8NWXD/events{/privacy}",
      "received_events_url": "https://api.github.com/users/U8NWXD/received_events",
      "type": "User",
      "user_view_type": "public",
      "site_admin": false
    },
    "event": "assigned",
    "commit_id": null,
    "commit_url": null,
    "created_at": "2026-06-03T02:30:10Z",
    "assignee": {
      "login": "kevintab95",
      "id": 11008603,
      "node_id": "MDQ6VXNlcjExMDA4NjAz",
      "avatar_url": "https://avatars.githubusercontent.com/u/11008603?v=4",
      "gravatar_id": "",
      "url": "https://api.github.com/users/kevintab95",
      "html_url": "https://github.com/kevintab95",
      "followers_url": "https://api.github.com/users/kevintab95/followers",
      "following_url": "https://api.github.com/users/kevintab95/following{/other_user}",
      "gists_url": "https://api.github.com/users/kevintab95/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/kevintab95/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/kevintab95/subscriptions",
      "organizations_url": "https://api.github.com/users/kevintab95/orgs",
      "repos_url": "https://api.github.com/users/kevintab95/repos",
      "events_url": "https://api.github.com/users/kevintab95/events{/privacy}",
      "received_events_url": "https://api.github.com/users/kevintab95/received_events",
      "type": "User",
      "user_view_type": "public",
      "site_admin": false
    },
    "performed_via_github_app": null
  }
...
]

U8NWXD · 2026-06-03T02:37:50Z

also sorry I keep missing this PR because we don't have automatic assignment in this repo. I'm leaving myself assigned this time

Yashbhu added 6 commits April 22, 2026 20:04

Fix #22820: Fix UnboundLocalError by initializing updated_pr_dict ear…

23761af

…ly and providing fallback for missing assignment events

added

b73d6ad

added

3035bba

Revert unintended changes to README.md

2eb781d

Revert unintended changes to PENDING_REVIEW_NOTIFICATION_TEMPLATE.md

4173666

Refactor: Use copy.deepcopy and setdefault for cleaner PR logic

4ad5d71

Copilot AI review requested due to automatic review settings April 25, 2026 20:09

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Copilot started reviewing on behalf of Yashbhu April 25, 2026 20:19 View session

Apply suggestions from code review

27e251d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

U8NWXD requested changes Apr 27, 2026

View reviewed changes

U8NWXD mentioned this pull request Apr 27, 2026

Fix KeyError: 'created_at' and handle missing timestamps #12

Open

added the changes

0dde4ba

Yashbhu requested a review from U8NWXD May 4, 2026 11:59

U8NWXD assigned Yashbhu May 8, 2026

U8NWXD assigned U8NWXD and unassigned Yashbhu Jun 1, 2026

U8NWXD mentioned this pull request Jun 3, 2026

[DO NOT MERGE] PR timeline investigation oppia/oppia#26292

Closed

U8NWXD assigned Yashbhu Jun 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix flake 22820 :UnboundLocalError: local variable 'updated_pr_dict' referenced before assignment (Send Pending Reviews Notification)#13

Fix flake 22820 :UnboundLocalError: local variable 'updated_pr_dict' referenced before assignment (Send Pending Reviews Notification)#13
Yashbhu wants to merge 8 commits into
oppia:developfrom
Yashbhu:fix-flake-22820

Yashbhu commented Apr 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

U8NWXD left a comment

Uh oh!

Yashbhu commented May 4, 2026

Uh oh!

Yashbhu commented May 4, 2026

Uh oh!

U8NWXD commented May 8, 2026

Uh oh!

Yashbhu commented May 22, 2026

Uh oh!

Yashbhu commented May 22, 2026

Uh oh!

Yashbhu commented May 26, 2026

Uh oh!

U8NWXD commented Jun 3, 2026

Uh oh!

U8NWXD commented Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		self.assertIsInstance(response, github_domain.PullRequest)
		self.assertEqual(response.url, mocked_response['html_url'])

+        expected_assignees = {
+            assignee['login'] for assignee in mocked_response['assignees']
+        }
+        expected_created_at = datetime.datetime(
+, 7, 31, 22, 24, 38, tzinfo=tzutc())
+        assignment_time_mapping = None
+        for attr_name in (
+            'reviewer_assignment_timestamps_by_username',
+            'reviewer_assignment_timestamps',
+            'assignee_assignment_timestamps_by_username',
+            'assignee_assignment_timestamps'):
+            if hasattr(response, attr_name):
+                assignment_time_mapping = getattr(response, attr_name)
+                break
+        self.assertIsNotNone(
+            assignment_time_mapping,
+            'PullRequest should expose reviewer/assignee assignment times.')
+        self.assertEqual(set(assignment_time_mapping.keys()), expected_assignees)
+        for assignee in expected_assignees:
+            self.assertEqual(
+                assignment_time_mapping[assignee], expected_created_at)

Conversation

Yashbhu commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

U8NWXD left a comment

Choose a reason for hiding this comment

Uh oh!

Yashbhu commented May 4, 2026

Uh oh!

Yashbhu commented May 4, 2026

Uh oh!

U8NWXD commented May 8, 2026

Uh oh!

Yashbhu commented May 22, 2026

Uh oh!

Yashbhu commented May 22, 2026

Uh oh!

Yashbhu commented May 26, 2026

Uh oh!

U8NWXD commented Jun 3, 2026

Uh oh!

U8NWXD commented Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Yashbhu commented Apr 25, 2026 •

edited

Loading