feat: opt-in cadence to skip re-reviewing unchanged pull requests#370
feat: opt-in cadence to skip re-reviewing unchanged pull requests#370saariuslystoned wants to merge 2 commits into
Conversation
We run ClawSweeper's review brain on a metered subscription and noticed open pull requests get re-reviewed about once a day even when nothing changed since the last review, reproducing an identical verdict and re-spending review budget each time. reviewCadenceMs already drops to hourly when there is activity since the last review and uses the daily hot-window cadence for new pull requests, but a pull request past its hot window with no activity still falls to the daily branch. This adds an optional CLAWSWEEPER_STALE_PULL_REQUEST_REVIEW_DAYS that extends only that branch (for example 7 for weekly). Unset, non-numeric, or sub-daily values keep the current daily behavior, and the hourly-activity and daily hot-window branches are untouched, so changed and brand-new pull requests are still reviewed promptly. Behavior is unchanged unless an operator opts in. - src: stalePullRequestReviewCadenceMs() helper, read in reviewCadenceMs - test: scheduler-policy cadence cases (default daily, extended weekly, invalid/sub-daily fallback) through the exported shouldReviewItem - docs: scheduler.md cadence section Release note (CHANGELOG.md left untouched -- it is release-owned per AGENTS.md): adds optional CLAWSWEEPER_STALE_PULL_REQUEST_REVIEW_DAYS to extend the re-review cadence for open PRs past their hot window with no activity since last review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 1:07 PM ET / 17:07 UTC. Summary Reproducibility: yes. Source inspection shows that with a weekly stale-PR cadence, Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Make the configured stale-PR cadence apply consistently to due selection, active-floor current-review backfill, dashboard cadence math, and docs/tests, unless maintainers explicitly choose and document a narrower due-only setting. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows that with a weekly stale-PR cadence, Is this the best way to solve the issue? No. The helper and workflow wiring are the right narrow shape, but the implementation should either apply the same cadence to active-floor backfill or explicitly narrow the documented behavior with tests. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 323f7bdf7236. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
Addresses ClawSweeper's review of openclaw#370. The new CLAWSWEEPER_STALE_PULL_REQUEST_REVIEW_DAYS was read in reviewCadenceMs, but the scheduled planner in .github/workflows/sweep.yml passes an explicit env allowlist to `pnpm run plan`, so setting the documented repository variable would not reach production scheduled runs -- stale PRs would keep the daily cadence after opt-in. Per AGENTS.md, scheduler tuning must update both src/clawsweeper.ts and .github/workflows/sweep.yml. - sweep.yml: pass vars.CLAWSWEEPER_STALE_PULL_REQUEST_REVIEW_DAYS into the planner `select` step (the only cadence-selection path -- review shards run exact --item-numbers from the plan matrix) and into the dashboard `audit --update-dashboard` step. - clawsweeper.ts: cadenceBucketForReview reuses stalePullRequestReviewCadenceMs so the dashboard "current within cadence" math matches what the planner schedules. - docs/scheduler.md: document the repository-variable wiring. Unset/non-numeric/<= daily keeps the daily default everywhere, so behavior is unchanged unless an operator opts in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the review! Addressed the workflow-wiring finding: CLAWSWEEPER_STALE_PULL_REQUEST_REVIEW_DAYS is now wired into .github/workflows/sweep.yml (the scheduled planner select step + the Real behavior proof (built dist/, Node 24): node --test test/scheduler-policy.test.ts → 13 tests, pass 13, fail 0. Cadence flip via shouldReviewItem from dist/, stale PR with no activity: @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
We run ClawSweeper's review brain on a metered subscription and noticed open pull requests get re-reviewed about once a day even when nothing changed since the last review, reproducing an identical verdict and re-spending review budget each time.
reviewCadenceMs already drops to hourly when there is activity since the last review and uses the daily hot-window cadence for new pull requests, but a pull request past its hot window with no activity still falls to the daily branch. This adds an optional CLAWSWEEPER_STALE_PULL_REQUEST_REVIEW_DAYS that extends only that branch (for example 7 for weekly). Unset, non-numeric, or sub-daily values keep the current daily behavior, and the hourly-activity and daily hot-window branches are untouched, so changed and brand-new pull requests are still reviewed promptly. Behavior is unchanged unless an operator opts in.
Release note (CHANGELOG.md left untouched -- it is release-owned per AGENTS.md): adds optional CLAWSWEEPER_STALE_PULL_REQUEST_REVIEW_DAYS to extend the re-review cadence for open PRs past their hot window with no activity since last review.