ci(licensing): support license diff for PRs#72
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
github-actions
Bot
added
size:XXL
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⛔ Files ignored due to path filters (1)
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR implements npm workspace support for the NeMo-Flow repository and adds a license-diff CI check. It establishes a root package.json with workspace definitions, refactors build/test infrastructure to use workspace-scoped commands, introduces a new license-diff workflow that compares dependency snapshots across languages, improves license attribution generation with better normalization, and updates all developer documentation to reflect the new command patterns. Changesnpm Workspaces & License Tracking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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 |
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/ci-path-filters.yml:
- Around line 18-27: The dependencies filter group currently only lists
manifest/lock files and misses the licensing scripts, so add the path pattern
for the licensing scripts to this group; update the dependencies array (the
block containing 'Cargo.lock', 'package.json', etc.) to include
'scripts/licensing/**' so changes to files like
scripts/licensing/license_diff.py or
scripts/licensing/attributions_lockfile_md.py will trigger the ci_license_diff
job.
In @.github/workflows/ci_license_diff.yml:
- Around line 55-56: Remove the "continue-on-error: true" setting from the
ci_license_diff job so failures are not suppressed; locate the ci_license_diff
job block in the workflow and delete the continue-on-error: true line (or set it
to false) to ensure step and tool failures are visible in the workflow UI.
In `@scripts/licensing/attributions_lockfile_md.py`:
- Around line 849-858: The npm install invocation in _node_license_checker_data
currently runs subprocess.run(["npm", "ci"], ...) which may execute lifecycle
scripts; update that call to include the "--ignore-scripts" flag (i.e.,
subprocess.run(["npm", "ci", "--ignore-scripts"], ...)) so the license scan
remains read-only and prevents any package scripts from running during the CI
license check.
In `@scripts/licensing/license_diff.py`:
- Around line 215-219: The current comprehension silently drops any payload
entries whose value isn't a list; instead validate and fail fast: iterate over
payload.items(), and for each (language, rows) if not isinstance(rows, list)
raise a ValueError (including the language key and actual type) so malformed
inventory JSON surfaces immediately; then perform the existing cast to
LicenseInventoryEntry for each row and return the dict mapping str(language) to
the list of casted dicts (use the existing payload and LicenseInventoryEntry
symbols to locate the code).
- Around line 233-235: The subprocess.run invocation that executes git worktree
add (the call in license_diff.py that passes ["git", "-C", str(root),
"worktree", "add", "--detach", "--quiet", str(worktree), ref]) should include
the "--" separator before the ref to avoid treating refs that start with "-" as
options; update the arguments list in that subprocess.run call to insert "--"
immediately before ref (using the existing variables root, worktree, ref) so the
command becomes ... "worktree", "add", ..., str(worktree), "--", ref.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: fada1aa9-c6a1-404b-8577-d75bc4372269
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
.agents/skills/maintain-packaging/SKILL.md.agents/skills/test-node-binding/SKILL.md.agents/skills/test-wasm-binding/SKILL.md.agents/skills/update-project-version/SKILL.md.agents/skills/validate-change/SKILL.md.github/ci-path-filters.yml.github/workflows/ci.yaml.github/workflows/ci_changes.yml.github/workflows/ci_license_diff.yml.pre-commit-config.yamlAGENTS.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.mdATTRIBUTIONS-Rust.mdabout.tomlcrates/core/Cargo.tomlcrates/wasm/README.mdcrates/wasm/package.jsondocs/conf.pydocs/contribute/testing-and-docs.mddocs/getting-started/installation.mddocs/getting-started/nodejs.mddocs/getting-started/prerequisites.mddocs/resources/support-and-faqs.mddocs/troubleshooting/troubleshooting-guide.mdjustfilepackage.jsonscripts/README.mdscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.py
💤 Files with no reviewable changes (2)
- docs/getting-started/prerequisites.md
- AGENTS.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (39)
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst,txt}: SpellNVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as 'here' or 'read more.'
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not add trademark symbols to NeMo Flow learning documentation by default.
Do not rewrite API names, package names, command flags, or code literals for style reasons.
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.md
**/*.{md,markdown,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,markdown,rst}: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Use monospace formatting for code elements, commands, parameters, package names, and expressions
Use monospace formatting for directories, file names, and paths
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Use quotation marks for error messages and strings in documentation
Use bold formatting for UI buttons, menus, fields, and labels in documentation
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use in documentation
Use italics for publication titles in documentation
Use plain text formatting for keyboard shortcuts in documentation
Prefer[NVIDIA/NeMo](link)format for GitHub repository references over generic phrases like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text in documentation
Avoid generic link anchors such as 'here,' 'this page,' and 'read more' in documentation
Include the acronym in link text if a linked term includes an acronym
Do not link long sentences or multiple sentences in documentation
Avoid links that pull readers away from a procedure unles...
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license headers in HTML and Markdown files using HTML comment syntax
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.md
docs/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
./scripts/build-docs.shfor documentation site changes
docs/**/*.md: Relevant getting-started or reference docs must be updated when examples change
Release-policy docs must point to GitHub Releases as the only release-history source of truth
docs/**/*.md: Use title case for headings in technical documentation
Introduce code blocks, tables, and lists with complete lead-in sentences in documentationKeep primary documentation focused on Rust, Python, and Node.js. Treat Go, WebAssembly, and raw FFI as experimental and source-first unless binding-support guidance changes.
Files:
docs/getting-started/nodejs.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mddocs/getting-started/installation.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run Markdown link checking via
lycheeforREADME.md,CONTRIBUTING.md, anddocs/through pre-commit hooks
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.md
**/*.{md,markdown,py,sh,bash,js,ts,java,cpp,go,rust}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current in documentation
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mddocs/conf.pyscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.pyATTRIBUTIONS-Python.md
{RELEASING.md,CHANGELOG.md,docs/**/*.md}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep release-process and release-notes guidance in repo-maintainer docs such as RELEASING.md, not as user-facing docs pages or CHANGELOG.md
Files:
docs/getting-started/nodejs.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mddocs/getting-started/installation.md
**/*.{md,markdown,py,sh,bash}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep stable user-facing wrappers at scripts/ root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mddocs/conf.pyscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.pyATTRIBUTIONS-Python.md
**/*.{md,markdown,py,sh,bash,js,ts,example}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Example commands must match current package names and paths
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mddocs/conf.pyscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.pyATTRIBUTIONS-Python.md
{scripts/*.sh,docs/**/*.md}
📄 CodeRabbit inference engine (.agents/skills/contribute-integration/SKILL.md)
Use root
./scripts/*.shcommands in docs and contributor guidance as documented, with implementations underscripts/third-party/
Files:
docs/getting-started/nodejs.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mddocs/getting-started/installation.md
{docs/**,examples/**,crates/adaptive/**,python/nemo_flow/**,go/nemo_flow/**,**/{example,component}.{ts,tsx,js,rs,py,go}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Any new adaptive component kind must have documentation, examples, and binding coverage across all supported languages
Files:
docs/getting-started/nodejs.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mddocs/getting-started/installation.mddocs/conf.py
{README*,CHANGELOG*,docs/**/*.{md,rst,txt},examples/**/*,*.md}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update documentation, examples, and getting-started guides with new package/module/crate names after rename operations
Files:
docs/getting-started/nodejs.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.md
**/*.{md,txt,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,txt,rst}: Ensure commands, package names, file paths, and APIs in documentation are correct and not stale; flag incorrect or outdated information as blocking issues
Ensure examples and procedures in documentation will execute successfully with current APIs and commands
Use consistent user-facing terminology throughout documentation that matches current repo terminology
Capitalize NVIDIA correctly in all documentation and public-facing text
Format code, commands, paths, and filenames as inline code (monospace) in documentation
Use descriptive anchor text for links instead of bare URLs or weak labels like 'here' in documentation
Prefer active voice, present tense, short sentences, and plain English in documentation
Structure documentation procedures as imperative steps that are easy to scan and not too long for a single sequence
Prefer 'after' instead of 'once' for temporal references in documentation
Use 'can' instead of 'may' when describing possibility (rather than permission) in documentation
Avoid ambiguous numeric dates and ordinal dates in documentation body text
Files:
docs/getting-started/nodejs.mdscripts/README.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mdATTRIBUTIONS-Python.md
**/*.{rs,py,ts,tsx,js,jsx,go,sh,yml,yaml,toml,md,txt,json,config}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
docs/getting-started/nodejs.mdabout.tomlcrates/wasm/package.jsonscripts/README.mdpackage.jsondocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mdcrates/core/Cargo.tomlcrates/wasm/README.mddocs/getting-started/installation.mdATTRIBUTIONS-Node.mddocs/conf.pyscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.pyATTRIBUTIONS-Python.md
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/getting-started/nodejs.mddocs/resources/support-and-faqs.mddocs/contribute/testing-and-docs.mddocs/troubleshooting/troubleshooting-guide.mddocs/getting-started/installation.mddocs/conf.py
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license headers in TOML files using TOML comment syntax
Files:
about.tomlcrates/core/Cargo.toml
crates/wasm/package.json
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Do not treat
crates/wasm/package.jsonas the publishable package manifest unless it gains an explicitversionfield
Files:
crates/wasm/package.json
crates/wasm/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For WebAssembly binding changes, use
test-wasm-binding
Files:
crates/wasm/package.jsoncrates/wasm/README.md
crates/{python,ffi,node,wasm}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node,wasm}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/wasm/package.jsoncrates/wasm/README.md
**/README.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant crate or package README when that surface changed
Relevant package or crate README.md files must be updated when examples or binding guidance changes
Files:
scripts/README.mdcrates/wasm/README.md
{.github/**/*.{yml,yaml},*.patch,scripts/**/*,*.sh,*.bat,Dockerfile*}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update CI configuration, patch files, and build scripts with new functional identifiers after rename operations
Files:
scripts/README.md.github/ci-path-filters.yml.github/workflows/ci_license_diff.yml.github/workflows/ci_changes.yml.github/workflows/ci.yamlscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.py
{README.md,docs/index.md,**/README.md}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Update entry-point documentation (README.md, docs/index.md, package READMEs, binding-level source READMEs) whenever public behavior changes
Files:
scripts/README.mdcrates/wasm/README.md
**/{README.md,RELEASING.md,*.example.{js,ts,py,go},**/examples/**/*}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Update documentation and examples to reflect current install/import/build commands
Files:
scripts/README.mdcrates/wasm/README.md
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}
⚙️ CodeRabbit configuration file
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.
Files:
scripts/README.md.github/ci-path-filters.yml.github/workflows/ci_license_diff.yml.github/workflows/ci_changes.yml.pre-commit-config.yaml.github/workflows/ci.yamlscripts/licensing/attributions_lockfile_md.pyjustfilescripts/licensing/license_diff.py
{Cargo.toml,setup.py,setup.cfg,pyproject.toml,go.mod,go.sum,package.json,package-lock.json,yarn.lock,pom.xml,*.gradle}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update repository manifest files (Cargo.toml, setup.py, go.mod, package.json, etc.) with new package/crate names during rename operations
Files:
package.json
package.json
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistent Node workspace metadata in root
package.jsonandpackage-lock.json
Files:
package.json
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-ci/SKILL.md)
.github/workflows/**/*.{yml,yaml}: Putpermissions:on each job that needs token access in GitHub Actions workflows
Avoid workflow-level permissions unless the repository intentionally centralizes them and the inheritance tradeoff is documented
Keep third-party actions pinned to full commit SHAs and preserve the readable version comment after the SHA
Prefer action-native or ecosystem-native caching over genericactions/cachein GitHub Actions workflows
Use lockfiles or dependency manifests to drive cache invalidation in GitHub Actions workflows
Keep deploy and publish permissions isolated to the jobs that need them
Read both caller and callee when a workflow usesworkflow_callin GitHub Actions
Put release-tag validation in the earliest practical caller job when the pipeline has tag-based publish behavior
Keep release-tag policy aligned withRELEASING.md: raw SemVer tags only, no leadingv
contents: readis the default minimum for checkout-based build, test, docs, and packaging jobs
pull-requests: readis required for PR metadata lookup jobs in GitHub Actions workflows
pages: writeandid-token: writeshould be limited to Pages deployment jobs and any caller that invokes them through a reusable workflow
For reusable workflows, the caller must grant every permission the called jobs require and the callee cannot elevate beyond what the caller provides
Preferastral-sh/setup-uvcache support withcache-dependency-globanchored touv.lockfor Python dependency caching
PreferSwatinem/rust-cachewith explicitshared-keyandworkspacesinstead of ad hoc target-directory caching
Avoid caching generated outputs that can hide stale behavior unless the repo already relies on them deliberately
Ensure each job has the minimum permissions it needs during GitHub Actions CI review
Ensure reusable workflow callers grant only the scopes their callees require
Ensure every external action is pinned to a full SHA in GitHub Actions workflows
Ensure cache ...
Files:
.github/workflows/ci_license_diff.yml.github/workflows/ci_changes.yml.github/workflows/ci.yaml
{.github/workflows/**/*.{yml,yaml},**/.gitlab-ci.yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure CI workflows reference the same package names as local workflows in CI configuration files
Files:
.github/workflows/ci_license_diff.yml.github/workflows/ci_changes.yml.github/workflows/ci.yaml
**/Cargo.{toml,lock}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
cargo deny checkfor Rust dependency auditing as configured indeny.toml
Files:
crates/core/Cargo.toml
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistent Rust package names and workspace metadata in
Cargo.tomlMaintain
Cargo.toml[workspace.package].versionas the source of truth for the Rust workspace and Python build versioningKeep
Cargo.toml[workspace.dependencies]self-references aligned when the workspace version changes
Files:
crates/core/Cargo.toml
{crates/core/**,crates/adaptive/**,crates/node/src/**}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
When the change touched
crates/core,crates/adaptive, or the generated Rust binding layer undercrates/node/src, also usevalidate-changeskill
Files:
crates/core/Cargo.toml
**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
Format changed Python wrapper and test files with
uv run ruff format python
**/*.py: Use Ruff with rule sets E, F, W, I for Python linting
Use Ruff formatter for Python code with line length 120 and double quotes
Usetyfor Python type checking
Use snake_case naming convention for Python code
Include SPDX license headers in all Python source files using Python comment syntaxIf a language surface changed, always run that language's test target even when Rust core did not change
For Python-only wrapper or binding changes, use
test-python-bindingPre-commit hooks run Ruff formatting on selected Python files and trigger
py check . ...for the Python project
Files:
docs/conf.pyscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.py
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
SONAR_IGNORE_START/SONAR_IGNORE_ENDmarkers only for documented false positives that cannot be resolved in code; keep ignored blocks small, add explanatory comments, and require reviewer sign-offFollow binding naming conventions: Rust and Python
snake_case, C FFI exports prefixednemo_flow_, GoPascalCasefor public APIs, Node.jscamelCase.
Files:
docs/conf.pyscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.py
**/*.{js,ts,tsx,jsx,py,rs,go,java,c,cpp,h,cc,cxx,cs,rb,php,swift,kt}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changed files must be formatted with the language-native formatter
Files:
docs/conf.pyscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.py
**/*.{sh,py}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For third-party integration or patch changes, run patch validation with
./scripts/apply-patches.sh --checkand the relevant integration tests, keeping the root./scripts/*.shwrappers for third-party flows
Files:
docs/conf.pyscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.py
justfile
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When updating helper code, keep
set_project_version,set_cargo_workspace_version, andset_node_package_versionhelper functions aligned with version source filesUse
set_npm_package_versionas the reusable npm JSON helper for Node and WebAssembly packaging recipes
Files:
justfile
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:29.376Z
Learning: Maintain consistent WebAssembly package naming and ensure generated packages land where downstream consumers expect
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:29.376Z
Learning: Maintain consistent FFI header and library naming
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:29.376Z
Learning: Release tags must use raw SemVer without a leading `v`
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:29.376Z
Learning: Release history and release notes must point to GitHub Releases, not `CHANGELOG.md` or docs pages
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:29.376Z
Learning: Verify that package names, import paths, and module names are internally consistent across all packaging surfaces
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:36.518Z
Learning: Use this skill when the change is primarily in `crates/node`, the generated Node surface, or Node-facing examples/docs
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:36.518Z
Learning: Use `karpathy-guidelines` alongside this skill for implementation or review work, keeping changes scoped and surfacing assumptions
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:43.178Z
Learning: Run the WebAssembly tests with `just test-wasm`
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:43.178Z
Learning: Use `just build-wasm` for explicit packaging/build pass
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:43.178Z
Learning: Use `just ci=true test-wasm` when coverage reports are needed
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:43.178Z
Learning: If the issue is only packaging or generated wrapper output, prioritize the build plus package-prep steps before the full test sweep
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:43.178Z
Learning: Use `karpathy-guidelines` alongside this skill for implementation or review work
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:43.178Z
Learning: Keep changes scoped, surface assumptions, and define focused validation before editing
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Run `cargo check --workspace` to refresh `Cargo.lock` after workspace package entries change
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Regenerate `ATTRIBUTIONS-Rust.md` with `./scripts/generate_attributions.sh rust` when Cargo metadata changes and committed attribution files must stay fresh
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Regenerate `ATTRIBUTIONS-Node.md` with `./scripts/generate_attributions.sh node` when `package-lock.json` changes
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Rebuild the generated WebAssembly package with `just build-wasm` or `NEMO_FLOW_WASM_RELEASE=1 npm run build:pkg --workspace=nemo-flow-wasm` and inspect `crates/wasm/pkg/package.json` when the change needs WebAssembly publish validation
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Audit remaining references to the old version with targeted search, separating true version pins from examples, generated attribution files, and unrelated third-party versions
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Do not update only `Cargo.toml` or only Node package metadata—ensure all version-related files are kept in sync
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Do not assume `crates/wasm/package.json` is the published npm manifest
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Do not forget to update `Cargo.lock`, `ATTRIBUTIONS-Rust.md`, or `ATTRIBUTIONS-Node.md` after changing versioned inputs that feed them
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:00:55.730Z
Learning: Avoid blind repository-wide search/replace across docs, patches, and generated attribution files when updating versions
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:12.323Z
Learning: Format changed files with the language-native formatter before the final lint/test pass
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:12.323Z
Learning: If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:12.323Z
Learning: During iteration, prefer `uv run pre-commit run --files <changed files...>`; before review or handoff, run `uv run pre-commit run --all-files`
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:12.323Z
Learning: For FFI surface changes, use `test-ffi-surface`
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Do not hand-edit generated or packaged outputs unless the repository workflow expects them to be checked in. Regenerate through the documented recipe or script.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Preserve the shared runtime model across bindings. Do not add behavior to one primary binding without considering Rust, Python, and Node.js parity.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Prefer documented public APIs and stable wrapper commands. Do not rely on internal helpers in examples or user-facing docs.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Prefer the repository `just` recipes over raw tool commands. Use raw `cargo`, `pytest`, `go test`, `npm`, or `wasm-pack` commands only for focused debugging or targeted single-test reruns that do not have a `just` recipe.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Run tests for every language affected by a change. If you touch the Rust core runtime, middleware semantics, event shape, scope behavior, typed codecs, plugins, or observability, expect to validate every affected binding because the bindings share the same runtime contract.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Use branch prefixes from the contributor docs: `feat/`, `fix/`, `docs/`, `test/`, or `refactor/`.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Use signed-off commits for PR work: `git commit -s`.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Before creating, opening, publishing, or editing a pull request, read `.github/pull_request_template.md` and use it as the PR body skeleton. Preserve its visible headings, checklist items, and related-issue guidance; fill the sections instead of replacing them with a generic summary.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: PR descriptions should include what changed, why, how it was tested, and any breaking changes within the repository template format.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Update `README.md`, `docs/`, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Keep release-process details in maintainer docs such as `RELEASING.md`. Do not move release-history policy into user-facing docs or `CHANGELOG.md`.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: Keep stable public wrappers at the `scripts/` root in docs and examples. Reference namespaced helper paths only when documenting internal maintenance work.
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:01:31.721Z
Learning: If repo-local PR guidance such as the `prepare-pr` skill conflicts with generic GitHub connector or plugin guidance, follow the repo-local PR guidance for PR body format and review handoff details.
📚 Learning: 2026-05-03T04:23:07.497Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Flow PR: 46
File: .github/workflows/ci_rust.yml:31-64
Timestamp: 2026-05-03T04:23:07.497Z
Learning: In GitHub Actions workflow YAML, it’s valid to conditionally disable a service container by setting the service container’s `image` to an empty string (`''`) via a matrix variable (e.g., `redis_service_image: ''`). This intentionally makes the runner skip service initialization for that matrix entry rather than failing the job. When reviewing workflows, don’t flag this as an error if the workflow uses an empty `image` to disable the service on specific matrix entries (e.g., OS-specific setups); verify the `image` is sourced from the matrix variable and that the service is only expected to be available when a non-empty image is provided.
Applied to files:
.github/workflows/ci_license_diff.yml.github/workflows/ci_changes.yml
📚 Learning: 2026-04-15T18:16:52.951Z
Learnt from: bbednarski9
Repo: NVIDIA/NeMo-Flow PR: 1
File: docs/atof-event-format.md:381-381
Timestamp: 2026-04-15T18:16:52.951Z
Learning: In docs/atof-event-format.md (and when reviewing references to this format across the NeMo-Flow repo), treat `AtifExporter.events_to_steps()` as the intended public/API method name. Do not flag it as inconsistent with internal Rust symbol names (e.g., `event_to_steps` in `crates/core/src/atif.rs`)—the documentation’s public-facing naming may differ intentionally from internal implementation details.
Applied to files:
.pre-commit-config.yaml
🪛 LanguageTool
.agents/skills/validate-change/SKILL.md
[style] ~144-~144: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ny.tomltriggerscargo deny check. - Matching Cargo.lock, uv.lock, or package-lo...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
ATTRIBUTIONS-Node.md
[warning] 99-99: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1373-1373: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1376-1376: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 1376-1376: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1377-1377: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1377-1377: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 2416-2416: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2419-2419: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 2419-2419: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2420-2420: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 2420-2420: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.15.12)
scripts/licensing/attributions_lockfile_md.py
[warning] 911-911: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
scripts/licensing/license_diff.py
[warning] 76-76: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 167-167: Use list.extend to create a transformed list
(PERF401)
[warning] 214-214: Prefer TypeError exception for invalid type
(TRY004)
[warning] 214-214: Avoid specifying long messages outside the exception class
(TRY003)
[error] 233-233: subprocess call: check for execution of untrusted input
(S603)
[error] 234-234: Starting a process with a partial executable path
(S607)
[error] 242-242: subprocess call: check for execution of untrusted input
(S603)
[error] 243-243: Starting a process with a partial executable path
(S607)
🔇 Additional comments (24)
docs/conf.py (1)
338-340: ⚡ Quick winThe error message and path check are properly aligned for the npm workspace setup. Running
npm install --ignore-scriptsfrom the repository root createsnode_modulesin all workspace directories, includingcrates/node/node_moduleswhich_require_node_modules()checks. The instruction is correct.ATTRIBUTIONS-Python.md (1)
98-98: License Label Normalization Looks ConsistentThese updates consistently normalize MIT-family labels to
MITacross changed package entries, which aligns with the license-diff stability goal and does not alter underlying attribution text.Also applies to: 920-920, 960-960, 1783-1783, 1911-1911, 2019-2019, 2089-2089, 2404-2404, 2434-2434, 2489-2489, 2633-2633, 2661-2661, 2844-2844, 3001-3001, 3433-3433, 3460-3460, 4746-4746, 4780-4780, 4844-4844, 5767-5767, 5866-5866
docs/resources/support-and-faqs.md (1)
520-520: Workspace-scoped Node test command looks correctThis update keeps the troubleshooting/test guidance aligned with root workspace usage.
.agents/skills/maintain-packaging/SKILL.md (1)
25-25: Good packaging-surface coverage updateIncluding root Node workspace metadata in audit areas and references is the right move for this repo layout.
Also applies to: 46-47
docs/contribute/testing-and-docs.md (1)
69-69: Node validation command update is correctThe workspace-scoped test command is consistent with the current repository workflow.
crates/wasm/README.md (1)
60-64: WASM local-dev command migration looks goodUsing root-level workspace-scoped commands keeps this README aligned with the workspace model.
docs/getting-started/nodejs.md (1)
23-23: Root workspace build command is alignedThis is consistent with the repo’s workspace-based Node setup.
.agents/skills/test-node-binding/SKILL.md (1)
22-22: Node skill commands and references are updated correctlyThe workspace-scoped command set and root manifest references match the current packaging model.
Also applies to: 39-39, 57-57, 68-69
docs/troubleshooting/troubleshooting-guide.md (1)
39-39: Troubleshooting command update is consistent with workspace flowThe revised steps correctly target the root workspace and Node binding test path.
Also applies to: 43-43
.agents/skills/test-wasm-binding/SKILL.md (1)
23-23: Workspace-scoped formatting command update looks correct.This aligns the WebAssembly skill with root workspace execution and keeps wrapper/test/script formatting paths explicit.
Also applies to: 39-39
docs/getting-started/installation.md (1)
86-90: Node.js source/setup instructions are now consistent with workspace builds.The shift to
npm run build --workspace=nemo-flow-nodematches the new root workspace command model.Also applies to: 143-145
.agents/skills/validate-change/SKILL.md (1)
97-103: Validation matrix command updates are coherent.Both Node.js and WebAssembly formatter paths now correctly use workspace-scoped npm commands.
Also applies to: 144-145
.github/workflows/ci.yaml (1)
131-146: License-diff job gating and permissions are well scoped.The new job runs only for relevant PRs and keeps token scope at
contents: read..pre-commit-config.yaml (1)
94-99: Pre-commit lockfile/attribution hooks are aligned with the root workspace model.The trigger and command updates match the repo’s new single
package-lock.jsonsource of truth.Also applies to: 153-158
.github/workflows/ci_changes.yml (1)
30-33:run_dependenciesoutput wiring is correct and complete.The new reusable output is properly declared and backed by the
changesjob output expression.Also applies to: 61-62
.agents/skills/update-project-version/SKILL.md (1)
27-30: Documentation updates align with workspace migration.The skill doc correctly reflects the npm workspace changes: root lockfile path for
packages["crates/node"].version, workspace-scoped commands, and updated references list. Consistent with the justfile implementation.Also applies to: 47-47, 58-63, 72-72, 76-77, 101-102
justfile (8)
153-156: Correct workspace root install for docs dependencies.Installing at repo root with
--ignore-scriptsis the right pattern for workspace-based npm setup.
215-298: Extended helper correctly handles workspace lockfile structure.The
lock_package_pathparameter properly supports updating specific entries withinpackagesmap. The validation at lines 253-266 ensures the expected structure exists before modification.Minor:
packageEntryKeyis computed twice (lines 261 and 275) but this is negligible for a helper that runs once per invocation.
425-428: Correct propagation of workspace package path.The
crates/nodelock_package_path argument correctly identifies the workspace entry in the root lockfile'spackagesmap.
848-881: Test recipe correctly handles workspace execution with local artifact paths.Running workspace commands from repo root while copying artifacts from
crates/node/is the correct pattern since npm workspaces execute scripts within the package directory.
883-901: Test recipe pattern consistent with test-node.Note that
test-wasmincludesnpm installat line 892, which is the expected pattern thatbuild-wasmappears to be missing.
921-951: Package recipe correctly uses extended version helper and workspace commands.The 4-argument
set_npm_package_versioncall correctly targets the workspace entry, and the full install→build→pack sequence ensures a reproducible package.
658-671: Workspace name matches package.json and npm build pattern is correct.The
--workspace=nemo-flow-nodeflag matches the actual package name incrates/node/package.json. The workspace-based npm workflow is correct:npm install --ignore-scriptsfollowed by conditionalnpm run build-debug(CI) ornpm run build(local) from repo root.
673-682: ⚡ Quick winNot an issue. The
crates/wasm/package.jsonhas no production dependencies and onlyc8(a coverage tool) as a devDependency. Thebuild:pkgscript directly executes a Node script without requiring npm dependencies, so the missingnpm installstep is not necessary for this recipe.> Likely an incorrect or invalid review comment.
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
mnajafian-nv
left a comment
mnajafian-nv
left a comment
There was a problem hiding this comment.
LGTM The root npm workspace and shared lockfile direction is clean, and the license-diff workflow is scoped appropriately as an informational PR check.
One small non-blocking note: I checked the diff locally and git diff --check reports trailing whitespace in the regenerated ATTRIBUTIONS-Rust.md. It looks like generated license text, but worth cleaning if any whitespace hygiene check runs on generated files.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@ATTRIBUTIONS-Python.md`:
- Around line 4656-4661: The attribution generator is currently emitting a
placeholder ("No license file found in locked artifact for sphinx-multiversion")
instead of failing or resolving the license; update the license resolution path
(the function that resolves/writes license text such as
resolve_license_for_package or write_license_block used when rendering the
`LICENSE` section for a package like "sphinx-multiversion") to fail closed or
attempt fetching the license body from sdist/wheel metadata or PyPI before
emitting a placeholder; specifically, if resolve_license_for_package cannot
locate a license file in the locked artifact, make it either (A) abort
generation with a clear error, or (B) query the package sdist/wheel metadata and
PyPI for license files and include that content, and only then emit a non-empty
license body — ensure the error path surfaces a failure instead of writing the
placeholder.
- Line 2335: The markdown inventory contains non-canonical license labels like
"License: `BSD`", "BSD License", "Apache 2.0", "Apache Software License", and
"ISC license" that must be converted to consistent SPDX expressions before
rendering; add a normalization step (e.g., implement
normalize_license_identifier or normalizeLicenseId) and call it from the
inventory renderer (e.g., render_markdown_inventory or render_attribution_entry)
to map known variants to canonical SPDX tokens (e.g., map common "BSD"/"BSD
License" variants, "Apache 2.0"/"Apache Software License", "ISC license", etc.)
and replace the raw label text in ATTRIBUTIONS-Python.md entries prior to
writing so all occurrences (including the examples at "License: `BSD`") are
output as normalized SPDX identifiers.
In `@scripts/licensing/attributions_lockfile_md.py`:
- Line 843: The list comprehension uses data.keys() unnecessarily; update the
expression that builds package_rows to iterate directly over the dict (e.g., for
key in data) instead of for key in data.keys() so package_rows =
[(*_split_node_package_key(key), key) for key in data if key not in
workspace_package_keys]; keep the same variables and helper
_split_node_package_key unchanged.
In `@scripts/licensing/license_diff.py`:
- Around line 223-225: The _filter_inventory function currently omits requested
languages that aren't present in the loaded inventory, which hides missing
snapshots; change _filter_inventory(Inventory, languages) to always include
every language in the input languages list and map missing entries to an empty
list (or whatever the Inventory's empty sentinel is) instead of dropping them,
so returned Inventory contains all requested languages (use Inventory and
_filter_inventory to locate the code and update the dict construction
accordingly).
- Around line 216-220: The loop building inventory currently casts each row with
dict(row) without validating its shape; update the for-loop that iterates
payload -> rows to validate each row before casting: check that each row is a
mapping (e.g., isinstance(row, dict) or collections.abc.Mapping) and contains
the required LicenseInventoryEntry keys (e.g., "id", "license", or whatever
fields LicenseInventoryEntry expects), and if not raise a ValueError including
the language, the offending row, its type and the path variable; only after
validation perform the cast to dict and append to inventory[str(language)] to
ensure malformed rows fail with clear errors instead of opaque exceptions.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: c24775d1-52d2-4feb-8bc3-3922770abbac
📒 Files selected for processing (5)
.github/ci-path-filters.yml.github/workflows/ci_license_diff.ymlATTRIBUTIONS-Python.mdscripts/licensing/attributions_lockfile_md.pyscripts/licensing/license_diff.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check / Run
🧰 Additional context used
📓 Path-based instructions (19)
**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
Format changed Python wrapper and test files with
uv run ruff format python
**/*.py: Use Ruff with rule sets E, F, W, I for Python linting
Use Ruff formatter for Python code with line length 120 and double quotes
Usetyfor Python type checking
Use snake_case naming convention for Python code
Include SPDX license headers in all Python source files using Python comment syntaxFor Python-only wrapper or binding changes, use
test-python-bindingFor Python files, run
uv run ruff format pythonFollow Python naming convention: use snake_case for variables, functions, and module names
Files:
scripts/licensing/license_diff.pyscripts/licensing/attributions_lockfile_md.py
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
SONAR_IGNORE_START/SONAR_IGNORE_ENDmarkers only for documented false positives that cannot be resolved in code; keep ignored blocks small, add explanatory comments, and require reviewer sign-off
Files:
scripts/licensing/license_diff.pyscripts/licensing/attributions_lockfile_md.py
**/*.{md,markdown,py,sh,bash,js,ts,java,cpp,go,rust}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current in documentation
Files:
scripts/licensing/license_diff.pyscripts/licensing/attributions_lockfile_md.pyATTRIBUTIONS-Python.md
**/*.{md,markdown,py,sh,bash}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep stable user-facing wrappers at scripts/ root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
Files:
scripts/licensing/license_diff.pyscripts/licensing/attributions_lockfile_md.pyATTRIBUTIONS-Python.md
**/*.{md,markdown,py,sh,bash,js,ts,example}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Example commands must match current package names and paths
Files:
scripts/licensing/license_diff.pyscripts/licensing/attributions_lockfile_md.pyATTRIBUTIONS-Python.md
{.github/**/*.{yml,yaml},*.patch,scripts/**/*,*.sh,*.bat,Dockerfile*}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update CI configuration, patch files, and build scripts with new functional identifiers after rename operations
Files:
scripts/licensing/license_diff.py.github/ci-path-filters.ymlscripts/licensing/attributions_lockfile_md.py.github/workflows/ci_license_diff.yml
**/*.{js,ts,tsx,jsx,py,rs,go,java,c,cpp,h,cc,cxx,cs,rb,php,swift,kt}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changed files must be formatted with the language-native formatter
Files:
scripts/licensing/license_diff.pyscripts/licensing/attributions_lockfile_md.py
**/*.{rs,py,js,ts,go,toml,yaml,yml,md,sh,json,xml}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files with Apache-2.0 license identifier
Files:
scripts/licensing/license_diff.pyscripts/licensing/attributions_lockfile_md.pyATTRIBUTIONS-Python.md
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}
⚙️ CodeRabbit configuration file
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.
Files:
scripts/licensing/license_diff.py.github/ci-path-filters.ymlscripts/licensing/attributions_lockfile_md.py.github/workflows/ci_license_diff.yml
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-ci/SKILL.md)
.github/workflows/**/*.{yml,yaml}: Putpermissions:on each job that needs token access in GitHub Actions workflows
Avoid workflow-level permissions unless the repository intentionally centralizes them and the inheritance tradeoff is documented
Keep third-party actions pinned to full commit SHAs and preserve the readable version comment after the SHA
Prefer action-native or ecosystem-native caching over genericactions/cachein GitHub Actions workflows
Use lockfiles or dependency manifests to drive cache invalidation in GitHub Actions workflows
Keep deploy and publish permissions isolated to the jobs that need them
Read both caller and callee when a workflow usesworkflow_callin GitHub Actions
Put release-tag validation in the earliest practical caller job when the pipeline has tag-based publish behavior
Keep release-tag policy aligned withRELEASING.md: raw SemVer tags only, no leadingv
contents: readis the default minimum for checkout-based build, test, docs, and packaging jobs
pull-requests: readis required for PR metadata lookup jobs in GitHub Actions workflows
pages: writeandid-token: writeshould be limited to Pages deployment jobs and any caller that invokes them through a reusable workflow
For reusable workflows, the caller must grant every permission the called jobs require and the callee cannot elevate beyond what the caller provides
Preferastral-sh/setup-uvcache support withcache-dependency-globanchored touv.lockfor Python dependency caching
PreferSwatinem/rust-cachewith explicitshared-keyandworkspacesinstead of ad hoc target-directory caching
Avoid caching generated outputs that can hide stale behavior unless the repo already relies on them deliberately
Ensure each job has the minimum permissions it needs during GitHub Actions CI review
Ensure reusable workflow callers grant only the scopes their callees require
Ensure every external action is pinned to a full SHA in GitHub Actions workflows
Ensure cache ...
Files:
.github/workflows/ci_license_diff.yml
{.github/workflows/**/*.{yml,yaml},.gitlab-ci.yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
CI workflows, install commands, and example commands must reference consistent package names
Files:
.github/workflows/ci_license_diff.yml
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
ATTRIBUTIONS-Python.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
ATTRIBUTIONS-Python.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst,txt}: SpellNVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as 'here' or 'read more.'
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not add trademark symbols to NeMo Flow learning documentation by default.
Do not rewrite API names, package names, command flags, or code literals for style reasons.
Files:
ATTRIBUTIONS-Python.md
**/*.{md,markdown,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,markdown,rst}: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Use monospace formatting for code elements, commands, parameters, package names, and expressions
Use monospace formatting for directories, file names, and paths
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Use quotation marks for error messages and strings in documentation
Use bold formatting for UI buttons, menus, fields, and labels in documentation
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use in documentation
Use italics for publication titles in documentation
Use plain text formatting for keyboard shortcuts in documentation
Prefer[NVIDIA/NeMo](link)format for GitHub repository references over generic phrases like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text in documentation
Avoid generic link anchors such as 'here,' 'this page,' and 'read more' in documentation
Include the acronym in link text if a linked term includes an acronym
Do not link long sentences or multiple sentences in documentation
Avoid links that pull readers away from a procedure unles...
Files:
ATTRIBUTIONS-Python.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license headers in HTML and Markdown files using HTML comment syntax
Files:
ATTRIBUTIONS-Python.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run Markdown link checking via
lycheeforREADME.md,CONTRIBUTING.md, anddocs/through pre-commit hooks
Files:
ATTRIBUTIONS-Python.md
{README*,CHANGELOG*,docs/**/*.{md,rst,txt},examples/**/*,*.md}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update documentation, examples, and getting-started guides with new package/module/crate names after rename operations
Files:
ATTRIBUTIONS-Python.md
**/*.{md,txt,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,txt,rst}: Ensure commands, package names, file paths, and APIs in documentation are correct and not stale; flag incorrect or outdated information as blocking issues
Ensure examples and procedures in documentation will execute successfully with current APIs and commands
Use consistent user-facing terminology throughout documentation that matches current repo terminology
Capitalize NVIDIA correctly in all documentation and public-facing text
Format code, commands, paths, and filenames as inline code (monospace) in documentation
Use descriptive anchor text for links instead of bare URLs or weak labels like 'here' in documentation
Prefer active voice, present tense, short sentences, and plain English in documentation
Structure documentation procedures as imperative steps that are easy to scan and not too long for a single sequence
Prefer 'after' instead of 'once' for temporal references in documentation
Use 'can' instead of 'may' when describing possibility (rather than permission) in documentation
Avoid ambiguous numeric dates and ordinal dates in documentation body text
Files:
ATTRIBUTIONS-Python.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:26.959Z
Learning: WebAssembly package naming and generated package expectations must be consistent with downstream consumer expectations
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:26.959Z
Learning: FFI header and library naming must be consistent and follow expected naming conventions
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:26.959Z
Learning: Release tags must use raw SemVer without a leading `v`
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:26.959Z
Learning: Package names, import paths, and module names must be internally consistent across all package manifests
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:26.959Z
Learning: Generated artifacts must still land where downstream consumers expect
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:34.146Z
Learning: When changes touch `crates/core`, `crates/adaptive`, or the generated Rust binding layer under `crates/node/src`, use the `validate-change` skill alongside this guideline
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:34.146Z
Learning: For documentation-only changes around Node usage, keep validation targeted rather than running full test suites
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:34.146Z
Learning: Use `karpathy-guidelines` skill alongside this skill for implementation or review work on Node binding changes
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:40.826Z
Learning: Run WebAssembly tests with `just test-wasm`
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:40.826Z
Learning: Use `just build-wasm` for explicit WebAssembly packaging/build pass
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:40.826Z
Learning: Use `just ci=true test-wasm` when coverage reports are needed
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:40.826Z
Learning: If the change touched shared runtime semantics in `crates/core` or `crates/adaptive`, also use `validate-change` skill
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:40.826Z
Learning: If the issue is only packaging or generated wrapper output, prioritize the build plus package-prep steps before the full test sweep
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:40.826Z
Learning: Use `karpathy-guidelines` alongside this skill for implementation or review work on WebAssembly changes
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:52.627Z
Learning: Run `cargo check --workspace` to refresh `Cargo.lock` when workspace package entries change
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:52.627Z
Learning: Do not update only `Cargo.toml` or only Node package metadata; keep all version sources in sync
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:52.627Z
Learning: Do not assume `crates/wasm/package.json` is the published npm manifest; inspect the regenerated `crates/wasm/pkg/package.json` for the actual published version
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:52.627Z
Learning: Do not forget `Cargo.lock`, `ATTRIBUTIONS-Rust.md`, or `ATTRIBUTIONS-Node.md` after changing versioned inputs that feed them
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:52.627Z
Learning: Do not perform blind repository-wide search/replace across docs, patches, and generated attribution files when updating versions
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:52.627Z
Learning: Do not commit temporary non-release version suffixes set by `just package-node`, `just package-python`, or `just package-wasm` as the canonical project version unless the release process requires that exact string
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:52.627Z
Learning: Use `just set-version <version>` to update release-version source files including `[workspace.package].version`, workspace dependencies, and Node package metadata
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:26:52.627Z
Learning: Audit remaining references to the old version with targeted search after running `just set-version`, separating true version pins from examples, generated attribution files, and unrelated third-party versions
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:08.771Z
Learning: Format changed files with the language-native formatter before the final lint/test pass
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:08.771Z
Learning: If a language surface changed, always run that language's test target even when Rust core did not change
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:08.771Z
Learning: If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:08.771Z
Learning: During iteration, prefer `uv run pre-commit run --files <changed files...>`
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:08.771Z
Learning: Before review or handoff, run `uv run pre-commit run --all-files`
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:08.771Z
Learning: When running pre-commit during iteration, use `uv run pre-commit run --files <changed files...>` to run hooks only on specified files
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:25.386Z
Learning: Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:25.386Z
Learning: Do not hand-edit generated or packaged outputs unless the repository workflow expects them to be checked in; regenerate through the documented recipe or script
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:25.386Z
Learning: Preserve the shared runtime model across bindings; do not add behavior to one primary binding without considering Rust, Python, and Node.js parity
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:25.386Z
Learning: Run tests for every language affected by a change; if touching Rust core runtime, middleware semantics, event shape, scope behavior, typed codecs, plugins, or observability, validate every affected binding
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:25.386Z
Learning: Prefer the repository 'just' recipes over raw tool commands for build, test, and docs tasks
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:25.386Z
Learning: Use branch prefixes from contributor docs: 'feat/', 'fix/', 'docs/', 'test/', or 'refactor/' for Git branch names
Learnt from: CR
Repo: NVIDIA/NeMo-Flow
Timestamp: 2026-05-08T15:27:25.386Z
Learning: Use signed-off commits for PR work: 'git commit -s'
📚 Learning: 2026-05-03T04:23:07.497Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Flow PR: 46
File: .github/workflows/ci_rust.yml:31-64
Timestamp: 2026-05-03T04:23:07.497Z
Learning: In GitHub Actions workflow YAML, it’s valid to conditionally disable a service container by setting the service container’s `image` to an empty string (`''`) via a matrix variable (e.g., `redis_service_image: ''`). This intentionally makes the runner skip service initialization for that matrix entry rather than failing the job. When reviewing workflows, don’t flag this as an error if the workflow uses an empty `image` to disable the service on specific matrix entries (e.g., OS-specific setups); verify the `image` is sourced from the matrix variable and that the service is only expected to be available when a non-empty image is provided.
Applied to files:
.github/workflows/ci_license_diff.yml
🪛 markdownlint-cli2 (0.22.1)
ATTRIBUTIONS-Python.md
[warning] 1972-1972: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1972-1972: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 2010-2010: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2043-2043: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2110-2110: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2189-2189: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 2189-2189: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 2219-2219: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2756-2756: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2816-2816: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2876-2876: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2944-2944: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3275-3275: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 4521-4521: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 4595-4595: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 4599-4599: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 4599-4599: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 4625-4625: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 4659-4659: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 4659-4659: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 5661-5661: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 5661-5661: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 5687-5687: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5975-5975: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5979-5979: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 5979-5979: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 6005-6005: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Ruff (0.15.12)
scripts/licensing/license_diff.py
[warning] 76-76: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 167-167: Use list.extend to create a transformed list
(PERF401)
[warning] 214-214: Prefer TypeError exception for invalid type
(TRY004)
[warning] 214-214: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 218-218: Prefer TypeError exception for invalid type
(TRY004)
[warning] 218-218: Avoid specifying long messages outside the exception class
(TRY003)
[error] 234-234: subprocess call: check for execution of untrusted input
(S603)
[error] 235-235: Starting a process with a partial executable path
(S607)
[error] 243-243: subprocess call: check for execution of untrusted input
(S603)
[error] 244-244: Starting a process with a partial executable path
(S607)
[warning] 259-259: Avoid specifying long messages outside the exception class
(TRY003)
scripts/licensing/attributions_lockfile_md.py
[warning] 641-641: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 843-843: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
🔇 Additional comments (12)
scripts/licensing/attributions_lockfile_md.py (9)
28-48: LGTM!The
NODE = ROOTchange correctly reflects the npm workspace migration to the repository root. TheLicenseInventoryEntryTypedDict provides clear typing for the machine-readable inventory format consumed bylicense_diff.py.
127-160: LGTM!The license normalization helpers are well-designed.
_is_weak_license_namecorrectly identifies placeholder labels,_normalize_license_nameconsolidates common MIT variants, and_looks_like_mit_licenseuses robust canonical phrase matching without being overly specific to formatting.
162-187: LGTM!The enhanced license inference with content-based fallbacks (ISC keyword and MIT structural detection) improves coverage for packages with non-standard heading formats.
265-313: LGTM!The refactored rendering correctly defers markdown assembly until after collection and sorting. The
(name.lower(), version)sort key produces stable, deterministic output.
316-343: LGTM!The inventory function correctly consolidates multiple license IDs per crate into SPDX-style
" OR "expressions while maintaining deterministic sorted output.
393-416: LGTM!Applying
_clean_license_textuniformly to both wheel and sdist extraction ensures consistent whitespace handling in rendered attributions.
607-641: LGTM!The refined fallback cascade correctly prioritizes sdist for platform-agnostic license texts while falling back to wheel metadata for better license labels. The combination logic at lines 632-633 elegantly merges the best of both sources.
773-797: LGTM!The
--ignore-scriptsflag at line 787 addresses the past review feedback. The fallback tocrates/node/package-lock.jsonmaintains backward compatibility during migration.
813-837: LGTM!The inventory reads directly from the lockfile without side effects, correctly filters workspace and extraneous packages, and properly handles scoped package names via path inference.
.github/workflows/ci_license_diff.yml (1)
65-66: Unresolved: step-levelcontinue-on-errorstill suppresses failure signalThis remains the same concern from earlier review: the step can fail without surfacing a proper failing status, reducing visibility of real execution failures.
scripts/licensing/license_diff.py (1)
233-236: Good hardening on worktree checkout argsIncluding
--beforerefin thegit worktree addinvocation is the right defensive fix..github/ci-path-filters.yml (1)
18-28: Dependency filter coverage looks correctIncluding
scripts/licensing/**with lockfiles/manifests closes the trigger gap for license-diff workflow changes.
Signed-off-by: Will Killian <wkillian@nvidia.com>
Salonijain27
left a comment
Salonijain27
left a comment
There was a problem hiding this comment.
Approved from a dependency point of view
|
/merge |
3 participants
Overview
Adds a lockfile license inventory diff workflow so pull requests that change dependency manifests or lockfiles get a Markdown summary of Rust, Node.js, and Python license changes directly in GitHub Actions.
Details
scripts/licensing/license_diff.py, which generates grouped Rust, Node.js, and Python license inventories and compares them against a base ref or JSON snapshot.cargo aboutfor Rust inventory, reads rootpackage-lock.jsonfor Node.js inventory, and readsuv.lockartifacts for Python inventory without running environment setup builds.ci_license_diffworkflow and adependenciespath-filter group so PR license diffs run only for dependency changes, after Check, without blocking CI.Validation recorded for this PR:
ruff check scripts/licensing/license_diff.py scripts/licensing/attributions_lockfile_md.pypython3 -m py_compile scripts/licensing/license_diff.py scripts/licensing/attributions_lockfile_md.pyty check scripts/licensing/license_diff.py scripts/licensing/attributions_lockfile_md.pypython3 scripts/licensing/license_diff.py --base-ref HEAD noderuby -e 'require "yaml"; Dir[".github/workflows/*.{yml,yaml}"].each { |f| YAML.load_file(f) }; YAML.load_file(".github/ci-path-filters.yml"); puts "yaml-ok"'bash -non the extractedci_license_diffshell blockgit diff --check -- .github/workflows/ci.yaml .github/workflows/ci_changes.yml .github/workflows/ci_license_diff.yml .github/ci-path-filters.ymlWhere should the reviewer start?
Start with
scripts/licensing/license_diff.pyfor the new comparison behavior, then review.github/workflows/ci_license_diff.ymland.github/ci-path-filters.ymlfor the PR-only CI integration.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Documentation
Chores