diff --git a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-3-valid-no-cve/expected.json b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-3-valid-no-cve/expected.json index 8c7d5a79d..e38e12f07 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-3-valid-no-cve/expected.json +++ b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-3-valid-no-cve/expected.json @@ -1,5 +1,4 @@ { "process_step": "6", - "step_description": "Consensus reached: issue is valid and CVE-worthy, but no CVE has been allocated yet", - "key_signals": ["team consensus recorded in comments", "no CVE tool link", "no cve allocated label", "scope label airflow set"] + "step_description": "Consensus reached: issue is valid and CVE-worthy, but no CVE has been allocated yet" } diff --git a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-4-cve-allocated-no-pr/expected.json b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-4-cve-allocated-no-pr/expected.json index eb08ba666..ff28fa742 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-4-cve-allocated-no-pr/expected.json +++ b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-4-cve-allocated-no-pr/expected.json @@ -1,5 +1,4 @@ { "process_step": "7", - "step_description": "CVE allocated, no fix PR opened yet", - "key_signals": ["cve allocated label set", "CVE-2025-44812 in CVE tool link field", "no PR with the fix"] + "step_description": "CVE allocated, no fix PR opened yet" } diff --git a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-5-pr-merged-awaiting-release/expected.json b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-5-pr-merged-awaiting-release/expected.json index 49a3251af..109439c36 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-5-pr-merged-awaiting-release/expected.json +++ b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-5-pr-merged-awaiting-release/expected.json @@ -1,5 +1,4 @@ { "process_step": "11", - "step_description": "Fix PR merged into upstream; release carrying the fix has not shipped yet", - "key_signals": ["pr merged label set", "fix PR merged at 2025-10-14", "release 3.0.3 not yet on PyPI"] + "step_description": "Fix PR merged into upstream; release carrying the fix has not shipped yet" } diff --git a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-7-announced-awaiting-cveorg/expected.json b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-7-announced-awaiting-cveorg/expected.json index faca7ddd4..9a690f159 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-7-announced-awaiting-cveorg/expected.json +++ b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/case-7-announced-awaiting-cveorg/expected.json @@ -1,5 +1,4 @@ { - "process_step": "14", - "step_description": "Advisory sent and Public advisory URL populated; announced label set; awaiting cve.org propagation before closing", - "key_signals": ["announced label set", "Public advisory URL populated", "cve.org state is RESERVED not PUBLISHED"] + "process_step": "14-15", + "step_description": "Archive URL captured — the combined apply is due, and the tracker is still open" } diff --git a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/output-spec.md b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/output-spec.md index 1a5ecf0d9..aba979e6f 100644 --- a/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/output-spec.md +++ b/tools/skill-evals/evals/security-issue-sync/step-1f-process-step/fixtures/output-spec.md @@ -20,6 +20,10 @@ Use the exact step number strings from the table (e.g. `"1-2"`, `"3"`, `"4"`, `"5/6"`, `"6"`, `"7"`, `"11"`, `"12"`, `"13"`, `"14"`, `"15"`). `key_signals` lists the labels, body-field values, or observable facts that determined the step — helps reviewers understand why the step was chosen. +Always emit it. +It is explanatory rather than asserted: which facts are worth naming, and +how many, is a judgement call, so the cases grade `process_step` and +`step_description` and leave this field to the reader. If the observed state simultaneously satisfies two CONSECUTIVE numbered rows of the table — i.e. the tracker has just completed the earlier row's diff --git a/tools/skill-evals/src/skill_evals/runner.py b/tools/skill-evals/src/skill_evals/runner.py index 3a67e992f..3f73c48fe 100644 --- a/tools/skill-evals/src/skill_evals/runner.py +++ b/tools/skill-evals/src/skill_evals/runner.py @@ -610,9 +610,38 @@ def batch_grade_prose_fields( failure (timeout, OSError, non-zero exit, unparsable output, missing path in the verdict), every pair without a clean verdict is returned as ``(False, )``. + + A verdict the grader simply *omits* is retried once, on its own, before + being reported. One prompt covers every pair, so a grader that drops a + path — which it does intermittently on larger batches — fails that pair + for a reason that has nothing to do with the candidate output, and is + indistinguishable in the report from a real mismatch. Re-asking for the + dropped subset removes that class of false red. A pair the grader + actively judges is never re-asked: only silence is retried, so a `NO` + cannot be turned into a `YES` by asking twice. """ if not pairs: return {} + graded = _batch_grade_once(pairs, grader_cli, timeout) + missing = [pair for pair in pairs if pair[0] not in graded] + if missing: + graded.update(_batch_grade_once(missing, grader_cli, timeout, final=True)) + return graded + + +def _batch_grade_once( + pairs: list[tuple[str, object, object]], + grader_cli: str, + timeout: int, + *, + final: bool = False, +) -> dict[str, tuple[bool, str]]: + """One grader round-trip. + + Pairs the grader returned no verdict for are omitted from the result + unless ``final``, in which case they are reported as a failure so the + caller never silently drops a field. + """ prompt = BATCH_GRADER_RUBRIC.format(fields_block=_format_batch_fields_block(pairs)) try: stdout, stderr, rc = run_cli(grader_cli, prompt, timeout=timeout) @@ -629,7 +658,8 @@ def batch_grade_prose_fields( for path, _, _ in pairs: entry = verdict.get(path) if not isinstance(entry, dict) or "match" not in entry: - result[path] = (False, f"grader did not return a verdict for {path}") + if final: + result[path] = (False, f"grader did not return a verdict for {path}") continue match = bool(entry.get("match")) reason = str(entry.get("reason", "")).strip() diff --git a/tools/skill-evals/tests/_grader_drops_one.py b/tools/skill-evals/tests/_grader_drops_one.py new file mode 100644 index 000000000..d3382e29b --- /dev/null +++ b/tools/skill-evals/tests/_grader_drops_one.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +"""Mock batched grader that silently drops one path on its first call. + +Stands in for the real grader's intermittent habit of omitting a field from +a larger batch. The first invocation returns a verdict for every ``Field:`` +path except the last one; every later invocation verdicts everything it is +asked about. Call state is kept in the file named by +``GRADER_DROP_STATE_FILE``. +""" + +from __future__ import annotations + +import json +import os +import re +import sys +from pathlib import Path + + +def main() -> None: + paths = re.findall(r"^Field: (\S+)$", sys.stdin.read(), flags=re.MULTILINE) + state = Path(os.environ["GRADER_DROP_STATE_FILE"]) + first_call = not state.exists() + state.write_text((state.read_text() if state.exists() else "") + "call\n") + answered = paths[:-1] if first_call and len(paths) > 1 else paths + print(json.dumps({p: {"match": True, "reason": "ok"} for p in answered})) + + +if __name__ == "__main__": + main() diff --git a/tools/skill-evals/tests/test_runner.py b/tools/skill-evals/tests/test_runner.py index 2dd3f6f5b..05d4557c5 100644 --- a/tools/skill-evals/tests/test_runner.py +++ b/tools/skill-evals/tests/test_runner.py @@ -1437,6 +1437,40 @@ def test_batch_grade_grader_failure_marks_all_fail(): assert ok is False +def test_batch_grade_retries_a_dropped_verdict(tmp_path: Path): + """A path the grader silently omits is re-asked, not failed. + + The grader drops fields from larger batches intermittently, which has + nothing to do with the candidate output and is indistinguishable in the + report from a real mismatch. + """ + state = tmp_path / "drop-state" + inner = ( + f"GRADER_DROP_STATE_FILE={shlex.quote(str(state))} " + f"python3 {shlex.quote(str(_TESTS_DIR / '_grader_drops_one.py'))}" + ) + grader = f"bash -c {shlex.quote(inner)}" + pairs = [("$.a", "x", "y"), ("$.b", "x", "y")] + result = batch_grade_prose_fields(pairs, grader, timeout=5) + assert result["$.a"] == (True, "") + assert result["$.b"] == (True, "") + assert state.read_text().count("call") == 2 + + +def test_batch_grade_reports_a_verdict_missing_twice(tmp_path: Path): + """Silence that survives the retry is still reported, never dropped.""" + script = _TESTS_DIR / "_grader_empty.py" + script.write_text("import sys; sys.stdin.read(); print('{}')\n") + try: + pairs = [("$.a", "x", "y")] + result = batch_grade_prose_fields(pairs, f"python3 {shlex.quote(str(script))}", timeout=5) + ok, note = result["$.a"] + assert ok is False + assert "did not return a verdict" in note + finally: + script.unlink() + + # --------------------------------------------------------------------------- # compare_with_grader (uses the batched grader path) # ---------------------------------------------------------------------------