Make the coverage-analysis plateau fixture self-consistent

eng/eval-quality/README.md

@@ -15,7 +15,7 @@ python eng/eval-quality/selftest_eval_quality.py       # prove the gate still fi
 
 ## Failing checks
 
-All six are **structural** — they inspect file existence, git state, declared
+All seven are **structural** — they inspect file existence, git state, declared
 numbers, or YAML keys. None of them interprets prose, so they cannot fire
 spuriously on a well-written eval.
 
@@ -86,7 +86,27 @@ Fix it by making the totals agree with both the declared rate and the summed
 the rate — that leaves one number for every reader. The same applies to
 `branches-covered`/`branches-valid` against `branch-rate`.
 
-### 5. Grader with a missing or empty required config
+### 5. Aggregate `line-rate` contradicts the `<lines>` beneath it
+
+A file, package or class whose declared `line-rate` disagrees with the `<line>`
+elements underneath it — the same split-brain as checks 3 and 4, at the level
+the prompt usually quotes.
+
+This shipped for a while as a warning because of one fixture:
+`coverage-analysis/fixtures/plateau` declared 75% while its `<lines>` implied
+47%, and the scenario prompt said *"my coverage is stuck at 75%"*. It could not
+be repaired by recomputing — `CalculateGpa` contributes 24 of the 47 lines at 0%
+and the rubric requires it to stay the blocker, capping the achievable rate at
+23/47 = 48.9% — so the fix reached into the scenario itself. It was resolved by
+restating the plateau at 47% (declared rates and totals aligned to 22/47, prompt
+reworded); the plateau story depends on one method dominating the shortfall, not
+on the specific number. With that fixture repaired there are no offenders left,
+so the check now fails instead of warning.
+
+Fix an occurrence the same way: make the declared rate match the payload, and if
+a prompt or rubric quotes the old figure, update it in the same change.
+
+### 6. Grader with a missing or empty required config
 
 A grader whose `config` is absent, null, or missing its required key
 (`pattern`, `substring`, `command`, `path`) parses as valid YAML and **enforces
@@ -109,7 +129,7 @@ bespoke regex validator caught it — the validator did
 `(g.get("config") or {}).get("pattern")` and silently skipped the entry, so the
 pattern count was identical before and after the fix. Only review caught it.
 
-### 6. Dormancy guard that also sets `reject_skills`
+### 7. Dormancy guard that also sets `reject_skills`
 
 A dormancy guard is a stimulus with `expect_activation: false`: an off-target
 request where the skill should stay dormant rather than hijack the task.
@@ -124,22 +144,10 @@ The repo convention is `expect_activation: false` **alone** (see
 `system-text-json-net11`), so the skill is actually loaded and the guard
 measures the real property.
 
-## Warnings (reported, never failing)
-
-### Aggregate `line-rate` vs the lines beneath it
+## Warnings (reported; failing only under `--strict`)
 
-A file, package or class whose declared `line-rate` disagrees with the `<line>`
-elements underneath it. This is a warning rather than an error because a real
-coverage report may legitimately summarise more than it enumerates, and because
-the correct fix sometimes reaches into the scenario itself.
-
-Live example: `coverage-analysis/fixtures/plateau` declares 75% while its
-`<lines>` imply 47%, and the scenario prompt says *"my coverage is stuck at
-75%"*. It cannot simply be recomputed — `CalculateGpa` contributes 24 lines at
-0% coverage and the rubric requires it to stay the 0% blocker, which caps the
-achievable rate at 23/47 = 48.9%. Making the payload true would mean rewriting
-the fixture and the prompt together, so the gate reports it and leaves the
-judgement to a human.
+CI runs the gate without `--strict`, so these are informational there. Passing
+`--strict` returns exit code 1 when any warning is present.
 
 ### Statistical power
 
@@ -154,11 +162,20 @@ sqrt(n) × (mean / sd) > t(n-1)
 | n | required mean/sd |
 | ---: | ---: |
 | 1 | undefined — a single trial decides |
-| 2 | 3.04 |
-| 3 | 1.84 |
-| 4 | 1.39 |
-| 6 | 1.00 |
-| 8 | 0.82 |
+| 2 | 8.98 |
+| 3 | 2.48 |
+| 4 | 1.59 |
+| 6 | 1.05 |
+| 8 | 0.84 |
+
+These are `t(n-1)/sqrt(n)`. An earlier revision of this table read the critical
+value at `t(n)` instead, understating every row — n=3 appeared to need 1.84 when
+it really needs 2.48, and n=2 appeared to need 3.04 against a true 8.98. That
+made a thin eval look one good trial away from credible when it was not. The
+worked example below is the check: `coverage-analysis` scoring 0.4/1.0/0.4 has
+mean/sd = 1.73, which reads as a near miss against the old 1.84 but is 30% short
+of the real 2.48 — a difference that changes the remedy from "re-run it" to
+"raise n or runs".
 
 Consequences seen in practice: `coverage-analysis` **won 100% of its trials in
 four consecutive runs and failed all four**; `migrate-static-to-wrapper` missed

eng/eval-quality/check_eval_quality.py

@@ -5,27 +5,39 @@
 cannot silently recur in any plugin.
 
 FAILS on unambiguous bugs:
-  1. A stimulus references a fixture that is missing on disk.
-  2. A stimulus references a fixture that exists but is NOT tracked by git.
-     `.gitignore` once silently swallowed a Cobertura fixture: the scenarios
-     passed locally and would have failed at setup in CI.
-  3. A Cobertura fixture whose declared `line-rate` contradicts its own
-     `<lines>` data. The crap-score skill documents both parse paths, so the
-     two arms of a comparison can legitimately read different inputs and the
-     eval measures the disagreement instead of the skill.
-  4. A dormancy guard (`expect_activation: false`) that also sets
-     `reject_skills`. That forces the skilled arm skill-free, making it
-     identical to the baseline arm, so the score is judge noise.
+  1. Referenced fixture missing on disk. The scenario fails at setup, which
+     reads as a skill failure.
+  2. Referenced fixture not tracked by git. `.gitignore` once silently swallowed
+     a Cobertura fixture: the scenarios passed locally and would have failed in
+     CI.
+  3. Cobertura `line-rate` contradicts its own `<lines>`. The crap-score skill
+     documents both parse paths, so the two arms can read different inputs and
+     the eval measures that disagreement instead of the skill.
+  4. Whole-file Cobertura totals contradict the declared file rate, for lines
+     (`lines-covered`/`lines-valid` vs `line-rate`) or branches
+     (`branches-covered`/`branches-valid` vs `branch-rate`). Summary attributes
+     are another parse path, so mismatched totals split readers on the same
+     fixture.
+  5. Aggregate `line-rate` contradicts the `<lines>` beneath it. File, package,
+     and class rates are often the prompt-level coverage number, so disagreement
+     there changes what the scenario is asking about.
+  6. Grader with a missing or empty required config. The YAML parses, but the
+     grader silently enforces nothing and the scenario has one fewer assertion
+     than it appears to.
+  7. Dormancy guard that also sets `reject_skills`. That forces the skilled arm
+     skill-free, making it identical to the baseline arm, so the score is judge
+     noise.
 
 Every failing check above is structural — it inspects file existence, git
-state, or YAML keys — so it cannot fire spuriously on well-written content.
+state, declared numbers, or YAML shape/keys — so it cannot fire spuriously on
+well-written content.
 
-REPORTS (does not fail) pre-existing debt and judgement calls: statistical
-power, orphaned fixtures, skills with no eval, and dormancy guards that appear
-to lack an anti-hijack rubric item. That last one is deliberately a warning:
-detecting "the rubric says the skill should stay dormant" needs phrase
-matching, which will always have false positives, and a gate that blocks a PR
-spuriously is a gate the team turns off.
+REPORTS warnings for pre-existing debt and judgement calls: statistical power,
+orphaned fixtures, skills with no eval, and dormancy guards that appear to lack
+an anti-hijack rubric item. Warnings do not fail unless `--strict` is passed.
+That last one is deliberately a warning: detecting "the rubric says the skill
+should stay dormant" needs phrase matching, which will always have false
+positives, and a gate that blocks a PR spuriously is a gate the team turns off.
 
 Usage:  python eng/eval-quality/check_eval_quality.py [--strict]
 """
@@ -45,8 +57,12 @@
     print("PyYAML is required: pip install pyyaml", file=sys.stderr)
     raise SystemExit(2)
 
-T95 = {2: 4.303, 3: 3.182, 4: 2.776, 5: 2.571, 6: 2.447, 7: 2.365, 8: 2.306,
-       9: 2.262, 10: 2.228, 11: 2.201, 12: 2.179, 13: 2.160, 14: 2.145, 15: 2.131}
+# 95% two-sided t critical values, keyed by DEGREES OF FREEDOM (n - 1), which is
+# what the pass gate's confidence interval uses. Keying this by n instead is an
+# easy and costly slip: it makes every reported threshold too lenient.
+T95 = {1: 12.706, 2: 4.303, 3: 3.182, 4: 2.776, 5: 2.571, 6: 2.447, 7: 2.365,
+       8: 2.306, 9: 2.262, 10: 2.228, 11: 2.201, 12: 2.179, 13: 2.160,
+       14: 2.145, 15: 2.131}
 
 ANTI_HIJACK = ("derail", "did not attempt", "outside the scope", "out of scope",
                "did not perform", "declined", "does not load", "does not reference",
@@ -207,10 +223,13 @@ def check_cobertura() -> None:
                     f"different whole-file {unit} coverage numbers, so the arms disagree "
                     f"depending on which attribute a skill happens to read")
 
-        # Aggregates vs the underlying payload. Reported rather than failed:
-        # a real report may legitimately summarise more than it enumerates,
-        # and forcing a rewrite of a scenario whose prompt quotes the declared
-        # figure is a bigger change than this check should compel.
+        # Aggregates vs the underlying payload. A file, package or class that
+        # declares one rate while the <line> elements beneath it imply another
+        # is the same split-brain bug one level up: a skill that trusts the
+        # attribute and one that recomputes read different inputs. Held as a
+        # warning only while coverage-analysis/fixtures/plateau declared 75%
+        # against a 47% payload; that fixture is now self-consistent, so the
+        # check fails instead of warning.
         for el, label in (
             [(tree.getroot(), "file")]
             + [(p, f"package '{p.get('name')}'") for p in tree.iter("package")]
@@ -221,9 +240,11 @@ def check_cobertura() -> None:
             if not total or declared is None:
                 continue
             if abs(covered / total - float(declared)) >= 0.011:
-                warnings.append(
+                errors.append(
                     f"{path}: {label} declares line-rate={float(declared):.2f} but the "
-                    f"<lines> beneath it imply {covered / total:.2f} ({covered}/{total})")
+                    f"<lines> beneath it imply {covered / total:.2f} ({covered}/{total}); "
+                    f"make the declared rate match the payload, and if a scenario prompt "
+                    f"or rubric quotes the old figure, update it too")
 
 
 def report_power(specs: list[str]) -> None:
@@ -233,7 +254,7 @@ def report_power(specs: list[str]) -> None:
             doc = yaml.safe_load(fh) or {}
         n = len(doc.get("stimuli") or [])
         if n <= 3:
-            need = T95.get(n, 1.96) / math.sqrt(n) if n >= 2 else float("inf")
+            need = T95.get(n - 1, 1.96) / math.sqrt(n) if n >= 2 else float("inf")
             thin.append((n, need, spec))
     if not thin:
         return
@@ -305,7 +326,7 @@ def main() -> int:
 
     print(f"Eval quality gate — checked {len(specs)} eval spec(s).\n")
     if warnings:
-        print("WARNINGS (reported, not failing):")
+        print("WARNINGS (reported; failing only with --strict):")
         for w in warnings:
             print(f"  {w}")
         print()

eng/eval-quality/selftest_eval_quality.py

@@ -70,6 +70,28 @@ def case(label, mutate, expect_fail):
         shutil.rmtree(d, ignore_errors=True)
 
 
+def output_case(label, mutate, expect_substring):
+    """Assert on what the gate *reports*, for checks that warn rather than fail.
+
+    The exit code is asserted too: warnings are printed before errors, so a
+    scratch tree that failed for an unrelated reason would still emit the
+    expected substring and this case would pass while the gate was broken.
+    """
+    d = scratch()
+    try:
+        mutate(d)
+        subprocess.run(["git", "add", "-A"], cwd=d, capture_output=True)
+        code, out = run_gate(d)
+        ok = code == 0 and expect_substring in out
+        print(f"  [{'OK ' if ok else 'BAD'}] {label:<52} expected={expect_substring!r}")
+        if not ok:
+            print(f"        exit={code}")
+            print("        " + out.strip().replace("\n", "\n        ")[:900])
+        return ok
+    finally:
+        shutil.rmtree(d, ignore_errors=True)
+
+
 EV = lambda d: os.path.join(d, "tests", "demo", "widget", "eval.yaml")
 
 
@@ -119,6 +141,28 @@ def inconsistent_file_totals(d):
         )
 
 
+def aggregate_contradicts_payload(d):
+    # Every method agrees with its own <lines>, and the file summary attributes
+    # agree with the declared file line-rate — so checks 3 and 4 both pass. Only
+    # the file/package/class rates contradict the lines actually enumerated
+    # (1/4 = 0.25, not 0.75). This is the coverage-analysis/plateau shape.
+    p = os.path.join(d, "tests", "demo", "widget", "fixtures", "sample", "coverage.cobertura.xml")
+    with open(p, "w") as f:
+        f.write(
+            '<?xml version="1.0"?>'
+            '<coverage line-rate="0.75" lines-covered="3" lines-valid="4">'
+            '<packages><package name="p" line-rate="0.75">'
+            '<classes><class name="C" filename="C.cs" line-rate="0.75"><methods>'
+            '<method name="Covered" signature="()" line-rate="1.00">'
+            '<lines><line number="1" hits="1"/></lines>'
+            "</method>"
+            '<method name="Blocker" signature="()" line-rate="0.00">'
+            '<lines><line number="3" hits="0"/><line number="4" hits="0"/>'
+            '<line number="5" hits="0"/></lines>'
+            "</method></methods></class></classes></package></packages></coverage>"
+        )
+
+
 def empty_grader_config(d):
     # An edit that leaves `- type: output-matches` / `config:` with the pattern
     # attached to the NEXT list item. The document still parses; the grader
@@ -134,6 +178,20 @@ def empty_grader_config(d):
         )
 
 
+def three_scenarios(d):
+    # n=3, so the power warning must quote t(n-1)/sqrt(n) = 4.303/sqrt(3) = 2.48.
+    # Reading the critical value at t(n) instead yields 1.84 — the off-by-one
+    # that made thin evals look close to credible when they were not.
+    with open(EV(d), "a") as f:
+        for i in (2, 3):
+            f.write(
+                f"  - name: Does the thing {i}\n"
+                f"    prompt: do it {i}\n"
+                f"    rubric:\n"
+                f"      - Did the thing {i}\n"
+            )
+
+
 def guard_with_reject_skills(d):
     with open(EV(d), "a") as f:
         f.write(
@@ -166,9 +224,11 @@ def guard_ok(d):
     case("fixture present but NOT tracked by git", untracked_fixture, expect_fail=True),
     case("Cobertura line-rate contradicts its <lines>", bad_cobertura, expect_fail=True),
     case("Cobertura file totals contradict file line-rate", inconsistent_file_totals, expect_fail=True),
+    case("Cobertura aggregate rate contradicts its payload", aggregate_contradicts_payload, expect_fail=True),
     case("grader with an empty config enforces nothing", empty_grader_config, expect_fail=True),
     case("dormancy guard also sets reject_skills", guard_with_reject_skills, expect_fail=True),
     case("well-formed dormancy guard", guard_ok, expect_fail=False),
+    output_case("power threshold uses t(n-1), not t(n)", three_scenarios, "> 2.48"),
 ]
 print()
 if all(results):

tests/dotnet-test/coverage-analysis/eval.yaml

@@ -71,7 +71,7 @@ stimuli:
       - Identifies CalculateGpa as untested or highest risk due to complexity with no coverage
       - Provides prioritized recommendations for where to add tests
   - name: Coverage plateau diagnosis
-    prompt: My coverage is stuck at 75% and I can't get it higher. What's blocking me? Coverage data is in
+    prompt: My coverage is stuck at 47% and I can't get it higher. What's blocking me? Coverage data is in
       TestResults/coverage.cobertura.xml.
     environment:
       files:

tests/dotnet-test/coverage-analysis/fixtures/plateau/coverage.cobertura.xml

@@ -1,9 +1,9 @@
 <?xml version="1.0" encoding="utf-8"?>
-<coverage line-rate="0.75" branch-rate="0.60" version="1.0" timestamp="1712437200" lines-covered="45" lines-valid="60" branches-covered="12" branches-valid="20">
+<coverage line-rate="0.47" branch-rate="0.44" version="1.0" timestamp="1712437200" lines-covered="22" lines-valid="47" branches-covered="7" branches-valid="16">
   <packages>
-    <package name="ContosoUniversity.Services" line-rate="0.75" branch-rate="0.60" complexity="18">
+    <package name="ContosoUniversity.Services" line-rate="0.47" branch-rate="0.44" complexity="18">
       <classes>
-        <class name="ContosoUniversity.Services.StudentService" filename="ContosoUniversity/StudentService.cs" line-rate="0.75" branch-rate="0.60" complexity="18">
+        <class name="ContosoUniversity.Services.StudentService" filename="ContosoUniversity/StudentService.cs" line-rate="0.47" branch-rate="0.44" complexity="18">
           <methods>
             <method name="Enroll" signature="(System.String,System.String,System.Int32)" line-rate="0.94" branch-rate="0.83" complexity="6">
               <lines>