Skip to content

fix: keep --no-llm negative cases off-skill - #100

Open
mimran-khan wants to merge 6 commits into
NVIDIA:mainfrom
mimran-khan:fix/no-llm-negative-case
Open

fix: keep --no-llm negative cases off-skill#100
mimran-khan wants to merge 6 commits into
NVIDIA:mainfrom
mimran-khan:fix/no-llm-negative-case

Conversation

@mimran-khan

Copy link
Copy Markdown
Contributor

Summary

--full --no-llm was generating a negative case that asked what the skill does by name. That is an explicit invocation, not a negative.

The default prompt is now an unrelated question, expected_skill stays null, and the behaviors talk about not applying this skill. Fixes #90.

Verification

  • I am familiar with the Contributing Guidelines
  • Added or updated focused tests
  • Updated documentation for user-visible changes
  • Ran make lint
  • Ran make test
  • Ran make build
  • Did not add credentials, private datasets, or proprietary benchmark content

Release Impact

  • Updated CHANGELOG.md

The default template asked what the skill does by name, which is an
explicit invocation, not a negative case. Use an unrelated prompt
instead, and keep expected_skill null.

Fixes NVIDIA#90

Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The focused generator suite passes (18 tests), and Ruff/diff checks are clean, but the deterministic negative remains invalid for a class of skills; reproduction is inline. CI also has a packaging smoke-test failure after build/install, in addition to the unrelated-history Gitleaks failure.

"question": hint_qs[3]
if len(hint_qs) > 3
else f"What does the {name} skill do and what are its capabilities?",
else "What's a good way to organize weekend errands in a new city?",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not hard-code a potentially on-skill negative

This prompt is itself a positive trigger for skills such as errand-planner, task organizers, and city assistants. For example, a skill described as “Organizes weekend errands efficiently in a new city” receives this exact question with expected_skill: null, recreating the self-contradictory dataset in another domain. The test only proves the skill name is absent, not that the question is unrelated. Please derive/validate a negative against the skill domain, require an author-provided negative when deterministic generation cannot do that safely, or omit the negative rather than label a plausible trigger off-skill.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that errand prompt is a positive for an errand-planner skill. I was only checking that the skill name was absent.

--no-llm now walks a few canned negatives and keeps the first one that does not share domain tokens with the skill name or description. If every candidate would be on-skill, the negative bucket is omitted instead of labeling a plausible trigger as off-skill. Added tests for the errand-planner case and the omit path.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for replacing the name-based negative. Token intersection still cannot establish that a candidate is off-skill. For example, with description has no shared domain token with the first candidate, so the generator labels organizing weekend errands as negative even though that is directly on-skill. Please use an explicitly authored negative, stronger semantic validation, or omit the negative when its domain cannot be established safely; token mismatch alone is not sufficient.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for replacing the name-based negative. Token intersection still cannot establish that a candidate is off-skill. For example, a day-planner described as planning grocery runs and appointments has no shared domain token with the first candidate, so the generator labels organizing weekend errands as negative even though that is directly on-skill. Please use an explicitly authored negative, stronger semantic validation, or omit the negative when its domain cannot be established safely; token mismatch alone is not sufficient.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped the errand-themed canned negative and only keep off-domain prompts now. Matching also checks skill name substrings and token stems (errand vs errands), and the negative bucket is omitted when no safe candidate survives. Author-provided hint questions still win when present.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed another round: errand canned negative removed, stem/name matching added, omit negative when nothing safe remains.

A hard-coded errand question is itself a positive for city/task skills.
Choose the first canned prompt that does not share domain tokens with the
skill name or description, and omit the negative when none is safe.

Fixes NVIDIA#90

Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
# Conflicts:
#	CHANGELOG.md
@mimran-khan

Copy link
Copy Markdown
Contributor Author

Merged main. The off-skill negative prompt fix is still in place. Ready for re-review.

Drop errand-themed canned negatives, match skill name substrings and token
stems before accepting a template negative, and omit the negative bucket when
no safe off-domain prompt exists.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: --no-llm negative eval case asks what the skill does

2 participants