fix(reviewer): bind failed checks to actionable source evidence - #548
Conversation
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughFinding 계약을 확장하고, 실패한 현재 헤드 검사에 대한 개별 RCA 바인딩과 제안 diff 앵커 검증을 추가했습니다. GitHub Actions 로그 수집은 검증된 job URL을 사용하며, 리뷰 본문과 인라인 suggestion 게시 형식도 확장했습니다. Changes리뷰 계약 및 게이트
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant apply_gates
participant GitHub
Agent->>apply_gates: 구조화된 Finding과 suggested_diff 제출
apply_gates->>apply_gates: 실패 검사 RCA와 diff 앵커 검증
apply_gates-->>GitHub: 검증된 verdict 게시
GitHub-->>GitHub: RIGHT-side suggestion comment 생성
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 review |
|
* feat(reviewer): enforce actionable finding contract Signed-off-by: Seongho Bae <me@seonghobae.me> * test(reviewer): align causal findings with action contract Signed-off-by: Seongho Bae <me@seonghobae.me> --------- Signed-off-by: Seongho Bae <me@seonghobae.me>
|
@coderabbitai review |
|
seonghobae
added
bug
ChatGPT Codex Connector
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| elif line[0] != "-": | ||
| line_number = None |
There was a problem hiding this comment.
🟡 No-newline patches reject valid suggestions
A standard no-newline marker resets _right_side_diff_lines before the replacement line. Valid suggestions then block the entire review as unanchored.
Prompt for agents
Update reviewer/noema_reviewer/gating.py so _right_side_diff_lines handles the standard unified-diff `\ No newline at end of file` metadata without terminating the active hunk. The marker does not consume either side's line number and can appear between a deleted line and its added replacement. Add a regression test using an actual one-line replacement where both old and new files lack a trailing newline, and verify the new-side line remains a valid suggestion anchor.
Was this helpful? React with 👍 or 👎 to provide feedback.
| actionable_checks = { | ||
| finding.check_name | ||
| for finding in verdict.findings | ||
| if finding.check_name is not None | ||
| and finding.severity in BLOCKING_SEVERITIES | ||
| and finding.path in changed_paths | ||
| and isinstance(finding.line, int) | ||
| and not isinstance(finding.line, bool) | ||
| and finding.line > 0 |
There was a problem hiding this comment.
🟡 Unrelated evidence clears failed checks
A finding with any evidence_type can clear a failed check by copying its name. The gate never requires failed-check or log evidence.
| actionable_checks = { | |
| finding.check_name | |
| for finding in verdict.findings | |
| if finding.check_name is not None | |
| and finding.severity in BLOCKING_SEVERITIES | |
| and finding.path in changed_paths | |
| and isinstance(finding.line, int) | |
| and not isinstance(finding.line, bool) | |
| and finding.line > 0 | |
| actionable_checks = { | |
| finding.check_name | |
| for finding in verdict.findings | |
| if finding.check_name is not None | |
| and finding.severity in BLOCKING_SEVERITIES | |
| and finding.evidence_type is EvidenceType.FAILED_CHECK | |
| and finding.path in changed_paths | |
| and isinstance(finding.line, int) | |
| and not isinstance(finding.line, bool) | |
| and finding.line > 0 | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| actionable_checks = { | ||
| finding.check_name | ||
| for finding in verdict.findings | ||
| if finding.check_name is not None | ||
| and finding.severity in BLOCKING_SEVERITIES | ||
| and finding.path in changed_paths | ||
| and isinstance(finding.line, int) | ||
| and not isinstance(finding.line, bool) | ||
| and finding.line > 0 | ||
| } | ||
| unresolved = [name for name in failed if name not in actionable_checks] |
There was a problem hiding this comment.
🟡 Duplicate failures share one finding
Two failed runs with the same name are both cleared by one finding. actionable_checks loses occurrence counts because it is a set.
Prompt for agents
Preserve multiplicity when matching failed Check Runs to findings in reviewer/noema_reviewer/gating.py. Each blocking finding must consume at most one failed check occurrence, even when multiple CheckConclusion entries share the same name. Add a regression test with two failed checks named identically and one bound finding, which must leave one blocker unresolved.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Scope
Bind failed checks to actionable current-head source evidence while keeping provider/model routing, quarantine/security and outbound authority outside Noema.
Finding.lineremains an exact positive integer orNone.Current exact authority — 2026-09-07 KST
Protected Noema is GitHub-verified
main@4c1d174adae3a3cc1ced54913ac2515d768647efafter normal #536 integration. Protected central.github/mainis GitHub-verified78a4937c684a54ca8e415822c913742f41c6efc4after #2009.#548 is ordinary/non-force converged onto the protected Noema foundation. Helper #557 merged protected
main@4c1d174...into the feature branch with the normal merge method, producing unchanged exact headfb44888bd571cae61dbfc93c1b46675855fbfc9c. This preserves both histories; no force push, destructive rebase, squash, or predecessor-GREEN reuse occurred.The failed-check/source-binding delta remains valid: current-head failed checks bind to actionable source evidence and
Finding.lineretains the exact positive-integer-or-None contract. The merge preserves #536's provider-neutralnoema-coreShared Kernel and its package/install/test wiring together with #548's source-provenance schema, parser/sandbox contract, prompt evidence fields, tests and documentation. No provider/model discovery, retry/failover, quarantine/security, outbound authority or gate threshold is moved into this lane.Fresh exact-head verification
For unchanged exact
fb44888bd571cae61dbfc93c1b46675855fbfc9c:ci 34097045395: terminal success;reviewer-ci 34097045410: terminal success;Security Scan 34097045483: terminal success;patch-validator-image 34097045443: in progress.The image job is assigned to GitHub-hosted runner
1001738797; exact checkout, stale-head refusal, Trivy/Syft/Grype setup, exact dependency materialization, and Buildx setup completed successfully.Build exact-head patch-validator imageremains the active step. No terminal image GREEN is claimed.Fresh review-thread authority is clean. Keep Draft until the unchanged exact head receives terminal-success image verification and protected Noema/central identities are rechecked immediately before normal integration. Predecessor checks do not transfer.
Summary by CodeRabbit
새로운 기능
버그 수정
문서