Migrate PR reviews to structured rendering

[wwind123]

Summary

• add a structured-first PR review parser that accepts the live footer contract, preserves markdown fallback, and round-trips ### Blocking issues
• render public PR review comments from normalized ParsedReview data in the orchestrator while keeping summary-only blocking item tracking
• extend prompts and regression coverage for structured parsing failures, fallback behavior, renderer ownership, tracking semantics, and resume safety

Testing

• python3 -m pytest tests/test_agent_loop.py
• python3 -m pytest

Fixes #154

[wwind123]

Implemented issue #154 in PR #165.

PR review handling now validates structured responses first, falls back to markdown only when no leading structured object is present, round-trips ### Blocking issues, and renders public PR review comments from normalized orchestrator-owned data while keeping summary-only blocking-item tracking.

Tests: python3 -m pytest tests/test_agent_loop.py passed; python3 -m pytest passed.

-- OpenAI Codex

[wwind123]

The structured PR review migration is clean and correctly implements all the constraints from the parent plan. Verified 370 tests pass.

What was reviewed:

protocol.py:

• _extract_json_object_prefix correctly hard-fails (raises AgentLoopError) when text starts with { but JSON is malformed — intentional change from the previous silent None return.
• _extract_structured_pr_review_payload enforces the footer contract (AGENT_STATE marker + signature must follow the JSON object, no intervening prose), and checks that the JSON state field matches the footer AGENT_STATE. The double HUMAN_REQUIREMENTS_RESOLVED_RE.match call is a minor inefficiency but not a bug.
• parse_pr_review correctly tries parse_structured_pr_review first and falls back to parse_review only when no structured candidate is present (text does not start with {).
• BLOCKING_ISSUES_HEADING_RE is correctly added to the review_freeform_summary_text skip-list so ### Blocking issues bullets don't bleed into the tracked summary. V1 tracking semantics (summary-only) are preserved.
• _finalize_parsed_review correctly rejects approved reviews that carry blocking_items, completing the invariant enforcement.

orchestrator.py:

• _render_public_pr_review_comment renders from normalized ParsedReview data using \n\n-separated sections; the _attach_round_metadata/_strip_round_metadata round-trip correctly normalizes the final posted body. Section ordering (summary → blocking → same-PR → future → prior dispositions → footer) matches the test assertions.
• blocking_summary = parsed_review.summary correctly replaces _review_freeform_summary_text(review_output) — both produce the same value, but using parsed_review.summary is semantically cleaner and directly expresses the v1 "summary is the sole tracked item source" constraint.
• The resume path uses parse_review (markdown) on the orchestrator-rendered comment, which is correct: _render_public_pr_review_comment always emits markdown, never JSON.
• new_items was accepted by the old _render_public_review_comment but was never used in the function body; dropping it from the new renderer is correct.
• Future followups are intentionally dropped from blocking review public comments (via _finalize_parsed_review stripping followups.future when state == "blocking"). The test update (not in for "Consider a broader cleanup later.") reflects this correctly.
• Legacy ### Non-blocking follow-ups is normalized to ### Future follow-ups in rendered output — test updated accordingly.

Test coverage:

• Structured round-trip success with footer contract ✓
• Markdown fallback (no structured candidate) ✓
• Hard-fail on invalid structured candidates (missing fields, unknown keys, bad item IDs, enum violations, trailing prose, footer mismatch) ✓
• _validate_review_response with structured input ✓
• _render_public_pr_review_comment unit test with expected section/footer layout ✓
• Markdown ↔ structured rendering equivalence ✓
• Resume path with orchestrator-rendered comment ✓
• Integration: tracking uses only summary even when blocking_items phrase the issue differently ✓

-- Anthropic Claude

[wwind123]

I have reviewed the changes in PR #165 and found the implementation to be correct and well-tested.

The migration to structured PR reviews is handled surgically, preserving current tracking semantics while enabling richer rendering of multiple blocking items. The validation logic for the new structured format is strict and correctly enforces the expected "JSON + footer" contract. I also verified that the markdown fallback remains fully functional and even benefits from the new normalized rendering path.

Key verified aspects:

• Structured Parsing: parse_pr_review correctly prefers the JSON format while falling back to markdown.
• Render-only Blocking Items: The orchestrator now uses summary for item tracking but renders all blocking_items in the public comment, as requested.
• Prompting: Instructions were updated to guide reviewers toward the structured format.
• Tests: All 370 tests passed, including extensive new cases for the structured flow.

No blocking issues found.

-- Google Gemini