fix(oracle): count Infer# PULSE_RESOURCE_LEAK + product-only scoping (--exclude-tests)#26
Conversation
The first real three-way run (DapperLib/Dapper) exposed two issues: 1. Infer# reported 0 leaks in the comparison, but it actually found 3 — modern Infer# (Pulse engine, v1.2+) emits `PULSE_RESOURCE_LEAK`, not the older `RESOURCE_LEAK`, so the comparator's INFER_LEAK set missed them and they were mis-filed under "out of scope" context. Now match the Infer leak family by substring (`*RESOURCE_LEAK*` / `*MEMORY_LEAK*`), robust to version drift. 2. Scope asymmetry: Infer# builds only the product project (the #24 heuristic), while own-check and CodeQL scan the whole source tree — so their leak counts were inflated by test/benchmark code the others never saw (agree was depressed accordingly). Add `--exclude-tests` (comparator) + an `include_tests` workflow input (default off => product-only). The exclusion is applied uniformly to all tools in the comparator rather than per-tool, because CodeQL's `paths-ignore` is unreliable for compiled C# with `build-mode: none`. Report header notes the scope. docs/notes/oracle.md updated (scope note + the input). selftest now 15/15 (adds PULSE classification + the exclude-tests predicate); ruff clean; run_tests.py exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds test-path exclusion to the oracle comparison pipeline. A new ChangesOracle comparator: exclude-tests filtering + Pulse rule fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 @.github/workflows/oracle.yml:
- Around line 37-40: The `include_tests` input is defined as a free-form string
without type specification, causing case-sensitive comparisons to fail for
values like "True" or "TRUE". Add `type: boolean` to the `include_tests` input
definition to normalize it to a string literal "true" or "false", and update the
condition at line 172 where this input is evaluated to use the
`${INCLUDE_TESTS,,}` parameter expansion operator for case-insensitive matching
instead of exact lowercase comparison.
In `@scripts/oracle_compare.py`:
- Around line 239-241: The scope note is currently shown only when
excluded_tests has a value greater than zero, which creates ambiguity when the
exclude-tests mode is enabled but no tests are actually excluded. Instead of
checking if excluded_tests is non-zero in the conditional at lines 239-241,
check if the exclude-tests flag or mode is enabled (the configuration setting
that controls whether test filtering is active). This way the scope note will
always be displayed whenever product-only filtering is active, regardless of the
count. Apply the same fix to the similar conditional block at lines 426-427.
🪄 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: 38efc7e0-41dd-481c-a21a-3b01d74e93e6
📒 Files selected for processing (3)
.github/workflows/oracle.ymldocs/notes/oracle.mdscripts/oracle_compare.py
Both Minor, both valid:
- oracle.yml: `include_tests` was a free-form string matched against exact
"true", so `True`/`TRUE` (e.g. via the API) would silently still exclude tests.
Declare it `type: boolean` (dispatch UI checkbox; normalised to "true"/"false")
and lower-case the shell compare (`${INCLUDE_TESTS,,}`).
- oracle_compare.py: the scope note was gated on `excluded_tests > 0`, so a
product-only run that excluded nothing read like a full-scope run. Gate the
line on the mode (`--exclude-tests` enabled), not the count; it now always
states the scope (even "0 excluded").
selftest 16/16 (adds the scope-note-at-zero check); ruff clean; oracle.yml valid YAML.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
Make the first three-way run correct: Infer# leaks + product-only scope
The first real Own.NET / Infer# / CodeQL run on DapperLib/Dapper finally fired all three tools — and exposed two issues in the comparator. Both fixed here.
1. Infer# showed 0 leaks — it actually found 3
Modern Infer# (Pulse engine, v1.2+) emits
PULSE_RESOURCE_LEAK, not the olderRESOURCE_LEAK. The comparator'sINFER_LEAKset predated that rename, so Infer#'s 3 resource leaks were mis-classified as "out of scope" context (you could seeinfersharp:PULSE_RESOURCE_LEAK x3sitting in the 269-item context bucket).→ Match the Infer leak family by substring (
*RESOURCE_LEAK*/*MEMORY_LEAK*), robust to version drift. Real totals: Own.NET 1 / Infer# 3 / CodeQL 15.2. The three tools were comparing different code
Infer# builds only the product project (the #24 heuristic), but own-check and CodeQL scan the whole source tree — so their leak counts were inflated by
tests//benchmarks/code Infer# never saw, andagreewas depressed because the tools literally looked at different files.→ New
--exclude-tests(comparator) + aninclude_testsworkflow input (default off ⇒ product-only). Findings undertest/benchmark/sample/examplepaths are dropped uniformly across all tools, so the diff is apples-to-apples. Done in the comparator (not per-tool) because CodeQL'spaths-ignoreis unreliable for compiled C# withbuild-mode: none. The report header notes the scope;include_tests=truecompares across everything.What the Dapper recall gap actually was (for context)
Of CodeQL's 15: 6 are exception-path (
cs/dispose-not-called-on-throw— we don't model exceptional edges, by design), 6 areDataTable(we treat it as dispose-optional — our Task/DataTable exemption that keeps precision), and 3 are genuine coverage gaps (SqlCommand/UnitOfWork/CancellationTokenSource— async methods we skip, or unresolved customIDisposable). So our gap is policy + coverage, not logic — a strong case for the extractor's--statsand async support next.Verified
oracle_compare.py --selftest15/15 (adds PULSE classification + the exclude-tests predicate) · exclude-tests smoke check passes ·oracle.ymlvalid YAML ·ruffclean ·run_tests.pyexit 0. No change to the three tools — comparator + docs + one workflow input only.After merge
Re-dispatch
oracle (cross-tool),repo = DapperLib/Dapper, all inputs empty: Infer# now counts (3), and the diff is restricted to Dapper's product code — soagreefinally has a chance to be non-zero.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
New Features
include_testsworkflow option to control whether test/benchmark/sample/example code is included in oracle comparisons (default: excluded).Bug Fixes
Documentation