Skip to content

Make public review verdicts explicit#173

Merged
wwind123 merged 3 commits into
mainfrom
codex/explicit-public-review-verdict
May 27, 2026
Merged

Make public review verdicts explicit#173
wwind123 merged 3 commits into
mainfrom
codex/explicit-public-review-verdict

Conversation

@wwind123
Copy link
Copy Markdown
Owner

Summary

  • add a visible Review verdict line to rendered public PR review comments
  • keep resume parsing clean by stripping that verdict line from reparsed freeform summaries
  • update PR-loop renderer tests and posted-comment expectations accordingly

Testing

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

@wwind123
Copy link
Copy Markdown
Owner Author

{
  "schema_version": 1,
  "kind": "pr_review",
  "state": "approved",
  "summary": "Clean, focused change: prepends a visible `**Review verdict:** Approved/Blocking` line to every rendered public PR review comment and strips it back out during freeform-summary reparsing. All 417 tests pass locally. CI checks pass.",
  "blocking_items": [],
  "same_pr_followups": [],
  "future_followups": [],
  "prior_item_dispositions": []
}

-- Anthropic Claude

@wwind123
Copy link
Copy Markdown
Owner Author

{
  "schema_version": 1,
  "kind": "pr_review",
  "state": "blocking",
  "summary": "The PR correctly implements explicit review verdicts for PR reviews and ensures robust parsing by stripping the verdict line during summary extraction. However, there are a few items that need to be addressed for consistency and robustness before merge, most notably the omission of explicit verdicts for Plan reviews.",
  "blocking_items": [
    "The explicit verdict line is missing from `_render_public_plan_review_comment` in `src/coding_review_agent_loop/orchestrator.py`. Since `review_freeform_summary_text` (in `src/coding_review_agent_loop/protocol.py`) now strips this line for all reviews, any `**Review verdict:**` line included in a plan review will be lost upon re-rendering. It should be added to `_render_public_plan_review_comment` for consistency.",
    "The regex in `review_freeform_summary_text` (`src/coding_review_agent_loop/protocol.py`) should be case-insensitive (e.g., by adding `re.I`) to ensure that manually written verdicts that do not follow the exact capitalization of `state.title()` are also stripped. This prevents duplicate verdict lines if an agent responds with a lowercase verdict."
  ],
  "same_pr_followups": [
    "Update `test_render_public_plan_review_comment_normalizes_sections` in `tests/test_agent_loop.py` to assert the inclusion of the explicit verdict line once implemented for plan reviews.",
    "Consider whether `parse_structured_pr_review` should also strip the verdict line from the `summary` field if it's present in the JSON payload, to prevent doubling if an agent includes it in both places."
  ],
  "prior_item_dispositions": []
}

-- Google Gemini

@wwind123
Copy link
Copy Markdown
Owner Author

{"schema_version":1,"kind":"coder_followup","state":"blocking","summary":"Implemented the requested fix for plan-review verdict rendering, made verdict-line stripping case-insensitive, and updated the affected plan-flow tests.","addressed_items":["item-1"],"remaining_items":[],"human_requirements":{"addressed_ids":[],"checked_discussion_directly":false},"tests_run":["python3 -m pytest tests/test_agent_loop.py -k "render_public_plan_review_comment_normalizes_sections or review_freeform_summary_text_strips_structured_followup_sections or parse_structured_pr_review_normalizes_v1_payload_with_footer_contract"","python3 -m pytest tests/test_agent_loop.py"]}

-- OpenAI Codex

@wwind123
Copy link
Copy Markdown
Owner Author

{
  "schema_version": 1,
  "kind": "pr_review",
  "state": "approved",
  "summary": "Round-2 additions fully address Gemini's blocking items: `_render_public_plan_review_comment` now prepends the verdict line, the filter regex is case-insensitive, the plan-review normalization test is updated to assert the verdict prefix, and a case-insensitive stripping test is included. All 417 tests pass locally; GitHub CI is passing.",
  "blocking_items": [],
  "same_pr_followups": [],
  "future_followups": [
    "Consider stripping a `**Review verdict:**` line from the `summary` field inside `parse_structured_pr_review` (JSON path), to guard against doubling if an agent embeds it in both the structured summary and the rendered comment. Low priority — in practice the orchestrator controls rendering."
  ],
  "prior_item_dispositions": [
    {"item_id": "item-1", "disposition": "resolved"}
  ]
}

Prior unresolved item dispositions

  • [item-1] Blocking issue from Google Gemini, round 1: ```json -> resolved

-- Anthropic Claude

@wwind123
Copy link
Copy Markdown
Owner Author

The PR correctly addresses the primary blocking items from round 1: it now includes the explicit verdict line in public plan review comments and uses a case-insensitive regex to strip redundant verdict lines during summary extraction. All tests passed. However, the robustness improvement for structured reviews (stripping the verdict line from the JSON summary field) is still pending and should be implemented to complete the task.

Same-PR follow-ups

  • In src/coding_review_agent_loop/protocol.py, update parse_structured_pr_review and parse_structured_plan_review to pass the summary extracted from the JSON payload through review_freeform_summary_text. This ensures that redundant verdict lines or structured headers are stripped if an agent mistakenly includes them in the JSON summary field.

Prior unresolved item dispositions

  • [item-1] Blocking issue from Google Gemini, round 1: ```json -> same-pr: Verdict line added to plan reviews and regex made case-insensitive. Test coverage updated. Robustness for structured reviews is the last remaining sub-item.

-- Google Gemini

@wwind123
Copy link
Copy Markdown
Owner Author

{"schema_version":1,"kind":"coder_followup","state":"blocking","summary":"Normalized structured PR and plan review summaries through the shared freeform-summary sanitizer so redundant verdict lines and structured headings are stripped from JSON payload summaries. Added regression coverage for both structured parser entry points and pushed the fix for Gemini re-review.","addressed_items":["item-1","item-2","item-3"],"remaining_items":[],"human_requirements":{"addressed_ids":[],"checked_discussion_directly":false},"tests_run":["python3 -m pytest tests/test_agent_loop.py -k "structured_pr_review or structured_plan_review or review_freeform_summary_text or render_public_plan_review_comment"","python3 -m pytest tests/test_agent_loop.py"]}

-- OpenAI Codex

@wwind123
Copy link
Copy Markdown
Owner Author

Gemini's review, before failing the run:

{
  "schema_version": 1,
  "kind": "pr_review",
  "state": "approved",
  "summary": "The PR is now complete and robust. It correctly implements explicit review verdicts for both PR and plan reviews, ensures that redundant verdict lines are stripped using a case-insensitive regex, and includes the requested robustness improvement for structured reviews by normalizing the JSON summary field. Test coverage has been updated to verify all these behaviors.",
  "blocking_items": [],
  "same_pr_followups": [],
  "future_followups": [],
  "prior_item_dispositions": [
    {"item_id": "item-1", "disposition": "resolved"},
    {"item_id": "item-2", "disposition": "resolved"},
    {"item_id": "item-3", "disposition": "resolved"}
  ]
}

@wwind123
Copy link
Copy Markdown
Owner Author

{
  "schema_version": 1,
  "kind": "pr_review",
  "state": "approved",
  "summary": "All three prior unresolved items are fully addressed in this round. Both renderers prepend the verdict line, the stripping regex is case-insensitive, and both structured parsers now sanitize the JSON summary field through `review_freeform_summary_text`. Two new regression tests cover the JSON-payload stripping path. GitHub CI is passing.",
  "blocking_items": [],
  "same_pr_followups": [],
  "future_followups": [],
  "prior_item_dispositions": [
    {"item_id": "item-1", "disposition": "resolved"},
    {"item_id": "item-2", "disposition": "resolved"},
    {"item_id": "item-3", "disposition": "resolved"}
  ]
}

Prior unresolved item dispositions

  • [item-1] Blocking issue from Google Gemini, round 1: ```json -> resolved
  • [item-2] Blocking issue from Google Gemini, round 2: The PR correctly addresses the primary blocking items from round 1: it now includes the explicit... -> resolved
  • [item-3] Same-PR follow-up from Google Gemini, round 2: In src/coding_review_agent_loop/protocol.py, update parse_structured_pr_review and `parse_str... -> resolved

-- Anthropic Claude

@wwind123 wwind123 merged commit 679b575 into main May 27, 2026
1 check passed
@wwind123 wwind123 deleted the codex/explicit-public-review-verdict branch May 27, 2026 06:16
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.

1 participant