feat: interactive provider + API-key setup in the TUI#24
Conversation
When no provider API key is set, the translate command now guides the user through a clean Rich input flow instead of failing inside pydantic-ai: - If no model is given, show a provider picker (Anthropic/OpenAI/Google/OpenRouter) - Prompt for the matching key in a masked input box, derived from -m when given - Save the key to .env and auto-load .env on startup (adds python-dotenv) Non-TTY runs and --dry-run skip the prompt, preserving existing behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds dotenv/readchar deps, loads ChangesInteractive Provider Setup and API Key Management
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labelsfeature, dependencies, tests, security 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment has been minimized.
This comment has been minimized.
![lanfermann-reviewer[bot]](https://avatars.githubusercontent.com/in/4002102?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Replace the numbered provider prompt with an interactive ↑/↓ menu (also j/k and number-key shortcuts, Enter to select) rendered via Rich Live + readchar. Falls back to the numbered prompt when stdin is not an interactive TTY. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
|
@kody start-review |
|
@kody start-review |
Annotate the new TestProviderKeySetup methods and the _feed_keys helper with parameter and return types (pytest.MonkeyPatch, Path, list[str], -> None) to satisfy the project's Typed classifier / py.typed marker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
coderabbitai
Bot
added
dependencies
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/xcstrings_translator/cli.py (1)
694-709:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
--dry-runfully side-effect free.Line 696 skips key prompts, but Line 708 still passes
fetch_live_pricing=not no_fetch, and_translate_one_file()createsXCStringsTranslatorbefore the dry-run return path. That means a dry run can still perform network I/O for live pricing. Gate pricing fetches behindnot dry_runtoo.Proposed fix
common = { "languages": target_langs, "model": model, "resolved": resolved, "batch_size": batch_size, "concurrency": concurrency, "overwrite": overwrite, "dry_run": dry_run, "app_context": app_context, "fill_missing": fill_missing, - "fetch_live_pricing": not no_fetch, + "fetch_live_pricing": not dry_run and not no_fetch, }As per coding guidelines,
src/xcstrings_translator/cli.py: "dry-run paths that perform writes or live API calls".🤖 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 `@src/xcstrings_translator/cli.py` around lines 694 - 709, The dry-run path still triggers live pricing and potentially constructs XCStringsTranslator; change the common dict entry "fetch_live_pricing" to be False when dry_run is true (i.e., "fetch_live_pricing": not no_fetch and not dry_run) and ensure callers like _translate_one_file do not instantiate XCStringsTranslator or otherwise perform network I/O when dry_run is true—either by early-returning before creating XCStringsTranslator or by passing dry_run through and gating network calls inside XCStringsTranslator. Use the existing symbols _ensure_provider_and_key, common, _translate_one_file, XCStringsTranslator, fetch_live_pricing, dry_run and no_fetch to locate the code to change.Source: Coding guidelines
🤖 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 `@src/xcstrings_translator/cli.py`:
- Around line 573-580: Update the help string for the CLI option named model
(the Annotated typer.Option in cli.py) to remove the misleading "Default:
sonnet" text and instead describe the actual behavior when no model is provided
(e.g., default is None, users may be prompted to choose a provider or a
provider-specific default may apply); ensure the help mentions valid formats
(sonnet, gpt-5, gemini-2.5-flash, openrouter:vendor/model or provider:model) and
that first-time users without an Anthropic key may be prompted to select a
provider.
In `@tests/test_cli.py`:
- Around line 300-305: The test test_ensure_non_tty_no_prompt unpacks (model,
resolved) from _ensure_provider_and_key but only asserts model; add an assertion
that verifies the second return value indicates no API key was resolved (e.g.,
assert resolved is False) to fully validate the non-TTY behavior of
_ensure_provider_and_key.
- Around line 307-312: The test unpacks both model and resolved but only asserts
resolved; add an assertion that model == "gpt-5.4" to confirm
_ensure_provider_and_key returns the expected model string. Update the
test_ensure_dry_run_skips_key case to include assert model == "gpt-5.4"
immediately before or after the existing assert on resolved so both return
values are validated.
---
Outside diff comments:
In `@src/xcstrings_translator/cli.py`:
- Around line 694-709: The dry-run path still triggers live pricing and
potentially constructs XCStringsTranslator; change the common dict entry
"fetch_live_pricing" to be False when dry_run is true (i.e.,
"fetch_live_pricing": not no_fetch and not dry_run) and ensure callers like
_translate_one_file do not instantiate XCStringsTranslator or otherwise perform
network I/O when dry_run is true—either by early-returning before creating
XCStringsTranslator or by passing dry_run through and gating network calls
inside XCStringsTranslator. Use the existing symbols _ensure_provider_and_key,
common, _translate_one_file, XCStringsTranslator, fetch_live_pricing, dry_run
and no_fetch to locate the code to 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: 61746637-4b0b-4f73-84cb-85a99416666c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (3)
pyproject.tomlsrc/xcstrings_translator/cli.pytests/test_cli.py
- Fix --model help text now that the option defaults to None instead of sonnet - Make --dry-run side-effect free: skip live pricing fetch (no network I/O) - Assert both return values in the non-TTY and dry-run resolution tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
All review findings addressed in e8c7f4b:
|
|
@lanfermann-reviewer is this PR fine |
1 similar comment
|
@lanfermann-reviewer is this PR fine |
|
@lanfermann-reviewer is this PR fine |
|
The PR looks solid overall. Adding interactive TUI provider selection and API key setup greatly improves onboarding. Here are a few points to consider:
Otherwise, the changes are well‑contained and follow good UX practices. I’d approve it after addressing the terminal fallback check. |
1 participant
This pull request introduces an interactive, TUI-based provider selection and API key setup for the
translatecommand. When no API key is configured for the required provider (or when no model is specified), the CLI now guides the user through a friendly terminal prompt to choose a provider and enter their API key—persisting it to a local.envfile for future use.Key changes
Provider selection menu
If no model or API key is provided, a menu lists available providers (Anthropic, OpenAI, Google, OpenRouter) with their default model. The user picks one.
API key prompt
For the selected provider, the user is asked to enter their key (masked input). The key is saved to
.envin the current directory and exported to the environment for the current run.Automatic resolution
Dry‑run support
The
--dry-runflag bypasses the API key requirement entirely—no prompts appear, even in an interactive terminal..env loading
The project now loads
.envfiles from the working directory on startup, so keys saved during a previous interactive session are automatically picked up.Model validation
The
--modeloption is now optional. When omitted, the CLI defaults tosonnet(Anthropic) if the key is present, or the user’s chosen provider otherwise. Explicit model validation is only performed when a model argument is supplied.New test coverage
Unit tests cover the provider resolution logic,
.envsaving, menu/key prompt flow, and dry‑run behavior.This enhancement makes it easier for new users to get started without manually editing configuration files, while still being transparent and scriptable in non‑interactive environments.
Summary by CodeRabbit
New Features
--modelis now optional; defaults to Anthropic when credentials exist, otherwise prompts for provider.Bug Fixes
Tests