feat(guardian): tell the skeptic when no checker ran, and which verdict that takes (#407)

benchmarks/guardian/experiments/407-no-checker-appeal/README.md

@@ -0,0 +1,106 @@
+# Telling the skeptic that no checker ran (#407)
+
+Measuring #401 needed a control arm: the same 24 findings judged with **no checker output at
+all**. In it, 6 of 24 rationales cited `mypy` or `ruff` anyway — every one to *confirm* a
+false claim, on a conditional the model could not check:
+
+> The use of `Any` does indeed bypass strict type checking in mypy … and **if** `mypy
+> --strict` is configured to disallow `Any` explicitly, it would be flagged.
+
+`_evidence_section(None)` returned the empty string, so the skeptic was told nothing about
+checkers and supplied its own. Two prompts were tried against that.
+
+## Method
+
+The replay control arm is exactly this configuration, so each attempt costs 24 skeptic calls
+and no finder call. Same recording (`../401-evidence/pr-399-recording.json`), same absence of
+evidence, one variable: the prompt.
+
+Registered before running: **primary** — how many of the six checker-appealing findings stop
+being confirmed on that basis; **recall guard** — finding 4, the one substantive finding of
+the 24, must stay confirmed; **noise floor** — two draws of the original prompt moved 4 and 1
+verdicts out of 24, so anything smaller than that is not a result.
+
+## Attempt 1 — a prohibition. Failed.
+
+> Do not rest a verdict on what a checker would report — not to confirm a finding … and not
+> to refute one …
+
+| | original | prohibition |
+|---|---:|---:|
+| rationales citing a checker | 6/24 | 5/24 |
+| the six, still confirmed | 6 | 5 |
+| finding 4 | confirmed | confirmed |
+
+Inside the noise floor, and the text got **worse**. Finding 0's conditional became a flat
+assertion:
+
+> This is an issue that would be caught by the project's **mandatory `mypy --strict` gate**.
+
+There is no such gate over that file: `make type-check` runs `mypy src`, and `pyproject.toml`
+excludes `scripts`. Told what not to do, the model complied in form and confirmed anyway —
+with more confidence than before.
+
+## Attempt 2 — a directive. Worked.
+
+> If a finding rests entirely on what a checker reports … then it cannot be settled here.
+> Mark it 'uncertain'. Not 'confirmed': you have not seen a checker agree. Not 'refuted': you
+> have not seen one disagree either.
+
+| finding | original control | directive |
+|---|---|---|
+| 0, 2, 3, 5, 15 — claim a mypy rule | confirmed | **uncertain** |
+| 12 — `sys.path` "violates type safety" | confirmed | confirmed |
+| **4 — the one true finding** | confirmed | **confirmed** ✅ |
+| 21, 22 — propose relaxing a ratchet | confirmed | **refuted** |
+
+Five of six moved, all in the same direction, all on cited-checker rows: outside the 1–4
+noise floor and mechanistically legible. Finding 0 now states its position instead of
+inventing a gate:
+
+> Without access to the project's `mypy` configuration or output, it is not possible to
+> confirm if this specific usage would be flagged.
+
+The recall guard held, with the correct reasoning — `CalledProcessError` is raised and not
+caught, which is the defect that was really fixed in #399.
+
+### An unregistered gain
+
+Findings 21 and 22 were refuted. Those are the two the RFC singled out as **beyond any
+tool**: they proposed relaxing a `>= 72` ratchet floor and an `ambiguous_hits` invariant, and
+were wrong about *intent* rather than about behaviour. The model got there on its own:
+
+> The assertion is a deliberate guardrail … its failure due to a diminished corpus is
+> **intended behavior, not a defect**.
+
+Not predicted, not registered, one draw — recorded as an observation, not a claim.
+
+## What this does not buy
+
+**`uncertain` is kept.** `apply_judgements` multiplies confidence by 0.9 as a ranking signal
+and nothing more, and `render_review_body` takes `threshold: int = 0`. So all five findings
+are still posted to the pull request; only their ordering changes. The verdict is now
+truthful — the reader's experience is very nearly the same.
+
+That makes this an epistemic fix, not a noise fix. Suppressing unadjudicable findings is a
+separate knob (the `impact_threshold` that #246 §3.5 records as shipping inert), and it
+should be decided on its own evidence rather than folded in here.
+
+**One draw per arm, and a recall denominator of one.** The primary effect is well outside the
+noise floor; nothing else here is.
+
+**`skeptic.py` is inside the review-fingerprint closure** (43 modules), so this re-mints every
+reviewer identity — by design (#375).
+
+## Reproducing
+
+From the repository root — the paths are root-relative, because the command above them is:
+
+```bash
+scripts/guardian_replay_skeptic.py \
+    --recording benchmarks/guardian/experiments/401-evidence/pr-399-recording.json \
+    --control --out rows.jsonl
+```
+
+With `GUARDIAN_SKEPTIC=gemini`, `GUARDIAN_SKEPTIC_MODEL=gemini-2.5-flash`. 24 skeptic calls,
+no finder calls.

benchmarks/guardian/experiments/407-no-checker-appeal/pr-399-control-directive-rows.jsonl

@@ -0,0 +1,24 @@
+{"index": 0, "file": "scripts/recordings_from_corpus.py", "line": 38, "title": "Use of `Any` in type annotations violates strict mypy mode", "was": "confirmed", "now": "uncertain", "cites_a_checker": true, "rationale": "The claim states that the use of `Any` violates strict mypy mode. Without access to the project's `mypy` configuration or output, it is not possible to confirm if this specific usage would be flagged as a violation by the existing tooling."}
+{"index": 1, "file": "scripts/recordings_from_corpus.py", "line": 41, "title": "Unsafe YAML loading without `safe_load` or `Loader` specification", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The current code correctly uses `yaml.safe_load`. However, the import `import yaml` does leave `yaml.load` accessible, which is an unsafe function if used without a specific Loader. The claim highlights a plausible risk for future maintainers accidentally using the unsafe `load` function instead of `safe_load`."}
+{"index": 2, "file": "scripts/recordings_from_corpus.py", "line": 50, "title": "Function parameter uses `Any` in type annotation", "was": "confirmed", "now": "uncertain", "cites_a_checker": true, "rationale": "The code uses `dict[str, Any]` as quoted. Whether this specific usage violates a 'strict mypy rule' as claimed cannot be confirmed without access to the project's mypy configuration and checker output. If `mypy --strict` were configured to disallow `Any` in this context, it would be caught by the existing mandatory tooling, resulting in a low impact."}
+{"index": 3, "file": "scripts/recordings_from_corpus.py", "line": 56, "title": "Function parameter uses `Any` in type annotation", "was": "confirmed", "now": "uncertain", "cites_a_checker": true, "rationale": "The quoted code is present in the diff, but confirming whether `dict[str, Any]` violates the project's strict mypy rules requires checker output, which is not available for this review."}
+{"index": 4, "file": "scripts/recordings_from_corpus.py", "line": 62, "title": "Empty diff raises `MissingFixtureError` but may not be the only failure mode", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The `subprocess.run(check=True)` call can raise a `CalledProcessError` if `git diff` fails for reasons other than producing an empty diff, and this specific exception is not caught, leading to a crash with a generic error instead of a more specific `MissingFixtureError` or handled failure."}
+{"index": 5, "file": "scripts/recordings_from_corpus.py", "line": 80, "title": "Function parameter uses `Any` in type annotation", "was": "confirmed", "now": "uncertain", "cites_a_checker": true, "rationale": "The code uses `dict[str, Any]` as quoted. However, whether this *violates* a 'strict mypy rule' depends on the project's specific mypy configuration, which cannot be verified without checker output. If it were a violation, it would be caught by tooling."}
+{"index": 6, "file": "scripts/recordings_from_corpus.py", "line": 89, "title": "Potential type mismatch in `Finding.model_validate`", "was": "confirmed", "now": "refuted", "cites_a_checker": false, "rationale": "The `Finding.model_validate` method is specifically designed to perform schema validation and will raise a `ValidationError` if the input dictionary does not match the `Finding`'s expected schema, which is the correct and robust behavior for handling potentially untrusted data. The dictionary comprehension explicitly removes known annotation keys (`_ANNOTATIONS`) that are not part of the `Finding` model, thereby preventing validation errors for these specific fields and ensuring that only relevant keys are passed for validation. There is no silent data dropping unless the `Finding` model is configured with `extra='ignore'`, which is not indicated here; Pydantic's default is `extra='forbid'`, causing errors for unexpected keys."}
+{"index": 7, "file": "scripts/recordings_from_corpus.py", "line": 95, "title": "No validation that `results` file exists or is readable", "was": "refuted", "now": "refuted", "cites_a_checker": false, "rationale": "The call to `results.read_text()` will raise a `FileNotFoundError` or `PermissionError` if the file does not exist or is not readable, preventing silent failure. If the file is empty, the function correctly processes no rows and reports '0 recordings written', which is an accurate and non-silent outcome."}
+{"index": 8, "file": "scripts/recordings_from_corpus.py", "line": 100, "title": "No handling for case where no frozen passes are found", "was": "refuted", "now": "refuted", "cites_a_checker": false, "rationale": "The script already prints \"0 recordings written to {args.out}\" when no frozen passes are found. This explicit output informs the caller, mitigating any potential \"unexpected behavior\" and sufficiently handling the case without needing an additional warning or early return."}
+{"index": 9, "file": "scripts/recordings_from_corpus.py", "line": 102, "title": "Race condition in directory creation", "was": "refuted", "now": "refuted", "cites_a_checker": false, "rationale": "The `exist_ok=True` argument in `Path.mkdir()` is specifically designed to handle concurrent directory creation attempts, making the operation idempotent and race-safe without raising a `FileExistsError`."}
+{"index": 10, "file": "scripts/recordings_from_corpus.py", "line": 110, "title": "No validation that `save_finder_recording` succeeds", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The call to save_finder_recording is not wrapped in a try-except block, so file system errors (e.g., permissions, disk full) would cause the script to crash or fail to write the file without handling, potentially leaving an incomplete set of recordings and reporting success where there was none."}
+{"index": 11, "file": "scripts/recordings_from_corpus.py", "line": 117, "title": "No error handling for `build` failures", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The `build` function can explicitly raise `MissingFixtureError` or `NotAFrozenPassError` with descriptive messages; however, the `main` function does not catch these expected exceptions to present a user-friendly error message, instead allowing a raw Python traceback to be displayed, which is a less graceful exit for a command-line tool."}
+{"index": 12, "file": "tests/unit/test_recordings_from_corpus.py", "line": 12, "title": "Modification of `sys.path` at runtime violates type safety", "was": "confirmed", "now": "confirmed", "cites_a_checker": true, "rationale": "Dynamically modifying sys.path at runtime, especially with `insert(0)`, can lead to less predictable module resolution and violates the principle of explicit imports, making the project structure harder to reason about for both humans and static analysis tools. While it doesn't directly violate Python's fundamental type system, it can complicate static type checking."}
+{"index": 13, "file": "tests/unit/test_recordings_from_corpus.py", "line": 15, "title": "Import from a script file may fail in some environments", "was": "refuted", "now": "refuted", "cites_a_checker": false, "rationale": "The code explicitly adds the directory containing `recordings_from_corpus.py` to `sys.path` on line 13, directly handling the concern that 'the file is not in `sys.path`'. Python environments generally support direct imports from `.py` files, making the second part of the claim's rationale incorrect. While importing from a script that isn't a formal module can sometimes have other subtle issues, the specific reasons for failure cited in the claim are addressed or incorrect by the provided diff."}
+{"index": 14, "file": "tests/unit/test_recordings_from_corpus.py", "line": 20, "title": "Hardcoded path traversal may break in some environments", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The code explicitly assumes a fixed directory depth (three levels up from `__file__`), making `REPO_ROOT` fragile to changes in the test file's location or the project's overall structure, which could cause test failures."}
+{"index": 15, "file": "tests/unit/test_recordings_from_corpus.py", "line": 28, "title": "Function return type uses `Any` in type annotation", "was": "confirmed", "now": "uncertain", "cites_a_checker": true, "rationale": "The `Any` type is present as claimed in the diff. However, whether this violates a project-specific strict mypy rule or mypy --strict cannot be verified without running the checker. If it does violate such a rule, it would be caught by mandatory project tooling, leading to a low impact score."}
+{"index": 16, "file": "tests/unit/test_recordings_from_corpus.py", "line": 30, "title": "No validation that `RESULTS` file exists or is readable", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The code does not explicitly validate that the RESULTS file exists or is readable, and as stated, will raise a FileNotFoundError if it's missing or unreadable. While this behavior for a test helper indicates a missing fixture, it is a factual lack of explicit validation."}
+{"index": 17, "file": "tests/unit/test_recordings_from_corpus.py", "line": 35, "title": "Return type uses `Path` but may include non-Path values", "was": "refuted", "now": "refuted", "cites_a_checker": false, "rationale": "The reviewer's own description confirms that the return type annotation `dict[str, Path]` correctly reflects that the function returns a dictionary where values are `Path` objects, stating \"This is correct\". The claim then concludes \"No action needed; the type is correct. This is a false positive.\", effectively refuting the initial suggestion of a problem."}
+{"index": 18, "file": "tests/unit/test_recordings_from_corpus.py", "line": 50, "title": "Test assumes existence of a judged row in the corpus", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The `next()` call without a default argument will raise `StopIteration` if no row in the corpus satisfies the condition, making the test fragile if the corpus changes."}
+{"index": 19, "file": "tests/unit/test_recordings_from_corpus.py", "line": 57, "title": "Test assumes existence of an unscored row in the corpus", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The quoted code is present in the diff. The `next()` function will raise a `StopIteration` error if the generator expression yields no elements, meaning the test is brittle and relies on the implicit existence of a specific type of row in the external corpus."}
+{"index": 20, "file": "tests/unit/test_recordings_from_corpus.py", "line": 64, "title": "Test uses a hardcoded PR number (9999) that may not be meaningful", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The test correctly identifies a potential future flakiness where a hardcoded PR number (9999) could eventually clash with a real PR, causing the test to fail if a ground-truth fixture for that PR is created."}
+{"index": 21, "file": "tests/unit/test_recordings_from_corpus.py", "line": 75, "title": "Test assumes at least 72 frozen passes exist in the corpus", "was": "confirmed", "now": "refuted", "cites_a_checker": false, "rationale": "The assertion is a deliberate guardrail to ensure the test operates on a sufficiently large and representative corpus, as indicated by the accompanying comment, and its failure due to a diminished corpus is intended behavior, not a defect."}
+{"index": 22, "file": "tests/unit/test_recordings_from_corpus.py", "line": 100, "title": "Test assumes existence of `ambiguous_hits` in some rows", "was": "confirmed", "now": "refuted", "cites_a_checker": false, "rationale": "The assertion explicitly verifies a specific historical invariant: that the rows requiring the pre-#345 precision rule are exactly those carrying 'ambiguous_hits'. The test is designed to fail if this invariant no longer holds, which is its correct and intended behavior to flag a change in the corpus's historical properties. The claim misinterprets the test's role in enforcing this invariant as a defect."}
+{"index": 23, "file": "tests/unit/test_recordings_from_corpus.py", "line": 145, "title": "Floating-point comparison may be too strict for some use cases", "was": "confirmed", "now": "confirmed", "cites_a_checker": false, "rationale": "The use of an absolute tolerance for floating-point comparisons, while potentially sufficient for the limited range of recall and precision values (0 to 1), is generally less robust than using `math.isclose` with relative and absolute tolerances. This is a common best practice to prevent subtle test failures due to floating-point arithmetic variations across different environments or complex calculations, and thus constitutes a plausible, albeit minor, robustness improvement."}