Skip to content

Fix stale approved follow-up issue creation#74

Open
wwind123 wants to merge 1 commit into
mainfrom
fix-issue-73-final-approved-followups
Open

Fix stale approved follow-up issue creation#74
wwind123 wants to merge 1 commit into
mainfrom
fix-issue-73-final-approved-followups

Conversation

@wwind123
Copy link
Copy Markdown
Owner

Summary

  • Process approved-review future follow-ups only from the final approval round
  • Stop carrying earlier future follow-ups across same-PR fix rounds
  • Update tests and docs for final-round follow-up behavior

Fixes #73

Tests

  • /tmp/coding-review-agent-loop/scratch/issue-73-venv/bin/python -m pytest

@wwind123
Copy link
Copy Markdown
Owner Author

Implemented the fix for #73 and opened PR #74 against main.

Changed behavior so approved-review future follow-ups are only processed from the final approval round, preventing stale earlier-round items from creating issues after same-PR fixes. Updated tests and docs accordingly.

Tests passed: /tmp/coding-review-agent-loop/scratch/issue-73-venv/bin/python -m pytest

-- OpenAI Codex

@wwind123
Copy link
Copy Markdown
Owner Author

Review of PR #74 — Fix stale approved follow-up issue creation

The fix is correct and well-scoped. The core change in orchestrator.py is minimal: remove pending_future_followups, drop the accumulation call in the same-PR path, and use only round_future_followups on final approval. This directly eliminates the bug described in #73, where future follow-up items from an earlier same-PR fix round would survive into the final-approval step and create stale issues.

The test renames and updates are accurate. Notably, test_pr_loop_fix_and_summarize_only_uses_final_round_future_followups is the strongest of the three: it confirms both that stale items from round 1 are absent and that fresh items from the final round are present in the summary, which is the right two-sided assertion.

The README and docs updates correctly set the new expectation — reviewers must restate still-relevant future work in their final round — and match the implementation.

Same-PR follow-ups

Stale comment in the else (blocking) branch of orchestrator.py — the comment block that begins:

# Future follow-ups are only retained for fully approved same-PR fix
# rounds. If any reviewer blocks, future-work suggestions from that
# round are discarded ...

was written to explain why the old code skipped pending_future_followups.extend() in the blocking path. Now that pending_future_followups is gone entirely, the first sentence ("only retained for fully approved same-PR fix rounds") is the opposite of true — nothing is retained from any non-final round. A reader encountering this comment would incorrectly infer that the selective-retention behaviour still exists. This comment should be removed or replaced with a one-liner that matches the new policy (e.g., "Future follow-ups from non-final rounds are discarded; reviewers restate still-relevant items in their final pass."). The file is already touched by this PR so the fix is in scope.

-- Anthropic Claude

@wwind123
Copy link
Copy Markdown
Owner Author

The changes in this PR correctly address the issue of stale approved follow-up suggestions by ensuring that only those provided in the final approval round are processed. This prevents follow-ups from earlier rounds, which might have been rendered obsolete or incorrect by subsequent same-PR fixes, from being automatically carried forward.

Summary of changes:

  • Orchestrator logic: Removed the accumulation of pending_future_followups across rounds. The final approval now only considers follow-ups from the current round.
  • Documentation: Updated README.md and docs/local_agent_loop.md to explicitly state that reviewers should restate any still-relevant future work in their final approval.
  • Tests: Updated existing tests and added new assertions to verify that stale future follow-ups are ignored and only final-round suggestions are processed (for both summaries and issue creation).

The implementation is clean and the test coverage for this specific behavior is robust.

-- Google Gemini

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.

Avoid creating stale approved-followup issues from earlier PR rounds

1 participant