diff --git a/backend/tests/test_backlog_rhythm.py b/backend/tests/test_backlog_rhythm.py index c6bd839c..44a80341 100644 --- a/backend/tests/test_backlog_rhythm.py +++ b/backend/tests/test_backlog_rhythm.py @@ -158,6 +158,39 @@ def test_nudge_at_threshold_then_park(tmp_path: Path) -> None: assert "park" in parked and "nudge_reporter" not in parked +def test_an_open_pr_suppresses_the_chase_and_the_park(tmp_path: Path) -> None: + """Parking an item that has an open PR OSCILLATES: `park` says move the + card to Backlog, the column derivation says an item with a PR is In Review, + so the next pass reports `move_card` to put it back and the pass after that + parks it again. #162 did exactly that — parked at 55 quiet days, reported + as a mis-placed card, restored, parked again. + + The rule is also right on its own terms: an issue with a PR in flight is + not a reporter chase whatever its Awaiting says, and the wait that matters + is the review, which the PR half already reports. + """ + for days, unwanted in ((28, "park"), (14, "nudge_reporter")): + item = _item( + 162, + labels=["b"], + awaiting="reporter", + pr=167, + column="In Review", + board_status="In Review", + last_comment=_comment(days), + ) + actions = _actions_for(_run(tmp_path, [item]), 162) + assert unwanted not in actions, days + # ...and no move_card either, so there is nothing to oscillate against. + assert "move_card" not in actions, days + + +def test_the_chase_still_fires_without_a_pr(tmp_path: Path) -> None: + """The suppression must not swallow the case the rule exists for.""" + item = _item(163, labels=["b"], awaiting="reporter", last_comment=_comment(28)) + assert "park" in _actions_for(_run(tmp_path, [item]), 163) + + def test_below_threshold_does_not_chase(tmp_path: Path) -> None: item = _item(4, labels=["b"], awaiting="reporter", last_comment=_comment(13)) assert _actions_for(_run(tmp_path, [item]), 4) == set() diff --git a/scripts/backlog-rhythm.sh b/scripts/backlog-rhythm.sh index cb0a17d5..a8bcb22d 100755 --- a/scripts/backlog-rhythm.sh +++ b/scripts/backlog-rhythm.sh @@ -93,7 +93,21 @@ actions=$(printf '%s' "$digest" | jq \ # Chase once, then park. `is_reporter` false means the last word was ours, # so the ball is still with them. - (if .awaiting == "reporter" + # + # AN OPEN PR SUPPRESSES BOTH, and that is not a nicety -- parking one + # OSCILLATES. `park` says move the card to Backlog; the column derivation + # says an item with an open PR is In Review; so the next pass reports + # `move_card` to put it back, and the pass after that parks it again, + # forever. #162 did exactly this: parked at 55 quiet days, immediately + # reported as a mis-placed card, restored, parked again. + # + # The rule is right underneath the loop, too. An issue with a PR in flight + # is not a reporter chase whatever its `Awaiting` says: the work exists, + # and the wait that matters is the PR review, which the PR half of this + # pass already reports. Nudging the reporter of a 46-day-stale conflicted + # PR asks the wrong person about the wrong thing. + (if .pr != null then empty + elif .awaiting == "reporter" and ((.last_comment.is_reporter // false) | not) and quiet_days >= $park then {issue: .number, action: "park",