PR #713 follow-ups: on-demand spawns log as a grace expiry, and the digest nudge has no batch clause nf-2fi ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/722 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer review of #713 (result=CONVERGED at 0acf8eb). Neither blocked that PR — both sit below the gate floor and are recorded here rather than cycling it. Unclaimed.

1 — dispatch.py: an explicitly-named PR logs as a grace expiry.

tools/reviewer/dispatch.py, decide() step 6b:

```python if not explicit and within_grace(state.activity, now, grace_min): return Decision("defer", ...) return Decision( "spawn", f"...coder activity ({activity}) newer than verdict ({verdict_ts}) " f"and past the {grace_min}m grace — reviewing anyway (no review was pulled)", ) ```

explicit correctly lifts the grace, but it then falls through to the message written for the OTHER way out of that branch. So the documented primary workflow — land the batch, reply on the PR, run ./tools/pr/review_now.py <n> — prints, seconds after the reply:

> 713: coder activity (07:18:26Z) newer than verdict (07:12:40Z) and past the 240m grace — reviewing anyway (no review was pulled)

Both clauses are false: the activity is minutes old, not past a 4h grace, and a review WAS pulled — that is what produced the run. review_now.py prints the dispatcher output above its own summary, so this is the first thing a session reads on the path #713 promotes, and it tells that session its pull did not register. A later reader debugging the grace against these lines would conclude it is not working.

test_an_EXPLICIT_pr_never_defers asserts action == "spawn" and does not look at the message, which is why it passes.

Shape: branch the message on which condition lifted the defer — explicit gets something like *named explicitly — reviewing now*; the grace expiry keeps the current wording. One assertion on the explicit message pins it.

Minor and separate: at --grace-min 0 every coder-activity spawn reads past the 0m grace ... (no review was pulled), which is the documented revert path describing itself oddly.

2 — poll.py: the nudge fires every tick with no "still mid-batch" clause.

tools/pr/poll.py, emit_tracked_prs() — the lines added for #718:

``` -> the reviewer has NOT seen this head. Waiting will not fix it: ./tools/pr/review_now.py <n> ```

It is right about waiting, and the phrasing was chosen deliberately to contradict it. But it is unconditional and repeats every 30 minutes, and it carries no counterpart to the *"Ask once per batch, not once per push"* rule in notes/reviewer.md. A session actively working a chunk sees it after every push, and a compliant session pulls a review on each tick.

That reproduces the waste #702 measured (36 of 76 reviews were re-reviews of an already-converged PR) through the on-demand door, where no grace guards it — explicit lifts the grace by design. The cron side is now rate-limited; the pull side is not, and the digest is the surface telling sessions to pull.

Shape: keep the line, add a half-sentence naming the batch rule (*"...once the batch is done"*), so the proximate instruction and the CLAUDE.md one agree. Suppressing it while the head is younger than some small window would also work, but the wording change is cheaper and adds no second clock.

Refs #702, #718, #586

Dependencies

None.

Comments

No comments.

Add a comment