Commit
Β·
4bfa475
1
Parent(s):
a5ad664
fix(coderabbit): Address all CodeRabbit review findings
Browse filesCritical fixes:
- HuggingFaceProvider now requires HF_TOKEN (raises clear RuntimeError if missing)
- Fixed test claiming "no keys" but setting hf_token
Minor fixes:
- service_loader.py: sk-ant- keys now properly excluded from OpenAI path
- llamaindex_rag.py: Added defense-in-depth key prefix validation
- Removed all Anthropic mentions from user-facing messages (4 places)
src/agent_factory/judges.py
CHANGED
|
@@ -2,6 +2,7 @@
|
|
| 2 |
|
| 3 |
import asyncio
|
| 4 |
import json
|
|
|
|
| 5 |
from functools import partial
|
| 6 |
from typing import Any, ClassVar
|
| 7 |
|
|
@@ -93,14 +94,21 @@ def get_model(api_key: str | None = None) -> Any:
|
|
| 93 |
# Priority 3: HuggingFace (free fallback)
|
| 94 |
# Use 7B model to stay on HuggingFace native infrastructure (avoid Novita 500s)
|
| 95 |
model_name = settings.huggingface_model or "Qwen/Qwen2.5-7B-Instruct"
|
| 96 |
-
|
|
|
|
|
|
|
|
|
|
| 97 |
if hf_token:
|
| 98 |
hf_provider = HuggingFaceProvider(api_key=hf_token)
|
| 99 |
return HuggingFaceModel(model_name, provider=hf_provider)
|
| 100 |
|
| 101 |
-
#
|
| 102 |
-
|
| 103 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
| 104 |
|
| 105 |
|
| 106 |
class JudgeHandler:
|
|
@@ -519,7 +527,7 @@ IMPORTANT: Respond with ONLY valid JSON matching this schema:
|
|
| 519 |
"The HuggingFace Inference API free tier limit has been reached. "
|
| 520 |
"The search results listed below were retrieved but could not be "
|
| 521 |
"analyzed by the AI. "
|
| 522 |
-
"Please try again later, or add an OpenAI
|
| 523 |
"for unlimited access."
|
| 524 |
),
|
| 525 |
)
|
|
@@ -555,7 +563,7 @@ IMPORTANT: Respond with ONLY valid JSON matching this schema:
|
|
| 555 |
f"Search found {len(evidence)} sources (listed below) but they could not "
|
| 556 |
"be analyzed by AI.\n\n"
|
| 557 |
"**Options:**\n"
|
| 558 |
-
"- Add an OpenAI
|
| 559 |
"- Try again later when HF Inference is available\n"
|
| 560 |
"- Review the raw search results below"
|
| 561 |
),
|
|
@@ -584,7 +592,7 @@ IMPORTANT: Respond with ONLY valid JSON matching this schema:
|
|
| 584 |
f"{question} clinical trials",
|
| 585 |
f"{question} drug candidates",
|
| 586 |
],
|
| 587 |
-
reasoning=f"HF Inference failed: {error}. Recommend configuring OpenAI
|
| 588 |
)
|
| 589 |
|
| 590 |
async def synthesize(self, system_prompt: str, user_prompt: str) -> str:
|
|
@@ -741,6 +749,6 @@ class MockJudgeHandler:
|
|
| 741 |
reasoning=(
|
| 742 |
f"Demo mode assessment based on {evidence_count} real search results. "
|
| 743 |
"For AI-powered analysis with drug candidate identification and "
|
| 744 |
-
"evidence synthesis, configure OPENAI_API_KEY
|
| 745 |
),
|
| 746 |
)
|
|
|
|
| 2 |
|
| 3 |
import asyncio
|
| 4 |
import json
|
| 5 |
+
import os
|
| 6 |
from functools import partial
|
| 7 |
from typing import Any, ClassVar
|
| 8 |
|
|
|
|
| 94 |
# Priority 3: HuggingFace (free fallback)
|
| 95 |
# Use 7B model to stay on HuggingFace native infrastructure (avoid Novita 500s)
|
| 96 |
model_name = settings.huggingface_model or "Qwen/Qwen2.5-7B-Instruct"
|
| 97 |
+
|
| 98 |
+
# Try settings.hf_token first, then fall back to HF_TOKEN env var
|
| 99 |
+
# HuggingFaceProvider requires a token - it won't work without one
|
| 100 |
+
hf_token = settings.hf_token or os.environ.get("HF_TOKEN")
|
| 101 |
if hf_token:
|
| 102 |
hf_provider = HuggingFaceProvider(api_key=hf_token)
|
| 103 |
return HuggingFaceModel(model_name, provider=hf_provider)
|
| 104 |
|
| 105 |
+
# No HF token available - raise clear error
|
| 106 |
+
raise RuntimeError(
|
| 107 |
+
"No LLM API key available. Either:\n"
|
| 108 |
+
" 1. Set OPENAI_API_KEY for premium tier, or\n"
|
| 109 |
+
" 2. Set HF_TOKEN for free HuggingFace tier\n"
|
| 110 |
+
"Get a free HF token at: https://huggingface.co/settings/tokens"
|
| 111 |
+
)
|
| 112 |
|
| 113 |
|
| 114 |
class JudgeHandler:
|
|
|
|
| 527 |
"The HuggingFace Inference API free tier limit has been reached. "
|
| 528 |
"The search results listed below were retrieved but could not be "
|
| 529 |
"analyzed by the AI. "
|
| 530 |
+
"Please try again later, or add an OpenAI API key above "
|
| 531 |
"for unlimited access."
|
| 532 |
),
|
| 533 |
)
|
|
|
|
| 563 |
f"Search found {len(evidence)} sources (listed below) but they could not "
|
| 564 |
"be analyzed by AI.\n\n"
|
| 565 |
"**Options:**\n"
|
| 566 |
+
"- Add an OpenAI API key for reliable analysis\n"
|
| 567 |
"- Try again later when HF Inference is available\n"
|
| 568 |
"- Review the raw search results below"
|
| 569 |
),
|
|
|
|
| 592 |
f"{question} clinical trials",
|
| 593 |
f"{question} drug candidates",
|
| 594 |
],
|
| 595 |
+
reasoning=f"HF Inference failed: {error}. Recommend configuring OpenAI API key.",
|
| 596 |
)
|
| 597 |
|
| 598 |
async def synthesize(self, system_prompt: str, user_prompt: str) -> str:
|
|
|
|
| 749 |
reasoning=(
|
| 750 |
f"Demo mode assessment based on {evidence_count} real search results. "
|
| 751 |
"For AI-powered analysis with drug candidate identification and "
|
| 752 |
+
"evidence synthesis, configure OPENAI_API_KEY."
|
| 753 |
),
|
| 754 |
)
|
src/services/llamaindex_rag.py
CHANGED
|
@@ -90,6 +90,19 @@ class LlamaIndexRAGService:
|
|
| 90 |
if not self.api_key:
|
| 91 |
raise ConfigurationError("OPENAI_API_KEY required for LlamaIndex RAG service")
|
| 92 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 93 |
# Configure LlamaIndex settings (use centralized config)
|
| 94 |
self._Settings.llm = OpenAI(
|
| 95 |
model=settings.openai_model,
|
|
|
|
| 90 |
if not self.api_key:
|
| 91 |
raise ConfigurationError("OPENAI_API_KEY required for LlamaIndex RAG service")
|
| 92 |
|
| 93 |
+
# Defense-in-depth: Validate key prefix to prevent cryptic auth errors
|
| 94 |
+
# Note: Anthropic keys start with sk-ant-, which would pass startswith("sk-")
|
| 95 |
+
if self.api_key.startswith("sk-ant-"):
|
| 96 |
+
raise ConfigurationError(
|
| 97 |
+
"Anthropic keys (sk-ant-...) are not supported for embeddings. "
|
| 98 |
+
"LlamaIndex RAG requires an OpenAI API key (sk-...)."
|
| 99 |
+
)
|
| 100 |
+
if not self.api_key.startswith("sk-"):
|
| 101 |
+
raise ConfigurationError(
|
| 102 |
+
f"Invalid API key format. Expected OpenAI key starting with 'sk-', "
|
| 103 |
+
f"got key starting with '{self.api_key[:8]}...'."
|
| 104 |
+
)
|
| 105 |
+
|
| 106 |
# Configure LlamaIndex settings (use centralized config)
|
| 107 |
self._Settings.llm = OpenAI(
|
| 108 |
model=settings.openai_model,
|
src/utils/service_loader.py
CHANGED
|
@@ -66,9 +66,15 @@ def get_embedding_service(api_key: str | None = None) -> "EmbeddingServiceProtoc
|
|
| 66 |
ImportError: If no embedding service dependencies are available
|
| 67 |
"""
|
| 68 |
# Determine if we have a valid OpenAI key (BYOK or Env)
|
|
|
|
| 69 |
has_openai = False
|
| 70 |
-
if api_key
|
| 71 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 72 |
elif settings.has_openai_key:
|
| 73 |
has_openai = True
|
| 74 |
|
|
|
|
| 66 |
ImportError: If no embedding service dependencies are available
|
| 67 |
"""
|
| 68 |
# Determine if we have a valid OpenAI key (BYOK or Env)
|
| 69 |
+
# Note: Must check sk-ant- BEFORE sk- since Anthropic keys start with sk-ant-
|
| 70 |
has_openai = False
|
| 71 |
+
if api_key:
|
| 72 |
+
if api_key.startswith("sk-ant-"):
|
| 73 |
+
# Anthropic key - not supported for embeddings
|
| 74 |
+
logger.warning("Anthropic keys don't support embeddings, falling back to free tier")
|
| 75 |
+
elif api_key.startswith("sk-"):
|
| 76 |
+
# OpenAI BYOK
|
| 77 |
+
has_openai = True
|
| 78 |
elif settings.has_openai_key:
|
| 79 |
has_openai = True
|
| 80 |
|
tests/unit/agent_factory/test_get_model_auto_detect.py
CHANGED
|
@@ -48,15 +48,21 @@ class TestGetModelAutoDetect:
|
|
| 48 |
model = get_model()
|
| 49 |
assert isinstance(model, HuggingFaceModel)
|
| 50 |
|
| 51 |
-
def
|
| 52 |
-
"""No keys at all β
|
| 53 |
monkeypatch.setattr(settings, "openai_api_key", None)
|
| 54 |
-
monkeypatch.setattr(settings, "hf_token",
|
| 55 |
monkeypatch.setattr(settings, "huggingface_model", "Qwen/Qwen2.5-7B-Instruct")
|
|
|
|
|
|
|
| 56 |
|
| 57 |
-
# Should
|
| 58 |
-
|
| 59 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
| 60 |
|
| 61 |
def test_openai_env_takes_priority_over_huggingface(self, monkeypatch):
|
| 62 |
"""OpenAI env key present β OpenAI wins over HuggingFace."""
|
|
|
|
| 48 |
model = get_model()
|
| 49 |
assert isinstance(model, HuggingFaceModel)
|
| 50 |
|
| 51 |
+
def test_raises_when_no_api_keys_available(self, monkeypatch):
|
| 52 |
+
"""No keys at all β RuntimeError with helpful message."""
|
| 53 |
monkeypatch.setattr(settings, "openai_api_key", None)
|
| 54 |
+
monkeypatch.setattr(settings, "hf_token", None)
|
| 55 |
monkeypatch.setattr(settings, "huggingface_model", "Qwen/Qwen2.5-7B-Instruct")
|
| 56 |
+
# Also ensure HF_TOKEN env var is not set
|
| 57 |
+
monkeypatch.delenv("HF_TOKEN", raising=False)
|
| 58 |
|
| 59 |
+
# Should raise clear error when no tokens available
|
| 60 |
+
import pytest
|
| 61 |
+
|
| 62 |
+
with pytest.raises(RuntimeError) as exc_info:
|
| 63 |
+
get_model()
|
| 64 |
+
assert "No LLM API key available" in str(exc_info.value)
|
| 65 |
+
assert "HF_TOKEN" in str(exc_info.value)
|
| 66 |
|
| 67 |
def test_openai_env_takes_priority_over_huggingface(self, monkeypatch):
|
| 68 |
"""OpenAI env key present β OpenAI wins over HuggingFace."""
|