feat(eval): corpus benchmark — recall + specificity on real C# (P-012 slice 1)#50
Merged
Conversation
… slice 1) The labeled corpus was already half a benchmark (before.cs/after.cs/expected/ case.own per case), but the only thing scoring it — tests/test_corpus.py — checks the .own *reduction* and even conceded "not that the tool scanned real C#." The P-001 extractor exists now, so close the gap. scripts/benchmark.py runs the ACTUAL before.cs/after.cs through the extractor + core (own-check.sh --format sarif) and measures what the .own check cannot: - recall — the bug is CAUGHT in real before.cs (>= 1 verdict); - specificity — the real after.cs (the fix) is SILENT (0 verdicts, no false alarm). The metric is code-agnostic (OWN001 vs OWN014 both count as caught), so it survives a sound reclassification an exact-code match would spuriously fail; a verdict is an error/warning SARIF result, the advisory OWN050 note excluded. Aggregate: one defensible line "N caught / N · K fixes clean / N · F false positives". Validated two ways (the harness pattern): --selftest pins the SARIF-parse + scoring logic with no SDK (wired into the lint job's selftests), and a new dotnet-backed corpus-benchmark CI job runs the real benchmark — materializing the WindowsDesktop ref pack (OWN_EXTRA_REF_DIRS) so framework events resolve, same as the oracle/mine jobs. The gate: every before.cs caught and every after.cs silent. This is the measurement spine — a reproducible recall/specificity number over real C#, pinned against regression, and the verifiable reward for any future learning loop (built before any proposer/LLM layer, never trusting a source). docs: P-012 status, the stale test_corpus note, and docs/notes/corpus-benchmark.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
…at floor The first real run measured 3/9 bugs caught in real C# · 9/9 fixes clean · 0 false positives. The .own reductions all fire (tests/test_corpus.py); the 6 missed are cases the C# *frontend* does not yet extract (pool double-return/use-after-return, ownership-handoff, a few dispose/escape shapes) — the extractor's recall debt, now measured. So gate honestly rather than hard-asserting a 9/9 the tool cannot yet deliver: precision is absolute (every after.cs silent, zero false positives — a regression there is crying wolf on correct code), recall is pinned at a floor (--min-recall, set to 3 in CI) that ratchets up as extraction coverage grows. The number is *reported* and forbidden from regressing — exactly what a measurement spine does. benchmark.py: extract gate() (pure, selftested), run() takes min_recall, --min-recall CLI; selftest now covers the gate (11 checks). docs/notes/corpus-benchmark.md and P-012 record the honest first number + the itemized recall backlog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds ChangesCorpus Benchmark Harness
Sequence Diagram(s)sequenceDiagram
participant CLI as main(argv)
participant Run as run()
participant ScoreCorpus as score_corpus()
participant Scan as _scan()
participant Gate as gate()
CLI->>Run: root, corpus_dirs, min_recall
Run->>ScoreCorpus: discover corpus cases
ScoreCorpus->>Scan: run own-check.sh on before.cs
Scan-->>ScoreCorpus: SARIF verdict codes
ScoreCorpus->>Scan: run own-check.sh on after.cs
Scan-->>ScoreCorpus: SARIF verdict codes
ScoreCorpus-->>Run: list[CaseScore]
Run->>Gate: caught, clean, total, fps, min_recall
Gate-->>Run: failure reasons
Run-->>CLI: exit code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1e6f6ebde
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+119
to
+123
| proc = subprocess.run( | ||
| [script, "--root", root, "--format", "sarif", "--", cs_file], | ||
| capture_output=True, text=True, | ||
| ) | ||
| return proc.stdout |
There was a problem hiding this comment.
When own-check.sh exits with a hard error (for example the extractor crashes, dotnet is unavailable, or fact loading returns rc >= 2), this returns whatever stdout happened to contain and sarif_codes() treats empty/malformed output as no verdict. That means an after.cs analysis failure is reported as clean, and failures in cases below the recall floor are silently hidden, so the benchmark can pass without actually scanning all real C# inputs. Please check proc.returncode here and surface stderr/raise on non-zero hard failures.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/corpus-benchmark.md`:
- Around line 19-21: The fenced code block containing the benchmark output is
missing a language tag specification, which violates the markdown linting rule
MD040. Add a language hint such as "text" immediately after the opening triple
backticks of the fenced code block that contains the benchmark statistics line
(the one showing "3/9 bugs caught in real C#...").
In `@scripts/benchmark.py`:
- Around line 119-123: The _own_check function in the subprocess.run call does
not validate the process exit status and lacks a timeout mechanism. Modify the
subprocess.run invocation to include a timeout parameter to prevent hangs, and
after the call completes, check the proc.returncode value and raise an exception
if it is non-zero instead of silently returning stdout which may be empty on
failure. This ensures the benchmark fails fast with clear error information
rather than recording misleading results when the checker tool encounters
errors.
- Around line 269-274: The `--min-recall` argument currently accepts negative
values, which would trivially pass the recall gate and weaken the regression
contract by mistake. After the `ap.parse_args(argv)` call that populates the
`args` object, add validation to ensure that `args.min_recall` is not negative.
If a negative value is provided, raise a ValueError with a clear message stating
that `--min-recall` must be a non-negative integer. Place this validation before
the `if args.selftest:` check to catch the configuration error early.
🪄 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: 3bb7be65-8450-4e0f-96ca-7f0f8bb6f81b
📒 Files selected for processing (5)
.github/workflows/ci.ymldocs/notes/corpus-benchmark.mddocs/proposals/P-012-bug-corpus-mining.mdscripts/benchmark.pytests/test_corpus.py
…floor (review) Codex + CodeRabbit: the benchmark must not pass on output that never ran. - own-check exits 0 for clean AND findings (no --fail-on-finding), so any non-zero return means the file was not analyzed (extractor crash, no SDK, drifted facts). _scan now routes stdout/returncode through _verdicts_or_raise, which raises BenchmarkError on a non-zero exit (surfacing stderr) instead of letting sarif_codes read empty output as "no verdict" — which would score a failed after.cs as 'clean' and hide a sub-floor before.cs miss. run() catches it and fails loudly. Added a subprocess timeout (300s) so a hung extractor cannot stall CI (CodeRabbit). - --min-recall now uses a non-negative-int argparse type; a negative floor would trivially pass the recall gate and silently weaken the contract (CodeRabbit). - docs/notes/corpus-benchmark.md: language tag on the output fence (MD040). selftest grows to 15 checks (the fail-fast guard + the arg validation). The honest 3/9 baseline is unchanged — those scans all exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
There was a problem hiding this comment.
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 `@scripts/benchmark.py`:
- Around line 142-149: The current exception handling in the subprocess.run call
only catches subprocess.TimeoutExpired, but if the script is missing or not
executable, subprocess.run will raise an OSError that is not caught. Add an
additional except clause to catch OSError (or a broader exception like
Exception) after the subprocess.TimeoutExpired handler, and raise a
BenchmarkError with an appropriate message that indicates the process launch
failed on the given cs_file. This ensures consistent fail-loud behavior across
all failure modes in the subprocess execution.
- Around line 122-135: The function _verdicts_or_raise currently only validates
the return code and trusts that returncode == 0 means successful analysis, but
the sarif_codes parser is permissive and returns an empty set for malformed
SARIF. This causes invalid analysis output to silently score as clean findings.
Add validation after the returncode check to ensure the SARIF output from stdout
is valid and well-formed; if sarif_codes returns an empty set when stdout is not
empty and returncode is 0, raise a BenchmarkError indicating that the SARIF
output was invalid or malformed, preventing silent misscoring of unanalyzed
files.
🪄 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: 16be979d-d1a9-474b-9520-0b5cb7c58fbc
📒 Files selected for processing (2)
docs/notes/corpus-benchmark.mdscripts/benchmark.py
✅ Files skipped from review due to trivial changes (1)
- docs/notes/corpus-benchmark.md
…SARIF (review) CodeRabbit, two more fail-loud holes in the benchmark's own-check path: - A process-launch failure (own-check.sh missing / not executable) raises OSError, which bypassed run()'s BenchmarkError handler. _scan now catches OSError and raises BenchmarkError, like the timeout path. - _verdicts_or_raise trusted rc==0 and handed stdout to the *permissive* sarif_codes parser, which returns an empty set for malformed/empty output — so an unanalyzed file with a 0 exit could score as a clean 'no verdict'. Now validate stdout is a well-formed SARIF log (a runs list) before parsing; a valid log with zero results stays the legitimate clean case. selftest grows to 17 checks (valid/empty/invalid SARIF, missing-script launch failure, the existing rc/gate/arg guards). The 3/9 baseline is unchanged — real scans exit 0 with well-formed SARIF. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
This was referenced Jun 20, 2026
feat(extractor): inter-procedural consume contract → use-after-handoff OWN002 (recall 8/10→9/11)
#55
Merged
Closed
Merged
Merged
feat(pool): POOL005 — full-length view of a pooled buffer over-reads its logical length → OWN025
#71
Merged
Merged
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
2 participants
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.
What
A corpus benchmark that scores the checker against the labeled corpus on the real
before.cs/after.cs(not just the.ownreductiontests/test_corpus.pychecks). This is the measurement spine for P-012 — and the verifiable reward for any future learning loop, built before any proposer/LLM layer, never trusting a source.scripts/benchmark.pyruns the actual C# through the extractor + core (own-check.sh --format sarif) and measures what the.owncheck cannot:before.cs(≥ 1 verdict);after.cs(the fix) is silent (0 verdicts — no false alarm).The catch/clean metric is code-agnostic (OWN001 vs OWN014 both count as caught), so it survives a sound reclassification that an exact-code match would spuriously fail. A verdict is an error/warning SARIF result; the advisory
OWN050note is excluded.The first measurement (the payoff)
zombie-viewmodel, two static-event escapes); the six missed (arraypool-double-return,arraypool-use-after-return,ownership-handoff-consume,screentogif-loaded-subscription,handler-use-after-dispose,viewmodel-escapes-to-app) are cases the.ownreductions all catch but the C# frontend doesn't yet extract. The benchmark quantified + itemized the frontend's recall debt.The gate is honest, not aspirational
Precision is absolute (every fix silent, zero FP — a regression there means crying wolf on correct code); recall is pinned at a floor (
--min-recall, set to 3 in CI) that ratchets up as extraction coverage grows. We deliberately do not hard-assert a 9/9 the tool can't yet deliver — the spine reports recall and forbids it regressing. (The first commit hard-asserted 9/9 and the job went red — the benchmark catching reality; the second commit fixed the gate.)Validated two ways (the harness pattern)
--selftest(no SDK) — SARIF-parsing + scoring + gate logic pinned on embedded fixtures (11 checks); wired into the lint job's selftests.corpus-benchmarkCI job (dotnet) — runs the real benchmark, materializing the WindowsDesktop ref pack (OWN_EXTRA_REF_DIRS, same mechanism as the oracle/mine jobs) so framework events resolve. ✅ green at the honest baseline.Files
scripts/benchmark.py— the harness (sarif_codes,CaseScore,summarize,gate,--selftest,--min-recall)..github/workflows/ci.yml— lint selftest + thecorpus-benchmarkjob.tests/test_corpus.py— the stale "no C# front-end" note corrected.docs/— P-012 status + first number,docs/notes/corpus-benchmark.md.Next: raise recall case-by-case (the 6 missed are a bug-driven backlog), then mining at scale (P-012 stages 1–2).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
Generated by Claude Code
Summary by CodeRabbit
before/afterpairs and reports recall, specificity, and false positives.corpus-benchmarkjob to run the benchmark (including Windows reference-assembly support when available) and apply regression gates.before/afterscanning.