diff --git a/.changes/evals/fix-11979-measured-fire-5c1e8d20.md b/.changes/evals/fix-11979-measured-fire-5c1e8d20.md new file mode 100644 index 00000000000..f9054713037 --- /dev/null +++ b/.changes/evals/fix-11979-measured-fire-5c1e8d20.md @@ -0,0 +1,12 @@ +--- +bump: patch +--- + +### Fixed + +- `run-validity.py` takes `--measured-fire ` (repeatable) for a should-trigger case whose + firing is the quantity a comparison measures, such as the base side of a change that makes a + skill fire. The skill-fired check reports that case's misses as a measured rate, as it does for + an invocation probe, instead of marking the whole file INVALID; every other check is unchanged. + `reading-results.md` says when to pass it + ([#11979](https://github.com/melodic-software/claude-code-plugins/issues/11979)). diff --git a/plugins/evals/skills/plugin-eval/reference/reading-results.md b/plugins/evals/skills/plugin-eval/reference/reading-results.md index 0d97a2a94a1..df3fe957a2f 100644 --- a/plugins/evals/skills/plugin-eval/reference/reading-results.md +++ b/plugins/evals/skills/plugin-eval/reference/reading-results.md @@ -21,7 +21,9 @@ statement of record: with `--json ` the terminal summary table is suppress [SKILL.md, Reading the delta](../SKILL.md#reading-the-delta). Add `--scaffolded` only when the eval ran with `--scaffold`. Without the flag, the `scaffold` check fails each case whose `case.yaml` sets `context.scaffold_script`; a `case.yaml` it cannot locate or read leaves that - check unchecked. + check unchecked. When the comparison measures whether a skill fires, as on the base side of a + change that makes it fire, name each such case with `--measured-fire `: its misses are + reported as a measured rate, not a failure, and every other check still applies. - **Pointer**: [Add setup or history with case.yaml](https://code.claude.com/docs/en/plugin-evals#add-setup-or-history-with-case-yaml); the `--scaffold` row of [Command options](https://code.claude.com/docs/en/plugin-evals#command-options); diff --git a/plugins/evals/skills/plugin-eval/scripts/run-validity.py b/plugins/evals/skills/plugin-eval/scripts/run-validity.py index 3114492b240..93a5591b736 100755 --- a/plugins/evals/skills/plugin-eval/scripts/run-validity.py +++ b/plugins/evals/skills/plugin-eval/scripts/run-validity.py @@ -1,11 +1,14 @@ #!/usr/bin/env python3 """run-validity - decide whether a `claude plugin eval` run's score may count. - run-validity --runs N [--scaffolded] + run-validity --runs N [--scaffolded] [--measured-fire CASE]... --runs is the run count the eval was invoked with (its --runs, else the case default). --scaffolded records that the eval was invoked with --scaffold, which -the result file does not record. Reads the result file, each case's case.yaml, +the result file does not record. --measured-fire names a should-trigger case +whose firing is the quantity a comparison measures, such as the base side of a +change that makes a skill fire; the skill-fired check reports its misses as a +measured rate, as for an invocation probe, and every other check is unchanged. Reads the result file, each case's case.yaml, and, where each run's tracePath resolves, the kept trace (`--keep-temp`). A relative tracePath or suite root is read relative to the result file. Prints one line per check, detail lines indented under it: @@ -53,8 +56,9 @@ held; a should-fire miss with a denial takes the rules above skill fired every with-arm run of a should-trigger case fired the skill; - in an invocation-probe case a miss is the trigger rate it - measures, reported per case and overall as a warning + in an invocation-probe case, or one named by + --measured-fire, a miss is the trigger rate it measures, + reported per case and overall as a warning probe tools no invocation-probe run's session could call Read, Glob, Grep or Agent, which the emitted README removes with --disallow-tools; one that could is a warning naming that @@ -1011,9 +1015,9 @@ def check_denials(runs, roots, root): return PASS, "no denied tool call in %s" % plural(len(traced), "trace"), [], None -def check_fired(cases, runs, root): +def check_fired(cases, runs, root, measured_fire=frozenset()): unfired, unread, controls, unknown, checked = [], [], [], [], 0 - measured = {} # invocation-probe case name -> [fired, with-arm runs] + measured = {} # invocation-probe or --measured-fire case name -> [fired, with-arm runs] for case in cases: requiring, _ = skill_graders(case, root) if is_control(case, root): @@ -1022,7 +1026,7 @@ def check_fired(cases, runs, root): if not requiring: unknown.append(case.get(CASE_NAME)) continue - probe = is_probe(case, root) + probe = is_probe(case, root) or case.get(CASE_NAME) in measured_fire for r in runs: if r.case is not case or r.arm != "with": continue @@ -1047,10 +1051,11 @@ def check_fired(cases, runs, root): notes.append("no skill-fired grader, so not checked: %s" % ", ".join(unknown)) fired = sum(f for f, _ in measured.values()) probed = sum(n for _, n in measured.values()) + label = "measured cases" if set(measured) & measured_fire else "invocation probes" if measured: notes.append( - "invocation probes fired in %d of %s, the measured rate, not gated" - % (fired, plural(probed, "with-arm run")) + "%s fired in %d of %s, the measured rate, not gated" + % (label, fired, plural(probed, "with-arm run")) ) rates = [ "measured: case %s fired in %d of %s" % (name, f, plural(n, "with-arm run")) @@ -1060,7 +1065,7 @@ def check_fired(cases, runs, root): tail = ("; " + "; ".join(notes)) if notes else "" fired_in = "fired in %s" % plural(checked, "with-arm run") if measured and not checked: - fired_in = "no should-trigger case outside the invocation probes" + fired_in = "no should-trigger case outside the %s" % label if unfired: return ( FAIL, @@ -1085,7 +1090,7 @@ def check_fired(cases, runs, root): fired_in + tail, rates, ( - "invocation probes fired in %d of %s" % (fired, plural(probed, "run")) + "%s fired in %d of %s" % (label, fired, plural(probed, "run")) if rates else None ), @@ -1322,6 +1327,13 @@ def main(argv=None): action="store_true", help="the eval was invoked with --scaffold", ) + parser.add_argument( + "--measured-fire", + action="append", + default=[], + metavar="CASE", + help="a should-trigger case whose firing is the quantity compared (repeatable)", + ) args = parser.parse_args(argv) if args.runs < 1: sys.stderr.write("error: --runs must be 1 or more\n") @@ -1383,7 +1395,7 @@ def main(argv=None): later = [ ("session started", check_started(runs)), ("permission denials", check_denials(runs, plugin_roots, root)), - ("skill fired", check_fired(cases, runs, root)), + ("skill fired", check_fired(cases, runs, root, frozenset(args.measured_fire))), ("probe tools", check_probe_tools(runs, root)), ("models", check_models(result, runs)), ("judge votes", check_votes(runs)), diff --git a/plugins/evals/skills/plugin-eval/scripts/test_run_validity.py b/plugins/evals/skills/plugin-eval/scripts/test_run_validity.py index 4248d3bed41..26fa2f3815e 100755 --- a/plugins/evals/skills/plugin-eval/scripts/test_run_validity.py +++ b/plugins/evals/skills/plugin-eval/scripts/test_run_validity.py @@ -1119,6 +1119,36 @@ def test_an_unfired_invocation_probe_is_its_measured_rate(self): ) self.assertNotIn("unfired:", proc.stdout) + def test_an_unfired_case_named_measured_fire_is_its_measured_rate(self): + result = self.clean() + result["cases"][2]["arms"]["with"][0]["graders"][1]["passed"] = False + path = self.write(result) + proc = self.run_script(path, "--runs", "1") + self.assertVerdict(proc, "INVALID", 1) + proc = self.run_script( + path, "--runs", "1", "--measured-fire", "noise-before-gain" + ) + self.assertVerdict(proc, "VALID", 0) + self.assertIn("check skill fired: WARN", proc.stdout) + self.assertIn( + " measured: case noise-before-gain fired in 0 of 1 with-arm run\n", + proc.stdout, + ) + self.assertIn("measured cases fired in 0 of 1", proc.stdout) + self.assertNotIn("invocation probes", proc.stdout) + self.assertNotIn("unfired:", proc.stdout) + + def test_measured_fire_leaves_other_unfired_cases_invalid(self): + result = self.clean() + result["cases"][1]["arms"]["with"][0]["graders"][1]["passed"] = False + result["cases"][2]["arms"]["with"][0]["graders"][1]["passed"] = False + proc = self.run_script( + self.write(result), "--runs", "1", "--measured-fire", "noise-before-gain" + ) + self.assertVerdict(proc, "INVALID", 1) + self.assertIn(" unfired: case measurable-criterion, with-arm run 1", proc.stdout) + self.assertNotIn("unfired: case noise-before-gain", proc.stdout) + def test_an_unfired_case_beside_an_unfired_probe_is_still_invalid(self): result = self.clean() result["cases"][1]["arms"]["with"][0]["graders"][1]["passed"] = False