Skip to content

ci(batching-gate): a PR that closes nothing is told it "delivers exactly one issue", then told the opposite two lines later #16855

Closed
Closed
ci(batching-gate): a PR that closes nothing is told it "delivers exactly one issue", then told the opposite two lines later#16855
Labels
bugSomething isn't workinginfrastructureneeds-decisionBlocked on an owner decision; options and a recommendation are on the issue

Description

@mrveiss
mrveiss
opened on Sep 17, 2026

Problem

scripts/check_pr_issue_batching.py fails a PR that links an issue only with a mention keyword, and the failure text contradicts itself.

For a body containing Refs #16803 and no rationale line, check() (:232-244) computes closing = {} and referenced = {"16803"}, falls through to _rationale_failure(body, closing, referenced) (:244), and prints RATIONALE_HINT (:86-93) followed by _mention_note (:158-174):

This PR delivers exactly one issue. Batch same-scope issues into one PR
(one CI suite per batch, not per issue), or state why this one stands alone
by adding a line to the PR body:
...
#16803 is linked with a non-closing keyword (refs/references/part of), which
is context -- an umbrella, a follow-up, a dependency -- not a second delivered
issue. Only resolves/closes/fixes count toward batching.

The first sentence says the PR delivers one issue. The last paragraph explains that the only issue it names is not a delivered issue. Both are printed together, so the true count — zero — is stated nowhere, and the author is asked to justify why a single issue "stands alone" when no issue is being delivered at all.

This is the same class as #16793 and #16779: the gate is right about the state of the world and wrong about what it tells you, so the cost lands on whoever reads it.

Evidence

  • scripts/check_pr_issue_batching.py:86-93 — RATIONALE_HINT, a fixed string opening "This PR delivers exactly one issue.", emitted for every rationale failure regardless of the closing count.
  • scripts/check_pr_issue_batching.py:185-187 — _rationale_failure returns RATIONALE_HINT + _mention_note(...) without consulting closing.
  • scripts/check_pr_issue_batching.py:247-256 — _scope() already has the correct phrasing for this exact state: if not closing: return f"Closes nothing; references {_render(referenced)}". It is used only on the passing path, so the accurate wording exists and the failing path does not reach it.
  • Observed on docs(roadmap): replace the December 2025 snapshot with measured current counts #16834 (Refs #16803, a docs PR delivering no issue), whose Check same-scope batching run failed on this.

Note the run on #16834 predates b00a92756/ff2ade1fc and printed the older "references exactly one issue". The defect described here is against current origin/main and is not fixed by those commits — I re-read the merged source rather than assuming the rename covered it.

Acceptance criteria

  • When closing is empty, the failure names the real count — the PR closes nothing and links #N as context — instead of asserting it delivers one issue.
  • The batching ask is not put to a PR that closes nothing, or is phrased so the author can see which rule applies to them. Whether such a PR should need a rationale at all is the open question below.
  • A test asserts the closes-nothing failure text and the closes-one failure text differ, so the two cannot be collapsed again — the same guard shape ci(pr-template): a missing heading is reported as an empty section, so a complete PR body fails with a misleading error #16793 used.
  • No change to which PRs pass or fail unless the question below is answered "no".

Open question for the owner

Should a PR that closes nothing be required to give a single-issue rationale at all? Batching is about not spreading closing keywords across PRs; a docs or chore PR that merely references an umbrella delivers no issue and has nothing to batch with. Current behaviour requires the rationale line anyway.

Both readings are defensible and it changes which PRs go red, so I have not picked one:

  • Keep requiring it — cheap, and it makes the author state why the work is not riding with a delivery. Then only the wording above needs fixing.
  • Exempt closes-nothing PRs — the rule stops applying where it has no subject. Then check() returns early when closing is empty and referenced is not.

Happy to implement either; I did not want to widen or narrow a merge gate on my own judgement.

Related

Gate introduced by #16795 (b00a92756), messaging sibling #16793 (ff2ade1fc). Surfaced while sweeping #16834.

🤖 Generated with Claude Code

Activity

  1. mrveiss
    added
    bugSomething isn't working
    on Sep 17, 2026
  2. github-actions
    added
    infrastructure
    on Sep 17, 2026
  3. mrveiss
    added a commit that references this issue on Sep 17, 2026

    Merge pull request #16856 from mrveiss/issue-16855-batching-count

    b9addfd

  4. mrveiss commented on Sep 17, 2026

    @mrveiss
    mrveiss
    on Sep 17, 2026
    OwnerAuthor

    The wording half landed in PR #16856, merged to main. The policy half stays open — this issue is not closed by that merge.

    What changed: a PR whose body links an issue only with a mention keyword (Refs #N) used to fail with RATIONALE_HINT, which opens "This PR delivers exactly one issue" — a count of one where the count is zero — with _mention_note appended underneath saying the only issue named is not a delivered one. The output asserted both at once, so the true number appeared nowhere. _closes_nothing_hint now states it plainly: the PR closes no issue, only resolves/closes/fixes count toward batching, and there is nothing here to batch with.

    What is still undecided, and why it was deliberately not settled in that PR: should a PR that closes nothing need a single-issue rationale at all? The gate currently requires one, and #16856 changed only the wording, not the verdict — so the behaviour is unchanged for every PR. The tests pin that on purpose: check(ONE)[1] == RATIONALE_HINT asserts the one-issue path is byte-identical to before, and closes_nothing != closes_one asserts only that the two states now read differently.

    Two ways to go:

    1. Keep requiring it. A PR closing nothing is still a CI run someone pays for; the rationale line is where the author says why it could not ride with delivered work. Costs nothing but a line.
    2. Exempt it. Batching is about not spending N CI runs on N issues. A PR delivering zero issues has no sibling to batch with by definition, so the rule has no work to do there and the required line is ceremony.

    I lean (1), weakly, and only because the exemption creates an incentive to drop a Closes keyword to skip a gate. That is a guess about author behaviour, not a measurement — I have not counted how many closes-nothing PRs actually exist, so treat the lean as worth exactly that.

    Labelled needs-decision. Whoever rules, the change is a few lines in _rationale_failure either way.

  5. mrveiss
    added
    needs-decisionBlocked on an owner decision; options and a recommendation are on the issue
    on Sep 17, 2026

  6. mrveiss commented on Oct 3, 2026

    @mrveiss
    mrveiss
    on Oct 3, 2026
    OwnerAuthor

    Stale headline, live criteria — the advisory was fixed; AC4 waits on the question in the body

    The title's claim — a PR that closes nothing is told it "delivered" one issue — is no longer true on main. Fixed by 03316b6be8, "fix(ci): the batching gate stops claiming one issue where it counts zero (#16855)", with no PR link, which is why it never showed up as closeable.

    AC1 — when closing is empty, the failure names the real count. Met. scripts/check_pr_issue_batching.py:168 is a dedicated branch whose docstring is this issue: "The failure for a PR that closes nothing (#16855)." :171-172 records what the collapsed message used to say — "one issue delivered, and the only issue named not a delivered one" — i.e. the contradiction this issue was filed about, now a separate path.

    AC3 — a test asserts the two failure texts differ, so they cannot be collapsed again. Met. scripts/check_pr_issue_batching_test.py:387 test_the_advisory_says_the_pr_closes_nothing.

    AC4 — "No change to which PRs pass or fail unless the question below is answered 'no'." This criterion is conditional on an unanswered question in this issue's own body, so it cannot be verified from code at all — it is a decision, not an implementation. That is what keeps this issue live, and it is also why it should not be closed on the two criteria above.

    AC2 — whether the batching ask is withheld from, or re-phrased for, a PR that closes nothing — depends on the same answer.

    Remainder: the design question in the body. Someone with the authority to answer it should, after which AC2 and AC4 are a small change or a no-op. Flagging rather than guessing the answer; the code half is done.

  7. mrveiss commented on Oct 4, 2026

    @mrveiss
    mrveiss
    on Oct 4, 2026
    OwnerAuthor

    Closing — all four criteria verified against merged main at baec13de40.

    AC1 — a PR that closes nothing gets a hint that says so. ✅ scripts/check_pr_issue_batching.py:315-316, inside _rationale_failure and reached from _verdict:463:

    if not closing:
    return _closes_nothing_hint(referenced)

    with the text at :180-187. The contradiction this issue was filed for — being told a PR "delivers exactly one issue" and then the opposite — cannot occur, because the two branches now produce different text.

    AC2 — the hint names the rule rather than just the symptom. ✅ the same text states that only resolves/closes/fixes count toward batching, and that a rationale line is still required while the rule stands. A reader who hits it learns why, not only that.

    AC3 — the two advisory texts are pinned as distinct. ✅ check_pr_issue_batching_test.py:396 test_the_two_advisory_texts_are_not_the_same, plus :382, :387, :406. That assertion is the one that stops the two messages drifting back together, which is how the defect arose.

    AC4 — a closing PR's behaviour is unchanged. ✅ :406 test_a_closing_pr_still_gets_the_unchanged_hint pins no verdict change for the existing path. The open question this AC recorded was later settled by #17128 (closed), and that resolution is documented at check_pr_issue_batching_test.py:416-440.

  8. mrveiss
    closed this as completedon Oct 4, 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

    bugSomething isn't workinginfrastructureneeds-decisionBlocked on an owner decision; options and a recommendation are on the issue

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions