Skip to content

Parse approved followup headings with colons#79

Merged
wwind123 merged 1 commit into
mainfrom
fix-approved-followup-colon-headings
May 18, 2026
Merged

Parse approved followup headings with colons#79
wwind123 merged 1 commit into
mainfrom
fix-approved-followup-colon-headings

Conversation

@wwind123
Copy link
Copy Markdown
Owner

Summary

  • allow approved-review follow-up headings to end with an optional colon
  • add regression coverage for same-PR, future, and legacy non-blocking follow-up headings with trailing colons

Fixes #78

Tests

  • /tmp/coding-review-agent-loop/scratch/issue78-venv/bin/python -m pytest tests/test_agent_loop.py -k followups
  • /tmp/coding-review-agent-loop/scratch/issue78-venv/bin/python -m pytest

@wwind123
Copy link
Copy Markdown
Owner Author

Implemented the fix for issue #78 and opened PR #79 against main.

Changed:

  • Approved follow-up heading regexes now accept optional trailing colons.
  • Added regression coverage for Same-PR, Future, and legacy Non-blocking follow-up headings ending in :.

Verified:

  • pytest tests/test_agent_loop.py -k followups: 29 passed
  • pytest: 134 passed

-- OpenAI Codex

@wwind123
Copy link
Copy Markdown
Owner Author

The fix is correct and minimal.

Regex change (protocol.py): Inserting :? before \s*$ in all three heading patterns is the right approach. It allows an optional trailing colon without being over-permissive (e.g. :: or : still won't match, which is appropriate). All three constants — SAME_PR_FOLLOWUP_HEADING_RE, FUTURE_FOLLOWUP_HEADING_RE, and LEGACY_FOLLOWUP_HEADING_RE — are updated consistently.

Test (test_agent_loop.py): The new test exercises all three heading variants with trailing colons in a single round-trip through parse_approved_followups, verifying that same-PR items land in same_pr, and future and legacy non-blocking items both land in future. This matches the exact scenario described in issue #78 (Gemini using ### Future follow-ups:).

No concerns with correctness, security, or maintainability.

-- Anthropic Claude

@wwind123
Copy link
Copy Markdown
Owner Author

The PR correctly addresses issue #78 by updating the follow-up heading regexes to tolerate an optional trailing colon. This ensures that reviews from agents like Gemini, which often include a colon in headings, are correctly parsed for both same-PR and future follow-ups.

Summary of Changes

  • Regex Updates: Updated SAME_PR_FOLLOWUP_HEADING_RE, FUTURE_FOLLOWUP_HEADING_RE, and LEGACY_FOLLOWUP_HEADING_RE in src/coding_review_agent_loop/protocol.py to include :? at the end of the patterns.
  • New Test Case: Added test_parse_approved_followups_accepts_trailing_colons_on_headings in tests/test_agent_loop.py to verify that headings with trailing colons are correctly identified and their contents extracted.

Future follow-ups

  1. Markdown Variants: Consider supporting other common Markdown variants such as bolded headings (e.g., ### **Future follow-ups**) or headings ending with other punctuation like a period if they are observed in practice.

The implementation is surgical and the new test case provides good coverage for the reported bug.

-- Google Gemini

@wwind123
Copy link
Copy Markdown
Owner Author

Created approved-review future follow-up issues for PR #79:

These were mentioned in approved reviews as future work and did not block merge readiness.

-- coding-review-agent-loop

@wwind123 wwind123 merged commit c8b568b into main May 18, 2026
1 check passed
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.

Parse approved-followup headings with trailing colons

1 participant