Skip to content

fix(cli): restore first-run launcher setup#59

Open
zhongxuanwang-nv wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
zhongxuanwang-nv:fix/launcher-bootstrap-config-args
Open

fix(cli): restore first-run launcher setup#59
zhongxuanwang-nv wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
zhongxuanwang-nv:fix/launcher-bootstrap-config-args

Conversation

@zhongxuanwang-nv

@zhongxuanwang-nv zhongxuanwang-nv commented Jul 13, 2026

Copy link
Copy Markdown

What

  • Populate the first-run launcher's synthetic configure arguments with the routing-profile and skill-distillation defaults expected by cmd_configure.
  • Make the bootstrap regression test execute the real configure path and verify that provider, model, and credentials are persisted.

Why

First-run launcher setup crashes with AttributeError because its hand-built argparse.Namespace does not satisfy the configure command's argument contract. The skill-distillation fields trigger the reported failure, and the missing routing_profiles field would fail next.

Closes #60

How tested

  • uv run ruff check . clean
  • uv run mypy switchyard clean
  • uv run pytest tests/ green
  • Manual smoke (not run; it would spawn an external agent CLI and use live provider credentials)

Checklist

  • One class per file; filename = snake_case of the primary class. (No classes added.)
  • New public symbols exported from switchyard/__init__.py.__all__ if intended for downstream use. (No public symbols added.)
  • Unit tests added for new components / bug fixes.
  • README / --help updated if customer-facing surface changed. (No documented interface changed.)
  • Commits signed off (Signed-off-by: Your Name <email>) per the DCO.

Notes for reviewers

maybe_bootstrap_launch_config is shared by Claude, Codex, and OpenClaw. The focused test exercises that shared helper through the Claude first-run path without making live provider calls.

Summary by CodeRabbit

  • Bug Fixes
    • Bootstrap configuration now preserves routing profile selections and skill-distillation settings.
    • Launch configuration correctly persists the selected provider, base URL, target model, and API key.

Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
@zhongxuanwang-nv zhongxuanwang-nv requested a review from a team as a code owner July 13, 2026 22:50
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Bootstrap configuration now forwards routing and skill-distillation settings. The Claude launch test validates persisted provider, base URL, model, and credential values directly.

Changes

Launch bootstrap configuration

Layer / File(s) Summary
Bootstrap configuration forwarding
switchyard/cli/launch_command.py
cmd_configure receives routing profiles and explicit skill-distillation settings.
Persisted provider validation
tests/test_launch_claude.py
The test loads saved configuration and credentials and verifies provider selection, base URL, launch model, and API key propagation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

I’m a rabbit with a config to pass,
Routing profiles tucked in grass.
Skill settings hop along the way,
Saved credentials now have their say.
“Claude,” I cheer, “the tests are right!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: restoring first-run launcher setup behavior in the CLI.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/test_launch_claude.py (1)

65-114: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Good regression coverage for the AttributeError fix, but routing_profiles forwarding itself isn't exercised.

Running the real cmd_configure path (rather than mocking it) is the right approach to catch the AttributeError this PR fixes. However, args.routing_profiles stays None in this test (Line 92), so the new routing_profiles=getattr(args, "routing_profiles", None) forwarding added in launch_command.py isn't distinctly validated — a regression that always forwards None regardless of input would still pass here.

Consider adding a second test (or parametrizing this one) with a non-None routing_profiles value and asserting it is persisted, to actually cover the forwarding behavior rather than just the crash fix.

🤖 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 `@tests/test_launch_claude.py` around lines 65 - 114, Extend the launch
configuration regression test around maybe_bootstrap_launch_config with a
non-None args.routing_profiles value, preferably by adding a focused case or
parameterization. Invoke the real configuration flow and assert the supplied
routing profiles are persisted in the loaded user configuration, while retaining
the existing assertions for provider credentials and model routing.
🤖 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 `@tests/test_launch_claude.py`:
- Around line 65-114: Extend the launch configuration regression test around
maybe_bootstrap_launch_config with a non-None args.routing_profiles value,
preferably by adding a focused case or parameterization. Invoke the real
configuration flow and assert the supplied routing profiles are persisted in the
loaded user configuration, while retaining the existing assertions for provider
credentials and model routing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 81418d39-5138-4cf5-820f-8e52f984c5e5

📥 Commits

Reviewing files that changed from the base of the PR and between a2a939d and 29e1ceb.

📒 Files selected for processing (2)
  • switchyard/cli/launch_command.py
  • tests/test_launch_claude.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] First-run launcher setup crashes before configuration

1 participant