Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 17 additions & 30 deletions RAGManager/app/agents/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@
from app.agents.nodes import (
agent_host,
context_builder,
fallback_final,
fallback_inicial,
generator,
fallback,
guard,
Comment thread
NicoWagner2005 marked this conversation as resolved.
parafraseo,
retriever,
)
from app.agents.routing import route_after_fallback_final, route_after_guard
from app.agents.state import AgentState

from app.agents.routing import route_after_guard

def create_agent_graph() -> StateGraph:
"""
Expand All @@ -23,13 +20,13 @@ def create_agent_graph() -> StateGraph:
The graph implements the following flow:
1. START -> agent_host (Nodo 1)
2. agent_host -> guard (Nodo 2)
3. guard -> [conditional] -> fallback_inicial (Nodo 3) or END
4. fallback_inicial -> parafraseo (Nodo 4)
3. guard -> [conditional] -> fallback (Nodo 3) or END
4. fallback -> parafraseo (Nodo 4)
5. parafraseo -> retriever (Nodo 5)
6. retriever -> context_builder (Nodo 6)
7. context_builder -> generator (Nodo 7)
8. generator -> fallback_final (Nodo 8)
9. fallback_final -> [conditional] -> END (with final_response) or END (with error)
8. generator -> fallback (Nodo 8)
9. fallback -> [conditional] -> END (with final_response) or END (with error)
Comment on lines 23 to +29
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation describes an outdated workflow that doesn't match the actual implementation. It mentions steps 7 (generator) and 8 (fallback as Nodo 8) that no longer exist in the code. The actual flow is: agent_host -> guard -> [fallback or parafraseo] -> retriever -> context_builder -> guard -> [fallback or END]. The documentation should be updated to reflect the current workflow.

Copilot uses AI. Check for mistakes.
Comment on lines +23 to +29
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation is outdated and does not match the actual workflow implementation. According to the code, after guard detects a malicious prompt, it goes to fallback and then END, not "fallback (Nodo 3) or END". The documentation should reflect that fallback is always a node that terminates the flow.

Copilot uses AI. Check for mistakes.

Returns:
Configured StateGraph instance ready for execution
Expand All @@ -40,12 +37,10 @@ def create_agent_graph() -> StateGraph:
# Add nodes
workflow.add_node("agent_host", agent_host)
workflow.add_node("guard", guard)
workflow.add_node("fallback_inicial", fallback_inicial)
workflow.add_node("fallback", fallback)
workflow.add_node("parafraseo", parafraseo)
workflow.add_node("retriever", retriever)
workflow.add_node("context_builder", context_builder)
workflow.add_node("generator", generator)
workflow.add_node("fallback_final", fallback_final)

# Define edges
# Start -> agent_host
Expand All @@ -59,37 +54,29 @@ def create_agent_graph() -> StateGraph:
"guard",
route_after_guard,
{
"malicious": END, # End with error if malicious
"continue": "fallback_inicial", # Continue to fallback_inicial if valid
"malicious": "fallback", # go to fallback if malicious
"continue": "parafraseo", # Continue to parafraseo if valid
},
)

# fallback_inicial -> parafraseo
workflow.add_edge("fallback_inicial", "parafraseo")

# parafraseo -> retriever
workflow.add_edge("parafraseo", "retriever")

# retriever -> context_builder
workflow.add_edge("retriever", "context_builder")

# context_builder -> generator
# Note: Primary LLM is called within context_builder node
workflow.add_edge("context_builder", "generator")
# context_builder -> guard
workflow.add_edge("context_builder", "guard")

# generator -> fallback_final
workflow.add_edge("generator", "fallback_final")

# fallback_final -> conditional routing
# guard -> conditional routing
workflow.add_conditional_edges(
"fallback_final",
route_after_fallback_final,
"guard",
route_after_guard,
{
"risky": END, # End with error if risky
"continue": END, # End with final_response if valid
# Note: Final LLM is called within fallback_final node
"malicious": "fallback", # go to fallback if malicious
"continue": END, # if there's no error ends
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment contains a grammatical error. It should read "if there's no error, ends" or better yet "End if no error is detected".

Suggested change
"continue": END, # if there's no error ends
"continue": END, # End if no error is detected

Copilot uses AI. Check for mistakes.
},
)
Comment on lines 72 to 79
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplicate conditional edge definition for the "guard" node. Lines 58-65 already define conditional edges for "guard" with route_after_guard. In LangGraph, adding a second conditional edge to the same node will overwrite the first one, meaning the routing defined at lines 58-65 will be ignored and only this second definition will be active. This creates a logical error where the workflow cannot reach the parafraseo node at all, since the first guard check is effectively removed.

Copilot uses AI. Check for mistakes.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +68 to 79
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The graph has two conditional edges from the 'guard' node (lines 53-60 and 72-79), but the workflow uses the same guard node for both inicial and final validation. This creates conflicting routing logic. The second guard should be a separate node called 'guard_final' to validate the generated response for PII, as indicated by the guard_final.py file that was created but never integrated into the graph.

Copilot uses AI. Check for mistakes.

workflow.add_edge("fallback", END)
# Compile the graph
return workflow.compile()
8 changes: 2 additions & 6 deletions RAGManager/app/agents/nodes/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,16 @@

from app.agents.nodes.agent_host import agent_host
from app.agents.nodes.context_builder import context_builder
from app.agents.nodes.fallback_final import fallback_final
from app.agents.nodes.fallback_inicial import fallback_inicial
from app.agents.nodes.generator import generator
from app.agents.nodes.fallback import fallback
from app.agents.nodes.guard import guard
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import statement is importing from 'guard' module, but the actual file is named 'guard_inicial.py'. This will cause an ImportError. Either rename the file to 'guard.py' or change the import to 'from app.agents.nodes.guard_inicial import guard_inicial'.

Copilot uses AI. Check for mistakes.
from app.agents.nodes.parafraseo import parafraseo
from app.agents.nodes.retriever import retriever

__all__ = [
"agent_host",
"guard",
"fallback_inicial",
"fallback",
"parafraseo",
"retriever",
"context_builder",
"generator",
"fallback_final",
]
5 changes: 4 additions & 1 deletion RAGManager/app/agents/nodes/agent_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ def agent_host(state: AgentState) -> AgentState:

# Placeholder: For now, we'll just store the prompt as initial context
updated_state = state.copy()
updated_state["initial_context"] = state.get("prompt", "")
initial_message = state["messages"][-1]
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code accesses state["messages"][-1] without first checking if the messages list is empty. If state["messages"] is an empty list, this will raise an IndexError. Add a check to ensure the messages list is not empty before accessing the last element.

Suggested change
initial_message = state["messages"][-1]
initial_message = state["messages"][-1] if state["messages"] else None

Copilot uses AI. Check for mistakes.
updated_state["initial_context"] = (
initial_message.content if initial_message else ""
)
Comment thread
NicoWagner2005 marked this conversation as resolved.
Outdated

return updated_state
Comment on lines 31 to 144
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function returns updated_state which is a copy of the entire state. However, most other nodes in the workflow return partial state updates (just dict with changed fields). For consistency with MessagesState patterns and other nodes like context_builder and fallback, this should return a dict with just the updated fields: {'initial_context': initial_message.content if initial_message else ''}.

Copilot uses AI. Check for mistakes.
25 changes: 17 additions & 8 deletions RAGManager/app/agents/nodes/context_builder.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
"""Nodo 6: Context Builder - Enriches query with retrieved context."""

from app.agents.state import AgentState
from langchain_core.messages import SystemMessage
from langchain_openai import ChatOpenAI

llm = ChatOpenAI(model="gpt-5-nano")
Comment thread
NicoWagner2005 marked this conversation as resolved.
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LLM is initialized at module level, which means it will be created immediately when the module is imported, even if the context_builder function is never called. This could waste resources during startup or testing. Consider using lazy initialization with a function like _get_llm() similar to the pattern used in guard_inicial.py and guard_final.py.

Copilot uses AI. Check for mistakes.


def context_builder(state: AgentState) -> AgentState:
Comment thread
NicoWagner2005 marked this conversation as resolved.
Expand Down Expand Up @@ -31,13 +35,18 @@ def context_builder(state: AgentState) -> AgentState:
paraphrased = state.get("paraphrased_text", "")
chunks = state.get("relevant_chunks", [])

# Build enriched query
context_section = "\n\n".join(chunks) if chunks else ""
enriched_query = f"{paraphrased}\n\nContext:\n{context_section}" if context_section else paraphrased
updated_state["enriched_query"] = enriched_query
# Build enriched query with context
context_section = "\n\n".join(chunks) if chunks else "No relevant context found."

system_content = f"""You are a helpful assistant. Use the following context to answer the user's question.
If the answer is not in the context, say you don't know.

Context:
{context_section}"""

messages = [SystemMessage(content=system_content)] + state["messages"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use defensive access for state["messages"] to avoid KeyError.

Direct dictionary access will raise KeyError if messages is not present in the state. Use .get() with a default empty list for consistency with other state access patterns in this function.

-    messages = [SystemMessage(content=system_content)] + state["messages"]
+    messages = [SystemMessage(content=system_content)] + state.get("messages", [])
📝 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
messages = [SystemMessage(content=system_content)] + state["messages"]
messages = [SystemMessage(content=system_content)] + state.get("messages", [])
🤖 Prompt for AI Agents
In RAGManager/app/agents/nodes/context_builder.py around line 47, the code
directly accesses state["messages"] which can raise KeyError if messages is
missing; change to use state.get("messages", []) so it falls back to an empty
list (preserving existing behavior and matching other defensive access patterns
in this function), and ensure the resulting messages variable is a list before
concatenation.

Comment on lines 31 to +47
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function uses state.get("paraphrased_text", "") but this value is set by the parafraseo node and should always exist at this point in the workflow. However, it's never used in the actual LLM invocation - only the messages from state["messages"] and the context are used. This means the paraphrasing step is effectively ignored, which is likely a bug. Either use the paraphrased text in the query, or reconsider if the paraphrasing step is needed.

Copilot uses AI. Check for mistakes.

# TODO: Call Primary LLM here
# updated_state["primary_response"] = call_primary_llm(enriched_query)
updated_state["primary_response"] = None
# Call Primary LLM
response = llm.invoke(messages)

return updated_state
return {"messages": [response]}
42 changes: 42 additions & 0 deletions RAGManager/app/agents/nodes/fallback.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
"""Nodo 3: Fallback Inicial - Initial fallback processing."""
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says 'Nodo 3: Fallback Inicial' but this node is now used as a general fallback for both inicial and final validation failures (as seen in the graph routing). The documentation should be updated to reflect its dual purpose.

Suggested change
"""Nodo 3: Fallback Inicial - Initial fallback processing."""
"""Nodo 3: Fallback General - Handles fallback processing for both initial and final validation failures."""

Copilot uses AI. Check for mistakes.

Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module docstring incorrectly states "Nodo 3: Fallback Inicial" when this node is now a unified fallback that can be called from different points in the workflow (both after initial guard and after context_builder). The docstring should be updated to reflect the new unified purpose.

Suggested change
"""Nodo 3: Fallback Inicial - Initial fallback processing."""
"""Unified fallback node - Handles fallback processing from multiple workflow points.
This node can be invoked after the initial guard or after the context builder,
providing a consistent fallback mechanism across the workflow.
"""

Copilot uses AI. Check for mistakes.
import logging

from app.agents.state import AgentState
from langchain_core.messages import SystemMessage
from langchain_openai import ChatOpenAI

logger = logging.getLogger(__name__)

llm = ChatOpenAI(
model="gpt-5-nano",
)
Comment on lines +11 to +13
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Invalid model name gpt-5-nano.

Same issue as in parafraseo.py - this model doesn't exist and will cause a runtime error.

 llm = ChatOpenAI(
-    model="gpt-5-nano",
+    model="gpt-4o-mini",
 )
📝 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
llm = ChatOpenAI(
model="gpt-5-nano",
)
llm = ChatOpenAI(
model="gpt-4o-mini",
)
🤖 Prompt for AI Agents
RAGManager/app/agents/nodes/fallback.py around lines 11-13: the ChatOpenAI
instantiation uses an invalid model name "gpt-5-nano" which will raise at
runtime; change it to a valid model (e.g., "gpt-4o-mini" or your project’s
configured default/ENV model variable) and make it consistent with parafraseo.py
(use the same valid model or centralize the model name into a config/env var and
reference that here).

Comment on lines +11 to +13
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LLM is initialized at module level, which means it will be created immediately when the module is imported, even if the fallback function is never called. This could waste resources during startup or testing. Consider using lazy initialization with a function like _get_llm() similar to the pattern used in guard_inicial.py and guard_final.py.

Copilot uses AI. Check for mistakes.

# TO DO: implementar clase nodo fallback y inicializar el llm en el init
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Track the TODO for class-based refactor.

This comment indicates planned work to refactor into a class with proper initialization.

Would you like me to open an issue to track this refactor, or help implement a class-based node structure?

🤖 Prompt for AI Agents
RAGManager/app/agents/nodes/fallback.py around line 15: there's a TODO noting to
implement a fallback node as a class and initialize the LLM in __init__; replace
the inline/TODO approach by creating a FallbackNode class that accepts
configuration (e.g., llm settings, logger) in its constructor, initialize the
LLM client/adapter in __init__ (handling errors and retries), expose a clear
public method (e.g., handle(input) or run(context)) to perform fallback logic,
and update any call sites to instantiate and use this class instead of
procedural code; include docstring and simple unit tests to validate
initialization and fallback behavior.

Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment contains "TO DO" which should be written as "TODO" (one word, no space) for consistency with standard TODO comment conventions.

Suggested change
# TO DO: implementar clase nodo fallback y inicializar el llm en el init
# TODO: implementar clase nodo fallback y inicializar el llm en el init

Copilot uses AI. Check for mistakes.
def fallback(state: AgentState) -> AgentState:
"""
Fallback node - Performs fallback processing.

This node:
1. Alerts about malicious prompt
2. Generates an error_message from llm to show the user

Args:
state: Agent state containing the prompt or initial context

Returns:
error_message
Comment on lines +27 to +28
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Docstring and return value inconsistency.

The docstring states it returns error_message, but the function returns {"messages": [error_message]}. Additionally, the AgentState schema has an error_message field that isn't being set. Consider updating the return to set error_message for downstream nodes that may rely on it.

     error_message = llm.invoke(messages)
-    return {"messages": [error_message]}
+    return {"messages": [error_message], "error_message": error_message.content}

Also applies to: 41-41

🤖 Prompt for AI Agents
In RAGManager/app/agents/nodes/fallback.py around lines 27-28 (and also at line
41), the docstring claims the function returns error_message but the function
actually returns {"messages": [error_message]} and doesn't set
AgentState.error_message; update the function to (1) set the
AgentState.error_message field to the error string before returning, (2) return
a structure that matches the docstring (or update the docstring to reflect
returning both error_message and messages), and (3) ensure downstream callers
expect AgentState.error_message — i.e., assign state.error_message =
error_message and return either {"error_message": error_message, "messages":
[error_message]} or change the docstring to match the current return shape
consistently at both locations.

Comment on lines +22 to +28
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring states the function returns "error_message", but the actual return statement returns a dictionary with a "messages" key containing the LLM response. The docstring should accurately describe the return value format to match the implementation.

Suggested change
2. Generates an error_message from llm to show the user
Args:
state: Agent state containing the prompt or initial context
Returns:
error_message
2. Generates an error message from llm to show the user
Args:
state: Agent state containing the prompt or initial context
Returns:
dict: A dictionary with a "messages" key containing a list with the generated error message from the LLM.

Copilot uses AI. Check for mistakes.
"""

logger.warning(
"Defensive check triggered: Malicious prompt detected"
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message says 'Defensive check triggered' but this is now a fallback handler, not a defensive check. Also, since this node handles both malicious prompts and risky responses, the message should be more generic.

Suggested change
"Defensive check triggered: Malicious prompt detected"
"Fallback handler triggered: Malicious prompt or risky response detected."

Copilot uses AI. Check for mistakes.
)

messages = [
SystemMessage(
content="Your job is to generate an error message in user's language for the user explaining the database doesn't have the information to respond what the user asked"
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message prompt says 'explaining the database doesn't have the information' which is inaccurate. This fallback is triggered by malicious/risky content detection, not by missing information in the database. The prompt should instruct the LLM to generate a message explaining that the request cannot be processed due to security or safety concerns.

Suggested change
content="Your job is to generate an error message in user's language for the user explaining the database doesn't have the information to respond what the user asked"
content="Your job is to generate an error message in the user's language for the user, explaining that their request cannot be processed due to security or safety concerns."

Copilot uses AI. Check for mistakes.
)
] + state["messages"]
Comment on lines +35 to +39
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use defensive access for state["messages"].

Direct dictionary access state["messages"] will raise KeyError if the key is missing. Use .get() with a default for consistency with other state access patterns.

     messages = [
         SystemMessage(content=system_message_content)
-    ] + state["messages"]
+    ] + state.get("messages", [])
📝 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
messages = [
SystemMessage(content=system_message_content)
] + state["messages"]
messages = [
SystemMessage(content=system_message_content)
] + state.get("messages", [])
🤖 Prompt for AI Agents
In RAGManager/app/agents/nodes/fallback.py around lines 70-72, the code accesses
state["messages"] directly which can raise KeyError; replace that direct access
with defensive access using state.get("messages", []) (or wrap with list(...) /
ensure it's a list) so the messages concatenation becomes stable and consistent
with other state access patterns.

error_message = llm.invoke(messages)
Comment on lines +35 to +40
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

System prompt is misleading for malicious input handling.

The prompt instructs the LLM to say "the database doesn't have the information," but this node handles malicious prompts. This misleads users about why their request failed. Consider a prompt that politely declines without revealing detection logic.

     messages = [
         SystemMessage(
-            content="Your job is to generate an error message in user's language for the user explaining the database doesn't have the information to respond what the user asked"
+            content="Your job is to generate a polite error message in the user's language explaining that you cannot process this request. Do not reveal that it was flagged as malicious."
         )
     ] + state["messages"]
🤖 Prompt for AI Agents
In RAGManager/app/agents/nodes/fallback.py around lines 35-40, the SystemMessage
currently instructs the LLM to claim "the database doesn't have the information"
which is misleading for malicious or disallowed inputs; change the system prompt
to instead instruct the model to politely decline disallowed requests without
revealing detection logic (e.g., apologize, state it cannot assist with that
request, offer safe alternatives or resources), ensure the refusal is phrased in
the user's language, and keep the rest of the message flow intact so the node
returns a polite, non-revealing decline for malicious prompts.

return {"messages": [error_message]}
Comment on lines +35 to +41
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add error handling for LLM invocation in fallback node.

The llm.invoke() call has no error handling. Since this is the fallback node (a last-resort error handler), if the LLM call fails due to network issues, API errors, or the invalid model name, the entire fallback will crash. Consider wrapping the invocation in try/except and returning a hardcoded fallback message on failure.

     messages = [
         SystemMessage(content=system_message_content)
     ] + state["messages"]
     
-    error_message = llm.invoke(messages)
+    try:
+        error_message = llm.invoke(messages)
+    except Exception as e:
+        logger.error(f"LLM invocation failed in fallback node: {e}")
+        from langchain_core.messages import AIMessage
+        error_message = AIMessage(
+            content="I'm sorry, but I cannot process your request at this time. Please try again later."
+        )
+    
     return {"messages": [error_message]}
📝 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
messages = [
SystemMessage(content=system_message_content)
] + state["messages"]
error_message = llm.invoke(messages)
return {"messages": [error_message]}
messages = [
SystemMessage(content=system_message_content)
] + state["messages"]
try:
error_message = llm.invoke(messages)
except Exception as e:
logger.error(f"LLM invocation failed in fallback node: {e}")
from langchain_core.messages import AIMessage
error_message = AIMessage(
content="I'm sorry, but I cannot process your request at this time. Please try again later."
)
return {"messages": [error_message]}
🤖 Prompt for AI Agents
In RAGManager/app/agents/nodes/fallback.py around lines 62-67, the
llm.invoke(messages) call is unprotected and can raise (network/API/model)
errors causing the fallback node to crash; wrap the llm.invoke call in a
try/except that catches Exception, log the exception (or attach a minimal safe
error string), and return a deterministic fallback response (e.g. a single
assistant/SystemMessage with a hardcoded apology/temporary-fallback text) in the
same {"messages": [...]} shape so the pipeline continues even if the LLM call
fails.


Comment on lines +11 to +42
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TODO comment suggests implementing a class for the fallback node and initializing the LLM in init. This is a valid concern since the current module-level LLM initialization is inconsistent with the lazy initialization pattern used in guard_inicial.py and guard_final.py. Consider addressing this TODO before merging, as it impacts the maintainability and consistency of the codebase.

Suggested change
llm = ChatOpenAI(
model="gpt-5-nano",
)
# TO DO: implementar clase nodo fallback y inicializar el llm en el init
def fallback(state: AgentState) -> AgentState:
"""
Fallback node - Performs fallback processing.
This node:
1. Alerts about malicious prompt or PII detection
2. Generates an error_message from llm to show the user
Args:
state: Agent state containing the prompt or initial context
Returns:
error_message
"""
# Check for PII/Risky content (from guard_final)
if state.get("is_risky"):
logger.warning(
"Defensive check triggered: PII/Risky content detected in response"
)
system_message_content = (
"Your job is to generate an error message in user's language explaining "
"that the response cannot be provided because it contains sensitive or private information."
)
# Check for Malicious prompt (from guard_inicial)
elif state.get("is_malicious"):
logger.warning(
"Defensive check triggered: Malicious prompt detected"
)
system_message_content = (
"Your job is to generate an error message in user's language for the user "
"explaining the database doesn't have the information to answer the user's question"
)
# Generic Fallback (neither risky nor malicious)
else:
logger.info(
"Fallback triggered: Generic fallback (no risky/malicious flag)"
)
system_message_content = (
"Your job is to generate an error message in user's language for the user "
"explaining the database doesn't have the information to answer the user's question"
)
messages = [
SystemMessage(content=system_message_content)
] + state["messages"]
error_message = llm.invoke(messages)
return {"messages": [error_message]}
class FallbackNode:
"""
Fallback node - Performs fallback processing.
This node:
1. Alerts about malicious prompt or PII detection
2. Generates an error_message from llm to show the user
"""
def __init__(self):
self.llm = ChatOpenAI(
model="gpt-5-nano",
)
def __call__(self, state: AgentState) -> AgentState:
"""
Args:
state: Agent state containing the prompt or initial context
Returns:
error_message
"""
# Check for PII/Risky content (from guard_final)
if state.get("is_risky"):
logger.warning(
"Defensive check triggered: PII/Risky content detected in response"
)
system_message_content = (
"Your job is to generate an error message in user's language explaining "
"that the response cannot be provided because it contains sensitive or private information."
)
# Check for Malicious prompt (from guard_inicial)
elif state.get("is_malicious"):
logger.warning(
"Defensive check triggered: Malicious prompt detected"
)
system_message_content = (
"Your job is to generate an error message in user's language for the user "
"explaining the database doesn't have the information to answer the user's question"
)
# Generic Fallback (neither risky nor malicious)
else:
logger.info(
"Fallback triggered: Generic fallback (no risky/malicious flag)"
)
system_message_content = (
"Your job is to generate an error message in user's language for the user "
"explaining the database doesn't have the information to answer the user's question"
)
messages = [
SystemMessage(content=system_message_content)
] + state["messages"]
error_message = self.llm.invoke(messages)
return {"messages": [error_message]}

Copilot uses AI. Check for mistakes.
40 changes: 0 additions & 40 deletions RAGManager/app/agents/nodes/fallback_final.py

This file was deleted.

48 changes: 0 additions & 48 deletions RAGManager/app/agents/nodes/fallback_inicial.py

This file was deleted.

31 changes: 0 additions & 31 deletions RAGManager/app/agents/nodes/generator.py

This file was deleted.

3 changes: 2 additions & 1 deletion RAGManager/app/agents/nodes/guard.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ def guard(state: AgentState) -> AgentState:
Updated state with is_malicious and error_message set
"""
updated_state = state.copy()
prompt = state.get("prompt", "")
last_message = state["messages"][-1]
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code accesses state["messages"][-1] without checking if the messages list exists or is non-empty, which could raise an IndexError if the state is not properly initialized. While the subsequent check handles empty prompt strings, it doesn't protect against a missing or empty messages list. Add a defensive check for the messages list before accessing it.

Suggested change
last_message = state["messages"][-1]
messages = state.get("messages")
last_message = messages[-1] if isinstance(messages, list) and messages else None

Copilot uses AI. Check for mistakes.
prompt = last_message.content if last_message else ""

Comment on lines 39 to 43
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Good shift to messages-based prompt + length-only logging; harden messages type and exception logging.
If state["messages"] can be non-list/None, treat it as empty. Also log exceptions with stack trace for ops.

     updated_state = state.copy()
-    messages = state.get("messages", [])
+    messages = state.get("messages")
+    if not isinstance(messages, list):
+        messages = []
     last_message = messages[-1] if messages else None
     prompt = last_message.content if last_message else ""
@@
-        logger.error(f"Error during jailbreak detection: {e}")
+        logger.exception("Error during jailbreak detection")

Also applies to: 67-75

if not prompt:
# Empty prompt is considered safe
Expand Down
19 changes: 15 additions & 4 deletions RAGManager/app/agents/nodes/parafraseo.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
"""Nodo 4: Parafraseo - Paraphrases user input."""

from app.agents.state import AgentState
from langchain_core.messages import SystemMessage
from langchain_openai import ChatOpenAI

llm = ChatOpenAI(model="gpt-5-nano")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Invalid model name gpt-5-nano.

gpt-5-nano is not a valid OpenAI model. This will cause a runtime error when invoking the LLM. Use a valid model such as gpt-4o-mini or gpt-3.5-turbo.

-llm = ChatOpenAI(model="gpt-5-nano")
+llm = ChatOpenAI(model="gpt-4o-mini")
📝 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
llm = ChatOpenAI(model="gpt-5-nano")
llm = ChatOpenAI(model="gpt-4o-mini")
🤖 Prompt for AI Agents
In RAGManager/app/agents/nodes/parafraseo.py around line 7, the ChatOpenAI
instantiation uses an invalid model name `gpt-5-nano`; replace it with a
supported model (for example `gpt-4o-mini` or `gpt-3.5-turbo`) by updating the
model parameter to a valid OpenAI model string, and ensure any other code or
config that depends on model-specific options is adjusted accordingly.

Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LLM is initialized at module level, which means it will be created immediately when the module is imported, even if the parafraseo function is never called. This could waste resources during startup or testing. Consider using lazy initialization with a function like _get_llm() similar to the pattern used in guard_inicial.py and guard_final.py.

Copilot uses AI. Check for mistakes.


def parafraseo(state: AgentState) -> AgentState:
Expand All @@ -24,9 +28,16 @@ def parafraseo(state: AgentState) -> AgentState:
# 2. Improve clarity, adjust tone, or format as needed
# 3. Set paraphrased_text with the result

# Placeholder: For now, we'll use the adjusted_text as-is
updated_state = state.copy()
text_to_paraphrase = state.get("adjusted_text") or state.get("prompt", "")
updated_state["paraphrased_text"] = text_to_paraphrase
# Paraphrase the last message using history

system_instruction = """You are an expert at paraphrasing user questions to be standalone and clear, given the conversation history.
Reformulate the last user message to be a self-contained query that includes necessary context from previous messages.
Do not answer the question, just rewrite it."""

messages = [SystemMessage(content=system_instruction)] + state["messages"]

response = llm.invoke(messages)
updated_state = state.copy() # Create a copy of the state to update
updated_state["paraphrased_text"] = response.content
Comment on lines +39 to +41
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add error handling for LLM invocation.

If llm.invoke() fails (e.g., network error, rate limit), the exception will propagate unhandled. Consider wrapping with try/except and setting an appropriate error state.

🤖 Prompt for AI Agents
In RAGManager/app/agents/nodes/parafraseo.py around lines 39-41, the call to
llm.invoke(messages) is unprotected and will raise uncaught exceptions; wrap the
invocation in a try/except block that catches broad LLM/network errors, log the
exception, and update a copied state with error details (e.g.,
updated_state["error"]="paraphrase_failed" and
updated_state["error_message"]=str(exc)) before returning or re-raising as
appropriate so the caller can handle failure gracefully.


return updated_state
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function creates an updated_state copy and sets 'paraphrased_text', but returns the entire updated_state object. However, context_builder returns a dict with just messages, and the graph expects nodes to return partial state updates when inheriting from MessagesState. For consistency with other nodes in this workflow, this should return a dict with the updated fields rather than the full state copy.

Suggested change
return updated_state
return {"paraphrased_text": response.content}

Copilot uses AI. Check for mistakes.
17 changes: 0 additions & 17 deletions RAGManager/app/agents/routing.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,3 @@ def route_after_guard(state: AgentState) -> str:
if state.get("is_malicious", False):
return "malicious"
return "continue"


def route_after_fallback_final(state: AgentState) -> str:
"""
Route after Fallback Final node (Nodo 8) validation.

Determines the next step based on whether the response was flagged as risky.

Args:
state: Current agent state

Returns:
"risky" if the response is risky, "continue" otherwise
"""
if state.get("is_risky", False):
return "risky"
return "continue"
Loading