fix(report): re-derive results.vulnerable from deduped findings; emit protected as its own key - #381
Merged
Merged
Conversation
… protected as its own key
pipeline_output.json reported results.vulnerable: 183 alongside 175 entries
in findings and 175 disclosure docs on disk — 183-175=8, exactly the dedup
count logged by _dedup_caller_callee. The same dict also folded protected
into safe (lossy): the summary template has a Protected row and printed
Protected | 0 where the metric blocks said 414/408.
Two changes to the results block in build_pipeline_output:
(1) vulnerable = len(findings) (the DEDUPED list — the invariant the issue
asks for: results.vulnerable == len(findings)), plus
vulnerable_before_dedup and a deduplicated delta so the difference is
explicit rather than contradictory;
(2) protected is emitted as its own key; safe is no longer
safe+protected. The unit-partition now closes as
vulnerable + deduplicated + safe + protected + inconclusive + errors
(+ needs_review) == total (F13 extended).
The F13 reconciliation tests are updated to the new contract (they locked
in the old fold); test_issue289_results_recompute.py locks the new one,
including the invariant on a fixture where the dedup FIRES (a.py:callee
collapsed into a.py:caller, same CWE, single-caller edge).
Counts (commands run at commit time):
- pytest tests/test_issue289_results_recompute.py tests/report/test_results_bucket_reconciliation.py -q → 9 passed
- mutation smoke: fix reverted → 8 failed, 1 passed; re-applied → 9 passed
- hermeticity oracle: env -i HOME=/tmp/fakehome-noconfig → 9 passed
- pytest tests/ -q → 3153 passed, 2 failed (pre-existing zig local-env,
stash-verified; CI green with them), 32 skipped
- ruff check . → clean
Fixes #289
gadievron
requested review from
dgeyshis,
shahar-davidson and
sounil
as code owners
August 28, 2026 20:25
gadievron
added a commit
that referenced
this pull request
Aug 28, 2026
…where, not as completed Five counter sites branched on error vs everything-else, so INCOMPLETE work counted as COMPLETED: a verify checkpoint summary reported 175 completed where 41 verifications produced a verdict (134 incomplete were folded in; 175+48=223 looked consistent while the number that matters was 4.3x over-reported). The three per-unit states: completed (verdict reached), incomplete (the loop ran out / degenerate exit), errored. Sites fixed: - finding_verifier: _verify_one branches three-way on verification.incomplete (truthful note; no false 'Changed from X to X'); the summary callback has a third bucket; restored-incomplete checkpoints seed it (resume arithmetic unchanged); the FAM-REPORT-2 severity-upgrade fail-safe PROPAGATES correct_finding on the incomplete path (wave catch: not propagating silently dropped an upgraded vuln at the disclosure filter). - context_enhancer: _update_summary buckets INCOMPLETE_CLASSIFICATION; restored-incomplete units seed it; the auto-retry loop flips an error to the incomplete bucket when a retry lands on the degenerate exit (wave catch). - StepCheckpoint.write_summary ALWAYS emits completed/incomplete/errors (completed+incomplete+errors == total_units, all three visible). - StepCheckpoint.status() — the Go CLI resume path's source of truth — classifies verify/enhance incomplete checkpoints into the third bucket and honors the #286 top-level error key (which it previously missed, counting errored verify checkpoints completed); Go Summary/Info structs pair the schema (rule: Python + Go sides together) and the resume prompt shows the third bucket when nonzero (wave catch). - core/enhancer: units_enhanced excludes incomplete. - experiment.py harness: verifications_incomplete metric (harness-only site; issue lists it 'for completeness'). ADJUDICATED DEVIATION (wave BLOCKER, accepted after re-derivation): 'anconclusive' is NOT bucketed as incomplete. It is a first-class COMPLETED verdict — verdict_taxonomy FINDING_VERDICT_ORDER, the Stage-1 prompt's own enum ('safe|protected|vulnerable|inconclusive'), the #289/PR #381 results partition, and experiment.py's ground-truth metrics all treat it so. The issue's analyzer row conflated a deliberate 'I analyzed it and am unsure' answer with the degenerate no-verdict state; analyze has no third state. Wave findings adjudicated (R06): 3 BLOCKER + 2 MAJOR, all verified in-code before acceptance; the analyzer-classification BLOCKER resolved by REVERTING my first implementation. Counts (commands run at commit time): - pytest tests/test_issue293_three_state_counters.py -q → 13 passed - mutation smoke: production stashed → 13 failed; restored → 13 passed - hermeticity oracle: env -i HOME=/tmp/fakehome-noconfig → 13 passed - pytest tests/ -q → 3182 passed, 2 failed (pre-existing zig local-env, stash-verified; CI green with them), 32 skipped - ruff check . → clean - go build ./... + go vet ./internal/checkpoint/ + gofmt (my file clean) Fixes #293
gadievron
added a commit
that referenced
this pull request
Aug 28, 2026
…where, not as completed Five counter sites branched on error vs everything-else, so INCOMPLETE work counted as COMPLETED: a verify checkpoint summary reported 175 completed where 41 verifications produced a verdict (134 incomplete were folded in; 175+48=223 looked consistent while the number that matters was 4.3x over-reported). The three per-unit states: completed (verdict reached), incomplete (the loop ran out / degenerate exit), errored. Sites fixed: - finding_verifier: _verify_one branches three-way on verification.incomplete (truthful note; no false 'Changed from X to X'); the summary callback has a third bucket; restored-incomplete checkpoints seed it (resume arithmetic unchanged); the FAM-REPORT-2 severity-upgrade fail-safe PROPAGATES correct_finding on the incomplete path (wave catch: not propagating silently dropped an upgraded vuln at the disclosure filter). - context_enhancer: _update_summary buckets INCOMPLETE_CLASSIFICATION; restored-incomplete units seed it; the auto-retry loop flips an error to the incomplete bucket when a retry lands on the degenerate exit (wave catch). - StepCheckpoint.write_summary ALWAYS emits completed/incomplete/errors (completed+incomplete+errors == total_units, all three visible). - StepCheckpoint.status() — the Go CLI resume path's source of truth — classifies verify/enhance incomplete checkpoints into the third bucket and honors the #286 top-level error key (which it previously missed, counting errored verify checkpoints completed); Go Summary/Info structs pair the schema (rule: Python + Go sides together) and the resume prompt shows the third bucket when nonzero (wave catch). - core/enhancer: units_enhanced excludes incomplete. - experiment.py harness: verifications_incomplete metric (harness-only site; issue lists it 'for completeness'). ADJUDICATED DEVIATION (wave BLOCKER, accepted after re-derivation): 'anconclusive' is NOT bucketed as incomplete. It is a first-class COMPLETED verdict — verdict_taxonomy FINDING_VERDICT_ORDER, the Stage-1 prompt's own enum ('safe|protected|vulnerable|inconclusive'), the #289/PR #381 results partition, and experiment.py's ground-truth metrics all treat it so. The issue's analyzer row conflated a deliberate 'I analyzed it and am unsure' answer with the degenerate no-verdict state; analyze has no third state. Wave findings adjudicated (R06): 3 BLOCKER + 2 MAJOR, all verified in-code before acceptance; the analyzer-classification BLOCKER resolved by REVERTING my first implementation. Counts (commands run at commit time): - pytest tests/test_issue293_three_state_counters.py -q → 13 passed - mutation smoke: production stashed → 13 failed; restored → 13 passed - hermeticity oracle: env -i HOME=/tmp/fakehome-noconfig → 13 passed - pytest tests/ -q → 3182 passed, 2 failed (pre-existing zig local-env, stash-verified; CI green with them), 32 skipped - ruff check . → clean - go build ./... + go vet ./internal/checkpoint/ + gofmt (my file clean) Fixes #293
This was referenced Aug 28, 2026
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
1 participant
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.
Summary
pipeline_output.jsoncontradicted itself:results.vulnerable: 183alongside 175 entries infindingsand 175 disclosure docs on disk —183-175=8, exactly the count logged by_dedup_caller_callee. The sameresultsdict also foldedprotectedintosafe(lossy): the summary template has a Protected row and printedProtected | 0where the metric blocks said 414/408.Two changes to the
resultsblock inbuild_pipeline_output:vulnerableis re-derived from the deduped findings list (len(findings)— the invariant the issue asks for:results.vulnerable == len(findings)), plusvulnerable_before_dedupand adeduplicateddelta so the difference is explicit rather than contradictory.protectedis its own key — the lossysafe = safe + protectedfold is gone; the summary template's Protected row is now fillable from real data.The unit-partition closes as
vulnerable + deduplicated + safe + protected + inconclusive + errors (+ needs_review) == total(F13 extended — a deduped unit was still vulnerable; it is reported once under its caller). The F13 reconciliation tests are updated to this contract (they had locked in the old fold); new tests lock the invariant on a fixture where the dedup fires.Note: the private-run figures (183/175/414) quoted from the issue are provenance-labeled there and not independently reproducible; the code defects and the fix contract are fully checkable and test-covered.
Test plan
9 hermetic tests: the
vulnerable == len(findings)invariant on a dedup-firing fixture (callee collapsed into single-caller, same CWE); explicit pre-dedup/delta counts;protectedas own key; partition reconciliation including the delta; updated F13 suite.Verification evidence (commands + results)
pytest tests/test_issue289_results_recompute.py tests/report/test_results_bucket_reconciliation.py -qenv -i HOME=/tmp/fakehome-noconfig python -m pytest <same suite>pytest tests/ -qruff check .(CI scope)Fixes #289