ci: Manage more tooling with flox - #76
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds Flox-managed developer and tool environments. CI uses Flox for selected tools, Node.js setup reads ChangesFlox environment and workflow integration
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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 @.flox/env/manifest.toml:
- Line 11: Typo in the environment manifest: rename the variable
BORDER_FORGROUND to BORDER_FOREGROUND (add the missing "E") in the manifest and
update any code or tooling that reads BORDER_FORGROUND to use BORDER_FOREGROUND
instead so nothing breaks at runtime; search for occurrences of BORDER_FORGROUND
and replace them with BORDER_FOREGROUND (including any scripts, config parsers,
or CI/deploy steps that consume the manifest).
In `@packages/.flox/env/manifest.toml`:
- Around line 30-31: The install step currently runs "pre-commit install"
without ensuring the .git/hooks directory exists; update the script around the
check for ".git/hooks/pre-commit" so it first verifies or creates the directory
(e.g., test -d .git/hooks || mkdir -p .git/hooks) before running "pre-commit
install", or conditionally run "pre-commit install" only if ".git/hooks" exists
to avoid failures.
In `@tools/osv-scanner/.flox/.gitignore`:
- Line 5: The .gitignore currently contains an ineffective negation pattern
"!env/"; update the file so that everything is ignored except the env/ directory
by adding a global ignore pattern (e.g., "*") before the negation and keep the
"!env/" entry, and ensure you have requested/obtained Write permissions before
editing files inside dot-folders (like .flox) as per the repo guidelines.
In `@tools/osv-scanner/.flox/env/manifest.toml`:
- Line 4: The manifest entry for the package "osv-scanner" is unpinned; update
the manifest.toml entry for the osv-scanner install (the table/entry starting
with osv-scanner = { pkg-path = "osv-scanner", pkg-group = "osv-scanner" }) to
include a deterministic version field (e.g., add version = "x.y.z") so the line
becomes something like osv-scanner = { pkg-path = "osv-scanner", pkg-group =
"osv-scanner", version = "1.2.3" } using the actual desired pinned version.
In `@web/.flox/env/manifest.toml`:
- Line 21: The command `pnpm install -C web --frozen-lockfile` in the manifest
is relative to the current working directory and can fail when Flox activation
is run outside the repo; update the entry in the manifest under the profile
common to use the project-root path by replacing it with `pnpm install -C
"$FLOX_ENV_PROJECT/web" --frozen-lockfile` or alternatively prepend a `cd
"$FLOX_ENV_PROJECT"` before the install so the `-C` path is resolved from the
repository root.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f93bde0a-0c43-49a2-8e22-010d04ce4135
⛔ Files ignored due to path filters (5)
.flox/env/manifest.lockis excluded by!**/*.lockpackages/.flox/env/manifest.lockis excluded by!**/*.locktools/open-policy-agent/.flox/env/manifest.lockis excluded by!**/*.locktools/osv-scanner/.flox/env/manifest.lockis excluded by!**/*.lockweb/.flox/env/manifest.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.flox/env/manifest.toml.github/workflows/ci.yamlpackages/.flox/.gitattributespackages/.flox/.gitignorepackages/.flox/env.jsonpackages/.flox/env/manifest.tomlpyproject.tomltools/open-policy-agent/.flox/.gitattributestools/open-policy-agent/.flox/.gitignoretools/open-policy-agent/.flox/env.jsontools/open-policy-agent/.flox/env/manifest.tomltools/osv-scanner/.flox/.gitattributestools/osv-scanner/.flox/.gitignoretools/osv-scanner/.flox/env.jsontools/osv-scanner/.flox/env/manifest.tomlweb/.flox/.gitattributesweb/.flox/.gitignoreweb/.flox/env.jsonweb/.flox/env/manifest.toml
8880a21 to
786681a
Compare
d6268c4 to
64d7db6
Compare
64d7db6 to
4fd809d
Compare
78a20aa to
a3f0beb
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yaml (1)
23-24: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd
contents: readto the workflow permissions.actions/checkoutneeds repository read access, and unspecified permissions becomenoneoncepermissionsis set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yaml around lines 23 - 24, Update the workflow-level permissions block to add contents: read alongside actions: read, preserving the existing actions permission so actions/checkout can read repository contents.
🤖 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.
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 23-24: Update the workflow-level permissions block to add
contents: read alongside actions: read, preserving the existing actions
permission so actions/checkout can read repository contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ddffdefc-a370-43c9-b43c-1f72aa58de4d
⛔ Files ignored due to path filters (1)
.flox/env/manifest.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.flox/env/manifest.toml.github/actions/build-nemo-platform-wheel/action.yaml.github/actions/build-policy-wasm/action.yaml.github/actions/changes/action.yaml.github/workflows/ci.yaml.pre-commit-config.yamlCONTRIBUTING.md
a3f0beb to
308f578
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
tools/lint/check_uv_version.py (1)
31-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the new CI gate.
Cover matching constraints, mismatches, missing/non-string values, and malformed TOML so this validator cannot regress silently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/lint/check_uv_version.py` around lines 31 - 55, Add tests for main’s UV constraint validation covering matching values, mismatches, missing or non-string constraints, and malformed TOML inputs. Exercise the existing load_toml and get_nested_value paths, asserting the expected return status and stderr output without changing the validator behavior.
🤖 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/workflows/ci.yaml:
- Line 59: Update both actions/checkout steps in .github/workflows/ci.yaml at
lines 59-59 and 1673-1674 to set persist-credentials to false, preventing the
checkout token from being stored in local Git configuration before subsequent
Flox actions run.
In `@CONTRIBUTING.md`:
- Around line 22-38: Replace hardcoded “Flox” product-name references in the new
“Full Developer Environment with Flox” documentation section with the
repository’s configured documentation substitution, including the heading and
prose, while preserving the existing commands and link.
- Around line 35-38: Update the activation instructions in CONTRIBUTING.md to
tell users to leave the flox-activated subshell with exit (or Ctrl-D), replacing
the flox deactivate instruction while preserving the surrounding setup guidance.
In `@flox-environments/common/.flox/env/manifest.toml`:
- Around line 12-14: Replace the invalid 514 color values assigned to
GUM_LOG_MESSAGE_FOREGROUND, FOREGROUND, and BORDER_FOREGROUND with a valid Gum
color value in the supported ANSI 0–255 or hex range.
In `@flox-environments/python/.flox/env/manifest.toml`:
- Around line 23-31: Move the virtual-environment setup from the bash, zsh, and
fish profile entries into the manifest’s hook configuration so it runs during
exec mode as well as interactive activation. Preserve the existing
shell-specific activation behavior and ensure flox activate -- commands receive
the .venv environment and PATH.
In `@tools/lint/lint-all.sh`:
- Line 9: Add check-uv-version to the no-fix lint list handled by
is_no_fix_lint() in lint-all.sh, so the validation-only
tools/lint/check-uv-version.sh is not reported as fixable or included in make
lint-fix suggestions.
---
Nitpick comments:
In `@tools/lint/check_uv_version.py`:
- Around line 31-55: Add tests for main’s UV constraint validation covering
matching values, mismatches, missing or non-string constraints, and malformed
TOML inputs. Exercise the existing load_toml and get_nested_value paths,
asserting the expected return status and stderr output without changing the
validator behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e2e543a0-8e6e-4ec4-a522-8b38156cc8be
⛔ Files ignored due to path filters (8)
.flox/env/manifest.lockis excluded by!**/*.lockflox-environments/actionlint/.flox/env/manifest.lockis excluded by!**/*.lockflox-environments/common/.flox/env/manifest.lockis excluded by!**/*.lockflox-environments/helm/.flox/env/manifest.lockis excluded by!**/*.lockflox-environments/open-policy-agent/.flox/env/manifest.lockis excluded by!**/*.lockflox-environments/osv-scanner/.flox/env/manifest.lockis excluded by!**/*.lockflox-environments/python/.flox/env/manifest.lockis excluded by!**/*.lockflox-environments/web/.flox/env/manifest.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.flox/env/manifest.toml.github/actions/build-nemo-platform-wheel/action.yaml.github/actions/build-policy-wasm/action.yaml.github/actions/changes/action.yaml.github/workflows/ci.yaml.pre-commit-config.yamlCONTRIBUTING.mdflox-environments/actionlint/.flox/.gitattributesflox-environments/actionlint/.flox/.gitignoreflox-environments/actionlint/.flox/env.jsonflox-environments/actionlint/.flox/env/manifest.tomlflox-environments/common/.flox/.gitattributesflox-environments/common/.flox/.gitignoreflox-environments/common/.flox/env.jsonflox-environments/common/.flox/env/manifest.tomlflox-environments/helm/.flox/.gitattributesflox-environments/helm/.flox/.gitignoreflox-environments/helm/.flox/env.jsonflox-environments/helm/.flox/env/manifest.tomlflox-environments/open-policy-agent/.flox/.gitattributesflox-environments/open-policy-agent/.flox/.gitignoreflox-environments/open-policy-agent/.flox/env.jsonflox-environments/open-policy-agent/.flox/env/manifest.tomlflox-environments/osv-scanner/.flox/.gitattributesflox-environments/osv-scanner/.flox/.gitignoreflox-environments/osv-scanner/.flox/env.jsonflox-environments/osv-scanner/.flox/env/manifest.tomlflox-environments/python/.flox/.gitattributesflox-environments/python/.flox/.gitignoreflox-environments/python/.flox/env.jsonflox-environments/python/.flox/env/manifest.tomlflox-environments/web/.flox/.gitattributesflox-environments/web/.flox/.gitignoreflox-environments/web/.flox/env.jsonflox-environments/web/.flox/env/manifest.tomlpyproject.tomltools/lint/check-uv-version.shtools/lint/check_uv_version.pytools/lint/lint-all.sh
🚧 Files skipped from review as they are similar to previous changes (6)
- pyproject.toml
- .github/actions/build-nemo-platform-wheel/action.yaml
- .github/actions/build-policy-wasm/action.yaml
- .github/actions/changes/action.yaml
- .flox/env/manifest.toml
- .pre-commit-config.yaml
| #### Full Developer Environment with Flox | ||
|
|
||
| For a complete contributor environment, including Python tooling and TypeScript | ||
| dependencies, install [Flox](https://flox.dev/docs/install-flox/install) and | ||
| activate the repository environment: | ||
|
|
||
| ```bash | ||
| # macOS with Homebrew; see the Flox installation guide for other platforms. | ||
| brew install flox | ||
|
|
||
| flox activate | ||
| ``` | ||
|
|
||
| The first activation installs the pinned Python and Node.js toolchains, | ||
| synchronizes Python dependencies, and installs the pnpm workspace dependencies. | ||
| It also installs the repository's pre-commit hook when needed. Leave the | ||
| environment with `flox deactivate` when you are done. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the configured product-name substitution.
The new section hardcodes Flox in headings and prose. Use the repository’s documentation substitution instead.
As per coding guidelines: “Never hardcode product names; use substitutions in Sphinx configuration to maintain consistency.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CONTRIBUTING.md` around lines 22 - 38, Replace hardcoded “Flox” product-name
references in the new “Full Developer Environment with Flox” documentation
section with the repository’s configured documentation substitution, including
the heading and prose, while preserving the existing commands and link.
Source: Coding guidelines
| The first activation installs the pinned Python and Node.js toolchains, | ||
| synchronizes Python dependencies, and installs the pnpm workspace dependencies. | ||
| It also installs the repository's pre-commit hook when needed. Leave the | ||
| environment with `flox deactivate` when you are done. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use exit to leave the activated subshell.
flox activate starts a subshell; the documented way to leave it is exit or Ctrl-D. Replace flox deactivate, which is not part of the documented CLI flow. (flox.dev)
Proposed fix
-Leave the environment with `flox deactivate` when you are done.
+Leave the environment with `exit` or Ctrl-D when you are done.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The first activation installs the pinned Python and Node.js toolchains, | |
| synchronizes Python dependencies, and installs the pnpm workspace dependencies. | |
| It also installs the repository's pre-commit hook when needed. Leave the | |
| environment with `flox deactivate` when you are done. | |
| The first activation installs the pinned Python and Node.js toolchains, | |
| synchronizes Python dependencies, and installs the pnpm workspace dependencies. | |
| It also installs the repository's pre-commit hook when needed. Leave the | |
| environment with `exit` or Ctrl-D when you are done. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CONTRIBUTING.md` around lines 35 - 38, Update the activation instructions in
CONTRIBUTING.md to tell users to leave the flox-activated subshell with exit (or
Ctrl-D), replacing the flox deactivate instruction while preserving the
surrounding setup guidance.
| GUM_LOG_MESSAGE_FOREGROUND="514" | ||
| FOREGROUND="514" | ||
| BORDER_FOREGROUND="514" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
while IFS= read -r color; do
[[ "$color" =~ ^(0|[1-9][0-9]?|1[0-9]{2}|2[0-4][0-9]|25[0-5])$ ]] || {
printf 'invalid ANSI color: %s\n' "$color" >&2
exit 1
}
done < <(
sed -nE 's/^(GUM_LOG_MESSAGE_FOREGROUND|FOREGROUND|BORDER_FOREGROUND)="([^"]+)"/\2/p' \
flox-environments/common/.flox/env/manifest.toml
)Repository: NVIDIA-NeMo/nemo-platform
Length of output: 216
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '1,40p' flox-environments/common/.flox/env/manifest.toml
printf '\n---\n'
rg -n 'GUM_LOG_MESSAGE_FOREGROUND|FOREGROUND|BORDER_FOREGROUND|gum style|termenv|lipgloss' -S flox-environments/common -g '!**/node_modules/**' || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 522
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('flox-environments/common/.flox/env/manifest.toml')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 25:
print(f"{i}: {line}")
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 592
🌐 Web query:
gum style color value ANSI range termenv foreground accepts hex or 0-255
💡 Result:
The gum style command uses the lipgloss library [1], which accepts both hex color codes (e.g., "#FF0000") and ANSI color numbers (0-255) for foreground and background color flags [1][2]. Under the hood, gum relies on the termenv library to handle terminal color capabilities [3][4]. When you provide a color to gum: 1. Hex values: You can specify colors using standard hex notation (e.g., --foreground "#FF0000") [1][5]. Termenv will automatically degrade these colors to match the capabilities of your terminal (TrueColor, ANSI 256, or ANSI 16) [3][4]. 2. ANSI numbers: You can specify colors using their numeric ANSI index (0-255) (e.g., --foreground "69") [3][4][1]. Termenv manages these conversions based on the detected terminal profile [3][2]. If your terminal does not support TrueColor (RGB), it will map the provided color to the nearest available match in the 256-color or 16-color ANSI space [3][4]. While earlier versions of gum required hex or numeric inputs [1], recent updates have been implemented to support common named colors (e.g., "red", "green", "blue") by resolving them to their corresponding hex or ANSI values [6][7].
Citations:
- 1: Is it possible to specify a color name in
gum style --foreground? charmbracelet/gum#836 - 2: https://pkg.go.dev/github.com/muesli/termenv@v0.15.3
- 3: https://pkg.go.dev/github.com/muesli/termenv
- 4: https://github.com/muesli/termenv/blob/master/README.md
- 5: gum style --foreground red silently fails - no validation or error for invalid color names charmbracelet/gum#980
- 6: charmbracelet/gum@fd0c575
- 7: fix(style): support common named foreground/background colors charmbracelet/gum#1076
Use a valid Gum color value. 514 is outside Gum’s supported color range (ANSI 0–255 or hex). Replace GUM_LOG_MESSAGE_FOREGROUND, FOREGROUND, and BORDER_FOREGROUND with a valid value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flox-environments/common/.flox/env/manifest.toml` around lines 12 - 14,
Replace the invalid 514 color values assigned to GUM_LOG_MESSAGE_FOREGROUND,
FOREGROUND, and BORDER_FOREGROUND with a valid Gum color value in the supported
ANSI 0–255 or hex range.
| bash = ''' | ||
| source .venv/bin/activate | ||
| ''' | ||
| zsh = ''' | ||
| source .venv/bin/activate | ||
| ''' | ||
| fish = ''' | ||
| set -gx VIRTUAL_ENV "$PWD/.venv" | ||
| set -gx PATH "$VIRTUAL_ENV/bin" $PATH |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== file outline ==\n'
ast-grep outline flox-environments/python/.flox/env/manifest.toml || true
printf '\n== manifest excerpt ==\n'
sed -n '1,120p' flox-environments/python/.flox/env/manifest.toml
printf '\n== search for exec/profile/hook references ==\n'
rg -n "flox activate --|profile|hook|exec mode|activate" flox-environments/python/.flox -S || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 1763
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path('flox-environments/python/.flox/env/manifest.toml')
text = p.read_text()
print('contains profile:', '[profile]' in text)
print('contains hook:', '[hook]' in text or 'hook' in text)
for i, line in enumerate(text.splitlines(), 1):
if 1 <= i <= 80:
print(f'{i:03d}: {line}')
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 970
🏁 Script executed:
set -euo pipefail
printf '\n== repo files under flox-environments/python ==\n'
find flox-environments/python -maxdepth 4 -type f | sort | sed -n '1,120p'
printf '\n== manifest.toml with line numbers ==\n'
nl -ba flox-environments/python/.flox/env/manifest.toml | sed -n '1,120p'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 517
🏁 Script executed:
set -euo pipefail
rg -n "flox activate --|flox activate -- |activate --" . -S || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 276
🌐 Web query:
flox activate -- exec mode profile scripts skipped documentation
💡 Result:
In Flox version 1.9.0 and later, running "flox activate -- [command]" intentionally skips the execution of your user shell and profile scripts [1][2][3]. This change was introduced as part of a significant update to the Flox activation subsystem to improve efficiency and maintain the same process ID (PID) throughout the command execution [3]. By design, this "exec mode" (invoked with the -- flag) now uses the exec system call to run the provided command directly [4][3]. Because it bypasses the shell initialization process, any environment customizations defined in the [profile] section of your manifest—such as aliases, shell-specific functions, or prompt configurations—will not be loaded or executed [4][5]. If you require the previous behavior where profile scripts are executed, you can use the following alternatives introduced or clarified in the 1.9.0 release: 1. Use "flox activate -c 'command'": This feature provides the previous semantics of the -- flag, executing your command within your default shell and ensuring that profile scripts are sourced [1][3]. 2. Use "eval '$(flox activate)'" or standard interactive "flox activate": These methods remain unchanged and continue to source profile scripts as part of the environment configuration [4][5]. This shift ensures that automated tools or service managers that rely on tracking a consistent PID can effectively manage processes spawned via "flox activate --" [1][3].
Citations:
- 1: https://discourse.flox.dev/t/big-release-flox-1-9-0-is-now-available-with-some-major-updates/1229
- 2: https://newreleases.io/project/github/flox/flox/release/v1.9.0
- 3: https://github.com/flox/flox/releases/tag/v1.9.0
- 4: https://flox.dev/docs/concepts/activation
- 5: https://flox.mintlify.app/concepts/activation
Make the venv available in exec mode. flox activate -- ... skips [profile], so source .venv/bin/activate never runs there. Move the venv setup into [hook] or use flox activate -c/profile mode in CI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flox-environments/python/.flox/env/manifest.toml` around lines 23 - 31, Move
the virtual-environment setup from the bash, zsh, and fish profile entries into
the manifest’s hook configuration so it runs during exec mode as well as
interactive activation. Preserve the existing shell-specific activation behavior
and ensure flox activate -- commands receive the .venv environment and PATH.
| cd "${PROJECT_ROOT}" || exit 1 | ||
|
|
||
| declare -a scripts=( | ||
| "check-uv-version:tools/lint/check-uv-version.sh" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 'is_no_fix_lint|--fix|scripts|lint_name' tools/lint/lint-all.shRepository: NVIDIA-NeMo/nemo-platform
Length of output: 2410
🏁 Script executed:
sed -n '60,110p' tools/lint/lint-all.sh
printf '\n---\n'
sed -n '1,200p' tools/lint/check-uv-version.shRepository: NVIDIA-NeMo/nemo-platform
Length of output: 1272
Add check-uv-version to is_no_fix_lint() lint-all.sh will treat this lint as fixable and suggest make lint-fix, even though tools/lint/check-uv-version.sh is validation-only.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/lint/lint-all.sh` at line 9, Add check-uv-version to the no-fix lint
list handled by is_no_fix_lint() in lint-all.sh, so the validation-only
tools/lint/check-uv-version.sh is not reported as fixable or included in make
lint-fix suggestions.
308f578 to
0fda19f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/nodejs/.flox/env/manifest.toml`:
- Around line 20-22: Update the manifest activation hook around package_manager
and the web dependency commands to resolve the repository root explicitly before
accessing web/package.json or using -C ./web. Ensure direct activation from
tools/nodejs and documented root activation both target the repository’s actual
web directory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2fd2dc2b-b0ad-4a36-9999-909606a0a99b
⛔ Files ignored due to path filters (2)
.flox/env/manifest.lockis excluded by!**/*.locktools/nodejs/.flox/env/manifest.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.flox/env/manifest.toml.github/actions/changes/action.yamltools/nodejs/.flox/.gitattributestools/nodejs/.flox/.gitignoretools/nodejs/.flox/env.jsontools/nodejs/.flox/env/manifest.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/actions/changes/action.yaml
- .flox/env/manifest.toml
| corepack enable | ||
|
|
||
| package_manager="$(node -p "require('./web/package.json').packageManager")" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- manifest ---'
cat -n tools/nodejs/.flox/env/manifest.toml
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(README|CONTRIBUTING|.*flox.*|.*manifest.*|package\.json)$' | head -200
printf '%s\n' '--- working-directory and activation references ---'
rg -n --hidden -g '!node_modules' -g '!dist' -g '!build' \
'tools/nodejs|flox activate|flox\.env|packageManager|pnpm install|corepack enable|repo(root|sitory)' \
README* .github tools 2>/dev/null | head -300Repository: NVIDIA-NeMo/nemo-platform
Length of output: 14013
🌐 Web query:
Flox manifest on-activate hook current working directory flox activate project environment
💡 Result:
When you run flox activate, the on-activate hook executes in the current working directory from which you initiated the command [1][2][3]. Key details regarding the on-activate hook and directory context: 1. Working Directory: The hook runs in the shell environment where flox activate is invoked [1][2]. If you are in a specific project directory when you run the command, your shell will be in that directory, and relative paths used within the on-activate script will resolve against that current working directory [4][5]. 2. Project Directory Variable: Flox automatically sets the environment variable $FLOX_ENV_PROJECT [6][7]. This variable points to the directory of the project using the Flox environment [6][7]. For environments stored locally, this is the directory containing the environment (the folder where the.flox manifest is located) [6][7]. When using flox activate -r, $FLOX_ENV_PROJECT is explicitly set to the current working directory [6][7]. You can use this variable within your on-activate hook to reliably reference project files regardless of where the command was run from [6][7]. 3. Hook Execution: The on-activate script is sourced from a Bash shell as part of the environment activation process [1][2][3]. Because it is sourced in the shell context, any changes it makes to the current working directory—or environment variables like PATH—persist for the activated shell session [1][2][8]. It is recommended to keep on-activate scripts idempotent, as they may be triggered multiple times depending on the activation method [1].
Citations:
- 1: https://flox.dev/docs/man/manifest.toml
- 2: https://flox.dev/docs/concepts/activation
- 3: https://flox.mintlify.app/concepts/activation
- 4: https://flox.mintlify.app/tutorials/customizing-environments
- 5: https://flox.dev/docs/tutorials/customizing-environments
- 6: https://flox.dev/docs/man/flox-activate
- 7: https://flox.mintlify.app/man/flox-activate
- 8: https://github.com/PostHog/posthog/blob/723d2a3f/.flox/env/on-activate.sh
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- nodejs environment metadata ---'
cat -n tools/nodejs/.flox/env.json
printf '%s\n' '--- sibling activation hooks ---'
for f in tools/*/.flox/env/manifest.toml; do
if rg -q 'on-activate|pnpm install|package\.json' "$f"; then
echo "--- $f"
cat -n "$f" | sed -n '1,100p'
fi
done
printf '%s\n' '--- references to the nodejs Flox environment ---'
rg -n --hidden -g '!node_modules' -g '!dist' -g '!build' \
'tools/nodejs|nodejs.*flox|flox.*nodejs|flox activate' . 2>/dev/null | head -300Repository: NVIDIA-NeMo/nemo-platform
Length of output: 2766
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- contribution instructions ---'
cat -n CONTRIBUTING.md | sed -n '1,70p'
printf '%s\n' '--- root Flox manifest ---'
cat -n .flox/env/manifest.toml | sed -n '1,80p'
printf '%s\n' '--- root manifest references to nodejs ---'
rg -n -C 5 'tools/nodejs|nodejs' .flox/env/manifest.toml .flox/env/manifest.lock | head -160Repository: NVIDIA-NeMo/nemo-platform
Length of output: 5367
Resolve the repository root before installing web dependencies.
The documented root activation works, but direct activation from tools/nodejs resolves ./web/package.json and -C "./web" under tools/nodejs, so the hook fails. Resolve the repository root explicitly before these commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/nodejs/.flox/env/manifest.toml` around lines 20 - 22, Update the
manifest activation hook around package_manager and the web dependency commands
to resolve the repository root explicitly before accessing web/package.json or
using -C ./web. Ensure direct activation from tools/nodejs and documented root
activation both target the repository’s actual web directory.
Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: shuffle the dirs to actual paths Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: adding more includes Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: update OPA to pull from flox managed binaries Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: update action to match new inputs Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: can only run one command Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: disable flox metrics collection Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: update job name to match test Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: move nmp install to hook FLOX_ENV_PROJECT env var does not reflect the current path w/ layered environments Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: pass in disable metrics as inputs, global envvar didnt seem to pick that up Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: remove old opa install and build action replaced by flox Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: use flox env for lint Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: add stdlib for python Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: add libpath for gcc Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: add more libs Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: update to python3full for glibc compat Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: update parent flox env Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: add unwrapped for stdlibc++ Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: attempt gcc Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: add glibc Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: remove conflicting pkg Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: swap gcc for unwrapped w/ glibc Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: ditch nix python3, use uv distro Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: add start/complete msgs in activate scripts Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: create new env for lint tooling, add common env Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: need osv scanner for linting Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: allow nodejs to be activated from different compositions Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: remove flox project paths to better resolve relative path for multiple includes" Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: move to pinned action checkout Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> ci: fix action pin and whitespaces Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: add helm-docs to tools Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: move helm-docs to local binary in path, supplied by flox common tools Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> fix: pin helm-docs Signed-off-by: Philip Mattingly <pmattingly@nvidia.com> chore: restructure the flox setup, test on different shells and add notes Signed-off-by: Brooke Storm <brookes@nvidia.com> fix: correct the helm lint for the new setup Signed-off-by: Brooke Storm <brookes@nvidia.com> fix: clean up lint stuff Signed-off-by: Brooke Storm <brookes@nvidia.com> fix: add missing file Signed-off-by: Brooke Storm <brookes@nvidia.com> fix: pin osv scanner Signed-off-by: Brooke Storm <brookes@nvidia.com> docs: simple wording change...both are true about flox exit Signed-off-by: Brooke Storm <brookes@nvidia.com>
Signed-off-by: Philip Mattingly <pmattingly@nvidia.com>
Signed-off-by: Philip Mattingly <pmattingly@nvidia.com>
Signed-off-by: Philip Mattingly <pmattingly@nvidia.com>
Signed-off-by: Philip Mattingly <pmattingly@nvidia.com>
Signed-off-by: Philip Mattingly <pmattingly@nvidia.com>
05d3e90 to
e56517b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tools/osv-scanner/.flox/env/manifest.toml (1)
4-4: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDocument the OSV-Scanner catalog limitation.
Upstream
v2.4.0is available, but the Flox/Nixpkgs catalog still provides2.3.3. Document this limitation and keep the manifest unchanged until the catalog includes a newer version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/osv-scanner/.flox/env/manifest.toml` at line 4, Document the OSV-Scanner catalog limitation near the osv-scanner dependency declaration, noting that upstream v2.4.0 is available but the Flox/Nixpkgs catalog only provides v2.3.3; keep the existing manifest entry unchanged until a newer catalog version is available.
🤖 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 `@tools/lint/check-uv-version.sh`:
- Line 12: Update the diagnostic output in the version-check flow around
_python_ver and _flox_ver so it does not depend on an unavailable column
executable, or declare the package providing column in the relevant Flox
manifest. Ensure the mismatch diagnostic still prints both versions when column
is absent from PATH.
- Around line 7-10: Update the version extraction and validation in the script
around _python_ver and _flox_ver: use a quoted or bracketed yq path for
required-version, then reject either null or empty values before the comparison
so missing UV constraints cannot pass. Preserve the existing mismatch check for
valid, non-empty versions.
---
Nitpick comments:
In `@tools/osv-scanner/.flox/env/manifest.toml`:
- Line 4: Document the OSV-Scanner catalog limitation near the osv-scanner
dependency declaration, noting that upstream v2.4.0 is available but the
Flox/Nixpkgs catalog only provides v2.3.3; keep the existing manifest entry
unchanged until a newer catalog version is available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f79a897e-0fb6-41a0-abe6-4a8209378fcf
⛔ Files ignored due to path filters (8)
.flox/env/manifest.lockis excluded by!**/*.locktools/actionlint/.flox/env/manifest.lockis excluded by!**/*.locktools/common/.flox/env/manifest.lockis excluded by!**/*.locktools/helm/.flox/env/manifest.lockis excluded by!**/*.locktools/nodejs/.flox/env/manifest.lockis excluded by!**/*.locktools/open-policy-agent/.flox/env/manifest.lockis excluded by!**/*.locktools/osv-scanner/.flox/env/manifest.lockis excluded by!**/*.locktools/python/.flox/env/manifest.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.flox/env/manifest.toml.github/actions/build-nemo-platform-wheel/action.yaml.github/actions/build-policy-wasm/action.yaml.github/actions/changes/action.yaml.github/workflows/ci.yaml.pre-commit-config.yamlCONTRIBUTING.mdpyproject.tomltools/actionlint/.flox/.gitattributestools/actionlint/.flox/.gitignoretools/actionlint/.flox/env.jsontools/actionlint/.flox/env/manifest.tomltools/common/.flox/.gitattributestools/common/.flox/.gitignoretools/common/.flox/env.jsontools/common/.flox/env/manifest.tomltools/helm/.flox/.gitattributestools/helm/.flox/.gitignoretools/helm/.flox/env.jsontools/helm/.flox/env/manifest.tomltools/lint/check-uv-version.shtools/lint/check_uv_version.pytools/lint/lint-all.shtools/nodejs/.flox/.gitattributestools/nodejs/.flox/.gitignoretools/nodejs/.flox/env.jsontools/nodejs/.flox/env/manifest.tomltools/open-policy-agent/.flox/.gitattributestools/open-policy-agent/.flox/.gitignoretools/open-policy-agent/.flox/env.jsontools/open-policy-agent/.flox/env/manifest.tomltools/osv-scanner/.flox/.gitattributestools/osv-scanner/.flox/.gitignoretools/osv-scanner/.flox/env.jsontools/osv-scanner/.flox/env/manifest.tomltools/python/.flox/.gitattributestools/python/.flox/.gitignoretools/python/.flox/env.jsontools/python/.flox/env/manifest.toml
🚧 Files skipped from review as they are similar to previous changes (13)
- tools/nodejs/.flox/env.json
- tools/nodejs/.flox/.gitattributes
- tools/nodejs/.flox/env/manifest.toml
- tools/nodejs/.flox/.gitignore
- pyproject.toml
- tools/lint/lint-all.sh
- .github/actions/build-nemo-platform-wheel/action.yaml
- .github/actions/changes/action.yaml
- .github/actions/build-policy-wasm/action.yaml
- CONTRIBUTING.md
- .pre-commit-config.yaml
- tools/lint/check_uv_version.py
- .flox/env/manifest.toml
| _python_ver="$(yq '.tool.uv.required-version' pyproject.toml)" | ||
| _flox_ver="$(yq '.install.uv.version' tools/python/.flox/env/manifest.toml)" | ||
|
|
||
| if [[ "${_python_ver}" != "${_flox_ver}" ]]; then |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat >"$tmpdir/pyproject.toml" <<'EOF'
[tool.uv]
required-version = ">=0.9.14,<0.10.0"
EOF
expected='>=0.9.14,<0.10.0'
actual="$(yq '.tool.uv."required-version"' "$tmpdir/pyproject.toml")"
[[ "$actual" == "$expected" ]]
printf '[tool.uv]\n' >"$tmpdir/missing.toml"
missing="$(yq '.tool.uv."required-version"' "$tmpdir/missing.toml")"
[[ "$missing" == "null" ]]Repository: NVIDIA-NeMo/nemo-platform
Length of output: 204
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- checker ---'
cat -n tools/lint/check-uv-version.sh
printf '%s\n' '--- yq references and pins ---'
rg -n -S '(^|[^[:alnum:]_-])yq([^[:alnum:]_-]|$)|YQ_VERSION|mikefarah/yq|pip.*yq' . \
-g '!**/node_modules/**' -g '!**/.git/**' | head -200
printf '%s\n' '--- relevant configuration files ---'
git ls-files | rg '(^|/)(pyproject\.toml|.*flox.*manifest.*|.*ya?ml|.*Dockerfile.*|.*lock.*)$' | head -200Repository: NVIDIA-NeMo/nemo-platform
Length of output: 9526
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- root constraints ---'
rg -n -C 3 'required-version|^\[tool\.uv\]|\.install\.uv|check-uv-version|column|yq' \
pyproject.toml tools .flox .github .pre-commit-config.yaml 2>/dev/null | head -240
printf '%s\n' '--- source documents ---'
sed -n '1,80p' pyproject.toml
sed -n '1,100p' tools/python/.flox/env/manifest.toml
printf '%s\n' '--- exact TOML values using Python stdlib ---'
python3 - <<'PY'
import tomllib
from pathlib import Path
for name in ("pyproject.toml", "tools/python/.flox/env/manifest.toml"):
data = tomllib.loads(Path(name).read_text())
print(name)
if name == "pyproject.toml":
print(repr(data.get("tool", {}).get("uv", {}).get("required-version")))
else:
print(repr(data.get("install", {}).get("uv", {}).get("version")))
PY
printf '%s\n' '--- caller references ---'
rg -n -C 4 'check-uv-version\.sh|tools/lint' .github .flox .pre-commit-config.yaml Makefile* 2>/dev/null | head -240Repository: NVIDIA-NeMo/nemo-platform
Length of output: 22868
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- root Flox packages ---'
cat -n .flox/env/manifest.toml | sed -n '1,130p'
printf '%s\n' '--- lock entries for yq and column ---'
rg -n -C 5 '"(yq-go|util-linux|column|uutils-coreutils|coreutils)"|yq-go-[0-9]|column' \
.flox/env/manifest.lock | head -180
printf '%s\n' '--- tool availability in the sandbox ---'
for tool in yq column jq ruby perl; do
if command -v "$tool" >/dev/null 2>&1; then
printf '%s: %s\n' "$tool" "$(command -v "$tool")"
"$tool" --version 2>&1 | head -1 || true
else
printf '%s: unavailable\n' "$tool"
fi
doneRepository: NVIDIA-NeMo/nemo-platform
Length of output: 4736
🌐 Web query:
mikefarah yq v4.53.3 hyphenated key path expression required-version missing key null TOML
💡 Result:
In yq v4.53.3, accessing hyphenated keys or keys containing special characters (such as dots) in a path expression requires using bracket notation with double quotes, such as.["hyphenated-key"] or.["key.with.dots"] [1]. Regarding your note on TOML and "null" or "missing key" issues: 1. Handling Special Characters: If your TOML keys contain special characters (like hyphens or colons), newer versions of yq have included fixes (e.g., in PR #2648) to ensure the TOML encoder properly quotes these keys, preventing invalid TOML output that would fail to re-parse [2][3]. 2. Version Issues: If you are encountering mangled TOML output or unexpected "null" values during processing, ensure you are using at least v4.53.3. This version specifically addresses multiple TOML-related bugs, including issues where empty arrays in tables were omitted [4], and fixes for TOML decoder state management that caused incorrect evaluation or mangled output in multi-document or complex TOML files [5][6]. 3. Best Practice: If a key is not being found or results in "null", verify your path syntax. Use .["key-name"] rather than relying on dot-notation when the key contains characters that are not valid in standard identifiers or that conflict with yq's internal path parsing [7][1]. If you are performing updates (e.g., -i or -o toml), ensuring your yq version is up to date is critical as TOML support has received significant stability improvements in the 4.53.x release cycle [5][8].
Citations:
- 1: Unable to read key with period in v4 mikefarah/yq#1092
- 2: Fix TOML encoder to quote keys containing special characters mikefarah/yq#2648
- 3: 转换为toml文件时如果key包含特殊字符会由于没有加引号导致二次解析失败 mikefarah/yq#2617
- 4: TOML decoding omits empty array inside table. mikefarah/yq#2674
- 5: https://github.com/mikefarah/yq/releases/tag/v4.53.3
- 6: yq with TOML in GitHub Actions results in mangled output mikefarah/yq#2710
- 7: https://mikefarah.gitbook.io/yq/v3.x/usage/path-expressions.md
- 8: https://raw.githubusercontent.com/mikefarah/yq/master/release_notes.txt
🌐 Web query:
site:mikefarah.gitbook.io/yq v4 path expressions hyphen key .["required-version"] missing key null
💡 Result:
In yq version 4, when attempting to access a key that contains special characters—such as a hyphen—or when a requested key does not exist in the YAML/JSON structure, specific syntax must be used to ensure the expression is evaluated correctly [1][2][3]. 1. Keys with Hyphens: If a key name contains a hyphen (e.g., "required-version"), it cannot be accessed using standard dot notation (e.g.,.required-version) because the hyphen is interpreted as a subtraction operator. Instead, you must use the bracketed string lookup syntax [1]:.["required-version"] 2. Missing Keys and Null Values: In yq v4, if a path expression does not find a matching key, it returns nothing (or null) [3][4]. If you need to handle missing keys by providing a default value instead of receiving an empty response, use the alternative operator (//) [3][4]:.["required-version"] // "default-value" This approach ensures that if.["required-version"] evaluates to null or is missing, the expression will return the provided default value rather than terminating with no output [3][4].
Citations:
- 1: https://mikefarah.gitbook.io/yq/v3.x/usage/path-expressions.md
- 2: https://mikefarah.gitbook.io/yq/operators/traverse-read
- 3: https://mikefarah.gitbook.io/yq/upgrading-from-v3
- 4: https://mikefarah.gitbook.io/yq/upgrading-from-v3.md
🌐 Web query:
site:github.com/mikefarah/yq "required-version" "yq" expression
💡 Result:
There is no direct command-line flag or specific yq expression called "required-version" [1][2][3]. The command yq does not have a built-in feature to enforce a minimum version requirement for its expressions [2][3]. If you need to ensure a specific version of yq is used for a script or expression, the standard approach is to verify the version at the start of your script using the --version flag [1][3][4]. You can check the version of the installed yq binary with: yq --version To programmatically enforce a minimum version (e.g., version 4.50.0 or higher) in a shell script, you can use a comparison approach: # Example: Enforce yq version >= 4.50.0 REQUIRED_VERSION="4.50.0" CURRENT_VERSION=$(yq --version | sed 's/yq version //') if [ "$(printf '%s\n' "$REQUIRED_VERSION" "$CURRENT_VERSION" | sort -V | head -n1)"!= "$REQUIRED_VERSION" ]; then echo "Error: yq version $REQUIRED_VERSION or higher is required." exit 1 fi For authoritative and up-to-date technical usage, refer to the official yq documentation [5].
Citations:
- 1: https://github.com/mikefarah/yq/?tab=readme-ov-file
- 2: https://github.com/mikefarah/yq/blob/master/README.md
- 3: https://github.com/mikefarah/yq
- 4: https://github.com/mikefarah/yq/tree/v4.52.1
- 5: https://github.com/mikefarah/yq/blob/master/README.md?plain=1
🌐 Web query:
site:github.com/mikefarah/yq missing key output null yq eval
💡 Result:
In yq, when you attempt to access a key that does not exist in your YAML data, the default behavior is often to return null or create the key with a null value depending on the context [1][2][3]. To manage or avoid null outputs, consider the following strategies: 1. Use the Alternative (Default) Operator: The // operator provides a fallback value when the left-hand side is null or false [4]. This is the most common way to replace null with a specific value (e.g., an empty string) [1]. Example: yq '.key // ""' file.yaml [1] 2. Filter Out Nulls: If you want to prevent null results from appearing in your output, use select to filter them out [3][5]. Example: yq '.key | select(. != null)' file.yaml 3. Control Exit Status with -e: If you need yq to fail (return a non-zero exit code) when a key is missing or results in null, use the --exit-status (or -e) flag [2][6][7]. Note that this flag triggers if the final result is null, false, or empty [6][5]. Example: yq -e '.key' file.yaml [2][8] 4. Avoid Autocreation: yq can sometimes auto-create keys if you traverse into them [3]. If you are performing operations that inadvertently trigger this (like to_entries), use select(has("key")) to ensure the key exists before processing it [3]. If you are experiencing unexpected null values within complex expressions (such as inside select or and/or conditions), this may be due to known behavior where missing keys are handled differently depending on the context [9]. Always ensure your path exists or use the alternative operator to handle potential missing values explicitly [4].
Citations:
- 1: How to get an empty string instead of
nullfor non existent value? mikefarah/yq#1471 - 2: Don't print 'null' to stdout for no-match mikefarah/yq#235
- 3: Accessing missing key in variable breaks to_entries mikefarah/yq#1094
- 4: https://github.com/mikefarah/yq/blob/v4.53.2/pkg/yqlib/doc/operators/alternative-default-value.md
- 5: Errors get ignored mikefarah/yq#1220
- 6: https://github.com/mikefarah/yq
- 7: https://github.com/mikefarah/yq/blob/master/README.md?plain=1
- 8: length does not error when evaluating a non-existing element mikefarah/yq#683
- 9: Missing keys are dropped from [...] inside select/and/or mikefarah/yq#2782
Fail closed on missing UV constraints. Use a quoted or bracketed yq path for required-version, then reject either null or empty result before comparing. Otherwise, missing constraints can compare equal and pass the check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/lint/check-uv-version.sh` around lines 7 - 10, Update the version
extraction and validation in the script around _python_ver and _flox_ver: use a
quoted or bracketed yq path for required-version, then reject either null or
empty values before the comparison so missing UV constraints cannot pass.
Preserve the existing mismatch check for valid, non-empty versions.
|
|
||
| if [[ "${_python_ver}" != "${_flox_ver}" ]]; then | ||
| printf "uv version constraints differ:\n\n" | ||
| printf "pyproject.toml flox-env\n%s %s\n" "${_python_ver}" "${_flox_ver}" | column -t |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
command -v column
rg -n 'column|util-linux' --glob 'manifest.toml' .Repository: NVIDIA-NeMo/nemo-platform
Length of output: 163
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- target script ---'
cat -n tools/lint/check-uv-version.sh
printf '%s\n' '--- common Flox manifest ---'
cat -n tools/common/.flox/env/manifest.toml
printf '%s\n' '--- repository references to column and manifests ---'
rg -n -i '(^|[^[:alnum:]_])column([^[:alnum:]_]|$)|util-linux|yq-go' --glob 'manifest.toml' --glob '*.sh' .Repository: NVIDIA-NeMo/nemo-platform
Length of output: 1512
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- all Flox manifests ---'
find . -path '*/.flox/env/manifest.toml' -type f -print -exec sh -c 'echo "--- $1"; cat -n "$1"' sh {} \;
printf '%s\n' '--- invocations and environment setup ---'
rg -n 'check-uv-version|tools/common|flox (activate|install|env)|PATH=' .github tools Makefile* . 2>/dev/null | head -n 250
printf '%s\n' '--- package/config references ---'
rg -n -i 'util-linux|column|coreutils|common.*manifest|manifest\.toml' --glob '!*.lock' --glob '!*.sum' .Repository: NVIDIA-NeMo/nemo-platform
Length of output: 50382
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- tracked Flox manifests ---'
git ls-files | grep -E '(^|/)\.flox/env/manifest\.toml$' || true
printf '%s\n' '--- exact script references ---'
rg -n -C 3 'check-uv-version\.sh|tools/lint' --glob '!web/**' --glob '!services/**' --glob '!*.jsonl' . || true
printf '%s\n' '--- relevant manifest package declarations ---'
for f in $(git ls-files | grep -E '(^|/)\.flox/env/manifest\.toml$'); do
if grep -Eiq '(^|[^[:alnum:]_])(column|util-linux|yq-go|coreutils)([^[:alnum:]_]|$)' "$f"; then
echo "--- $f"
grep -Ein -C 2 'column|util-linux|yq-go|coreutils' "$f"
fi
doneRepository: NVIDIA-NeMo/nemo-platform
Length of output: 5515
🏁 Script executed:
#!/usr/bin/env bash
set -u
for f in .flox/env/manifest.toml tools/python/.flox/env/manifest.toml tools/common/.flox/env/manifest.toml; do
echo "--- $f"
cat -n "$f"
done
printf '%s\n' '--- shell environment and Flox activation documentation ---'
rg -n -i -C 3 'flox|common|activate|environment' README.md CONTRIBUTING.md docs .github tools Makefile 2>/dev/null | grep -E -i 'flox|common|activate|environment' | head -n 200Repository: NVIDIA-NeMo/nemo-platform
Length of output: 29163
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
import shutil
import tomllib
root = tomllib.loads(Path(".flox/env/manifest.toml").read_text())
included = [entry["dir"] for entry in root["include"]["environments"]]
common = tomllib.loads(Path("tools/common/.flox/env/manifest.toml").read_text())
python_env = tomllib.loads(Path("tools/python/.flox/env/manifest.toml").read_text())
script = Path("tools/lint/check-uv-version.sh").read_text()
print("root_includes_tools_common:", "tools/common" in included)
print("root_includes_tools_python:", "tools/python" in included)
print("common_declares_column_provider:",
any("column" in key.lower() or "util-linux" in str(value).lower()
for key, value in common.get("install", {}).items()))
print("common_declares_yq_go:", "yq-go" in common.get("install", {}))
print("python_declares_column_provider:",
any("column" in key.lower() or "util-linux" in str(value).lower()
for key, value in python_env.get("install", {}).items()))
print("script_uses_column:", "| column -t" in script)
print("host_column:", shutil.which("column"))
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 283
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
import re
import shutil
root = Path(".flox/env/manifest.toml").read_text()
common = Path("tools/common/.flox/env/manifest.toml").read_text()
python_env = Path("tools/python/.flox/env/manifest.toml").read_text()
script = Path("tools/lint/check-uv-version.sh").read_text()
included = re.findall(r'\{\s*dir\s*=\s*"([^"]+)"\s*\}', root)
declares_provider = lambda text: bool(re.search(r'(?im)^\s*(?:column|util-linux)\s*=|pkg-path\s*=\s*"util-linux"', text))
print("root_includes_tools_common:", "tools/common" in included)
print("root_includes_tools_python:", "tools/python" in included)
print("common_declares_yq_go:", bool(re.search(r'(?im)^\s*yq-go\s*=', common)))
print("common_declares_column_provider:", declares_provider(common))
print("python_declares_column_provider:", declares_provider(python_env))
print("script_uses_column:", "| column -t" in script)
print("host_column:", shutil.which("column"))
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 379
Keep the diagnostic dependency inside Flox. The root environment includes tools/common and tools/python, but neither manifest provides column. Add its provider or avoid column -t; otherwise, the mismatch branch fails when column is absent from PATH.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/lint/check-uv-version.sh` at line 12, Update the diagnostic output in
the version-check flow around _python_ver and _flox_ver so it does not depend on
an unavailable column executable, or declare the package providing column in the
relevant Flox manifest. Ensure the mismatch diagnostic still prints both
versions when column is absent from PATH.
…v to pull in osv scanner from flox Signed-off-by: Philip Mattingly <pmattingly@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
709-712: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the Flox CLI version used by CI.
The action is pinned, but the Flox binary is not. The pinned action defaults to the
stablechannel whenversionis empty. Pinversionto the manifest schema version,1.14.0, or add an explicit compatibility check. (github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yaml around lines 709 - 712, Update the “Install Flox” step using flox/install-flox-action to set its version input explicitly to the manifest schema version 1.14.0, while preserving the existing action pin and disable-metrics setting.
🤖 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.
Nitpick comments:
In @.github/workflows/ci.yaml:
- Around line 709-712: Update the “Install Flox” step using
flox/install-flox-action to set its version input explicitly to the manifest
schema version 1.14.0, while preserving the existing action pin and
disable-metrics setting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 961df7b6-0638-4a7f-8288-da578278e6b7
⛔ Files ignored due to path filters (1)
tools/lint/.flox/env/manifest.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/actions/build-policy-wasm/action.yaml.github/workflows/ci.yamltools/lint/.flox/.gitattributestools/lint/.flox/.gitignoretools/lint/.flox/env.jsontools/lint/.flox/env/manifest.toml
💤 Files with no reviewable changes (1)
- .github/actions/build-policy-wasm/action.yaml
Signed-off-by: Philip Mattingly <pmattingly@nvidia.com>
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation