Skip to content

Decide Whether a Note-Stripped File-Table Row Counts as Coverage on Its Own (auto-2251) #2522

Closed

Description

@ptr727

Question

Should a Copilot file-table row that names a changed path only once its trailing parenthesized note is dropped count as covering that path, or only stop counting as a row that names a path the diff does not carry?

This comes from handoff #2375 (track auto-2251, fixing #2251), branch feature/auto-2251, pushed at fc94dcfa, with no pull request open yet. The branch carries the answer to #2376 and current develop merged in.

The second local-strict-review pass after the #2376 edit round raised two findings, both classed introduced. The two-round edit budget for this push is spent, so editing stopped. The counts per class are 2 introduced, 0 pre-existing, and 0 style.

  1. A note-stripped row counts as coverage on its own. row_paths reads a row again without its note while the row names nothing, and the path it then names joins the covered set. So with docs/a.md and docs/b.md changed, a table of docs/a.md and docs/b.md (not reviewed) reads as coverage=table and exits 0, where it exited 45 before. The note's own text is never read. Both shapes in pr_review.py Reads a Copilot File-Table Cell Naming Two Paths as One, Exiting 45 on Full Coverage #2251 put the noted row beside a bare row for the same file, so neither needs a noted row to add coverage. Whether Copilot writes a qualifying note like that has not been measured, and the failure, where it happens, lets a merge through silently.
  2. The split-cell wording is broader than the code. The scripts/README.md sentence and the cell_paths docstring say a cell of code spans separated by commas is read one row per span, while TABLE_SPAN matches single-backtick spans only. A cell such as ``docs/a.md``, ``docs/b.md`` is read whole and refuses, which is the safe direction.

An earlier pass's finding, that a split cell carrying a note (`docs/a.md`, `docs/b.md` (cleanup)) refuses, stays declined: the shape is unobserved, it refuses, and admitting notes into the split grammar would let a span inside a note count as covering a path.

Options

  1. Recommended: narrow the note reading so a note-stripped row only stops refusing, and say "single-backtick". A row that names a path only after its note comes off is no longer reported as naming a path the diff does not carry, and it adds nothing to the covered set, so only a bare row covers. Both tables in pr_review.py Reads a Copilot File-Table Cell Naming Two Paths as One, Exiting 45 on Full Coverage #2251 still stand in. A table whose only row for a file carries a note refuses, which sends that pull request to the maintainer. test_a_trailing_note_is_read_past_on_every_row_form is rewritten for that, and the README sentence and the row_paths docstring change with it. This is one more edit round and one more local pass, then the pull request into develop.
  2. Keep the note reading as it stands, decline finding 1, and fix the wording only. The decline would rest on pr_review.py Reads a Copilot File-Table Cell Naming Two Paths as One, Exiting 45 on Full Coverage #2251's comment, which asks that a path followed by an annotation count as naming that path, and on the shape being unmeasured. A table whose only row for a file carries a note keeps reading as coverage. The edit is the single-backtick wording, then one local pass and the pull request.
  3. Open the pull request as it stands and let the hosted reviewers judge both. This spends a review round on findings a local pass already raised, and finding 1 would still need this decision if a hosted reviewer raises it too.

Blocks

Handoff #2375 and branch feature/auto-2251. No pull request is open yet, so no pull request is blocked beyond the one this lane would open, and no develop -> main promotion pull request is blocked by it.

Activity

  1. added
    decisionA question waiting on the maintainer, alone or beside whatever else the issue carries
    on Oct 8, 2026
  2. ptr727 commented on Oct 8, 2026

    @ptr727
    OwnerAuthor

    Answered

    The maintainer chose option 1: narrow the note reading so a note-stripped row only stops refusing and adds nothing to the covered set, so only a bare row covers a path, and say "single-backtick" in the split-cell wording. The reason given with the recommendation: the other reading lets an unreviewed path pass the merge gate silently, which is the failure class "Trust Boundaries and Hardening Effort" says to guard, while this one fails loud by sending the pull request to the maintainer.

    Lane auto-2251 is handed back to the unattended loop with this answer in its next steps.

  3. removed
    decisionA question waiting on the maintainer, alone or beside whatever else the issue carries
    on Oct 8, 2026
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