You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/09/22 22:16:00 UTC
(magpie) branch fix/skill-evals-grader-retry-and-1f-expectations created (now 21aa3626)
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a change to branch fix/skill-evals-grader-retry-and-1f-expectations
in repository https://gitbox.apache.org/repos/asf/magpie.git
at 21aa3626 fix(skill-evals): retry a dropped grader verdict, and correct the step 1f expectations
This branch includes the following new commits:
new 21aa3626 fix(skill-evals): retry a dropped grader verdict, and correct the step 1f expectations
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(magpie) 01/01: fix(skill-evals): retry a dropped grader verdict, and correct the step 1f expectations
Posted by po...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch fix/skill-evals-grader-retry-and-1f-expectations
in repository https://gitbox.apache.org/repos/asf/magpie.git
commit 21aa3626d7cf379176ff16555bca1ddc491b297f
Author: Jarek Potiuk <po...@apache.org>
AuthorDate: Wed Sep 23 00:11:38 2026 +0200
fix(skill-evals): retry a dropped grader verdict, and correct the step 1f expectations
Two of the cases left red by #1340 were the fixture disagreeing with the
documented process rather than the model getting it wrong, and verifying
that turned up a third problem in the runner itself.
**A verdict the grader drops is no longer a failure.**
`batch_grade_prose_fields` sends one rubric prompt covering every pair,
and the grader intermittently omits a path from a larger batch. That
pair was then recorded as `(False, "grader did not return a verdict")` —
a FAIL indistinguishable in the report from a real mismatch, for a
reason that has nothing to do with the candidate output. It is the
mechanism behind cases that appear to flap between runs of unchanged
code. The dropped subset is now re-asked once; silence that survives the
retry is still reported, and a verdict the grader actively gives is
never re-asked, so a NO cannot become a YES by asking twice. The
structural-assertion path already drew this distinction with a tri-state
verdict; the prose path now does too.
**`case-7` expected the wrong step.** The process table in `gather.md` —
the table this step tells the model to cross-reference — puts "Archive
URL captured" at `14 → 15`, and the step's output spec says in as many
words that when two consecutive rows both apply the answer is the
inclusive hyphenated range. The tracker in this fixture is still open
with the archive URL populated, so it sits on that row. The model
answered `"14-15"` on every run; the fixture asserted `"14"`.
**`case-5` asserted a field the spec calls explanatory.** `key_signals`
exists so a reviewer can see why a step was chosen, and which facts are
worth naming is a judgement call. The model named five signals that
subsume the expected three and the case failed on the difference. Cases
1, 2 and 6 in this same step already omit the field, so 3, 4 and 5 now
match the step's own majority and the spec records that it is emitted
but not graded. Same fragility #1340 removed from the guardrails
`evidence` field.
`case-7`'s `step_description` is also shortened to the one-line shape
the spec asks for; the first attempt enumerated every action in the
combined apply, which made a prose-graded field fail on whichever
sub-action the model left out.
The verifying run is 43/45 with no dropped verdicts, against 42/45 and
two dropped verdicts before. The two that remain are the grader's
judgement on free text — one on whether "valid" implies "CVE-worthy",
one on whether naming the confirmation step over-describes the action —
and both have passed on other runs. They are left alone deliberately:
rewording a fixture until a rubric happens to agree tunes the test to
the model. Narrowing what the prose fields assert is the real fix, and
it wants deciding on its own terms rather than inside this change.
Generated-by: Claude Opus 5
---
.../fixtures/case-3-valid-no-cve/expected.json | 3 +-
.../case-4-cve-allocated-no-pr/expected.json | 3 +-
.../expected.json | 3 +-
.../case-7-announced-awaiting-cveorg/expected.json | 5 ++-
.../step-1f-process-step/fixtures/output-spec.md | 4 +++
tools/skill-evals/src/skill_evals/runner.py | 32 +++++++++++++++++-
tools/skill-evals/tests/_grader_drops_one.py | 39 ++++++++++++++++++++++
tools/skill-evals/tests/test_runner.py | 34 +++++++++++++++++++
8 files changed, 113 insertions(+), 10 deletions(-)
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 8c7d5a79..e38e12f0 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 eb08ba66..ff28fa74 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 49a3251a..109439c3 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 faca7ddd..9a690f15 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 1a5ecf0d..aba979e6 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 3a67e992..3f73c48f 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, <one-line explanation>)``.
+
+ 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 00000000..d3382e29
--- /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 2dd3f6f5..05d4557c 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)
# ---------------------------------------------------------------------------