feat(guardian): recall-lean universal finder + skeptic reconciliation (#249)

Recall-lean finder (drop confidence gate + finding cap, Tier-1; per-function CoT + bug-class focus areas + few-shot, Tier-2) takes pr-144 from 0/5 recall (across 5 model tiers) to median 3/5. Skeptic reconciled so it no longer auto-refutes the finder's deliberate low-confidence findings. Prompt assembly degrades cleanly on repos without CONTRIBUTING.md/ontology (read_file → "" via is_file()).

Follow-ups: #246 cross-model skeptic (noise-trimming), #247 blind classes (cosine/float-eq), #248 robustness + self-consistency union.

Author: zaebee

src/cgis/guardian/collector.py

@@ -111,10 +111,18 @@ def get_changed_py_files(self) -> list[str]:
         return [p for p in result.stdout.splitlines() if p.endswith(".py")]
 
     def read_file(self, relative_path: str) -> str:
-        """Reads a file from the project root."""
+        """Reads a file from the project root; returns "" when it does not exist.
+
+        An empty string (not an error marker) lets the prompt builder omit the
+        corresponding section entirely — a repo without CONTRIBUTING.md or
+        docs/ontology/ should review cleanly, not get "Error: File ..." injected
+        as if it were the standards/ontology text. `is_file()` (not `exists()`)
+        so a path that resolves to a directory degrades to "" instead of raising
+        IsADirectoryError on read.
+        """
         file_path = self.project_root / relative_path
-        if not file_path.exists():
-            return f"Error: File {relative_path} not found."
+        if not file_path.is_file():
+            return ""
         return file_path.read_text(encoding="utf-8")
 
     def collect_full_files(self, files: list[str] | None = None) -> str:

src/cgis/guardian/prompts.py

@@ -8,17 +8,23 @@ class PromptBuilder:
     def build_system_prompt() -> str:
         """Return the system prompt that establishes Guardian's reviewer persona."""
         return (
-            "You are the CGIS Guardian — a Senior Software Architect doing a code review. "
-            "Your goal is HIGH PRECISION: every finding you report must be a real problem "
-            "that exists in the actual diff. A false positive wastes engineering time just "
-            "as much as a missed bug. Quality of findings beats quantity. "
+            "You are the CGIS Guardian finder — a Senior Software Architect hunting for "
+            "defects in a code review. You are the FIRST of two stages: a separate skeptic "
+            "verifier runs after you and removes false positives. Because of that division "
+            "of labour, your job is RECALL — surface every plausible real defect. A missed "
+            "bug is the expensive error here; a borderline finding is cheap, because the "
+            "skeptic filters it. Surface a finding whenever you can name a CONCRETE failure "
+            "scenario — a specific input, state, timing, or platform that makes the code "
+            "wrong. Do not self-censor because you are unsure: hand the uncertainty to the "
+            "skeptic with your reasoning, do not suppress it. "
             "You prioritise: (1) Logic correctness — wrong output, crashes, data corruption; "
             "(2) Library boundary contracts — convention mismatches at third-party API calls; "
             "(3) Missing test coverage for real edge cases in the diff; "
             "(4) Type safety violations that mypy strict would catch; "
             "(5) Ontology compliance — wrong NodeType/EdgeType mappings corrupt the graph. "
-            "You do NOT flag style preferences, naming conventions, or design disagreements "
-            "unless they cause a concrete defect. If the code is correct and tested, say so."
+            "You still do NOT flag style preferences, naming conventions, or design "
+            "disagreements that have no concrete failure scenario. If the code is correct "
+            "and tested, say so."
         )
 
     @staticmethod
@@ -62,42 +68,79 @@ def build_user_prompt(context: dict[str, str]) -> str:
 {drift}
 """
 
-        return f"""Review the following Pull Request diff for real defects.
-
-### 1. ENGINEERING STANDARDS (from CONTRIBUTING.md)
+        contributing_section = ""
+        if contributing:
+            contributing_section = f"""### 1. ENGINEERING STANDARDS (from CONTRIBUTING.md)
 {contributing}
 
-### 2. PROJECT ONTOLOGY (from docs/ontology/)
+"""
+
+        ontology_section = ""
+        if ontology:
+            ontology_section = f"""### 2. PROJECT ONTOLOGY (from docs/ontology/)
 {ontology}
 
-### 3. CHANGES TO REVIEW (git diff)
+"""
+
+        return f"""Review the following Pull Request diff for real defects.
+
+{contributing_section}{ontology_section}### 3. CHANGES TO REVIEW (git diff)
 {diff}
 {graph_section}{full_files_section}{drift_section}
 ---
-### PRECISION RULES — read before writing a single finding:
+### HUNTING RULES — read before writing a single finding:
 
-1. **Evidence first.** Quote the exact line(s) from the diff that prove the problem.
-   The quoted text MUST appear verbatim in section 3. If you cannot find it, do not raise it.
+1. **Evidence first.** Quote the exact line(s) from the diff that the finding sits on.
+   The quoted text MUST appear verbatim in section 3 (it positions the inline comment).
+   If you cannot find the line, do not raise it.
 
-2. **Confidence gate.** Before writing a finding, ask yourself:
-   "Am I at least 80% confident this is a real defect in this diff?"
-   If not — omit it entirely. Uncertain findings are not helpful.
+2. **Surface on a nameable failure scenario.** Before writing a finding, ask yourself:
+   "Can I describe one concrete input, state, timing, or platform where this code
+   misbehaves?" If yes — REPORT it, and put your honest `confidence` in the field.
+   Confidence does NOT gate inclusion: a 40%-confident finding with a real failure
+   scenario is worth surfacing, because the skeptic verifier decides what to keep.
+   Only drop a candidate when you cannot construct ANY failure scenario for it.
 
 3. **No ghost issues.** If the code already handles a case, acknowledge it and move on.
-   Do not flag something as missing when it is present.
+   Do not flag something as missing when it is present (this is a wrong finding, not
+   an uncertain one — the skeptic cannot rescue precision from a fabricated claim).
 
 4. **No invented rules.** Only cite standards explicitly written in CONTRIBUTING.md or the
    ontology files provided above. Do not apply rules from outside this context.
 
-5. **Cap at 5 findings.** Report only the 5 most important real issues.
-   If you find fewer real issues, report fewer. Zero is a valid answer.
+5. **No finding cap.** Report every real candidate you find — more genuine findings is
+   strictly better here, since recall is your job and the skeptic trims the list. Order
+   them most-severe first. Zero is still a valid answer when the diff is clean.
+
+6. **Reason per changed function before deciding.** For each function the diff touches,
+   briefly walk these before you write findings for it:
+   - What are its inputs and where do they come from (caller, config/YAML/JSON, env,
+     request)? Is any of them external/untrusted?
+   - What happens on the awkward inputs: empty / None / zero / a non-dict where a dict
+     is assumed / a scalar where an iterable is assumed / unsorted / duplicate / oversized?
+   - Did a deleted or changed line hold an invariant (a guard, a validation, an error
+     path)? Is it re-established?
+   Only after that walk do you decide what to surface. Skipping this walk is the main
+   reason subtle defects (unvalidated data, exact-equality, dropped guards) go unseen.
 
 ---
 ### WHAT TO LOOK FOR (focus areas):
 
 **Logic bugs** — inputs that produce wrong output, division by zero, off-by-one errors,
 incorrect algorithm behaviour. Think: empty collections, None values, boundary conditions.
 
+**Unvalidated external data** — a value read from config/YAML/JSON, env, or a request is
+used as a `dict`/`list`/`set`/iterable (subscripted, iterated, passed to `set()`/`dict()`,
+`.items()`, `for x in value`) without first checking its type or presence. A YAML key the
+author expects to be a mapping can legally be a scalar or a list; a `value or {{}}` idiom
+catches `None` but lets a non-dict truthy value through to operations that then misbehave
+silently rather than erroring. Flag each such use that lacks a type/shape guard.
+
+**Exact-equality on floats / money** — bare `==` or `!=` comparing floating-point or
+Decimal values, *including in test assertions* (`assert x == 0.3`). Floating-point
+rounding makes these flaky or wrong; they should use a tolerance compare. Check changed
+test files for this too.
+
 **Missing test coverage** — code paths in the diff that have no test. Focus on edge cases
 that could silently return wrong results (not just "coverage for coverage's sake").
 
@@ -116,6 +159,32 @@ def build_user_prompt(context: dict[str, str]) -> str:
 **Ontology compliance** — wrong NodeType/EdgeType assignments, FQNs not derived from file
 paths, unresolved calls not using `raw_call:` prefix.
 
+---
+### WORKED EXAMPLES (how the per-function walk turns into a finding):
+
+These show the kind of subtle, borderline defect that is easy to skip but worth surfacing.
+Do not look for these exact lines — learn the *pattern* and apply it to the diff above.
+
+Example A — unvalidated config value used as a mapping:
+```
+def layers_for(self, name: str) -> set[str]:
+    cfg = self._patterns.get(name)        # cfg comes from a YAML file
+    return set(cfg)                        # <-- assumes cfg is iterable-of-str
+```
+Walk: input `cfg` is external (YAML); the author assumes a list/mapping, but YAML lets
+that key be a scalar (`name: layered`) → `set("layered")` silently yields `{{'l','a',...}}`,
+not an error. → FINDING: `set(cfg)` lacks a type/shape guard on external config data.
+(confidence ~50 — borderline, but it has a concrete failure scenario, so surface it.)
+
+Example B — exact-equality on floats in a test:
+```
+def test_drift_score():
+    assert scorer.score(fp) == 0.3        # <-- bare == on a float result
+```
+Walk: `score()` returns a computed float; `== 0.3` is exact float equality → rounding can
+make this assert flaky/false. → FINDING: float compared with bare `==` in a test; use a
+tolerance (`pytest.approx` / `math.isclose`). (confidence ~55.)
+
 ---
 ### OUTPUT FORMAT:
 
@@ -140,8 +209,9 @@ def build_user_prompt(context: dict[str, str]) -> str:
   diff in section 3 (no paraphrasing, no `+`/`-` marker). It is used to position the inline
   comment deterministically — if your "line" guess is off, a correct "anchor" still lands the
   comment on the right line. Use null only for genuinely file-level findings.
-- "confidence" must be >= 80 to include a finding (the gate above).
-- max 5 findings; fewer is fine; an empty list means LGTM.
+- "confidence" is your honest 0-100 estimate; it does NOT gate inclusion — the skeptic
+  verifier uses it to prioritise, so report findings below 80 too.
+- no finding cap; report every real candidate, most-severe first; an empty list means LGTM.
 - "summary" is mandatory; for an LGTM it lists what you checked and found correct.
 
 Example LGTM response: {{"findings": [], "summary": "Checked diff for logic and types; ok."}}"""

src/cgis/guardian/skeptic.py

@@ -10,9 +10,9 @@
 
 log = structlog.getLogger(__name__)
 
-_CONFIDENCE_GATE = 80
-_UNCERTAIN_MULTIPLIER = 0.9  # keeps original confidence >= 89; smaller would
-# refute EVERY uncertain finding and make the branch dead code (spec §5.3).
+_UNCERTAIN_MULTIPLIER = 0.9  # an 'uncertain' verdict discounts confidence as a
+# ranking signal only — it NEVER refutes. The recall-lean finder emits genuine
+# low-confidence findings on purpose; only an explicit 'refuted' verdict drops one.
 
 
 class SkepticVerdict(BaseModel, frozen=True):
@@ -32,15 +32,19 @@ class SkepticResult(BaseModel, frozen=True):
 # Confirm-by-default stance. The original refute-by-default wording over-killed
 # in benchmarks: a gemini-3.5-flash skeptic refuted 7/7 finder findings,
 # including 2 ground-truth matches on PR 122 (gate allows at most 1 lost match).
+# The finder is now recall-lean (no confidence gate, no cap), so it surfaces
+# genuine low-confidence findings BY DESIGN and leans on this pass for precision —
+# the skeptic must cut hallucinations without re-introducing the gate it removed.
 SKEPTIC_SYSTEM_PROMPT = (
     "You are a skeptical senior reviewer double-checking another reviewer's findings. "
-    "For each finding, verify the quoted evidence against the diff and judge whether "
-    "the claimed defect is real. Refute a finding ONLY when you can point to concrete "
-    "evidence that it is wrong: the quoted code does not appear in the diff, the case "
-    "is already handled, or the claim misreads what the code does. "
-    "If a finding is plausible and you cannot disprove it, confirm it — the finder "
-    "already applied a confidence gate; your job is to catch its hallucinations, "
-    "not to second-guess its judgement calls."
+    "That reviewer optimises for RECALL: it deliberately surfaces plausible, sometimes "
+    "low-confidence candidates and relies on you to remove only the ones that are wrong. "
+    "For each finding, verify the quoted evidence against the diff and judge whether the "
+    "claimed defect is real. Refute a finding ONLY when you can point to concrete evidence "
+    "that it is wrong: the quoted code does not appear in the diff, the case is already "
+    "handled, or the claim misreads what the code does. Do NOT refute a finding merely for "
+    "being low-confidence, speculative, or a judgement call — if it is plausible and you "
+    "cannot disprove it, mark it 'confirmed' or 'uncertain' (both are kept), never 'refuted'."
 )
 
 
@@ -74,8 +78,9 @@ def apply_verdicts(findings: list[Finding], skeptic: SkepticResult) -> list[Find
     """Merge skeptic verdicts into new frozen Finding copies (spec §5.3).
 
     Out-of-range / duplicate indices are discarded and logged. Unruled findings
-    keep verdict=None. uncertain discounts confidence x0.9; below the 80 gate
-    it becomes refuted.
+    keep verdict=None. uncertain discounts confidence x0.9 as a ranking signal but
+    is KEPT (only an explicit 'refuted' verdict drops a finding) — the recall-lean
+    finder relies on the skeptic to cut hallucinations, not low confidence.
     """
     by_index: dict[int, SkepticVerdict] = {}
     for v in skeptic.verdicts:
@@ -94,12 +99,12 @@ def apply_verdicts(findings: list[Finding], skeptic: SkepticResult) -> list[Find
             merged.append(finding)  # absence of a verdict is not a refutation
             continue
         if verdict.verdict == "uncertain":
+            # Kept, not refuted: discount confidence as a ranking signal only.
             discounted = round(finding.confidence * _UNCERTAIN_MULTIPLIER)
-            final = "refuted" if discounted < _CONFIDENCE_GATE else "uncertain"
             merged.append(
                 finding.model_copy(
                     update={
-                        "verdict": final,
+                        "verdict": "uncertain",
                         "skeptic_note": verdict.rationale,
                         "confidence": discounted,
                     }

tests/unit/test_guardian_core.py

@@ -40,6 +40,20 @@ def test_build_user_prompt_includes_all_sections() -> None:
     assert "the ontology" in prompt
 
 
+def test_build_user_prompt_omits_standards_and_ontology_when_empty() -> None:
+    """A repo without CONTRIBUTING.md / ontology (empty strings) drops both sections.
+
+    Guards the universal-reviewer path: no "Error: File ... not found." text and
+    no empty PROJECT ONTOLOGY heading leaking into a non-cgis repo's prompt.
+    """
+    context = {"diff": "the diff", "contributing": "", "ontology": ""}
+    prompt = PromptBuilder.build_user_prompt(context)
+    assert "the diff" in prompt
+    assert "ENGINEERING STANDARDS" not in prompt
+    assert "PROJECT ONTOLOGY" not in prompt
+    assert "not found" not in prompt
+
+
 def test_build_user_prompt_includes_graph_section() -> None:
     """Graph context is injected as section 4 when present."""
     context = {
@@ -320,8 +334,8 @@ def test_get_changed_py_files_on_error(tmp_path: Path) -> None:
 
 
 def test_read_file_missing(tmp_path: Path) -> None:
-    """Returns error string for missing file."""
-    assert "not found" in ContextCollector(project_root=tmp_path).read_file("nonexistent.md")
+    """Returns "" for a missing file so the prompt omits its section."""
+    assert ContextCollector(project_root=tmp_path).read_file("nonexistent.md") == ""
 
 
 def test_read_file_exists(tmp_path: Path) -> None:

tests/unit/test_guardian_runner.py

@@ -154,9 +154,9 @@ async def test_run_guardian_smoke(tmp_path: Path) -> None:
 
     ContextCollector.collect_all() gracefully handles a non-git tmp_path:
     get_git_diff() returns an error string (no exception), and read_file()
-    returns "Error: File ... not found." for missing CONTRIBUTING.md /
-    ontology.yaml.  The FakeProvider ignores the prompt content and always
-    returns valid JSON, so no mock of collect_all() is needed.
+    returns "" for missing CONTRIBUTING.md / ontology.yaml (so those prompt
+    sections are simply omitted).  The FakeProvider ignores the prompt content
+    and always returns valid JSON, so no mock of collect_all() is needed.
     """
     metrics = tmp_path / "m.jsonl"
     collector = ContextCollector(project_root=tmp_path)

tests/unit/test_guardian_skeptic.py

@@ -40,19 +40,21 @@ def test_refuted_marks_but_keeps_finding() -> None:
     assert merged[0].verdict == "refuted"
 
 
-def test_uncertain_discounts_to_above_gate() -> None:
-    """uncertain at confidence 89 → round(89*0.9)=80, stays uncertain (boundary)."""
+def test_uncertain_discounts_confidence_but_keeps_finding() -> None:
+    """uncertain → confidence x0.9 (ranking signal), verdict stays uncertain."""
     f = _FINDING.model_copy(update={"confidence": 89})
     merged = apply_verdicts([f], SkepticResult(verdicts=[_verdict(0, "uncertain")]))
     assert merged[0].verdict == "uncertain"
     assert merged[0].confidence == 80
 
 
-def test_uncertain_discount_below_gate_refutes() -> None:
-    """uncertain at confidence 88 → round(88*0.9)=79 < 80 → treated as refuted."""
-    f = _FINDING.model_copy(update={"confidence": 88})
+def test_uncertain_low_confidence_is_kept_not_refuted() -> None:
+    """Recall-lean: a low-confidence uncertain finding survives (no gate auto-refute)."""
+    f = _FINDING.model_copy(update={"confidence": 30})
     merged = apply_verdicts([f], SkepticResult(verdicts=[_verdict(0, "uncertain")]))
-    assert merged[0].verdict == "refuted"
+    assert merged[0].verdict == "uncertain"
+    assert merged[0].confidence == 27  # round(30 * 0.9)
+    assert visible_findings(merged) == merged  # not dropped
 
 
 def test_out_of_range_and_duplicate_indices_discarded() -> None: