fix(report): "deduplicated" is a true dedup delta — never negative (#423)

libs/openant-core/core/reporter.py

@@ -404,7 +404,27 @@ def build_pipeline_output(
     call_graph_path = os.path.join(
         os.path.dirname(os.path.abspath(results_path)), "call_graph.json"
     )
+    # #423: the pre-dedup count is over the SAME population `vulnerable`
+    # counts (the confirmed findings list) — NOT the analyze-stage metrics.
+    # Stage-2 adjudication can retain/create vulnerabilities Stage-1
+    # detection did not flag (a live run: detect 0 vulnerable, verify 2 that
+    # stayed), so the metrics population and the findings population diverge
+    # and the #289/#381 reconciliation contract (before >= after) broke,
+    # emitting `"deduplicated": -2` — a count that can never be explained as
+    # deduplication. Deriving both from one list makes the delta a TRUE
+    # dedup delta.
+    confirmed_before_dedup = len(confirmed)
     confirmed = _dedup_caller_callee(confirmed, all_results, call_graph_path)
+    # #423 (wave r1): the disclosure/metrics overlap, split by the recount's
+    # own predicates (verifier.py:497-513) — the confirmed rows the recount
+    # ALSO buckets into errors / needs_review, counted once (in vulnerable,
+    # via the findings list) and subtracted from those buckets.
+    _k_overlap_error = sum(1 for c in confirmed if c.get("error"))
+    _k_overlap_needs = sum(
+        1 for c in confirmed
+        if not c.get("error")
+        and isinstance(c.get("verification"), dict)
+        and c["verification"].get("incomplete"))
 
     # Build findings in PipelineOutput schema
     findings_data = []
@@ -726,21 +746,40 @@ def build_pipeline_output(
             # report 183 in results.vulnerable alongside 175 entries in
             # findings. The pre-dedup count and the dedup delta are
             # explicit so the difference is explainable, not contradictory.
+            # #423: before_dedup counts the SAME list's PRE-dedup length
+            # (captured above _dedup_caller_callee), so the delta is a true
+            # dedup delta — never negative (the analyze-metrics population
+            # diverges from the findings population when Stage-2 retains
+            # vulnerabilities Stage-1 did not flag).
             "vulnerable": len(findings_data),
-            "vulnerable_before_dedup": metrics.get("vulnerable", 0) + metrics.get("bypassable", 0),
-            "deduplicated": (metrics.get("vulnerable", 0) + metrics.get("bypassable", 0)) - len(findings_data),
+            "vulnerable_before_dedup": confirmed_before_dedup,
+            "deduplicated": confirmed_before_dedup - len(findings_data),
             # #289: protected is its OWN key — the lossy safe-fold destroyed
             # a verdict the pipeline computes and the template has a row for.
             "safe": metrics.get("safe", 0),
             "protected": metrics.get("protected", 0),
             "inconclusive": metrics.get("inconclusive", 0),
-            # F13: errored units are part of `total` (see units_analyzed above), so the
-            # results buckets must include them or they cannot reconcile to `total`.
-            "errors": metrics.get("errors", 0),
-            # #284 (wave catch): incomplete verifications are ALSO part of total —
-            # the partition must carry needs_review or the buckets cannot reconcile
-            # on any scan with incomplete units (the F13 invariant, extended).
-            "needs_review": metrics.get("needs_review", 0),
+            # #423 (wave r1, three axes — the F13 partition): the metrics
+            # recount and the disclosure list are TWO populations that
+            # deliberately overlap (verifier.py's #284 note keeps
+            # errored/incomplete rows whose Stage-1 finding is vulnerable in
+            # confirmed_findings; the recount buckets those same rows into
+            # errors/needs_review first). With `vulnerable` counting the
+            # disclosure list, the overlap rows are counted ONCE here and
+            # their metrics-bucket entries are subtracted below — otherwise
+            # the partition over-sums total by exactly the overlap (the
+            # pre-round fix traded the negative `deduplicated` for this
+            # silent +k on the live run's own shape).
+            # F13: errored units are part of `total` (see units_analyzed
+            # above), so the results buckets must include them or they
+            # cannot reconcile to `total` — MINUS the overlap already
+            # counted in `vulnerable`.
+            "errors": max(0, metrics.get("errors", 0) - _k_overlap_error),
+            # #284 (wave catch): incomplete verifications are ALSO part of
+            # total — the partition must carry needs_review or the buckets
+            # cannot reconcile (the F13 invariant, extended) — minus the
+            # same overlap.
+            "needs_review": max(0, metrics.get("needs_review", 0) - _k_overlap_needs),
             "total": total_units,
         },
         "findings": findings_data,

libs/openant-core/tests/test_issue423_dedup_reconcile.py

@@ -0,0 +1,143 @@
+"""#423: "deduplicated" is a true dedup delta — never negative.
+
+`vulnerable` is re-derived from the DEDUPED findings list (#289), but
+`vulnerable_before_dedup` came from the ANALYZE-stage metrics
+(`metrics.vulnerable + metrics.bypassable`) — a DIFFERENT population. When Stage-2
+adjudication retains/creates vulnerabilities Stage-1 detection did not flag (the live
+run: detect classified 0 vulnerable; verify disagreed on 2 that stayed vulnerable per
+the verifier taxonomy), the two populations diverge and the #289/#381 reconciliation
+contract (before >= after) breaks — a live run emitted `"deduplicated": -2`, a count
+that can never be explained as deduplication.
+
+The fix derives `vulnerable_before_dedup` from the SAME population `vulnerable`
+counts — the PRE-dedup confirmed findings list — so the delta is a true dedup delta:
+Stage-2 reclassifications no longer produce negative "dedup", and a real caller/callee
+collapse still reports the exact number dropped.
+"""
+import json
+import sys
+from pathlib import Path
+
+_CORE = Path(__file__).resolve().parents[1]
+if str(_CORE) not in sys.path:
+    sys.path.insert(0, str(_CORE))
+
+from core.reporter import build_pipeline_output  # noqa: E402
+from utilities.file_io import write_json  # noqa: E402
+
+
+def _run_build(tmp_path, results, call_graph=None):
+    results_path = tmp_path / "results.json"
+    write_json(results_path, results)
+    if call_graph is not None:
+        write_json(tmp_path / "call_graph.json", call_graph)
+    out_path = tmp_path / "pipeline_output.json"
+    build_pipeline_output(
+        results_path=str(results_path), output_path=str(out_path),
+        language="python", repo_name="t/r", processing_level="reachable",
+    )
+    return json.loads(out_path.read_text())["results"]
+
+
+def _vuln(route, cwe=79):
+    return {"route_key": route, "finding": "vulnerable",
+            "verdict": "VULNERABLE", "cwe_id": cwe}
+
+
+def test_stage2_only_vulns_do_not_negative_dedup(tmp_path):
+    """The live-run shape: analyze metrics say 0 vulnerable, Stage 2 retains
+    2 — pristine emitted deduplicated=-2 (before < after). The two counters
+    must describe ONE population: before=2, after=2, delta=0."""
+    res = {
+        "dataset": "t",
+        "code_by_route": {"a.py:f": "def f(): pass", "a.py:g": "def g(): pass"},
+        "metrics": {"total": 25, "errors": 0, "vulnerable": 0, "bypassable": 0,
+                    "safe": 23, "protected": 0, "inconclusive": 0},
+        "confirmed_findings": [_vuln("a.py:f"), _vuln("a.py:g")],
+        "results": [],
+    }
+    out = _run_build(tmp_path, res)
+    assert out["vulnerable"] == 2
+    assert out["vulnerable_before_dedup"] == 2, (
+        f"before={out['vulnerable_before_dedup']} is the ANALYZE-metrics "
+        "population, not the same one `vulnerable` counts — the contract "
+        "before >= after broke on the live run"
+    )
+    assert out["deduplicated"] == 0, (
+        f"deduplicated={out['deduplicated']} — a negative dedup count can "
+        "never be explained as deduplication"
+    )
+
+
+def test_real_dedup_reports_the_true_delta(tmp_path):
+    """A genuine caller/callee collapse (same CWE, callee reachable only via
+    the caller): before=2, after=1, deduplicated=1 — from ONE population."""
+    caller, callee = _vuln("a.py:run"), _vuln("a.py:query")
+    res = {
+        "dataset": "t",
+        "code_by_route": {"a.py:run": "r", "a.py:query": "q"},
+        "metrics": {"total": 10, "errors": 0, "vulnerable": 2, "bypassable": 0,
+                    "safe": 8, "protected": 0, "inconclusive": 0},
+        "confirmed_findings": [caller, callee],
+        "results": [],
+    }
+    cg = {
+        "call_graph": {"a.py:run": ["a.py:query"]},
+        "reverse_call_graph": {"a.py:query": ["a.py:run"]},
+    }
+    out = _run_build(tmp_path, res, cg)
+    assert out["vulnerable"] == 1, out
+    assert out["vulnerable_before_dedup"] == 2
+    assert out["deduplicated"] == 1
+
+
+def test_manual_filter_path_same_population(tmp_path):
+    """No confirmed_findings key: the manual final-verdict filter feeds the
+    dedup — the same-population rule must hold on this path too."""
+    res = {
+        "dataset": "t",
+        "code_by_route": {"a.py:f": "r"},
+        "metrics": {"total": 3, "errors": 0, "vulnerable": 1, "bypassable": 0,
+                    "safe": 2, "protected": 0, "inconclusive": 0},
+        "results": [{"route_key": "a.py:f", "finding": "vulnerable",
+                     "verdict": "VULNERABLE", "cwe_id": 79},
+                    {"route_key": "a.py:s", "finding": "safe",
+                     "verdict": "SAFE"}],
+    }
+    out = _run_build(tmp_path, res)
+    assert out["vulnerable"] == 1
+    assert out["vulnerable_before_dedup"] == 1
+    assert out["deduplicated"] == 0
+
+
+def test_overlap_row_counted_once_full_reconciliation(tmp_path):
+    """famBCR panel (sonnet): the overlap-subtraction (a confirmed row that
+    ALSO carries error/incomplete) was completely untested — a row in both
+    the disclosure list AND the metrics errors/needs_review buckets must
+    subtract, so the partition reconciles to total exactly."""
+    # 3 units total; ONE row: Stage-1 vulnerable (kept in confirmed_findings)
+    # whose verification ERRORED — the metrics recount buckets it under
+    # errors; the disclosure list keeps it under vulnerable.
+    res = {
+        "dataset": "t",
+        "code_by_route": {"a.py:f": "r", "a.py:s": "s", "a.py:g": "g"},
+        "metrics": {"total": 3, "errors": 1, "needs_review": 0, "vulnerable": 0,
+                    "bypassable": 0, "safe": 2, "protected": 0, "inconclusive": 0},
+        "results": [
+            {"route_key": "a.py:f", "finding": "vulnerable", "verdict": "VULNERABLE",
+             "cwe_id": 79, "error": "LLMResponseError: filtered"},
+            {"route_key": "a.py:s", "finding": "safe", "verdict": "SAFE"},
+            {"route_key": "a.py:g", "finding": "safe", "verdict": "SAFE"},
+        ],
+        "confirmed_findings": [
+            {"route_key": "a.py:f", "finding": "vulnerable", "verdict": "VULNERABLE",
+             "cwe_id": 79, "error": "LLMResponseError: filtered"},
+        ],
+    }
+    r = _run_build(tmp_path, res)
+    # the overlap row counted ONCE: vulnerable=1 (the disclosure list),
+    # errors=0 after subtracting the overlap already counted in vulnerable
+    assert r["vulnerable"] == 1, r
+    assert r["errors"] == 0, r
+    assert r["safe"] == 2, r
+    assert r["vulnerable"] + r["errors"] + r["safe"] == r["total"] == 3, r