skills: prep for nvidia/skills catalog onboarding - #1310
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds NVIDIA PSIRT security vulnerability reporting guidance via SECURITY.md, configures CI/CODEOWNERS for that file, updates skill metadata descriptions, and reformats numeric array literals across documentation and example files to multi-line layouts without changing values or behavior. ChangesSecurity Policy Infrastructure
Documentation Descriptions and Example Formatting
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
🤖 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-install/SKILL.md`:
- Line 4: This PR touching skills/cuopt-install/SKILL.md is missing confirmation
that required skills-validation steps were executed; please run pre-commit hooks
and linting (pre-commit run --all-files and pre-commit run --all-files
--show-diff-on-failure), post the NVSkills CI trigger by commenting
"/nvskills-ci" on the PR, and ensure the original signature commit for the
skills change remains in the PR before merging so the repo policy for the
skills/ directory is satisfied.
In `@skills/cuopt-numerical-optimization-api-python/assets/portfolio/model.py`:
- Around line 18-20: Add a pytest that imports the portfolio.model module and
asserts the changed constants r1, r2, r3 have the expected values (0.12, 0.08,
0.05) so the change is covered by tests; place the test in the repository's
Python test suite following the existing pytest conventions and naming (e.g.,
test_*.py) and ensure it is discoverable by the CI test runner.
In `@skills/cuopt-numerical-optimization-api-python/references/qp_examples.md`:
- Around line 26-28: This PR changes content under skills/ (see qp_examples.md)
so before merging, post the comment `/nvskills-ci` on the pull request to
trigger NVSkills CI and verify the signature commit is still present in the PR
history; if the signature commit is missing, re-add it or restore the signed
commit before running the CI trigger.
🪄 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: 3f106cc4-eca5-48bc-83bb-c2a1bbcf4da8
📒 Files selected for processing (12)
SECURITY.mdskills/cuopt-install/SKILL.mdskills/cuopt-numerical-optimization-api-c/SKILL.mdskills/cuopt-numerical-optimization-api-c/references/examples.mdskills/cuopt-numerical-optimization-api-python/SKILL.mdskills/cuopt-numerical-optimization-api-python/assets/mps_solver/results.mdskills/cuopt-numerical-optimization-api-python/assets/portfolio/model.pyskills/cuopt-numerical-optimization-api-python/references/qp_examples.mdskills/cuopt-server-api-python/assets/lp_basic/client.pyskills/cuopt-server-api-python/assets/milp_basic/client.pyskills/numerical-optimization-formulation/SKILL.mdskills/skill-evolution/SKILL.md
| name: cuopt-install | ||
| version: "26.08.00" | ||
| description: Install cuOpt for Python, C, or as a server (pip, conda, Docker) — system requirements, install commands, and verification. Use when the user wants to install or verify cuOpt for any user-facing interface. For building cuOpt from source or contributing to cuOpt, see cuopt-developer. | ||
| description: Install cuOpt for Python, C, or server via pip, conda, or Docker; verify the install. For building cuOpt from source, see cuopt-developer. |
There was a problem hiding this comment.
Please confirm required skills-validation steps were executed before merge.
For this skills/ PR, please confirm you ran pre-commit checks, posted /nvskills-ci, and kept the signature commit in the PR as required by repo policy.
As per coding guidelines, "Install pre-commit hooks and run pre-commit run --all-files before committing code to ensure linting and formatting compliance", "Use pre-commit run --all-files --show-diff-on-failure to check code formatting and linting on all files before committing", and "For PRs changing content under skills/ directory, request NVSkills CI validation by commenting /nvskills-ci and ensure the signature commit remains in the PR".
🤖 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-install/SKILL.md` at line 4, This PR touching
skills/cuopt-install/SKILL.md is missing confirmation that required
skills-validation steps were executed; please run pre-commit hooks and linting
(pre-commit run --all-files and pre-commit run --all-files
--show-diff-on-failure), post the NVSkills CI trigger by commenting
"/nvskills-ci" on the PR, and ensure the original signature commit for the
skills change remains in the PR before merging so the repo policy for the
skills/ directory is satisfied.
| r1 = 0.12 | ||
| r2 = 0.08 | ||
| r3 = 0.05 |
There was a problem hiding this comment.
Add/point to pytest coverage for this Python code change.
This .py asset changed but no corresponding test update is included in this review set; please add (or explicitly reference existing) pytest coverage for this path.
As per coding guidelines, "**/*.{cpp,cc,cxx,c,h,hpp,py}: Add unit tests for code changes; refer to cpp/src/tests for C/C++ gtest examples and python/cuopt/cuopt/tests for Python pytest examples".
🤖 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-numerical-optimization-api-python/assets/portfolio/model.py`
around lines 18 - 20, Add a pytest that imports the portfolio.model module and
asserts the changed constants r1, r2, r3 have the expected values (0.12, 0.08,
0.05) so the change is covered by tests; place the test in the repository's
Python test suite following the existing pytest conventions and naming (e.g.,
test_*.py) and ensure it is discoverable by the CI test runner.
| r1 = 0.12 # 12% | ||
| r2 = 0.08 # 8% | ||
| r3 = 0.05 # 5% |
There was a problem hiding this comment.
Please trigger NVSkills CI validation for this skills PR.
Before merge, comment /nvskills-ci on the PR and confirm the signature commit is still present.
As per coding guidelines, "skills/**/*: For PRs changing content under skills/ directory, request NVSkills CI validation by commenting /nvskills-ci and ensure the signature commit remains in the PR".
🤖 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-numerical-optimization-api-python/references/qp_examples.md`
around lines 26 - 28, This PR changes content under skills/ (see qp_examples.md)
so before merging, post the comment `/nvskills-ci` on the pull request to
trigger NVSkills CI and verify the signature commit is still present in the PR
history; if the signature commit is missing, re-add it or restore the signed
commit before running the CI trigger.
…to dodge GPS-coord PII false-positives Last CI run on this branch (commit b531169) cleared all 4 AGENT_EVAL HIGHs from the eval simplification, but a single HIGH still gated: the PII detector flagged 9 MEDIUM "GPS coordinates" findings on inline numeric arrays in C example code, which the gate aggregates into one HIGH. Files / lines previously flagged: * SKILL.md:33 — cuopt_float_t values[] = {2.0, 3.0, 4.0, 2.0}; * references/examples.md:49 — cuopt_float_t values[] = {3.0, 4.0, 2.7, 10.1}; * references/examples.md:52 — cuopt_float_t objective_coefficients[] = {-0.2, 0.1}; * references/examples.md:55 — cuopt_float_t constraint_upper_bounds[] = {5.4, 4.9}; * references/examples.md:59 — cuopt_float_t var_lower_bounds[] = {0.0, 0.0}; * references/examples.md:143, 145, 146, 148 — same in the MILP example (values, objective_coefficients, constraint_upper, var_lower). The detector regex matches the inline-array shape "{N.N, N.N, ...};" as a GPS coordinate pair. Reformatting the arrays multi-line breaks that shape — one value per line — without changing C semantics. Identical to the fix applied to other numerical-optimization assets on PR #1310 (skills/onboarding-prep-securitymd-pii-descs). Ported here directly because PR #1310 will not merge before this PR needs to clear CI. No content change — only whitespace/formatting on the array literals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
ramakrishnap-nv
added
non-breaking
|
Actionable comments posted: 0 |
698e020 to
be52a41
Compare
|
/nvskills-ci |
|
Actionable comments posted: 0 |
jameslamb
left a comment
jameslamb
left a comment
There was a problem hiding this comment.
Looks great, thanks for adopting this SECURITY.md.
| "upper_bounds": [ | ||
| 240.0, | ||
| 200.0, | ||
| ], |
There was a problem hiding this comment.
Out of curiosity, where did all these style-only changes come from? I don't see any linter configs getting changed in this PR.
I recommend making sure these files are covered by pre-commit, so you don't have contributors' IDEs flipping the style of lines like this.
There was a problem hiding this comment.
@jameslamb I have raised with internal team on the static analysis on these, so analysis flag these as coordinates and raises PII issue, even though this is just a numerical array.
|
Actionable comments posted: 0 |
|
/nvskills-ci |
Aligns with the NVIDIA Agent Skills Publishing Onboarding Guide: * Add top-level SECURITY.md so the catalog's "Getting Help & Contributing" links resolve once this repo is registered in components.d/<slug>.yml (guide §"Recommended Repository Layout"). * Wire .github/CODEOWNERS and .github/workflows/pr.yaml so doc-only edits to SECURITY.md don't trigger the full CI matrix (cpp-build, python-build, docs-build, conda-build path-filter blocks). * Shorten cuopt-skill-evolution SKILL.md description to fit the 50-150 char range the NV-BASE quality scorer recommends. The previous CI run reported "Description very long" (guide §"Writing Skills for Good Agent Triggering"). Per-skill description and PII-array changes for the other skills moved to dedicated follow-up PRs (#1343, #1344, #1345, #1346) so each PR's CI scope is limited to one skill. No behavior change; documentation/metadata only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
c772a1e to
2ee9594
Compare
|
/nvskills-ci |
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
4 participants
Aligns this repo's skills with the NVIDIA Agent Skills Publishing Onboarding Guide.
SECURITY.mdso the catalog's Getting Help & Contributing links resolve once cuopt is registered incomponents.d/.