Feature: Support Custom LLM Providers & Configurable Models (#16)#24
Feature: Support Custom LLM Providers & Configurable Models (#16)#24Shyam5806 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR introduces a centralized LLM factory (repo2readme/llm/factory.py) supporting per-stage default and runtime-overridden provider/model/temperature configuration. config.py generalizes API key handling via a SUPPORTED_API_KEYS mapping. CLI gains --provider/--model/--base-url flags wired via configure_llm(). Summarizer, generator, and reviewer modules now use get_llm() instead of direct provider instantiation. ChangesProvider-agnostic LLM configuration
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as cli/main.py
participant Config as config.py
participant Factory as llm/factory.py
participant Consumer as summarizer/generator/reviewer
User->>CLI: run(--provider, --model, --base-url)
CLI->>Config: get_api_keys()
Config-->>CLI: dict of API keys
CLI->>Factory: configure_llm(provider, model, base_url)
Factory->>Factory: update _RUNTIME_CONFIG
CLI->>Consumer: invoke summarize/generate/review
Consumer->>Factory: get_llm(stage)
Factory->>Factory: create_llm(stage) resolves provider/model
Factory-->>Consumer: chat model instance
Consumer-->>CLI: response
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 51-70: The key-prompting logic in get_api_keys() is too broad
because it iterates over SUPPORTED_API_KEYS and asks for every missing provider
key, even when only one provider is actually needed. Update the flow so it only
prompts for the active provider(s) selected by the CLI/factory, or otherwise
skips prompting when the selected provider’s key is already available; use
get_api_keys(), SUPPORTED_API_KEYS, and save_env() as the main touchpoints. Also
handle empty input explicitly so a blank response is not saved as an empty
string and retriggered on the next run.
In `@repo2readme/llm/factory.py`:
- Around line 40-47: `configure_llm()` is storing runtime overrides that
`create_llm()` does not actually honor, and the CLI is advertising providers
that the factory rejects. Update `create_llm()` to read
`_RUNTIME_CONFIG["base_url"]` and pass it into the provider-specific client
setup, and either add support for the advertised `openai` and `anthropic`
providers or remove them from the CLI help/validation so the accepted providers
match the implementation.
- Around line 65-99: The provider/model resolution in create_llm() is mixing a
global provider override with a stage-specific default model, causing
incompatible pairs like google with an OpenAI model. Update create_llm() to
resolve the model from the selected provider when provider is overridden, or
require an explicit model in that case, using the existing provider-selection
logic and ChatGroq/ChatGoogleGenerativeAI branches to keep provider and model
aligned.
🪄 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: 81196560-8b84-4e8d-a965-9124680379b0
📒 Files selected for processing (7)
repo2readme/cli/main.pyrepo2readme/config.pyrepo2readme/llm/factory.pyrepo2readme/llm/init.pyrepo2readme/readme/readme_generator.pyrepo2readme/readme/reviewer_agent.pyrepo2readme/summarize/summary.py
| env = load_env() | ||
|
|
||
| missing = [ | ||
| key | ||
| for key in SUPPORTED_API_KEYS | ||
| if not env.get(key) | ||
| ] | ||
|
|
||
| if missing: | ||
| rprint("[yellow]Some API keys are missing.[/yellow]\n") | ||
|
|
||
| save_env(env) | ||
| for key in missing: | ||
| provider_name = SUPPORTED_API_KEYS[key] | ||
| env[key] = input( | ||
| f"Enter your {provider_name} API key: " | ||
| ).strip() | ||
|
|
||
| rprint("[green]API keys saved successfully![/green]") | ||
| save_env(env) | ||
|
|
||
| return groq, gemini | ||
| rprint("[green]API keys saved successfully![/green]") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prompts for every provider's key, even when only one provider is used.
missing collects all of SUPPORTED_API_KEYS, so a user running with only Groq is still forced to enter a Google Gemini key (and vice versa). Since provider selection lives in the CLI/factory, get_api_keys() has no way to prompt only for the relevant key. This is a UX regression against the goal of provider-agnostic selection. Consider scoping the prompt to the active provider(s), or allowing the run to proceed when the selected provider's key is present.
Also note: a blank Enter stores "", which env.get(key) treats as missing and re-prompts on the next run.
🤖 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 51 - 70, The key-prompting logic in
get_api_keys() is too broad because it iterates over SUPPORTED_API_KEYS and asks
for every missing provider key, even when only one provider is actually needed.
Update the flow so it only prompts for the active provider(s) selected by the
CLI/factory, or otherwise skips prompting when the selected provider’s key is
already available; use get_api_keys(), SUPPORTED_API_KEYS, and save_env() as the
main touchpoints. Also handle empty input explicitly so a blank response is not
saved as an empty string and retriggered on the next run.
| def configure_llm(provider=None, model=None, base_url=None): | ||
| """ | ||
| Configure runtime overrides supplied from the CLI. | ||
| """ | ||
|
|
||
| _RUNTIME_CONFIG["provider"] = provider | ||
| _RUNTIME_CONFIG["model"] = model | ||
| _RUNTIME_CONFIG["base_url"] = base_url |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
git ls-files repo2readme/llm repo2readme/cli | sed -n '1,120p'Repository: agsaru/Repo2Readme
Length of output: 259
🏁 Script executed:
sed -n '1,220p' repo2readme/llm/factory.py && printf '\n--- CLI ---\n' && sed -n '1,220p' repo2readme/cli/main.py && printf '\n--- init ---\n' && sed -n '1,160p' repo2readme/llm/init.pyRepository: agsaru/Repo2Readme
Length of output: 9171
🏁 Script executed:
rg -n "base_url|openai|anthropic|ChatOpenAI|ChatAnthropic|Unsupported provider|configure_llm|get_llm|create_llm" repo2readmeRepository: agsaru/Repo2Readme
Length of output: 1569
Wire base_url and advertised providers through create_llm, or remove them from the CLI help
configure_llm() stores base_url, but create_llm() never reads it. The CLI also advertises openai/anthropic, yet create_llm() only accepts groq and google/gemini, so those flags end in Unsupported provider and --base-url is ignored.
🤖 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 40 - 47, `configure_llm()` is
storing runtime overrides that `create_llm()` does not actually honor, and the
CLI is advertising providers that the factory rejects. Update `create_llm()` to
read `_RUNTIME_CONFIG["base_url"]` and pass it into the provider-specific client
setup, and either add support for the advertised `openai` and `anthropic`
providers or remove them from the CLI help/validation so the accepted providers
match the implementation.
| provider = ( | ||
| provider | ||
| or _RUNTIME_CONFIG["provider"] | ||
| or config["provider"] | ||
| ) | ||
|
|
||
| model = ( | ||
| model | ||
| or _RUNTIME_CONFIG["model"] | ||
| or config["model"] | ||
| ) | ||
|
|
||
| temperature = ( | ||
| temperature | ||
| if temperature is not None | ||
| else config["temperature"] | ||
| ) | ||
|
|
||
| provider = provider.lower() | ||
|
|
||
| if provider == "groq": | ||
| return ChatGroq( | ||
| model=model, | ||
| api_key=os.getenv("GROQ_API_KEY"), | ||
| temperature=temperature, | ||
| ) | ||
|
|
||
| if provider in ("google", "gemini"): | ||
| return ChatGoogleGenerativeAI( | ||
| model=model, | ||
| google_api_key=os.getenv("GOOGLE_API_KEY"), | ||
| temperature=temperature, | ||
| ) | ||
|
|
||
| raise ValueError(f"Unsupported provider: {provider}") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- factory outline ---\n'
ast-grep outline repo2readme/llm/factory.py --view expanded || true
printf '\n--- relevant lines from factory.py ---\n'
cat -n repo2readme/llm/factory.py | sed -n '1,220p'
printf '\n--- search runtime config / defaults ---\n'
rg -n "_RUNTIME_CONFIG|provider|model|temperature|summarizer|openai/gpt-oss-120b|groq" repo2readme -g '!**/__pycache__/**'Repository: agsaru/Repo2Readme
Length of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline repo2readme/llm/factory.py --view expanded || true
printf '\n==== factory.py ====\n'
cat -n repo2readme/llm/factory.py | sed -n '1,220p'
printf '\n==== searches ====\n'
rg -n "_RUNTIME_CONFIG|summarizer|openai/gpt-oss-120b|groq|google|gemini|provider|model" repo2readmeRepository: agsaru/Repo2Readme
Length of output: 7224
🏁 Script executed:
set -euo pipefail
cat -n repo2readme/llm/factory.py | sed -n '1,220p'
printf '\n---\n'
rg -n "_RUNTIME_CONFIG|summarizer|openai/gpt-oss-120b|groq|google|gemini|provider|model" repo2readmeRepository: agsaru/Repo2Readme
Length of output: 6994
Global provider overrides should not inherit a stage-specific model.
create_llm() resolves provider and model independently, so --provider google on summarizer still uses openai/gpt-oss-120b from DEFAULT_CONFIG["summarizer"]. That mismatched pair needs an explicit --model, or provider-specific model selection.
🤖 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 65 - 99, The provider/model
resolution in create_llm() is mixing a global provider override with a
stage-specific default model, causing incompatible pairs like google with an
OpenAI model. Update create_llm() to resolve the model from the selected
provider when provider is overridden, or require an explicit model in that case,
using the existing provider-selection logic and ChatGroq/ChatGoogleGenerativeAI
branches to keep provider and model aligned.
Overview
This PR implements support for configurable LLM providers by introducing a centralized LLM abstraction layer while preserving the existing default behavior.
This is my first open-source contribution and my first Pull Request, so I spent a significant amount of time understanding the existing codebase, the repository workflow, and how the LLM pipeline works before implementing the requested changes. I tried my best to keep the implementation clean, maintain backward compatibility, and minimize changes to the existing architecture.
Changes Made
--provider--model--base-urlTesting
Notes
Since this is my first contribution to an open-source project, there may still be improvements or better approaches that I may have overlooked. I would really appreciate any feedback or suggestions from the maintainers and I'm more than happy to make any requested changes.
Thank you for maintaining this project and for providing an opportunity to contribute. Working on this issue helped me learn a lot about designing extensible software and contributing to open source.
Closes #16
Summary by CodeRabbit