Fold Typographic Punctuation in pr_review.py reply --match

[ptr727]

Summary

A --match string copied verbatim from a rendered finding fails to select the thread it was copied from when the finding's typographic quotes, dashes, or ellipsis don't survive the copy in ASCII, and NO_MATCH reads identically to the thread being missing or already resolved.

• matching_threads() now folds both the --match pattern and each candidate thread body through an ASCII table (_TYPOGRAPHIC_FOLD) before the case-insensitive substring compare, covering the typographic single/double quotes, en/em dash, and ellipsis.
• NO_MATCH now states how many unresolved threads exist on the pull request, so a zero and a several no longer read the same, per the issue's secondary suggestion.
• A parameterized test checks each of the issue's seven characters independently of the fold table itself, so dropping one from the table still fails the test that covers it, rather than the test silently losing coverage along with the entry.

Closes on promotion: #1299

Two related gaps found during local review are filed separately as out of scope for this fix: #1876 (an ASCII -- and a real em dash still miss each other) and #1877 (a --match pattern copied from this script's own truncated/whitespace-collapsed unresolved: line can still miss).

Test plan

• python3 -m unittest scripts.tests.test_pr_review (434 tests, all passing)
• uvx ruff@latest format --check and uvx ruff@latest check clean on both changed files
• uvx pyright shows only the same 2 pre-existing errors present on develop before this change
• Reverted the fix locally and confirmed the new tests fail (NO_MATCH / missing count) without it, then restored it and re-confirmed green
• Local strict review (adversarial subagent pass) run and recorded via scripts/local_review.py

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5429365d-813b-4c42-997e-c4eb9387ec99

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Copilot review overview

🟢 Approval recommended

The functional change is small and well-covered by new tests, with only a minor test-robustness improvement suggested.

Review effort: Lite
Findings: 1

Open (1)

• Reset captured stdout for each subtest iteration · New

What changed in this PR

This pull request improves scripts/pr_review.py reply --match so it can reliably select threads even when the thread text or the copied --match pattern differs only by typographic punctuation (curly quotes, en/em dashes, ellipsis), and it makes NO_MATCH refusals more actionable by including the total unresolved-thread count.

Changes:

• Add a typographic-to-ASCII folding table and apply it to both the --match needle and candidate thread bodies before case-insensitive substring matching.
• Extend the NO_MATCH message to include the total number of unresolved threads on the PR.
• Add targeted tests covering the fold behavior per character and the improved NO_MATCH message.

File	Description
scripts/​pr_review.py	Folds typographic punctuation on both sides of --match comparisons; includes unresolved-thread count in NO_MATCH.
scripts/​tests/​test_pr_review.py	Adds regression tests for typographic folding and the updated NO_MATCH output.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ptr727]

Re: the stdout-buffer finding on scripts/tests/test_pr_review.py:5266 (thread already shows resolved, so answering here for the record rather than via reply --resolve, which only targets unresolved threads): fixed in 7818762. Each subTest iteration in test_every_documented_character_selects_across_the_ascii_boundary now clears the shared stdout buffer (self.out.seek(0); self.out.truncate(0)) before wiring and running its own reply, so its assertIn checks only its own iteration's output rather than one accumulated across all seven. Swept the file for the same setUp-captures-stdout-once-then-loops-subTests shape in every other test class (TestCoverageExitCodes, TestCli, CommentCase, ReplyCase) and found no other instance of the same latent bug.

[Copilot]

Copilot review overview

🟢 Approval recommended

The change is narrowly scoped, covered by new unit tests, and the updated matching behavior is applied symmetrically to both the pattern and candidate text.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Reset captured stdout for each subtest iteration