chore: add Prettier formatting for root-level JS files#1200
Conversation
- Add root .prettierrc matching nemoclaw/ plugin config - Add .prettierignore for build artifacts and non-JS files - Add prettier devDependency and format/format:check scripts to root package.json - Add prettier-js pre-commit hook for bin/ and test/ JS files (priority 5) - Extend Makefile format target with format-cli subtarget - Format all 45 JS files in bin/ and test/
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (8)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded Prettier formatting (config and ignore), a pre-commit hook, npm scripts, and a Makefile target; reformatted many JS source and test files (trailing commas, multiline expressions, try/catch formatting) with no behavioral changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bin/nemoclaw.js (1)
253-255: Complexity suppressions should be paid down in these touched command paths.These branch-heavy functions are still bypassing the complexity threshold; splitting state classification/recovery/message emission into helpers would align this file with repo limits.
As per coding guidelines "
**/*.{js,ts,tsx}: Maintain cyclomatic complexity limit of 20 (target: ratchet down to 15) for all functions."Also applies to: 438-439, 698-699
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/nemoclaw.js` around lines 253 - 255, The function getReconciledSandboxGatewayState is bypassing the cyclomatic complexity limit due to branching; split its responsibilities into smaller helpers: extract state classification logic into a classifySandboxGatewayState(sandboxName, rawState) helper, move recovery/repair branching into a recoverSandboxGatewayState(classifiedState) helper, and move message emission into emitSandboxGatewayStateEvents(recoveredState); update getReconciledSandboxGatewayState to orchestrate these three calls and return the final state. Apply the same decomposition pattern to the other branch-heavy command functions in this file so each top-level command becomes a thin orchestrator calling focused helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/onboard.js`:
- Around line 2749-2759: The code assigns the entire result of
validateOpenAiLikeSelection(...) to preferredInferenceApi but
validateOpenAiLikeSelection returns an object { ok, api }; change the assignment
to destructure the result (e.g., const { ok, api } = await
validateOpenAiLikeSelection(...)), then set preferredInferenceApi = api only
when ok is true, and keep the existing break and continue selectionLoop
behavior; reference validateOpenAiLikeSelection, preferredInferenceApi,
selectionLoop, and the call parameters (remoteConfig.label, endpointUrl, model,
credentialEnv, retryMessage) to locate and fix the code.
---
Nitpick comments:
In `@bin/nemoclaw.js`:
- Around line 253-255: The function getReconciledSandboxGatewayState is
bypassing the cyclomatic complexity limit due to branching; split its
responsibilities into smaller helpers: extract state classification logic into a
classifySandboxGatewayState(sandboxName, rawState) helper, move recovery/repair
branching into a recoverSandboxGatewayState(classifiedState) helper, and move
message emission into emitSandboxGatewayStateEvents(recoveredState); update
getReconciledSandboxGatewayState to orchestrate these three calls and return the
final state. Apply the same decomposition pattern to the other branch-heavy
command functions in this file so each top-level command becomes a thin
orchestrator calling focused helpers.
🪄 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: Pro
Run ID: b65119ae-0499-4316-b0e2-041f7e4686ab
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (47)
.pre-commit-config.yaml.prettierignore.prettierrcMakefilebin/lib/credentials.jsbin/lib/local-inference.jsbin/lib/nim.jsbin/lib/onboard-session.jsbin/lib/onboard.jsbin/lib/policies.jsbin/lib/preflight.jsbin/lib/registry.jsbin/lib/resolve-openshell.jsbin/lib/runner.jsbin/lib/runtime-recovery.jsbin/nemoclaw.jspackage.jsontest/cli.test.jstest/credential-exposure.test.jstest/credentials.test.jstest/dns-proxy.test.jstest/e2e/brev-e2e.test.jstest/gateway-cleanup.test.jstest/inference-config.test.jstest/install-preflight.test.jstest/local-inference.test.jstest/nemoclaw-cli-recovery.test.jstest/nemoclaw-start.test.jstest/nim.test.jstest/onboard-readiness.test.jstest/onboard-selection.test.jstest/onboard-session.test.jstest/onboard.test.jstest/platform.test.jstest/policies.test.jstest/preflight.test.jstest/registry.test.jstest/runner.test.jstest/runtime-recovery.test.jstest/runtime-shell.test.jstest/security-binaries-restriction.test.jstest/security-c2-dockerfile-injection.test.jstest/security-c4-manifest-traversal.test.jstest/security-method-wildcards.test.jstest/service-env.test.jstest/setup-sandbox-name.test.jstest/uninstall.test.js
| preferredInferenceApi = await validateOpenAiLikeSelection( | ||
| remoteConfig.label, | ||
| endpointUrl, | ||
| model, | ||
| credentialEnv, | ||
| retryMessage, | ||
| remoteConfig.helpUrl | ||
| ); | ||
| if (preferredInferenceApi) { | ||
| break; | ||
| } | ||
| continue selectionLoop; |
There was a problem hiding this comment.
Critical: Bedrock validation result is assigned incorrectly to preferredInferenceApi.
validateOpenAiLikeSelection(...) returns an object ({ ok, api }), but this block assigns the entire object to preferredInferenceApi and only checks truthiness. That can store a non-string object and corrupt downstream inference API config.
🐛 Suggested fix
- preferredInferenceApi = await validateOpenAiLikeSelection(
+ const validation = await validateOpenAiLikeSelection(
remoteConfig.label,
endpointUrl,
model,
credentialEnv,
retryMessage,
);
- if (preferredInferenceApi) {
+ if (validation.ok) {
+ preferredInferenceApi = validation.api;
break;
}
+ if (validation.retry === "credential" || validation.retry === "retry") {
+ continue;
+ }
continue selectionLoop;📝 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.
| preferredInferenceApi = await validateOpenAiLikeSelection( | |
| remoteConfig.label, | |
| endpointUrl, | |
| model, | |
| credentialEnv, | |
| retryMessage, | |
| remoteConfig.helpUrl | |
| ); | |
| if (preferredInferenceApi) { | |
| break; | |
| } | |
| continue selectionLoop; | |
| const validation = await validateOpenAiLikeSelection( | |
| remoteConfig.label, | |
| endpointUrl, | |
| model, | |
| credentialEnv, | |
| retryMessage, | |
| ); | |
| if (validation.ok) { | |
| preferredInferenceApi = validation.api; | |
| break; | |
| } | |
| if (validation.retry === "credential" || validation.retry === "retry") { | |
| continue; | |
| } | |
| continue selectionLoop; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/lib/onboard.js` around lines 2749 - 2759, The code assigns the entire
result of validateOpenAiLikeSelection(...) to preferredInferenceApi but
validateOpenAiLikeSelection returns an object { ok, api }; change the assignment
to destructure the result (e.g., const { ok, api } = await
validateOpenAiLikeSelection(...)), then set preferredInferenceApi = api only
when ok is true, and keep the existing break and continue selectionLoop
behavior; reference validateOpenAiLikeSelection, preferredInferenceApi,
selectionLoop, and the call parameters (remoteConfig.label, endpointUrl, model,
credentialEnv, retryMessage) to locate and fix the code.
ericksoa
left a comment
There was a problem hiding this comment.
LGTM. Prettier config matches the existing plugin style, .prettierignore is sensible, and the pre-commit hook is wired in at the right priority. Thanks for extending formatting coverage to the CLI and test code.
Merge origin/main to resolve conflicts from recent changes: - NVIDIA#1208 core blocker lifecycle regressions - NVIDIA#1200 Prettier formatting - NVIDIA#836 GPU VRAM checks Jetson detection now leverages main's UNIFIED_MEMORY_GPU_TAGS (Orin/Thor/Xavier) with added jetson flag and /proc/device-tree fallback. All 118 tests pass.
Merge origin/main into feat/jetson-orin-nano-support to resolve conflicts from recent changes (NVIDIA#1208, NVIDIA#1200, NVIDIA#836, NVIDIA#1221, NVIDIA#1223). Jetson detection now leverages main's UNIFIED_MEMORY_GPU_TAGS with added jetson flag and /proc/device-tree fallback. All 116 tests pass.
## Summary Extends the existing Prettier setup (which only covered `nemoclaw/src/**/*.ts`) to also format the root-level `bin/` and `test/` JavaScript files. ## Changes | File | What | |------|------| | `.prettierrc` | New root config — matches the `nemoclaw/` plugin style (double quotes, semicolons, 100-char print width, trailing commas) | | `.prettierignore` | Ignores `node_modules/`, `dist/`, `coverage/`, blueprint YAML, built docs, and `.md` files | | `package.json` | Added `prettier` devDependency, `format` and `format:check` scripts | | `.pre-commit-config.yaml` | Added `prettier-js` hook at priority 5 for `bin/` and `test/` JS files | | `Makefile` | `make format` now includes a `format-cli` subtarget | | 43 JS files | Reformatted to match project style | ## Validation - `npx prettier --check 'bin/**/*.js' 'test/**/*.js'` — ✅ all clean - `npm test` — ✅ 677 tests pass, 3 skipped - All pre-commit hooks pass (including new `Prettier (JavaScript)` hook) - All pre-push hooks pass (tsc, coverage ratchet) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Chores** * Added standardized code formatting configuration with Prettier * Introduced npm scripts for code formatting and format verification * Enabled pre-commit hooks to automatically format code * Applied consistent formatting throughout the codebase * **Tests** * Updated test files to adhere to new formatting standards <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Extends the existing Prettier setup (which only covered `nemoclaw/src/**/*.ts`) to also format the root-level `bin/` and `test/` JavaScript files. ## Changes | File | What | |------|------| | `.prettierrc` | New root config — matches the `nemoclaw/` plugin style (double quotes, semicolons, 100-char print width, trailing commas) | | `.prettierignore` | Ignores `node_modules/`, `dist/`, `coverage/`, blueprint YAML, built docs, and `.md` files | | `package.json` | Added `prettier` devDependency, `format` and `format:check` scripts | | `.pre-commit-config.yaml` | Added `prettier-js` hook at priority 5 for `bin/` and `test/` JS files | | `Makefile` | `make format` now includes a `format-cli` subtarget | | 43 JS files | Reformatted to match project style | ## Validation - `npx prettier --check 'bin/**/*.js' 'test/**/*.js'` — ✅ all clean - `npm test` — ✅ 677 tests pass, 3 skipped - All pre-commit hooks pass (including new `Prettier (JavaScript)` hook) - All pre-push hooks pass (tsc, coverage ratchet) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Chores** * Added standardized code formatting configuration with Prettier * Introduced npm scripts for code formatting and format verification * Enabled pre-commit hooks to automatically format code * Applied consistent formatting throughout the codebase * **Tests** * Updated test files to adhere to new formatting standards <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Extends the existing Prettier setup (which only covered
nemoclaw/src/**/*.ts) to also format the root-levelbin/andtest/JavaScript files.Changes
.prettierrcnemoclaw/plugin style (double quotes, semicolons, 100-char print width, trailing commas).prettierignorenode_modules/,dist/,coverage/, blueprint YAML, built docs, and.mdfilespackage.jsonprettierdevDependency,formatandformat:checkscripts.pre-commit-config.yamlprettier-jshook at priority 5 forbin/andtest/JS filesMakefilemake formatnow includes aformat-clisubtargetValidation
npx prettier --check 'bin/**/*.js' 'test/**/*.js'— ✅ all cleannpm test— ✅ 677 tests pass, 3 skippedPrettier (JavaScript)hook)Summary by CodeRabbit
Chores
Tests