Summary

Add typing to Review Bot's resources.

Review Request #14710 — Created Dec. 1, 2025 and submitted March 18, 2026, 7:23 p.m.

Information

Owner

maubin

Repository

ReviewBot

Branch

release-4.x

Bugs

Depends On

Reviewers

Groups

reviewbot

People

Description

This adds typing to Review Bot's resources.

Testing Done

Used in an upcoming change.

Commits

Summary	ID
Add typing to Review Bot's resources. This adds typing to Review Bot's resources.	f2fabb06fb3394d54173527242669209340d9a37

Issues

Description	From	Last Updated
While here, can you update the name to use our standard naming? _normalize_comments_json?	chipx86	March 17, 2026, 9:57 a.m.
It might be nice to define a TypedDict that documents/enforces the interface on this.	david	March 17, 2026, 9:57 a.m.
Since this is inside if TYPE_CHECKING it should be fine to import without the try/except.	david	March 17, 2026, 10:26 p.m.
_normalize_comments_json doesn't treat this as a required key. We probably should make this be NotRequired[...]	david	March 17, 2026, 11:32 p.m.
This should be HttpRequest, not ReviewRequest	david	March 17, 2026, 10:36 p.m.
While here let's switch this to "Create"	david	March 17, 2026, 10:45 p.m.
This should be fixed up too.	david	March 17, 2026, 10:37 p.m.
diff_comments_norm here is a Sequence[DiffCommentData], which implies immutability, but the pop is mutating the dicts inside the sequence. The pop …	david	March 17, 2026, 11:27 p.m.
Since extra_keys is typed as a sequence and base_comment_keys will likely get typed as a list, this will flag in …	david	March 17, 2026, 11:30 p.m.
'collections.abc.Sequence' imported but unused Column: 5 Error code: F401	reviewbot	March 17, 2026, 11:47 p.m.
I think we can use Mapping here, if we're not modifying this anywhere.	chipx86	March 18, 2026, 1:31 a.m.
These can probably be Sequence, unless we're modifying them or passing them somewhere that requires a list.	chipx86	March 18, 2026, 1:31 a.m.

flake8 passed.

JSHint passed.

extension/reviewbotext/resources.py (Diff revision 1)
The issue has been resolved. Show all issues
```
It might be nice to define a TypedDict that documents/enforces the interface on this.
```

extension/reviewbotext/resources.py (Diff revision 1)

The issue has been resolved. Show all issues

While here, can you update the name to use our standard naming? _normalize_comments_json?

Change Summary:


Moved to release 4.1.
Added typed dicts for normalized comment data.
Fixed the comment normalization method name.

Commits:

	Summary	ID
	Add typing to Review Bot's resources. This adds typing to Review Bot's resources.	43a9d98a4a12bedeb2e5209ee4ba467e5055a635
	Add typing to Review Bot's resources. This adds typing to Review Bot's resources.	6c19fb57b52f0465e68705b5911db76998fc2d46

Diff:

Revision 2 (+256 -62)

Show changes

extension/reviewbotext/resources.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

extension/reviewbotext/resources.py (Diff revision 2)

The issue has been resolved. Show all issues

Since this is inside if TYPE_CHECKING it should be fine to import without the try/except.

extension/reviewbotext/resources.py (Diff revision 2)

The issue has been dropped. Show all issues

_normalize_comments_json doesn't treat this as a required key. We probably should make this be NotRequired[...]

maubin March 17, 2026, 11:45 p.m.

It's not one of the expected keys from the JSON payload but we do always set it based on issue_opened, so it'll always appear in the comment data returned from _normalize_comments_json.

extension/reviewbotext/resources.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This should be HttpRequest, not ReviewRequest
```
extension/reviewbotext/resources.py (Diff revision 2)
The issue has been resolved. Show all issues
```
While here let's switch this to "Create"
```
extension/reviewbotext/resources.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This should be fixed up too.
```

extension/reviewbotext/resources.py (Diff revision 2)

The issue has been resolved. Show all issues

diff_comments_norm here is a Sequence[DiffCommentData], which implies immutability, but the pop is mutating the dicts inside the sequence. The pop also makes it no longer satisfy the DiffCommentData interface.

extension/reviewbotext/resources.py (Diff revision 2)

The issue has been resolved. Show all issues

Since extra_keys is typed as a sequence and base_comment_keys will likely get typed as a list, this will flag in some type checkers. We probably should do expected_keys = set(base_comment_keys) | set(extra_keys)

maubin March 17, 2026, 11:45 p.m.

I decided to just make everything sets from the get go.

Commits:

	Summary	ID
	Add typing to Review Bot's resources. This adds typing to Review Bot's resources.	6c19fb57b52f0465e68705b5911db76998fc2d46
	Add typing to Review Bot's resources. This adds typing to Review Bot's resources.	caf43a925ac2dd45f14a6f7af57948134e0dfdf2

Diff:

Revision 3 (+284 -80)

Show changes

extension/reviewbotext/resources.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

extension/reviewbotext/resources.py (Diff revision 3)
The issue has been resolved. Show all issues
```
'collections.abc.Sequence' imported but unused

Column: 5
Error code: F401
```

extension/reviewbotext/resources.py (Diff revision 3)
The issue has been resolved. Show all issues
```
I think we can use Mapping here, if we're not modifying this anywhere.
```

extension/reviewbotext/resources.py (Diff revision 3)

The issue has been dropped. Show all issues

These can probably be Sequence, unless we're modifying them or passing them somewhere that requires a list.

maubin March 18, 2026, 1:32 a.m.

We do modify them so they need to be a list.

Commits:

	Summary	ID
	Add typing to Review Bot's resources. This adds typing to Review Bot's resources.	caf43a925ac2dd45f14a6f7af57948134e0dfdf2
	Add typing to Review Bot's resources. This adds typing to Review Bot's resources.	f2fabb06fb3394d54173527242669209340d9a37

Diff:

Revision 4 (+282 -80)

Show changes

extension/reviewbotext/resources.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-4.x (b8ea697)