Docs/Refactor: add provider-agnostic configuration examples and dynamic key resolution (#4)

[phatngo511]

Hi @sunilp,
I've completed issue #4 with these improvements:

1. Structured .env.example with clear provider blocks.
2. Updated ModelConfig to dynamically resolve the correct API key.
3. Minor fix in create_client for local providers to support custom keys.
   Fixes Add provider-agnostic configuration examples #4.

Summary by CodeRabbit

• Chores
  • Updated the environment configuration template to a provider-selection format with clear examples for OpenAI (default), Anthropic, and Local/Ollama, and reorganized shared model variables for clarity.
• Bug Fixes
  • Improved API key selection so credentials load based on the chosen provider, and local provider will use a supplied API key when provided instead of forcing a fallback.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a24ab560-21cd-4f95-a996-3a2248e7ab73

📥 Commits

Reviewing files that changed from the base of the PR and between 587ff48 and 6ef7bc3.

📒 Files selected for processing (1)

• src/shared/model_client.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/shared/model_client.py

---

📝 Walkthrough

Walkthrough

Refactors configuration to be provider-agnostic: .env.example now shows commented provider blocks (OpenAI, Anthropic, local/Ollama). ModelConfig.api_key is a runtime property that reads the provider-specific environment variable. create_client for local accepts a caller api_key fallback instead of always "local".

Changes

Cohort / File(s)	Summary
Configuration Template /.env.example	Replaced single active OpenAI example with provider-selection template: commented blocks for OpenAI (active), Anthropic, and local/Ollama. Removed standalone ANTHROPIC_API_KEY and separate local examples; introduced MODEL_PROVIDER, provider-specific key/URL, and model name variables as commented alternatives. Shared model variables reorganized under OpenAI section.
Provider-Aware API Key Handling src/shared/config.py	Removed api_key as a computed field at class-definition time. Added @property def api_key(self) -> str which returns the environment key matching self.provider (OPENAI_API_KEY, ANTHROPIC_API_KEY, or "" for others). Eliminates prior cross-provider fallback.
Client Creation Logic src/shared/model_client.py	Changed create_client() local-provider branch to pass a caller-provided api_key when non-empty; otherwise fall back to "local". Previously the local branch always used "local" as the API key.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

Poem

🐇 I nibble configs, tidy and spry,
Three providers now hop in a line,
OpenAI, Anthropic, Ollama too,
Swap them with ease — the choice is your cue,
A little rabbit cheers: "New paths, new view!"

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: updating configuration examples and implementing dynamic API key resolution based on providers.
Linked Issues check	✅ Passed	The PR successfully implements provider-agnostic configuration examples [#4] with .env.example restructured, ModelConfig refactored for dynamic key resolution, and create_client updated.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to completing issue #4: .env.example restructuring, ModelConfig provider-aware implementation, and create_client local provider fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (3)

src/shared/config.py (1)

31-35: Consider supporting LOCAL_API_KEY for authenticated local endpoints.

Since create_client now supports custom API keys for local providers, you may want to add support for a LOCAL_API_KEY environment variable here for consistency:

Suggested enhancement

     `@property`
     def api_key(self) -> str:
         """Dynamically fetch the correct API key based on the provider."""
         if self.provider == "openai":
             return os.getenv("OPENAI_API_KEY", "")
         if self.provider == "anthropic":
             return os.getenv("ANTHROPIC_API_KEY", "")
+        if self.provider == "local":
+            return os.getenv("LOCAL_API_KEY", "")
         return ""

This would enable authenticated local deployments (e.g., vLLM with API keys) to work seamlessly through ModelConfig.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/shared/config.py` around lines 31 - 35, The current API key selection
only returns OPENAI_API_KEY or ANTHROPIC_API_KEY based on self.provider; add
support for authenticated local endpoints by checking if self.provider ==
"local" (or whatever local provider identifier is used in ModelConfig) and
returning os.getenv("LOCAL_API_KEY", "") so create_client can use custom API
keys for local deployments—update the method/property that inspects
self.provider (the code block shown) to include this additional branch while
preserving existing fallbacks.

src/shared/model_client.py (1)

202-204: Translate comment to English for codebase consistency.

The logic change is correct and aligns well with the dynamic key resolution from ModelConfig. However, the comment is in Vietnamese while the rest of the codebase uses English.

Suggested fix

-        # FIX: Sửa lỗi hardcode 'local'. Ưu tiên dùng api_key nếu có.
+        # Use provided api_key if available, otherwise default to "local"
         actual_key = api_key if api_key else "local"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/shared/model_client.py` around lines 202 - 204, Replace the Vietnamese
inline comment above the dynamic key resolution with an English comment that
explains the change: that we avoid hardcoding "local" and prefer the supplied
api_key (falling back to "local" only if api_key is falsy). Update the comment
near actual_key = api_key if api_key else "local" to something like "Prefer
provided api_key; fall back to 'local' to avoid hardcoding" and ensure
references to ModelConfig/OpenAIClient/model_name/base_url (url) remain
unchanged.

.env.example (1)

21-26: Consider documenting LOCAL_API_KEY option for authenticated local endpoints.

The create_client function now supports custom API keys for the local provider, but there's no corresponding environment variable documented here. Some local deployments (e.g., vLLM with authentication) may require an API key.

Suggested addition

 # --- Option 3: Local Models (e.g., Ollama) ---
 # MODEL_PROVIDER=local
+# LOCAL_API_KEY=your-local-key-here  # Optional: only if your local server requires auth
 # LOCAL_MODEL_URL=http://localhost:11434/v1
 # MODEL_NAME=llama3
 # MODEL_NAME_CHEAP=llama3
 # EVAL_JUDGE_MODEL=llama3

Note: This would also require updating ModelConfig.api_key in src/shared/config.py to read LOCAL_API_KEY when the provider is local.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.env.example around lines 21 - 26, Add a documented LOCAL_API_KEY entry to
.env.example and wire it into the config so local provider auth works: update
the example block for the local model option to include a commented
LOCAL_API_KEY line, and modify ModelConfig.api_key (used by create_client) to
read LOCAL_API_KEY when MODEL_PROVIDER or equivalent is set to "local" so
authenticated local endpoints are supported by create_client.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.env.example:
- Around line 21-26: Add a documented LOCAL_API_KEY entry to .env.example and
wire it into the config so local provider auth works: update the example block
for the local model option to include a commented LOCAL_API_KEY line, and modify
ModelConfig.api_key (used by create_client) to read LOCAL_API_KEY when
MODEL_PROVIDER or equivalent is set to "local" so authenticated local endpoints
are supported by create_client.

In `@src/shared/config.py`:
- Around line 31-35: The current API key selection only returns OPENAI_API_KEY
or ANTHROPIC_API_KEY based on self.provider; add support for authenticated local
endpoints by checking if self.provider == "local" (or whatever local provider
identifier is used in ModelConfig) and returning os.getenv("LOCAL_API_KEY", "")
so create_client can use custom API keys for local deployments—update the
method/property that inspects self.provider (the code block shown) to include
this additional branch while preserving existing fallbacks.

In `@src/shared/model_client.py`:
- Around line 202-204: Replace the Vietnamese inline comment above the dynamic
key resolution with an English comment that explains the change: that we avoid
hardcoding "local" and prefer the supplied api_key (falling back to "local" only
if api_key is falsy). Update the comment near actual_key = api_key if api_key
else "local" to something like "Prefer provided api_key; fall back to 'local' to
avoid hardcoding" and ensure references to
ModelConfig/OpenAIClient/model_name/base_url (url) remain unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 89387843-98aa-42cd-bc36-d19a820eb884

📥 Commits

Reviewing files that changed from the base of the PR and between ef452e0 and 587ff48.

📒 Files selected for processing (3)

• .env.example
• src/shared/config.py
• src/shared/model_client.py

[sunilp]

Thanks for the contribution! The .env.example restructure and the dynamic api_key property are both clean improvements.

Two changes needed before merge:

1. Restore the "Why this matters" docstring in model_client.py (lines 5-9 in the original). This is a book repo where educational comments are intentional teaching content, not boilerplate to trim.

2. Change the comment on line 202 to English. The repo is English-only.

Happy to merge once those are addressed.

[phatngo511]

Hi @sunilp, I've updated model_client.py to restore the original docstring and translated the comment to English. Thanks for your guidance!