Skip to content

pr_review.py compares vetted reviewer markers case-sensitively, so a case drift reads as an unknown shape #1132

Description

@ptr727

The defect

unrecognized_in() compares each marker in a review body against VETTED_HEADINGS, VETTED_SUMMARIES and VETTED_LABELS. normal() reduces a marker to ASCII, collapses whitespace and rewrites a count as (N), but does not fold letter case. So a heading that drifts by one letter's case reads as a shape the script has never seen, even when that exact section is already in the vetted list.

Measured on PR #1125, Copilot review 5061995517:

unrecognized as shipped: ['heading: ### Reviewed Changes']
vetted list entry      : ['### Reviewed changes']
heading in the body    : ['### Reviewed Changes']
still unrecognized with a casefold: []

The section is ### Reviewed changes, the fifth entry in VETTED_HEADINGS. The only difference is the C.

Why this is worse than an unknown section

Every actual reader in the script is already case-insensitive: SUPPRESSED, CR_OUTSIDE_DIFF and REFUSAL all carry re.IGNORECASE. On the round above, the coverage reader found the file table and reported coverage=full correctly. So the review was read correctly and only the vetted-list comparison failed, while status reported shapes=UNRECOGNIZED with the standing wording that "nothing it reports about this review is trustworthy".

That is the gate-fails-loud design working as intended against a false input, and the cost is the expensive one: an unrecognized shape blocks the review loop by rule, so a clean round does not close and the maintainer is asked for an override that nothing actually warranted. A vetting list whose entries stop matching for a reason that carries no meaning is the "silently narrows" failure GOVERNANCE.md "Verification Discipline" names, arriving loudly instead of quietly, which is better but still wrong.

Suggested fix

Compare folded and display raw: keep normal() returning the marker as written, since that string is what the report quotes and what a reader greps for, and fold only at the membership test in unrecognized_in(), for all three lists rather than headings alone. Letter case carries no meaning in any of these markers, so folding cannot mask a genuinely different section.

A regression case belongs with it, asserting that a vetted marker with its case altered still reads as recognized, and that a genuinely unknown marker still reports. Per GOVERNANCE.md "a gate that finds nothing is indistinguishable from a gate with nothing to find", the second half is what keeps the first from turning the check off.

Decision this needs

Whether to fix this before closing PR #1125's review loop, or to override the block there and fix it separately. The block on #1125 is spurious by the measurement above, so an override would be correct in substance, but the loop closing on its own after the fix is the outcome that leaves no judgement call on record.

History

Filed originally as "no reader for Copilot's ### Reviewed Changes heading", which was wrong: the script has a reader and a vetted entry for that section. Rewritten once the cause was measured rather than inferred.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions