fix(skill-creator): close run_eval's 0% recall and harden the loop - #996
fix(skill-creator): close run_eval's 0% recall and harden the loop#996ob-aion wants to merge 2 commits into
Conversation
|
@maheshmurag @rlancemartin — could one of you take a look when you get a chance? This fixes #556 (run_eval.py reporting a 0% trigger rate) and has been open since April, mergeable with no conflicts. Update: #1298 independently reworks the same run_eval.py path, and its isolated temp-project approach is the cleaner base — I've followed up there to consolidate. The two are largely complementary: #1298 handles run_eval, while this PR also fixes run_loop.py, where an installed same-name skill shadows the eval and pins recall at 0%. Happy to fold that into #1298 so there's a single thing to review. |
run_eval reported 0% recall for every query, so the optimizer scored noise (anthropics#556). Two root causes: - The candidate was written to .claude/commands/, which claude -p never auto-triggers. It now installs as a real skill in an isolated, UUID-keyed temp project — the surface that actually auto-triggers. - Skill resolution is user > project, so a same-name skill already under ~/.claude/skills shadowed the candidate and the eval measured the installed description. run_eval now moves that copy aside for the run. Carries over the isolated-project reader (threads+queue over select() on Windows, shutil.which for the .cmd shim, --max-turns) and field-scoped detection, so a name like "pdf" stops counting reading report.pdf as a hit. Detection is extracted to a pure _detect_trigger, unit-tested offline. Co-authored-by: MartinCajiao <177078233+MartinCajiao@users.noreply.github.com> Co-authored-by: Khalifa Gad <9096880+KhalifaGad@users.noreply.github.com>
Two crashes on the early-exit paths the eval fix makes more reachable: - run_loop and generate_report call max(history, ...) unguarded, so a run that exits before the first iteration appends (e.g. --max-iterations 0) hits max() on an empty sequence. Both now short-circuit. - An improve_description failure (rate limit, model error, expired creds) propagated uncaught and discarded every iteration collected so far. It is now caught; the loop breaks and keeps the partial history. Co-authored-by: Vaikri-costume <256043949+Vaikri-costume@users.noreply.github.com>
fe90da1 to
046d87c
Compare
|
@maheshmurag @rlancemartin — refreshed this into the complete fix for #556 and rebased on current It folds in @MartinCajiao's #1298 (the isolated-project Also adds the loop crash guards (@Vaikri-costume) and the repo's first offline test scaffolding — 18 tests, stdlib only, no API cost. Verified end to end: a matching query now scores 1.0, a non-matching one 0.0. Merging this closes #556 and supersedes #1298/#1323 in one pass. Happy to adjust anything. |
Summary
Closes the 0% trigger-rate bug in
skill-creator's description eval (#556) end to end.run_eval.pyreported 0% recall for every query, sorun_loop.pywas optimizing against a metric that was always zero.This builds directly on @MartinCajiao's #1298 — I've folded its isolated-project rewrite in here with credit, added the one fix it was missing, and pinned the whole thing with tests, so there's a single reviewable change instead of scattered halves.
Root causes:
.claude/commands/file, whichclaude -pnever auto-triggers. It now installs as a real skill in an isolated, UUID-keyed temp project — the surface that actually auto-triggers.~/.claude/skillsoverrode the candidate and the eval measured the installed description. fix(skill-creator): run_eval.py always reports 0% recall — install the eval artifact as a real skill; fix Windows stream reading, trigger detection, and parallel workers #1298 warns about this; hererun_evalmoves the installed copy aside for the run and restores it after. Without it, recall still reads 0% whenever you optimize a skill you already have installed — the common case.pdfno longer counts readingreport.pdfas a hit. Windows stream reading uses reader threads overselect(), andshutil.whichresolves theclaude.cmdshim.run_loopandgenerate_reportcrashed onmax()over empty history for runs that exit before the first iteration; animprove_descriptionfailure discarded every collected iteration. Both are guarded.What's new since #1298
_detect_trigger, and the repo gets its first test scaffolding: 18 offline tests pinning the metric — short-name false positives, Windows paths, explore-then-trigger, shadow move/restore, the loop guards. Stdlib only, no API cost.Consolidates the scattered attempts at this bug (#1298, #1323). Merging this closes #556 in one pass.
Test plan
cd skills/skill-creator && python3 -m unittest discover -s scripts/tests— 18 passpython -m scripts.run_eval --eval-set <set>.json --skill-path <installed-skill>: recall reflects the description under test, not the installed copypython -m scripts.run_loop --eval-set <set>.json --skill-path <skill> --model <m> --max-iterations 0exits cleanlyFixes #556. Credit: @MartinCajiao (isolated-project rewrite), @KhalifaGad (field-scoped detection), @Vaikri-costume (loop guards), @xg-gh-25 (review that shaped the isolation).