anthropics / anthropics/skills
skill-creator run_eval: a crashed eval run is recorded as "did not trigger", so infrastructure failure scores as a PASS on negative queries (and steers run_loop's optimiser)
- Dominant language
- Python
- Stars
- 176k
- Forks
- 20.8k
- Avg merge
- 7h 21m
- Merged PRs (30d)
- 5
Description
## Summary
In `skills/skill-creator/scripts/run_eval.py`, a query whose eval run raises — a subprocess crash, a timeout, an API error — is recorded as `False`, the same value a legitimate "the skill did not trigger" produces. Nothing in the emitted payload distinguishes them.
For a query with `should_trigger: false`, the pass criterion is `trigger_rate < threshold`, so **an infrastructure failure is scored as a correct result**. Crashing improves the score. And because `run_loop.py` imports `run_eval` as a function and selects its "best" description from exactly these counts, the description optimiser cannot see the difference either.
## Where
```python
# run_eval.py, in run_eval()
try:
query_triggers[query].append(future.result())
except Exception as e:
print(f"Warning: query failed: {e}", file=sys.stderr)
query_triggers[query].append(False) # <-- crash becomes "did not trigger"
```
The emitted envelope carries `query`, `should_trigger`, `trigger_rate`, `triggers`, `runs`, `pass`, and a summary of `total` / `passed` / `failed`. There is no `errored` field, and `runs` counts the crashed run as a real run — so even the denominator looks healthy.
The only trace is the stderr warning. `run_loop.py:20` imports `run_eval` directly (`from scripts.run_eval import find_project_root, run_eval`) and calls it in-process, so for the optimisation loop that warning is not a channel at all — it never touches the data the loop selects on.
## Reproduction
Self-contained; the aggregation is transcribed verbatim from `run_eval` with only the process-pool machinery replaced by injected outcomes.
```python
#!/usr/bin/env python3
"""An infrastructure crash is indistinguishable from — and scores better than — a real result."""
import json
TRIGGER_THRESHOLD = 0.5
def aggregate(query_triggers, query_items):
"""Verbatim from run_eval.py's aggregation."""
results = []
for query, triggers in query_triggers.items():
item = query_items[query]
trigger_rate = sum(triggers) / len(triggers)
should_trigger = item["should_trigger"]
did_pass = (trigger_rate >= TRIGGER_THRESHOLD) if should_trigger else (trigger_rate < TRIGGER_THRESHOLD)
results.append({"query": query, "should_trigger": should_trigger,
"trigger_rate": trigger_rate, "triggers": sum(triggers),
"runs": len(triggers), "pass": did_pass})
passed = sum(1 for r in results if r["pass"])
return {"results": results,
"summary": {"total": len(results), "passed": passed, "failed": len(results) - passed}}
ITEMS = {"please do the thing this skill is for": {"should_trigger": True},
"what is the capital of France": {"should_trigger": False}}
def case(negative_outcome):
return aggregate({"please do the thing this skill is for": [True],
"what is the capital of France": [negative_outcome]}, ITEMS)
healthy = case(False) # negative query ran, correctly did not trigger
crashed = case(False) # negative query CRASHED -> upstream appends False
wrong = case(True) # negative query ran and WRONGLY triggered (a real defect)
assert json.dumps(healthy, sort_keys=True) == json.dumps(crashed, sort_keys=True)
print("payloads byte-identical:", True)
print("score when the negative query crashed: ", crashed["summary"]["passed"], "/2")
print("score when it ran and exposed a real problem: ", wrong["summary"]["passed"], "/2")
```
Output:
```
payloads byte-identical: True
score when the negative query crashed: 2 /2
score when it ran and exposed a real problem: 1 /2
```
To be explicit about what the first assertion is and isn't: `healthy` and `crashed` are the same call *because the coercion upstream has already made them the same call* — `except: append(False)` is what turns a crash into that argument. The assertion isn't demonstrating a surprise inside `aggregate`; it's showing that by the time the aggregation runs, the information needed to tell the two apart is already gone, and nothing downstream can recover it. The third case is the one carrying the weight: the crash doesn't merely lose a diagnostic, it substitutes a passing outcome for a failing one.
The failure mode is not "we lost some diagnostics." It is that a degraded process is mapped into evidence of success, in the signal an optimiser consumes. A flaky negative query is silently a free point, and a description that happens to provoke timeouts scores better than one that does not.
## Suggested fix
Keep errors out of the trigger-outcome value space rather than coercing them into it:
```python
query_errors: dict[str, int] = {}
...
try:
query_triggers[query].append(future.result())
except Exception as e:
print(f"Warning: query failed: {e}", file=sys.stderr)
query_errors[query] = query_errors.get(query, 0) + 1 # do NOT append a trigger outcome
```
then carry it to the surface and let the consumer decide:
```python
errored = query_errors.get(query, 0)
results.append({
...,
"runs": len(triggers), # now genuinely the number of runs that produced an outcome
"errored": errored,
"pass": did_pass,
})
...
"summary": {"total": total, "passed": passed, "failed": total - passed,
"errored": sum(query_errors.values())},
```
A query with `len(triggers) == 0` (every run errored) needs an explicit decision rather than a `ZeroDivisionError` — most likely excluded from `total` with the exclusion reported.
`run_loop.py` can then refuse to compare iterations whose `errored` counts differ materially, instead of optimising over a number it cannot interpret. Even leaving the loop's policy unchanged, having the field means the corruption is *detectable*, which it currently is not.
Happy to open a PR if the shape above looks right — though the `errored`-run policy in `run_loop.py` (exclude the iteration, retry, or just warn) is a judgement call I would rather have a maintainer make first.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.