Skip to content

fix(review): distinguish leftover diff fences from applyable suggestions - #975

Closed
seonghobae wants to merge 17 commits into
mainfrom
fix/inline-comment-leftover-diff-receipts
Closed

fix(review): distinguish leftover diff fences from applyable suggestions#975
seonghobae wants to merge 17 commits into
mainfrom
fix/inline-comment-leftover-diff-receipts

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

Overview receipts listed applyable GitHub suggestion ranges, but authors could not tell those one-click applies from comments that still have only a ```diff fence.

  • After hunk prefilter and suggestion conversion, persist leftover ```diff fences as path:line plus `cannot-provide` or `LEFT`.
  • The overview lists applyable ranges first, then These comments still have a suggested-diff fence that GitHub cannot apply: with the reason on each row.
  • Includes fix(review): list applyable suggestion ranges in overview receipts #974 so this branch can land independently.

#954/#956#974 remain MERGEABLE with auto-merge; they are blocked by the two-approval ruleset, not a current-head code fail. Do not invoke @opencode-agent until #954 is on main.

Test plan

  • pytest tests/test_opencode_inline_comment_fallback.py tests/test_opencode_agent_contract.py — 76 passed
  • coverage run -m pytest tests — leftover helper 100% statements/branches, interrogate 100%
  • Current-head required checks on this PR

When GitHub refuses inline review comments, the PR-level fallback now
lists each sanitized current-head finding location instead of a generic
sentence. Suggested diffs stay out of the body.
Rebuild the fallback from gh api stderr after a refused attach so the
OpenCode overview keeps each trusted path:line next to the GitHub 422
phrase instead of a location-only list.
A single invalid path:line 422s the whole comments array. After that
failure, split the payload and retry each comment so surviving hunks
still attach; remaining failures keep the overview receipts.
The publisher moved that phrase out of the workflow YAML, so the
exact-head path-policy harness failed looking in the old file.
When some one-at-a-time inline comments attach and others 422, the
overview must list only the refused locations so attached hunks are
not reported as failed.
Mixed one-at-a-time retries can fail for different reasons. Record
path:line plus that comment's gh api error so the overview does not
reuse one shared sentence for every refused hunk.
Unbounded one-at-a-time retry after a batch 422 can thrash GitHub,
and mixed receipts listed only refused locations. Cap retries at 20,
persist attached path:line beside refused ones, and record leftovers
the cap left untried so the overview shows every outcome.
GitHub 422s review comments that sit outside every current-head @@ hunk.
Filter the payload against git diff --unified=3 first, post only the
on-hunk comments, and persist skipped path:line as overview receipts.
Authors could not one-click apply OpenCode inline repairs because the
payload only posted ```diff fences. Convert + lines from those diffs
into ```suggestion blocks on surviving RIGHT-side hunk comments.
A surviving suggested_diff that removes more than one current-head line
still posted as a single-line comment, so Apply suggestion only replaced
the first line. Set start_line, line, and start_side when the full span
sits on the same hunk; leave off-hunk spans single-line to avoid 422.
Authors could see refused and skipped path:line after a 422, but not
which surviving hunks shipped as one-click GitHub suggestions. Persist
path:line or path:start-end for comments that carry a suggestion fence.
Overview receipts listed applyable path:start-end ranges, but authors
could not tell leftover ```diff fences (cannot-provide / LEFT) from
one-click GitHub suggestions. Persist those leftover path:line reasons
in a separate overview section.
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 95 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 18494c6d-d7b9-4687-a8cf-11a2acc30f1f

📥 Commits

Reviewing files that changed from the base of the PR and between 6eb06cd and f4c9671.

📒 Files selected for processing (12)
  • .github/workflows/opencode-review-dispatch.yml
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • docs/doctoring/review-inline-comment-422-fallback.md
  • scripts/ci/materialize_base_python_requirements.py
  • scripts/ci/opencode_inline_comment_fallback.py
  • scripts/ci/test_strix_quick_gate.sh
  • tests/test_materialize_base_python_requirements.py
  • tests/test_opencode_agent_contract.py
  • tests/test_opencode_inline_comment_fallback.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review please review this leftover-diff overview increment. Authors need to see applyable GitHub suggestions separately from leftover ```diff fences (cannot-provide / `LEFT`).

A leftover ```diff whose prose mentioned ```suggestion was advertised
as applyable, and a LEFT suggestion fence vanished from leftover
receipts. Classify applyable vs leftover by a closed fence match and
keep every LEFT fence as leftover LEFT.
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review Please review this exact current head (0501bc97). The head moved after the earlier leftover-heading mention. Independent Noema APPROVE on this SHA is required; do not lower the two-approval ruleset.

Leftover path:line reason rows live in <!-- opencode-review-overview -->.
Strip comment delimiters and HTML metacharacters so a leftover path or
reason cannot close that comment (CWE-116).
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review Please review this exact current head. Leftover receipt path/reason rows now drop HTML comment delimiters and metacharacters so they cannot close <!-- opencode-review-overview --> (CWE-116). Independent Noema APPROVE on this SHA is required.

Leftover cannot-provide and LEFT receipts now keep a bounded Manual-edit
```diff block so authors can copy the replacement. Excerpts reuse the
existing leftover sanitizer so leftover text cannot close
`<!-- opencode-review-overview -->` (CWE-116).
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review exact current head daa2126b — leftover vs applyable fences distinguished; leftover receipt rows and Manual-edit excerpts sanitized. Head moved after earlier mention. Independent Noema APPROVE on this SHA is required. Do not wait on two-approval. Reviewers remain edit: deny. NVIDIA_NIM_API_KEY only.

Materialize a base Python lock only when every package line is an exact
SHA-256 pin or a two-token relative -r/--requirement include of a
candidate lock path. A lone --require-hashes directive, ./dotted paths,
and -r other-hashes.txt no longer enter the trusted build context.
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review exact current head e440d2d1 — leftover increment kept; materialize accepts only bounded relative -r includes. Head moved. Independent Noema APPROVE on this SHA is required. Do not wait on two-approval. Reviewers remain edit: deny. NVIDIA_NIM_API_KEY only.

Reject leftover 422-fallback paths that contain -->, <!--, or a
suggestion fence so a leftover cannot close the overview HTML comment
or reopen an applyable GitHub suggestion block.
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review exact current head f4c9671d — leftover overview paths with --> / <!-- / suggestion-fence closers omitted so leftover cannot break the overview HTML comment. Head moved after e440d2d1. Independent Noema APPROVE on this SHA is required. Do not wait on two-approval. Reviewers remain edit: deny. NVIDIA_NIM_API_KEY only.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for f4c9671d1ca9bf21caed77f1bba1c8f275aa8c98.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: opencode-review-dispatch.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: opencode-review-dispatch.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file (4 files)"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file (4 files)"]
  R2 --> V2["required checks"]
  Evidence --> S3["Docs: review-inline-comment-422-fallback.md"]
  S3 --> I3["operator or user guidance"]
  I3 --> R3["Review risk: Docs: review-inline-comment-422-fallback.md"]
  R3 --> V3["docs review"]
  Evidence --> S4["CI script (3 files)"]
  S4 --> I4["review and security gate shell path"]
  I4 --> R4["Review risk: CI script (3 files)"]
  R4 --> V4["bash -n plus Strix self-test"]
  Evidence --> S5["Test (3 files)"]
  S5 --> I5["regression suite"]
  I5 --> R5["Review risk: Test (3 files)"]
  R5 --> V5["targeted test run"]
Loading

@opencode-agent

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: f4c9671d1ca9bf21caed77f1bba1c8f275aa8c98
  • Workflow run: 31753407544
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for f4c9671d1ca9bf21caed77f1bba1c8f275aa8c98.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: opencode-review-dispatch.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: opencode-review-dispatch.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file (4 files)"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file (4 files)"]
  R2 --> V2["required checks"]
  Evidence --> S3["Docs: review-inline-comment-422-fallback.md"]
  S3 --> I3["operator or user guidance"]
  I3 --> R3["Review risk: Docs: review-inline-comment-422-fallback.md"]
  R3 --> V3["docs review"]
  Evidence --> S4["CI script (3 files)"]
  S4 --> I4["review and security gate shell path"]
  I4 --> R4["Review risk: CI script (3 files)"]
  R4 --> V4["bash -n plus Strix self-test"]
  Evidence --> S5["Test (3 files)"]
  S5 --> I5["regression suite"]
  I5 --> R5["Review risk: Test (3 files)"]
  R5 --> V5["targeted test run"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 14, 2026 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant