Repository navigation
fix(noema): reject truncated-diff approvals - #2650
Open
seonghobae wants to merge 1 commit into
Open
seonghobae wants to merge 1 commit into
seonghobae wants to merge 1 commit into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 10.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check PR 제목은 잘린 diff에 대한 승인을 거부하는 핵심 변경을 정확하고 간결하게 설명합니다. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![cwl-noema-review[bot]](https://avatars.githubusercontent.com/u/295022177?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Noema LLM review
The new guard rejects approvals based on truncated diffs without changing admission of complete-diff approvals or partial-context comments and change requests. Static analysis falsified the regression hypotheses below; the added tests cover rejection, publication boundaries, and error classification.
Reviewed changed lines
scripts/ci/noema_review_gate.py:1699 (RIGHT): The conjunction limits rejection to truncated approvals. Complete-diff approvals and other supported decisions continue through the existing validation path.scripts/ci/noema_review_gate.py:1700 (RIGHT): Uses the model-output error class for an unacceptable verdict rather than treating it as transport-capacity failure.tests/test_noema_review_gate.py:1452 (RIGHT): Updates the successful-approval fixture to use a complete diff, keeping the existing transport/configuration test compatible with the new admission policy.tests/test_noema_truncated_approval.py:243 (RIGHT): The consumer regression checks that rejection leaves no publication envelope; surrounding assertions also check one model request, unchanged workflow output, and no review posts.tests/test_noema_truncated_approval.py:269 (RIGHT): The prepare/publish regression checks the exact GitHub event for complete-diff approval, truncated comment, and truncated request_changes.
Adversarial validation
scripts/ci/noema_review_gate.py:1699 (RIGHT)falsified: An otherwise valid approval can still be admitted when the diff supplied to the reviewer was truncated. — Static evaluation makes the new condition true and immediately raises NoemaModelOutputError before subsequent verdict validation. The new consumer fixture additionally constructs an oversized diff with an omitted source file and asserts rejection without a publication envelope.tests/test_noema_truncated_approval.py:269 (RIGHT)falsified: The guard also blocks complete-diff approvals or prevents actionable partial-context reviews from being published. — The condition is false for all three combinations, leaving their existing validation path unchanged. The parameterized consumer test exercises these combinations and asserts APPROVE, COMMENT, and REQUEST_CHANGES respectively, along with the exact head commit.scripts/ci/noema_review_gate.py:1700 (RIGHT)falsified: A truncated approval is rejected as provider-capacity failure and consequently enters the transport retry path. — The added rejection explicitly raises NoemaModelOutputError rather than NoemaTransportError. The regression tests distinguish these exception classes, assert unchanged workflow output for the rejected approval, and separately retain transport classification checks for HTTP failures.- Residual risk: Validation was static: the supplied tests were inspected but not executed. Surrounding production-file context was only partially provided, so runtime integration remains unverified.
Findings
- No blocking findings.
- Result: APPROVE
- Head SHA:
923d96cee78fe644d104acf2c9ca2e4e15ca99ba - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
scripts/ci/noema_review_gate.py— review and security gate shell pathtests/test_noema_review_gate.py— regression suitetests/test_noema_truncated_approval.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: noema_review_gate.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: noema_review_gate.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_noema_review_gate.py (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_noema_review_gate.py (2 files)"]
R2 --> V2["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
923d96cee78fe644d104acf2c9ca2e4e15ca99ba - Workflow run: 38510545621
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: noema_review_gate.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: noema_review_gate.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_noema_review_gate.py (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_noema_review_gate.py (2 files)"]
R2 --> V2["targeted test run"]
Contributor
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
This branch has not been deployed
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.
Adopts only the independently reviewed three-file Noema patch (SHA256 891365c4ab2a44f9ef8504abe805798e5c80daac862bbcb46fa3775277f9eddb) on main acb8b34. Rejects truncated approve with NoemaModelOutputError; preserves complete approve, comment and request_changes and the formal App publisher. Nested workflow pin already shipped in PR2646 and is not changed. Retained independent review: 153 passed. Local adoption checks: 150 passed with two Bash-3 harness failures; both failed cases passed unchanged using Bash 5; new focused regression 15 passed. Missing actual Actions execution and formal App approval remain incomplete; no security/protection/runner/key changes.
Summary by CodeRabbit