skills/cuopt-skill-evolution: add eval - #1337
Merged
Merged
Conversation
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
ramakrishnap-nv
added
non-breaking
Collaborator
Author
|
/nvskills-ci |
📝 WalkthroughWalkthroughAdds a new eval case for cuopt-skill-evolution routing, updates the skill’s benchmark (verdict, findings, publication recommendation), edits skill-card metadata/output format, and refreshes the DSSE signature bundle. ChangesCuopt skill evolution edits
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/cuopt-skill-evolution/evals/evals.json`:
- Line 7: Update the eval description in the "ground_truth" text inside
evals.json to reflect the five-field "Skill update proposal" contract (Target,
Trigger, Scored, Removal, Diff) instead of four fields, and append the
CI/request instructions to ask for "/nvskills-ci" and to run "pre-commit run
--all-files --show-diff-on-failure"; locate the string in
skills/cuopt-skill-evolution/evals/evals.json (the "ground_truth" value) and the
SKILL.md "Skill update proposal" reference to ensure wording matches exactly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6393cc30-0804-4f23-8c3b-6c793e521037
📒 Files selected for processing (1)
skills/cuopt-skill-evolution/evals/evals.json
| "question": "I corrected your last answer because you suggested an API method that doesn't exist in cuopt-routing. You then found the right method. Should you do anything else?", | ||
| "expected_skill": "cuopt-skill-evolution", | ||
| "expected_script": null, | ||
| "ground_truth": "Yes. The user correction is a trigger for the cuopt-skill-evolution workflow. After solving the user's original task, the agent distills the generalizable learning, targets the single highest-impact skill (here cuopt-routing-api-python — the API skill where the missing method lives), and presents a proposal in the four-field format (Target, Trigger, Scored, Diff) for the user to approve before any change is applied.", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Extract the proposal format specification from SKILL.md
rg -A 10 'Proposal format' skills/cuopt-skill-evolution/SKILL.mdRepository: NVIDIA/cuopt
Length of output: 638
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== skills/cuopt-skill-evolution/SKILL.md: Proposal format section ==="
# Print a bit more context around the proposal format text
rg -n "Proposal format" skills/cuopt-skill-evolution/SKILL.md
sed -n '1,200p' skills/cuopt-skill-evolution/SKILL.md | head -n 220
echo
echo "=== skills/cuopt-skill-evolution/evals/evals.json: lines 1-40 ==="
nl -ba skills/cuopt-skill-evolution/evals/evals.json | sed -n '1,60p'
echo
echo "=== occurrences of 'four-field' and 'Removal' in evals.json ==="
rg -n "four-field|Removal|Diff" skills/cuopt-skill-evolution/evals/evals.json || trueRepository: NVIDIA/cuopt
Length of output: 14265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== skills/cuopt-skill-evolution/SKILL.md: lines 110-150 ==="
sed -n '110,150p' skills/cuopt-skill-evolution/SKILL.md
echo
echo "=== skills/cuopt-skill-evolution/evals/evals.json: lines 1-25 (with line numbers via python) ==="
python3 - <<'PY'
import itertools, json
path = "skills/cuopt-skill-evolution/evals/evals.json"
with open(path, "r", encoding="utf-8") as f:
lines = f.readlines()
for i, line in enumerate(lines[:25], start=1):
print(f"{i:>3}: {line.rstrip()}")
PY
echo
echo "=== occurrences in evals.json of key field terms ==="
rg -n "four-field|Removal|Diff" skills/cuopt-skill-evolution/evals/evals.json || trueRepository: NVIDIA/cuopt
Length of output: 4490
Fix eval’s “four-field format” wording to include Removal (per SKILL.md proposal contract)
skills/cuopt-skill-evolution/evals/evals.json describes a “four-field format (Target, Trigger, Scored, Diff)”, but skills/cuopt-skill-evolution/SKILL.md’s “Skill update proposal” block includes Removal: no|yes as well (Target/Trigger/Scored/Removal/Diff). Also request /nvskills-ci and ensure pre-commit run --all-files --show-diff-on-failure is run.
🔧 Proposed fix to include all five fields
- "ground_truth": "Yes. The user correction is a trigger for the cuopt-skill-evolution workflow. After solving the user's original task, the agent distills the generalizable learning, targets the single highest-impact skill (here cuopt-routing-api-python — the API skill where the missing method lives), and presents a proposal in the four-field format (Target, Trigger, Scored, Diff) for the user to approve before any change is applied.",
+ "ground_truth": "Yes. The user correction is a trigger for the cuopt-skill-evolution workflow. After solving the user's original task, the agent distills the generalizable learning, targets the single highest-impact skill (here cuopt-routing-api-python — the API skill where the missing method lives), and presents a proposal in the five-field format (Target, Trigger, Scored, Removal, Diff) for the user to approve before any change is applied.",
"expected_behavior": [
"Identifies the user correction as a cuopt-skill-evolution trigger and targets the cuopt-routing-api-python skill",
- "Presents a proposal in the four-field format (Target, Trigger, Scored, Diff) and does not apply the change without user approval",
+ "Presents a proposal in the five-field format (Target, Trigger, Scored, Removal, Diff) and does not apply the change without user approval",
"Does not propose modifying cuopt-skill-evolution itself (self-modify is forbidden)"
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "ground_truth": "Yes. The user correction is a trigger for the cuopt-skill-evolution workflow. After solving the user's original task, the agent distills the generalizable learning, targets the single highest-impact skill (here cuopt-routing-api-python — the API skill where the missing method lives), and presents a proposal in the four-field format (Target, Trigger, Scored, Diff) for the user to approve before any change is applied.", | |
| "ground_truth": "Yes. The user correction is a trigger for the cuopt-skill-evolution workflow. After solving the user's original task, the agent distills the generalizable learning, targets the single highest-impact skill (here cuopt-routing-api-python — the API skill where the missing method lives), and presents a proposal in the five-field format (Target, Trigger, Scored, Removal, Diff) for the user to approve before any change is applied.", | |
| "expected_behavior": [ | |
| "Identifies the user correction as a cuopt-skill-evolution trigger and targets the cuopt-routing-api-python skill", | |
| "Presents a proposal in the five-field format (Target, Trigger, Scored, Removal, Diff) and does not apply the change without user approval", | |
| "Does not propose modifying cuopt-skill-evolution itself (self-modify is forbidden)" | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/cuopt-skill-evolution/evals/evals.json` at line 7, Update the eval
description in the "ground_truth" text inside evals.json to reflect the
five-field "Skill update proposal" contract (Target, Trigger, Scored, Removal,
Diff) instead of four fields, and append the CI/request instructions to ask for
"/nvskills-ci" and to run "pre-commit run --all-files --show-diff-on-failure";
locate the string in skills/cuopt-skill-evolution/evals/evals.json (the
"ground_truth" value) and the SKILL.md "Skill update proposal" reference to
ensure wording matches exactly.
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/cuopt-skill-evolution/skill-card.md`:
- Line 29: The documentation is inconsistent: the "Risk: Review before
execution..." note in the skill-card.md (the Risk paragraph) indicates proposals
require explicit user approval, but "Other Properties Related to Output"
currently shows "[None]"; either reflect that approval is required by adding a
property like "proposals require explicit user approval before application"
under the Other Properties Related to Output section, or if approval is no
longer required, remove/update the Risk line to match the new workflow—edit the
"Other Properties Related to Output" heading or the "Risk: Review before
execution..." line so both state the same approval requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a85d1108-1ed2-4855-92e3-4e13936e2388
📒 Files selected for processing (3)
skills/cuopt-skill-evolution/BENCHMARK.mdskills/cuopt-skill-evolution/skill-card.mdskills/cuopt-skill-evolution/skill.oms.sig
|
|
||
| ## Evaluation Tasks: <br> | ||
| NVSkills-Eval `external` profile, 3-Tier evaluation. Overall verdict: PASS. Tier 1 static validation: 9 checks (passed with observations). Tier 2 deduplication: 2 checks (passed). <br> | ||
| **Other Properties Related to Output:** [None] <br> |
There was a problem hiding this comment.
Potential inconsistency between risk documentation and output properties.
Line 29 now states [None] for "Other Properties Related to Output," removing the previous documentation that "proposals require explicit user approval before application." However, line 18 still documents: "Risk: Review before execution as proposals could introduce incorrect or misleading guidance into skills."
If proposals from this skill require review/approval before execution (as line 18 suggests), this property should be documented in the output properties section. Changing it to [None] may mislead users about the approval workflow.
Please verify whether:
- The skill output no longer requires approval (in which case line 18 should also be updated), or
- Approval is still required (in which case line 29 should document this property)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/cuopt-skill-evolution/skill-card.md` at line 29, The documentation is
inconsistent: the "Risk: Review before execution..." note in the skill-card.md
(the Risk paragraph) indicates proposals require explicit user approval, but
"Other Properties Related to Output" currently shows "[None]"; either reflect
that approval is required by adding a property like "proposals require explicit
user approval before application" under the Other Properties Related to Output
section, or if approval is no longer required, remove/update the Risk line to
match the new workflow—edit the "Other Properties Related to Output" heading or
the "Risk: Review before execution..." line so both state the same approval
requirement.
3 tasks
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
2 participants
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.
Restore the eval that was deleted during the rename from skill-evolution to cuopt-skill-evolution (#1330). Updated expected_skill and references to the new name.