Skip to content

copilot-review: suppressed comments are never named, so the highest-value finding can be the one that gets skipped #77

Description

@mmcky

Copilot sometimes marks a finding as suppressed. A suppressed comment appears only as text inside the review body, under a Suppressed comments (N) heading — it has no comment id, so it cannot be replied to via /replies and cannot be resolved from the UI.

Neither scripts/fetch-copilot.sh nor skills/copilot-review/SKILL.md mentions them. Confirmed: grep -i suppress over both returns nothing.

Why it matters

The skill's premise is "reply to each comment so the author can resolve them". Suppressed comments fall outside that loop entirely, and nothing says so. They reach the reader only incidentally, because the script dumps the review body verbatim, and a reader who has learned to work from the ## Inline comments section will skip straight past them.

The count line reinforces that. It reports inline comments only, so a review with three inline and one suppressed comment ends with (3 inline Copilot comment(s)) — which reads as the total.

This is not theoretical. On QuantEcon/project-monorepo#44 Copilot generated four findings: three inline, one suppressed. The suppressed one was the most valuable of the four, and the only one describing a live interaction between that PR and one merged an hour earlier: a missing field would have been read as "everything is closed", turning every card on a public page into a false contradiction. The three inline comments were an unescaped href, a duplicated list comprehension, and a missing link.

Once the three threads were resolved, that PR's conversation list showed three tidy resolved threads and no trace of the finding that mattered most.

What would fix it

Three things, in rough order of value.

Surface them as their own section. After ## Inline comments, add a ## Suppressed comments section parsed out of the review body, each one flagged as no thread — cannot be replied to or resolved. They are already in the text the script has; they just need lifting out of the blob.

Make the closing count honest. (3 inline + 1 suppressed) rather than (3 inline Copilot comment(s)), so a reader cannot mistake the inline count for the finding count.

Say what to do with them in SKILL.md. Step 2 should assess suppressed comments alongside inline ones, and step 4 should say that a suppressed comment cannot be threaded — so if it warrants a response, it goes in a PR comment (step 5), which is currently marked optional and is the only route available. Worth stating plainly that a suppressed comment is not a lower-severity comment; Copilot's own suppression heuristic is not a severity ranking.

Caveats for whoever picks this up

  • The parse is against Copilot's rendered markdown, not an API field, so it is inherently brittle. Failing open — printing the raw body when the heading is not found, as today — is better than dropping content.
  • Worth checking whether the heading is stable across review types, and whether a re-review can carry a different suppressed set. The script already prefers the most detailed review over the newest, which may or may not be the right choice for suppressed comments specifically.
  • The | line-prefix boundary should apply to suppressed comment text too — it is third-party content and untrusted in exactly the same way.

Related: #3 tracks the plugin's remaining work.

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

    enhancementImprovement to existing content or functionality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions