fix(onboard): persist model and provider to sandbox registry after creation#1883
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA single call to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
TruongNguyenG
changed the title
TruongNguyenG
force-pushed
the
fix/registry-model-provider
branch
from
00eebfe to
28f1954
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/onboard.ts`:
- Line 5254: The call to registry.updateSandbox(sandboxName, { model, provider
}) can fail silently (return false) and leave the sandbox unpersisted; change
the code to check the boolean result of registry.updateSandbox and if it returns
false, call a fallback to create the entry (e.g., registry.createSandbox or
equivalent) with sandboxName and the same { model, provider } payload, and log
or surface any error from the fallback; reference registry.updateSandbox,
sandboxName, model, provider and the fallback creation method to locate and
implement the change.
🪄 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 Plus
Run ID: e6dc3a85-1c1f-4192-b4d4-46e515f32d40
📒 Files selected for processing (1)
src/lib/onboard.ts
| agent, | ||
| dangerouslySkipPermissions, | ||
| ); | ||
| registry.updateSandbox(sandboxName, { model, provider }); |
There was a problem hiding this comment.
Add a fallback when updateSandbox returns false.
Line 5254 reuses an API that can fail silently. If the sandbox is reused but its local registry entry is missing, model/provider still won’t persist.
Suggested patch
- registry.updateSandbox(sandboxName, { model, provider });
+ if (!registry.updateSandbox(sandboxName, { model, provider })) {
+ registry.registerSandbox({ name: sandboxName, model, provider });
+ }📝 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.
| registry.updateSandbox(sandboxName, { model, provider }); | |
| if (!registry.updateSandbox(sandboxName, { model, provider })) { | |
| registry.registerSandbox({ name: sandboxName, model, provider }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/onboard.ts` at line 5254, The call to
registry.updateSandbox(sandboxName, { model, provider }) can fail silently
(return false) and leave the sandbox unpersisted; change the code to check the
boolean result of registry.updateSandbox and if it returns false, call a
fallback to create the entry (e.g., registry.createSandbox or equivalent) with
sandboxName and the same { model, provider } payload, and log or surface any
error from the fallback; reference registry.updateSandbox, sandboxName, model,
provider and the fallback creation method to locate and implement the change.
…eation registry.updateSandbox() at Step 4 (inference setup) runs before the sandbox entry exists in the registry. updateSandbox silently returns false when the entry is not found (registry.ts line 177), so the model and provider are never saved. registerSandbox() at Step 6 (create sandbox) later creates the entry without model or provider. Add a registry.updateSandbox() call after createSandbox() returns, so the model and provider are written to the entry that now exists. Before: nemoclaw list shows model: unknown provider: unknown After: nemoclaw list shows model: nvidia/nemotron-3-super-120b-a12b provider: nvidia-prod Signed-off-by: Truong Nguyen <tgnguyen@nvidia.com>
TruongNguyenG
force-pushed
the
fix/registry-model-provider
branch
from
28f1954 to
c832a4c
Compare
|
Nice find — I hit the same root cause and opened #1884 for this yesterday. Same fix: I also added a source-order regression test in #1884 to prevent reintroduction. Might be worth checking if the approaches can be consolidated, or one can close in favor of the other. Either way, glad it's getting attention! |
|
@BenediktSchackenberg @ShevaNguyen merged both! thank you :) |
3 participants
Summary
Fix
nemoclaw listshowingmodel: unknown provider: unknownafter onboard.registry.updateSandbox()at Step 4 (inference setup) runs before the sandboxentry exists in the registry.
updateSandboxsilently returnsfalsewhen theentry is not found, so the model and provider are never saved.
registerSandbox()at Step 6 (create sandbox) later creates the entry withoutmodel or provider.
Add
registry.updateSandbox(sandboxName, { model, provider })aftercreateSandbox()returns, so the update runs after the entry exists.Related Issue
issue 1881
Changes
Type of Change
Testing
npx prek run --all-filespasses (or equivalentlymake check).npm testpasses.make docsbuilds without warnings. (for doc-only changes)Checklist
General
Code Changes
npx prek run --all-filesauto-fixes formatting (ormake formatfor targeted runs).Doc Changes
nemoclaw-contributor-update-docsagent skill to draft changes while complying with the style guide. For example, prompt your agent with "/nemoclaw-contributor-update-docscatch up the docs for the new changes I made in this PR."Signed-off-by: Truong Nguyen tgnguyen@nvidia.com
Summary by CodeRabbit