fix(review): turn surviving suggested diffs into GitHub suggestions - #972
fix(review): turn surviving suggested diffs into GitHub suggestions#972seonghobae wants to merge 15 commits into
Conversation
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.
|
Warning Review limit reached
Next review available in: 89 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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
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. Comment |
|
@cwl-noema-review Please review the current head. Convert surviving OpenCode suggested diffs into GitHub suggestion blocks so authors can apply the replacement on the hunk in one click. |
Convert surviving suggested diffs to GitHub suggestion blocks, but do not start one-at-a-time retry on a 422 substring inside a SHA or issue number (CWE-1288).
|
@cwl-noema-review Please review this exact current head ( |
A surviving suggested diff that removes more than one current-head hunk line must post as one start_line/line range so authors can apply it in one click.
|
@cwl-noema-review |
A multi-line GitHub suggestion still posts as one range after a batch 422 is retried one comment at a time, instead of collapsing to the last line.
|
@cwl-noema-review exact current head |
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.
|
@cwl-noema-review exact current head |
Leftover path:line receipts live in the overview HTML comment. Strip HTML metacharacters and suggestion fences so a leftover cannot close the comment or reopen an applyable GitHub suggestion block.
|
@cwl-noema-review exact current head |
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.
|
@cwl-noema-review exact current head |
OpenCode Review Overview
--> Changed-File Evidence Mapflowchart 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"]
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: APPROVE - current-head evidence (head_sha 76e68b0) affirmatively supports PR #972 intent; no blocking findings; mergeStateStatus BLOCKED is branch policy, not a conflict (mergeable: true), so no merge-conflict repair is needed.
Verification posture: trusted bounded evidence only - CodeGraph current-head index up to date (140 files, 2953 nodes) and Coverage execution evidence Result: PASS with supported repository test suites passed; no OPENCODE_EXECUTION_RECEIPT browser/DevTools lines exist and none are claimed.
Linter/static: lint_commands list only hadolint Dockerfile; no lint defects for changed files in bounded evidence.
TDD/regression: new tests/test_opencode_inline_comment_fallback.py (1519 lines) plus tests/test_opencode_agent_contract.py (lines 1612, 1643) and tests/test_materialize_base_python_requirements.py (lines 33, 753) updated in the same commit chain (050b2c9, 68264f1, 8eab74f, 760c8d7, 0ab29df, 76e68b0 per changed-file history).
Coverage: Coverage execution evidence reports Result: PASS - supported repository test suites passed (coverage_commands: pytest --fail-under=100).
Docstring coverage: Coverage execution evidence reports configured repository docstring gates passed or advisory; no docstring blocker.
DAG: source-backed base-to-head flowchart - create_pull_review_with_payload (.github/workflows/opencode-review-dispatch.yml) -> prefilter_inline_comments_to_hunks (merge-base diff, --filter-hunks) -> opencode_inline_comment_fallback.py split/filter/refusal -> one-at-a-time 422 retry with start_line and cap 20 -> overview HTML comment with sanitized skipped/refused path:line receipts (see mermaid).
PoC/execution: no OPENCODE_EXECUTION_RECEIPT lines exist for the new helper; execution provenance limited to the passed supported test suites in Coverage evidence.
DDD/domain: CI review-publication domain only; no domain-model or schema change.
CDD/context: docs/doctoring/review-inline-comment-422-fallback.md (new, 116 lines) documents the 422 fallback; AGENTS.md, CLAUDE.md, ARCHITECTURE.md, CHANGELOG.md updated in the same chain.
Similar issues: chain commits 68264f1, d37885d, 3f06fb8, 94ee03d, dc261ff, e099c28, ab69188 evolved the same workflow/helper/tests without contradictions.
Claim/concept check: PR claims surviving suggested diffs convert to suggestion fences - focused hunk at workflow lines 5933-6002 shows the JQ conversion emitting regression_test_direction and the suggested_diff fence; materialize change (8b6ca9c bounded relative -r includes) matches its test updates.
Standards search: CWE-116 (improper output encoding) matches 0ab29df/76e68b0d overview-path sanitization; no other external standard claim made.
Compatibility/convention: new identifiers retry_inline_comments_one_at_a_time, prefilter_inline_comments_to_hunks, skipped_locations_file, attached_locations_file, deferred_locations_file, opencode_inline_comment_fallback.py are multi-word idiomatic snake_case; no single-word/reserved names; no DB/API/config schema renamed.
Breaking-change/backcompat: additive only - optional positional args (${5:-}, ${6:-}), env-defaulted OPENCODE_INLINE_COMMENT_RETRY_LIMIT:-20, REVIEW_PUBLICATION_STALE_HEAD guard; base SHA 6eb06cd to head 76e68b0 keeps existing callers working.
Performance: one extra git merge-base diff per review and at most 20 serial comment POSTs only after a real 422; bounded.
Developer experience: DX surface is the OpenCode review-publication path - authors gain one-click Apply suggestion on RIGHT-side diffs and explicit path:line receipts for refused/skipped comments; judged from focused hunks and docs/doctoring/review-inline-comment-422-fallback.md.
User experience: UX surface is the PR review-comment output - suggestion fences render as GitHub Apply-suggestion affordance instead of inert ```diff fences; non-web interaction surface (CLI/API/review-comment output) reviewed from hunks.
Visual/DOM: non-web change; web_app_review_requirements empty; no DOM/ARIA surface to validate.
Accessibility/i18n: no motion/animation or aria changes; review output remains English; no i18n surface.
Supply-chain/license: no dependency additions or manifest changes; security_commands (npm audit, pip_audit, bandit, trivy) unchanged.
Packaging: unpackaged_source_surfaces empty; python contract pyproject.toml requires-python >=3.10 with python3 -m pytest tests.
Security/privacy: head commit 76e68b0 omits leftover overview paths containing comment closers (-->, <!--, suggestion-fence closers) so untrusted path text cannot break the overview HTML comment (CWE-116); 8b6ca9c bounds relative -r includes in scripts/ci/materialize_base_python_requirements.py (lines 90, 181); review_write_token usage and permissions unchanged; no new secrets.
Mermaid DAG (base-to-head changed flow):
flowchart LR
A["create_pull_review_with_payload (.github/workflows/opencode-review-dispatch.yml)"] --> B["prefilter_inline_comments_to_hunks (merge-base diff)"]
B --> C["opencode_inline_comment_fallback.py --filter-hunks --split-payload"]
C --> D["one-at-a-time 422 retry keeps start_line (cap 20)"]
D --> E["overview HTML comment with sanitized refused/skipped path:line (CWE-116)"]
D --> F["main risk: silent off-hunk comment loss or HTML-comment breakage"]
E --> G["verification: python3 -m pytest tests (Coverage evidence PASS)"]
F --> G
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including .github/workflows/opencode-review-dispatch.yml, AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md, and 7 more.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects .github/workflows/opencode-review-dispatch.yml to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":".github/workflows/opencode-review-dispatch.yml","line":5990,"hypothesis":"Off-hunk inline comments survive the hunk prefilter and are POSTed to GitHub, where they are rejected and reviewer findings are silently lost.","attack_or_counterexample":"A review payload with a comment targeting a path:line outside the merge-base..head diff hunks (unchanged file region or LEFT-side-only line).","evidence":"Trusted source trace at .github/workflows/opencode-review-dispatch.yml:5990: the focused changed hunk shows prefilter_inline_comments_to_hunks builds `git diff --unified=3 $merge_base $PR_HEAD_SHA` and runs the helper --filter-hunks; the observed branch `if [ \"$comment_count\" = \"0\" ] && [ -s \"$skipped_locations_file\" ]` builds build_inline_comment_failure_body from the skipped-locations file and `del(.comments)` before POST, so findings are surfaced as overview receipts instead of being dropped; Coverage execution evidence reports Result: PASS with supported repository test suites passed, which includes the 1519-line tests/test_opencode_inline_comment_fallback.py added by this PR. source-line-sha256=ca270a57beefb678df9192213cd38013c1cb4aa2b8599cf32834dbc364c57429","outcome":"falsified"},{"path":".github/workflows/opencode-review-dispatch.yml","line":5626,"hypothesis":"Any review POST failure (for example a 403 token failure) enters the one-at-a-time retry loop, hammering the API and delaying the verdict.","attack_or_counterexample":"A GitHub API 403 or 500 response instead of the expected HTTP 422, with REVIEW_PUBLICATION_STALE_HEAD unset.","evidence":"Trusted source trace at .github/workflows/opencode-review-dispatch.yml:5626: the focused changed hunk shows retry_inline_comments_one_at_a_time is reached only after the helper --is-unprocessable gate confirms a real HTTP 422 (commit 8eab74f9 'require a real HTTP 422 before suggestion retry'), attempts are capped by OPENCODE_INLINE_COMMENT_RETRY_LIMIT:-20, and REVIEW_PUBLICATION_STALE_HEAD=1 aborts the loop with return 1 into the fallback-body path; changed-file history evidence shows commits 8eab74f9, 760c8d74, and 0ab29dff updated tests/test_opencode_inline_comment_fallback.py and tests/test_opencode_agent_contract.py pinning the gate, start_line preservation, and CWE-116 sanitization, and Coverage execution evidence reports Result: PASS with supported repository test suites passed. source-line-sha256=fa0ba5fc9931ef07554cf65b7864508b3910b91b65e3d5377235946eca85e123","outcome":"falsified"}],"residual_risk":"The 966-line helper and 1519-line test file were assessed through focused hunks, commit-history evidence, and the passed supported test suites rather than full-file reads (isolated-session limitation). The one-at-a-time retry posts one review per comment; if GitHub's review-state semantics for bare COMMENT posts ever change, the PR verdict state could differ, though the first POST always carries the REQUEST_CHANGES event and review body."}- Result: APPROVE
- Reason: Evidence affirmatively supports PR #972 (surviving suggested diffs become GitHub suggestion fences, off-hunk comments dropped before POST, one-at-a-time 422 retry preserves start_line, overview receipts omit comment-closer paths): Coverage execution evidence reports Result: PASS with supported repository test suites passed, no completed failed GitHub Checks, no unresolved review threads, a new 1519-line focused test suite plus agent-contract and materializer tests updated in the same commit chain, and both adversarial probes (off-hunk comment filtering, non-422 retry gating) were falsified by source trace and passed-suite evidence.
- Head SHA:
76e68b0db98ec3f0ae6493c5ace5a8b1e27f1525 - Workflow run: 31748649739
- Workflow attempt: 1
1 participant
Summary
OpenCode already posted source-backed suggested diffs, but they were fenced as
diff, so GitHub never offered Apply suggestion.+lines from surviving RIGHT-side```diffblocks into```suggestionfences.n/a, “cannot provide”, LEFT-side comments, and fence-breaking replacements unchanged.#954/#968–#971 remain MERGEABLE with auto-merge; they are blocked by the two-approval ruleset, not a current-head code fail.
Test plan
pytest tests/test_opencode_inline_comment_fallback.py tests/test_opencode_agent_contract.py— 63 passedcoverage run -m pytest tests --ignore=tests/test_strix_quick_gate.sh— 1000 passed, 100% statements/branches, interrogate 100%