Skip to content

fix(review): cite trusted path:line in GitHub 422 inline fallback - #963

Closed
seonghobae wants to merge 8 commits into
mainfrom
fix/inline-comment-422-path-line-fallback
Closed

fix(review): cite trusted path:line in GitHub 422 inline fallback#963
seonghobae wants to merge 8 commits into
mainfrom
fix/inline-comment-422-path-line-fallback

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

When GitHub returns HTTP 422 for OpenCode inline review comments, the fallback already named each trusted path:line. Multi-line leftover findings that carry start_line were flattened to the last line, so authors could not see the leftover range that failed to attach.

This increment cites leftover findings as path:start-end when control JSON has a trusted start_line. Single-line leftovers stay path:line. An inverted start after the end is omitted.

Verification

  • start_line 7 / line 12 renders scripts/ci/example.py:7-12. Missing start_line stays path:line. start_line after line is dropped.
  • coverage run -m pytest tests && coverage report --show-missing twice at 100% (982 passed, scripts/ci 100% statement/branch), then interrogate 100%.

@cwl-noema-review

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.
@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: 51 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: 46568e39-20a5-4b99-beb9-8268c9160387

📥 Commits

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

📒 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
seonghobae enabled auto-merge (squash) August 13, 2026 07:00
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review please review this current head. The 422 inline-comment fallback must list each trusted path:line from the control JSON.

The publisher moved that phrase out of the workflow YAML, so the
exact-head path-policy harness failed looking in the old file.
@seonghobae

Copy link
Copy Markdown
Contributor Author

@cwl-noema-review please review this current head. The 422 sentence now lives in the Python helper; the path-policy contract follows it.

CWE-20: LLM control JSON often emits line as text. Keep a positive
digit-only string such as "9" and still reject "1.5", "0", and None.
Do not cite a 422 path:line whose path contains backticks or HTML
metacharacters, so a receipt cannot break the overview Markdown fence.
When a refused inline finding carries a trusted start_line, the 422
overview cites path:start-end. Single-line leftovers stay path:line.
An inverted start after the end is omitted.
Leftover deleted-line findings now render as path:start-end LEFT so
authors do not treat a leftover LEFT range as an attachable RIGHT hunk.
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 f09f0348 — 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 df121f24 — leftover 422-fallback paths that contain -->, <!--, or a suggestion fence are omitted (CWE-116). Head moved. Independent Noema APPROVE on this SHA is required. 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 df121f2449315a8a0cf4289e2b76ef70a13bff03.
  • 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: df121f2449315a8a0cf4289e2b76ef70a13bff03
  • Workflow run: 31754390936
  • 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 df121f2449315a8a0cf4289e2b76ef70a13bff03.
  • 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

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