Skip to content

pr_review.py reads a <summary> tag written inside a code span as a real section #1169

Description

@ptr727

scripts/pr_review.py reads <summary> tags out of a review body to identify collapsible section titles, and it does not exclude one written inside a Markdown code span. A reviewer that mentions the tag in prose therefore produces an unrecognized shape, and status then refuses to close the review loop on a review that is otherwise clean.

Observed

On #1168, a Copilot review body carried this bullet:

- Extend `pr_review` marker-shape floor coverage by adding the missing `<summary>`/summary-arm case to the "unknown marker" test.

status reported:

shapes=UNRECOGNIZED
  summary: `/summary-arm case to the unknown marker test. - Make the canonical-review symlink test skip
  (with an execution-boundary reason) on hosts that cannot create symlinks (notably Windows without
  privilege/Developer Mode). - Update local-strict-review refusal-table wording to avoid referencing
  rows by position; refresh derived skill distributions and the canonical-review ledger entry.
  </details> <details> <summary>File summaries  (round 37f7de3d)

The reader took the code-span <summary> as an opening tag and consumed everything to the next </summary>, swallowing the rest of the bullet list and running into the real File summaries section.

Reproduced in isolation

import pr_review
body = '''### Pull request overview

**Changes:**
- Extend `pr_review` marker-shape floor coverage by adding the missing `<summary>`/summary-arm case to the "unknown marker" test.
- Another bullet.

<details><summary>File summaries</summary>
</details>
'''
pr_review.unrecognized_in(body)
# ['heading: ### Pull request overview',
#  'summary: `/summary-arm case to the "unknown marker" test. - Another bullet. <details><summary>File summaries']

pr_review.unrecognized_in(body.replace("`<summary>`/summary-arm", "the summary-arm"))
# ['heading: ### Pull request overview']

The second call isolates the cause: with the tag out of the code span, the spurious summary shape disappears.

Why it is worth fixing rather than vetting

The vetting lists exist to make an unknown section loud. This is not an unknown section, it is prose that mentions a tag, so adding the swallowed text to VETTED_SUMMARIES would be wrong and would not generalize to the next reviewer that quotes the tag differently. Stripping code spans (and fenced blocks) before scanning for tags is the shape of the fix.

This is also self-inflicted in an amusing way: the review body that triggered it was describing a change that added a <summary> arm to this very script's own marker test, so any future work on that area is likely to reproduce it.

Effect today

status sets shapes=UNRECOGNIZED and refuses to close the loop, which is the check working as designed given what it saw. The consequence is that a clean review reads as unclosable until a person decides to merge anyway, which the digest correctly says is the maintainer's call.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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