fix(cli): restore first-run launcher setup#59
Conversation
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
WalkthroughBootstrap configuration now forwards routing and skill-distillation settings. The Claude launch test validates persisted provider, base URL, model, and credential values directly. ChangesLaunch bootstrap configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_launch_claude.py (1)
65-114: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGood regression coverage for the AttributeError fix, but routing_profiles forwarding itself isn't exercised.
Running the real
cmd_configurepath (rather than mocking it) is the right approach to catch theAttributeErrorthis PR fixes. However,args.routing_profilesstaysNonein this test (Line 92), so the newrouting_profiles=getattr(args, "routing_profiles", None)forwarding added inlaunch_command.pyisn't distinctly validated — a regression that always forwardsNoneregardless of input would still pass here.Consider adding a second test (or parametrizing this one) with a non-
Nonerouting_profilesvalue 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
📒 Files selected for processing (2)
switchyard/cli/launch_command.pytests/test_launch_claude.py
What
cmd_configure.Why
First-run launcher setup crashes with
AttributeErrorbecause its hand-builtargparse.Namespacedoes not satisfy the configure command's argument contract. The skill-distillation fields trigger the reported failure, and the missingrouting_profilesfield would fail next.Closes #60
How tested
uv run ruff check .cleanuv run mypy switchyardcleanuv run pytest tests/greenChecklist
snake_caseof the primary class. (No classes added.)switchyard/__init__.py.__all__if intended for downstream use. (No public symbols added.)--helpupdated if customer-facing surface changed. (No documented interface changed.)Signed-off-by: Your Name <email>) per the DCO.Notes for reviewers
maybe_bootstrap_launch_configis 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