Skip to content

fixed-in-PR: a label for an issue whose fix is already written - #825

Open
lmoresi wants to merge 2 commits into
developmentfrom
tooling/fixed-in-pr-label
Open

lmoresi wants to merge 2 commits into
developmentfrom
tooling/fixed-in-pr-label

Conversation

@lmoresi

@lmoresi lmoresi commented Oct 7, 2026

Copy link
Copy Markdown
Member

Replaces #822, which named the label fixed-in-development. The rename is not cosmetic — it changed what the tool looks for, and the first framing was wrong about the common case.

Why not fixed-in-development

That name framed the problem as the gap between a merge and a release, so the tool scanned merged PRs only. It found one issue.

Scanning open PRs as well finds four more, and those are the ones that matter:

FIX ALREADY WRITTEN  (a PR declares it; `fixed-in-PR` not applied)
  #611   #656 (merged)              test_global_evaluate_after_migration hangs at np=4,
  #783   #785 (open)                Particle Lagrangian stress history: an inflow cell i
  #784   #785 (open)                Swarm.repopulate: after a removal the coordinate row
  #788   #789 (open)                Viscoelastic stress history in a units model: the pa
  #791   #794 (open)                The automatic cold-start 'Picard' warm-up (and solve

Four of five are in an unmerged PR. #785 carries the fix for #783 and #784 and cannot merge — its base feature/forward-parallel has no PR of its own, so the whole stack (#785 → #789 → #795 → #800) has nothing to merge into. fixed-in-development would have been wrong for every one of them.

What the label says, and what it does not

It says the fix is written. It does not say which PR, or whether that PR has merged — that lives in the issue and in scripts/triage.py, where it can be kept true. A label cannot be.

The merge-to-release half still holds: GitHub closes a linked issue on a default-branch merge, and that is main, which this project reaches infrequently. Worth recording that the mechanism is not broken — across the project's life 139 declared closes produced 5 that slipped. It is release-gated, and establishing that mattered before adding anything, because the backlog was not caused by it.

Three outcomes, not two

    fixed and you want it off the board   -> gh issue close <N>
    fixed, waiting on a release           -> gh issue edit <N> --add-label fixed-in-PR
    NOT fixed -- the PR addressed a neighbour, or papered over it
                                          -> leave open, say which part is live

#611 is the standing example of the third and the reason the tool reports candidates rather than fixes. #656 was credited with closing it; the hang is actually handled by scripts/test.sh:289, a --deselect at the np=4 pass — the rank count the issue reports hanging at. A close-or-label tool would have labelled it and the hang would have gone quiet for good.

Rolled out

Label created, and applied to #783, #784, #788 and #791, each with a line in the issue naming its PR and saying why it cannot land. #611 deliberately left bare.

Underworld development team with AI support from Claude Code

GitHub closes a linked issue when its PR reaches the DEFAULT branch. Here that
is `main`, while the work merges to `development`, and merges to `main` are
infrequent. So between a merge and a release an issue is fixed and still open,
and nothing in the repository says so.

The mechanism itself is sound: across the project's life 139 declared closes
produced 5 that slipped. It is release-gated, not broken, and it was worth
establishing that before adding anything -- the backlog was not caused by it.

`fixed-in-development` is applied at merge and the release closes everything
carrying it in one pass. Closing at merge stays a reasonable choice for a
defect nobody outside is waiting on; it is only misleading for one somebody is.

scripts/triage.py now lists every issue a merged PR declared that is neither
closed nor labelled, and names three outcomes rather than two. The third is
the reason the list is candidates and not fixes: #611 was credited to #656 and
its hang is handled by a `--deselect` in scripts/test.sh at the very rank
count the issue reports. The tool finds exactly that one today.

Underworld development team with AI support from Claude Code
… all

The label was named for the gap between a merge and a release, and that framing
made the tool scan merged PRs only. It found one issue. Scanning open PRs as
well finds four more, and those are the ones that matter: #785 carries the fix
for #783 and #784 and cannot merge, because its base branch has no PR of its
own. On the day the label was added, four of the five issues it applied to were
in that state.

So the label says the fix is written, and nothing more. Which PR, and whether
it has merged, lives in the issue and in this tool's output, where it can be
kept true -- a label cannot be.

The merge-to-release half still holds: GitHub closes a linked issue on a
DEFAULT-branch merge and that is `main`, reached infrequently here. 139
declared closes across the project's life produced 5 that slipped, so the
mechanism is release-gated rather than broken, and it was worth establishing
that before adding anything.

Underworld development team with AI support from Claude Code
Copilot AI balanced review requested due to automatic review settings October 7, 2026 08:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The base-branch filter causes valid fixes in merged stacked PRs to disappear from triage output.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds fixed-in-PR tracking for open issues whose fixes exist but remain unreleased.

Changes:

  • Reports unlabelled issues declared fixed by open or merged PRs.
  • Documents the label’s workflow and meaning.
  • One bug remains: merged stacked PRs targeting non-development branches are excluded.
File Description
scripts/​triage.py Adds detection and reporting for written fixes.
docs/​developer/​guides/​adversarial-review.md Documents the new issue-triage label.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/triage.py
Comment on lines +242 to +245
for p in merged:
if p.get("baseRefName") not in (None, BASE):
continue
for n in {int(m) for m in CLOSES.findall(prose(p["body"]))}:

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants