Make public review verdicts explicit#173
Conversation
{
"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 |
{
"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 |
|
{"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 |
{
"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
-- Anthropic Claude |
|
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
Prior unresolved item dispositions
-- Google Gemini |
|
{"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 |
|
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"}
]
} |
{
"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
-- Anthropic Claude |
Summary
Review verdictline to rendered public PR review commentsTesting