feat: add Render Free Tier Lite Mode deployment and optimizations - #352
feat: add Render Free Tier Lite Mode deployment and optimizations#352MasumRab wants to merge 1 commit into
Conversation
Co-authored-by: MasumRab <8943353+MasumRab@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Merging to
|
Reviewer's GuideAdds a Render Free Tier deployment guide and corresponding Lite Mode runtime configuration by offloading Gemma/RAG compute to Google GenAI APIs, tightening rate-limit IP handling, and fixing RAG/orchestration/tooling integration and Render-specific setup behavior. Sequence diagram for rag_search tool invocation with new RAG wiringsequenceDiagram
actor User
participant Browser
participant Backend
participant AgentOrchestrator
participant RAGFunctions
participant DeepSearchRAG
participant Embeddings
User->>Browser: Enter query
Browser->>Backend: HTTP request with query
Backend->>AgentOrchestrator: handle_request(query)
AgentOrchestrator->>RAGFunctions: is_rag_enabled()
RAGFunctions-->>AgentOrchestrator: enabled flag
alt RAG enabled
AgentOrchestrator->>RAGFunctions: create_rag_tool(resources)
RAGFunctions->>DeepSearchRAG: __init__(storage_type="chroma")
DeepSearchRAG->>Embeddings: initialize based on embedding_provider
RAGFunctions-->>AgentOrchestrator: DeepSearchRAG instance
AgentOrchestrator->>DeepSearchRAG: retrieve(query)
DeepSearchRAG->>Embeddings: embed_documents(chunks)
Embeddings-->>DeepSearchRAG: vectors
DeepSearchRAG-->>AgentOrchestrator: ranked evidence
else RAG disabled
AgentOrchestrator-->>Backend: no rag_search tool available
end
AgentOrchestrator-->>Backend: response content
Backend-->>Browser: HTTP response
Browser-->>User: Rendered answer
Class diagram for updated Gemma client provider architectureclassDiagram
class GemmaClient {
<<abstract>>
+invoke(prompt: str, kwargs) str
}
class VertexAIGemmaClient {
+invoke(prompt: str, kwargs) str
}
class OllamaGemmaClient {
+invoke(prompt: str, kwargs) str
}
class GoogleGenAIGemmaClient {
-api_key: str
-client
-model_name: str
+__init__()
+invoke(prompt: str, kwargs) str
}
class AppConfig {
+gemma_provider: str
+gemma_model_name: str
}
class GemmaClientFactory {
+get_gemma_client() GemmaClient
}
GemmaClient <|-- VertexAIGemmaClient
GemmaClient <|-- OllamaGemmaClient
GemmaClient <|-- GoogleGenAIGemmaClient
AppConfig ..> GemmaClientFactory : gemma_provider
AppConfig ..> GoogleGenAIGemmaClient : gemma_model_name
GemmaClientFactory ..> VertexAIGemmaClient
GemmaClientFactory ..> OllamaGemmaClient
GemmaClientFactory ..> GoogleGenAIGemmaClient
Class diagram for updated RAG configuration and embeddingsclassDiagram
class _RAGConfig {
+enabled: bool
+enable_fallback: bool
+max_documents: int
+embedding_provider: str
+__init__()
}
class DeepSearchRAG {
+storage_type: str
+max_context_chunks: int
+use_faiss: bool
+use_chroma: bool
+embedder
+faiss_index
+chroma
+__init__(storage_type: str)
+retrieve(query: str)
+export_state() Dict
}
class LangChainEmbeddingAdapter {
+lc_embeddings
+__init__(langchain_embeddings)
+__call__(texts: List~str~) List~List~float~~
+encode(texts: List~str~) List~List~float~~
}
class GoogleGenerativeAIEmbeddings {
+embed_documents(texts: List~str~) List~List~float~~
}
class SentenceTransformer {
+encode(texts: List~str~) List~List~float~~
}
class ChromaStore {
+collection_name: str
+persist_path: str
}
class FAISSIndexFlatL2 {
+add(vectors)
+search(vectors, k: int)
}
class RAGFunctions {
+rag_config: _RAGConfig
+is_rag_enabled() bool
+create_rag_tool(resources) DeepSearchRAG
}
RAGFunctions o-- _RAGConfig : singleton
RAGFunctions ..> DeepSearchRAG : create_rag_tool
DeepSearchRAG ..> _RAGConfig : reads
DeepSearchRAG ..> LangChainEmbeddingAdapter : optional
DeepSearchRAG ..> SentenceTransformer : optional
DeepSearchRAG ..> ChromaStore : optional
DeepSearchRAG ..> FAISSIndexFlatL2 : optional
LangChainEmbeddingAdapter ..> GoogleGenerativeAIEmbeddings : wraps
Class diagram for updated agent orchestration and RAG tool wiringclassDiagram
class AgentOrchestrator {
+_load_default_tools()
+register(name: str, func, description: str, category: str)
}
class DeepSearchRAG {
+retrieve(query: str)
}
class RAGFunctions {
+is_rag_enabled() bool
+create_rag_tool(resources) DeepSearchRAG
}
AgentOrchestrator ..> RAGFunctions : uses
AgentOrchestrator ..> DeepSearchRAG : registers rag_search
Class diagram for updated RateLimitMiddleware IP extractionclassDiagram
class RateLimitMiddleware {
-trust_proxy_headers: bool
+dispatch(request: Request, call_next)
}
class Request {
+headers: dict
+client
}
RateLimitMiddleware ..> Request : reads X-Forwarded-For
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✨ 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the deployability and resource efficiency of the Gemini Fullstack Agent, particularly for free-tier cloud environments like Render. It achieves this by introducing a detailed manual deployment guide, integrating Google GenAI for Gemma and RAG embeddings to offload local computations, and implementing configuration options to disable resource-intensive features. Additionally, it includes crucial fixes for rate-limiting security and RAG tool invocation, ensuring a more robust and adaptable application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The new patch_.py scripts (patch_rag.py, patch_gemma_client.py, patch_orchestration.py, patch_security.py) introduce regex-based source mutation at runtime; consider applying these edits directly to the tracked source files and removing the patch scripts to avoid brittle, order-dependent code generation logic in the repo.
- DeepSearchRAG.init returns early when RAG_ENABLED is false, which leaves instances partially initialized; instead of returning from init, consider not constructing DeepSearchRAG at all (handled in create_rag_tool) or raising an exception so consumers never see a half-initialized object.
- create_rag_tool(resources) now ignores the resources parameter entirely; if callers pass resources expecting them to influence the RAG configuration, consider either wiring that argument into DeepSearchRAG or removing the unused parameter to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new patch_*.py scripts (patch_rag.py, patch_gemma_client*.py, patch_orchestration.py, patch_security.py) introduce regex-based source mutation at runtime; consider applying these edits directly to the tracked source files and removing the patch scripts to avoid brittle, order-dependent code generation logic in the repo.
- DeepSearchRAG.__init__ returns early when RAG_ENABLED is false, which leaves instances partially initialized; instead of returning from __init__, consider not constructing DeepSearchRAG at all (handled in create_rag_tool) or raising an exception so consumers never see a half-initialized object.
- create_rag_tool(resources) now ignores the resources parameter entirely; if callers pass resources expecting them to influence the RAG configuration, consider either wiring that argument into DeepSearchRAG or removing the unused parameter to avoid confusion.
## Individual Comments
### Comment 1
<location path="patch_rag.py" line_range="52-60" />
<code_context>
+ self.embedder = None
+ embedding_function_for_chroma = None
+
+ if rag_config.embedding_provider == "google_genai":
+ try:
+ from langchain_google_genai import GoogleGenerativeAIEmbeddings
+ lc_embeddings = GoogleGenerativeAIEmbeddings(model="models/text-embedding-004")
+ self.embedder = LangChainEmbeddingAdapter(lc_embeddings)
+ embedding_function_for_chroma = self.embedder
+ logger.info("Using Google GenAI for RAG embeddings")
+ except ImportError:
+ logger.warning("langchain-google-genai not found, falling back to local embeddings")
+ rag_config.embedding_provider = "local"
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid mutating global configuration inside DeepSearchRAG initialization
Catching `ImportError` and then changing `rag_config.embedding_provider` turns `_RAGConfig` into mutable runtime state. This can lead to subtle bugs if later code or other `DeepSearchRAG` instances rely on the original configuration. Instead, keep `rag_config` immutable and handle the fallback via a local variable (e.g., `provider = rag_config.embedding_provider` and adjust that) or by storing the resolved provider on the instance (e.g., `self.embedding_provider`), so the config remains the source of truth.
Suggested implementation:
```python
# Load embedding function based on configuration
self.embedder = None
embedding_function_for_chroma = None
# Resolve embedding provider for this instance without mutating global config
provider = rag_config.embedding_provider
if provider == "google_genai":
try:
from langchain_google_genai import GoogleGenerativeAIEmbeddings
lc_embeddings = GoogleGenerativeAIEmbeddings(model="models/text-embedding-004")
self.embedder = LangChainEmbeddingAdapter(lc_embeddings)
embedding_function_for_chroma = self.embedder
logger.info("Using Google GenAI for RAG embeddings")
# Provider successfully resolved
provider = "google_genai"
except ImportError:
logger.warning("langchain-google-genai not found, falling back to local embeddings")
# Fallback only for this instance, do not mutate rag_config
provider = "local"
# Expose the resolved provider on the instance
self.embedding_provider = provider
```
1. Anywhere in `DeepSearchRAG` (or related code in `rag.py`) that currently reads `rag_config.embedding_provider` **after** initialization should be updated to use `self.embedding_provider` instead, to ensure it sees the resolved provider (including the local fallback).
2. If `patch_rag.py` constructs or patches code that branches on `rag_config.embedding_provider` later (e.g. to choose between FAISS/Chroma or different embedding backends), those branches should be updated to use the local `provider` or `self.embedding_provider` consistently.
</issue_to_address>
### Comment 2
<location path="patch_rag.py" line_range="1" />
<code_context>
+import re
+
+with open('backend/src/agent/gemma_client.py', 'r') as f:
</code_context>
<issue_to_address>
**issue (complexity):** Consider moving the new RAG configuration and adapter logic directly into backend/src/agent/rag.py and deleting this regex-based patching script to avoid brittle meta-programming.
You can eliminate the regex‑based patching entirely by moving the new logic directly into `backend/src/agent/rag.py`. This keeps all behavior intact while removing brittle meta‑programming and duplicated templates.
Concretely:
1. **Add the adapter class directly to `rag.py`**
Place this near the other RAG‑related classes:
```python
class LangChainEmbeddingAdapter:
"""Adapter to make LangChain embeddings look like sentence-transformers for Chroma."""
def __init__(self, langchain_embeddings):
self.lc_embeddings = langchain_embeddings
def __call__(self, texts: List[str]) -> List[List[float]]:
# This is the interface Chroma expects if passed as embedding_function
return self.lc_embeddings.embed_documents(texts)
def encode(self, texts: List[str]) -> List[List[float]]:
return self.lc_embeddings.embed_documents(texts)
```
2. **Replace `_RAGConfig` and `is_rag_enabled` in `rag.py`**
Update the existing `_RAGConfig` definition instead of regex‑replacing:
```python
class _RAGConfig:
def __init__(self):
self.enabled = os.getenv("RAG_ENABLED", "true").lower() == "true"
self.enable_fallback = True
self.max_documents = 5
# 'local' or 'google_genai'
self.embedding_provider = os.getenv(
"RAG_EMBEDDING_PROVIDER",
"local",
).lower()
rag_config = _RAGConfig()
def is_rag_enabled() -> bool:
return rag_config.enabled
```
3. **Update `DeepSearchRAG.__init__` in `rag.py`**
Replace just the constructor body with the new behavior (no need for `re.sub`):
```python
class DeepSearchRAG:
...
def __init__(self, storage_type: str = "chroma"):
if not is_rag_enabled():
return
self.storage_type = storage_type
self.max_context_chunks = 20
self.use_faiss = storage_type == "faiss"
self.use_chroma = storage_type == "chroma"
# Load embedding function based on configuration
self.embedder = None
embedding_function_for_chroma = None
if rag_config.embedding_provider == "google_genai":
try:
from langchain_google_genai import GoogleGenerativeAIEmbeddings
lc_embeddings = GoogleGenerativeAIEmbeddings(
model="models/text-embedding-004"
)
self.embedder = LangChainEmbeddingAdapter(lc_embeddings)
embedding_function_for_chroma = self.embedder
logger.info("Using Google GenAI for RAG embeddings")
except ImportError:
logger.warning(
"langchain-google-genai not found, falling back to local embeddings"
)
rag_config.embedding_provider = "local"
if rag_config.embedding_provider == "local":
if SENTENCE_TRANSFORMERS_AVAILABLE:
self.embedder = SentenceTransformer("all-MiniLM-L6-v2")
else:
logger.warning(
"SentenceTransformers not available. Embeddings disabled."
)
if self.use_faiss:
if not FAISS_AVAILABLE:
logger.warning("FAISS not installed, falling back to Chroma.")
self.use_faiss = False
self.use_chroma = True
elif self.embedder:
self.doc_store = {}
# 384 is dimension of all-MiniLM-L6-v2, may break for other models if not handled
self.faiss_index = faiss.IndexFlatL2(384)
if self.use_chroma:
if not CHROMA_AVAILABLE:
logger.warning("ChromaDB not available.")
else:
self.chroma = ChromaStore(
collection_name="deep_search_evidence",
persist_path="./chroma_db",
embedding_function=embedding_function_for_chroma,
)
```
4. **Update `create_rag_tool` directly in `rag.py`**
Replace the function body instead of regex‑patching:
```python
def create_rag_tool(resources):
"""
Returns an instance of DeepSearchRAG configured for tool use.
"""
if not is_rag_enabled():
logger.warning("RAG is disabled via configuration.")
return None
return DeepSearchRAG()
```
5. **Remove the patching script**
Once the above changes are in `rag.py`, you can delete the regex‑based script entirely. All functionality is preserved, but the code is now:
- Single‑sourced (no duplicated templates),
- Easier to debug,
- Free of multi‑line regex text rewriting of Python source.
</issue_to_address>
### Comment 3
<location path="patch_gemma_client.py" line_range="1" />
<code_context>
+import re
+
+with open('backend/src/agent/gemma_client.py', 'r') as f:
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing the regex-based patch script and implementing the GoogleGenAIGemmaClient and updated factory directly in gemma_client.py instead.
The indirection through a regex‑based patch script is unnecessary and brittle; you can keep the exact same functionality by modifying `backend/src/agent/gemma_client.py` directly and deleting this script.
Instead of:
```python
# separate patch script
import re
with open('backend/src/agent/gemma_client.py', 'r') as f:
content = f.read()
# ... giant new_class string ...
content = content.replace("def get_gemma_client() -> GemmaClient:", new_class + "def get_gemma_client() -> GemmaClient:")
content = re.sub(r'def get_gemma_client\(\) -> GemmaClient:.*?return OllamaGemmaClient\(\)', factory_replacement, content, flags=re.DOTALL)
with open('backend/src/agent/gemma_client.py', 'w') as f:
f.write(content)
```
add the class and factory directly in `gemma_client.py`:
```python
class GoogleGenAIGemmaClient(GemmaClient):
"""Client for Gemma models via Google GenAI API."""
def __init__(self):
"""Initialize Google GenAI client."""
try:
from google import genai
except ImportError:
logger.error("google-genai not installed.")
raise ImportError(
"Please install 'google-genai' to use GoogleGenAIGemmaClient"
)
import os
self.api_key = os.getenv("GEMINI_API_KEY")
if not self.api_key:
logger.error("GEMINI_API_KEY environment variable is missing.")
raise ValueError(
"GEMINI_API_KEY is required for GoogleGenAIGemmaClient"
)
self.client = genai.Client(api_key=self.api_key)
self.model_name = app_config.gemma_model_name
def invoke(self, prompt: str, **kwargs) -> str:
"""Generate text completion."""
try:
response = self.client.models.generate_content(
model=self.model_name,
contents=prompt,
)
return response.text
except Exception as e:
logger.error(f"Google GenAI generation failed: {e}")
raise
```
Update the factory function in the same file:
```python
def get_gemma_client() -> GemmaClient:
"""Factory function to get the configured Gemma client."""
provider = (app_config.gemma_provider or "ollama").lower()
if provider == "vertex":
return VertexAIGemmaClient()
elif provider == "ollama":
return OllamaGemmaClient()
elif provider == "google_genai":
return GoogleGenAIGemmaClient()
else:
logger.warning(
f"Unknown or unsupported Gemma provider: {provider}. "
"Defaulting to Ollama."
)
return OllamaGemmaClient()
```
Then remove the patch script entirely.
Benefits:
- Eliminates string duplication of real code and keeps everything type‑checked.
- Removes regex/text‑based mutation of a Python source file, avoiding fragile coupling to `gemma_client.py`’s exact formatting.
- Makes the feature discoverable and reviewable in one place (`gemma_client.py`) instead of requiring readers to mentally apply a patch.
</issue_to_address>
### Comment 4
<location path="patch_gemma_client2.py" line_range="54" />
<code_context>
+ logger.error(f"Google GenAI generation failed: {e}")
+ raise e"""
+
+content = re.sub(r'class GoogleGenAIGemmaClient.*?raise e', class_replacement, content, flags=re.DOTALL)
+
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the regex-based class rewrite and exact multi-line else-block replacement with more explicit, bounded patch regions and pattern-aware logic to make the patch safer and easier to maintain.
The main complexity comes from regex‑patching an entire class body in another file. This is fragile and hard to maintain. You can keep the same behavior while making the transform explicit and safer.
Two concrete ways to reduce complexity:
---
### 1. Use explicit region markers instead of a greedy regex
Instead of:
```python
content = re.sub(
r'class GoogleGenAIGemmaClient.*?raise e',
class_replacement,
content,
flags=re.DOTALL,
)
```
have `backend/src/agent/gemma_client.py` expose an explicit replacement region:
```python
# gemma_client.py
# BEGIN_GOOGLE_GENAI_GEMMA_CLIENT
class GoogleGenAIGemmaClient(GemmaClient):
...
# END_GOOGLE_GENAI_GEMMA_CLIENT
```
Then your patch script becomes a simple, bounded replacement, not a file‑wide regex:
```python
start = '# BEGIN_GOOGLE_GENAI_GEMMA_CLIENT'
end = '# END_GOOGLE_GENAI_GEMMA_CLIENT'
start_idx = content.index(start) + len(start)
end_idx = content.index(end)
content = (
content[:start_idx]
+ '\n' + class_replacement.strip() + '\n'
+ content[end_idx:]
)
```
This keeps the same end result but makes the patch:
- Localized and obvious.
- Robust against unrelated changes elsewhere in the file.
- Easier for future reviewers to understand.
---
### 2. Avoid full‑string duplicate block replacement
The duplicate `else` fix currently relies on an exact multi‑line string match, which will break on minor formatting changes. If you want to keep this fix in a patch script, you can target the duplicated block more defensively.
For example, replace only the second `else` by searching for the specific pattern around it:
```python
needle = """ else:
logger.warning(f"Unknown or unsupported Gemma provider: {provider}. Defaulting to Ollama.")
return OllamaGemmaClient()
"""
occurrences = [m.start() for m in re.finditer(re.escape(needle), content)]
if len(occurrences) > 1:
# keep the first, remove extras
first = occurrences[0]
for pos in reversed(occurrences[1:]):
content = content[:pos] + content[pos + len(needle):]
```
Functionality is unchanged (duplicate `else` is removed), but the script no longer depends on a single exact multi‑line literal and is less brittle to whitespace or logging‑format tweaks.
Both of these keep your “patching” approach but reduce the meta‑programming complexity and maintenance risk that the reviewer called out.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if rag_config.embedding_provider == "google_genai": | ||
| try: | ||
| from langchain_google_genai import GoogleGenerativeAIEmbeddings | ||
| lc_embeddings = GoogleGenerativeAIEmbeddings(model="models/text-embedding-004") | ||
| self.embedder = LangChainEmbeddingAdapter(lc_embeddings) | ||
| embedding_function_for_chroma = self.embedder | ||
| logger.info("Using Google GenAI for RAG embeddings") | ||
| except ImportError: | ||
| logger.warning("langchain-google-genai not found, falling back to local embeddings") |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid mutating global configuration inside DeepSearchRAG initialization
Catching ImportError and then changing rag_config.embedding_provider turns _RAGConfig into mutable runtime state. This can lead to subtle bugs if later code or other DeepSearchRAG instances rely on the original configuration. Instead, keep rag_config immutable and handle the fallback via a local variable (e.g., provider = rag_config.embedding_provider and adjust that) or by storing the resolved provider on the instance (e.g., self.embedding_provider), so the config remains the source of truth.
Suggested implementation:
# Load embedding function based on configuration
self.embedder = None
embedding_function_for_chroma = None
# Resolve embedding provider for this instance without mutating global config
provider = rag_config.embedding_provider
if provider == "google_genai":
try:
from langchain_google_genai import GoogleGenerativeAIEmbeddings
lc_embeddings = GoogleGenerativeAIEmbeddings(model="models/text-embedding-004")
self.embedder = LangChainEmbeddingAdapter(lc_embeddings)
embedding_function_for_chroma = self.embedder
logger.info("Using Google GenAI for RAG embeddings")
# Provider successfully resolved
provider = "google_genai"
except ImportError:
logger.warning("langchain-google-genai not found, falling back to local embeddings")
# Fallback only for this instance, do not mutate rag_config
provider = "local"
# Expose the resolved provider on the instance
self.embedding_provider = provider- Anywhere in
DeepSearchRAG(or related code inrag.py) that currently readsrag_config.embedding_providerafter initialization should be updated to useself.embedding_providerinstead, to ensure it sees the resolved provider (including the local fallback). - If
patch_rag.pyconstructs or patches code that branches onrag_config.embedding_providerlater (e.g. to choose between FAISS/Chroma or different embedding backends), those branches should be updated to use the localproviderorself.embedding_providerconsistently.
| @@ -0,0 +1,105 @@ | |||
| import re | |||
There was a problem hiding this comment.
issue (complexity): Consider moving the new RAG configuration and adapter logic directly into backend/src/agent/rag.py and deleting this regex-based patching script to avoid brittle meta-programming.
You can eliminate the regex‑based patching entirely by moving the new logic directly into backend/src/agent/rag.py. This keeps all behavior intact while removing brittle meta‑programming and duplicated templates.
Concretely:
- Add the adapter class directly to
rag.py
Place this near the other RAG‑related classes:
class LangChainEmbeddingAdapter:
"""Adapter to make LangChain embeddings look like sentence-transformers for Chroma."""
def __init__(self, langchain_embeddings):
self.lc_embeddings = langchain_embeddings
def __call__(self, texts: List[str]) -> List[List[float]]:
# This is the interface Chroma expects if passed as embedding_function
return self.lc_embeddings.embed_documents(texts)
def encode(self, texts: List[str]) -> List[List[float]]:
return self.lc_embeddings.embed_documents(texts)- Replace
_RAGConfigandis_rag_enabledinrag.py
Update the existing _RAGConfig definition instead of regex‑replacing:
class _RAGConfig:
def __init__(self):
self.enabled = os.getenv("RAG_ENABLED", "true").lower() == "true"
self.enable_fallback = True
self.max_documents = 5
# 'local' or 'google_genai'
self.embedding_provider = os.getenv(
"RAG_EMBEDDING_PROVIDER",
"local",
).lower()
rag_config = _RAGConfig()
def is_rag_enabled() -> bool:
return rag_config.enabled- Update
DeepSearchRAG.__init__inrag.py
Replace just the constructor body with the new behavior (no need for re.sub):
class DeepSearchRAG:
...
def __init__(self, storage_type: str = "chroma"):
if not is_rag_enabled():
return
self.storage_type = storage_type
self.max_context_chunks = 20
self.use_faiss = storage_type == "faiss"
self.use_chroma = storage_type == "chroma"
# Load embedding function based on configuration
self.embedder = None
embedding_function_for_chroma = None
if rag_config.embedding_provider == "google_genai":
try:
from langchain_google_genai import GoogleGenerativeAIEmbeddings
lc_embeddings = GoogleGenerativeAIEmbeddings(
model="models/text-embedding-004"
)
self.embedder = LangChainEmbeddingAdapter(lc_embeddings)
embedding_function_for_chroma = self.embedder
logger.info("Using Google GenAI for RAG embeddings")
except ImportError:
logger.warning(
"langchain-google-genai not found, falling back to local embeddings"
)
rag_config.embedding_provider = "local"
if rag_config.embedding_provider == "local":
if SENTENCE_TRANSFORMERS_AVAILABLE:
self.embedder = SentenceTransformer("all-MiniLM-L6-v2")
else:
logger.warning(
"SentenceTransformers not available. Embeddings disabled."
)
if self.use_faiss:
if not FAISS_AVAILABLE:
logger.warning("FAISS not installed, falling back to Chroma.")
self.use_faiss = False
self.use_chroma = True
elif self.embedder:
self.doc_store = {}
# 384 is dimension of all-MiniLM-L6-v2, may break for other models if not handled
self.faiss_index = faiss.IndexFlatL2(384)
if self.use_chroma:
if not CHROMA_AVAILABLE:
logger.warning("ChromaDB not available.")
else:
self.chroma = ChromaStore(
collection_name="deep_search_evidence",
persist_path="./chroma_db",
embedding_function=embedding_function_for_chroma,
)- Update
create_rag_tooldirectly inrag.py
Replace the function body instead of regex‑patching:
def create_rag_tool(resources):
"""
Returns an instance of DeepSearchRAG configured for tool use.
"""
if not is_rag_enabled():
logger.warning("RAG is disabled via configuration.")
return None
return DeepSearchRAG()- Remove the patching script
Once the above changes are in rag.py, you can delete the regex‑based script entirely. All functionality is preserved, but the code is now:
- Single‑sourced (no duplicated templates),
- Easier to debug,
- Free of multi‑line regex text rewriting of Python source.
| @@ -0,0 +1,60 @@ | |||
| import re | |||
There was a problem hiding this comment.
issue (complexity): Consider removing the regex-based patch script and implementing the GoogleGenAIGemmaClient and updated factory directly in gemma_client.py instead.
The indirection through a regex‑based patch script is unnecessary and brittle; you can keep the exact same functionality by modifying backend/src/agent/gemma_client.py directly and deleting this script.
Instead of:
# separate patch script
import re
with open('backend/src/agent/gemma_client.py', 'r') as f:
content = f.read()
# ... giant new_class string ...
content = content.replace("def get_gemma_client() -> GemmaClient:", new_class + "def get_gemma_client() -> GemmaClient:")
content = re.sub(r'def get_gemma_client\(\) -> GemmaClient:.*?return OllamaGemmaClient\(\)', factory_replacement, content, flags=re.DOTALL)
with open('backend/src/agent/gemma_client.py', 'w') as f:
f.write(content)add the class and factory directly in gemma_client.py:
class GoogleGenAIGemmaClient(GemmaClient):
"""Client for Gemma models via Google GenAI API."""
def __init__(self):
"""Initialize Google GenAI client."""
try:
from google import genai
except ImportError:
logger.error("google-genai not installed.")
raise ImportError(
"Please install 'google-genai' to use GoogleGenAIGemmaClient"
)
import os
self.api_key = os.getenv("GEMINI_API_KEY")
if not self.api_key:
logger.error("GEMINI_API_KEY environment variable is missing.")
raise ValueError(
"GEMINI_API_KEY is required for GoogleGenAIGemmaClient"
)
self.client = genai.Client(api_key=self.api_key)
self.model_name = app_config.gemma_model_name
def invoke(self, prompt: str, **kwargs) -> str:
"""Generate text completion."""
try:
response = self.client.models.generate_content(
model=self.model_name,
contents=prompt,
)
return response.text
except Exception as e:
logger.error(f"Google GenAI generation failed: {e}")
raiseUpdate the factory function in the same file:
def get_gemma_client() -> GemmaClient:
"""Factory function to get the configured Gemma client."""
provider = (app_config.gemma_provider or "ollama").lower()
if provider == "vertex":
return VertexAIGemmaClient()
elif provider == "ollama":
return OllamaGemmaClient()
elif provider == "google_genai":
return GoogleGenAIGemmaClient()
else:
logger.warning(
f"Unknown or unsupported Gemma provider: {provider}. "
"Defaulting to Ollama."
)
return OllamaGemmaClient()Then remove the patch script entirely.
Benefits:
- Eliminates string duplication of real code and keeps everything type‑checked.
- Removes regex/text‑based mutation of a Python source file, avoiding fragile coupling to
gemma_client.py’s exact formatting. - Makes the feature discoverable and reviewable in one place (
gemma_client.py) instead of requiring readers to mentally apply a patch.
| logger.error(f"Google GenAI generation failed: {e}") | ||
| raise e""" | ||
|
|
||
| content = re.sub(r'class GoogleGenAIGemmaClient.*?raise e', class_replacement, content, flags=re.DOTALL) |
There was a problem hiding this comment.
issue (complexity): Consider replacing the regex-based class rewrite and exact multi-line else-block replacement with more explicit, bounded patch regions and pattern-aware logic to make the patch safer and easier to maintain.
The main complexity comes from regex‑patching an entire class body in another file. This is fragile and hard to maintain. You can keep the same behavior while making the transform explicit and safer.
Two concrete ways to reduce complexity:
1. Use explicit region markers instead of a greedy regex
Instead of:
content = re.sub(
r'class GoogleGenAIGemmaClient.*?raise e',
class_replacement,
content,
flags=re.DOTALL,
)have backend/src/agent/gemma_client.py expose an explicit replacement region:
# gemma_client.py
# BEGIN_GOOGLE_GENAI_GEMMA_CLIENT
class GoogleGenAIGemmaClient(GemmaClient):
...
# END_GOOGLE_GENAI_GEMMA_CLIENTThen your patch script becomes a simple, bounded replacement, not a file‑wide regex:
start = '# BEGIN_GOOGLE_GENAI_GEMMA_CLIENT'
end = '# END_GOOGLE_GENAI_GEMMA_CLIENT'
start_idx = content.index(start) + len(start)
end_idx = content.index(end)
content = (
content[:start_idx]
+ '\n' + class_replacement.strip() + '\n'
+ content[end_idx:]
)This keeps the same end result but makes the patch:
- Localized and obvious.
- Robust against unrelated changes elsewhere in the file.
- Easier for future reviewers to understand.
2. Avoid full‑string duplicate block replacement
The duplicate else fix currently relies on an exact multi‑line string match, which will break on minor formatting changes. If you want to keep this fix in a patch script, you can target the duplicated block more defensively.
For example, replace only the second else by searching for the specific pattern around it:
needle = """ else:
logger.warning(f"Unknown or unsupported Gemma provider: {provider}. Defaulting to Ollama.")
return OllamaGemmaClient()
"""
occurrences = [m.start() for m in re.finditer(re.escape(needle), content)]
if len(occurrences) > 1:
# keep the first, remove extras
first = occurrences[0]
for pos in reversed(occurrences[1:]):
content = content[:pos] + content[pos + len(needle):]Functionality is unchanged (duplicate else is removed), but the script no longer depends on a single exact multi‑line literal and is less brittle to whitespace or logging‑format tweaks.
Both of these keep your “patching” approach but reduce the meta‑programming complexity and maintenance risk that the reviewer called out.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a "Lite Mode" for deployment on Render's free tier, which cleverly offloads computation to Google's GenAI APIs. The changes include a new deployment guide, a new GoogleGenAIGemmaClient, and configuration options to switch between local and cloud-based providers for both LLM inference and RAG embeddings. The PR also contains important bug fixes, including a security fix for IP spoofing in the rate limiter and a fix for tool invocation in the agent orchestrator. My review focuses on improving code style and error handling in the new Gemma client. Overall, this is a solid set of changes that significantly enhances the deployability and flexibility of the agent.
| logger.error("google-genai not installed.") | ||
| raise ImportError("Please install 'google-genai' to use GoogleGenAIGemmaClient") | ||
|
|
||
| import os |
There was a problem hiding this comment.
According to PEP 8 style guidelines, imports should be placed at the top of the file. Please move import os to the top-level of the module to improve code organization and readability.
References
- PEP 8, the style guide for Python code, recommends that all imports should be at the top of the file, just after any module comments and docstrings, and before module globals and constants. (link)
| return response.text | ||
| except Exception as e: | ||
| logger.error(f"Google GenAI generation failed: {e}") | ||
| raise e |
There was a problem hiding this comment.
Using raise without an argument within an except block re-raises the original exception, preserving its full stack trace. This is generally preferred over raise e as it provides more context for debugging. The original traceback helps pinpoint the root cause of the error more effectively.
| raise e | |
| raise |
References
- When re-raising an exception, use a bare
raisestatement. This preserves the original exception's traceback, which is crucial for effective debugging and error analysis. Usingraise eresets the traceback to the point of the re-raise, losing valuable context about the error's origin.
|
@jules Resolve conflicts: Avoid full repo diff - focus only on your changed paths. |
1 participant
DEPLOY_RENDER.mdfor manual web service without Blueprint requirements.GoogleGenAIGemmaClientusinggoogle-genaito offload local Ollama compute to API, mapped viaGEMMA_PROVIDER=google_genai.agent.ragby adding an adapter to uselangchain-google-genaiembeddings (RAG_EMBEDDING_PROVIDER=google_genai) and handlingRAG_ENABLED=falsegracefully.setup_env.shinstructions and Playwright skips forRENDER=true.RateLimitMiddlewaretest spoofing vulnerability andagent.orchestration.pytool resolution.PR created automatically by Jules for task 11438400044889294952 started by @MasumRab
Summary by Sourcery
Add a lightweight Render free-tier deployment mode that relies on Google GenAI-hosted models instead of local compute, and harden RAG, rate limiting, and tooling behavior to work reliably in that environment.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: