fix(deepagents-code): report onboard upstream provider in Deep Code TUI#6462
Conversation
Managed inference always routes through the OpenAI-compatible adapter, so the Deep Code TUI status bar and the model-identity system prompt reported the wire provider "openai" instead of the onboard-selected upstream. Show the upstream provider (from NEMOCLAW_UPSTREAM_PROVIDER) at those surfaces while the adapter key stays "openai" for routing. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds managed provider display handling for Deep Agents Code, applies it to agent/status/welcome patching, and updates fixtures and tests to cover the new provider-label behavior. ChangesManaged provider label override
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Patcher as patch-managed-deepagents-code.py
participant Runtime as managed_display_provider
participant TUI as StatusBar/WelcomeBanner
participant Agent as build_model_identity_section
Patcher->>Runtime: inject display-provider rewrite hooks
Agent->>Runtime: resolve provider for model identity text
TUI->>Runtime: resolve provider for status and welcome text
Runtime-->>Agent: mapped or original provider label
Runtime-->>TUI: mapped or original provider label
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the branch is 96%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the branch is 76%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/helpers/langchain-deepagents-code-patch-fixture.ts (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo cleanup for generated fixture temp directories.
createPackageFixture()creates a newmkdtempSyncdirectory on every call but nothing removes it afterward. This helper is now shared across the direct-module-patch, progressive-disclosure-adjacent, and provider-label tests, so every test run accumulates additional temp directories under the OS tmp dir with no teardown.Consider returning a disposer (or exporting a
cleanupFixture(tempDir)helper) that callers invoke inafterEach/finally.♻️ Suggested cleanup helper
export function createPackageFixture(version = "0.1.34"): string { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-dcode-patch-")); ... return tempDir; } + +export function cleanupFixture(tempDir: string): void { + fs.rmSync(tempDir, { recursive: true, force: true }); +}Also applies to: 604-605
🤖 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 `@test/helpers/langchain-deepagents-code-patch-fixture.ts` around lines 20 - 22, The createPackageFixture helper is leaking temporary directories because it calls mkdtempSync but never removes the generated tempDir. Update createPackageFixture (and any shared callers in the direct-module-patch, progressive-disclosure-adjacent, and provider-label tests) to return or expose a disposer such as cleanupFixture(tempDir), then invoke it in afterEach/finally so the temp directory is always deleted after use.
🤖 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 `@agents/langchain-deepagents-code/managed-dcode-runtime.py`:
- Around line 872-878: The provider selection logic in managed-dcode-runtime.py
is too broad: the upstream display override in the adapter selection block is
applied to any provider string, which can relabel non-managed providers.
Restrict the substitution to the managed OpenAI-compatible adapter by checking
the adapter value in the same path that uses adapter_provider and
_UPSTREAM_PROVIDER_ENV, and only return upstream when the wire adapter matches
the specific managed adapter this fix targets; otherwise keep returning the
original adapter.
In `@agents/langchain-deepagents-code/patch-managed-deepagents-code.py`:
- Around line 1116-1117: The fast path in patch-managed-deepagents-code.py only
checks AGENT_PATCH, so partial or corrupt edits in status.py or welcome.py can
still be treated as fully patched. Update the early-return logic around the
marker_states check to verify the new UI patches explicitly, using the status
and welcome marker_states entries in addition to AGENT_PATCH before returning.
Keep the fix localized to the patch detection flow so the existing provider
label fix is only considered active when all required markers are present.
---
Nitpick comments:
In `@test/helpers/langchain-deepagents-code-patch-fixture.ts`:
- Around line 20-22: The createPackageFixture helper is leaking temporary
directories because it calls mkdtempSync but never removes the generated
tempDir. Update createPackageFixture (and any shared callers in the
direct-module-patch, progressive-disclosure-adjacent, and provider-label tests)
to return or expose a disposer such as cleanupFixture(tempDir), then invoke it
in afterEach/finally so the temp directory is always deleted after use.
🪄 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: 94e66c93-df1a-4551-9e4e-2912c73dbe35
📒 Files selected for processing (6)
agents/langchain-deepagents-code/managed-dcode-runtime.pyagents/langchain-deepagents-code/patch-managed-deepagents-code.pytest/helpers/langchain-deepagents-code-patch-fixture.tstest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-progressive-tool-disclosure.test.tstest/langchain-deepagents-code-provider-label.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
E2E Target Results — ✅ All selected jobs passedRun: 28944667744
|
|
Maintainer sequencing override for Nemotron Ultra PR #6462 is intentionally sequenced before draft PR #6341. A current-main merge-tree audit shows that their changes to The remaining Ultra findings were reviewed as follows:
This explicitly overrides the #6341 sequencing recommendation with rationale; it does not make #6462 depend on #6341. |
<!-- markdownlint-disable MD041 --> ## Summary Adds the v0.0.77 release-note section from the shipped release announcement and release commit range. This is post-release docs recovery, so the PR is labeled for the next patch release train. ## Changes - Added `v0.0.77` to `docs/about/release-notes.mdx` with links to the deeper Deep Agents, architecture, inference, security, and agent-docs pages. - Source summary: - #6469 -> `docs/about/release-notes.mdx`: Documents Deep Agents Code base-image publication and stale-version validation. - #6471 -> `docs/about/release-notes.mdx`: Documents the managed runtime disabling LangGraph CLI analytics. - #6462 -> `docs/about/release-notes.mdx`: Documents the TUI, launch banner, and model-identity provider display behavior. - #6460 -> `docs/about/release-notes.mdx`: Documents bounded, best-effort OTLP trace credential scrubbing and the remaining collector-side redaction requirement. - #6423 -> `docs/about/release-notes.mdx`: Documents the checked-in loopback-only local credential form used by starter prompts. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: release-note prose only, no runtime behavior or code samples changed. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: tests not applicable for release-note prose only. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Verification note: `npm run docs` passed. `fern check --warnings` reports the existing light-mode accent color contrast warning: `2.41:1`, expected at least `3:1`. --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new release-notes entry for **v0.0.77** at the top of the changelog. * Highlighted improved package validation, tighter handling of telemetry and trace data, and safer starter prompt behavior with stronger redaction and local-only submission. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…UI (NVIDIA#6462) ## Summary Managed Deep Code routes all inference through the OpenAI-compatible adapter, so the TUI status bar and the model-identity system prompt reported the wire provider `openai` even when onboarding selected a different upstream (e.g. NVIDIA Cloud / Nemotron). This shows the onboard-selected upstream provider at those surfaces instead, while the adapter key stays `openai` for routing. ## Related Issue Fixes NVIDIA#6453 ## Changes - Add a `managed_display_provider()` helper to the managed runtime that maps the OpenAI-compatible adapter to the onboard-selected upstream (read from the existing `NEMOCLAW_UPSTREAM_PROVIDER` runtime env), falling back to the adapter name when no valid upstream is present. - Route the provider through that helper at the three display surfaces via the managed package patch: `StatusBar.set_model` (status bar), `WelcomeBanner.update_model` (launch banner), and `build_model_identity_section` (agent self-report system prompt). Add `status.py` and `welcome.py` to the patched-file set with AST symbol guards. - Add a behaviour test that runs the real patcher against a package fixture and asserts the substitution end to end (status bar, banner, self-report), plus the fallback cases (unset, equal-to-adapter, invalid token). - Extract the shared package-fixture builder into `test/helpers/langchain-deepagents-code-patch-fixture.ts` so the new focused test reuses it, keeping every test file within the size budget. The displayed token mirrors the onboard upstream provider name (default `nvidia`) — the provider family, not the marketing string; carrying the full display label would require threading it through the onboard → Dockerfile → config chain and is out of scope here. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: corrects an internal TUI status-bar / self-report label; no documented behaviour or user-facing doc page describes the provider label. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: maintainer review requested; the change is display-only and does not alter the managed inference route, adapter key, credential handling, or the patch's fail-closed guards. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: `vitest run` on `langchain-deepagents-code-provider-label`, `-direct-module-patch`, `-progressive-tool-disclosure`, `-image`, `-managed-entrypoints` — 169 tests passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Expanded managed upstream provider labeling to additional interface surfaces (including Status and Welcome). * Added environment-driven mapping so the displayed provider label can reflect the upstream provider when valid. * **Bug Fixes** * Improved validation and fallback behavior to reliably show the managed provider label when upstream input is missing or invalid. * **Tests** * Added end-to-end coverage for provider label mapping and UI display behavior, including edge cases. * Refreshed test fixtures/harness to verify patching and corruption detection across more UI components. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the v0.0.77 release-note section from the shipped release announcement and release commit range. This is post-release docs recovery, so the PR is labeled for the next patch release train. ## Changes - Added `v0.0.77` to `docs/about/release-notes.mdx` with links to the deeper Deep Agents, architecture, inference, security, and agent-docs pages. - Source summary: - NVIDIA#6469 -> `docs/about/release-notes.mdx`: Documents Deep Agents Code base-image publication and stale-version validation. - NVIDIA#6471 -> `docs/about/release-notes.mdx`: Documents the managed runtime disabling LangGraph CLI analytics. - NVIDIA#6462 -> `docs/about/release-notes.mdx`: Documents the TUI, launch banner, and model-identity provider display behavior. - NVIDIA#6460 -> `docs/about/release-notes.mdx`: Documents bounded, best-effort OTLP trace credential scrubbing and the remaining collector-side redaction requirement. - NVIDIA#6423 -> `docs/about/release-notes.mdx`: Documents the checked-in loopback-only local credential form used by starter prompts. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: release-note prose only, no runtime behavior or code samples changed. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: tests not applicable for release-note prose only. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Verification note: `npm run docs` passed. `fern check --warnings` reports the existing light-mode accent color contrast warning: `2.41:1`, expected at least `3:1`. --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new release-notes entry for **v0.0.77** at the top of the changelog. * Highlighted improved package validation, tighter handling of telemetry and trace data, and safer starter prompt behavior with stronger redaction and local-only submission. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Managed Deep Code routes all inference through the OpenAI-compatible adapter, so the TUI status bar and the model-identity system prompt reported the wire provider
openaieven when onboarding selected a different upstream (e.g. NVIDIA Cloud / Nemotron). This shows the onboard-selected upstream provider at those surfaces instead, while the adapter key staysopenaifor routing.Related Issue
Fixes #6453
Changes
managed_display_provider()helper to the managed runtime that maps the OpenAI-compatible adapter to the onboard-selected upstream (read from the existingNEMOCLAW_UPSTREAM_PROVIDERruntime env), falling back to the adapter name when no valid upstream is present.StatusBar.set_model(status bar),WelcomeBanner.update_model(launch banner), andbuild_model_identity_section(agent self-report system prompt). Addstatus.pyandwelcome.pyto the patched-file set with AST symbol guards.test/helpers/langchain-deepagents-code-patch-fixture.tsso the new focused test reuses it, keeping every test file within the size budget.The displayed token mirrors the onboard upstream provider name (default
nvidia) — the provider family, not the marketing string; carrying the full display label would require threading it through the onboard → Dockerfile → config chain and is out of scope here.Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablevitest runonlangchain-deepagents-code-provider-label,-direct-module-patch,-progressive-tool-disclosure,-image,-managed-entrypoints— 169 tests passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests