fix(skill_eval): add conflict guard and normalize descriptionOverride#28
Open
mxl wants to merge 1 commit into
Open
fix(skill_eval): add conflict guard and normalize descriptionOverride#28mxl wants to merge 1 commit into
mxl wants to merge 1 commit into
Conversation
- Add assertNoInstalledSkillConflict() to detect when a skill with the same base name is already available to opencode, which causes false negatives in trigger eval (synthetic skill never gets triggered). - Normalize descriptionOverride: treat empty string as omitted so callers can't accidentally override with . - Add Trigger Eval Preflight section to SKILL.md documenting the issue. - Apply guards in both skill_eval and skill_optimize_loop tools.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
skill_evalcreates a synthetic skill named<skill-name>-skill-<id>and only counts invocations of that exact temporary name as triggered. If a real skill with the base name is already visible to OpenCode (viaskills.paths, installed in~/.config/opencode/skills/, or project-level.opencode/skills/), the model may load the real skill instead of the synthetic one, producing false negatives — the eval reportstrigger_rate: 0even though the skill description is good.Additionally, passing an empty string as
descriptionOverridewould override the SKILL.md description with"", which is almost never intentional.Changes
plugin/lib/run-eval.ts: AddassertNoInstalledSkillConflict()that runsopencode debug skilland throws a clear error if the skill name appears in the installed skills list.plugin/skill-creator.ts:normalizeDescriptionOverride()— treats empty/whitespace-only strings asundefinedso the SKILL.md description is used.assertNoInstalledSkillConflict()before bothskill_evalandskill_optimize_loopexecution.plugin/skill/SKILL.md: Add a "Trigger Eval Preflight" section documenting the issue and the automatic guard.Testing
npm run build)skills.pathsresolves the false-negative issue