Skip to content

fix: reduce false positives in SARIF review - #510

Merged
ajianaz merged 1 commit into
developfrom
fix/sarif-false-positives-507
Aug 6, 2026
Merged

fix: reduce false positives in SARIF review#510
ajianaz merged 1 commit into
developfrom
fix/sarif-false-positives-507

Conversation

@ajianaz

@ajianaz ajianaz commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What

Three improvements to reduce false positives in cora's review output:

  1. Context line filter — Drops Minor and Info findings on unchanged (context/removed) diff lines. These are pre-existing code that appeared in the diff due to surrounding changes, not new code introduced by the PR.

  2. Enhanced system prompt — Added LANGUAGE-SPECIFIC FALSE POSITIVE AWARENESS section to the review system prompt:

    • In-place mutation methods (Vec::retain, Vec::append, Vec::sort, etc.) should not be flagged as "result ignored"
    • Error/early-return paths (Err => return) should not be flagged for missing happy-path post-conditions
  3. SARIF non-blocking — Fixed cora-review-action (separate PR) so Check for blocking issues step respects upload-sarif: 'false' input.

Why

Issue #507 identified recurring false positive patterns:

These false positives erode developer trust in automated review and create noise that masks real issues.

How

  • apply_context_line_filter() in review.rs: builds (file, line) -> DiffLineType lookup from parsed diff chunks, then retains only findings on Add lines (or Critical/Major on any line)
  • System prompt additions in llm.rs: explicit Rust mutation method list + error path guidance
  • cora-review-action/action.yml: gate blocking check behind upload-sarif == 'true'

Testing

  • cargo build — clean
  • cargo test — 22/22 pass (16 unit + 6 config)
  • cargo clippy — 0 warnings
  • cargo fmt --check — clean
  • Pre-commit cora review — passed

Related Issues

Fixes #507

Checklist

  • Code follows project style (cargo fmt)
  • No clippy warnings
  • All tests pass
  • Self-reviewed

- Add context line filter: drop Minor/Info findings on unchanged diff
  lines (pre-existing code flagged as new issue)
- Enhanced system prompt: in-place mutation awareness (Vec::retain etc.)
  and error path vs happy path distinction
- Import Severity type for filter comparison
@ajianaz
ajianaz merged commit 8e703de into develop Aug 6, 2026
13 checks passed
@ajianaz
ajianaz deleted the fix/sarif-false-positives-507 branch August 6, 2026 03:20
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.

Cora Review false positives: in-place mutation, error paths, and pre-existing code

1 participant