Skip to content

feat(sarif): make SARIF the live path — mine_report reads it; producers emit it#43

Merged
PhysShell merged 2 commits into
mainfrom
claude/sarif-finish
Jun 19, 2026
Merged

feat(sarif): make SARIF the live path — mine_report reads it; producers emit it#43
PhysShell merged 2 commits into
mainfrom
claude/sarif-finish

Conversation

@PhysShell

@PhysShell PhysShell commented Jun 19, 2026

Copy link
Copy Markdown
Owner

Finishes the SARIF follow-ups recorded in docs/notes/sarif-export.md (after #42 added the emitter + oracle ingestion). SARIF is now the actual eval path, not just a capability — retiring the documented parser-drift bug on the real mine/oracle runs.

What changed

  • mine_report.py reads SARIF — the second consumer of own-check output (after the oracle). parse() sniffs a {-leading runs log and reads it structurally, yielding the same finding dicts as the text path (level → severity: error→error, warning/note→advisory, so OWN050 and injected-source warnings stay advisory; kind from properties.resourceKind; the trailing [resource: …] split off the message). The regex parser is now off the default path for both consumers (oracle + miner).
  • Producers emit SARIFmine.sh defaults to --format sarif; oracle.yml runs own-check --format sarif. The intermediate (findings.txt / own.txt) carries a SARIF log and the human view stays the rendered report.md. Both consumers sniff the format, so this is backward-compatible (a text intermediate still parses); filenames kept to keep the CI edit minimal (content, not extension, is what's read).
  • Docs: sarif-export.md follow-ups → done; mining.md note; own-check.sh help.

Deferred (recorded in sarif-export.md)

  • GitHub code-scanning upload — this repo's C# is test fixtures, so uploading own-check SARIF to its own Security tab is low-value; code scanning belongs in the P-013 distribution surface, on a real target.
  • Per-rule helpUri — once a stable per-code docs anchor exists.

Validation

mine_report selftest 14/14 (new SARIF case + human/SARIF aggregation parity), oracle_compare 24/24, full suite green, ruff + mypy --strict clean, bash -n + YAML-parse clean.

⚠️ Honest caveat: oracle.yml / mine-on-push.yml trigger on claude/zen-pasteur-76hfs1 + a sentinel file, not on this branch — so this PR's CI does not exercise the flipped workflows. Both Python consumers are tested and the flip is mechanical + backward-compatible; the live validation is the next sentinel-triggered mine/oracle run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED


Generated by Claude Code

Summary by CodeRabbit

  • New Features
    • Code analysis findings now emit SARIF by default across the analysis pipeline.
    • Reporting now accepts Own.NET SARIF input and converts results consistently.
  • Documentation
    • Updated mining and SARIF export instructions to reflect SARIF intermediates as the live path.
    • Refreshed usage/help text for --format sarif|human options.
  • Tests
    • Expanded self-tests to cover SARIF ingestion, kind/message splitting, and malformed SARIF handling.

…rs emit it

Finishes the SARIF follow-ups from sarif-export.md: SARIF is now the actual eval
path, not just a capability — retiring the documented parser-drift bug on the
real mine/oracle runs, not only in the comparator's capability.

- mine_report.py: parse() now sniffs a {-leading `runs` log and reads SARIF
  structurally, yielding the same finding dicts as the text path (level ->
  severity: error->error, warning/note->advisory, so OWN050 and injected-source
  warnings stay advisory; kind from properties.resourceKind; trailing [resource:]
  split off the message). The regex parser is now off the default path for BOTH
  consumers (oracle + miner). New SARIF selftest + human/SARIF aggregation parity
  (14/14).
- mine.sh defaults to --format sarif; oracle.yml runs own-check --format sarif.
  The intermediate (findings.txt / own.txt) carries a SARIF log; both consumers
  sniff the format, so it is backward-compatible (a text intermediate still
  parses) and the human view stays the rendered report.md. Filenames kept to keep
  the CI edit minimal (content, not extension, is what the consumers read).
- own-check.sh help + docs (sarif-export.md follow-ups -> done; mining.md note).

Deferred (recorded in sarif-export.md): GitHub code-scanning upload (this repo's
C# is test fixtures -> belongs in the P-013 distribution surface, on a real
target); per-rule helpUri.

bash -n + YAML parse clean; mine_report 14/14, oracle 24/24, full suite green,
ruff + mypy --strict clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f6222518-976b-447a-aff9-39e1bd61a3a9

📥 Commits

Reviewing files that changed from the base of the PR and between ca487fd and 9cb8591.

📒 Files selected for processing (2)
  • docs/notes/sarif-export.md
  • scripts/mine_report.py
✅ Files skipped from review due to trivial changes (1)
  • docs/notes/sarif-export.md

📝 Walkthrough

Walkthrough

Switches the Own.NET findings pipeline to SARIF as the default format. mine_report.py gains a SARIF parser that sniffs JSON input and maps runs[].results[] into the existing finding-dict shape. mine.sh and oracle.yml are updated to invoke own-check with --format sarif. own-check.sh help text and two documentation files are updated accordingly.

Changes

SARIF-first Own.NET findings pipeline

Layer / File(s) Summary
SARIF parser and self-tests in mine_report.py
scripts/mine_report.py
Module docstring updated to document SARIF input. parse() detects {-leading JSON, loads it, and dispatches to new SARIF helpers that map runs[].results[] to finding dicts (extracting severity, ruleId, properties.resourceKind, and splitting [resource: kind] tails). Non-SARIF JSON falls through as unparsed drift. _selftest() adds 7 new checks covering SARIF aggregation, OWN050 kind/message splitting, non-SARIF JSON drift, and malformed SARIF robustness; expected pass count raised from 8 to 15.
SARIF default in producers and help text
scripts/mine.sh, .github/workflows/oracle.yml, scripts/own-check.sh
mine.sh changes the default format variable from human to sarif and updates its usage comment. oracle.yml changes the own-check invocation to --format sarif, updates post-step logging to report SARIF byte size, and clarifies inline comments about SARIF-based output consumed by the oracle comparator. own-check.sh help text adds sarif to the documented --format values.
Documentation for SARIF live path
docs/notes/mining.md, docs/notes/sarif-export.md
mining.md specifies findings.txt as a SARIF 2.1.0 log parsed structurally by mine_report. sarif-export.md replaces the old "recorded, not done here" checklist with a "Follow-up pass — SARIF is now the live path" section documenting format sniffing for backward compatibility, SARIF as the default from producers, aggregation parity across formats, stable filenames, and GitHub code-scanning upload as a deferred item.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PhysShell/Own.NET#10: Introduces the --format CLI surface in scripts/own-check.sh that this PR extends with the sarif value.
  • PhysShell/Own.NET#19: Original implementation of the corpus-mining pipeline (scripts/mine.sh, scripts/mine_report.py) that this PR updates to default to SARIF output and add SARIF parsing.
  • PhysShell/Own.NET#21: Establishes the oracle harness in .github/workflows/oracle.yml that parses Own.NET own-check output alongside Infer#/CodeQL SARIF, which this PR wires to SARIF-format own-check output.

Poem

🐇 Hoppity-hop through the pipeline I go,
Text files were cozy but SARIF's the show!
I sniff the JSON — "runs" key? Let's parse!
Each finding dict built, not a single misfarce.
The oracle reads it, the miner agrees —
SARIF by default, with backward-compat ease! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and precisely summarizes the main objective: establishing SARIF as the primary path for mine_report and producers (mine.sh, oracle.yml), which aligns with the core changes across all modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/sarif-finish

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/notes/sarif-export.md`:
- Line 62: Remove the leading space inside the inline code span in the sentence
containing "trailing ` [resource: …]`". The MD038 linting error is triggered
because there is a space immediately after the opening backtick in the code
span. Change the inline code from ` [resource: …]` to `[resource: …]` by
removing the space after the backtick.

In `@scripts/mine_report.py`:
- Around line 102-106: The _parse_sarif function lacks validation for the
structure of runs and results elements, causing it to fail on malformed SARIF
documents instead of handling them gracefully. Add type checking before
processing each run and result object to ensure they are dictionaries before
calling .get() on them or passing them to _sarif_finding(). This way, malformed
entries can be skipped or logged as warnings rather than causing the entire
function to abort.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c55869ef-40c7-442f-aec5-9c183da6cd54

📥 Commits

Reviewing files that changed from the base of the PR and between 23f0a7f and ca487fd.

📒 Files selected for processing (6)
  • .github/workflows/oracle.yml
  • docs/notes/mining.md
  • docs/notes/sarif-export.md
  • scripts/mine.sh
  • scripts/mine_report.py
  • scripts/own-check.sh

Comment thread docs/notes/sarif-export.md Outdated
Comment thread scripts/mine_report.py Outdated
Addresses the two CodeRabbit review comments on #43.

- mine_report._parse_sarif / _sarif_finding: shape-guard every SARIF node so a
  malformed runs/results/result or a garbage sub-field degrades to a default
  instead of crashing the report ({"runs":[1]}, {"runs":[{"results":1}]}, a
  non-dict result, a non-str message/locations all now skip/default). Same
  degrade-don't-crash posture as the {-leading-non-SARIF guard. New selftest
  case pins it (15/15).
- docs/notes/sarif-export.md: drop the leading space inside the `[resource: …]`
  inline code span (markdownlint MD038).

ruff + mypy --strict clean; mine_report 15/15, oracle 24/24, full suite green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
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.

2 participants