chore(ci): exclude NVSkills-generated files from trailing-whitespace hook by ramakrishnap-nv · Pull Request #1465 · NVIDIA/cuopt

ramakrishnap-nv · 2026-06-25T19:39:22Z

NVSkills-Eval regenerates skill-card.md and BENCHMARK.md with trailing whitespace after SKILL.md changes; exclude them alongside skill.oms.sig.\n\nRelated: #1464

copy-pr-bot · 2026-06-25T19:39:25Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

NVSkills-Eval regenerates skill-card.md with trailing whitespace after SKILL.md changes; exclude it alongside skill.oms.sig. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

coderabbitai · 2026-06-25T19:40:55Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7f51f8cb-e1de-4ca0-a089-0f581bca3310

📥 Commits

Reviewing files that changed from the base of the PR and between ff7d038 and 626ec5a.

📒 Files selected for processing (1)

.pre-commit-config.yaml

📝 Walkthrough

Walkthrough

The pre-commit config adds a top-level exclude block for several skills/ markdown files and removes one skill-specific pattern from two hook exclusions.

Changes

Pre-commit skill file exclusions

Layer / File(s)	Summary
Update skill file exclusions `.pre-commit-config.yaml`	The top-level `exclude` block now matches `skills/./skill.oms.sig`, `skills/./skill-card.md`, and `skills/./BENCHMARK.md`, and the `end-of-file-fixer` and `trailing-whitespace` hook excludes drop the `skills/./skill.oms.sig` pattern.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

NVIDIA/cuopt#1453: Updates pre-commit exclude patterns for the same skills/** file types.
NVIDIA/cuopt#1460: Changes the same .pre-commit-config.yaml exclusion rules alongside edits to matching skills/ files.

Suggested labels

Agentic

Suggested reviewers

tmckayus

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main change: excluding NVSkills-generated files from trailing-whitespace handling.
Description check	✅ Passed	The description accurately describes the generated files being excluded and the whitespace issue being addressed.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/skill-card-trailing-whitespace

_{Comment @coderabbitai help to get the list of available commands.}

ramakrishnap-nv · 2026-06-25T19:45:14Z

/merge

jameslamb · 2026-06-25T19:50:29Z

        exclude: ^(datasets|helmchart)/.*\.(mps|json|yaml|yml|txt)$|^skills/.*/skill\.oms\.sig$
      - id: trailing-whitespace
-        exclude: ^datasets/.*\.(mps|json|yaml|yml|txt)$|^skills/.*/skill\.oms\.sig$
+        exclude: ^datasets/.*\.(mps|json|yaml|yml|txt)$|^skills/.*/skill\.oms\.sig$|^skills/.*/skill-card\.md$|^skills/.*/BENCHMARK\.md$


Why do this? Can't the process that modifies these files be modified to also not have any trailing whitespace?

This is generated or added by internal CI projects and it is controlled by nvcarps-team. So, it would take time to push them to update this.

In our offline discussion, @ramakrishnap-nv said this has to be excluded because some internal process produces it alongside a signature (content-based checksum), and we don't have much control over that process.

If that's the case, that these files should never be modified, then they should be added to the top-level exclude: in this file, not just this one specific hook.

Pushed an update

Update: Agree regarding the exclusion, I have updated it, @jameslamb for viz

Ignore:
Top-level exclude: would skip these files from all hooks including validate-skills and verify-copyright, which we do want to run on SKILL.md changes (and the generated files alongside it). Hook-level exclusion keeps the other checks active. Applied @bdice's formatting suggestion.

Updated — moved all three NVSkills-generated files (skill.oms.sig, skill-card.md, BENCHMARK.md) to the top-level exclude: and removed the per-hook patterns.

jameslamb

Left a question, this PR is confusing to me.

…hook NVSkills-Eval regenerates skill-card.md and BENCHMARK.md with trailing whitespace after SKILL.md changes; exclude them alongside skill.oms.sig. Use verbose (?x) regex format for readability. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

…xclude skill.oms.sig, skill-card.md, and BENCHMARK.md are produced by an internal NVSkills-Eval service alongside a content-based signature; exclude them from all hooks at the top level rather than per-hook. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

jameslamb · 2026-06-25T20:16:12Z

+    skills/.*/skill\.oms\.sig|
+    skills/.*/skill-card\.md|
+    skills/.*/BENCHMARK\.md
+  )$


Should this be skills/.*? Wouldn't SKILL.md (for example) changing also change the signature?

Yes, in that case we need to run CI which would generate new files and signature associated with it and push as commit, for reference #1460 (comment)

But we want to follow guidelines on all other things that we write and control like SKILL.md or any other underlying references, only want to skip which are generated.

jameslamb

Going to say unhelpfully that this seems annoying to have to manage, but the changes are fine based on my understanding of how the process is working. Thanks for answering my questions.

ramakrishnap-nv marked this pull request as ready for review June 25, 2026 19:40

ramakrishnap-nv requested a review from a team as a code owner June 25, 2026 19:40

ramakrishnap-nv requested a review from jakirkham June 25, 2026 19:40

ramakrishnap-nv force-pushed the fix/skill-card-trailing-whitespace branch from 3388c67 to bfc3faf Compare June 25, 2026 19:40

ramakrishnap-nv changed the title ~~chore(ci): exclude skill-card.md from trailing-whitespace hook~~ chore(ci): exclude NVSkills-generated files from trailing-whitespace hook Jun 25, 2026

ramakrishnap-nv self-assigned this Jun 25, 2026

ramakrishnap-nv added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Jun 25, 2026

ramakrishnap-nv requested review from tmckayus and removed request for jakirkham June 25, 2026 19:48

jameslamb reviewed Jun 25, 2026

View reviewed changes

ramakrishnap-nv requested review from bdice and jameslamb June 25, 2026 20:00

jameslamb reviewed Jun 25, 2026

View reviewed changes

jameslamb approved these changes Jun 25, 2026

View reviewed changes

rapids-bot Bot merged commit 229c0d8 into main Jun 25, 2026
24 checks passed

jameslamb deleted the fix/skill-card-trailing-whitespace branch June 25, 2026 21:10

Uh oh!

Conversation

ramakrishnap-nv commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot Bot commented Jun 25, 2026

Uh oh!

coderabbitai Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

ramakrishnap-nv commented Jun 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jameslamb Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ramakrishnap-nv Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jameslamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jameslamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ramakrishnap-nv commented Jun 25, 2026 •

edited

Loading

coderabbitai Bot commented Jun 25, 2026 •

edited

Loading

jameslamb Jun 25, 2026 •

edited

Loading

ramakrishnap-nv Jun 25, 2026 •

edited

Loading