Skip to content

fix(report): filter empty LLM findings and add SARIF rules[] array#158

Merged
rng1995 merged 1 commit into
NVIDIA:mainfrom
mimran-khan:fix/sarif-compliance-and-empty-findings
Jun 23, 2026
Merged

fix(report): filter empty LLM findings and add SARIF rules[] array#158
rng1995 merged 1 commit into
NVIDIA:mainfrom
mimran-khan:fix/sarif-compliance-and-empty-findings

Conversation

@mimran-khan

Copy link
Copy Markdown
Contributor

Summary

Two SARIF output correctness issues:

  1. Empty findings from LLM failures inflate results — When the LLM meta-analyzer fails or returns partial data, findings with empty rule_id or message leak into the SARIF output, producing invalid results that confuse downstream tooling (IDEs, CI integrations).

  2. Missing tool.driver.rules[] array — The SARIF 2.1.0 spec recommends rules[] containing reportingDescriptor entries for each rule referenced by results. Many consumers (GitHub Code Scanning, VS Code SARIF Viewer) use this to display rule metadata.

Changes

src/skillspector/sarif_models.py

  • Added SarifReportingDescriptor model with id and shortDescription fields
  • Added optional rules field to SarifDriver

src/skillspector/nodes/report.py (_build_sarif)

  • Filter findings where rule_id or message is falsy before building results
  • Collect unique rule IDs during result building
  • Generate sorted rules[] array from seen rule IDs

Testing

12 new tests covering:

  • Empty/None rule_id filtered
  • Empty/None message filtered
  • All-empty findings produce zero results
  • Valid findings unchanged
  • rules[] present with correct id and shortDescription
  • Multiple findings for same rule produce single rule entry
  • Rules sorted alphabetically by id
  • No rules emitted when all findings are empty

Existing 44 tests in test_sarif.py and test_report.py continue to pass.

Fixes #146, Fixes #148

Two SARIF output issues fixed:

1. Empty findings (missing rule_id or message) from LLM meta-analyzer
   failures are now filtered before SARIF serialization. Previously
   these could produce invalid SARIF results with empty ruleId.

2. SARIF output now includes the tool.driver.rules[] array containing
   reportingDescriptor entries for each unique rule referenced by
   results. This brings output closer to SARIF 2.1.0 spec compliance
   and enables IDE integrations that require rule metadata.

Added SarifReportingDescriptor model to sarif_models.py.

Fixes NVIDIA#146, NVIDIA#148
@mimran-khan
mimran-khan force-pushed the fix/sarif-compliance-and-empty-findings branch from e5a5739 to dbbdb64 Compare June 23, 2026 09:45

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: Approve — clean, spec-compliant SARIF improvement, well tested.

What's good

  • _build_sarif now skips findings missing rule_id/message (src/skillspector/nodes/report.py, ~L26-28) so malformed entries don't emit invalid SARIF results.
  • Builds a deduped, sorted tool.driver.rules[] from referenced rule IDs (~L36-45) and sets rules=None when empty (~L56), so the field is omitted rather than emitted as [] — correct SARIF 2.1.0 behavior. New SarifReportingDescriptor model with proper shortDescription aliasing.

Non-blocking nits

  • SARIF results[] don't set ruleIndex to cross-reference the new rules[] array. Optional in 2.1.0, but adding it improves consumer linkage.
  • The empty/malformed-finding filter is applied only in the SARIF path; JSON/markdown/terminal renderers still emit those malformed findings. Minor inconsistency — consider filtering once upstream.

Tests

Thorough: empty/None rule_id/message filtering, all-empty → zero results, dedup, sort order, rules omitted when empty, schema/version. LGTM.

@rng1995
rng1995 merged commit 48fbbcd into NVIDIA:main Jun 23, 2026
@mimran-khan
mimran-khan deleted the fix/sarif-compliance-and-empty-findings branch July 3, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants