Skip to content

feat(review): --post publishes PR reviews with inline high-confidence comments#64

Merged
JohnnyVicious merged 1 commit intomainfrom
feat/post-review-to-pr
Apr 14, 2026
Merged

feat(review): --post publishes PR reviews with inline high-confidence comments#64
JohnnyVicious merged 1 commit intomainfrom
feat/post-review-to-pr

Conversation

@JohnnyVicious
Copy link
Copy Markdown
Owner

Summary

  • New `--post` flag on `/opencode:review` and `/opencode:adversarial-review` (opt-in, requires `--pr N`) publishes the review back to the PR as a GitHub review.
  • Summary body: verdict badge, model, findings table, collapsible full-details section.
  • Inline review comments: findings with `confidence >= --confidence-threshold` (default 0.8) whose line is addressable in the PR's unified diff are anchored inline as conversations on that exact line. Findings below the threshold or outside the diff stay in the summary table only.
  • Review is always published with `event: "COMMENT"` — never `REQUEST_CHANGES`. This tool is advisory.
  • Posting failures are non-fatal: local review output has already been printed, so GitHub API / auth glitches log a stderr warning and do not flip the run to a non-zero exit.

Design decisions

  • Opt-in, not automatic. Anyone already running `--pr N` for private sanity checks should not get surprise public comments. `--post` is easy to alias.
  • Confidence threshold is user-tunable. Accepts `0..1` floats (`0.8`) or percentages (`80`, `80%`). Default 0.8.
  • Unified-diff parsing before posting. GitHub rejects review comments on lines outside the PR diff, so each file's `patch` field is parsed into a `Set` and high-confidence findings whose lines aren't addressable are silently demoted to summary-only — we never drop the finding, we just don't crash the whole post.

Implementation

  • New module: `plugins/opencode/scripts/lib/pr-comments.mjs` (`postReviewToPr` + pure helpers).
  • Wired into `handleReview` / `handleAdversarialReview` in `opencode-companion.mjs`.
  • Slash-command docs updated.
  • `gh api` is called with a JSON payload on stdin (avoids fragile `-f`/`-F` array serialization).

Test plan

  • 21 unit tests in `tests/pr-comments.test.mjs` cover: unified-diff parsing (additions / deletions / context / multi-hunk / 'No newline' marker), finding classification (confidence threshold / file not in diff / line range missing the diff / range-to-first-addable anchoring), inline comment payload shape, and summary body rendering (verdict, model, findings table, pipe/newline escaping, fallback to raw rendered when structured is null).
  • Full suite: `npm test` → 243/243 pass.
  • Manual smoke test on a real PR (will run after merge).

Follow-ups (out of scope)

  • Update existing inline comments on re-review instead of stacking new ones.
  • Support `side: "LEFT"` comments for findings that target deleted lines.

When `/opencode:review --pr N --post` or `/opencode:adversarial-review
--pr N --post` runs, the companion now publishes the review back to
the GitHub PR as a review comment:

- The summary body is a structured markdown block: verdict badge,
  model, a findings table (severity / confidence / file / lines /
  title), and a collapsible "Full findings" section with body +
  recommendation per finding.
- Findings with confidence at or above `--confidence-threshold`
  (default 0.8, configurable as a float or percent) whose line is
  addressable on the PR's unified diff are posted as inline review
  comments anchored to that specific line. Findings below the
  threshold or outside the diff stay in the summary table only.
- Reviews are always published with `event: "COMMENT"` — never
  `REQUEST_CHANGES` — because this tool is advisory.
- Posting failures are non-fatal: the local review output is already
  on stdout, so a network or auth glitch on GitHub logs a warning to
  stderr instead of flipping the whole run to a non-zero exit.

Opt-in was the explicit design choice: anyone already using
`--pr N` for private sanity checks should not get surprise public
comments, and `--post` is easy to add to muscle memory.

Also relax the 7-second body-stall assertion in
send-prompt-body-timeout.test.mjs — the deepEqual already proves the
code path survives the stall, and the timing bound flaked under WSL2
clock skew.
@JohnnyVicious JohnnyVicious merged commit 180e705 into main Apr 14, 2026
1 check passed
@JohnnyVicious JohnnyVicious deleted the feat/post-review-to-pr branch April 14, 2026 17:51
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