Fix post-reduction review actionability and final summary consistency

Author: tzhouam

src/infermatrix_copilot/engine/agent_runtime/ensemble.py

@@ -603,6 +603,15 @@ def _ok(obj) -> bool:
                     kept[i]["severity"] = str(v["severity"])
         merged["summary"] = str(reduced.get("summary")
                                 or merged.get("summary") or "")
+        if merge_key == "review_comments":
+            # Later coverage passes need the selection decisions too. Raw
+            # findings alone can otherwise resurrect a concern the reducer
+            # already established was harmless or outside this change.
+            merged["_rejected_review_comments"] = [
+                {"candidate": candidates[i], "reason": str(v.get("why") or "")}
+                for i, v in by_i.items()
+                if str(v.get("action", "")).lower() == "drop"
+            ]
     else:
         merged["summary"] = (f"ensemble of {len(samples)} samples "
                              "(merge reduction failed; unverified union)")

src/infermatrix_copilot/engine/steps/review/refinement.py

@@ -66,6 +66,11 @@ async def _promote_uncovered(ctx: StepContext, output: dict,
         "'covered by weekly only') is a FINDING wearing a validation "
         "stamp — promote it as the pointed question or ask it implies. "
         "Rules: "
+        "A true code fact is not automatically a defect. Require a concrete "
+        "current trigger and adverse effect or an unmet requirement. Do not "
+        "promote harmless unused configuration or hypothetical future code "
+        "changes. Respect REJECTED COMMENTS and their reasons; reopen one "
+        "only with new evidence that defeats the rejection. "
         "(1) ONLY promote what the raw lines already state — no new claims, "
         "no re-investigation; quote the source line in `evidence`. (2) A "
         "promoted line KEEPS its directive force: a concern naming a "
@@ -84,6 +89,9 @@ async def _promote_uncovered(ctx: StepContext, output: dict,
                                              "comment")}
                       for c in comments], ensure_ascii=False, indent=1)
         + "\n\n## RAW FINDINGS\n" + "\n".join(f"- {x}" for x in findings)
+        + "\n\n## REJECTED COMMENTS\n"
+        + json.dumps(output.get("_rejected_review_comments") or [],
+                     ensure_ascii=False)
         + ("\n\n## BLOCKERS\n" + "\n".join(f"- {x}" for x in blockers)
            if blockers else "")
         + ("\n\n## ASSUMPTIONS\n" + "\n".join(f"- {x}" for x in assumptions)
@@ -229,6 +237,11 @@ async def _second_round(ctx: StepContext, output: dict, common: dict,
         "You are the SECOND-ROUND reviewer. The first round produced the "
         "KEPT COMMENTS in your evidence; your job is ONLY its coverage "
         "holes — do not re-litigate or duplicate what is already covered.\n"
+        "Respect rejected_comments and their reasons. Reopen a rejected "
+        "concern only with new evidence defeating that reason. A true code "
+        "fact needs a current trigger and adverse effect or an unmet "
+        "requirement to become a finding; harmless unused values and "
+        "hypothetical future changes are not defects.\n"
         + (("These diff hunks (file:start-line) have NO comment and NO "
             "recorded verification near them. Review each AT MAINTAINER "
             "INLINE GRANULARITY: page to the hunk in situ, and either raise "
@@ -271,7 +284,10 @@ async def _second_round(ctx: StepContext, output: dict, common: dict,
                     "evidence": {**common["evidence"],
                                  "uncovered_hunk_diffs": hunk_evidence,
                                  "kept_comments": json.dumps(
-                                     kept, ensure_ascii=False)}}
+                                     kept, ensure_ascii=False),
+                                 "rejected_comments": json.dumps(
+                                     output.get("_rejected_review_comments")
+                                     or [], ensure_ascii=False)}}
     result, extra = await run_agent_step(
         ctx, **round_kwargs,
         max_iters=ctx.settings.review_second_round_max_iters, **routing)
@@ -333,13 +349,20 @@ async def _second_round(ctx: StepContext, output: dict, common: dict,
 2. Claim: is the asserted problem true of the code you just read? If the
    comment asserts consumers/callers/tests elsewhere, grep for them and read
    the one that decides the claim.
-3. Severity: major requires a real defect in the changed code or a required
+3. Actionability: establish a concrete trigger on this PR head and its
+   adverse effect, or a specific unmet requirement. A code fact being true
+   does not establish a defect. Unused optional configuration without an
+   affected consumer, and a problem contingent on hypothetical future code
+   changes, are not actionable. Check rejected_comments before reopening
+   a concern; identify new evidence that defeats its rejection.
+4. Severity: major requires a real defect in the changed code or a required
    update the diff lacks; "consider adding X" polish is minor at most.
 
 Verdicts:
 - confirmed: you READ the code that makes the claim true. You MUST return
   `evidence` as SELF-CONTAINED PROOF a reader with no repo access can check:
-  the decisive code line(s) QUOTED VERBATIM with their file:line, e.g.
+  the current trigger and adverse effect (or unmet requirement), together
+  with the decisive code line(s) QUOTED VERBATIM with their file:line, e.g.
   'serving_speech.py:3711 `extra_args["tts_local_seed"] = seed` — set for
   every model, no qwen3_tts gate'. A narrative like "read the file, claim
   holds" is NOT proof and scores as speculation downstream. When the
@@ -352,6 +375,9 @@ async def _second_round(ctx: StepContext, output: dict, common: dict,
   the substance, never soften a confirmed defect.
 - refuted: the code CONTRADICTS the claim (misread, already handled, wrong
   file). Refuted is an evidence conclusion, never a budget one.
+- not_actionable: the cited facts may be true, but no current defect or
+  unmet requirement is demonstrated. Explain why the proposed ask is not
+  needed on this head. This verdict drops the comment.
 - unverifiable: you could not decide within budget.
 
 Confirming from plausibility alone is the one failure this pass exists to
@@ -363,7 +389,7 @@ async def _verify_comments(ctx: StepContext, output: dict,
     """Per-comment agentic verification (val-gate lesson: with recall at
     parity the arm lost on per-comment grounding). Each draft comment gets
     one small tool-loop that must anchor and re-derive the claim on the
-    PR-time tree: refuted comments drop, unverifiable ones demote one
+    PR-time tree: refuted and non-actionable comments drop, unverifiable ones demote one
     severity step, confirmed ones may be tightened in wording/position. A
     verification-step failure keeps the comment unchanged — this pass may
     only improve precision, never silently delete recall."""
@@ -385,19 +411,23 @@ async def _one(i: int, c: dict):
                 purpose="Verify one draft review comment against the "
                         "PR-time tree.",
                 guidance=_VERIFY_GUIDANCE,
-                expected="verdict confirmed|refuted|unverifiable, with "
+                expected="verdict confirmed|refuted|not_actionable|unverifiable, with "
                          "optional comment/line/severity corrections",
                 evidence={"pr_diff": diff_ev,
                           "draft_comment": json.dumps(
-                              probe, ensure_ascii=False)},
+                              probe, ensure_ascii=False),
+                          "rejected_comments": json.dumps(
+                              output.get("_rejected_review_comments") or [],
+                              ensure_ascii=False)},
                 output_extension={
-                    "verdict": "confirmed|refuted|unverifiable",
+                    "verdict": "confirmed|refuted|not_actionable|unverifiable",
                     "comment": "optional tightened rewrite",
                     "line": "optional corrected line number",
                     "severity": "optional corrected severity",
                     "evidence": "confirmed only: verbatim-quoted decisive "
-                                "code line(s) with file:line — proof a "
-                                "repo-less reader can check"},
+                                "code line(s) with file:line, current trigger "
+                                "and adverse effect or unmet requirement — "
+                                "proof a repo-less reader can check"},
                 extra_tools=common.get("extra_tools"),
                 max_iters=ctx.settings.review_verify_max_iters)
             return i, result, (out or {})
@@ -411,10 +441,10 @@ async def _one(i: int, c: dict):
         c = dict(comments[i])
         verdict = str(out.get("verdict") or "").lower()
         if not result.ok or verdict not in ("confirmed", "refuted",
-                                            "unverifiable"):
+                                            "not_actionable", "unverifiable"):
             kept.append(c)          # fail-open: never delete on pass failure
             continue
-        if verdict == "refuted":
+        if verdict in ("refuted", "not_actionable"):
             n_drop += 1
             continue
         if verdict == "confirmed":

src/infermatrix_copilot/engine/steps/review/steps.py

@@ -295,6 +295,10 @@ async def _review_diff(ctx: StepContext) -> StepResult:
                                     f"{result.summary}",
                             outputs=result.outputs,
                             changed_files=result.changed_files)
+    draft_selection = [
+        (c.get("file"), c.get("line"), c.get("comment"), c.get("severity"))
+        for c in output.get("review_comments") or []
+    ]
     if plan is not None and result.ok:
         output = await refine_review(
             ctx, output, common=common, diff=str(diff), spec=spec,
@@ -320,10 +324,6 @@ async def _review_diff(ctx: StepContext) -> StepResult:
     published, withheld = finalize_review_dispositions(output)
     # at most three untested-function comments; the rest are named, not lost
     published, ut_dropped = cap_gap_comments(published, ut_report)
-    if ut_dropped:
-        output["summary"] = (str(output.get("summary") or "").rstrip()
-                             + "\n\nAlso without a unit test: "
-                             + ", ".join(f"`{n}`" for n in ut_dropped) + ".").lstrip()
     output["review_comments"] = published
     output["_withheld_findings"] = withheld
     comments = sorted(output.get("review_comments") or [],
@@ -333,6 +333,19 @@ async def _review_diff(ctx: StepContext) -> StepResult:
     for c in comments:
         c.pop("corroborated_by", None)
     output["review_comments"] = comments[:8]
+    final_selection = [
+        (c.get("file"), c.get("line"), c.get("comment"), c.get("severity"))
+        for c in output["review_comments"]
+    ]
+    if sorted(draft_selection, key=repr) != sorted(final_selection, key=repr):
+        # The reducer's prose describes its draft selection. After promotion,
+        # verification or a budget cut it may contradict the published set.
+        # Let the renderer derive a truthful summary from the final comments.
+        output["summary"] = ""
+    if ut_dropped:
+        output["summary"] = (str(output.get("summary") or "").rstrip()
+                             + "\n\nAlso without a unit test: "
+                             + ", ".join(f"`{n}`" for n in ut_dropped) + ".").lstrip()
     # Recorded AFTER the cut: a candidate the review chose to publish but the
     # budget dropped is `over_budget`, never `excluded`. A consumer checking
     # that no withheld finding was published must not trip over a healthy

src/infermatrix_copilot/engine/steps/review/utils.py

@@ -636,7 +636,8 @@ def _render_review_md(output: dict, pr_state: str = "") -> str:
             entry += f"\n\nProposed change (not a ready patch): {suggestion}"
         lines.append(entry)
     parts = _review_summary_parts(output)
-    parts.append("\n\n".join(lines) if lines else output.get("summary", "No findings."))
+    parts.append("\n\n".join(lines) if lines else
+                 (output.get("summary") or "No actionable findings."))
     overflow = output.get("_review_overflow") or []
     if overflow:
         # findings the comment budget cut — one line each, so a real (often

test/test_agent_ensemble.py

@@ -438,6 +438,82 @@ def test_coverage_promotion_converts_findings_to_comments(settings, trace,
     assert len(kept) == 2
     assert "remains unfixed" in kept[1]["comment"]
     assert next(trace.events("review_coverage_promoted"))["added"] == 1
+    assert "2 actionable finding(s)." in result.outputs["review_summary"]
+    assert "merged" not in result.outputs["review_summary"]
+    assert not llm._replies
+
+
+def test_rejected_facts_stay_available_and_non_actionable_additions_drop(
+        settings, trace, tmp_path, git_repo):
+    """A true unused-key observation must not become a published defect
+    merely because coverage revisits it after the reducer rejected it."""
+    settings.review_ensemble = True
+    settings.review_depth = "full"
+    settings.review_second_round = True
+    settings.review_verify_comments = True
+    settings.review_verify_concurrency = 1
+    harmless = {"file": "config.py", "line": 10, "severity": "minor",
+                "comment": "optional key also reaches an unused consumer",
+                "evidence": "config.py:10 `allowed |= {'model_config'}`"}
+    future = {"file": "config.py", "line": 40, "severity": "minor",
+              "comment": "validator may reject this if its fields change later",
+              "evidence": "config.py:40 `fields = current_fields()`"}
+    reason = "No current affected consumer; forwarding is harmless"
+    llm = ScriptedLLM([
+        contract(review_comments=[harmless],
+                 findings=["[validated] optional key is forwarded"]),
+        *[contract(review_comments=[])] * (len(_REVIEW_LENSES) - 1),
+        verdicts_reply({"i": 0, "action": "drop", "why": reason},
+                       summary="No actionable findings."),
+        Reply(blocks=[Block(type="text", text=json.dumps(
+            {"additions": [harmless]}))]),
+        contract(review_comments=[future]),
+        contract(verdict="not_actionable", evidence=reason),
+        contract(verdict="not_actionable",
+                 evidence="current_fields excludes the key on this head"),
+    ])
+    state = {"diff_text": "diff --git a/config.py b/config.py\n"
+                          "+++ b/config.py\n@@ -1,0 +1,1 @@\n+allowed = set()",
+             "task_spec": {"pr": 9}, "repo_path": str(git_repo)}
+    result = asyncio.run(register_builtin_steps(StepRegistry()).get(
+        "agent.review_diff").handler(
+            _ctx(settings, trace, tmp_path, state, llm=llm)))
+    assert result.ok
+    assert result.outputs["review_comments"] == []
+    assert result.outputs["review_summary"].endswith("**Verdict:** APPROVE")
+    assert "optional key also reaches" not in result.outputs["review_text"]
+    assert next(trace.events("review_comments_verified"))["dropped"] == 2
+    # The decision survives every boundary, including the second round and
+    # independent verifiers, instead of only living in a reducer archive.
+    for call in llm.calls[-4:]:
+        assert reason in json.dumps(call["messages"])
+    assert not llm._replies
+
+
+def test_verification_removal_clears_stale_reducer_summary(
+        settings, trace, tmp_path, git_repo):
+    settings.review_ensemble = True
+    settings.review_depth = "full"
+    settings.review_verify_comments = True
+    llm = ScriptedLLM([
+        contract(review_comments=[{"file": "config.py", "line": 1,
+                                   "severity": "major", "comment": "broken",
+                                   "evidence": "config.py:1"}]),
+        *[contract(review_comments=[])] * (len(_REVIEW_LENSES) - 1),
+        verdicts_reply({"i": 0, "action": "keep"},
+                       summary="One blocking defect remains."),
+        contract(verdict="refuted"),
+    ])
+    state = {"diff_text": "diff --git a/config.py b/config.py\n+x = 1",
+             "task_spec": {"pr": 9}, "repo_path": str(git_repo)}
+    result = asyncio.run(register_builtin_steps(StepRegistry()).get(
+        "agent.review_diff").handler(
+            _ctx(settings, trace, tmp_path, state, llm=llm)))
+    assert result.ok
+    assert result.outputs["review_comments"] == []
+    for field in ("review_text", "review_summary"):
+        assert "blocking defect remains" not in result.outputs[field]
+        assert "No actionable findings." in result.outputs[field]
     assert not llm._replies