Repository navigation
feat(review): --post flag for headless GitHub review submission - #36
Merged
Merged
Conversation
`prr review <N> --post` posts the synthesized findings as a single GitHub Review (event=COMMENT) with one inline comment per file+line-anchored finding. Mirrors the TUI's Ctrl+S batch-submit path so headless and interactive runs produce equivalent reviews. Confirms via stdin by default; --yes / -y bypasses the prompt and is required in non-TTY contexts (CI fails fast instead of blocking on a stdin read).
The aoi-benchmark-rework run in 4e91b34 only flipped AOI: true on the claude-code Opus 4.7 row; the anthropic-direct row was missed. Same model weights, same capabilities — surface it in the AOI picker for users on the direct API.
The synthesis system prompt listed "bug | security | performance | testing | style | architecture | docs" as allowed categories, but five of those are not in the embedded category slug set used by state.Category's UnmarshalJSON. Whenever the LLM took the prompt literally (e.g. picked "bug" for an error-handling finding), the whole ReviewOutput parse failed and the headless renderer fell through to dumping the raw JSON. Move findings construction out of the LLM: synthesis now writes only the four narrative fields it actually authors (summary, verdict, missing_tests, questions_for_author) and the findings array is built in Go from the post-recheck DeepFindings via BuildFindingsFromDeep — straight field copy, source_ids back to F-IDs, sorted by severity. ParseReviewOutput now hard-rejects empty summary and off-list verdict so the existing retry path engages on bad wrappers; if both attempts fail, RunSynthesis still returns a usable ReviewOutput with the deterministic findings and a verdict derived from severity counts.
andreujuanc
commented
May 25, 2026
andreujuanc
commented
May 25, 2026
| } | ||
| } | ||
| body := formatBatchBody(findings) | ||
| if err := git.SubmitReviewWithFindings(prNumber, commitSHA, body, comments); err != nil { |
Owner
Author
There was a problem hiding this comment.
[medium/error-handling] Error not wrapped with context
The error returned from git.SubmitReviewWithFindings is returned directly without wrapping, violating the project's mandatory error wrapping convention.
Suggestion: Wrap the error: return 0, fmt.Errorf("submitting review findings: %w", err)
Posted by prr AI review
| } | ||
| } | ||
| body := formatBatchBody(findings) | ||
| if err := git.SubmitReviewWithFindings(prNumber, commitSHA, body, comments); err != nil { |
Owner
Author
There was a problem hiding this comment.
[low/error-handling] Error not wrapped with context
The error returned from git.SubmitReviewWithFindings is not wrapped with context, violating the project's mandatory error wrapping convention.
Suggestion: Wrap the error: return 0, fmt.Errorf("submitting review: %w", err)
Posted by prr AI review
runReviewCallWithRetry used to short-circuit on errReviewParse with the reasoning that "retrying the same prompt won't fix prose-in-response." In practice LLM sampling variance is wide enough that a second attempt often produces valid JSON, and losing a 45s deep-review call (with up to 10 AOIs for grouped calls) to a one-off prose response is too expensive to leave unrecovered. Drop the short-circuit so parse failures get the same one-retry treatment as transient API errors. Context cancellation still short-circuits. Also flesh out TestParseReviewOutput_WrapperOnly to assert the missing_tests and questions_for_author fields parse through (caught by a review pass against the previous commit).
…questions The posted GitHub review body used to be a single line of severity counts. The summary paragraph, missing_tests, and questions_for_author fields that synthesis already authors weren't being surfaced to the PR reader — wasted context. Thread the full *state.ReviewOutput through PostStructuredReview and publishBatchReview (the TUI's Ctrl+S batch-submit) so both posting paths render the same enriched body: summary paragraph, severity counts, then optional Missing tests / Questions for the author sections. Empty sections are skipped.
andreujuanc
commented
May 25, 2026
andreujuanc
left a comment
andreujuanc
left a comment
Owner
Author
There was a problem hiding this comment.
AI Review Summary
The PR introduces a --post flag to the prr review command, enabling headless GitHub review submission, which is a valuable addition for CI automation. While the implementation is generally clean, there is a medium-severity risk of markdown injection in the generated review body that must be addressed before merging.
1 findings across 1 files: 1 med
Missing tests
- Unit or integration tests for the new functionality in
internal/review/publish.goto verify GitHub API interaction. - Regression test for the markdown injection vulnerability identified in
internal/ui/confirm_modal.go.
Questions for the author
- How do you plan to sanitize the AI-generated strings (
MissingTests,QuestionsForAuthor) before they are interpolated into the markdown body to prevent injection? - What is the expected behavior if the GitHub API returns an error during the batch submission process? Is it handled gracefully?
- Are there any plans to implement
--verdict=autoin the near future, or isCOMMENTsufficient for current use cases?
Posted by prr AI review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
prr review <N> --postposts the synthesized findings as a single GitHub Review (event=COMMENT) with one inline comment per file+line-anchored finding. Mirrors the TUI'sCtrl+Sbatch-submit path.--yes/-ybypasses the prompt. Non-TTY without--yesfails fast so CI runs don't block on a stdin read.claude-opus-4-7rowAOI: true. The aoi-benchmark-rework (4e91b34) only flipped the flag on the claude-code Opus 4.7 row; the anthropic-direct row was missed — same model weights, same capabilities.Context
Motivated by automating PR review across an org's repos via CI — the headless review pipeline already produced findings, but posting them back to GitHub required custom
jq+gh apiglue.--postremoves that step so a workflow can be a singleprr review <N> --post --yes.Verdict is hardcoded to
COMMENTfor v1;--verdict=auto(trust synth) is a follow-up if it proves useful.prr audit --postis not implemented — audit isn't tied to a PR, so there's no target.New code lives in
internal/review/publish.go(a thin wrapper around the existinggit.SubmitReviewWithFindings); the UI's batch-submit path was left untouched to keep the change surgical.