Support provider-specific API key configuration#31
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces a provider-agnostic LLM factory supporting Groq, Google/Gemini, OpenAI, Anthropic, and OpenRouter. Adds provider-specific API-key handling and threads provider, model, and base URL options through the CLI, workflow, summarization, README generation, and review pipeline. ChangesConfigurable LLM Provider Support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as run command
participant Config as get_api_key
participant Summary as summarize_file
participant Workflow as ReadmeState
participant Factory as create_llm
CLI->>Config: get_api_key(provider)
Config-->>CLI: provider API key
CLI->>Summary: provider, model, base_url
Summary->>Factory: create_llm(provider, model_name, base_url)
Factory-->>Summary: BaseChatModel
CLI->>Workflow: initial_state(provider, model, base_url)
Workflow->>Factory: create_llm through generation and review nodes
Factory-->>Workflow: BaseChatModel
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
repo2readme/cli/main.py (1)
146-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the provider-to-env-var mapping.
provider_envis an exact copy ofprovider_mapfromconfig.py'sget_api_key. Maintaining two independent copies risks silent drift — as already seen with"together"being present here but missing increate_llm. Export the mapping fromconfig.pyand reuse it.♻️ Proposed refactor: export and reuse provider mapping
In
config.py, move the mapping to module level and export it:+PROVIDER_ENV_VARS = { + "groq": "GROQ_API_KEY", + "google": "GOOGLE_API_KEY", + "gemini": "GOOGLE_API_KEY", + "openai": "OPENAI_API_KEY", + "anthropic": "ANTHROPIC_API_KEY", + "openrouter": "OPENROUTER_API_KEY", + "together": "TOGETHER_API_KEY", +} + def get_api_key(provider: str): env = load_env() - provider_map = { - "groq": "GROQ_API_KEY", - "google": "GOOGLE_API_KEY", - "gemini": "GOOGLE_API_KEY", - "openai": "OPENAI_API_KEY", - "anthropic": "ANTHROPIC_API_KEY", - "openrouter": "OPENROUTER_API_KEY", - "together": "TOGETHER_API_KEY", - } + provider_map = PROVIDER_ENV_VARSIn
main.py, import and reuse:-from repo2readme.config import ( +from repo2readme.config import ( get_api_keys, get_api_key, reset_api_keys, + PROVIDER_ENV_VARS, )if provider: api_key = get_api_key(provider) - provider_env = { - "groq": "GROQ_API_KEY", - "google": "GOOGLE_API_KEY", - "gemini": "GOOGLE_API_KEY", - "openai": "OPENAI_API_KEY", - "anthropic": "ANTHROPIC_API_KEY", - "openrouter": "OPENROUTER_API_KEY", - "together": "TOGETHER_API_KEY", - } - os.environ[provider_env[provider.lower()]] = api_key + os.environ[PROVIDER_ENV_VARS[provider.lower()]] = api_key🤖 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 `@repo2readme/cli/main.py` around lines 146 - 156, The provider-to-env-var mapping is duplicated in main.py and config.py, so reuse the single source of truth instead of maintaining a local copy. Move the provider mapping to module scope in config.py (alongside get_api_key) and export it, then update main.py to import and use that shared mapping when setting os.environ in the API key flow. Make sure any provider handling used by create_llm and get_api_key stays aligned through the shared mapping.
🤖 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 `@repo2readme/config.py`:
- Around line 57-62: The API key prompt flow in the config helper currently
saves whatever `input(...).strip()` returns, including an empty string when the
user just presses Enter. Update the API key handling logic around the prompt and
`save_env` call to validate that the trimmed value is non-empty before
persisting or returning it, and if it is empty, re-prompt or fail with a clear
message instead of writing it to the env file. Use the existing `provider`,
`env_var`, and `save_env` flow to keep the fix localized.
In `@repo2readme/llm/factory.py`:
- Around line 66-77: The create_llm factory is missing support for the
"together" provider, which causes a ValueError even though config.py and main.py
already accept it. Update create_llm in factory.py to handle provider ==
"together" alongside the existing provider branches, using the same pattern as
the other ChatOpenAI initializations and wiring the appropriate Together API
key/base URL settings. Keep the unsupported-provider ValueError only for truly
unknown providers so the provider flow stays consistent across files.
---
Nitpick comments:
In `@repo2readme/cli/main.py`:
- Around line 146-156: The provider-to-env-var mapping is duplicated in main.py
and config.py, so reuse the single source of truth instead of maintaining a
local copy. Move the provider mapping to module scope in config.py (alongside
get_api_key) and export it, then update main.py to import and use that shared
mapping when setting os.environ in the API key flow. Make sure any provider
handling used by create_llm and get_api_key stays aligned through the shared
mapping.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0483916-a4c3-40ac-a9d6-092dbd74bee8
📒 Files selected for processing (9)
README.mdrepo2readme/cli/main.pyrepo2readme/config.pyrepo2readme/llm/__init__.pyrepo2readme/llm/factory.pyrepo2readme/readme/agent_workflow.pyrepo2readme/readme/readme_generator.pyrepo2readme/readme/reviewer_agent.pyrepo2readme/summarize/summary.py
Comment on lines
+57
to
62
| rprint(f"[yellow]{provider} API key is missing![/yellow]\n") | ||
|
|
||
| api_key = input(f"Enter your {provider} API key: ").strip() | ||
|
|
||
| env[env_var] = api_key | ||
| save_env(env) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Validate non-empty API key before persisting.
input(...).strip() returns an empty string if the user presses Enter. The empty key is saved to the env file and returned, causing confusing authentication errors downstream with no clear root cause.
🛡️ Proposed fix: reject empty input
rprint(f"[yellow]{provider} API key is missing![/yellow]\n")
api_key = input(f"Enter your {provider} API key: ").strip()
+ if not api_key:
+ rprint("[red]API key cannot be empty. Aborting.[/red]")
+ raise ValueError("API key cannot be empty")
+
env[env_var] = api_key
save_env(env)📝 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.
Suggested change
| rprint(f"[yellow]{provider} API key is missing![/yellow]\n") | |
| api_key = input(f"Enter your {provider} API key: ").strip() | |
| env[env_var] = api_key | |
| save_env(env) | |
| rprint(f"[yellow]{provider} API key is missing![/yellow]\n") | |
| api_key = input(f"Enter your {provider} API key: ").strip() | |
| if not api_key: | |
| rprint("[red]API key cannot be empty. Aborting.[/red]") | |
| raise ValueError("API key cannot be empty") | |
| env[env_var] = api_key | |
| save_env(env) |
🤖 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 `@repo2readme/config.py` around lines 57 - 62, The API key prompt flow in the
config helper currently saves whatever `input(...).strip()` returns, including
an empty string when the user just presses Enter. Update the API key handling
logic around the prompt and `save_env` call to validate that the trimmed value
is non-empty before persisting or returning it, and if it is empty, re-prompt or
fail with a clear message instead of writing it to the env file. Use the
existing `provider`, `env_var`, and `save_env` flow to keep the fix localized.
Comment on lines
+66
to
+77
| elif provider == "openrouter": | ||
| return ChatOpenAI( | ||
| model=model, | ||
| api_key=api_key or os.getenv("OPENROUTER_API_KEY"), | ||
| base_url=base_url or "https://openrouter.ai/api/v1", | ||
| **kwargs, | ||
| ) | ||
|
|
||
| else: | ||
| raise ValueError( | ||
| f"Unsupported provider '{provider}'." | ||
| ) No newline at end of file |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Missing "together" provider creates cross-file inconsistency.
config.py's get_api_key and main.py's provider_env both support "together", but create_llm does not. A user passing --provider together would successfully configure an API key, then hit a ValueError during summarization — all documents fail and a poor README is generated from error dicts.
🔧 Proposed fix: add "together" provider support
elif provider == "openrouter":
return ChatOpenAI(
model=model,
api_key=api_key or os.getenv("OPENROUTER_API_KEY"),
base_url=base_url or "https://openrouter.ai/api/v1",
**kwargs,
)
+ elif provider == "together":
+ return ChatOpenAI(
+ model=model,
+ api_key=api_key or os.getenv("TOGETHER_API_KEY"),
+ base_url=base_url or "https://api.together.xyz/v1",
+ **kwargs,
+ )
+
else:
raise ValueError(
f"Unsupported provider '{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.
Suggested change
| elif provider == "openrouter": | |
| return ChatOpenAI( | |
| model=model, | |
| api_key=api_key or os.getenv("OPENROUTER_API_KEY"), | |
| base_url=base_url or "https://openrouter.ai/api/v1", | |
| **kwargs, | |
| ) | |
| else: | |
| raise ValueError( | |
| f"Unsupported provider '{provider}'." | |
| ) | |
| elif provider == "openrouter": | |
| return ChatOpenAI( | |
| model=model, | |
| api_key=api_key or os.getenv("OPENROUTER_API_KEY"), | |
| base_url=base_url or "https://openrouter.ai/api/v1", | |
| **kwargs, | |
| ) | |
| elif provider == "together": | |
| return ChatOpenAI( | |
| model=model, | |
| api_key=api_key or os.getenv("TOGETHER_API_KEY"), | |
| base_url=base_url or "https://api.together.xyz/v1", | |
| **kwargs, | |
| ) | |
| else: | |
| raise ValueError( | |
| f"Unsupported provider '{provider}'." | |
| ) |
🤖 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 `@repo2readme/llm/factory.py` around lines 66 - 77, The create_llm factory is
missing support for the "together" provider, which causes a ValueError even
though config.py and main.py already accept it. Update create_llm in factory.py
to handle provider == "together" alongside the existing provider branches, using
the same pattern as the other ChatOpenAI initializations and wiring the
appropriate Together API key/base URL settings. Keep the unsupported-provider
ValueError only for truly unknown providers so the provider flow stays
consistent across files.
Owner
|
@RajanyaSaha-27 can u solve the errors mentioned by coderabbit. And the tests are failing check those also. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves provider-specific API key handling.(fixes #16 )
Changes
get_api_key(provider)support.get_api_keys()behavior for backward compatibility.Testing
python -m pytestrepo2readme run --helpSummary by CodeRabbit
New Features
llm/folder.Bug Fixes