diff --git a/pilot/ai_review.py b/pilot/ai_review.py index 300fd90..5245b27 100644 --- a/pilot/ai_review.py +++ b/pilot/ai_review.py @@ -420,6 +420,39 @@ def parse_findings(text: str) -> list[dict]: return out +SALVAGE_MAX_CHARS = 4000 + + +def salvage_summary(text: str, max_chars: int = SALVAGE_MAX_CHARS) -> str: + """Recover something postable from agent output we could not parse. + + An opencode run costs minutes and millions of tokens. When the findings JSON + is missing or malformed, the analysis itself is usually still there in the + prose — discarding it to post "no parseable output" throws away the whole + run and tells the maintainer nothing. This keeps the tail of the prose (the + conclusion, which is what the agent writes last), drops fenced code blocks + so a half-written JSON blob doesn't dominate, and labels it plainly as + unstructured so nobody mistakes it for a normal review. + + Returns "" when there is genuinely nothing to salvage. + """ + if not text or not text.strip(): + return "" + # Drop fenced blocks — a truncated ```json block is noise here. + prose = re.sub(r"```.*?```", "", text, flags=re.DOTALL) + prose = re.sub(r"```.*$", "", prose, flags=re.DOTALL) # unterminated fence + prose = prose.strip() + if not prose: + return "" + if len(prose) > max_chars: + prose = "…" + prose[-max_chars:] + return ( + "⚠️ _The reviewer did not emit a parseable findings block, so there are " + "no inline comments. Its raw notes are below — treat them as unverified: " + "line numbers were not validated against the diff._\n\n" + prose + ) + + def parse_review_output(text: str) -> tuple[str, list[dict]]: """Parse the opengine's stdout into (summary, findings). @@ -904,9 +937,22 @@ def review_pr( ) review_summary, findings = parse_review_output(stdout) if not findings and not review_summary: - # opencode produced nothing parseable — fall back to a note. + # The findings JSON was missing or malformed. Don't discard the + # run: salvage the prose, keep the usage report (the label asked + # for it, and the tokens were spent either way), and log enough + # of the raw output to diagnose why the agent went off-format. + print( + f"pragent: {repo}#{index} sha={sha[:8]} unparseable output " + f"({len(stdout)} chars); tail: {stdout[-600:]!r}", + file=sys.stderr, flush=True, + ) + salvaged = salvage_summary(stdout) + usage_section = "" + if report_usage and usage: + usage_section = format_usage_section(usage, [], model) post_review(api, repo, index, token, format_review_body( - "AI review produced no parseable output.", model, sha)) + salvaged or "AI review produced no parseable output.", + model, sha, usage_section=usage_section)) return True else: user_prompt = build_user_prompt(title, body, diff, config, prior) diff --git a/tests/pilot/test_ai_review.py b/tests/pilot/test_ai_review.py index 192dedf..0c412c6 100644 --- a/tests/pilot/test_ai_review.py +++ b/tests/pilot/test_ai_review.py @@ -737,3 +737,45 @@ def test_fetch_pr_diff_error_reports_both_statuses(monkeypatch): assert "files=500" in str(e) else: raise AssertionError("expected RuntimeError") + + +# --------------------------------------------------------------------------- +# salvage_summary — don't discard an expensive run over a missing JSON block +# --------------------------------------------------------------------------- + + +def test_salvage_summary_keeps_the_prose(): + text = "I reviewed the diff. The retry loop in worker.py never terminates." + out = ai_review.salvage_summary(text) + assert "never terminates" in out + assert "unverified" in out + + +def test_salvage_summary_drops_fenced_blocks(): + text = 'Analysis here.\n\n```json\n{"findings": [ truncated...\n' + out = ai_review.salvage_summary(text) + assert "Analysis here." in out + # The half-written JSON blob is gone (the banner legitimately says + # "findings", so assert on the blob's own content instead). + assert "truncated..." not in out + assert "[" not in out.split("_\n\n", 1)[1] + + +def test_salvage_summary_drops_complete_fences_too(): + text = "Before.\n```python\nprint(1)\n```\nAfter." + out = ai_review.salvage_summary(text) + assert "Before." in out and "After." in out + assert "print(1)" not in out + + +def test_salvage_summary_keeps_the_tail_when_long(): + text = "x" * 9000 + " FINAL CONCLUSION" + out = ai_review.salvage_summary(text, max_chars=1000) + assert "FINAL CONCLUSION" in out # the conclusion is written last + assert len(out) < 1600 + + +def test_salvage_summary_empty_when_nothing_to_salvage(): + assert ai_review.salvage_summary("") == "" + assert ai_review.salvage_summary(" \n ") == "" + assert ai_review.salvage_summary("```json\n{}\n```") == ""