fix(review): avoid publishing empty review output

[oleksii-tumanov]

Follow-up to #2730, which fixed #2729.

Related to #2522, fixed by #2542, which handles empty model content earlier in the response pipeline.

Problem

When /review receives a non-empty model response that cannot produce a non-empty review mapping, _prepare_pr_review() returns an empty string. The publication path currently treats that result as publishable, so a persistent rerun can replace a previous review with empty content.

Changes

• Validate that review is a non-empty mapping before structured, push, or comment publication, while preserving the existing GitHub Action output behavior.
• Treat an empty per-chunk review as unusable so the existing single-call fallback runs.
• Route an unusable prepared review through the existing review-failure lifecycle.
• Add regression coverage for invalid YAML, an empty review, and an invalid review shape across persistent and non-persistent runs.

Testing

• PYTHONPATH=. uv run pytest -q tests/unittest — 3629 passed, 1 skipped, 1 xfailed
• uv run ruff check pr_agent/tools/pr_reviewer.py tests/unittest/test_pr_reviewer_core.py tests/unittest/test_review_large_diff_chunking.py — passed

[IsmaelMartinez]

Thanks for this, and for the regression coverage that came with it.

Correct fix, and it covers more than your description claims: against main, a chunked review where every chunk returns review: {} publishes a comment that is nothing but the "reviewed in N chunks" footer.

One follow-up, and it is not a fault in what you wrote. pr_reviewer.py:328 still accepts an empty review, so it now disagrees with your new guard at :391. Main skips the single-call fallback for that input too, so this predates you, but your guard turns the case into "Failed to review PR". Adding or not data["review"] at :328 fires that fallback for the first time and keeps the suite green. It sits between your two hunks so I could not leave it as a suggestion inline.

[oleksii-tumanov]

Good point. I updated the chunk validation so review: {} now triggers the existing single-call fallback, and added regression coverage for that path.

[IsmaelMartinez]

Thanks, the :328 guard is exactly right and the new fallback test covers it: 90 passed on the merged tree, 13 red with the source restored.

One question inline. Separately, this now conflicts in pr_agent/tools/pr_reviewer.py, which is my doing: #3036 and #2722 both landed there in the last hour. Sorry for the rebase.

[IsmaelMartinez]

Moving this below the guard is a behaviour change that is not in the description. On main github_action_output always ran, so an empty or malformed review still wrote review={} to $GITHUB_OUTPUT. Here it writes nothing at all, and a workflow reading that output goes from {} to unset.

Deliberate? I can see the argument either way, but it is visible outside pr-agent, so it is worth a line in the description rather than riding along with the fix.

[oleksii-tumanov]

You're right. I kept the existing GitHub Action behavior, so a malformed or empty review still writes the same output as before. The guard now only stops it from reaching the review publication paths, and I added coverage for that.

[qodo-code-review]

Code Review by Qodo

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again manually by commenting /agentic_review on this PR.

[IsmaelMartinez]

Thanks, that is exactly it. Approving.

github_action_output is back above the guard, so $GITHUB_OUTPUT keeps the contract it had on main, and your new test pins it. I merged onto today's main and ran the suite: 91 passed, and 14 go red with pr_reviewer.py restored, so the coverage holds.