Skip to content

Commit 0620dd9

Browse files
johanzanderclaude
andauthored
fix: an open PR suppresses the reporter chase, which was oscillating (#640)
Parking an item that has an open PR does not settle it -- it 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 and it was watched happen: parked at 55 quiet days, immediately reported as a mis-placed card, restored, reported as needing a park again. That is the "same conversation every tick" failure the board work exists to end, reappearing between two rules inside this script rather than between the script and the maintainer. The rule is also right on its own terms, independent of the loop. An issue with a PR in flight is not a reporter chase whatever its `Awaiting` field says: the work exists, and the wait that matters is the 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. So `.pr != null` suppresses both `park` and `nudge_reporter`. The suppression does not reach the case the rule exists for: an issue with no PR still parks at 28 quiet days, pinned by `test_the_chase_still_fires_without_a_pr`, which passes both before and after the change. Verified the oscillation test fails against the pre-fix script and passes after. Live effect: 23 actions -> 22, with no `park`/`move_card` pair left to trade #162 back and forth. Claude-Session: https://claude.ai/code/session_012LExo6fcbup75vtc9NfoAR Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 47d3c48 commit 0620dd9

2 files changed

Lines changed: 48 additions & 1 deletion

File tree

backend/tests/test_backlog_rhythm.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,39 @@ def test_nudge_at_threshold_then_park(tmp_path: Path) -> None:
158158
assert "park" in parked and "nudge_reporter" not in parked
159159

160160

161+
def test_an_open_pr_suppresses_the_chase_and_the_park(tmp_path: Path) -> None:
162+
"""Parking an item that has an open PR OSCILLATES: `park` says move the
163+
card to Backlog, the column derivation says an item with a PR is In Review,
164+
so the next pass reports `move_card` to put it back and the pass after that
165+
parks it again. #162 did exactly that — parked at 55 quiet days, reported
166+
as a mis-placed card, restored, parked again.
167+
168+
The rule is also right on its own terms: an issue with a PR in flight is
169+
not a reporter chase whatever its Awaiting says, and the wait that matters
170+
is the review, which the PR half already reports.
171+
"""
172+
for days, unwanted in ((28, "park"), (14, "nudge_reporter")):
173+
item = _item(
174+
162,
175+
labels=["b"],
176+
awaiting="reporter",
177+
pr=167,
178+
column="In Review",
179+
board_status="In Review",
180+
last_comment=_comment(days),
181+
)
182+
actions = _actions_for(_run(tmp_path, [item]), 162)
183+
assert unwanted not in actions, days
184+
# ...and no move_card either, so there is nothing to oscillate against.
185+
assert "move_card" not in actions, days
186+
187+
188+
def test_the_chase_still_fires_without_a_pr(tmp_path: Path) -> None:
189+
"""The suppression must not swallow the case the rule exists for."""
190+
item = _item(163, labels=["b"], awaiting="reporter", last_comment=_comment(28))
191+
assert "park" in _actions_for(_run(tmp_path, [item]), 163)
192+
193+
161194
def test_below_threshold_does_not_chase(tmp_path: Path) -> None:
162195
item = _item(4, labels=["b"], awaiting="reporter", last_comment=_comment(13))
163196
assert _actions_for(_run(tmp_path, [item]), 4) == set()

scripts/backlog-rhythm.sh

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,21 @@ actions=$(printf '%s' "$digest" | jq \
9393
9494
# Chase once, then park. `is_reporter` false means the last word was ours,
9595
# so the ball is still with them.
96-
(if .awaiting == "reporter"
96+
#
97+
# AN OPEN PR SUPPRESSES BOTH, and that is not a nicety -- parking one
98+
# OSCILLATES. `park` says move the card to Backlog; the column derivation
99+
# says an item with an open PR is In Review; so the next pass reports
100+
# `move_card` to put it back, and the pass after that parks it again,
101+
# forever. #162 did exactly this: parked at 55 quiet days, immediately
102+
# reported as a mis-placed card, restored, parked again.
103+
#
104+
# The rule is right underneath the loop, too. An issue with a PR in flight
105+
# is not a reporter chase whatever its `Awaiting` says: the work exists,
106+
# and the wait that matters is the PR review, which the PR half of this
107+
# pass already reports. Nudging the reporter of a 46-day-stale conflicted
108+
# PR asks the wrong person about the wrong thing.
109+
(if .pr != null then empty
110+
elif .awaiting == "reporter"
97111
and ((.last_comment.is_reporter // false) | not)
98112
and quiet_days >= $park
99113
then {issue: .number, action: "park",

0 commit comments

Comments
 (0)