Skip to content

Pair a Review Findings Entry With Its Thread Past the Line Suffix in pr_review.py #3181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ptr727 merged 2 commits into from
Oct 17, 2026
Merged

Pair a Review Findings Entry With Its Thread Past the Line Suffix in pr_review.py #3181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62f6a2a
Pair a Review Findings Entry With Its Thread Past the Line Suffix in …
ptr727 Oct 17, 2026
7e79e16
Pin the Line Suffix in the Narrative Reader and Drop a Test That Cann…
ptr727 Oct 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions scripts/pr_review.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2876,12 +2876,23 @@ def labelled_entries(body: str, label: re.Pattern[str]) -> list[str]:
return entries


LINE_SUFFIX = re.compile(r":[0-9]+(?:-[0-9]+)?$")


def entry_paths(entry: str) -> list[str]:
"""The paths one list entry names, read from its own code spans."""
"""The paths one list entry names, read from its own code spans.

A span written as `FILE:LINE` or `FILE:START-END` names the file, so the suffix is dropped.
A thread carries the path alone, which is what the entry's path is paired against.
"""
return [
named
for span in CODE_SPAN.finditer(entry)
if (named := bare_path(span.group()[len(span.group(1)) : -len(span.group(1))]).strip())
if (
named := LINE_SUFFIX.sub(
"", bare_path(span.group()[len(span.group(1)) : -len(span.group(1))]).strip()
)
)
]


Expand Down
9 changes: 8 additions & 1 deletion tests/test_pr_review.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5664,9 +5664,12 @@ def test_an_entry_on_a_file_no_thread_is_on_is_counted_where_the_totals_balance(
)

def test_every_entry_on_a_threaded_file_counts_none(self) -> None:
"""A trailing part, a zero-width space after a slash, and a directory each name a file."""
"""A trailing part, a zero-width space after a slash, a directory, and a path carrying a
line or a line range each name a file.
"""
for listed in (
PROSE_LISTED,
"- [P1] `lib/reader.py:340` drops an entry.\n- [P2] `writer.py:12-18` is stale.\n",
"- `lib/\u200breader.py` and `lib/writer.py`: one issue each (2 votes each).\n",
"- `lib/`: moderate retry issue (2 votes).\n",
"- A retry issue\n in `lib/reader.py` (2 votes).\n",
Expand Down Expand Up @@ -5863,6 +5866,10 @@ def test_entries_restating_threads_count_none(self) -> None:
self.assertEqual(0, self.count(with_narrative(listed)))
self.assertNotIn("FINDINGS WITH NO THREAD", self.digest(with_narrative(listed)))

def test_an_entry_naming_its_file_with_a_line_pairs_with_the_thread_on_it(self) -> None:
listed = "- `pkg/guard.py:12`: a failed read skips the check (**critical**, 1 vote).\n"
self.assertEqual(0, self.count(with_narrative(listed)))

def test_an_entry_stating_no_severity_this_reads_is_counted(self) -> None:
for listed in (
"- `pkg/guard.py`: a failed read skips the check (1 vote).\n",
Expand Down
Loading