Repository navigation
fix(skill-creator): make the trigger-eval harness measure skill triggering - #1755
giovannibeltrame wants to merge 1 commit into
Conversation
…ering run_eval.py and run_loop.py could not measure whether a description triggers a skill. Positives scored 0% recall and the same query alternated between 0.0 and 1.0 across runs, so the loop optimized against noise. Five defects, all the same class -- the experiment measured something other than the description: The probe competed with the real skill. find_project_root() walked up from cwd to the nearest .claude/, landing in a tree that already registers the skill under test. Claude invoked the real name while the probe watched for its uuid-suffixed decoy, scoring every genuine trigger as a miss. Each probe now builds and tears down its own root. The detector gave up at the first orienting command. It returned False the moment a tool call was not Skill or Read, and Claude's opening move is usually a Bash locating itself, consulting the skill a turn later. That coin flip was the source of the non-determinism. A trigger now counts anywhere in the run, bounded by MAX_PROBE_TURNS=4 so a run that was never going to trigger stops before executing the whole task. Probes shared a directory. With more than one worker, several decoys were live at once, identical but for the uuid, so a worker could watch its own copy while Claude invoked another's. No stdin redirect. Every probe waited 3s for piped input that never arrived. An installed copy still won. The probe root isolates project scope, but a user-level ~/.claude/skills entry or a plugin reaches the probe anyway and outscores the decoy -- the same false-miss, reported silently as 0%. The init event names what is registered, so the run now aborts with an error naming the offending copy. --project-root becomes --fixture, reflecting what it now means: a fixture copied into each probe's sandbox. Skills whose queries only make sense somewhere specific need one -- in a bare directory Claude answers "there's nothing here" instead of reaching for the skill, and every positive looks like a miss. Any .claude/skills or .claude/commands the fixture carries is stripped from the copy. The default stays a bare temp dir, and the harness stops writing probe files into the caller's .claude/commands/, which is not gitignored. --timeout rises from 30s to 120s, a safety net rather than the expected duration of a negative. Verified against skills/mcp-builder, two positives and two negatives, two runs each. Stock harness: positives 0.0/0.5, 0.0/0.5, 0.0/0.5 over three runs. Patched: 1.0/1.0 on all three, negatives 0.0 throughout. Collision detection confirmed against a skill installed at user scope. No temp roots survive a run and the fixture is never modified. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
|
Thanks for this — the five defects are real and the verification table is convincing. One safety edge case in the fixture-stripping logic ( If the fixture contains
Verified empirically on Python 3.9.6 (macOS default): with Suggested fix: guard claude = root / ".claude"
if claude.is_symlink() or claude.is_file():
claude.unlink()
elif claude.is_dir():
for leaked in (claude / "skills", claude / "commands"):
if leaked.is_symlink() or leaked.is_file():
leaked.unlink()
elif leaked.is_dir():
shutil.rmtree(leaked)Everything else reviewed clean — probe isolation, the turn cap, stdin fix, and the init-event collision guard all look right, and |
|
Reviewed the eval-harness isolation redesign — solid overall. Per-probe temp roots, the decoy-alone assertion on the One minor fidelity note: |
2 participants
skills/skill-creator/scripts/run_eval.pyandrun_loop.pycannot currently measure whether a description triggers a skill. Every positive query scores 0% recall, and the same query alternates between 0.0 and 1.0 across runs — sorun_loop.pyoptimizes descriptions against noise.Five defects, all the same class: the experiment measures something other than the description.
The defects
The probe competes with the real skill.
find_project_root()walks up from cwd to the nearest.claude/, which lands in a tree that already registers the skill under test — the common case, since you run the eval from the repo holding the skill. Claude invokes the real name while the probe watches for its uuid-suffixed decoy, so every genuine trigger scores as a miss. That is the 100% precision / 0% recall signature. Each probe now builds and tears down its own root.The detector gives up at the first orienting command. It returns
Falsethe moment a tool call is notSkillorRead. Reading the raw stream, Claude's opening move is usually aBashlocating itself (pwd,ls), and it consults the skill a turn later. That ordering is close to a coin flip, and it is the source of the non-determinism. A trigger now counts anywhere in the run, bounded byMAX_PROBE_TURNS = 4so a run that was never going to trigger stops before executing the whole queried task.Probes share a directory. With more than one worker, several decoy commands are live at once. They differ only by uuid, so a worker can watch its own copy while Claude invokes another's.
No stdin redirect.
claude -pis spawned withoutstdin, so every probe waits 3s for piped input that never arrives.An installed copy still wins. A private probe root isolates project scope, but a user-level
~/.claude/skills/<name>entry or a plugin reaches the probe anyway, and the real name outscores the decoy — the same false-miss, reported silently as 0%. This is easy to hit: testing a skill you actually use. Thesystem/initevent names what is registered, so the run now aborts with an error naming the offending copy instead of reporting a plausible zero.User-facing changes
--project-rootbecomes--fixture, reflecting what it now means: a fixture copied into each probe's sandbox. The old name invited pointing it at the real project — precisely the false-negative case the isolation exists to prevent. Skills whose queries only make sense somewhere specific need one: in a bare directory Claude answers "there's nothing here" rather than reaching for the skill, and every positive looks like a miss. Any.claude/skillsor.claude/commandsthe fixture carries is stripped from the copy, so the decoy stays the only registered skill; the rest of the fixture survives intact, and the original is never modified. The default remains a bare temp dir.--timeoutrises from 30s to 120s. With the turn cap in place it is a safety net rather than the expected duration of a negative.As a side effect, the harness stops writing probe files into the caller's
.claude/commands/. That path is not gitignored, so an interrupted run left untracked files staged for an accidental commit.Verification
Against
skills/mcp-builder(not installed locally, so the collision check passes), two positives and two negatives, 2 runs per query, 4 workers:Three independent runs of each. The stock harness never scores a positive above 0.5 and reports one of them as a hard failure; this PR separates positives from negatives cleanly and repeats it.
Also checked: the collision guard fires against a skill installed at user scope (
skill-creatoritself) and exits with a message naming it; no temp roots survive a run; the fixture is never modified; both CLIs exit with a clear error on a missing--fixture;quick_validate.pypasses.Run 8 probes in ~20s, against minutes before — the turn cap, not a change in what is measured.
Notes
This was found and fixed downstream while optimizing a skill description, against a vendored copy of
skill-creatorbyte-identical to this repo's currentmain. The fifth defect above surfaced only when re-verifying the port here, against a skill installed at user scope. No behavior outsideskill-creator's eval scripts changes.🤖 Generated with Claude Code